Update JS API to better specify opaqueData exception identity

[dschuff]

No description provided.

[aheejin]

Thanks for the PR! But I guess I still wasn't able to fully grasp what you mean by using the map of externaddr -> object you said in #248 (comment). Most of my questions in #248 (comment) still remain.

[aheejin]

Does this (preexisting) line mean we have the reverse mapping from opaqueData (or object) to externaddr..?

[dschuff]

My interpretation of the preexisting line is that the core spec was expected to be updated to say that func_invoke (from the core spec) returns the opaqueData (and is also expected to say where opaqueData comes from). There is no need for a reverse mapping because opaqueData is the externaddr and is passed directly into this step. I think of this like as basically just being a cast from opaqueData (a construct that was to be defined in the core spec) to externaddr (a construct that is used in this spec as a key in the map).

[aheejin]

How can we retrieve this opaqueData from payload? This is what I was asking about in #248 (comment). Also what exactly is "the returned exception"? Is that opaqueData or a combination of a tag and a payload?

[dschuff]

IMO "payload" is a separate concept from identity. Different exceptions (with different identity) can have the same payload, so the identity/opaqueData can't be uniquely identified from the payload. As written here, the exact mechanism used to track the identity is unspecified; it just says that exceptions have a unique identity and says where it needs to be created and preserved.

[aheejin]

Why?

[dschuff]

Same reason as above.

[aheejin]

How? Also I'm not sure what exactly is "the thrown exception". Is that a combination of a tag and a payload? Do we have an object that represents that combination which can be used as a key to some map? (And where do we have that map? Don't we only have a map of externaddr -> object?)

[dschuff]

This "association" is defined a little more down in the definition of Throw below.
(but really it's the hand-wavy bit).
But I just realized I wrote the definition of throw as taking opaqueData as an argument, so there's no need to use the term "association" or "thrown" exception here. It should just say
[=WebAssembly/Throw=] with |type|, |payload|, and |opaqueData| as it did before, and then the definition down below should say what the association means. I'll make that update here, and put more replies down there.

[dschuff]

My interpretation of the preexisting line is that the core spec was expected to be updated to say that func_invoke (from the core spec) returns the opaqueData (and is also expected to say where opaqueData comes from). There is no need for a reverse mapping because opaqueData is the externaddr and is passed directly into this step. I think of this like as basically just being a cast from opaqueData (a construct that was to be defined in the core spec) to externaddr (a construct that is used in this spec as a key in the map).

[dschuff]

This "association" is defined a little more down in the definition of Throw below.
(but really it's the hand-wavy bit).
But I just realized I wrote the definition of throw as taking opaqueData as an argument, so there's no need to use the term "association" or "thrown" exception here. It should just say
[=WebAssembly/Throw=] with |type|, |payload|, and |opaqueData| as it did before, and then the definition down below should say what the association means. I'll make that update here, and put more replies down there.

[dschuff]

But by the "thrown exception" I really mean the unique identity that the exception has as it propagates through the frames. The definition down below is what says that the identity needs to be preserved across a rethrow. Just a combination of tag+payload wouldn't be enough, because there could be more than one identical tag+payload (e.g. if an exception was thrown from JS, caught and swallowed in wasm, and then a fresh exception thrown from inside wasm with the same numeric payload; you would want to be able to tell the difference, it would have no stack trace, etc).
As you say, we only have a map of externaddr -> object, but because tag+payload isn't unique we can't have a mapping in this spec from tag+payload to externaddr (or any other form of unique identity).

Instead the implementation needs to create some kind of association between the propagating exception's identity and the opaqueData (i.e. its identity as defined in this spec). It does so in an unspecified way, that's the hand-wavy part: we just assume that this identity exists and can be tracked, and we specify that the identity is maintained across a rethrow, and that fresh throws start with a null identity.

[rossberg]

I may be a bit confused by all this, but why is there a distinction between payload and opaque data?

The way I would have assumed this all to be modelled is as follows:

1. The magic JS tag has a single parameter of type externref, which is the actual JS value. That way, the JS value can be obtained inside Wasm if need be.

2. A Wasm exception with the magic JS tag is converted to the JS value itself when escaping to JS. Vice versa, any plain JS value thrown that is not a Wasm exception is wrapped into that tag when escaping from JS to Wasm.

3. The mapping between the JS value and the Wasm extern address representing the exception's payload inside Wasm is what's stored in the map.

4. For all other exceptions, i.e., proper Wasm exceptions, the inverse happens: they are wrapped into an JS value on the way out, and unwrapped on the way back in.

I believe that this way, we do not even need to "patch" the core spec (at least not to handle JS exceptions), since the identity of the JS value is preserved in terms of the externref payload.

Does this make sense?

That leaves the question whether we should also guarantee that a Wasm exception maintains its JS-side identity, once observed. Specifically, consider this sandwich example:

(module
  (tag $e)
  (func $g (import "m" "g"))
  (func $f (export "f) (throw $e))
  (func $h1 (export "h1")
    (call $g)  ;; calls $f
  )
  (func $h2 (export "h2")
    (try (call $g) (catch $e (rethrow 0)))  ;; calls $f
  )
)

let instance = new Instance(module, {m: {g}})
let exn

function g() {
  try {
    instance.exports.f()  // throws Wasm exception
  } catch (e1) {
    exn = e1
    throw e1
  }
}

try {
  instance.exports.h1()  // calls g
} catch (e2) {
  assert (e1 === e2)  // is this guaranteed to hold?
}

try {
  instance.exports.h2()  // calls g
} catch (e2) {
  assert (e1 === e2)  // is this guaranteed to hold?
}

To be honest, I'm not sure it's worth guaranteeing either. I don't see the use case, and it would not be easy to specify. In fact, it would be easier to say that this equivalence never holds (by creating a fresh JS wrapper each time a Wasm exception escapes).

[rossberg]

Why is the tag address referred to as a "type" here?

[dschuff]

We'll fix the tag/type wording in a separate change.

[Ms2ger]

3. The mapping between the JS value and the Wasm extern address representing the exception's payload inside Wasm is what's stored in the map.

As far as I understand, there is no Wasm extern address representing the exception's payload.

[rossberg]

@Ms2ger, well, that's exactly what I'm suggesting, though: by exposing it as an externref, we'd naturally assign it such an address, and nothing else would be needed (AFAICS).

[dschuff]

Regarding your numbered points 1-4 about the JS tag:
This is not the behavior that the current spec has (or that engines currently implement) e.g. currently we specify that the JS tag has an empty payload and can't be imported. But I actually agree that this behavior makes sense and is what we want (Heejin and I have actually been discussing adding a solution like this in response to user requests to allow the JS value to be obtained inside wasm just as you mentioned). So we should add that.

However, it's not actually what this change is trying to accomplish; this change is trying to accomplish your second question, of whether we should guarantee that a wasm exception maintains its JS-side identity.
The answer there is yes, we should, and the implementations currently shipping in web engines do.
(Heejin also recently added a test similar to your example).
This is because Error objects thrown from JS and WebAssembly.Exception objects created in JS (as emscripten does here) need to preserve their properties (especially their attached stack traces) as they propagate through wasm frames all the way out to outermost try/catch blocks or onerror handlers. (This use case doesn't strictly require that the objects be === but it does require that the properties are the same). Many users obviously depend on being able collect these stack traces regardless of whether they propagate through wasm frames after being thrown, or whether they originate from JS or emscripten code.

I agree that it is not easy to specify! That's why the existing opaqueData situation and this proposed amendment are so complicated and hand-wavy! But it's a very important property to have. And, it's apparently not difficult to implement. The first JS implementations actually had this identity property even before the spec said that they should.

[rossberg]

Ah, thanks for the clarification. Now I see what this PR is trying to accomplish. However, I think there's still a problem, see comment below.

Another thing I noticed and that should be fixed is that the text repeatedly refers to exception tags as "types" (including with the name of the [[Type]] property). But that is a category error, a tag is not a type. It is a (second-class) value which has a type. In particular, multiple different tags can have the same type. This spec is only concerned with tag values (i.e., identities), and does not care about their types.

(The view of tags as types probably originates from OO languages traditionally piggybacking exceptions on nominal class types, which provide an identity through their runtime class. But that's not what's happening here.)

[rossberg]

This seems to assume that there is a catch block to handle it, but of course that does not need to be the case, and it is not when the exception actually escapes to JS.

[dschuff]

Indeed. Ideally this text should probably say something like "create an exception and unwind the stack" according to the language of the core spec, and "do whatever the core spec says should be done with exceptions"; i.e. unwind until the exception is caught or propagate it back to the embedder. Is there a particular spot in the core spec that defines that creation?

[dschuff]

Maybe what we actually need to do is define some kind of interface in the embedding appendix for that (that appendix at least needs to be updated to say what happens when an invoked function raises/propagates an exception). The question still remains though, since that interface will itself need to say what happens.

[dschuff]

#268 is the beginning of what I meant by the above comment; although I noticed that it doesn't say anything about calls from the instance into the embedder, so there may not be anything relevant to the case we're talking about here.

[rossberg]

Suggested change

	1. Let |opaqueData| be the [=externref=] assocated with the returned exception.
	1. Let |opaqueData| be the [=externref=] associated with the returned exception.

Though I think this wording is a bit insufficient. If the exception originates from a Wasm throw (and has not yet escaped to JS), then there is no externref associated with it yet, and you are actually have to create that association at this point. So I believe you need to distinguish this case (which should be the common case).

[dschuff]

The change down on line 1373 attempts to get at this. Those lines are the ones that really modify the behavior specified in the core spec, so it's not 100% obvious where they should go.
The rule that says "fresh exceptions created by wasm throw get null identities" could reasonably go here I think; but it's less obvious where the "identities propagate to JS" and "rethrows preserve identity" rules go, if not there.

I modified 1373 to try to make it a little more clear.

[aheejin]

Is this still relevant? If not, can we close this?

[dschuff]

Is this still relevant? If not, can we close this?

It's not relevant for shipping exnref to phase4. We do still have the appendix with the "legacy" format. I haven't looked at it yet and don't know whether it also includes anything about the JS API but I was thinking it might still be worthwhile to say something about how it should work, since browsers will be the most likely engines to have support for it in the medium/long term. At the very least we should probably still have the legacy tests somewhere.