[Exceptions] Fix cross-module Tag handling in interpreter #7955

kripken · 2025-10-07T21:22:23Z

Make ExnData store a Tag*, allowing proper identification.

Like FuncData etc., move that logic into the interpreter.

src/tools/execution-results.h

src/wasm-interpreter.h

tlively · 2025-10-07T22:23:28Z

src/wasm-interpreter.h

    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"); }


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.

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

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.

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.

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.

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.

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

I thought you meant to generate an internal module etc. - that would be a lot more work, obviously.

But I don't quite think I follow you, as the loop in getCanonicalTag is still necessary?

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.

Oh right, the loop isn't needed. Done.

test/lit/exec/tag-cross-module.wast

tlively · 2025-10-08T17:08:41Z

src/tools/execution-results.h

      if (tag->module == "fuzzing-support") {
        if (tag->base == "wasmtag") {
-          wasmTag = tag->name;
+          wasmTag.name = tag->name;


Is this renaming of tags still necessary? It seems like it is probably a vestige of using names to identify tags, and isn't something I would expect to be necessary. In general an external interface should not be modifying the module it is providing an environment to.

Edit: Ah, this shouldn't be necessary after the other comment is addressed.

tlively · 2025-10-08T17:11:56Z

src/wasm-interpreter.h

+    }
+    auto* tag = wasm.getTag(*export_->getInternalName());
+    if (tag->imported()) {
+      tag = externalInterface->getImportedTag(name);


This should pass the tag rather than the name so that the external interface can inspect the module and base import names to resolve the import. The external interface should never need to look at an internal name.

Oh nice, yes, that is cleaner. Done.

tlively · 2025-10-08T20:54:21Z

src/tools/execution-results.h

-          jsTag = tag->name;
-        }
+    // Set up tags. (Setting these values is useful for debugging - making the
+    // Tag objects valid - and also appears in fuzz-exec logging.)


We should also need to set the type of the tags to make them valid. Hopefully the fuzzer can find that.

Added, can't hurt for debugging. I'm not sure if we notice these atm enough for the fuzzer to.

tlively · 2025-10-08T21:01:07Z

src/wasm-interpreter.h

+    throwException(WasmException{
+      self()->makeExnData(self()->getModule()->getTag(curr->tag), arguments)});


We should probably return a NONCONSTANT_FLOW when the tag is imported.

Good catch, done.

tlively · 2025-10-08T21:02:42Z

src/wasm-interpreter.h

+    virtual Tag* getImportedTag(Tag* tag) {
+      WASM_UNREACHABLE("missing imported tag");
+    }


Is there a reason not to make this pure virtual like getImportedFunction? Should getImportedFunction also have an unreachable default implementation?

I think it's just that there are a bunch of cases that want the default impl for tags, and for functions we implement it anyhow. But I guess consistency matters more, I made it pure virtual.

kripken added 15 commits October 7, 2025 11:10

start

3bc8a54

work

403269c

work

1e98549

work

18acf83

work

6b45a18

work

167cc41

work

2b5a6d7

work

08827ee

work

7cd8db0

fix

b876369

fix

9bbdfe7

Merge remote-tracking branch 'origin/main' into tag.cross-module

7047703

work

fbcfc9c

fix

ac39b5f

todone

d13eca9

kripken requested a review from tlively October 7, 2025 21:22

tlively reviewed Oct 7, 2025

View reviewed changes

kripken added 7 commits October 7, 2025 15:42

todo

dda230b

Merge remote-tracking branch 'origin/main' into tag.cross-module

046fec3

Enable spec test

0eb9913

rename to getImportedTag

360f53f

Add and use getExportedTag

0b94658

format

5b32bee

unloop

c0a8436

tlively reviewed Oct 8, 2025

View reviewed changes

kripken added 2 commits October 8, 2025 13:19

Merge remote-tracking branch 'origin/main' into tag.cross-module

d764d4c

Send a Tag* to getImportedTag

6678d81

tlively reviewed Oct 8, 2025

View reviewed changes

kripken added 2 commits October 8, 2025 14:10

add host tag sigs

1c0e0af

handle imported tags in single-module case

c51dcb2

kripken added 2 commits October 8, 2025 14:16

make getImportedTag pure virtual

f4e39fd

oops

a0d9732

tlively approved these changes Oct 8, 2025

View reviewed changes

kripken merged commit 7a94261 into WebAssembly:main Oct 9, 2025
16 checks passed

kripken deleted the tag.cross-module branch October 9, 2025 15:01

		throwException(WasmException{
		self()->makeExnData(self()->getModule()->getTag(curr->tag), arguments)});

[Exceptions] Fix cross-module Tag handling in interpreter #7955

[Exceptions] Fix cross-module Tag handling in interpreter #7955

Uh oh!

Conversation

kripken commented Oct 7, 2025

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants