Open Bug 1335652 Opened 3 years ago Updated 7 days ago

Exception handling

Categories

(Core :: Javascript: WebAssembly, enhancement, P3)

enhancement

Tracking

()

ASSIGNED

People

(Reporter: dmajor, Assigned: lth)

References

Details

Attachments

(7 files, 3 obsolete files)

WIP patches for parsing, AST, text, etc. of try/catch/throw operators.

Currently these are just structural placeholders with no actual exception handling wired up.

For now, `throw` is more or less a `drop` that only accepts I32; `try` is like an `if` with no condition; and `catch` is like an `else` but with an element of `getLocal` for the catch-parameter.
This is roughly based on all the places where we have code for `growMemory`.
This works reasonably well and includes some tests.
Attachment #8832342 - Attachment is patch: true
Not sure what the deadline is. Marking as P3. Feel free to update.
Priority: -- → P3
Per policy at https://wiki.mozilla.org/Bug_Triage/Projects/Bug_Handling/Bug_Husbandry#Inactive_Bugs. If this bug is not an enhancement request or a bug not present in a supported release of Firefox, then it may be reopened.
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → INACTIVE
Status: RESOLVED → REOPENED
Resolution: INACTIVE → ---
Component: JavaScript Engine: JIT → Javascript: Web Assembly
Type: defect → enhancement
Summary: Wasm: Parsing for try/catch/throw → Exception handling: Parsing for try/catch/throw

Patches probably stale but exception handling is happening, so I will try to resurrect and see if any of this code is salvageable.

Assignee: nobody → lhansen

Text->binary translation, and hooks in all the large switches for new opcodes.

Part 1 (comment 7) extracts all the useful content from the original patches except binary parsing (ie the bodies of the WasmOpIter methods). Exception handling has changed a lot since the draft code was written, as have other aspects of the engine, so we'll need to write that from scratch, although part 3 of the original patch set contains useful sketches.

Status: REOPENED → ASSIGNED

Been reading the current spec overview, https://github.com/WebAssembly/exception-handling/blob/master/proposals/Exceptions.md. Mostly straightforward. Some notes.

There's a global space of opaque exception tags, these identify exceptions globally. If a module has a private exception E it has a tag that distinguishes it from all other exns in the system. If a module exports an exception E and some other module imports it, then the exported and imported exns have the same tag. For practical purposes, a tag could be a uint32_t acting as an index into a global table, or it could be a pointer to a descriptor in ditto table.

New exception type 'exnref' <: 'anyref' is added but is otherwise opaque; this represents a package of the values created by the throw along with the tag of the exception.

throw has two parts: box up the arguments from the stack and push an exception object, then effectively perform rethrow. Initially the box is most reasonably some JSObject*, as this allows for efficient representation as anyref, even if we might use less storage with other representations. One JSObject class for each signature of exception, probably.

br_on_exn is probably a simple comparison; if tags are ints then all sorts of techniques apply if we want to optimize a cluster of discriminants. (A sequence of br_on_exn instructions is basically a wordily encoded br_table.) Since the exn representation is opaque, loading of exception values at the branch target can be inlined from some optimized representation (though of course the loading happens along the branch edge so there's an intermediate jump island to handle the unboxing).

rethrow has two cases: if the try block is lexically nesting in the same function then it amounts to a branch; if not, it amounts to finding a nesting function call with a try block, unwinding to that function, and then branching. For a first cut, the latter case probably is something to handle in C++.

The only really difficult thing then is the unwinding logic for the hard part of rethrow, and the metadata we create for making that fast.

Summary: Exception handling: Parsing for try/catch/throw → Exception handling

This handles events, mostly.

Depends on D52625

Attached file Bug 1335652 - opiter + exnref. WIP (obsolete) —

Depends on D53939

Depends on D55430

Depends on D56520

Needs some ifdeffery

Depends on D56521

More ifdeffery wanted

Depends on D56522

Attachment #9108024 - Attachment is obsolete: true
Attachment #9110199 - Attachment is obsolete: true
Attachment #9110200 - Attachment is obsolete: true
Depends on: 1603496
Depends on: 1604453

There's an open spec issue (I opened it): is exnref nullable or not? Currently I assume it is not, but this assumption is not deeply baked into current code, and it is left as a TODO comment where the assumption may be depended on, if only in a comment.

My current patch set implements infrastructure, the exnref type, parsing of the new instructions and the events, and has a framework for validation and compilation with MOZ_CRASH("NYI") at the obvious leaves - in code generation functions and in the WasmOpIter readers notably. There are some test cases. There are probably bugs.

The main work items remaining are at least these:

  • solidifying the vague parts of the specification, I suspect there are numerous open issues both internal to wasm and in the JS API
  • validation
  • code generation for the individual instructions
  • designing a data representation for Exception event objects
  • designing a representation for unwind maps / the unwind logic
  • handling throws that propagate wasm -> JS and JS -> wasm, which may entail boxing/unboxing the wasm Exception in a JS exception

As far as the data representation of the Exception is concerned, I was thinking I would initially just use a JSObject with expandos since this would be simple, but there are certainly more efficient representations.

One might initially perhaps just implement exceptions that aren't allowed to cross the JS/wasm boundary, for simplicity.

(Patches updated now.)

One more thing about the patches: I decided to completely #ifdef this feature in all its aspects[*] because it is still very early stage; if we end up landing any of it, it's good to have the ifdefs as flags pointing us to the code that the feature touches. It would be nice to keep this up until the feature is late in stage 2 or so.

[*] With one exception: the text->binary tokenizer. Really this could be ifdeffed too.

(Patches updated now.)

You need to log in before you can comment on or make changes to this bug.