Skip to content

Commit

Permalink
Bug 1716043 - Support Wasm try block with no other clauses r=rhunt
Browse files Browse the repository at this point in the history
The proposal spec for exception handling was recently changed
to allow a `try` block with no `catch` or `delegate` clauses:

  WebAssembly/exception-handling#157

Differential Revision: https://phabricator.services.mozilla.com/D118088
  • Loading branch information
takikawa committed Jun 21, 2021
1 parent af292c6 commit 12526a3
Show file tree
Hide file tree
Showing 5 changed files with 85 additions and 33 deletions.
39 changes: 39 additions & 0 deletions js/src/jit-test/tests/wasm/exceptions/instructions.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,44 @@
// Tests for Wasm exception proposal instructions.

// Test try blocks with no handlers.
assertEq(
wasmEvalText(
`(module
(func (export "f") (result i32)
try (result i32) (i32.const 0) end))`
).exports.f(),
0
);

assertEq(
wasmEvalText(
`(module
(func (export "f") (result i32)
try (result i32) (i32.const 0) (br 0) (i32.const 1) end))`
).exports.f(),
0
);

assertEq(
wasmEvalText(
`(module
(type (func))
(event $exn (type 0))
(func (export "f") (result i32)
try (result i32)
try (result i32)
(throw $exn)
(i32.const 1)
end
drop
(i32.const 2)
catch $exn
(i32.const 0)
end))`
).exports.f(),
0
);

// Test trivial try-catch with empty bodies.
assertEq(
wasmEvalText(
Expand Down
11 changes: 11 additions & 0 deletions js/src/jit-test/tests/wasm/exceptions/throw-to-js.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,17 @@ function assertWasmThrowsExn(thunk) {
assertEq(thrown, true, "missing exception");
}

// Test that handler-less trys don't catch anything.
assertWasmThrowsExn(() =>
wasmEvalText(
`(module
(type (func (param)))
(event $exn (type 0))
(func (export "f")
try (throw $exn) end))`
).exports.f()
);

// Test throwing simple empty exceptions to JS.
assertWasmThrowsExn(() =>
wasmEvalText(
Expand Down
46 changes: 21 additions & 25 deletions js/src/jit-test/tests/wasm/exceptions/validation.js
Original file line number Diff line number Diff line change
Expand Up @@ -75,31 +75,6 @@ function testValidateDecode() {
/expected event index/
);

// Try blocks must have a second branch after the main body.
wasmInvalid(
moduleWithSections([
sigSection([emptyType]),
declSection([0]),
eventSection([{ type: 0 }]),
bodySection([
funcBody({
locals: [],
body: [
TryCode,
I32Code,
I32ConstCode,
0x01,
// Missing instruction here.
EndCode,
DropCode,
ReturnCode,
],
}),
]),
]),
/try without catch or catch_all not allowed/
);

// Rethrow must have a depth argument.
wasmInvalid(
moduleWithSections([
Expand Down Expand Up @@ -244,6 +219,27 @@ function testValidateTryCatch() {
wasmValid(valid1);
wasmInvalid(invalid1, /unused values not explicitly dropped/);
wasmValid(valid2);

// Test handler-less try blocks.
wasmValidateText(
`(module (func try end))`
);

wasmValidateText(
`(module (func (result i32) try (result i32) (i32.const 1) end))`
);

wasmValidateText(
`(module
(func (result i32)
try (result i32) (i32.const 1) (br 0) end))`
);

wasmFailValidateText(
`(module
(func try (result i32) end))`,
/popping value from empty stack/
);
}

function testValidateCatch() {
Expand Down
16 changes: 14 additions & 2 deletions js/src/wasm/WasmBaselineCompile.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -10262,9 +10262,21 @@ bool BaseCompiler::emitEnd() {
}
break;
#ifdef ENABLE_WASM_EXCEPTIONS
case LabelKind::Try:
MOZ_CRASH("Try-catch block cannot end without catch.");
case LabelKind::Try: {
// The beginning of a `try` block introduces this try note, but
// we need to remove it in case we have no handlers in this block
// as it can never be the target of an exception.
WasmTryNoteVector& tryNotes = masm.tryNotes();
// This will shift the indices of any try notes in between the `try`
// instruction and this `end`. This is fine as long as try notes are
// only accessed through ControlItems, as the shifted try notes will
// correspond only to items that are popped by this point.
tryNotes.erase(&tryNotes[controlItem().tryNoteIndex]);
if (!endBlock(type)) {
return false;
}
break;
}
case LabelKind::Catch:
case LabelKind::CatchAll:
if (!endTryCatch(type)) {
Expand Down
6 changes: 0 additions & 6 deletions js/src/wasm/WasmOpIter.h
Original file line number Diff line number Diff line change
Expand Up @@ -1277,12 +1277,6 @@ inline bool OpIter<Policy>::readEnd(LabelKind* kind, ResultType* type,

Control& block = controlStack_.back();

#ifdef ENABLE_WASM_EXCEPTIONS
if (block.kind() == LabelKind::Try) {
return fail("try without catch or catch_all not allowed");
}
#endif

if (!checkStackAtEndOfBlock(type, results)) {
return false;
}
Expand Down

0 comments on commit 12526a3

Please sign in to comment.