Update JS API for exnref #301

dschuff · 2024-03-29T22:18:01Z

This change updates exception object allocation, initialization, and construction for exnref;
as well as dealing with exception propagation from invoking exported functions; throwing
exceptions from host functions into wasm; and wrapping and unwrapping JS exceptions as they
propagate into and out of wasm.

It depends on the embedding interfaces added in #268.

This is a basic start of what we'll need to add to the embedder spec. I'm not sure if we also need a way to allow exceptions to bubble from the embedder into wasm ; I don't see any reference to what happens when the instance wants to call an import, so maybe not. Also the references aren't quite right, but I may need help to fix that

Co-authored-by: Andreas Rossberg <[email protected]>

document/js-api/index.bs

eqrion · 2024-04-03T16:11:25Z

I don't see this in this PR or the overview, but is it intended for exn to be passable to/from JS through params/results, or be like v128 and cause TypeError's?

dschuff · 2024-04-04T00:12:08Z

I don't see this in this PR or the overview, but is it intended for exn to be passable to/from JS through params/results, or be like v128 and cause TypeError's?

When Andreas updated the core spec he also did a minimal update to this spec that made it the latter (see e.g. here or in the getters/setters for table elements and globals).
In this PR when exceptions are thrown or propagate from wasm into JS, they end up wrapped in a WebAssembly.Exception object (that's line 1023 in this patch) just as they are in phase 3 exceptions. I'm not sure what a concrete use case would be for passing/returning but it seems slightly weird and asymmetric that you can get any exception instance out to JS by throwing it, but not by returning it.

rossberg · 2024-04-04T07:07:01Z

It is intentional. I can't find the original discussion right now, but being able to pass exception packages as first-class values to JS would have nasty implications, such as:

Instead of being able to just do reference counting on exnrefs internally, it would require engines to do either cross host-wasm GC and hence effectively create a dependency on GC again, or rely on host code correctly participating in RC, which could easily corrupt the Wasm engine when that's done wrong.
The host could pass it back as an externref, which implies that exnref becomes a subset of externref, which in turn means that GC/RC for exnrefs would have to include externrefs, with very unclear implications for non-JS embedding.
For JS embeddings, it would likely expose or constrain implementation details of the handling of the special JavaScript exception tag, because JS code could compare the identity of a thrown JS value and an exnref representing its catch.

Conceptually, its better to think of and exnref as not the exception instance being thrown, but a "pair" of an exception instance that's been caught plus abstract context information for rethrowing it that a Wasm engine might need internally. As such it is a different kind of object from the exception instance and entirely Wasm-internal.

eqrion · 2024-04-04T15:09:27Z

Okay, that was my understanding as well, but wanted to make sure. SpiderMonkey throws a TypeError in ToJSValue.

…tions. Wasm throwing is still not properly defined.

dschuff · 2024-04-05T00:45:21Z

I think this PR is now reasonably correct and self-contained according to the description in OP. I think it correctly (although may not yet completely) implements what we discussed in #282. (I had forgotten about that thread, we can move further discussion about the desired value semantics back there).
So I think it probably makes sense to finish this review and land it, to keep PRs of a reasonable size.

dschuff · 2024-04-05T00:50:30Z

document/js-api/index.bs


 <div algorithm>

-To <dfn for=WebAssembly>throw</dfn> with a [=tag address=] |type|, a matching [=list=] of WebAssembly values |payload|, and an [=externref=] |opaqueData|, perform the following steps:
+To <dfn for=WebAssembly>throw</dfn> with a [=tag address=] |type|, a matching [=list=] of WebAssembly values |payload|, and an [=exception address=] |address|, perform the following steps:

 1. Unwind the stack until reaching the *catching try block* given |type|.


This is the main bit that still needs to be updated, as there is currently nothing in the embedding appendix that I could find that even describes calling from wasm into the embedder, and thus nowhere to put a description of what happens when such calls throw.

The problem is that this can not be an "external" function, since it could only be invoked inside a Wasm context. So it can't easily be defined as a proper embedder function, which need a closed evaluation context. To make it work, we would need to pass it the "current evaluation context", which would require means for reifying an entire host/Wasm execution. There is no way to make that work with the JS spec style.

But this algorithm is invoked from another one that already describes Wasm semantics, not host semantics. So we're morally "inside Wasm". So what I'd suggest is to say something like "execute the Wasm instructions (ref.exn |address|) (throw_ref).

And with my other comment there is only one place left where this is invoked, so I'd just inline that step there and remove this algorithm.

Makes sense, done.

document/js-api/index.bs

rossberg · 2024-04-05T07:18:22Z

document/js-api/index.bs

-            1. Let |opaqueData| be [=ToWebAssemblyValue=](|v|, [=externref=])
-            1. [=WebAssembly/Throw=] with |type|, |payload| and |opaqueData|.
+                1. Let |type| be the result of [=get the JavaScript exception tag |getting the JavaScript exception tag=].
+                1. Let |payload| be [=ToWebAssemblyValue=](|v|, [=externref=]).


I think there is a meta-level type error here: extrapolating from its internal use of [=?=], ToWebAssemblyValue supposedly returns a JS completion and hence can e.g. fail with an exception, so we must assert here that it is a normal completion. (I think that's what [=!=] does in the JS spec.)

And FWIW I just noticed that there is a similar (pre-existing) problem with various other use sites of ToWebAssemblyValue failing to use [=?=].

Suggested change

1. Let |payload| be [=ToWebAssemblyValue=](|v|, [=externref=]).

1. Let |payload| be [=!=] [=ToWebAssemblyValue=](|v|, [=externref=]).

Yeah that makes sense, and you are right about !:
https://tc39.es/ecma262/#sec-returnifabrupt-shorthands

Probably makes sense to fix the other uses of ToWebAssemblyValue in a separate PR.

rossberg · 2024-04-05T07:21:56Z

document/js-api/index.bs


 <div algorithm>

-To <dfn for=WebAssembly>throw</dfn> with a [=tag address=] |type|, a matching [=list=] of WebAssembly values |payload|, and an [=externref=] |opaqueData|, perform the following steps:
+To <dfn for=WebAssembly>throw</dfn> with a [=tag address=] |type|, a matching [=list=] of WebAssembly values |payload|, and an [=exception address=] |address|, perform the following steps:

 1. Unwind the stack until reaching the *catching try block* given |type|.


The problem is that this can not be an "external" function, since it could only be invoked inside a Wasm context. So it can't easily be defined as a proper embedder function, which need a closed evaluation context. To make it work, we would need to pass it the "current evaluation context", which would require means for reifying an entire host/Wasm execution. There is no way to make that work with the JS spec style.

But this algorithm is invoked from another one that already describes Wasm semantics, not host semantics. So we're morally "inside Wasm". So what I'd suggest is to say something like "execute the Wasm instructions (ref.exn |address|) (throw_ref).

And with my other comment there is only one place left where this is invoked, so I'd just inline that step there and remove this algorithm.

…throw_ref

dschuff · 2024-04-05T18:56:28Z

But this algorithm is invoked from another one that already describes Wasm semantics, not host semantics. So we're morally "inside Wasm". So what I'd suggest is to say something like "execute the Wasm instructions (ref.exn |address|) (throw_ref).
And with my other comment there is only one place left where this is invoked, so I'd just inline that step there and remove this algorithm.

This is done too, but since the lines it was attached to are now gone, github won't let me reply...

aheejin · 2024-04-09T14:01:21Z

document/js-api/index.bs

+    1. Let |store| be the [=surrounding agent=]'s [=associated store=].
+    1. Let (|store|, |tagAddress|) be [=tag_alloc=](|store|, « [=externref=] » → « »).
+    1. Set the current agent's [=associated store=] to |store|.
+    1. Set the current agent's associated [=JavaScript exception tag=] to |tagAddress|.


Does this mean tag == tagAddress? Somewhere above in this doc contains

Set map[tagAddress] to tag.

So they seem to be different.

And this PR doesn't include how Wasm should handle the JS exception tag, right?

Line 1208 is where you're referring to. There tag and tagAddress are both basically function parameters, and tag is a JS object and tagAddress is an address as defined in the wasm runtime spec. Here tagAddress is basically a local variable (a wasm address), a return value of tag_alloc; and tag isn't a variable at all. This function just returns the address.

And yes, #269 is the PR that allows actually exposing the JS exception tag (which by extension allows it to be imported into a wasm module).

aheejin · 2024-04-09T14:56:53Z

document/js-api/index.bs

        1. If |result|.\[[Type]] is <emu-const>throw</emu-const>, then:
            1. Let |v| be |result|.\[[Value]].
            1. If |v| [=implements=] {{Exception}},
-                1. Let |type| be |v|.\[[Type]].
-                1. Let |payload| be |v|.\[[Payload]].
+                1. Let |address| be |v|.\[[Address]].


Does a WebAssembly.Exception object have Address?

Yes, Address is initialized in "initialize an Exception object". Exception is now just like all the other JS-exposed objects which also have their wasm Addresses stored in an internal slot in the JS object. (Although I just noticed that the naming convention isn't consistent right now; The Exception and Tag objects have the address in a field named Address whereas the rest have that field named after the object type, e.g. Global. Maybe I'll fix that in a followup change).

aheejin · 2024-04-09T14:58:26Z

document/js-api/index.bs

-            1. Let |opaqueData| be [=ToWebAssemblyValue=](|v|, [=externref=])
-            1. [=WebAssembly/Throw=] with |type|, |payload| and |opaqueData|.
+                1. Let |type| be the result of [=get the JavaScript exception tag |getting the JavaScript exception tag=].
+                1. Let |payload| be [=!=] [=ToWebAssemblyValue=](|v|, [=externref=]).


It looks v contains Type and Value. Shouldn't we only convert the Value part, rather than the whole v?

The Type and Value fields come from ECMAScript completion record, and result is the completion record.
On lines 1086 - 1087 above, result.Type is THROW and then v is result.Value, i.e. the value returned by the JS host function called on line 1081, so it's already a value. So I think that's the thing that we need to convert to a wasm externref and throw back into wasm.

... actually though, I just noticed that the handling of completion records between 'run a host function' and 'create a host function' wasn't quite right even before the EH spec. I just filed WebAssembly/spec#1743 about that.

document/js-api/index.bs

thibaudmichaud · 2024-04-10T11:32:07Z

document/js-api/index.bs

+        1. If |tagaddr| is equal to |jsTagAddr|,
+            1. Throw the result of [=retrieving an extern value=] from |payload|[0].


There is an optimization that was discussed before where the engine does not wrap/unwrap the exception at the boundary, and instead implements the JSTag behavior as a special-case in the catch handler. But with that optimization, if the user code creates a WebAssembly.Exception(WebAssembly.JSTag, ...) object explicitly and throws it, it would not get unwrapped automatically at the export, which would violate this spec, right?
We can still benefit from the optimization and add a check when the object crosses the JS/Wasm boundary to cover this special case, but I just wanted to make sure that this is indeed what this spec requires us to do.

I was thinking about that issue as well while porting the JSTag behavior to the exnref implementation in V8. I was wondering if we could disallow creating a WebAssembly.Exception(JSTag, ...) in the first place? As in, throw a RangeError or some other exception from the constructor of WA.Exception. That would probably simplify a number of things, both at the spec and implementation level.

Would that behavior be acceptable?

I would describe the spec as currently written as saying that regular JS exceptions just never get wrapped or unwrapped, the payload just passes through (although like all exceptions the payload is paired with a tag, even if that tag is not exposed to the user). I think as written today, if you created a WebAssembly.Exception(WebAssembly.JSTag, payload) and threw it into wasm, it would be treated as a wasm exception on entry and unrapped; and then when it came back out to JS it would not be rewrapped but come out as just payload. If I'm understanding the optimization you describe, it would naturally have exactly this behavior, so there wouldn't need to be special cases.

Having said that, that behavior is a little weird and may be unexpected since it's asymmetric. I don't really see a use case for an explicitly-wrapped WebAssembly.Exception with the JS tag, so maybe it does make sense to disallow creation of such an object.

I also realized that this discussion should probably go in #269 since that PR is where the JS Tag object actually gets exposed to JS explicitly and makes that behavior possible.

dschuff · 2024-04-11T18:15:16Z

I'm going to go ahead and land this so I can rebase #269 and do some more cleanups. @aheejin let me know if you have more comments, we can update as needed.

As discussed in #202 The JS tag was added to the JS API spec in #301, but it is not observable. This change exposes it on the WebAssembly namespace, allowing it to be imported into wasm modules. This allows wasm modules to explicitly extract the thrown JS objects as externrefs from the caught exrefs, and also to throw fresh exceptions with externref payloads that will propagate into JS and can be caught as bare JS objects not wrapped in a WebAssembly.Exception. It also places the restriction that WebAssembly.Exception objects cannot be created with the JS tag, because it would result in ambiguity or asymmetry when such objects unwind from JS into wasm and back out.

dschuff and others added 12 commits March 16, 2023 16:25

review comments

15a47cf

swap order of error/exception in func_invoke

904a8fb

review suggestions

ec302e1

Apply suggestions from code review

fdf1b7c

Co-authored-by: Andreas Rossberg <[email protected]>

Merge branch 'main' into embedding-invoke

4799452

add exception allocation

f57cdb8

udpate for exnref

ccbee0f

apply all suggestions other than exn_read

4d08a58

add exn_read

74a92cb

Update allocation, initialization, construction and call-exported-fn

b17459a

fix linkage, syntax

fa07dea

dschuff requested review from Ms2ger, rossberg and aheejin March 29, 2024 22:18

dschuff added 2 commits March 29, 2024 15:19

fix typo

ea5217b

fix vars

1c958c2

rossberg reviewed Apr 2, 2024

View reviewed changes

document/js-api/index.bs Show resolved Hide resolved

document/js-api/index.bs Show resolved Hide resolved

Merge branch 'main' into exnref-js

46de060

Ms2ger reviewed Apr 3, 2024

View reviewed changes

document/js-api/index.bs Show resolved Hide resolved

document/js-api/index.bs Outdated Show resolved Hide resolved

document/js-api/index.bs Show resolved Hide resolved

address comments

5b8380a

dschuff added 3 commits April 4, 2024 14:42

remove Tag parameter from getArg

4d03423

remove 2 more spurious references to webassembly throw

d073cfe

more properly allocate the JS exception tag, and wrap/unwrap JS excep…

b6a79f9

…tions. Wasm throwing is still not properly defined.

dschuff commented Apr 5, 2024

View reviewed changes

rossberg reviewed Apr 5, 2024

View reviewed changes

dschuff added 2 commits April 5, 2024 11:37

apply review comments

51338d7

inline throwing into 'create a host function' and define in terms of …

dfdd0b9

…throw_ref

dschuff mentioned this pull request Apr 5, 2024

Expose the JavaScript Exception tag and allow importing it #269

Merged

Base automatically changed from embedding-invoke to main April 9, 2024 13:13

aheejin reviewed Apr 9, 2024

View reviewed changes

rossberg approved these changes Apr 9, 2024

View reviewed changes

document/js-api/index.bs Outdated Show resolved Hide resolved

dschuff mentioned this pull request Apr 9, 2024

[JS API] 'run a host function' and 'create a host function' don't consistently handle completion records WebAssembly/spec#1743

Open

apply suggestion

624bcca

thibaudmichaud reviewed Apr 10, 2024

View reviewed changes

dschuff merged commit c97651f into main Apr 11, 2024
5 checks passed

dschuff mentioned this pull request Apr 11, 2024

Proposed exnref value type semantics #282

Closed

aheejin deleted the exnref-js branch April 12, 2024 10:34

aheejin mentioned this pull request Dec 17, 2024

Fuzz error on JS trapping on exnref on the boundary WebAssembly/binaryen#7145

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update JS API for exnref #301

Update JS API for exnref #301

dschuff commented Mar 29, 2024 •

edited

Loading

eqrion commented Apr 3, 2024

dschuff commented Apr 4, 2024

rossberg commented Apr 4, 2024 •

edited

Loading

eqrion commented Apr 4, 2024

dschuff commented Apr 5, 2024

dschuff Apr 5, 2024

rossberg Apr 5, 2024

dschuff Apr 5, 2024

rossberg Apr 5, 2024

dschuff Apr 5, 2024

dschuff Apr 5, 2024

rossberg Apr 5, 2024

dschuff commented Apr 5, 2024

aheejin Apr 9, 2024

dschuff Apr 9, 2024

aheejin Apr 9, 2024

dschuff Apr 9, 2024

aheejin Apr 9, 2024

dschuff Apr 9, 2024

thibaudmichaud Apr 10, 2024

sjrd Apr 10, 2024

dschuff Apr 10, 2024

dschuff Apr 10, 2024

dschuff commented Apr 11, 2024

	1. Let \|payload\| be [=ToWebAssemblyValue=](\|v\|, [=externref=]).
	1. Let \|payload\| be [=!=] [=ToWebAssemblyValue=](\|v\|, [=externref=]).

		1. If \|tagaddr\| is equal to \|jsTagAddr\|,
		1. Throw the result of [=retrieving an extern value=] from \|payload\|[0].

Update JS API for exnref #301

Update JS API for exnref #301

Conversation

dschuff commented Mar 29, 2024 • edited Loading

eqrion commented Apr 3, 2024

dschuff commented Apr 4, 2024

rossberg commented Apr 4, 2024 • edited Loading

eqrion commented Apr 4, 2024

dschuff commented Apr 5, 2024

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

dschuff commented Apr 5, 2024

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

dschuff commented Apr 11, 2024

dschuff commented Mar 29, 2024 •

edited

Loading

rossberg commented Apr 4, 2024 •

edited

Loading