Skip to content

Could WebAssembly.validate optionally provide a diagnostic? #1000

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
bnjbvr opened this issue Feb 27, 2017 · 7 comments
Open

Could WebAssembly.validate optionally provide a diagnostic? #1000

bnjbvr opened this issue Feb 27, 2017 · 7 comments

Comments

@bnjbvr
Copy link
Member

bnjbvr commented Feb 27, 2017

At the moment, WebAssembly.validate only returns a simple boolean answer, which is enough for feature detection. It might be useful to also get an error message and an indication where the error is. Currently, the only way to get a meaningful error message is to create a Module and catch the error, as done in the test harness.

The proposal is the following: we might have a second argument to WebAssembly.validate, which would be a function callback onError. Whenever we hit an error in the binary code passed to validate, the onError callback would be called with an object containing the following properties:

  • an absolute offset into the module binary, which would be a Number.
  • a String error message describing what is going wrong.

Another possibility is that the onError callback be just called with a string that contains the offset, but that is harder to programmatically handle, which might be useful for e.g. wasm Web IDEs. Devtools may also use the offset to create a link to the corresponding wasm text format (or source map) around the offset.

Going the opposite direction, we could also have an extended object with more properties describing the error, including error codes. Each error code would be documented and could have specific information in supplementary object properties. For instance, there could be one error code describing a type mismatch when consuming arguments to an opcode; additional error-code specific properties in the object could be expected (an array of types that were expected on the top of the stack) and observed (an array of the types present on the top of the stack).

As this would be a facultative argument, the diagnostic string wouldn't get generated if the argument is not provided, making this feature "pay-as-you-go", which is nice for implementations.

Having a callback per WebAssembly.validate call is a bit clumsy, since the onError callback would be probably stable across calls. Another option would be to have a global property WebAssembly.validate.onerror (the same way we have window.onerror), defaulting to null, enabling diagnostic string generation for all the validated modules iff it's set to a function.

@RyanLamansky
Copy link

I was hoping that the 1000th issue would be something interesting, and was not disappointed 🙂

I'm definitely against a global property for all the usual reasons globals are bad, but don't have a strong opinion on parameters vs. an object as input to the onError callback.

I think an error code is valuable, but who defines these codes, and how are WASM bugs vs implementation limits distinguished?

More broadly, this puts an additional burden on implementations to support the expanded specification. Maybe this is an optional feature?

@jfbastien
Copy link
Member

jfbastien commented Feb 28, 2017

This is for the JS API. It should not be optional for that API.

If we do offsets, we should use the same offset (file or function) as for the issue @dschuff filed last week.

I like onError in general, with error string. An overload sounds good, but we can invert its truthiness.

Sorry for terse, am carrying 👶🌯

@rossberg
Copy link
Member

rossberg commented Feb 28, 2017

I would propose that the onError function is given an Error object, the exact same one that would be thrown as an exception by compile. That way there's no bifurcation of error reporting formats, and clients need only one set of code for processing them. We can then extend the information provided by these objects (e.g., adding custom properties to CompileError) and all uses benefit uniformly.

I'd be absolutely opposed to making onError into global state instead of a parameter. Being higher-order that would be even worse than the errno mess on Unix (and similar mechanisms on Windows). It's like building an API that requires everybody to do (and compete on) monkey patching. What can possibly go wrong? :)

@jfbastien
Copy link
Member

I like Andreas's proposal.

@lukewagner
Copy link
Member

Yes, sounds fine to me. Presumably this would actually be a WebAssembly.CompileError (or WebAssembly.InternalError if we decided on that).

If we wanted to pull out bytecode offset or any other info (to avoid requiring clients to write per-UA custom error parsing) we could orthogonally add a .offset properties to these error objects (which would also help users of compile as well).

@jfbastien
Copy link
Member

@bnjbvr would you be willing to tackle this and provide a PR? @rossberg-chromium 's proposal seems acceptable to all.

@bnjbvr
Copy link
Member Author

bnjbvr commented May 12, 2017

@jfbastien Yes, will do next week.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants