From aa63272312c180eb698b6ca2b97a74636d0c047b Mon Sep 17 00:00:00 2001 From: stevenfontanella Date: Tue, 9 Dec 2025 00:19:34 +0000 Subject: [PATCH] Use shared_from_this to avoid lifetime issues with ModuleRunner --- src/shell-interface.h | 4 +-- src/tools/execution-results.h | 2 +- src/tools/wasm-ctor-eval.cpp | 13 ++++----- src/wasm-interpreter.h | 52 ++++++++++++++++++----------------- test/spec/linking0.wast | 30 ++++++++++++++++++++ 5 files changed, 66 insertions(+), 35 deletions(-) create mode 100644 test/spec/linking0.wast diff --git a/src/shell-interface.h b/src/shell-interface.h index 7a42617e711..ba5263bc03c 100644 --- a/src/shell-interface.h +++ b/src/shell-interface.h @@ -148,7 +148,7 @@ struct ShellExternalInterface : ModuleRunner::ExternalInterface { // Use a null instance because these are host functions. return Literal( std::make_shared(import->name, - nullptr, + /*self=*/0, [](const Literals& arguments) -> Flow { for (auto argument : arguments) { std::cout << argument << " : " @@ -159,7 +159,7 @@ struct ShellExternalInterface : ModuleRunner::ExternalInterface { import->type); } else if (import->module == ENV && import->base == EXIT) { return Literal(std::make_shared(import->name, - nullptr, + /*self=*/0, [](const Literals&) -> Flow { // XXX hack for torture tests std::cout << "exit()\n"; diff --git a/src/tools/execution-results.h b/src/tools/execution-results.h index 83f861ab7d4..50de3785a92 100644 --- a/src/tools/execution-results.h +++ b/src/tools/execution-results.h @@ -210,7 +210,7 @@ struct LoggingExternalInterface : public ShellExternalInterface { return {}; }; // Use a null instance because this is a host function. - return Literal(std::make_shared(import->name, nullptr, f), + return Literal(std::make_shared(import->name, /*self=*/0, f), import->type); } diff --git a/src/tools/wasm-ctor-eval.cpp b/src/tools/wasm-ctor-eval.cpp index b33fb5aa999..3dc90f00381 100644 --- a/src/tools/wasm-ctor-eval.cpp +++ b/src/tools/wasm-ctor-eval.cpp @@ -104,13 +104,12 @@ class EvallingModuleRunner : public ModuleRunnerBase { // This needs to be duplicated from ModuleRunner, unfortunately. Literal makeFuncData(Name name, Type type) { - auto allocation = - std::make_shared(name, this, [this, name](Literals arguments) { - return callFunction(name, arguments); + auto allocation = std::make_shared( + name, + reinterpret_cast(this), + [self = shared_from_this(), name](Literals arguments) { + return self->callFunction(name, arguments); }); -#if __has_feature(leak_sanitizer) || __has_feature(address_sanitizer) - __lsan_ignore_object(allocation.get()); -#endif return Literal(allocation, type); } }; @@ -320,7 +319,7 @@ struct CtorEvalExternalInterface : EvallingModuleRunner::ExternalInterface { // Use a null instance because these are either host functions or imported // from unknown sources. // TODO: Be more precise about the types we allow these imports to have. - return Literal(std::make_shared(import->name, nullptr, f), + return Literal(std::make_shared(import->name, /*self=*/0, f), import->type); } diff --git a/src/wasm-interpreter.h b/src/wasm-interpreter.h index f6fc5586a94..bcdd4ab8981 100644 --- a/src/wasm-interpreter.h +++ b/src/wasm-interpreter.h @@ -86,7 +86,7 @@ class Flow { } Literals values; - Name breakTo; // if non-null, a break is going on + Name breakTo; // if non-null, a break is going on Tag* suspendTag = nullptr; // if non-null, breakTo must be SUSPEND_FLOW, and // this is the tag being suspended @@ -137,17 +137,16 @@ struct FuncData { // The interpreter instance this function closes over, if any. (There might // not be an interpreter instance if this is a host function or an import from - // an unknown source.) This is only used for equality comparisons, as two - // functions are equal iff they have the same name and are defined by the same - // instance (in particular, we do *not* compare the |call| field below, which - // is an execution detail). - void* self; + // an unknown source.) Two functions are equal iff they have the same name and + // are defined by the same instance (in particular, we do *not* compare the + // |call| field below, which is an execution detail). + uintptr_t self; // A way to execute this function. We use this when it is called. using Call = std::function; Call call; - FuncData(Name name, void* self = nullptr, Call call = {}) + FuncData(Name name, uintptr_t self = 0, Call call = {}) : name(name), self(self), call(call) {} bool operator==(const FuncData& other) const { @@ -362,10 +361,8 @@ class ExpressionRunner : public OverriddenVisitor { Literal makeFuncData(Name name, Type type) { // Identify the interpreter, but do not provide a way to actually call the // function. - auto allocation = std::make_shared(name, this); -#if __has_feature(leak_sanitizer) || __has_feature(address_sanitizer) - __lsan_ignore_object(allocation.get()); -#endif + auto allocation = + std::make_shared(name, reinterpret_cast(this)); return Literal(allocation, type); } @@ -2917,7 +2914,9 @@ using GlobalValueSet = std::map; // template -class ModuleRunnerBase : public ExpressionRunner { +class ModuleRunnerBase + : public ExpressionRunner, + public std::enable_shared_from_this> { public: // // You need to implement one of these to create a concrete interpreter. The @@ -3207,9 +3206,10 @@ class ModuleRunnerBase : public ExpressionRunner { } return Literal(std::make_shared( func->name, - this, - [this, func](const Literals& arguments) -> Flow { - return callFunction(func->name, arguments); + reinterpret_cast(this), + [self = this->shared_from_this(), + func](const Literals& arguments) -> Flow { + return self->callFunction(func->name, arguments); }), func->type); } @@ -4774,7 +4774,7 @@ class ModuleRunnerBase : public ExpressionRunner { auto func = entry[0]; auto data = func.getFuncData(); // We must be in the right module to do the call using that name. - if (data->self != self()) { + if (data->self != (uintptr_t)self()) { // Restore the entry to the resume stack, as the other module's // callFunction() will read it. Then call into the other module. This // sets this up as if we called into the proper module in the first @@ -4866,10 +4866,9 @@ class ModuleRunnerBase : public ExpressionRunner { flow.values.pop_back(); arguments = flow.values; - if (nextData->self != this) { + if (nextData->self != (uintptr_t)self()) { // This function is in another module. Call from there. - auto other = (decltype(this))nextData->self; - flow = other->callFunction(name, arguments); + flow = nextData->doCall(arguments); break; } } @@ -5010,13 +5009,16 @@ class ModuleRunner : public ModuleRunnerBase { Literal makeFuncData(Name name, Type type) { // As the super's |makeFuncData|, but here we also provide a way to // actually call the function. - auto allocation = - std::make_shared(name, this, [this, name](Literals arguments) { - return callFunction(name, arguments); + auto allocation = std::make_shared( + name, + reinterpret_cast(this), + [moduleRunner = std::static_pointer_cast( + std::enable_shared_from_this< + ModuleRunnerBase>::shared_from_this()), + name](Literals arguments) { + return moduleRunner->callFunction(name, arguments); }); -#if __has_feature(leak_sanitizer) || __has_feature(address_sanitizer) - __lsan_ignore_object(allocation.get()); -#endif + return Literal(allocation, type); } }; diff --git a/test/spec/linking0.wast b/test/spec/linking0.wast new file mode 100644 index 00000000000..af9bb0ccea4 --- /dev/null +++ b/test/spec/linking0.wast @@ -0,0 +1,30 @@ +;; adapted from test/spec/testsuite/linking0.wast, with some assertions removed + +(module $Mt + (type (func (result i32))) + (type (func)) + + (table (export "tab") 10 funcref) + (elem (i32.const 2) $g $g $g $g) + (func $g (result i32) (i32.const 4)) + (func (export "h") (result i32) (i32.const -4)) + + (func (export "call") (param i32) (result i32) + (call_indirect (type 0) (local.get 0)) + ) +) +(register "Mt" $Mt) + +(assert_trap + (module + (table (import "Mt" "tab") 10 funcref) + (func $f (result i32) (i32.const 0)) + (elem (i32.const 7) $f) + (memory 0) + (memory $m 1) + (memory 0) + (data $m (i32.const 0x10000) "d") ;; out of bounds + ) + "out of bounds memory access" +) +(assert_return (invoke $Mt "call" (i32.const 7)) (i32.const 0))