[EH] Fuzz catch+rethrow on the JS side #7287

kripken · 2025-02-11T00:48:15Z

Add a parameter to call-export, call-ref, where the first bit says "catch and
rethrow any exception". This has no observable effect in binaryen, but causes us
to use different code paths in VMs (for example, a wasm exception may end up
caught in JS, then thrown in JS; passing wasm exnrefs to JS for throwing is not
possible otherwise, so this is the closest we can get).

…ch-ref

…ject'

aheejin · 2025-02-12T18:29:36Z

scripts/fuzz_shell.js

@@ -271,7 +275,7 @@ var imports = {
    'throw': (which) => {
      if (!which) {
        // Throw a JS exception.
-        throw 0;
+        throw new Error('js exception');


Isn't this effectively reverting #7286?

Partially, yes. That PR fixed us to throw similar things from JS and wasm, an i31 of 0 from wasm, and a JS number of 0. But it turns out that wasn't enough as the externref inside the exnref might end up logged out, and we need that logging to be identical. Throwing an object from both wasm and JS leads to both of them logging "object".

But the results in this line says not object but js exception.

Hmm, the difference can't show up there, because the fuzzer treats exceptions as interchangeable. That is, it converts all text beginning with "exception thrown: " into the same thing, so they all compare the same. (because different VMs have different error messages)

The content of the exception can be read without throwing it, though, if we catch it and return the contents to the outside. Then this difference is noticeable. Here is the test, showing object:

https://github.com/WebAssembly/binaryen/pull/7287/files#diff-a32958975d55fbae4aa24cc650a4d85d1cecaa8f4a8fdb5955372eb77fb7efcaR392

aheejin · 2025-02-12T18:35:54Z

src/tools/execution-results.h

+        // The second argument determines if we should catch and rethrow
+        // exceptions. There is no observable difference in those two modes in
+        // the binaryen interpreter, so we don't need to do anything.


Not sure what this means. Does that mean we only rethrow an exception (in case it occurs) in JS, so we don't have any difference in Binaryen? If so, why do we rethrow only in JS?

We rethrow in JS because we hope to find VM bugs. They might create a new JS exception around the wasm one, or something like that, but I don't know the full details - this is a suggestion the VM people gave me.

We could catch and rethrow in Binaryen too I guess, but that would literally be just try-catch and rethrow (we have no stack traces or other info in exceptions).

aheejin · 2025-02-12T19:41:58Z

scripts/fuzz_shell.js

+      if (!rethrow) {
+        /* await */ callFunc(exportList[index].value);
+      } else {
+        tryCall(/* async */ () => /* await */ callFunc(exportList[index].value),
+                rethrow);
+      }


How are these two modes different? What happens we call callFunc and it throws? Doesn't it also get rethrown, because we don't catch anything there?

The first arm of the if calls the function normally. If it throws, the exception keeps propagating out normally.

The second arm does the call inside a JS try-catch. If the call throws, we catch and rethrow it from JS.

VMs may implement those paths very different, I am told. This is just aiming to fuzz more things.

I see. Then why do we pass a function directly to callFunc in the first case and callFunc(()->func)) and why do we need async and await here?

In the second case we want tryCall to do the call inside a try-catch. So we give it code to run, as a () => { .. } function, rather than do the call first and give tryCall the result.

(Another option would be to write the try-catch here, that is, inline tryCall into this function. The only reason tryCall is separate is to avoid code duplication, see all the stuff in the catch body there.)

And, because of how async-await works in JavaScript, every function on the call stack must be declared async and called with await. So that part is just a side effect of our defining a function here. (I am actually not totally sure why all functions need async, but otherwise d8 ends up error "Pending promise rejection" - something to do with exception catching from Promises...)

Sorry I'm kind of confused why somewhere we print the contents and somewhere we don't, and when it matters and when it doesn't at this point 😵‍💫 But anyway it may not matter much.

Also async and await are all comments... Do you mean d8 errors out if you don't put those in comments?

In general we don't print exceptions, because the error text (and stack trace) may vary between VMs. Everything else, including the externref inside an exception - which would be a JS value - we do print, if we extracted it from the exception. I might be forgetting something though.

The comments are documented earlier in the file:

binaryen/scripts/fuzz_shell.js

Lines 10 to 13 in c83115a

// Whether we are fuzzing JSPI. In addition to this being set, the "async" and

// "await" keywords must be taken out of the /* KEYWORD */ comments (which they

// are normally in, so as not to affect normal fuzzing).

var JSPI;

We need async/await in JSPI mode, but don't want them to change anything in normal mode. So we have them in comments, and un-comment them in JSPI mode.

test/lit/exec/fuzzing-api.wast

aheejin · 2025-02-13T01:10:59Z

src/tools/fuzzing/fuzzing.cpp

@@ -851,12 +851,16 @@ void TranslateToFuzzReader::addImportCallingSupport() {

  if (choice & 1) {
    // Given an export index, call it from JS.
+    // A second parameter has flags. The first bit determines whether we catch
+    // and rethrow all exceptions. (This ends up giving us the same signature
+    // and behavior as the normal mode, so we just add the flags here rather


What's "normal mode"?

"normal" is when the bit is not set. I'll clarify the comment.

aheejin · 2025-02-13T01:16:20Z

src/tools/fuzzing/fuzzing.cpp

+      if (!catching) {
+        // The first bit matters here, so we can send anything.
+        args.push_back(make(Type::i32));
+      }


What does this mean? Shouldn't this be either 0 or 1 depending on whether to rthrow?

This is future-proof, as random values in the other bits might be used later, and have no downside now. I'll clarify in a comment.

aheejin · 2025-02-13T01:17:06Z

src/tools/fuzzing/fuzzing.cpp

+  if (!catching) {
+    // The first bit matters here, so we can send anything.
+    args.push_back(make(Type::i32));
+  }


(also updated this comment)

aheejin · 2025-02-13T01:18:29Z

scripts/fuzz_shell.js

+      if (!rethrow) {
+        /* await */ callFunc(exportList[index].value);
+      } else {
+        tryCall(/* async */ () => /* await */ callFunc(exportList[index].value),
+                rethrow);
+      }


Sorry I'm kind of confused why somewhere we print the contents and somewhere we don't, and when it matters and when it doesn't at this point 😵‍💫 But anyway it may not matter much.

Also async and await are all comments... Do you mean d8 errors out if you don't put those in comments?

Co-authored-by: Heejin Ahn <[email protected]>

aheejin · 2025-02-13T23:57:35Z

src/tools/fuzzing/fuzzing.cpp

+      if (!catching) {
+        // Only the first bit matters here, so we can send anything (this is
+        // future-proof for later bits, and has no downside now).
+        args.push_back(make(Type::i32));
+      }


I still don't understand sorry... why can we send anything if the first bit matters? What if make(Type::i32)) returns 1? (The same for below)

If the first bit in make(Type::i32) is 1 (including when the entire value is literally 1, or also when it is 3, etc.) then we will do the "rethrow" behavior, and if that first bit is 0 then we'll do the normal non-rethrow path. The other bits are currently ignored. But we get full coverage by just sending an arbitrary i32 here, since it gives both values for the first bit.

Maybe I'm not understanding your question. What are you worried about happening, or being missed?

Ah I see... I was confused and I thought 0 means catching and 1 means rethrowing somehow. But we have separate catching-and-returning functions and in these non-catching variants 0 means not doing anything and 1 means catching and rethrowing. Sorry for the confusion.

kripken added 30 commits February 6, 2025 09:37

fuzz.tag

2a07050

fix

53d123f

test

93b68ae

work

acbee3e

work

10323cf

fix

c9df9c0

fix

219e885

test

f93e130

start

7be44d8

work

15d90a4

work

d3b5402

clarify the expected behavior

a333277

Merge remote-tracking branch 'myself/fuzz.imported.tag' into fuzz.cat…

bf6b02c

…ch-ref

fix

3215f00

test

fe1dc4a

fix

d1e84c8

fix

982dd16

undo

8bd72b1

DEBUG

663194e

DEBUG2

060d3ac

DEBUG3

a4da2c4

DEBUG4

fd3ae28

UNDO

24f45bb

Merge remote-tracking branch 'origin/main' into fuzz.imported.tag

2ab135e

fix

0e1b1ec

start

d191f3d

fix

9dfbeab

test

470f267

fix

cb053d5

fixes

9718b5f

kripken added 8 commits February 10, 2025 14:22

fix

b97216a

work

a4216b0

fix

89e3ae4

fix

ece9449

Merge remote-tracking branch 'origin/main' into fuzz.catch-ref

2440f6a

typo

cbbcb59

typo

797a6b5

typo

ad988d7

kripken requested a review from aheejin February 11, 2025 00:48

kripken added 8 commits February 10, 2025 16:53

oops

4267acd

oops

1730e96

fix jspi

60a8747

simplify

c325697

skip.jspi

5533771

Merge remote-tracking branch 'origin/main' into fuzz.catch-ref

fcaa713

throw objects from JS and execution-results, so both print out as 'ob…

1f680a2

…ject'

test

c83115a

aheejin reviewed Feb 12, 2025

View reviewed changes

Merge remote-tracking branch 'origin/main' into fuzz.catch-ref

340f456

aheejin reviewed Feb 13, 2025

View reviewed changes

kripken and others added 6 commits February 13, 2025 09:21

Update test/lit/exec/fuzzing-api.wast

fcaf2fd

Co-authored-by: Heejin Ahn <[email protected]>

more

bf621f5

Merge remote-tracking branch 'myself/fuzz.catch-ref' into fuzz.catch-ref

0126238

comment

2015983

comments

e8312bf

better

f9fd6c6

aheejin reviewed Feb 13, 2025

View reviewed changes

aheejin approved these changes Feb 14, 2025

View reviewed changes

kripken merged commit 60d5626 into WebAssembly:main Feb 14, 2025
14 checks passed

kripken deleted the fuzz.catch-ref branch February 14, 2025 01:15

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[EH] Fuzz catch+rethrow on the JS side #7287

[EH] Fuzz catch+rethrow on the JS side #7287

kripken commented Feb 11, 2025

aheejin Feb 12, 2025

kripken Feb 12, 2025

aheejin Feb 12, 2025

kripken Feb 12, 2025

aheejin Feb 12, 2025

kripken Feb 12, 2025

aheejin Feb 12, 2025

kripken Feb 12, 2025

aheejin Feb 12, 2025

kripken Feb 12, 2025

aheejin Feb 13, 2025

kripken Feb 13, 2025

aheejin Feb 13, 2025

kripken Feb 13, 2025

aheejin Feb 13, 2025

kripken Feb 13, 2025

aheejin Feb 13, 2025

kripken Feb 13, 2025

aheejin Feb 13, 2025

aheejin Feb 13, 2025

kripken Feb 14, 2025

aheejin Feb 14, 2025

	// Whether we are fuzzing JSPI. In addition to this being set, the "async" and
	// "await" keywords must be taken out of the /* KEYWORD */ comments (which they
	// are normally in, so as not to affect normal fuzzing).
	var JSPI;

[EH] Fuzz catch+rethrow on the JS side #7287

[EH] Fuzz catch+rethrow on the JS side #7287

Conversation

kripken commented Feb 11, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment