Skip to content
Open
12 changes: 0 additions & 12 deletions src/literal.h
Original file line number Diff line number Diff line change
Expand Up @@ -785,18 +785,6 @@ struct GCData {
: type(type), values(std::move(values)), desc(desc) {}
};

// The data of a (ref exn) literal.
struct ExnData {
// The tag of this exn data.
// TODO: handle cross-module calls using something other than a Name here.
Name tag;

// The payload of this exn data.
Literals payload;

ExnData(Name tag, Literals payload) : tag(tag), payload(payload) {}
};

} // namespace wasm

namespace std {
Expand Down
35 changes: 25 additions & 10 deletions src/tools/execution-results.h
Original file line number Diff line number Diff line change
Expand Up @@ -42,13 +42,11 @@ struct LoggingExternalInterface : public ShellExternalInterface {
Name exportedTable;
Module& wasm;

// The name of the imported fuzzing tag for wasm.
Name wasmTag;
// The imported fuzzing tag for wasm.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't we need to handle an arbitrary number of imported tags?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is just for the special imported tags from JS:

var wasmTag = imports['fuzzing-support']['wasmtag'] = new WebAssembly.Tag({
'parameters': ['i32']
});
// The JSTag that represents a JS tag.
imports['fuzzing-support']['jstag'] = WebAssembly.JSTag;

Tag wasmTag;

// The name of the imported tag for js exceptions. If it is not imported, we
// use a default name here (which should differentiate it from any wasm
// exceptions).
Name jsTag = "__private";
// The imported tag for js exceptions.
Tag jsTag;

// The ModuleRunner and this ExternalInterface end up needing links both ways,
// so we cannot init this in the constructor.
Expand All @@ -67,17 +65,34 @@ struct LoggingExternalInterface : public ShellExternalInterface {
}
}

// Default names for tags.
wasmTag.module = "fuzzing-support";
wasmTag.base = wasmTag.name = "wasmtag";

jsTag.module = "fuzzing-support";
jsTag.base = "jstag";
jsTag.name = "__private";

for (auto& tag : wasm.tags) {
if (tag->module == "fuzzing-support") {
if (tag->base == "wasmtag") {
wasmTag = tag->name;
wasmTag.name = tag->name;
} else if (tag->base == "jstag") {
jsTag = tag->name;
jsTag.name = tag->name;
}
}
}
}

Tag* getHostTag(Name name) override {
if (name == jsTag.name) {
return &jsTag;
} else if (name == wasmTag.name) {
return &wasmTag;
}
Fatal() << "missing host tag " << name;
}

Literal getImportedFunction(Function* import) override {
if (linkedInstances.count(import->module)) {
return getImportInstance(import)->getExportedFunction(import->base);
Expand Down Expand Up @@ -122,7 +137,7 @@ struct LoggingExternalInterface : public ShellExternalInterface {
if (arguments[0].geti32() == 0) {
throwJSException();
} else {
auto payload = std::make_shared<ExnData>(wasmTag, arguments);
auto payload = std::make_shared<ExnData>(&wasmTag, arguments);
throwException(WasmException{Literal(payload)});
}
} else if (import->base == "table-get") {
Expand Down Expand Up @@ -213,7 +228,7 @@ struct LoggingExternalInterface : public ShellExternalInterface {
auto empty = HeapType(Struct{});
auto inner = Literal(std::make_shared<GCData>(empty, Literals{}), empty);
Literals arguments = {inner.externalize()};
auto payload = std::make_shared<ExnData>(jsTag, arguments);
auto payload = std::make_shared<ExnData>(&jsTag, arguments);
throwException(WasmException{Literal(payload)});
}

Expand Down
47 changes: 38 additions & 9 deletions src/wasm-interpreter.h
Original file line number Diff line number Diff line change
Expand Up @@ -160,6 +160,17 @@ struct FuncData {
}
};

// The data of a (ref exn) literal.
struct ExnData {
// The tag of this exn data.
Tag* tag;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This probably also needs an instance self pointer to differentiate tags defined by different instances of the same module.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point, I suppose it does. I'll add a TODO (atm we don't fuzz or test with that afaik)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

testsuite/try_table.wast depends on this (so we skip it atm).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, good. Then we can enable that later. (I just prefer to keep this PR small.)


// The payload of this exn data.
Literals payload;

ExnData(Tag* tag, Literals payload) : tag(tag), payload(payload) {}
};

// Suspend/resume support.
//
// As we operate directly on our structured IR, we do not have a program counter
Expand Down Expand Up @@ -324,7 +335,7 @@ class ExpressionRunner : public OverriddenVisitor<SubType, Flow> {
}

// Same as makeGCData but for ExnData.
Literal makeExnData(Name tag, const Literals& payload) {
Literal makeExnData(Tag* tag, const Literals& payload) {
auto allocation = std::make_shared<ExnData>(tag, payload);
#if __has_feature(leak_sanitizer) || __has_feature(address_sanitizer)
__lsan_ignore_object(allocation.get());
Expand Down Expand Up @@ -1890,9 +1901,13 @@ class ExpressionRunner : public OverriddenVisitor<SubType, Flow> {
Flow visitTry(Try* curr) { WASM_UNREACHABLE("unimp"); }
Flow visitTryTable(TryTable* curr) { WASM_UNREACHABLE("unimp"); }
Flow visitThrow(Throw* curr) {
// Single-module implementation. This is used from Precompute, for example.
// It is overriden in ModuleRunner to add logic for finding the proper
// imported tag (which single-module cases don't care about).
Literals arguments;
VISIT_ARGUMENTS(flow, curr->operands, arguments);
throwException(WasmException{makeExnData(curr->tag, arguments)});
throwException(WasmException{
self()->makeExnData(self()->getModule()->getTag(curr->tag), arguments)});
WASM_UNREACHABLE("throw");
}
Flow visitRethrow(Rethrow* curr) { WASM_UNREACHABLE("unimp"); }
Expand Down Expand Up @@ -2908,6 +2923,9 @@ class ModuleRunnerBase : public ExpressionRunner<SubType> {
virtual void trap(const char* why) = 0;
virtual void hostLimit(const char* why) = 0;
virtual void throwException(const WasmException& exn) = 0;
// Get the Tag instance for a tag implemented in the host, that is, not
// among the linked ModuleRunner instances.
virtual Tag* getHostTag(Name name) { WASM_UNREACHABLE("missing host tag"); }
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't this be a general getImportedTag? Host tags shouldn't be special for any reason and it would be best to be consistent in how we handle imports. Specifically, we should follow the same patterns for getImportedFunction and whatever we do for tags.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(And btw we should also follow the same shared pattern for tables and memories as well.)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We have these special host tags in the fuzzer, which we don't for functions - so this pattern is very convenient here, but maybe not for functions.

Perhaps this could be refactored to make these host tags come from a special internal module, I considered that but it was a lot more work, and I wasn't sure it would lead to benefits.

I do agree it makes sense to unify this, but I'm not sure which way, so I'd like to leave that for later.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We have lots of host functions in the fuzzer as well, but we treat them as normal imports by wrapping the custom functionality in the FuncData's std::function. Tags wouldn't need that last part, since only their identity matters.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, we handle tags differently by having an instance of them (which we can take the address of), unlike those functions. It's possible to handle it otherwise but it would be a lot more work I think.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think concretely the changes I would want to see here is to rename this function to getImportedTag and to add a ModuleRunnerBase::getExportedTag function that calls externalInterface->getImportedTag(tag) on imported tags. That would replace the while loop in getCanonicalTag to track down the definition of imported tags.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, I tried to do that in the last two commits, PTAL

  1. I thought you meant to generate an internal module etc. - that would be a lot more work, obviously.
  2. But I don't quite think I follow you, as the loop in getCanonicalTag is still necessary?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If getCanonicalTag calls out to the ExternalInterface to retrieve imported tags, then in multi-instance scenarios that ExternalInterface will look up the instance providing the import and call getExportedTag on it (which might in turn call externalInterface->getImportedTag again if it is a re-exported tag). So the loop is replaced with a recursion.


// the default impls for load and store switch on the sizes. you can either
// customize load/store, or the sub-functions which they call
Expand Down Expand Up @@ -3446,7 +3464,11 @@ class ModuleRunnerBase : public ExpressionRunner<SubType> {
auto* inst = self();
auto* tag = inst->wasm.getTag(name);
while (tag->imported()) {
inst = inst->linkedInstances.at(tag->module).get();
auto iter = inst->linkedInstances.find(tag->module);
if (iter == inst->linkedInstances.end()) {
return externalInterface->getHostTag(name);
}
inst = iter->second.get();
auto* tagExport = inst->wasm.getExport(tag->base);
tag = inst->wasm.getTag(*tagExport->getInternalName());
}
Expand Down Expand Up @@ -4354,7 +4376,8 @@ class ModuleRunnerBase : public ExpressionRunner<SubType> {

auto exnData = e.exn.getExnData();
for (size_t i = 0; i < curr->catchTags.size(); i++) {
if (curr->catchTags[i] == exnData->tag) {
auto* tag = self()->getCanonicalTag(curr->catchTags[i]);
if (tag == exnData->tag) {
multiValues.push_back(exnData->payload);
return processCatchBody(curr->catchBodies[i]);
}
Expand All @@ -4377,7 +4400,8 @@ class ModuleRunnerBase : public ExpressionRunner<SubType> {
auto exnData = e.exn.getExnData();
for (size_t i = 0; i < curr->catchTags.size(); i++) {
auto catchTag = curr->catchTags[i];
if (!catchTag.is() || catchTag == exnData->tag) {
if (!catchTag.is() ||
self()->getCanonicalTag(catchTag) == exnData->tag) {
Flow ret;
ret.breakTo = curr->catchDests[i];
if (catchTag.is()) {
Expand All @@ -4395,6 +4419,13 @@ class ModuleRunnerBase : public ExpressionRunner<SubType> {
throw;
}
}
Flow visitThrow(Throw* curr) {
Literals arguments;
VISIT_ARGUMENTS(flow, curr->operands, arguments);
throwException(WasmException{
self()->makeExnData(self()->getCanonicalTag(curr->tag), arguments)});
WASM_UNREACHABLE("throw");
}
Flow visitRethrow(Rethrow* curr) {
for (int i = exceptionStack.size() - 1; i >= 0; i--) {
if (exceptionStack[i].second == curr->target) {
Expand Down Expand Up @@ -4463,9 +4494,8 @@ class ModuleRunnerBase : public ExpressionRunner<SubType> {
assert(self()->restoredValuesMap.empty());
// Throw, if we were resumed by resume_throw;
if (auto* tag = currContinuation->exceptionTag) {
// XXX tag->name lacks cross-module support
throwException(WasmException{
self()->makeExnData(tag->name, currContinuation->resumeArguments)});
self()->makeExnData(tag, currContinuation->resumeArguments)});
}
return currContinuation->resumeArguments;
}
Expand Down Expand Up @@ -4668,9 +4698,8 @@ class ModuleRunnerBase : public ExpressionRunner<SubType> {
// set), so resuming is done. (And throw, if resume_throw.)
self()->continuationStore->resuming = false;
if (auto* tag = currContinuation->exceptionTag) {
// XXX tag->name lacks cross-module support
throwException(WasmException{
self()->makeExnData(tag->name, currContinuation->resumeArguments)});
self()->makeExnData(tag, currContinuation->resumeArguments)});
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/wasm/wasm-interpreter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ namespace wasm {

std::ostream& operator<<(std::ostream& o, const WasmException& exn) {
auto exnData = exn.exn.getExnData();
return o << exnData->tag << " " << exnData->payload;
return o << exnData->tag->name << " " << exnData->payload;
}

} // namespace wasm
26 changes: 26 additions & 0 deletions test/lit/exec/tag-cross-module.wast
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be good to get the newly-imported spec tests for this running as well. In general spec tests are better for testing multi-module execution because the several modules can go in a single file.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice, looks like that relevant spec test passes! I enabled it now.

Is it worth removing the new test from this PR? I think an exec test is still useful to have for this, as extra coverage. The fuzz-exec output is also more explicit than spec test output.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice! I'm fine keeping the lit test, too.

Copy link
Member

@tlively tlively Oct 7, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess the spec test doesn't tickle the case where we would need a self pointer to differentiate the tags, so it would be good to add such a spectest upstream.

(I can look at this since I need to add some new upstream tests anyway for the function import cast stuff.)

Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
;; RUN: wasm-opt %s -all --fuzz-exec-before --fuzz-exec-second=%s.second -q -o /dev/null 2>&1 | filecheck %s

;; Define a tag in this module, and another tag in the secondary module, with
;; the same name but different (incompatible) contents. The second module will
;; call our export, and when we throw our tag, it should not catch it.

(module
(tag $tag (param structref))

(export "primary-tag" (tag $tag))

(func $func (export "func") (result i32)
(throw $tag
(ref.null struct)
)
)
)

;; CHECK: [fuzz-exec] calling func
;; CHECK-NEXT: [exception thrown: tag nullref]
;; CHECK-NEXT: [fuzz-exec] calling func2-internal
;; CHECK-NEXT: [exception thrown: tag nullref]
;; CHECK-NEXT: [fuzz-exec] calling func2-imported
;; CHECK-NEXT: func2-imported => null


32 changes: 32 additions & 0 deletions test/lit/exec/tag-cross-module.wast.second
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
(module
(import "primary" "func" (func $import (result i32)))

(import "primary" "primary-tag" (tag $ptag (param structref)))

(tag $tag (param (ref array)))

(func $func2-internal (export "func2-internal") (result (ref array))
;; Try to catch the internal tag. This fails to catch.
(block $block (result (ref array))
(try_table (catch $tag $block)
(drop
(call $import)
)
)
(unreachable)
)
)

(func $func2-imported (export "func2-imported") (result structref)
;; Try to catch the imported tag. This successfully catches.
(block $block (result structref)
(try_table (catch $ptag $block)
(drop
(call $import)
)
)
(unreachable)
)
)
)

Loading