Skip to content

Commit

Permalink
[WebAssembly] Fix unwind mismatches in new EH (#114361)
Browse files Browse the repository at this point in the history
This fixes unwind mismatches for the new EH spec.

The main flow is similar to that of the legacy EH's unwind mismatch
fixing. The new EH shared `fixCallUnwindMismatches` and
`fixCatchUnwindMismatches` functions, which gather the range of
instructions we need to fix their unwind destination for, with the
legacy EH. But unlike the legacy EH that uses `try`-`delegate`s to fix
them, the new EH wrap those instructions with nested
`try_table`-`end_try_table`s that jump to a "trampoline" BB, where we
rethrow (using a `throw_ref`) the exception to the correct `try_table`.

For a simple example of a call unwind mismatch, suppose if `call foo`
should unwind to the outer `try_table` but is wrapped in another
`try_table` (not shown here):
```wast
try_table
  ...
  call foo    ;; Unwind mismatch. Should unwind to the outer try_table
  ...
end_try_table
```

Then we wrap the call with a new nested `try_table`-`end_try_table`, add
a `block` / `end_block` right inside the target `try_table`, and make
the nested `try_table` jump to it using a `catch_all_ref` clause, and
rethrow the exception using a `throw_ref`:
```wast
try_table
  block $l0 exnref
    ...
    try_table (catch_all_ref $l0)
      call foo
    end_try_table
    ...
  end_block             ;; Trampoline BB
  throw_ref
end_try_table
```

---

This fixes two existing bugs. These are not easy to test independently
without the unwind mismatch fixing. The first one is how we calculate
`ScopeTops`. Turns out, we should do it in the same way as in the legacy
EH even though there is no `end_try` at the end of `catch` block
anymore. `nested_try` in `cfg-stackify-eh.ll` tests this case.

The second bug is in `rewriteDepthImmediates`. `try_table`'s immediates
should be computed without the `try_table` itself, meaning
```wast
block
  try_table (catch ... 0)
  end_try_table
end_block
```
Here 0 should target not `end_try_table` but `end_block`. This bug
didn't crash the program because `placeTryTableMarker` generated only
the simple form of `try_table` that has a single catch clause and an
`end_block` follows right after the `end_try_table` in the same BB, so
jumping to an `end_try_table` is the same as jumping to the `end_block`.
But now we generate `catch` clauses with depths greater than 0 with when
fixing unwind mismatches, which uncovered this bug.

---

One case that needs a special treatment was when `end_loop` precedes an
`end_try_table` within a BB and this BB is a (true) unwind destination
when fixing unwind mismatches. In this case we need to split this
`end_loop` into a predecessor BB. This case is tested in
`unwind_mismatches_with_loop` in `cfg-stackify-eh.ll`.

---

`cfg-stackify-eh.ll` contains mostly the same set of tests with the
existing `cfg-stackify-eh-legacy.ll` with the updated FileCheck
expectations. As in `cfg-stackify-eh-legacy.ll`, the FileCheck lines
mostly only contain control flow instructions and calls for readability.
- `nested_try` and `unwind_mismatches_with_loop` are added to test newly
found bugs in the new EH.
- Some tests in `cfg-stackify-eh-legacy.ll` about the legacy-EH-specific
asepcts have not been added to `cfg-stackify-eh.ll`.
(`remove_unnecessary_instrs`, `remove_unnecessary_br`,
`fix_function_end_return_type_with_try_catch`, and
`branch_remapping_after_fixing_unwind_mismatches_0/1`)
  • Loading branch information
aheejin authored Nov 5, 2024
1 parent e28d440 commit 380fd09
Show file tree
Hide file tree
Showing 3 changed files with 2,205 additions and 33 deletions.
30 changes: 30 additions & 0 deletions llvm/lib/Target/WebAssembly/MCTargetDesc/WebAssemblyMCTargetDesc.h
Original file line number Diff line number Diff line change
Expand Up @@ -478,6 +478,22 @@ inline bool isMarker(unsigned Opc) {
}
}

inline bool isEndMarker(unsigned Opc) {
switch (Opc) {
case WebAssembly::END_BLOCK:
case WebAssembly::END_BLOCK_S:
case WebAssembly::END_LOOP:
case WebAssembly::END_LOOP_S:
case WebAssembly::END_TRY:
case WebAssembly::END_TRY_S:
case WebAssembly::END_TRY_TABLE:
case WebAssembly::END_TRY_TABLE_S:
return true;
default:
return false;
}
}

inline bool isTry(unsigned Opc) {
switch (Opc) {
case WebAssembly::TRY:
Expand Down Expand Up @@ -510,6 +526,20 @@ inline bool isCatch(unsigned Opc) {
}
}

inline bool isCatchAll(unsigned Opc) {
switch (Opc) {
case WebAssembly::CATCH_ALL_LEGACY:
case WebAssembly::CATCH_ALL_LEGACY_S:
case WebAssembly::CATCH_ALL:
case WebAssembly::CATCH_ALL_S:
case WebAssembly::CATCH_ALL_REF:
case WebAssembly::CATCH_ALL_REF_S:
return true;
default:
return false;
}
}

inline bool isLocalGet(unsigned Opc) {
switch (Opc) {
case WebAssembly::LOCAL_GET_I32:
Expand Down
Loading

0 comments on commit 380fd09

Please sign in to comment.