winch: catch exceptions

[macovedj]

Adds support for catching exceptions in Winch.
Winch tracks exception handlers in scope, records them in call-site metadata, and emits landing pads when a try_table ends. When Wasmtime’s unwinder selects a handler, its landing pad restores the expected stack state and VMContext.
Support for catch_ref and catch_all_ref is not yet implemented and will be added in follow-up work

[github-actions]

Subscribe to Label Action

cc @saulecabrera

Details

This issue or pull request has been labeled: "cranelift", "winch"

Thus the following users have been cc'd because of the following labels:

• saulecabrera: winch

To subscribe or unsubscribe from this label, edit the .github/subscribe-to-label.json configuration file.

Learn more.

[cfallin]

Thanks -- this looks pretty much OK to me (and the handler stack with checkpoints appears to be using the same approach that we have in the Wasm-to-CLIF translator, which is good). A few questions around the libcalls/barriers and register management below. It might also be good to get a second pair of eyes (@saulecabrera maybe?) to ensure that all of that is done properly.

[cfallin]

It seems that we have logic to re-compute object_addr (field_base) every iteration if invalidated. A few thoughts:

• Do we need to initialize object_addr above (line 178) if we're going to lazily recompute it if None in each iteration? In other words would None as an initializer work?
• Zooming out, why are we recomputing it? It seems that below we set it to None when a call (DRC barrier) clobbers registers, but we save the other registers; why do we need to re-derive object_addr from scratch? Does the read barrier have the option to relocate the heap or similar?
• All of these manual register save/restore sequences make me a little squeamish. Do we not have an abstraction to save active registers and restore them around calls?

[macovedj]

Thanks for pointing this out. On revisiting it, I realized the recomputation was there because the barrier was responsible for freeing the storage_base register. I changed the barrier to operate on an already-loaded GC reference, leaving responsibility for freeing the storage_base with the caller. This lets the exception path preserve the already-computed object_addr rather than recomputing it.

I'm not aware of a mechanism available other than the value stack for managing the registers, but these changes simplify the sequence by preserving the already-computed object_addr instead of preserving exception_reg and later reloading the heap metadata to reconstruct the address. If @saulecabrera has any thoughts about some potential helpers that could assist, I'm open to suggestions. Otherwise, the simplified barrier contract may be sufficient.

[saulecabrera]

It might also be good to get a second pair of eyes (@saulecabrera maybe?) to ensure that all of that is done properly.

I will take a look; thanks!

[saulecabrera]

With this branch, this test is failing for me:

(module
  (tag $e (param i32))
  (func $callee (result i32) (i32.const 9))
  (func (export "f") (result i32)
    (block $h (result i32)
      (try_table (result i32 i32 i32 i32 i32 i32) (catch $e $h)
        (i32.const 1) (i32.const 2) (i32.const 3) (i32.const 4) (i32.const 5)
        (call $callee))
      drop drop drop drop drop)))

$ wasmtime compile -W exceptions -C compiler=winch -C collector=copying -o out.cwasm repro.wat
Error: failed to compile: wasm[0]::function[1]

Caused by:
    Compilation error: Winch internal error: Invalid stack pointer offset

Sorry it took me a bit to review, I was trying to see if I could pinpoint exactly where the failure is, but it is probably faster for you.

The symptom is probably related to how the fall-through case is handled though; we need to be very careful when manipulating the sp in the Masm, as it always needs to be in sync with the machine's

[saulecabrera]

Can we allocate this register inside this function? It's generally easier to reason about register allocation when scoped to a particular function and avoids unwanted clobbering.

[saulecabrera]

Pressed enter too fast, could we also:

• Add tests for landing pads in aarch64
• Add tests with the null collector for caught payloads
• Add tests for blocks with multi-value