From 9324f58434a45df416bac698c0a57557e70725e2 Mon Sep 17 00:00:00 2001 From: Guanzhong Chen Date: Thu, 1 Aug 2019 14:22:49 -0700 Subject: [PATCH 1/9] Implement --check-stack-overflow --- src/tools/wasm-emscripten-finalize.cpp | 12 ++++ src/wasm-emscripten.h | 4 ++ src/wasm/wasm-emscripten.cpp | 79 ++++++++++++++++++++++++++ 3 files changed, 95 insertions(+) diff --git a/src/tools/wasm-emscripten-finalize.cpp b/src/tools/wasm-emscripten-finalize.cpp index 56a6c3b3971..bb957d0e855 100644 --- a/src/tools/wasm-emscripten-finalize.cpp +++ b/src/tools/wasm-emscripten-finalize.cpp @@ -48,6 +48,7 @@ int main(int argc, const char* argv[]) { bool debugInfo = false; bool isSideModule = false; bool legalizeJavaScriptFFI = true; + bool checkStackOverflow = false; uint64_t globalBase = INVALID_BASE; ToolOptions options("wasm-emscripten-finalize", "Performs Emscripten-specific transforms on .wasm files"); @@ -127,6 +128,13 @@ int main(int argc, const char* argv[]) { [&dataSegmentFile](Options* o, const std::string& argument) { dataSegmentFile = argument; }) + .add("--check-stack-overflow", + "", + "Check for stack overflows every time the stack is extended", + Options::Arguments::Zero, + [&checkStackOverflow](Options* o, const std::string&) { + checkStackOverflow = true; + }) .add_positional("INFILE", Options::Arguments::One, [&infile](Options* o, const std::string& argument) { @@ -221,6 +229,10 @@ int main(int argc, const char* argv[]) { } } + if (checkStackOverflow) { + generator.enforceStackLimit(); + } + generator.generateDynCallThunks(); // Legalize the wasm. diff --git a/src/wasm-emscripten.h b/src/wasm-emscripten.h index 0841708f269..fe7a2571f14 100644 --- a/src/wasm-emscripten.h +++ b/src/wasm-emscripten.h @@ -54,6 +54,8 @@ class EmscriptenGlueGenerator { void fixInvokeFunctionNames(); + void enforceStackLimit(); + // Emits the data segments to a file. The file contains data from address base // onwards (we must pass in base, as we can't tell it from the wasm - the // first segment may start after a run of zeros, but we need those zeros in @@ -76,6 +78,8 @@ class EmscriptenGlueGenerator { void generateStackSaveFunction(); void generateStackAllocFunction(); void generateStackRestoreFunction(); + void generateSetStackLimitFunction(); + Name importStackOverflowHandler(); }; } // namespace wasm diff --git a/src/wasm/wasm-emscripten.cpp b/src/wasm/wasm-emscripten.cpp index 3ee3e44244d..9f793d7fd02 100644 --- a/src/wasm/wasm-emscripten.cpp +++ b/src/wasm/wasm-emscripten.cpp @@ -37,8 +37,11 @@ static Name STACK_SAVE("stackSave"); static Name STACK_RESTORE("stackRestore"); static Name STACK_ALLOC("stackAlloc"); static Name STACK_INIT("stack$init"); +static Name STACK_LIMIT("__stack_limit"); +static Name SET_STACK_LIMIT("__set_stack_limit"); static Name POST_INSTANTIATE("__post_instantiate"); static Name ASSIGN_GOT_ENTIRES("__assign_got_enties"); +static Name STACK_OVERFLOW_IMPORT("__handle_stack_overflow"); void addExportedFunction(Module& wasm, Function* function) { wasm.addFunction(function); @@ -444,6 +447,82 @@ void EmscriptenGlueGenerator::replaceStackPointerGlobal() { wasm.removeGlobal(stackPointer->name); } +struct StackLimitEnforcer : public PostWalker { + StackLimitEnforcer(Global* stackPointer, + Global* stackLimit, + Builder& builder, + Name handler) + : stackPointer(stackPointer), stackLimit(stackLimit), builder(builder), + handler(handler) {} + + void visitGlobalSet(GlobalSet* curr) { + if (getModule()->getGlobalOrNull(curr->name) == stackPointer) { + auto newSP = Builder::addVar(getFunction(), stackPointer->type); + auto teeNewSP = builder.makeLocalTee(newSP, curr->value); + auto check = builder.makeIf( + builder.makeBinary( + BinaryOp::LtUInt32, + teeNewSP, + builder.makeGlobalGet(stackLimit->name, stackLimit->type)), + builder.makeCall(handler, {}, none)); + auto newSet = builder.makeGlobalSet( + stackPointer->name, builder.makeLocalGet(newSP, stackPointer->type)); + replaceCurrent(builder.blockify(check, newSet)); + } + } + +private: + Global* stackPointer; + Global* stackLimit; + Builder& builder; + Name handler; +}; + +void EmscriptenGlueGenerator::enforceStackLimit() { + Global* stackPointer = getStackPointerGlobal(); + + auto* stackLimit = builder.makeGlobal(STACK_LIMIT, + stackPointer->type, + builder.makeConst(Literal(0)), + Builder::Mutable); + wasm.addGlobal(stackLimit); + + auto handler = importStackOverflowHandler(); + + StackLimitEnforcer walker(stackPointer, stackLimit, builder, handler); + walker.walkModule(&wasm); + + generateSetStackLimitFunction(); +} + +void EmscriptenGlueGenerator::generateSetStackLimitFunction() { + std::vector params{{"0", i32}}; + Function* function = + builder.makeFunction(SET_STACK_LIMIT, std::move(params), none, {}); + LocalGet* getArg = builder.makeLocalGet(0, i32); + Expression* store = builder.makeGlobalSet(STACK_LIMIT, getArg); + function->body = store; + addExportedFunction(wasm, function); +} + +Name EmscriptenGlueGenerator::importStackOverflowHandler() { + ImportInfo info(wasm); + + if (auto* existing = info.getImportedFunction(ENV, STACK_OVERFLOW_IMPORT)) { + return existing->name; + } else { + auto* import = new Function; + import->name = STACK_OVERFLOW_IMPORT; + import->module = ENV; + import->base = STACK_OVERFLOW_IMPORT; + auto* functionType = ensureFunctionType("v", &wasm); + import->type = functionType->name; + FunctionTypeUtils::fillFunction(import, functionType); + wasm.addFunction(import); + return STACK_OVERFLOW_IMPORT; + } +} + const Address UNKNOWN_OFFSET(uint32_t(-1)); std::vector
getSegmentOffsets(Module& wasm) { From ae72862d302300d7394faa076ccd8940614f50fe Mon Sep 17 00:00:00 2001 From: Guanzhong Chen Date: Thu, 1 Aug 2019 14:31:41 -0700 Subject: [PATCH 2/9] Only check stack pointer subtractions --- src/wasm/wasm-emscripten.cpp | 28 +++++++++++++++++++++++++++- 1 file changed, 27 insertions(+), 1 deletion(-) diff --git a/src/wasm/wasm-emscripten.cpp b/src/wasm/wasm-emscripten.cpp index 9f793d7fd02..0b29943087f 100644 --- a/src/wasm/wasm-emscripten.cpp +++ b/src/wasm/wasm-emscripten.cpp @@ -455,8 +455,31 @@ struct StackLimitEnforcer : public PostWalker { : stackPointer(stackPointer), stackLimit(stackLimit), builder(builder), handler(handler) {} + void noteNonLinear(Expression* curr) { + // End of this basic block; clear sets. + sets.clear(); + } + + void visitLocalSet(LocalSet* curr) { sets[curr->index] = curr; } + + bool canBeSubtract(Expression* arg) { + while (auto* get = arg->dynCast()) { + auto* set = sets[get->index]; + if (set) { + assert(set->index == get->index); + arg = set->value; + } + } + if (auto* binary = arg->dynCast()) { + return binary->op == SubInt32; + } else { + return true; + } + } + void visitGlobalSet(GlobalSet* curr) { - if (getModule()->getGlobalOrNull(curr->name) == stackPointer) { + if (getModule()->getGlobalOrNull(curr->name) == stackPointer && + canBeSubtract(curr->value)) { auto newSP = Builder::addVar(getFunction(), stackPointer->type); auto teeNewSP = builder.makeLocalTee(newSP, curr->value); auto check = builder.makeIf( @@ -476,6 +499,9 @@ struct StackLimitEnforcer : public PostWalker { Global* stackLimit; Builder& builder; Name handler; + + // last sets in the current basic block, per index + std::map sets; }; void EmscriptenGlueGenerator::enforceStackLimit() { From beb4a808ce14f123d33152fbe9bb4df98cb38a70 Mon Sep 17 00:00:00 2001 From: Guanzhong Chen Date: Thu, 1 Aug 2019 14:50:25 -0700 Subject: [PATCH 3/9] Add tests --- scripts/test/lld.py | 10 +- test/lld/recursive_safe_stack.wast | 90 ++++++++++ test/lld/recursive_safe_stack.wast.out | 220 +++++++++++++++++++++++++ 3 files changed, 316 insertions(+), 4 deletions(-) create mode 100644 test/lld/recursive_safe_stack.wast create mode 100644 test/lld/recursive_safe_stack.wast.out diff --git a/scripts/test/lld.py b/scripts/test/lld.py index fc68a462076..1a553506fd1 100755 --- a/scripts/test/lld.py +++ b/scripts/test/lld.py @@ -22,10 +22,12 @@ def args_for_finalize(filename): - if 'shared' in filename: - return ['--side-module'] - else: - return ['--global-base=568'] + if 'safe_stack' in filename: + return ['--check-stack-overflow', '--global-base=568'] + elif 'shared' in filename: + return ['--side-module'] + else: + return ['--global-base=568'] def test_wasm_emscripten_finalize(): diff --git a/test/lld/recursive_safe_stack.wast b/test/lld/recursive_safe_stack.wast new file mode 100644 index 00000000000..67f7f3914a0 --- /dev/null +++ b/test/lld/recursive_safe_stack.wast @@ -0,0 +1,90 @@ +(module + (type $0 (func (param i32 i32) (result i32))) + (type $1 (func)) + (type $2 (func (result i32))) + (import "env" "printf" (func $printf (param i32 i32) (result i32))) + (memory $0 2) + (data (i32.const 568) "%d:%d\n\00Result: %d\n\00") + (table $0 1 1 funcref) + (global $global$0 (mut i32) (i32.const 66128)) + (global $global$1 i32 (i32.const 66128)) + (global $global$2 i32 (i32.const 587)) + (export "memory" (memory $0)) + (export "__wasm_call_ctors" (func $__wasm_call_ctors)) + (export "__heap_base" (global $global$1)) + (export "__data_end" (global $global$2)) + (export "main" (func $main)) + (func $__wasm_call_ctors (; 1 ;) (type $1) + ) + (func $foo (; 2 ;) (type $0) (param $0 i32) (param $1 i32) (result i32) + (local $2 i32) + (global.set $global$0 + (local.tee $2 + (i32.sub + (global.get $global$0) + (i32.const 16) + ) + ) + ) + (i32.store offset=4 + (local.get $2) + (local.get $1) + ) + (i32.store + (local.get $2) + (local.get $0) + ) + (drop + (call $printf + (i32.const 568) + (local.get $2) + ) + ) + (global.set $global$0 + (i32.add + (local.get $2) + (i32.const 16) + ) + ) + (i32.add + (local.get $1) + (local.get $0) + ) + ) + (func $__original_main (; 3 ;) (type $2) (result i32) + (local $0 i32) + (global.set $global$0 + (local.tee $0 + (i32.sub + (global.get $global$0) + (i32.const 16) + ) + ) + ) + (i32.store + (local.get $0) + (call $foo + (i32.const 1) + (i32.const 2) + ) + ) + (drop + (call $printf + (i32.const 575) + (local.get $0) + ) + ) + (global.set $global$0 + (i32.add + (local.get $0) + (i32.const 16) + ) + ) + (i32.const 0) + ) + (func $main (; 4 ;) (type $0) (param $0 i32) (param $1 i32) (result i32) + (call $__original_main) + ) + ;; custom section "producers", size 111 +) + diff --git a/test/lld/recursive_safe_stack.wast.out b/test/lld/recursive_safe_stack.wast.out new file mode 100644 index 00000000000..a587184e43a --- /dev/null +++ b/test/lld/recursive_safe_stack.wast.out @@ -0,0 +1,220 @@ +(module + (type $0 (func (param i32 i32) (result i32))) + (type $1 (func)) + (type $2 (func (result i32))) + (type $FUNCSIG$v (func)) + (import "env" "printf" (func $printf (param i32 i32) (result i32))) + (import "env" "__handle_stack_overflow" (func $__handle_stack_overflow)) + (memory $0 2) + (data (i32.const 568) "%d:%d\n\00Result: %d\n\00") + (table $0 1 1 funcref) + (global $global$0 (mut i32) (i32.const 66128)) + (global $global$1 i32 (i32.const 66128)) + (global $global$2 i32 (i32.const 587)) + (global $__stack_limit (mut i32) (i32.const 0)) + (export "memory" (memory $0)) + (export "__wasm_call_ctors" (func $__wasm_call_ctors)) + (export "__heap_base" (global $global$1)) + (export "__data_end" (global $global$2)) + (export "main" (func $main)) + (export "stackSave" (func $stackSave)) + (export "stackAlloc" (func $stackAlloc)) + (export "stackRestore" (func $stackRestore)) + (export "__growWasmMemory" (func $__growWasmMemory)) + (export "__set_stack_limit" (func $__set_stack_limit)) + (func $__wasm_call_ctors (; 2 ;) (type $1) + (nop) + ) + (func $foo (; 3 ;) (type $0) (param $0 i32) (param $1 i32) (result i32) + (local $2 i32) + (local $3 i32) + (block + (if + (i32.lt_u + (local.tee $3 + (local.tee $2 + (i32.sub + (global.get $global$0) + (i32.const 16) + ) + ) + ) + (global.get $__stack_limit) + ) + (call $__handle_stack_overflow) + ) + (global.set $global$0 + (local.get $3) + ) + ) + (i32.store offset=4 + (local.get $2) + (local.get $1) + ) + (i32.store + (local.get $2) + (local.get $0) + ) + (drop + (call $printf + (i32.const 568) + (local.get $2) + ) + ) + (global.set $global$0 + (i32.add + (local.get $2) + (i32.const 16) + ) + ) + (i32.add + (local.get $1) + (local.get $0) + ) + ) + (func $__original_main (; 4 ;) (type $2) (result i32) + (local $0 i32) + (local $1 i32) + (block + (if + (i32.lt_u + (local.tee $1 + (local.tee $0 + (i32.sub + (global.get $global$0) + (i32.const 16) + ) + ) + ) + (global.get $__stack_limit) + ) + (call $__handle_stack_overflow) + ) + (global.set $global$0 + (local.get $1) + ) + ) + (i32.store + (local.get $0) + (call $foo + (i32.const 1) + (i32.const 2) + ) + ) + (drop + (call $printf + (i32.const 575) + (local.get $0) + ) + ) + (global.set $global$0 + (i32.add + (local.get $0) + (i32.const 16) + ) + ) + (i32.const 0) + ) + (func $main (; 5 ;) (type $0) (param $0 i32) (param $1 i32) (result i32) + (call $__original_main) + ) + (func $stackSave (; 6 ;) (result i32) + (global.get $global$0) + ) + (func $stackAlloc (; 7 ;) (param $0 i32) (result i32) + (local $1 i32) + (local $2 i32) + (block + (if + (i32.lt_u + (local.tee $2 + (local.tee $1 + (i32.and + (i32.sub + (global.get $global$0) + (local.get $0) + ) + (i32.const -16) + ) + ) + ) + (global.get $__stack_limit) + ) + (call $__handle_stack_overflow) + ) + (global.set $global$0 + (local.get $2) + ) + ) + (local.get $1) + ) + (func $stackRestore (; 8 ;) (param $0 i32) + (local $1 i32) + (if + (i32.lt_u + (local.tee $1 + (local.get $0) + ) + (global.get $__stack_limit) + ) + (call $__handle_stack_overflow) + ) + (global.set $global$0 + (local.get $1) + ) + ) + (func $__growWasmMemory (; 9 ;) (param $newSize i32) (result i32) + (memory.grow + (local.get $newSize) + ) + ) + (func $__set_stack_limit (; 10 ;) (param $0 i32) + (global.set $__stack_limit + (local.get $0) + ) + ) +) +(; +--BEGIN METADATA -- +{ + "staticBump": 19, + "tableSize": 1, + "initializers": [ + "__wasm_call_ctors" + ], + "declares": [ + "printf", + "__handle_stack_overflow" + ], + "externs": [ + ], + "implementedFunctions": [ + "___wasm_call_ctors", + "_main", + "_stackSave", + "_stackAlloc", + "_stackRestore", + "___growWasmMemory", + "___set_stack_limit" + ], + "exports": [ + "__wasm_call_ctors", + "main", + "stackSave", + "stackAlloc", + "stackRestore", + "__growWasmMemory", + "__set_stack_limit" + ], + "namedGlobals": { + "__heap_base" : "66128", + "__data_end" : "587" + }, + "invokeFuncs": [ + ], + "features": [ + ], + "mainReadsParams": 0 +} +-- END METADATA -- +;) From c0606fda116e5653245997872fa25d26093e3d25 Mon Sep 17 00:00:00 2001 From: Guanzhong Chen Date: Thu, 1 Aug 2019 15:40:55 -0700 Subject: [PATCH 4/9] Support dynamic linking --- src/tools/wasm-emscripten-finalize.cpp | 8 ++-- src/wasm-emscripten.h | 2 +- src/wasm/wasm-emscripten.cpp | 54 +++++++++++++++++++------- test/lld/recursive_safe_stack.wast.out | 28 ++++++------- 4 files changed, 59 insertions(+), 33 deletions(-) diff --git a/src/tools/wasm-emscripten-finalize.cpp b/src/tools/wasm-emscripten-finalize.cpp index bb957d0e855..f5afb3e2414 100644 --- a/src/tools/wasm-emscripten-finalize.cpp +++ b/src/tools/wasm-emscripten-finalize.cpp @@ -208,6 +208,10 @@ int main(int argc, const char* argv[]) { } wasm.updateMaps(); + if (checkStackOverflow && !isSideModule) { + generator.enforceStackLimit(); + } + if (isSideModule) { generator.replaceStackPointerGlobal(); generator.generatePostInstantiateFunction(); @@ -229,10 +233,6 @@ int main(int argc, const char* argv[]) { } } - if (checkStackOverflow) { - generator.enforceStackLimit(); - } - generator.generateDynCallThunks(); // Legalize the wasm. diff --git a/src/wasm-emscripten.h b/src/wasm-emscripten.h index fe7a2571f14..1758aa23a00 100644 --- a/src/wasm-emscripten.h +++ b/src/wasm-emscripten.h @@ -73,7 +73,7 @@ class EmscriptenGlueGenerator { Global* getStackPointerGlobal(); Expression* generateLoadStackPointer(); - Expression* generateStoreStackPointer(Expression* value); + Expression* generateStoreStackPointer(Function* func, Expression* value); void generateDynCallThunk(std::string sig); void generateStackSaveFunction(); void generateStackAllocFunction(); diff --git a/src/wasm/wasm-emscripten.cpp b/src/wasm/wasm-emscripten.cpp index 0b29943087f..bf760966203 100644 --- a/src/wasm/wasm-emscripten.cpp +++ b/src/wasm/wasm-emscripten.cpp @@ -95,8 +95,28 @@ Expression* EmscriptenGlueGenerator::generateLoadStackPointer() { return builder.makeGlobalGet(stackPointer->name, i32); } +inline Expression* stackBoundsCheck(Builder& builder, + Function* func, + Expression* value, + Global* stackPointer, + Global* stackLimit, + Name handler) { + auto newSP = Builder::addVar(func, stackPointer->type); + auto teeNewSP = builder.makeLocalTee(newSP, value); + auto check = + builder.makeIf(builder.makeBinary( + BinaryOp::LtUInt32, + teeNewSP, + builder.makeGlobalGet(stackLimit->name, stackLimit->type)), + builder.makeCall(handler, {}, none)); + auto newSet = builder.makeGlobalSet( + stackPointer->name, builder.makeLocalGet(newSP, stackPointer->type)); + return builder.blockify(check, newSet); +} + Expression* -EmscriptenGlueGenerator::generateStoreStackPointer(Expression* value) { +EmscriptenGlueGenerator::generateStoreStackPointer(Function* func, + Expression* value) { if (!useStackPointerGlobal) { return builder.makeStore( /* bytes =*/4, @@ -110,6 +130,14 @@ EmscriptenGlueGenerator::generateStoreStackPointer(Expression* value) { if (!stackPointer) { Fatal() << "stack pointer global not found"; } + if (auto* stackLimit = wasm.getGlobalOrNull(STACK_LIMIT)) { + return stackBoundsCheck(builder, + func, + value, + stackPointer, + stackLimit, + importStackOverflowHandler()); + } return builder.makeGlobalSet(stackPointer->name, value); } @@ -135,7 +163,7 @@ void EmscriptenGlueGenerator::generateStackAllocFunction() { Const* subConst = builder.makeConst(Literal(~bitMask)); Binary* maskedSub = builder.makeBinary(AndInt32, sub, subConst); LocalSet* teeStackLocal = builder.makeLocalTee(1, maskedSub); - Expression* storeStack = generateStoreStackPointer(teeStackLocal); + Expression* storeStack = generateStoreStackPointer(function, teeStackLocal); Block* block = builder.makeBlock(); block->list.push_back(storeStack); @@ -152,7 +180,7 @@ void EmscriptenGlueGenerator::generateStackRestoreFunction() { Function* function = builder.makeFunction(STACK_RESTORE, std::move(params), none, {}); LocalGet* getArg = builder.makeLocalGet(0, i32); - Expression* store = generateStoreStackPointer(getArg); + Expression* store = generateStoreStackPointer(function, getArg); function->body = store; @@ -480,17 +508,12 @@ struct StackLimitEnforcer : public PostWalker { void visitGlobalSet(GlobalSet* curr) { if (getModule()->getGlobalOrNull(curr->name) == stackPointer && canBeSubtract(curr->value)) { - auto newSP = Builder::addVar(getFunction(), stackPointer->type); - auto teeNewSP = builder.makeLocalTee(newSP, curr->value); - auto check = builder.makeIf( - builder.makeBinary( - BinaryOp::LtUInt32, - teeNewSP, - builder.makeGlobalGet(stackLimit->name, stackLimit->type)), - builder.makeCall(handler, {}, none)); - auto newSet = builder.makeGlobalSet( - stackPointer->name, builder.makeLocalGet(newSP, stackPointer->type)); - replaceCurrent(builder.blockify(check, newSet)); + replaceCurrent(stackBoundsCheck(builder, + getFunction(), + curr->value, + stackPointer, + stackLimit, + handler)); } } @@ -506,6 +529,9 @@ struct StackLimitEnforcer : public PostWalker { void EmscriptenGlueGenerator::enforceStackLimit() { Global* stackPointer = getStackPointerGlobal(); + if (!stackPointer) { + return; + } auto* stackLimit = builder.makeGlobal(STACK_LIMIT, stackPointer->type, diff --git a/test/lld/recursive_safe_stack.wast.out b/test/lld/recursive_safe_stack.wast.out index a587184e43a..80095b05a19 100644 --- a/test/lld/recursive_safe_stack.wast.out +++ b/test/lld/recursive_safe_stack.wast.out @@ -17,11 +17,11 @@ (export "__heap_base" (global $global$1)) (export "__data_end" (global $global$2)) (export "main" (func $main)) + (export "__set_stack_limit" (func $__set_stack_limit)) (export "stackSave" (func $stackSave)) (export "stackAlloc" (func $stackAlloc)) (export "stackRestore" (func $stackRestore)) (export "__growWasmMemory" (func $__growWasmMemory)) - (export "__set_stack_limit" (func $__set_stack_limit)) (func $__wasm_call_ctors (; 2 ;) (type $1) (nop) ) @@ -118,10 +118,15 @@ (func $main (; 5 ;) (type $0) (param $0 i32) (param $1 i32) (result i32) (call $__original_main) ) - (func $stackSave (; 6 ;) (result i32) + (func $__set_stack_limit (; 6 ;) (param $0 i32) + (global.set $__stack_limit + (local.get $0) + ) + ) + (func $stackSave (; 7 ;) (result i32) (global.get $global$0) ) - (func $stackAlloc (; 7 ;) (param $0 i32) (result i32) + (func $stackAlloc (; 8 ;) (param $0 i32) (result i32) (local $1 i32) (local $2 i32) (block @@ -148,7 +153,7 @@ ) (local.get $1) ) - (func $stackRestore (; 8 ;) (param $0 i32) + (func $stackRestore (; 9 ;) (param $0 i32) (local $1 i32) (if (i32.lt_u @@ -163,16 +168,11 @@ (local.get $1) ) ) - (func $__growWasmMemory (; 9 ;) (param $newSize i32) (result i32) + (func $__growWasmMemory (; 10 ;) (param $newSize i32) (result i32) (memory.grow (local.get $newSize) ) ) - (func $__set_stack_limit (; 10 ;) (param $0 i32) - (global.set $__stack_limit - (local.get $0) - ) - ) ) (; --BEGIN METADATA -- @@ -191,20 +191,20 @@ "implementedFunctions": [ "___wasm_call_ctors", "_main", + "___set_stack_limit", "_stackSave", "_stackAlloc", "_stackRestore", - "___growWasmMemory", - "___set_stack_limit" + "___growWasmMemory" ], "exports": [ "__wasm_call_ctors", "main", + "__set_stack_limit", "stackSave", "stackAlloc", "stackRestore", - "__growWasmMemory", - "__set_stack_limit" + "__growWasmMemory" ], "namedGlobals": { "__heap_base" : "66128", From 35e97d76196c625a50259a1b37ab64b8edb2cc6c Mon Sep 17 00:00:00 2001 From: Guanzhong Chen Date: Thu, 1 Aug 2019 17:50:16 -0700 Subject: [PATCH 5/9] Make the code easier to understand --- src/wasm/wasm-emscripten.cpp | 24 +++++++++++++++++------- 1 file changed, 17 insertions(+), 7 deletions(-) diff --git a/src/wasm/wasm-emscripten.cpp b/src/wasm/wasm-emscripten.cpp index bf760966203..2eeafca421d 100644 --- a/src/wasm/wasm-emscripten.cpp +++ b/src/wasm/wasm-emscripten.cpp @@ -101,14 +101,18 @@ inline Expression* stackBoundsCheck(Builder& builder, Global* stackPointer, Global* stackLimit, Name handler) { + // Add a local to store the value of the expression. We need the value twice: + // once to check if it has overflowed, and again to assign to store it. auto newSP = Builder::addVar(func, stackPointer->type); - auto teeNewSP = builder.makeLocalTee(newSP, value); + // (if (i32.lt_u (local.tee $newSP (...value...)) (global.get $__stack_limit)) + // (call $handler)) auto check = builder.makeIf(builder.makeBinary( BinaryOp::LtUInt32, - teeNewSP, + builder.makeLocalTee(newSP, value), builder.makeGlobalGet(stackLimit->name, stackLimit->type)), builder.makeCall(handler, {}, none)); + // (global.set $__stack_pointer (local.get $newSP)) auto newSet = builder.makeGlobalSet( stackPointer->name, builder.makeLocalGet(newSP, stackPointer->type)); return builder.blockify(check, newSet); @@ -490,7 +494,10 @@ struct StackLimitEnforcer : public PostWalker { void visitLocalSet(LocalSet* curr) { sets[curr->index] = curr; } - bool canBeSubtract(Expression* arg) { + // Checks if an expression is addition. + // If we are adding to the stack pointer, for example, in the function + // epilogue, overflow is impossible. + bool isAddition(Expression* arg) { while (auto* get = arg->dynCast()) { auto* set = sets[get->index]; if (set) { @@ -498,16 +505,19 @@ struct StackLimitEnforcer : public PostWalker { arg = set->value; } } - if (auto* binary = arg->dynCast()) { - return binary->op == SubInt32; - } else { + + if (arg->is() || arg->is()) { return true; + } else if (auto* binary = arg->dynCast()) { + return binary->op == AddInt32; + } else { + return false; } } void visitGlobalSet(GlobalSet* curr) { if (getModule()->getGlobalOrNull(curr->name) == stackPointer && - canBeSubtract(curr->value)) { + !isAddition(curr->value)) { replaceCurrent(stackBoundsCheck(builder, getFunction(), curr->value, From 2dbc6347919625090bdd4f5b45c5d4aa91478abc Mon Sep 17 00:00:00 2001 From: Guanzhong Chen Date: Thu, 1 Aug 2019 17:51:54 -0700 Subject: [PATCH 6/9] Avoid NameType --- src/wasm/wasm-emscripten.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/wasm/wasm-emscripten.cpp b/src/wasm/wasm-emscripten.cpp index 2eeafca421d..6c8def6a6d3 100644 --- a/src/wasm/wasm-emscripten.cpp +++ b/src/wasm/wasm-emscripten.cpp @@ -558,9 +558,8 @@ void EmscriptenGlueGenerator::enforceStackLimit() { } void EmscriptenGlueGenerator::generateSetStackLimitFunction() { - std::vector params{{"0", i32}}; Function* function = - builder.makeFunction(SET_STACK_LIMIT, std::move(params), none, {}); + builder.makeFunction(SET_STACK_LIMIT, {i32}, none, {}); LocalGet* getArg = builder.makeLocalGet(0, i32); Expression* store = builder.makeGlobalSet(STACK_LIMIT, getArg); function->body = store; From 21d3863bbde8ee1efdf04616c4f4bf20ff77c0e0 Mon Sep 17 00:00:00 2001 From: Guanzhong Chen Date: Fri, 2 Aug 2019 10:50:55 -0700 Subject: [PATCH 7/9] Parallel processing and modify even epilogue --- src/wasm/wasm-emscripten.cpp | 39 +++---------------------- test/lld/recursive_safe_stack.wast.out | 40 ++++++++++++++++++++------ 2 files changed, 36 insertions(+), 43 deletions(-) diff --git a/src/wasm/wasm-emscripten.cpp b/src/wasm/wasm-emscripten.cpp index 6c8def6a6d3..9f0f847571b 100644 --- a/src/wasm/wasm-emscripten.cpp +++ b/src/wasm/wasm-emscripten.cpp @@ -479,7 +479,7 @@ void EmscriptenGlueGenerator::replaceStackPointerGlobal() { wasm.removeGlobal(stackPointer->name); } -struct StackLimitEnforcer : public PostWalker { +struct StackLimitEnforcer : public WalkerPass> { StackLimitEnforcer(Global* stackPointer, Global* stackLimit, Builder& builder, @@ -487,37 +487,10 @@ struct StackLimitEnforcer : public PostWalker { : stackPointer(stackPointer), stackLimit(stackLimit), builder(builder), handler(handler) {} - void noteNonLinear(Expression* curr) { - // End of this basic block; clear sets. - sets.clear(); - } - - void visitLocalSet(LocalSet* curr) { sets[curr->index] = curr; } - - // Checks if an expression is addition. - // If we are adding to the stack pointer, for example, in the function - // epilogue, overflow is impossible. - bool isAddition(Expression* arg) { - while (auto* get = arg->dynCast()) { - auto* set = sets[get->index]; - if (set) { - assert(set->index == get->index); - arg = set->value; - } - } - - if (arg->is() || arg->is()) { - return true; - } else if (auto* binary = arg->dynCast()) { - return binary->op == AddInt32; - } else { - return false; - } - } + bool isFunctionParallel() override { return true; } void visitGlobalSet(GlobalSet* curr) { - if (getModule()->getGlobalOrNull(curr->name) == stackPointer && - !isAddition(curr->value)) { + if (getModule()->getGlobalOrNull(curr->name) == stackPointer) { replaceCurrent(stackBoundsCheck(builder, getFunction(), curr->value, @@ -532,9 +505,6 @@ struct StackLimitEnforcer : public PostWalker { Global* stackLimit; Builder& builder; Name handler; - - // last sets in the current basic block, per index - std::map sets; }; void EmscriptenGlueGenerator::enforceStackLimit() { @@ -558,8 +528,7 @@ void EmscriptenGlueGenerator::enforceStackLimit() { } void EmscriptenGlueGenerator::generateSetStackLimitFunction() { - Function* function = - builder.makeFunction(SET_STACK_LIMIT, {i32}, none, {}); + Function* function = builder.makeFunction(SET_STACK_LIMIT, {i32}, none, {}); LocalGet* getArg = builder.makeLocalGet(0, i32); Expression* store = builder.makeGlobalSet(STACK_LIMIT, getArg); function->body = store; diff --git a/test/lld/recursive_safe_stack.wast.out b/test/lld/recursive_safe_stack.wast.out index 80095b05a19..7253a28fb30 100644 --- a/test/lld/recursive_safe_stack.wast.out +++ b/test/lld/recursive_safe_stack.wast.out @@ -28,6 +28,7 @@ (func $foo (; 3 ;) (type $0) (param $0 i32) (param $1 i32) (result i32) (local $2 i32) (local $3 i32) + (local $4 i32) (block (if (i32.lt_u @@ -61,10 +62,21 @@ (local.get $2) ) ) - (global.set $global$0 - (i32.add - (local.get $2) - (i32.const 16) + (block + (if + (i32.lt_u + (local.tee $4 + (i32.add + (local.get $2) + (i32.const 16) + ) + ) + (global.get $__stack_limit) + ) + (call $__handle_stack_overflow) + ) + (global.set $global$0 + (local.get $4) ) ) (i32.add @@ -75,6 +87,7 @@ (func $__original_main (; 4 ;) (type $2) (result i32) (local $0 i32) (local $1 i32) + (local $2 i32) (block (if (i32.lt_u @@ -107,10 +120,21 @@ (local.get $0) ) ) - (global.set $global$0 - (i32.add - (local.get $0) - (i32.const 16) + (block + (if + (i32.lt_u + (local.tee $2 + (i32.add + (local.get $0) + (i32.const 16) + ) + ) + (global.get $__stack_limit) + ) + (call $__handle_stack_overflow) + ) + (global.set $global$0 + (local.get $2) ) ) (i32.const 0) From 151b5859085e82f8f4a5b4272137ef49f3e9aee2 Mon Sep 17 00:00:00 2001 From: Guanzhong Chen Date: Fri, 2 Aug 2019 13:32:01 -0700 Subject: [PATCH 8/9] Deal with GCC's intolerance --- src/wasm/wasm-emscripten.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/wasm/wasm-emscripten.cpp b/src/wasm/wasm-emscripten.cpp index 9f0f847571b..3d074b5a7f6 100644 --- a/src/wasm/wasm-emscripten.cpp +++ b/src/wasm/wasm-emscripten.cpp @@ -528,7 +528,8 @@ void EmscriptenGlueGenerator::enforceStackLimit() { } void EmscriptenGlueGenerator::generateSetStackLimitFunction() { - Function* function = builder.makeFunction(SET_STACK_LIMIT, {i32}, none, {}); + Function* function = + builder.makeFunction(SET_STACK_LIMIT, std::vector({i32}), none, {}); LocalGet* getArg = builder.makeLocalGet(0, i32); Expression* store = builder.makeGlobalSet(STACK_LIMIT, getArg); function->body = store; From 5c9f6d9db86f19bd751ed7235ae23829d54a95b2 Mon Sep 17 00:00:00 2001 From: Guanzhong Chen Date: Fri, 2 Aug 2019 13:36:41 -0700 Subject: [PATCH 9/9] Use passes properly --- src/wasm/wasm-emscripten.cpp | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/wasm/wasm-emscripten.cpp b/src/wasm/wasm-emscripten.cpp index 3d074b5a7f6..c21016baa06 100644 --- a/src/wasm/wasm-emscripten.cpp +++ b/src/wasm/wasm-emscripten.cpp @@ -489,6 +489,10 @@ struct StackLimitEnforcer : public WalkerPass> { bool isFunctionParallel() override { return true; } + Pass* create() override { + return new StackLimitEnforcer(stackPointer, stackLimit, builder, handler); + } + void visitGlobalSet(GlobalSet* curr) { if (getModule()->getGlobalOrNull(curr->name) == stackPointer) { replaceCurrent(stackBoundsCheck(builder, @@ -522,7 +526,8 @@ void EmscriptenGlueGenerator::enforceStackLimit() { auto handler = importStackOverflowHandler(); StackLimitEnforcer walker(stackPointer, stackLimit, builder, handler); - walker.walkModule(&wasm); + PassRunner runner(&wasm); + walker.run(&runner, &wasm); generateSetStackLimitFunction(); }