Repository: bytecodealliance/wasmtime
Pull request #9320 (2024-09-27 16:43:18)

Always keep data inside GC objects in little-endian order
We were previously using native-endian on the assumption that it wasn't observable to Wasm, but it turns out that assumption isn't true. The `array.[init,new]_data` instructions take a data segment and copy it into an array. The arrays are not limited to `i8` arrays; they can be any non-reference type, so the endianness is visible, and Wasm requires little endian.

<!--
Please make sure you include the following information:

- If this work has been discussed elsewhere, please include a link to that
  conversation. If it was discussed in an issue, just mention "issue #...".

- Explain why this change is needed. If the details are in an issue already,
  this can be brief.

Our development process is documented in the Wasmtime book:
https://docs.wasmtime.dev/contributing-development-process.html

Please ensure all communication follows the code of conduct:
https://github.com/bytecodealliance/wasmtime/blob/main/CODE_OF_CONDUCT.md
-->



Patch:
diff --git a/crates/cranelift/src/gc/enabled.rs b/crates/cranelift/src/gc/enabled.rs
index 95fa1881b..0a217a5e3 100644
--- a/crates/cranelift/src/gc/enabled.rs
+++ b/crates/cranelift/src/gc/enabled.rs
@@ -25,10 +25,6 @@ fn unbarriered_load_gc_ref(
     flags: ir::MemFlags,
 ) -> WasmResult<ir::Value> {
     debug_assert!(ty.is_vmgcref_type());
-    assert!(
-        flags.explicit_endianness().is_none(),
-        "GC references are always native-endian"
-    );
     let gc_ref = builder.ins().load(ir::types::I32, flags, ptr_to_gc_ref, 0);
     if ty != WasmHeapType::I31 {
         builder.declare_value_needs_stack_map(gc_ref);
@@ -44,10 +40,6 @@ fn unbarriered_store_gc_ref(
     flags: ir::MemFlags,
 ) -> WasmResult<()> {
     debug_assert!(ty.is_vmgcref_type());
-    assert!(
-        flags.explicit_endianness().is_none(),
-        "GC references are always native-endian"
-    );
     builder.ins().store(flags, gc_ref, dst, 0);
     Ok(())
 }
@@ -156,42 +148,19 @@ pub fn translate_struct_get(
     // and relative to `gc_ref[0]`.
     let field_addr = func_env.prepare_gc_ref_access(builder, struct_ref, field_offset, field_size);
 
+    // Data inside GC objects is always little endian.
+    let flags = ir::MemFlags::trusted().with_endianness(ir::Endianness::Little);
+
     let field_val = match field_ty.element_type {
         WasmStorageType::Val(v) => match v {
-            WasmValType::I32 => {
-                builder
-                    .ins()
-                    .load(ir::types::I32, ir::MemFlags::trusted(), field_addr, 0)
-            }
-            WasmValType::I64 => {
-                builder
-                    .ins()
-                    .load(ir::types::I64, ir::MemFlags::trusted(), field_addr, 0)
-            }
-            WasmValType::F32 => {
-                builder
-                    .ins()
-                    .load(ir::types::F32, ir::MemFlags::trusted(), field_addr, 0)
-            }
-            WasmValType::F64 => {
-                builder
-                    .ins()
-                    .load(ir::types::F64, ir::MemFlags::trusted(), field_addr, 0)
-            }
-            WasmValType::V128 => {
-                builder
-                    .ins()
-                    .load(ir::types::I128, ir::MemFlags::trusted(), field_addr, 0)
-            }
+            WasmValType::I32 => builder.ins().load(ir::types::I32, flags, field_addr, 0),
+            WasmValType::I64 => builder.ins().load(ir::types::I64, flags, field_addr, 0),
+            WasmValType::F32 => builder.ins().load(ir::types::F32, flags, field_addr, 0),
+            WasmValType::F64 => builder.ins().load(ir::types::F64, flags, field_addr, 0),
+            WasmValType::V128 => builder.ins().load(ir::types::I128, flags, field_addr, 0),
             WasmValType::Ref(r) => match r.heap_type.top() {
                 WasmHeapTopType::Any | WasmHeapTopType::Extern => gc_compiler(func_env)?
-                    .translate_read_gc_reference(
-                        func_env,
-                        builder,
-                        r,
-                        field_addr,
-                        ir::MemFlags::trusted(),
-                    )?,
+                    .translate_read_gc_reference(func_env, builder, r, field_addr, flags)?,
                 WasmHeapTopType::Func => {
                     unimplemented!("funcrefs inside the GC heap")
                 }
@@ -237,17 +206,12 @@ fn translate_struct_get_and_extend(
     // TODO: See comment in `translate_struct_get` about the `prepare_gc_ref_access`.
     let field_addr = func_env.prepare_gc_ref_access(builder, struct_ref, field_offset, field_size);
 
+    // Data inside GC objects is always little endian.
+    let flags = ir::MemFlags::trusted().with_endianness(ir::Endianness::Little);
+
     let field_val = match field_ty.element_type {
-        WasmStorageType::I8 => {
-            builder
-                .ins()
-                .load(ir::types::I8, ir::MemFlags::trusted(), field_addr, 0)
-        }
-        WasmStorageType::I16 => {
-            builder
-                .ins()
-                .load(ir::types::I16, ir::MemFlags::trusted(), field_addr, 0)
-        }
+        WasmStorageType::I8 => builder.ins().load(ir::types::I8, flags, field_addr, 0),
+        WasmStorageType::I16 => builder.ins().load(ir::types::I16, flags, field_addr, 0),
         WasmStorageType::Val(_) => unreachable!(),
     };
 
@@ -320,35 +284,26 @@ pub fn translate_struct_set(
     // TODO: See comment in `translate_struct_get` about the `prepare_gc_ref_access`.
     let field_addr = func_env.prepare_gc_ref_access(builder, struct_ref, field_offset, field_size);
 
+    // Data inside GC objects is always little endian.
+    let flags = ir::MemFlags::trusted().with_endianness(ir::Endianness::Little);
+
     match &field_ty.element_type {
         WasmStorageType::I8 => {
-            builder
-                .ins()
-                .istore8(ir::MemFlags::trusted(), new_val, field_addr, 0);
+            builder.ins().istore8(flags, new_val, field_addr, 0);
         }
         WasmStorageType::I16 => {
-            builder
-                .ins()
-                .istore16(ir::MemFlags::trusted(), new_val, field_addr, 0);
+            builder.ins().istore16(flags, new_val, field_addr, 0);
         }
         WasmStorageType::Val(WasmValType::Ref(r)) if r.heap_type.top() == WasmHeapTopType::Func => {
             unimplemented!("funcrefs inside the GC heap")
         }
         WasmStorageType::Val(WasmValType::Ref(r)) => {
-            gc_compiler(func_env)?.translate_write_gc_reference(
-                func_env,
-                builder,
-                *r,
-                field_addr,
-                new_val,
-                ir::MemFlags::trusted(),
-            )?;
+            gc_compiler(func_env)?
+                .translate_write_gc_reference(func_env, builder, *r, field_addr, new_val, flags)?;
         }
         WasmStorageType::Val(_) => {
             assert_eq!(builder.func.dfg.value_type(new_val).bytes(), field_size);
-            builder
-                .ins()
-                .store(ir::MemFlags::trusted(), new_val, field_addr, 0);
+            builder.ins().store(flags, new_val, field_addr, 0);
         }
     }
 
diff --git a/crates/cranelift/src/gc/enabled/drc.rs b/crates/cranelift/src/gc/enabled/drc.rs
index cd351772e..8fe6eb43d 100644
--- a/crates/cranelift/src/gc/enabled/drc.rs
+++ b/crates/cranelift/src/gc/enabled/drc.rs
@@ -260,6 +260,9 @@ impl GcCompiler for DrcCompiler {
 
         // Second, initialize each of the newly-allocated struct's fields.
 
+        // Data inside GC objects is always little endian.
+        let flags = ir::MemFlags::trusted().with_endianness(ir::Endianness::Little);
+
         for ((ty, val), offset) in field_types.into_iter().zip(field_vals).zip(field_offsets) {
             let size_of_access =
                 wasmtime_environ::byte_size_of_wasm_ty_in_gc_heap(&ty.element_type);
@@ -276,29 +279,18 @@ impl GcCompiler for DrcCompiler {
                 }
                 WasmStorageType::Val(WasmValType::Ref(r)) => {
                     self.translate_init_gc_reference(
-                        func_env,
-                        builder,
-                        *r,
-                        field_addr,
-                        *val,
-                        ir::MemFlags::trusted(),
+                        func_env, builder, *r, field_addr, *val, flags,
                     )?;
                 }
                 WasmStorageType::I8 => {
-                    builder
-                        .ins()
-                        .istore8(ir::MemFlags::trusted(), *val, field_addr, 0);
+                    builder.ins().istore8(flags, *val, field_addr, 0);
                 }
                 WasmStorageType::I16 => {
-                    builder
-                        .ins()
-                        .istore16(ir::MemFlags::trusted(), *val, field_addr, 0);
+                    builder.ins().istore16(flags, *val, field_addr, 0);
                 }
                 WasmStorageType::Val(_) => {
                     assert_eq!(builder.func.dfg.value_type(*val).bytes(), size_of_access);
-                    builder
-                        .ins()
-                        .store(ir::MemFlags::trusted(), *val, field_addr, 0);
+                    builder.ins().store(flags, *val, field_addr, 0);
                 }
             }
         }
@@ -315,10 +307,6 @@ impl GcCompiler for DrcCompiler {
         flags: ir::MemFlags,
     ) -> WasmResult<ir::Value> {
         assert!(ty.is_vmgcref_type());
-        assert!(
-            flags.explicit_endianness().is_none(),
-            "GC references are always native-endian"
-        );
 
         let (reference_type, needs_stack_map) = func_env.reference_type(ty.heap_type);
         debug_assert!(needs_stack_map);
@@ -467,10 +455,6 @@ impl GcCompiler for DrcCompiler {
         flags: ir::MemFlags,
     ) -> WasmResult<()> {
         assert!(ty.is_vmgcref_type());
-        assert!(
-            flags.explicit_endianness().is_none(),
-            "GC references are always native-endian"
-        );
 
         let (ref_ty, needs_stack_map) = func_env.reference_type(ty.heap_type);
         debug_assert!(needs_stack_map);
diff --git a/crates/wasmtime/src/runtime/vm/gc/enabled/data.rs b/crates/wasmtime/src/runtime/vm/gc/enabled/data.rs
index 6a7cdc35c..ef117c636 100644
--- a/crates/wasmtime/src/runtime/vm/gc/enabled/data.rs
+++ b/crates/wasmtime/src/runtime/vm/gc/enabled/data.rs
@@ -6,21 +6,21 @@ use core::mem;
 /// A plain-old-data type that can be stored in a `ValType` or a `StorageType`.
 pub trait PodValType<const SIZE: usize>: Copy {
     /// Read an instance of `Self` from the given native-endian bytes.
-    fn read_ne(ne_bytes: &[u8; SIZE]) -> Self;
+    fn read_le(le_bytes: &[u8; SIZE]) -> Self;
 
     /// Write `self` into the given memory location, as native-endian bytes.
-    fn write_ne(&self, into: &mut [u8; SIZE]);
+    fn write_le(&self, into: &mut [u8; SIZE]);
 }
 
 macro_rules! impl_pod_val_type {
     ( $( $t:ty , )* ) => {
         $(
             impl PodValType<{mem::size_of::<$t>()}> for $t {
-                fn read_ne(ne_bytes: &[u8; mem::size_of::<$t>()]) -> Self {
-                    <$t>::from_ne_bytes(*ne_bytes)
+                fn read_le(le_bytes: &[u8; mem::size_of::<$t>()]) -> Self {
+                    <$t>::from_le_bytes(*le_bytes)
                 }
-                fn write_ne(&self, into: &mut [u8; mem::size_of::<$t>()]) {
-                    *into = self.to_ne_bytes();
+                fn write_le(&self, into: &mut [u8; mem::size_of::<$t>()]) {
+                    *into = self.to_le_bytes();
                 }
             }
         )*
@@ -39,11 +39,11 @@ impl_pod_val_type! {
 }
 
 impl PodValType<{ mem::size_of::<V128>() }> for V128 {
-    fn read_ne(ne_bytes: &[u8; mem::size_of::<V128>()]) -> Self {
-        u128::from_ne_bytes(*ne_bytes).into()
+    fn read_le(le_bytes: &[u8; mem::size_of::<V128>()]) -> Self {
+        u128::from_le_bytes(*le_bytes).into()
     }
-    fn write_ne(&self, into: &mut [u8; mem::size_of::<V128>()]) {
-        *into = self.as_u128().to_ne_bytes();
+    fn write_le(&self, into: &mut [u8; mem::size_of::<V128>()]) {
+        *into = self.as_u128().to_le_bytes();
     }
 }
 
@@ -112,7 +112,7 @@ impl<'a> VMGcObjectDataMut<'a> {
         let offset = usize::try_from(offset).unwrap();
         let end = offset.checked_add(N).unwrap();
         let bytes = self.data.get(offset..end).expect("out of bounds field");
-        T::read_ne(bytes.try_into().unwrap())
+        T::read_le(bytes.try_into().unwrap())
     }
 
     /// Read a POD field out of this object.
@@ -130,7 +130,7 @@ impl<'a> VMGcObjectDataMut<'a> {
         let offset = usize::try_from(offset).unwrap();
         let end = offset.checked_add(N).unwrap();
         let into = self.data.get_mut(offset..end).expect("out of bounds field");
-        val.write_ne(into.try_into().unwrap());
+        val.write_le(into.try_into().unwrap());
     }
 
     impl_pod_methods! {


Test patch:
diff --git a/tests/disas/gc/struct-get.wat b/tests/disas/gc/struct-get.wat
index 8d6b914af..b3cec94bb 100644
--- a/tests/disas/gc/struct-get.wat
+++ b/tests/disas/gc/struct-get.wat
@@ -42,7 +42,7 @@
 ;; @0033                               trapz v12, user65535
 ;; @0033                               v5 = load.i64 notrap aligned readonly v0+40
 ;; @0033                               v13 = iadd v5, v9
-;; @0033                               v14 = load.f32 notrap aligned v13
+;; @0033                               v14 = load.f32 notrap aligned little v13
 ;; @0037                               jump block1
 ;;
 ;;                                 block1:
@@ -68,7 +68,7 @@
 ;; @003c                               trapz v12, user65535
 ;; @003c                               v5 = load.i64 notrap aligned readonly v0+40
 ;; @003c                               v13 = iadd v5, v9
-;; @003c                               v14 = load.i8 notrap aligned v13
+;; @003c                               v14 = load.i8 notrap aligned little v13
 ;; @0040                               jump block1
 ;;
 ;;                                 block1:
@@ -95,7 +95,7 @@
 ;; @0045                               trapz v12, user65535
 ;; @0045                               v5 = load.i64 notrap aligned readonly v0+40
 ;; @0045                               v13 = iadd v5, v9
-;; @0045                               v14 = load.i8 notrap aligned v13
+;; @0045                               v14 = load.i8 notrap aligned little v13
 ;; @0049                               jump block1
 ;;
 ;;                                 block1:
@@ -125,7 +125,7 @@
 ;; @004e                               trapz v12, user65535
 ;; @004e                               v5 = load.i64 notrap aligned readonly v0+40
 ;; @004e                               v13 = iadd v5, v9
-;; @004e                               v14 = load.i32 notrap aligned v13
+;; @004e                               v14 = load.i32 notrap aligned little v13
 ;;                                     v54 = stack_addr.i64 ss0
 ;;                                     store notrap v14, v54
 ;; @004e                               v15 = iconst.i32 -2
diff --git a/tests/disas/gc/struct-new-default.wat b/tests/disas/gc/struct-new-default.wat
index 55b72eb12..5cc41c1a0 100644
--- a/tests/disas/gc/struct-new-default.wat
+++ b/tests/disas/gc/struct-new-default.wat
@@ -37,7 +37,7 @@
 ;; @0021                               v3 = f32const 0.0
 ;; @0021                               v13 = load.i64 notrap aligned readonly v0+40
 ;; @0021                               v21 = iadd v13, v17
-;; @0021                               store notrap aligned v3, v21  ; v3 = 0.0
+;; @0021                               store notrap aligned little v3, v21  ; v3 = 0.0
 ;; @0021                               v26 = iconst.i64 20
 ;; @0021                               v27 = uadd_overflow_trap v15, v26, user65535  ; v26 = 20
 ;; @0021                               v28 = iconst.i64 1
@@ -45,7 +45,7 @@
 ;; @0021                               v30 = icmp ult v29, v14
 ;; @0021                               trapz v30, user65535
 ;; @0021                               v31 = iadd v13, v27
-;; @0021                               istore8 notrap aligned v4, v31  ; v4 = 0
+;; @0021                               istore8 notrap aligned little v4, v31  ; v4 = 0
 ;; @0021                               v36 = iconst.i64 24
 ;; @0021                               v37 = uadd_overflow_trap v15, v36, user65535  ; v36 = 24
 ;; @0021                               v39 = uadd_overflow_trap v37, v18, user65535  ; v18 = 4
@@ -72,7 +72,7 @@
 ;;                                 block3:
 ;;                                     v85 = iconst.i32 0
 ;; @0021                               v41 = iadd.i64 v13, v37
-;; @0021                               store notrap aligned v85, v41  ; v85 = 0
+;; @0021                               store notrap aligned little v85, v41  ; v85 = 0
 ;; @0024                               jump block1
 ;;
 ;;                                 block1:
diff --git a/tests/disas/gc/struct-new.wat b/tests/disas/gc/struct-new.wat
index f592504fb..42ed90e78 100644
--- a/tests/disas/gc/struct-new.wat
+++ b/tests/disas/gc/struct-new.wat
@@ -39,7 +39,7 @@
 ;; @002a                               trapz v20, user65535
 ;; @002a                               v13 = load.i64 notrap aligned readonly v0+40
 ;; @002a                               v21 = iadd v13, v17
-;; @002a                               store notrap aligned v2, v21
+;; @002a                               store notrap aligned little v2, v21
 ;; @002a                               v26 = iconst.i64 20
 ;; @002a                               v27 = uadd_overflow_trap v15, v26, user65535  ; v26 = 20
 ;; @002a                               v28 = iconst.i64 1
@@ -47,7 +47,7 @@
 ;; @002a                               v30 = icmp ult v29, v14
 ;; @002a                               trapz v30, user65535
 ;; @002a                               v31 = iadd v13, v27
-;; @002a                               istore8 notrap aligned v3, v31
+;; @002a                               istore8 notrap aligned little v3, v31
 ;; @002a                               v36 = iconst.i64 24
 ;; @002a                               v37 = uadd_overflow_trap v15, v36, user65535  ; v36 = 24
 ;; @002a                               v39 = uadd_overflow_trap v37, v18, user65535  ; v18 = 4
@@ -83,7 +83,7 @@
 ;;                                 block3:
 ;;                                     v67 = load.i32 notrap v71
 ;; @002a                               v41 = iadd.i64 v13, v37
-;; @002a                               store notrap aligned v67, v41
+;; @002a                               store notrap aligned little v67, v41
 ;; @002d                               jump block1
 ;;
 ;;                                 block1:
diff --git a/tests/disas/gc/struct-set.wat b/tests/disas/gc/struct-set.wat
index d31f014ec..9252db68b 100644
--- a/tests/disas/gc/struct-set.wat
+++ b/tests/disas/gc/struct-set.wat
@@ -38,7 +38,7 @@
 ;; @0034                               trapz v12, user65535
 ;; @0034                               v5 = load.i64 notrap aligned readonly v0+40
 ;; @0034                               v13 = iadd v5, v9
-;; @0034                               store notrap aligned v3, v13
+;; @0034                               store notrap aligned little v3, v13
 ;; @0038                               jump block1
 ;;
 ;;                                 block1:
@@ -64,7 +64,7 @@
 ;; @003f                               trapz v12, user65535
 ;; @003f                               v5 = load.i64 notrap aligned readonly v0+40
 ;; @003f                               v13 = iadd v5, v9
-;; @003f                               istore8 notrap aligned v3, v13
+;; @003f                               istore8 notrap aligned little v3, v13
 ;; @0043                               jump block1
 ;;
 ;;                                 block1:
@@ -92,7 +92,7 @@
 ;; @004a                               trapz v12, user65535
 ;; @004a                               v5 = load.i64 notrap aligned readonly v0+40
 ;; @004a                               v13 = iadd v5, v9
-;; @004a                               v14 = load.i32 notrap aligned v13
+;; @004a                               v14 = load.i32 notrap aligned little v13
 ;; @004a                               v15 = iconst.i32 -2
 ;; @004a                               v16 = band v3, v15  ; v15 = -2
 ;;                                     v67 = iconst.i32 0
@@ -115,7 +115,7 @@
 ;; @004a                               jump block3
 ;;
 ;;                                 block3:
-;; @004a                               store.i32 notrap aligned v3, v13
+;; @004a                               store.i32 notrap aligned little v3, v13
 ;;                                     v72 = iconst.i32 -2
 ;;                                     v73 = band.i32 v14, v72  ; v72 = -2
 ;;                                     v74 = iconst.i32 0