refactor recursive reentrance checks

This commit makes a few changes related to recursive reentrance checks, instance
poisoning, etc.:

- Implements the more restrictive lift/lower rules described in WebAssembly/component-model#589 such that a component instance may not lower a function lifted by one of its ancestors, nor vice-versa.  Any such lower will result in a fused adapter which traps unconditionally, preventing guest-to-guest recursive reentrance without requiring data flow analysis.
    - Note that this required updating several WAST tests which were violating the new rule.
    - This is handled entirely in the `fact` module now; I've removed the `AlwaysTrap` case previously handled by `wasmtime-cranelift`.
- Removes `FLAG_MAY_ENTER` from `InstanceFlags`.  It is no longer needed for guest-to-guest calls due to the above, and for guest-to-host-to-guest calls we can rely on either `FLAG_NEEDS_POST_RETURN` for sync-lifted functions or the `GuestTask` call stack for async-lifted functions.
- Adds a `StoreOpaque::trapped` field which is set when _any_ instance belonging to that store traps, at which point the entire store is considered poisoned, meaning no instance belonging to it may be entered.  This prevents indeterminant concurrent task state left over from the trapping instance from leaking into other instances.

Note that this does _not_ include code to push and pop `GuestTask` instances for
guest-to-guest sync-to-sync calls, nor for host-to-guest calls using e.g. the
synchronous `Func::call` API, so certain intrinsics which expect a `GuestTask`
to be present such as `backpressure.inc` will still fail in such cases.  I'll
address that in a later PR.

Fixes bytecodealliance#12128

Author: dicej

crates/cranelift/src/compiler/component.rs

@@ -1,6 +1,6 @@
 //! Compilation support for the component model.
 
-use crate::{TRAP_ALWAYS, TRAP_CANNOT_ENTER, TRAP_INTERNAL_ASSERT, compiler::Compiler};
+use crate::{TRAP_INTERNAL_ASSERT, compiler::Compiler};
 use cranelift_codegen::ir::condcodes::IntCC;
 use cranelift_codegen::ir::{self, InstBuilder, MemFlags, Value};
 use cranelift_codegen::isa::{CallConv, TargetIsa};
@@ -20,7 +20,6 @@ struct TrampolineCompiler<'a> {
     offsets: VMComponentOffsets<u8>,
     block0: ir::Block,
     signature: &'a WasmFuncType,
-    tunables: &'a Tunables,
 }
 
 /// What host functions can be called, used in `translate_hostcall` below.
@@ -100,7 +99,6 @@ impl<'a> TrampolineCompiler<'a> {
         component: &'a Component,
         types: &'a ComponentTypesBuilder,
         signature: &'a WasmFuncType,
-        tunables: &'a Tunables,
     ) -> TrampolineCompiler<'a> {
         let isa = &*compiler.isa;
         let func = ir::Function::with_name_signature(
@@ -117,7 +115,6 @@ impl<'a> TrampolineCompiler<'a> {
             offsets: VMComponentOffsets::new(isa.pointer_bytes(), component),
             block0,
             signature,
-            tunables,
         }
     }
 
@@ -160,21 +157,6 @@ impl<'a> TrampolineCompiler<'a> {
                     },
                 );
             }
-            Trampoline::AlwaysTrap => {
-                if self.tunables.signals_based_traps {
-                    self.builder.ins().trap(TRAP_ALWAYS);
-                    return;
-                }
-                self.translate_libcall(
-                    host::trap,
-                    TrapSentinel::Falsy,
-                    WasmArgs::InRegisters,
-                    |me, params| {
-                        let code = wasmtime_environ::Trap::AlwaysTrapAdapter as u8;
-                        params.push(me.builder.ins().iconst(ir::types::I32, i64::from(code)));
-                    },
-                );
-            }
             Trampoline::ResourceNew { instance, ty } => {
                 // Currently this only supports resources represented by `i32`
                 assert_eq!(self.signature.params()[0], WasmValType::I32);
@@ -1130,13 +1112,8 @@ impl<'a> TrampolineCompiler<'a> {
         //      brif should_run_destructor, run_destructor_block, return_block
         //
         //    run_destructor_block:
-        //      ;; test may_enter, but only if the component instances
-        //      ;; differ
-        //      flags = load.i32 vmctx+$instance_flags_offset
-        //      masked = band flags, $FLAG_MAY_ENTER
-        //      trapz masked, CANNOT_ENTER_CODE
-        //
-        //      ;; set may_block to false, saving the old value to restore later
+        //      ;; set may_block to false, saving the old value to restore
+        //      ;; later, but only if the component instances differ
         //      old_may_block = load.i32 vmctx+$may_block_offset
         //      store 0, vmctx+$may_block_offset
         //
@@ -1184,25 +1161,13 @@ impl<'a> TrampolineCompiler<'a> {
 
         self.builder.switch_to_block(run_destructor_block);
 
-        // If this is a defined resource within the component itself then a
-        // check needs to be emitted for the `may_enter` flag. Note though
-        // that this check can be elided if the resource table resides in
-        // the same component instance that defined the resource as the
-        // component is calling itself.
+        // If this is a defined resource within the component itself then the
+        // `may_block` field must be updated. Note though that this update can
+        // be elided if the resource table resides in the same component
+        // instance that defined the resource as the component is calling
+        // itself.
         let old_may_block = if let Some(def) = resource_def {
             if self.types[resource].unwrap_concrete_instance() != def.instance {
-                let flags = self.builder.ins().load(
-                    ir::types::I32,
-                    trusted,
-                    vmctx,
-                    i32::try_from(self.offsets.instance_flags(def.instance)).unwrap(),
-                );
-                let masked = self
-                    .builder
-                    .ins()
-                    .band_imm(flags, i64::from(FLAG_MAY_ENTER));
-                self.builder.ins().trapz(masked, TRAP_CANNOT_ENTER);
-
                 // Stash the old value of `may_block` and then set it to false.
                 let old_may_block = self.builder.ins().load(
                     ir::types::I32,
@@ -1434,7 +1399,7 @@ impl ComponentCompiler for Compiler {
         types: &ComponentTypesBuilder,
         key: FuncKey,
         abi: Abi,
-        tunables: &Tunables,
+        _tunables: &Tunables,
         symbol: &str,
     ) -> Result<CompiledFunctionBody> {
         let (abi2, trampoline_index) = key.unwrap_component_trampoline();
@@ -1466,14 +1431,7 @@ impl ComponentCompiler for Compiler {
         }
 
         let mut compiler = self.function_compiler();
-        let mut c = TrampolineCompiler::new(
-            self,
-            &mut compiler,
-            &component.component,
-            types,
-            sig,
-            tunables,
-        );
+        let mut c = TrampolineCompiler::new(self, &mut compiler, &component.component, types, sig);
 
         // If we are crossing the Wasm-to-native boundary, we need to save the
         // exit FP and return address for stack walking purposes. However, we
@@ -1512,7 +1470,7 @@ impl ComponentCompiler for Compiler {
 
     fn compile_intrinsic(
         &self,
-        tunables: &Tunables,
+        _tunables: &Tunables,
         component: &ComponentTranslation,
         types: &ComponentTypesBuilder,
         intrinsic: UnsafeIntrinsic,
@@ -1557,7 +1515,6 @@ impl ComponentCompiler for Compiler {
             &component.component,
             &types,
             &wasm_func_ty,
-            tunables,
         );
 
         match intrinsic {

crates/cranelift/src/lib.rs

@@ -48,10 +48,6 @@ use self::compiler::Compiler;
 
 const TRAP_INTERNAL_ASSERT: TrapCode = TrapCode::unwrap_user(1);
 const TRAP_OFFSET: u8 = 2;
-pub const TRAP_ALWAYS: TrapCode =
-    TrapCode::unwrap_user(Trap::AlwaysTrapAdapter as u8 + TRAP_OFFSET);
-pub const TRAP_CANNOT_ENTER: TrapCode =
-    TrapCode::unwrap_user(Trap::CannotEnterComponent as u8 + TRAP_OFFSET);
 pub const TRAP_INDIRECT_CALL_TO_NULL: TrapCode =
     TrapCode::unwrap_user(Trap::IndirectCallToNull as u8 + TRAP_OFFSET);
 pub const TRAP_BAD_SIGNATURE: TrapCode =

crates/environ/src/component/dfg.rs

@@ -326,7 +326,6 @@ pub enum Trampoline {
         to: MemoryId,
         to64: bool,
     },
-    AlwaysTrap,
     ResourceNew {
         instance: RuntimeComponentInstanceIndex,
         ty: TypeResourceTableIndex,
@@ -945,7 +944,6 @@ impl LinearizeDfg<'_> {
                 to: self.runtime_memory(*to),
                 to64: *to64,
             },
-            Trampoline::AlwaysTrap => info::Trampoline::AlwaysTrap,
             Trampoline::ResourceNew { instance, ty } => info::Trampoline::ResourceNew {
                 instance: *instance,
                 ty: *ty,

crates/environ/src/component/info.rs

@@ -734,10 +734,6 @@ pub enum Trampoline {
         to64: bool,
     },
 
-    /// A small adapter which simply traps, used for degenerate lift/lower
-    /// combinations.
-    AlwaysTrap,
-
     /// A `resource.new` intrinsic which will inject a new resource into the
     /// table specified.
     ResourceNew {
@@ -1199,7 +1195,6 @@ impl Trampoline {
                 let to = if *to64 { "64" } else { "32" };
                 format!("component-transcode-{op}-m{from}-m{to}")
             }
-            AlwaysTrap => format!("component-always-trap"),
             ResourceNew { ty, .. } => format!("component-resource-new[{}]", ty.as_u32()),
             ResourceRep { ty, .. } => format!("component-resource-rep[{}]", ty.as_u32()),
             ResourceDrop { ty, .. } => format!("component-resource-drop[{}]", ty.as_u32()),

crates/environ/src/component/translate/adapt.rs

@@ -166,6 +166,9 @@ pub struct AdapterOptions {
     /// The Wasmtime-assigned component instance index where the options were
     /// originally specified.
     pub instance: RuntimeComponentInstanceIndex,
+    /// The ancestors (i.e. chain of instantiating instances) of the instance
+    /// specified in the `instance` field.
+    pub ancestors: Vec<RuntimeComponentInstanceIndex>,
     /// How strings are encoded.
     pub string_encoding: StringEncoding,
     /// The async callback function used by these options, if specified.