Skip to content

SAFE_HEAP: remove fastcomp, prepare for new emscripten approach #3078

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Aug 25, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 19 additions & 15 deletions src/passes/SafeHeap.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,7 @@
namespace wasm {

static const Name DYNAMICTOP_PTR_IMPORT("DYNAMICTOP_PTR");
static const Name GET_SBRK_PTR_IMPORT("emscripten_get_sbrk_ptr");
static const Name GET_SBRK_PTR_EXPORT("_emscripten_get_sbrk_ptr");
static const Name GET_SBRK_PTR("emscripten_get_sbrk_ptr");
static const Name SBRK("sbrk");
static const Name SEGFAULT_IMPORT("segfault");
static const Name ALIGNFAULT_IMPORT("alignfault");
Expand Down Expand Up @@ -66,12 +65,21 @@ static Name getStoreName(Store* curr) {
}

struct AccessInstrumenter : public WalkerPass<PostWalker<AccessInstrumenter>> {
// If the getSbrkPtr function is implemented in the wasm, we must not
// instrument that, as it would lead to infinite recursion of it calling
// SAFE_HEAP_LOAD that calls it and so forth.
Name getSbrkPtr;

bool isFunctionParallel() override { return true; }

AccessInstrumenter* create() override { return new AccessInstrumenter; }
AccessInstrumenter* create() override {
return new AccessInstrumenter(getSbrkPtr);
}

AccessInstrumenter(Name getSbrkPtr) : getSbrkPtr(getSbrkPtr) {}

void visitLoad(Load* curr) {
if (curr->type == Type::unreachable) {
if (getFunction()->name == getSbrkPtr || curr->type == Type::unreachable) {
return;
}
Builder builder(*getModule());
Expand All @@ -85,7 +93,7 @@ struct AccessInstrumenter : public WalkerPass<PostWalker<AccessInstrumenter>> {
}

void visitStore(Store* curr) {
if (curr->type == Type::unreachable) {
if (getFunction()->name == getSbrkPtr || curr->type == Type::unreachable) {
return;
}
Builder builder(*getModule());
Expand All @@ -108,7 +116,7 @@ struct SafeHeap : public Pass {
// add imports
addImports(module);
// instrument loads and stores
AccessInstrumenter().run(runner, module);
AccessInstrumenter(getSbrkPtr).run(runner, module);
// add helper checking funcs and imports
addGlobals(module, module->features);
}
Expand All @@ -118,24 +126,20 @@ struct SafeHeap : public Pass {
void addImports(Module* module) {
ImportInfo info(*module);
// Older emscripten imports env.DYNAMICTOP_PTR.
// Newer emscripten imports emscripten_get_sbrk_ptr(), which is later
// optimized to have the number in the binary (or in the case of fastcomp,
// emscripten_get_sbrk_ptr is an asm.js library function so it is inside
// the wasm, and discoverable via an export).
// Newer emscripten imports or exports emscripten_get_sbrk_ptr().
if (auto* existing = info.getImportedGlobal(ENV, DYNAMICTOP_PTR_IMPORT)) {
dynamicTopPtr = existing->name;
} else if (auto* existing =
info.getImportedFunction(ENV, GET_SBRK_PTR_IMPORT)) {
} else if (auto* existing = info.getImportedFunction(ENV, GET_SBRK_PTR)) {
getSbrkPtr = existing->name;
} else if (auto* existing = module->getExportOrNull(GET_SBRK_PTR_EXPORT)) {
} else if (auto* existing = module->getExportOrNull(GET_SBRK_PTR)) {
getSbrkPtr = existing->value;
} else if (auto* existing = info.getImportedFunction(ENV, SBRK)) {
sbrk = existing->name;
} else {
auto* import = new Function;
import->name = getSbrkPtr = GET_SBRK_PTR_IMPORT;
import->name = getSbrkPtr = GET_SBRK_PTR;
import->module = ENV;
import->base = GET_SBRK_PTR_IMPORT;
import->base = GET_SBRK_PTR;
import->sig = Signature(Type::none, Type::i32);
module->addFunction(import);
}
Expand Down
7 changes: 6 additions & 1 deletion test/passes/safe-heap_disable-simd.txt
Original file line number Diff line number Diff line change
Expand Up @@ -5750,8 +5750,13 @@
(import "env" "segfault" (func $segfault))
(import "env" "alignfault" (func $alignfault))
(memory $0 1 1)
(export "_emscripten_get_sbrk_ptr" (func $foo))
(export "emscripten_get_sbrk_ptr" (func $foo))
(func $foo (result i32)
(drop
(i32.load
(i32.const 0)
)
)
(i32.const 1234)
)
(func $SAFE_HEAP_LOAD_i32_1_1 (param $0 i32) (param $1 i32) (result i32)
Expand Down
3 changes: 2 additions & 1 deletion test/passes/safe-heap_disable-simd.wast
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,9 @@
)
(module
(memory 1 1)
(export "_emscripten_get_sbrk_ptr" (func $foo))
(export "emscripten_get_sbrk_ptr" (func $foo))
(func $foo (result i32)
(drop (i32.load (i32.const 0))) ;; should not be modified!
(i32.const 1234)
)
)