Harden Wasmtime's compiled GC code against corruption

In the spirit of #13320 this commit goes through the compiled code for
the GC proposal to ensure that, in the face of GC corruption, Wasmtime
by default can recover and return a "bug" to the embedder. This was also
discussed a bit in #13112 as well, and the changes made here are:

* Plumbing traps from translation into the runtime now uses a new
  `CompiledTrap` enum instead of just the normal `Trap`. This new enum
  has branches for `InternalAssert` (not previously present) and
  additionally `GcHeapCorrupted` (now added).
* Whether or not `CompiledTrap::{InternalAssert,GcHeapCorrupted}` is
  encoded into the final `*.cwasm` is now a `Tunables` configuration
  option. Internal asserts are not encoded by default but GC heap
  corruption is.
* Traps caught as `CompiledTrap::{InternalAssert,GcHeapCorrupted}` are
  turned into `WasmtimeBug` and propagated upwards. Traps stay as normal
  traps.
* All memory accesses to the GC heap now use
  `CompiledTrap::GcHeapCorrupted` as their trap code. Additionally
  they're also no longer marked as `readonly` in a few places.
* A few locations in GC translation using `InternalAssert` now use
  `GcHeapCorrupted`, such as the checked arithmetic around array
  lengths. Other assertions which are about control flow are left
  untouched.

The end state is that faults in the GC heap in compiled code itself
should show up as a `bug!` on the other end by default. This requires
extra metadata in `*.cwasm`s mapping traps, but this is similar to
linear-memory-using-wasms which have lots of trap metadata for
loads/stores. Being able to catch `InternalAssert` as a first-class
error (as opposed to a signal) is a debugging nicety I've added here but
remains off-by-default to avoid bloating `*.cwasm`s for internal debugging.

Closes #13112

Author: alexcrichton

crates/cli-flags/src/lib.rs

@@ -266,6 +266,13 @@ wasmtime_option_group! {
         #[serde(deserialize_with = "crate::opt::cli_parse_wrapper")]
         pub inlining: Option<wasmtime::Inlining>,
 
+        /// Whether or not trap metadata is present for wasm internal assertions
+        /// in compiled code.
+        pub metadata_for_internal_asserts: Option<bool>,
+        /// Whether or not trap metadata is present for detection of gc
+        /// corruption in compiled code.
+        pub metadata_for_gc_heap_corruption: Option<bool>,
+
         #[prefixed = "cranelift"]
         #[serde(default)]
         /// Set a cranelift-specific option. Use `wasmtime settings` to see
@@ -975,6 +982,12 @@ impl CommonOptions {
         if let Some(enable) = self.codegen.inlining {
             config.compiler_inlining(enable);
         }
+        if let Some(enable) = self.codegen.metadata_for_internal_asserts {
+            config.metadata_for_internal_asserts(enable);
+        }
+        if let Some(enable) = self.codegen.metadata_for_gc_heap_corruption {
+            config.metadata_for_gc_heap_corruption(enable);
+        }
 
         // async_stack_size enabled by either async or stack-switching, so
         // cannot directly use match_feature!

crates/cranelift/src/compiled_function.rs

@@ -7,6 +7,7 @@ use cranelift_codegen::{
 };
 use wasmtime_environ::{
     FilePos, FrameStateSlotBuilder, InstructionAddressMap, ModulePC, PrimaryMap, TrapInformation,
+    Tunables,
 };
 
 #[derive(Debug, Clone, PartialEq, Eq, Default)]
@@ -144,8 +145,14 @@ impl CompiledFunction {
     }
 
     /// Returns an iterator to the function's trap information.
-    pub fn traps(&self) -> impl Iterator<Item = TrapInformation> + '_ {
-        self.buffer.traps().iter().filter_map(mach_trap_to_trap)
+    pub fn traps<'a>(
+        &'a self,
+        tunables: &'a Tunables,
+    ) -> impl Iterator<Item = TrapInformation> + 'a {
+        self.buffer
+            .traps()
+            .iter()
+            .filter_map(move |t| mach_trap_to_trap(t, tunables))
     }
 
     /// Get the function's address map from the metadata.

crates/cranelift/src/compiler.rs

@@ -540,7 +540,10 @@ impl wasmtime_environ::Compiler for Compiler {
                 func.buffer.user_stack_maps(),
             );
 
-            traps.push(range.clone(), &func.traps().collect::<Vec<_>>());
+            traps.push(
+                range.clone(),
+                &func.traps(&self.tunables).collect::<Vec<_>>(),
+            );
             clif_to_env_exception_tables(
                 &mut exception_tables,
                 range.clone(),

crates/cranelift/src/func_environ/gc/enabled.rs

@@ -3,7 +3,7 @@ use crate::bounds_checks::BoundsCheck;
 use crate::func_environ::{Extension, FuncEnvironment};
 use crate::translate::{Heap, HeapData, MemoryKind, StructFieldsVec, TargetEnvironment};
 use crate::trap::TranslateTrap;
-use crate::{Reachability, TRAP_INTERNAL_ASSERT};
+use crate::{Reachability, TRAP_GC_HEAP_CORRUPT, TRAP_INTERNAL_ASSERT};
 use cranelift_codegen::ir::immediates::Offset32;
 use cranelift_codegen::ir::{BlockArg, ExceptionTableData, ExceptionTableItem};
 use cranelift_codegen::{
@@ -27,6 +27,9 @@ mod null;
 #[cfg(feature = "gc-copying")]
 mod copying;
 
+const GC_MEMFLAGS: ir::MemFlags =
+    ir::MemFlags::new().with_trap_code(Some(crate::TRAP_GC_HEAP_CORRUPT));
+
 /// Get the default GC compiler.
 pub fn gc_compiler(func_env: &mut FuncEnvironment<'_>) -> WasmResult<Box<dyn GcCompiler>> {
     // If this function requires a GC compiler, that is not too bad of an
@@ -161,12 +164,9 @@ fn emit_gc_kind_assert(
             object_size: wasmtime_environ::VM_GC_HEADER_SIZE,
         },
     );
-    let kind_and_reserved_bits = builder.ins().load(
-        ir::types::I32,
-        ir::MemFlags::trusted().with_readonly(),
-        kind_addr,
-        0,
-    );
+    let kind_and_reserved_bits = builder
+        .ins()
+        .load(ir::types::I32, GC_MEMFLAGS, kind_addr, 0);
     let kind_mask = builder
         .ins()
         .iconst(ir::types::I32, i64::from(VMGcKind::MASK));
@@ -202,7 +202,7 @@ fn read_field_at_addr(
     );
 
     // Data inside GC objects is always little endian.
-    let flags = ir::MemFlags::trusted().with_endianness(ir::Endianness::Little);
+    let flags = GC_MEMFLAGS.with_endianness(ir::Endianness::Little);
 
     let value = match ty {
         WasmStorageType::I8 => builder.ins().load(ir::types::I8, flags, addr, 0),
@@ -321,7 +321,7 @@ fn write_field_at_addr(
     new_val: ir::Value,
 ) -> WasmResult<()> {
     // Data inside GC objects is always little endian.
-    let flags = ir::MemFlags::trusted().with_endianness(ir::Endianness::Little);
+    let flags = GC_MEMFLAGS.with_endianness(ir::Endianness::Little);
 
     match field_ty {
         WasmStorageType::I8 => {
@@ -928,12 +928,9 @@ pub fn translate_array_len(
             access_size: u8::try_from(ir::types::I32.bytes()).unwrap(),
         },
     );
-    let result = builder.ins().load(
-        ir::types::I32,
-        ir::MemFlags::trusted().with_readonly(),
-        len_field,
-        0,
-    );
+    let result = builder
+        .ins()
+        .load(ir::types::I32, GC_MEMFLAGS, len_field, 0);
     log::trace!("translate_array_len(..) -> {result:?}");
     Ok(result)
 }
@@ -973,7 +970,7 @@ fn emit_array_size_info(
     let all_elems_size = builder.ins().imul(one_elem_size, array_len);
 
     let high_bits = builder.ins().ushr_imm(all_elems_size, 32);
-    builder.ins().trapnz(high_bits, TRAP_INTERNAL_ASSERT);
+    builder.ins().trapnz(high_bits, TRAP_GC_HEAP_CORRUPT);
 
     let all_elems_size = builder.ins().ireduce(ir::types::I32, all_elems_size);
     let base_size = builder
@@ -982,7 +979,7 @@ fn emit_array_size_info(
     let obj_size =
         builder
             .ins()
-            .uadd_overflow_trap(all_elems_size, base_size, TRAP_INTERNAL_ASSERT);
+            .uadd_overflow_trap(all_elems_size, base_size, TRAP_GC_HEAP_CORRUPT);
 
     let one_elem_size = builder.ins().ireduce(ir::types::I32, one_elem_size);
 
@@ -1244,12 +1241,9 @@ pub fn translate_ref_test(
                 object_size: wasmtime_environ::VM_GC_HEADER_SIZE,
             },
         );
-        let actual_kind = builder.ins().load(
-            ir::types::I32,
-            ir::MemFlags::trusted().with_readonly(),
-            kind_addr,
-            0,
-        );
+        let actual_kind = builder
+            .ins()
+            .load(ir::types::I32, GC_MEMFLAGS, kind_addr, 0);
         let expected_kind = builder
             .ins()
             .iconst(ir::types::I32, i64::from(expected_kind.as_u32()));
@@ -1301,12 +1295,7 @@ pub fn translate_ref_test(
                     access_size: func_env.offsets.size_of_vmshared_type_index(),
                 },
             );
-            let actual_shared_ty = builder.ins().load(
-                ir::types::I32,
-                ir::MemFlags::trusted().with_readonly(),
-                ty_addr,
-                0,
-            );
+            let actual_shared_ty = builder.ins().load(ir::types::I32, GC_MEMFLAGS, ty_addr, 0);
 
             func_env.is_subtype(builder, actual_shared_ty, expected_shared_ty)
         }
@@ -1319,11 +1308,8 @@ pub fn translate_ref_test(
             let expected_shared_ty =
                 func_env.module_interned_to_shared_ty(&mut builder.cursor(), expected_interned_ty);
 
-            let actual_shared_ty = func_env.load_funcref_type_index(
-                &mut builder.cursor(),
-                ir::MemFlags::trusted().with_readonly(),
-                val,
-            );
+            let actual_shared_ty =
+                func_env.load_funcref_type_index(&mut builder.cursor(), GC_MEMFLAGS, val);
 
             func_env.is_subtype(builder, actual_shared_ty, expected_shared_ty)
         }
@@ -1594,7 +1580,7 @@ impl FuncEnvironment<'_> {
             &gc_heap,
             gc_ref,
             bounds_check,
-            crate::TRAP_INTERNAL_ASSERT,
+            crate::TRAP_GC_HEAP_CORRUPT,
         ) {
             Reachability::Reachable(v) => v,
             Reachability::Unreachable => {

crates/cranelift/src/func_environ/gc/enabled/copying.rs

@@ -33,7 +33,7 @@ impl CopyingCompiler {
         val: ir::Value,
     ) -> WasmResult<()> {
         // Data inside GC objects is always little endian.
-        let flags = ir::MemFlags::trusted().with_endianness(ir::Endianness::Little);
+        let flags = GC_MEMFLAGS.with_endianness(ir::Endianness::Little);
 
         match ty {
             WasmStorageType::Val(WasmValType::Ref(r)) => match r.heap_type.top() {
@@ -109,9 +109,7 @@ impl GcCompiler for CopyingCompiler {
         let object_addr = builder.ins().iadd(base, extended_array_ref);
         let len_addr = builder.ins().iadd_imm(object_addr, i64::from(len_offset));
         let len = init.len(&mut builder.cursor());
-        builder
-            .ins()
-            .store(ir::MemFlags::trusted(), len, len_addr, 0);
+        builder.ins().store(GC_MEMFLAGS, len, len_addr, 0);
 
         // Initialize elements.
         let len_to_elems_delta = builder.ins().iconst(ptr_ty, i64::from(len_to_elems_delta));

crates/cranelift/src/func_environ/gc/enabled/drc.rs

@@ -39,9 +39,7 @@ impl DrcCompiler {
                 access_size: u8::try_from(ir::types::I64.bytes()).unwrap(),
             },
         );
-        builder
-            .ins()
-            .load(ir::types::I64, ir::MemFlags::trusted(), pointer, 0)
+        builder.ins().load(ir::types::I64, GC_MEMFLAGS, pointer, 0)
     }
 
     /// Generate code to update the given GC reference's ref count to the new
@@ -64,9 +62,7 @@ impl DrcCompiler {
                 access_size: u8::try_from(ir::types::I64.bytes()).unwrap(),
             },
         );
-        builder
-            .ins()
-            .store(ir::MemFlags::trusted(), new_ref_count, pointer, 0);
+        builder.ins().store(GC_MEMFLAGS, new_ref_count, pointer, 0);
     }
 
     /// Generate code to increment or decrement the given GC reference's ref
@@ -108,9 +104,7 @@ impl DrcCompiler {
 
         // Load the current first list element, which will be our new next list
         // element.
-        let next = builder
-            .ins()
-            .load(ir::types::I32, ir::MemFlags::trusted(), head, 0);
+        let next = builder.ins().load(ir::types::I32, GC_MEMFLAGS, head, 0);
 
         // Update our object's header to point to `next` and consider itself part of the list.
         self.set_next_over_approximated_stack_root(func_env, builder, gc_ref, next);
@@ -120,9 +114,7 @@ impl DrcCompiler {
         self.mutate_ref_count(func_env, builder, gc_ref, 1);
 
         // Commit this object as the new head of the list.
-        builder
-            .ins()
-            .store(ir::MemFlags::trusted(), gc_ref, head, 0);
+        builder.ins().store(GC_MEMFLAGS, gc_ref, head, 0);
     }
 
     /// Load a pointer to the first element of the DRC heap's
@@ -137,7 +129,7 @@ impl DrcCompiler {
         let vmctx = builder.ins().global_value(ptr_ty, vmctx);
         builder.ins().load(
             ptr_ty,
-            ir::MemFlags::trusted().with_readonly(),
+            GC_MEMFLAGS,
             vmctx,
             i32::from(func_env.offsets.ptr.vmctx_gc_heap_data()),
         )
@@ -163,7 +155,7 @@ impl DrcCompiler {
                 access_size: u8::try_from(ir::types::I32.bytes()).unwrap(),
             },
         );
-        builder.ins().store(ir::MemFlags::trusted(), next, ptr, 0);
+        builder.ins().store(GC_MEMFLAGS, next, ptr, 0);
     }
 
     /// Set the in-over-approximated-stack-roots list bit in a `VMDrcHeader`'s
@@ -199,9 +191,7 @@ impl DrcCompiler {
                 access_size: u8::try_from(ir::types::I32.bytes()).unwrap(),
             },
         );
-        builder
-            .ins()
-            .store(ir::MemFlags::trusted(), new_reserved, ptr, 0);
+        builder.ins().store(GC_MEMFLAGS, new_reserved, ptr, 0);
     }
 
     /// Write to an uninitialized field or element inside a GC object.
@@ -214,7 +204,7 @@ impl DrcCompiler {
         val: ir::Value,
     ) -> WasmResult<()> {
         // Data inside GC objects is always little endian.
-        let flags = ir::MemFlags::trusted().with_endianness(ir::Endianness::Little);
+        let flags = GC_MEMFLAGS.with_endianness(ir::Endianness::Little);
 
         match ty {
             WasmStorageType::Val(WasmValType::Ref(r)) => match r.heap_type.top() {
@@ -396,9 +386,7 @@ impl GcCompiler for DrcCompiler {
         let object_addr = builder.ins().iadd(base, extended_array_ref);
         let len_addr = builder.ins().iadd_imm(object_addr, i64::from(len_offset));
         let len = init.len(&mut builder.cursor());
-        builder
-            .ins()
-            .store(ir::MemFlags::trusted(), len, len_addr, 0);
+        builder.ins().store(GC_MEMFLAGS, len, len_addr, 0);
 
         // Finally, initialize the elements.
         let len_to_elems_delta = builder.ins().iconst(ptr_ty, i64::from(len_to_elems_delta));
@@ -666,9 +654,7 @@ impl GcCompiler for DrcCompiler {
                 access_size: u8::try_from(ir::types::I32.bytes()).unwrap(),
             },
         );
-        let reserved = builder
-            .ins()
-            .load(ir::types::I32, ir::MemFlags::trusted(), ptr, 0);
+        let reserved = builder.ins().load(ir::types::I32, GC_MEMFLAGS, ptr, 0);
         let in_set_bit = builder.ins().iconst(
             ir::types::I32,
             i64::from(wasmtime_environ::drc::HEADER_IN_OVER_APPROX_LIST_BIT),

crates/cranelift/src/func_environ/gc/enabled/null.rs

@@ -73,13 +73,13 @@ impl NullCompiler {
         let vmctx = func_env.vmctx_val(&mut builder.cursor());
         let ptr_to_next = builder.ins().load(
             pointer_type,
-            ir::MemFlags::trusted().with_readonly(),
+            GC_MEMFLAGS,
             vmctx,
             i32::from(func_env.offsets.ptr.vmctx_gc_heap_data()),
         );
         let next = builder
             .ins()
-            .load(ir::types::I32, ir::MemFlags::trusted(), ptr_to_next, 0);
+            .load(ir::types::I32, GC_MEMFLAGS, ptr_to_next, 0);
 
         // Increment the bump "pointer" to the requested alignment:
         //
@@ -157,20 +157,20 @@ impl NullCompiler {
             ),
         };
         builder.ins().store(
-            ir::MemFlags::trusted(),
+            GC_MEMFLAGS,
             kind_and_size,
             ptr_to_object,
             i32::try_from(wasmtime_environ::VM_GC_HEADER_KIND_OFFSET).unwrap(),
         );
         builder.ins().store(
-            ir::MemFlags::trusted(),
+            GC_MEMFLAGS,
             ty,
             ptr_to_object,
             i32::try_from(wasmtime_environ::VM_GC_HEADER_TYPE_INDEX_OFFSET).unwrap(),
         );
         builder
             .ins()
-            .store(ir::MemFlags::trusted(), end_of_object, ptr_to_next, 0);
+            .store(GC_MEMFLAGS, end_of_object, ptr_to_next, 0);
 
         log::trace!("emit_inline_alloc(..) -> ({aligned}, {ptr_to_object})");
         (aligned, ptr_to_object)
@@ -223,9 +223,7 @@ impl GcCompiler for NullCompiler {
         // any pointers or offsets out from the (untrusted) GC heap.
         let len_addr = builder.ins().iadd_imm(ptr_to_object, i64::from(len_offset));
         let len = init.len(&mut builder.cursor());
-        builder
-            .ins()
-            .store(ir::MemFlags::trusted(), len, len_addr, 0);
+        builder.ins().store(GC_MEMFLAGS, len, len_addr, 0);
 
         // Finally, initialize the elements.
         let len_to_elems_delta = builder.ins().iconst(ptr_ty, i64::from(len_to_elems_delta));