Repository: bytecodealliance/wasmtime
Pull request #9165 (2024-08-22 21:28:59)

Make `i31.get_{s.u}` instructions trap on null `i31ref`s
<!--
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
-->


fitzgen: Hm, I thought this was independent of https://github.com/bytecodealliance/wasmtime/pull/9166 but I guess we can land that first and then I can rebase this.

Patch:
diff --git a/crates/cranelift/src/func_environ.rs b/crates/cranelift/src/func_environ.rs
index 38ef9c928..12ec5698a 100644
--- a/crates/cranelift/src/func_environ.rs
+++ b/crates/cranelift/src/func_environ.rs
@@ -1734,6 +1734,7 @@ impl<'module_environment> cranelift_wasm::FuncEnvironment for FuncEnvironment<'m
         mut pos: FuncCursor,
         i31ref: ir::Value,
     ) -> WasmResult<ir::Value> {
+        pos.ins().trapz(i31ref, ir::TrapCode::NullI31Ref);
         Ok(pos.ins().sshr_imm(i31ref, 1))
     }
 
@@ -1742,6 +1743,7 @@ impl<'module_environment> cranelift_wasm::FuncEnvironment for FuncEnvironment<'m
         mut pos: FuncCursor,
         i31ref: ir::Value,
     ) -> WasmResult<ir::Value> {
+        pos.ins().trapz(i31ref, ir::TrapCode::NullI31Ref);
         Ok(pos.ins().ushr_imm(i31ref, 1))
     }
 


Test patch:
diff --git a/tests/misc_testsuite/gc/null-i31ref.wast b/tests/misc_testsuite/gc/null-i31ref.wast
new file mode 100644
index 000000000..ba7e5a8b3
--- /dev/null
+++ b/tests/misc_testsuite/gc/null-i31ref.wast
@@ -0,0 +1,11 @@
+(module
+  (func (export "get_u-null") (result i32)
+    (i31.get_u (ref.null i31))
+  )
+  (func (export "get_s-null") (result i32)
+    (i31.get_u (ref.null i31))
+  )
+)
+
+(assert_trap (invoke "get_u-null") "null i31 reference")
+(assert_trap (invoke "get_s-null") "null i31 reference")
diff --git a/tests/spec_testsuite b/tests/spec_testsuite
index e05365077..7570678ad 160000
--- a/tests/spec_testsuite
+++ b/tests/spec_testsuite
@@ -1,1 +1,1 @@
-Subproject commit e05365077e13a1d86ffe77acfb1a835b7aa78422
+Subproject commit 7570678ade1244ae69c9fefc990f4534c63ffaec
diff --git a/tests/wast.rs b/tests/wast.rs
index b4aa7ab90..fbac487ca 100644
--- a/tests/wast.rs
+++ b/tests/wast.rs
@@ -187,6 +187,10 @@ fn should_fail(test: &Path, strategy: Strategy) -> bool {
             "tail-call",
             "relaxed-simd",
             "threads",
+            // Winch technically supports memory64 but the upstream tests have
+            // gc/function-references/exceptions/etc all merged in now so Winch
+            // can no longer run those tests without panicking.
+            "memory64",
         ];
         if let Some(parent) = test.parent() {
             if unsupported_proposals
@@ -197,6 +201,30 @@ fn should_fail(test: &Path, strategy: Strategy) -> bool {
             }
         }
     }
+    let unsupported_gc_tests = [
+        "array_copy.wast",
+        "array_fill.wast",
+        "array_init_data.wast",
+        "array_init_elem.wast",
+        "array.wast",
+        "binary_gc.wast",
+        "br_on_cast_fail.wast",
+        "br_on_cast.wast",
+        "extern.wast",
+        "ref_cast.wast",
+        "ref_eq.wast",
+        "ref_test.wast",
+        "return_call_indirect.wast",
+        "return_call.wast",
+        "struct.wast",
+        "table_sub.wast",
+        "type_canon.wast",
+        "type_equivalence.wast",
+        "type-rec.wast",
+        "type-subtyping.wast",
+        "unreached_valid.wast",
+        "i31.wast",
+    ];
 
     for part in test.iter() {
         // Not implemented in Wasmtime yet
@@ -204,9 +232,9 @@ fn should_fail(test: &Path, strategy: Strategy) -> bool {
             return !test.ends_with("binary.wast");
         }
 
-        // Wasmtime doesn't implement the table64 extension yet.
         if part == "memory64" {
             if [
+                // Wasmtime doesn't implement the table64 extension yet.
                 "call_indirect.wast",
                 "table_copy.wast",
                 "table_get.wast",
@@ -217,43 +245,29 @@ fn should_fail(test: &Path, strategy: Strategy) -> bool {
                 "table_copy_mixed.wast",
                 "table_grow.wast",
                 "table_size.wast",
+                // wasmtime doesn't implement exceptions yet
+                "imports.wast",
+                "ref_null.wast",
+                "exports.wast",
+                "throw.wast",
+                "throw_ref.wast",
+                "try_table.wast",
+                "tag.wast",
             ]
             .iter()
             .any(|i| test.ends_with(i))
             {
                 return true;
             }
+            if unsupported_gc_tests.iter().any(|i| test.ends_with(i)) {
+                return true;
+            }
         }
 
         // Implementation of the GC proposal is a work-in-progress, this is
         // a list of all currently known-to-fail tests.
         if part == "gc" {
-            return [
-                "array_copy.wast",
-                "array_fill.wast",
-                "array_init_data.wast",
-                "array_init_elem.wast",
-                "array.wast",
-                "binary_gc.wast",
-                "br_on_cast_fail.wast",
-                "br_on_cast.wast",
-                "extern.wast",
-                "ref_cast.wast",
-                "ref_eq.wast",
-                "ref_test.wast",
-                "return_call_indirect.wast",
-                "return_call.wast",
-                "struct.wast",
-                "table_sub.wast",
-                "type_canon.wast",
-                "type_equivalence.wast",
-                "type-rec.wast",
-                "type-subtyping.wast",
-                "unreached_valid.wast",
-                "i31.wast",
-            ]
-            .iter()
-            .any(|i| test.ends_with(i));
+            return unsupported_gc_tests.iter().any(|i| test.ends_with(i));
         }
     }
 
@@ -274,16 +288,17 @@ fn run_wast(wast: &Path, strategy: Strategy, pooling: bool) -> anyhow::Result<()
     let custom_page_sizes = feature_found(wast, "custom-page-sizes");
     let multi_memory = feature_found(wast, "multi-memory")
         || feature_found(wast, "component-model")
-        || custom_page_sizes;
+        || custom_page_sizes
+        || memory64;
     let threads = feature_found(wast, "threads");
-    let gc = feature_found(wast, "gc");
-    let function_references = gc || feature_found(wast, "function-references");
+    let gc = feature_found(wast, "gc") || memory64;
+    let function_references = gc || memory64 || feature_found(wast, "function-references");
     let reference_types = !(threads && feature_found(wast, "proposals"));
     let relaxed_simd = feature_found(wast, "relaxed-simd");
     let tail_call = feature_found(wast, "tail-call") || feature_found(wast, "function-references");
     let use_shared_memory = feature_found_src(&wast_bytes, "shared_memory")
         || feature_found_src(&wast_bytes, "shared)");
-    let extended_const = feature_found(wast, "extended-const");
+    let extended_const = feature_found(wast, "extended-const") || memory64;
 
     if pooling && use_shared_memory {
         log::warn!("skipping pooling test with shared memory");