Skip to content

Commit 9181f9f

Browse files
authored
[Stack Switching] Handle imported tags properly (#7801)
We were using Tag names, but the internal name in a module may overlap with other modules etc. - we really need to look at the actual imported Tag, if it is imported. To do that, store Tag* and add a way to get the canonical Tag* - i.e., the imported one if imported, so all references, including from other modules, all refer to the same thing. To be able to test this, make `callImport` return a Flow (so we can suspend through import calls). As a bonus, replacing `Name tag` with `Tag* tag` makes our data structures smaller.
1 parent e81f964 commit 9181f9f

File tree

6 files changed

+75
-16
lines changed

6 files changed

+75
-16
lines changed

scripts/test/fuzzing.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -105,6 +105,7 @@
105105
'vacuum-desc.wast',
106106
'br_on_cast_desc.wast',
107107
# TODO: fuzzer support for stack switching
108+
'tag_linked.wast',
108109
'stack_switching.wast',
109110
'stack_switching_contnew.wast',
110111
'stack_switching_contbind.wast',

src/shell-interface.h

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -133,7 +133,7 @@ struct ShellExternalInterface : ModuleRunner::ExternalInterface {
133133
});
134134
}
135135

136-
Literals callImport(Function* import, const Literals& arguments) override {
136+
Flow callImport(Function* import, const Literals& arguments) override {
137137
if (import->module == SPECTEST && import->base.startsWith(PRINT)) {
138138
for (auto argument : arguments) {
139139
std::cout << argument << " : " << argument.type << '\n';
@@ -144,9 +144,7 @@ struct ShellExternalInterface : ModuleRunner::ExternalInterface {
144144
std::cout << "exit()\n";
145145
throw ExitException();
146146
} else if (auto* inst = getImportInstance(import)) {
147-
auto flow = inst->callExport(import->base, arguments);
148-
assert(!flow.suspendTag); // TODO: support stack switching on calls
149-
return flow.values;
147+
return inst->callExport(import->base, arguments);
150148
}
151149
Fatal() << "callImport: unknown import: " << import->module.str << "."
152150
<< import->name.str;

src/tools/execution-results.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,7 @@ struct LoggingExternalInterface : public ShellExternalInterface {
7575
}
7676
}
7777

78-
Literals callImport(Function* import, const Literals& arguments) override {
78+
Flow callImport(Function* import, const Literals& arguments) override {
7979
if (import->module == "fuzzing-support") {
8080
if (import->base.startsWith("log")) {
8181
// This is a logging function like log-i32 or log-f64

src/tools/wasm-ctor-eval.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -242,7 +242,7 @@ struct CtorEvalExternalInterface : EvallingModuleRunner::ExternalInterface {
242242
});
243243
}
244244

245-
Literals callImport(Function* import, const Literals& arguments) override {
245+
Flow callImport(Function* import, const Literals& arguments) override {
246246
Name WASI("wasi_snapshot_preview1");
247247

248248
if (ignoreExternalInput) {

src/wasm-interpreter.h

Lines changed: 25 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -80,15 +80,15 @@ class Flow {
8080
Flow(Name breakTo, Literal value) : values{value}, breakTo(breakTo) {}
8181
Flow(Name breakTo, Literals&& values)
8282
: values(std::move(values)), breakTo(breakTo) {}
83-
Flow(Name breakTo, Name suspendTag, Literals&& values)
83+
Flow(Name breakTo, Tag* suspendTag, Literals&& values)
8484
: values(std::move(values)), breakTo(breakTo), suspendTag(suspendTag) {
8585
assert(breakTo == SUSPEND_FLOW);
8686
}
8787

8888
Literals values;
8989
Name breakTo; // if non-null, a break is going on
90-
Name suspendTag; // if non-null, breakTo must be SUSPEND_FLOW, and this is the
91-
// tag being suspended
90+
Tag* suspendTag = nullptr; // if non-null, breakTo must be SUSPEND_FLOW, and
91+
// this is the tag being suspended
9292

9393
// A helper function for the common case where there is only one value
9494
const Literal& getSingleValue() {
@@ -123,7 +123,7 @@ class Flow {
123123
o << flow.values[i];
124124
}
125125
if (flow.suspendTag) {
126-
o << " [suspend:" << flow.suspendTag << ']';
126+
o << " [suspend:" << flow.suspendTag->name << ']';
127127
}
128128
o << "})";
129129
return o;
@@ -3056,8 +3056,7 @@ class ModuleRunnerBase : public ExpressionRunner<SubType> {
30563056
virtual ~ExternalInterface() = default;
30573057
virtual void init(Module& wasm, SubType& instance) {}
30583058
virtual void importGlobals(GlobalValueSet& globals, Module& wasm) = 0;
3059-
virtual Literals callImport(Function* import,
3060-
const Literals& arguments) = 0;
3059+
virtual Flow callImport(Function* import, const Literals& arguments) = 0;
30613060
virtual bool growMemory(Name name, Address oldSize, Address newSize) = 0;
30623061
virtual bool growTable(Name name,
30633062
const Literal& value,
@@ -3564,6 +3563,21 @@ class ModuleRunnerBase : public ExpressionRunner<SubType> {
35643563
return inst->globals[global->name];
35653564
}
35663565

3566+
// Get a tag object while looking through imports, i.e., this uses the name as
3567+
// the name of the tag in the current module, and finds the actual canonical
3568+
// Tag* object for it: the Tag in this module, if not imported, and if
3569+
// imported, the Tag in the originating module.
3570+
Tag* getCanonicalTag(Name name) {
3571+
auto* inst = self();
3572+
auto* tag = inst->wasm.getTag(name);
3573+
while (tag->imported()) {
3574+
inst = inst->linkedInstances.at(tag->module).get();
3575+
auto* tagExport = inst->wasm.getExport(tag->base);
3576+
tag = inst->wasm.getTag(*tagExport->getInternalName());
3577+
}
3578+
return tag;
3579+
}
3580+
35673581
public:
35683582
Flow visitCall(Call* curr) {
35693583
Name target = curr->target;
@@ -4775,7 +4789,8 @@ class ModuleRunnerBase : public ExpressionRunner<SubType> {
47754789
// We will resume from this precise spot, when the new continuation is
47764790
// resumed.
47774791
new_->resumeExpr = curr;
4778-
return Flow(SUSPEND_FLOW, curr->tag, std::move(arguments));
4792+
return Flow(
4793+
SUSPEND_FLOW, self()->getCanonicalTag(curr->tag), std::move(arguments));
47794794
}
47804795
Flow visitResume(Resume* curr) {
47814796
Literals arguments;
@@ -4825,10 +4840,10 @@ class ModuleRunnerBase : public ExpressionRunner<SubType> {
48254840
} else {
48264841
// We are suspending. See if a suspension arrived that we support.
48274842
for (size_t i = 0; i < curr->handlerTags.size(); i++) {
4828-
auto handlerTag = curr->handlerTags[i];
4843+
auto* handlerTag = self()->getCanonicalTag(curr->handlerTags[i]);
48294844
if (handlerTag == ret.suspendTag) {
48304845
// Switch the flow from suspending to branching.
4831-
ret.suspendTag = Name();
4846+
ret.suspendTag = nullptr;
48324847
ret.breakTo = curr->handlerBlocks[i];
48334848
// We can now update the continuation type, which was wrong until now
48344849
// (see comment in visitSuspend). The type is taken from the block we
@@ -4978,7 +4993,7 @@ class ModuleRunnerBase : public ExpressionRunner<SubType> {
49784993

49794994
#if WASM_INTERPRETER_DEBUG
49804995
std::cout << self()->indent() << "exiting " << function->name << " with "
4981-
<< flow.values << '\n';
4996+
<< flow << '\n';
49824997
#endif
49834998

49844999
if (flow.suspendTag) {

test/spec/tag_linked.wast

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
;; Test that we can properly use a tag from an imported module. The export name
2+
;; and the internal names all differ, to make sure we properly identify the tag.
3+
4+
(module
5+
(tag $suspend)
6+
7+
(export "suspendey" (tag $suspend))
8+
(export "suspender" (func $suspender))
9+
10+
(func $suspender
11+
(suspend $suspend)
12+
)
13+
)
14+
15+
(register "first")
16+
17+
(module
18+
(type $func (func))
19+
(type $cont (cont $func))
20+
21+
(tag $imported-suspend (import "first" "suspendey"))
22+
23+
(import "first" "suspender" (func $suspender))
24+
25+
(tag $suspend) ;; name overlap
26+
27+
(func (export "run") (result i32)
28+
(drop
29+
(block $internal (result (ref $cont))
30+
(block $imported (result (ref $cont))
31+
(resume $cont (on $imported-suspend $imported) (on $suspend $internal)
32+
(cont.new $cont (ref.func $suspender))
33+
)
34+
(unreachable)
35+
)
36+
(return (i32.const 42))
37+
)
38+
(return (i32.const 1337))
39+
)
40+
(unreachable)
41+
)
42+
)
43+
44+
(assert_return (invoke "run") (i32.const 42))
45+

0 commit comments

Comments
 (0)