diff options
author | Alon Zakai <azakai@google.com> | 2022-12-07 09:59:57 -0800 |
---|---|---|
committer | GitHub <noreply@github.com> | 2022-12-07 09:59:57 -0800 |
commit | 01fbf2287ed7f2d0f31f8c255b02ae1fc77ad9ca (patch) | |
tree | 1a037615111b42f97334b63f514405a3fd4a2c54 /src | |
parent | 7769139efbe818c7ba36d1a382db5114ebee9df8 (diff) | |
download | binaryen-01fbf2287ed7f2d0f31f8c255b02ae1fc77ad9ca.tar.gz binaryen-01fbf2287ed7f2d0f31f8c255b02ae1fc77ad9ca.tar.bz2 binaryen-01fbf2287ed7f2d0f31f8c255b02ae1fc77ad9ca.zip |
Fix Asyncify assertions after #5293 (#5328)
Followup to #5293, this fixes a small regression there regarding assertions. We do have
a need to visit non-instrumented functions if we want assertions, as we assert on some
things there, namely that such functions do not change the state (if they changed it,
we'd need to instrument them to handle that properly).
This moves that logic into a new pass. We run that pass when assertions are enabled.
Test diff basically undoes part the test diff from that earlier PR for that one file.
Diffstat (limited to 'src')
-rw-r--r-- | src/passes/Asyncify.cpp | 53 |
1 files changed, 49 insertions, 4 deletions
diff --git a/src/passes/Asyncify.cpp b/src/passes/Asyncify.cpp index 365891db4..7b2d57574 100644 --- a/src/passes/Asyncify.cpp +++ b/src/passes/Asyncify.cpp @@ -921,9 +921,6 @@ struct AsyncifyFlow : public Pass { // If the function cannot change our state, we have nothing to do - // we will never unwind or rewind the stack here. if (!analyzer->needsInstrumentation(func)) { - if (analyzer->asserts) { - addAssertsInNonInstrumented(func); - } return; } // Rewrite the function body. @@ -947,7 +944,6 @@ struct AsyncifyFlow : public Pass { private: std::unique_ptr<AsyncifyBuilder> builder; - Module* module; Function* func; @@ -1216,6 +1212,41 @@ private: // don't want it to be seen by asyncify itself. return builder->makeCall(ASYNCIFY_GET_CALL_INDEX, {}, Type::none); } +}; + +// Add asserts in non-instrumented code. +struct AsyncifyAssertInNonInstrumented : public Pass { + bool isFunctionParallel() override { return true; } + + ModuleAnalyzer* analyzer; + Type pointerType; + Name asyncifyMemory; + + std::unique_ptr<Pass> create() override { + return std::make_unique<AsyncifyAssertInNonInstrumented>( + analyzer, pointerType, asyncifyMemory); + } + + AsyncifyAssertInNonInstrumented(ModuleAnalyzer* analyzer, + Type pointerType, + Name asyncifyMemory) + : analyzer(analyzer), pointerType(pointerType), + asyncifyMemory(asyncifyMemory) {} + + void runOnFunction(Module* module_, Function* func) override { + // FIXME: This looks like it was never right, as it should ignore the top- + // most runtime, but it will actually instrument it (as it needs no + // instrumentation, like random code - but the top-most runtime is + // actually a place that needs neither instrumentation *nor* + // assertions, as the assertions will error when it changes the + // state). + if (!analyzer->needsInstrumentation(func)) { + module = module_; + builder = + make_unique<AsyncifyBuilder>(*module, pointerType, asyncifyMemory); + addAssertsInNonInstrumented(func); + } + } // Given a function that is not instrumented - because we proved it doesn't // need it, or depending on the only-list / remove-list - add assertions that @@ -1275,6 +1306,10 @@ private: walker.oldState = oldState; walker.walk(func->body); } + +private: + std::unique_ptr<AsyncifyBuilder> builder; + Module* module; }; // Instrument local saving/restoring. @@ -1689,6 +1724,16 @@ struct Asyncify : public Pass { runner.setValidateGlobally(false); runner.run(); } + if (asserts) { + // Add asserts in non-instrumented code. Note we do not use an + // instrumented pass runner here as we do want to run on all functions. + PassRunner runner(module); + runner.add(make_unique<AsyncifyAssertInNonInstrumented>( + &analyzer, pointerType, asyncifyMemory)); + runner.setIsNested(true); + runner.setValidateGlobally(false); + runner.run(); + } // Next, add local saving/restoring logic. We optimize before doing this, // to undo the extra code generated by flattening, and to arrive at the // minimal amount of locals (which is important as we must save and |