Fix usage of GC barriers when throwing an exnref

[alexcrichton]

• set_pending_exception didn't have a write barrier to handle any
  previously configured exception.
• When an exnref was caught it didn't go through expose_gc_ref_to_wasm.
• During throw_impl the exnref wasn't cloned to give a strong
  reference to the store.

All of these should now be fixed with some minor refactorings and such
to ensure we've got the right barriers in the right places.

Closes #13316

[github-actions]

Subscribe to Label Action

cc @fitzgen

Details

This issue or pull request has been labeled: "wasmtime:api", "wasmtime:ref-types"

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

• fitzgen: wasmtime:ref-types

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

Learn more.

[fitzgen]

Why this change? Seems like a step backwards, and we should always be able to cast a VMExnRef to a VMGcRef via helper methods if necessary.

[alexcrichton]

The main reason was gc_store.write_gc_ref(&mut self.pending_exception, Some(exnref)) where it was easier to change this than to add a full matrix of helpers for VMExnRef for this one call. How strongly do you feel this should remain VMExnRef?

[fitzgen]

Do we not already have as_gc_ref[_mut] helpers for VMExnRef like we do for VM{Struct,Array,...}Ref? Then we probably should, and they should each just be one liners

[alexcrichton]

We've got conversions from &mut VMExnRef -> &mut VMGcRef, but this is a different conversion from &mut Option<VMExnRef> -> &mut Option<VMGcRef> which I don't believe we have any precedent for

[fitzgen]

Unless you feel strongly, I'd prefer to keep this as VMExnRef and add the Option-based helpers as necessary

[alexcrichton]

Personally I'm pretty wary to add more helpers over what we already have. GC already suffers from quite a lot of code duplication which makes it difficult to change things and keep everything in-sync. Adding more conversions with more unsafe implementations (I don't think there's any way to do this safely or built on existing primitives) for a small amount of what is effectively documentation I feel isn't worth it.

For example I think that this would require this on every VM*Ref type:

     pub fn option_slot_as_gc_ref(slot: &mut Option<VMExnRef>) -> &mut Option<VMGcRef> {
         // SAFETY: `Option<VMExnRef>` and `Option<VMGcRef>` have the same layout, since
         // `VMExnRef` is `repr(transparent)` over `VMGcRef`.
         unsafe { &mut *(slot as *mut Option<VMExnRef> as *mut Option<VMGcRef>) }
     }

On top of that we'd theoretically need the inverse as well, going from &mut Option<VMGcRef> back to the typed representation eventually. On top of that most of these wouldn't be used so they'd be #[expect(dead_code)] or similar for now.

Overall it feels like quite a lot of infrastructure for not really much benefit. This only affects one field in one location which is already named "pending exception" and I can touch up the documentation to clearly indicate that it needs to be a VMExnRef.

[fitzgen]

Okay, that sounds fine for now. I think we can address these issues with a macro to reduce code duplication in a follow up though.

This only affects one field in one location which is already named "pending exception" and I can touch up the documentation to clearly indicate that it needs to be a VMExnRef.

Can you add debug-asserts if possible too? Fine to skip if ultimately too difficult.

[alexcrichton]

Sure yeah, most of the debug asserts were already there and I went ahead and added another that seemed appropriate too