Repository: bytecodealliance/wasm-tools
Pull request #1724 (2024-08-14 21:16:35)

threads: fix `ref.i31_shared` encoding
In the initial implementation, this encoding was incorrectly emitted as `0xFE 0x1F`, clobbering the encoding for `i64.atomic.rmw.add` (probably a copy-paste error). This change updates `wasm-encoder` to use the correct `0xFE 0x72` encoding.


Patch:
diff --git a/crates/wasm-encoder/src/core/code.rs b/crates/wasm-encoder/src/core/code.rs
index 8bfb8fdc..a08386a6 100644
--- a/crates/wasm-encoder/src/core/code.rs
+++ b/crates/wasm-encoder/src/core/code.rs
@@ -3693,7 +3693,7 @@ impl Encode for Instruction<'_> {
             }
             Instruction::RefI31Shared => {
                 sink.push(0xFE);
-                sink.push(0x1F);
+                sink.push(0x72);
             }
         }
     }
diff --git a/crates/wasmparser/src/validator/operators.rs b/crates/wasmparser/src/validator/operators.rs
index 39fc7804..81547f16 100644
--- a/crates/wasmparser/src/validator/operators.rs
+++ b/crates/wasmparser/src/validator/operators.rs
@@ -4534,7 +4534,9 @@ where
     }
     fn visit_ref_i31_shared(&mut self) -> Self::Output {
         self.pop_operand(Some(ValType::I32))?;
-        self.push_operand(ValType::Ref(RefType::I31)) // TODO: handle shared--is this correct?
+        self.push_operand(ValType::Ref(
+            RefType::I31.shared().expect("i31 is abstract"),
+        ))
     }
     fn visit_i31_get_s(&mut self) -> Self::Output {
         self.pop_operand(Some(ValType::Ref(RefType::I31REF)))?;
diff --git a/crates/wast/src/core/expr.rs b/crates/wast/src/core/expr.rs
index c4520310..026a5350 100644
--- a/crates/wast/src/core/expr.rs
+++ b/crates/wast/src/core/expr.rs
@@ -39,7 +39,7 @@ pub struct Expression<'a> {
 #[derive(Debug)]
 pub struct BranchHint {
     /// Index of instructions in `instrs` field of `Expression` that this hint
-    /// appplies to.
+    /// applies to.
     pub instr_index: usize,
     /// The value of this branch hint
     pub value: u32,
@@ -149,7 +149,7 @@ enum Level<'a> {
 
 /// Possible states of "what is currently being parsed?" in an `if` expression.
 enum If<'a> {
-    /// Only the `if` instructoin has been parsed, next thing to parse is the
+    /// Only the `if` instruction has been parsed, next thing to parse is the
     /// clause, if any, of the `if` instruction.
     ///
     /// This parse ends when `(then ...)` is encountered.
@@ -318,7 +318,7 @@ impl<'a> ExpressionParser<'a> {
 
     /// State transitions with parsing an `if` statement.
     ///
-    /// The syntactical form of an `if` stament looks like:
+    /// The syntactical form of an `if` statement looks like:
     ///
     /// ```wat
     /// (if ($clause)... (then $then) (else $else))
@@ -870,31 +870,31 @@ instructions! {
         GlobalAtomicRmwCmpxchg(Ordered<Index<'a>>) : [0xfe, 0x57] : "global.atomic.rmw.cmpxchg",
         TableAtomicGet(Ordered<TableArg<'a>>) : [0xfe, 0x58] : "table.atomic.get",
         TableAtomicSet(Ordered<TableArg<'a>>) : [0xfe, 0x59] : "table.atomic.set",
-        TableAtomicRmwXchg(Ordered<TableArg<'a>>) : [0xfe, 0x5A] : "table.atomic.rmw.xchg",
-        TableAtomicRmwCmpxchg(Ordered<TableArg<'a>>) : [0xFE, 0x5B] : "table.atomic.rmw.cmpxchg",
-        StructAtomicGet(Ordered<StructAccess<'a>>) : [0xFE, 0x5C] : "struct.atomic.get",
-        StructAtomicGetS(Ordered<StructAccess<'a>>) : [0xFE, 0x5D] : "struct.atomic.get_s",
-        StructAtomicGetU(Ordered<StructAccess<'a>>) : [0xFE, 0x5E] : "struct.atomic.get_u",
-        StructAtomicSet(Ordered<StructAccess<'a>>) : [0xFE, 0x5F] : "struct.atomic.set",
-        StructAtomicRmwAdd(Ordered<StructAccess<'a>>) : [0xFE, 0x60] : "struct.atomic.rmw.add",
-        StructAtomicRmwSub(Ordered<StructAccess<'a>>) : [0xFE, 0x61] : "struct.atomic.rmw.sub",
-        StructAtomicRmwAnd(Ordered<StructAccess<'a>>) : [0xFE, 0x62] : "struct.atomic.rmw.and",
-        StructAtomicRmwOr(Ordered<StructAccess<'a>>) : [0xFE, 0x63] : "struct.atomic.rmw.or",
-        StructAtomicRmwXor(Ordered<StructAccess<'a>>) : [0xFE, 0x64] : "struct.atomic.rmw.xor",
-        StructAtomicRmwXchg(Ordered<StructAccess<'a>>) : [0xFE, 0x65] : "struct.atomic.rmw.xchg",
-        StructAtomicRmwCmpxchg(Ordered<StructAccess<'a>>) : [0xFE, 0x66] : "struct.atomic.rmw.cmpxchg",
-        ArrayAtomicGet(Ordered<Index<'a>>) : [0xFE, 0x67] : "array.atomic.get",
-        ArrayAtomicGetS(Ordered<Index<'a>>) : [0xFE, 0x68] : "array.atomic.get_s",
-        ArrayAtomicGetU(Ordered<Index<'a>>) : [0xFE, 0x69] : "array.atomic.get_u",
-        ArrayAtomicSet(Ordered<Index<'a>>) : [0xFE, 0x6A] : "array.atomic.set",
-        ArrayAtomicRmwAdd(Ordered<Index<'a>>) : [0xFE, 0x6B] : "array.atomic.rmw.add",
-        ArrayAtomicRmwSub(Ordered<Index<'a>>) : [0xFE, 0x6C] : "array.atomic.rmw.sub",
-        ArrayAtomicRmwAnd(Ordered<Index<'a>>) : [0xFE, 0x6D] : "array.atomic.rmw.and",
-        ArrayAtomicRmwOr(Ordered<Index<'a>>) : [0xFE, 0x6E] : "array.atomic.rmw.or",
-        ArrayAtomicRmwXor(Ordered<Index<'a>>) : [0xFE, 0x6F] : "array.atomic.rmw.xor",
-        ArrayAtomicRmwXchg(Ordered<Index<'a>>) : [0xFE, 0x70] : "array.atomic.rmw.xchg",
-        ArrayAtomicRmwCmpxchg(Ordered<Index<'a>>) : [0xFE, 0x71] : "array.atomic.rmw.cmpxchg",
-        RefI31Shared : [0xFE, 0x72] : "ref.i31_shared",
+        TableAtomicRmwXchg(Ordered<TableArg<'a>>) : [0xfe, 0x5a] : "table.atomic.rmw.xchg",
+        TableAtomicRmwCmpxchg(Ordered<TableArg<'a>>) : [0xfe, 0x5b] : "table.atomic.rmw.cmpxchg",
+        StructAtomicGet(Ordered<StructAccess<'a>>) : [0xfe, 0x5c] : "struct.atomic.get",
+        StructAtomicGetS(Ordered<StructAccess<'a>>) : [0xfe, 0x5d] : "struct.atomic.get_s",
+        StructAtomicGetU(Ordered<StructAccess<'a>>) : [0xfe, 0x5e] : "struct.atomic.get_u",
+        StructAtomicSet(Ordered<StructAccess<'a>>) : [0xfe, 0x5f] : "struct.atomic.set",
+        StructAtomicRmwAdd(Ordered<StructAccess<'a>>) : [0xfe, 0x60] : "struct.atomic.rmw.add",
+        StructAtomicRmwSub(Ordered<StructAccess<'a>>) : [0xfe, 0x61] : "struct.atomic.rmw.sub",
+        StructAtomicRmwAnd(Ordered<StructAccess<'a>>) : [0xfe, 0x62] : "struct.atomic.rmw.and",
+        StructAtomicRmwOr(Ordered<StructAccess<'a>>) : [0xfe, 0x63] : "struct.atomic.rmw.or",
+        StructAtomicRmwXor(Ordered<StructAccess<'a>>) : [0xfe, 0x64] : "struct.atomic.rmw.xor",
+        StructAtomicRmwXchg(Ordered<StructAccess<'a>>) : [0xfe, 0x65] : "struct.atomic.rmw.xchg",
+        StructAtomicRmwCmpxchg(Ordered<StructAccess<'a>>) : [0xfe, 0x66] : "struct.atomic.rmw.cmpxchg",
+        ArrayAtomicGet(Ordered<Index<'a>>) : [0xfe, 0x67] : "array.atomic.get",
+        ArrayAtomicGetS(Ordered<Index<'a>>) : [0xfe, 0x68] : "array.atomic.get_s",
+        ArrayAtomicGetU(Ordered<Index<'a>>) : [0xfe, 0x69] : "array.atomic.get_u",
+        ArrayAtomicSet(Ordered<Index<'a>>) : [0xfe, 0x6a] : "array.atomic.set",
+        ArrayAtomicRmwAdd(Ordered<Index<'a>>) : [0xfe, 0x6b] : "array.atomic.rmw.add",
+        ArrayAtomicRmwSub(Ordered<Index<'a>>) : [0xfe, 0x6c] : "array.atomic.rmw.sub",
+        ArrayAtomicRmwAnd(Ordered<Index<'a>>) : [0xfe, 0x6d] : "array.atomic.rmw.and",
+        ArrayAtomicRmwOr(Ordered<Index<'a>>) : [0xfe, 0x6e] : "array.atomic.rmw.or",
+        ArrayAtomicRmwXor(Ordered<Index<'a>>) : [0xfe, 0x6f] : "array.atomic.rmw.xor",
+        ArrayAtomicRmwXchg(Ordered<Index<'a>>) : [0xfe, 0x70] : "array.atomic.rmw.xchg",
+        ArrayAtomicRmwCmpxchg(Ordered<Index<'a>>) : [0xfe, 0x71] : "array.atomic.rmw.cmpxchg",
+        RefI31Shared : [0xfe, 0x72] : "ref.i31_shared",
 
         // proposal: simd
         //
@@ -1159,7 +1159,7 @@ instructions! {
         TryTable(TryTable<'a>) : [0x1f] : "try_table",
         Throw(Index<'a>) : [0x08] : "throw",
 
-        // Deprecated exception handling optocdes
+        // Deprecated exception handling opcodes
         Try(Box<BlockType<'a>>) : [0x06] : "try",
         Catch(Index<'a>) : [0x07] : "catch",
         Rethrow(Index<'a>) : [0x09] : "rethrow",


Test patch:
diff --git a/tests/local/shared-everything-threads/i31.wast b/tests/local/shared-everything-threads/i31.wast
new file mode 100644
index 00000000..eec9a707
--- /dev/null
+++ b/tests/local/shared-everything-threads/i31.wast
@@ -0,0 +1,5 @@
+(module
+    (func (result (ref null (shared i31)))
+        (ref.i31_shared (i32.const 0))
+    )
+)
diff --git a/tests/snapshots/local/shared-everything-threads/i31.wast.json b/tests/snapshots/local/shared-everything-threads/i31.wast.json
new file mode 100644
index 00000000..7c5e72a7
--- /dev/null
+++ b/tests/snapshots/local/shared-everything-threads/i31.wast.json
@@ -0,0 +1,10 @@
+{
+  "source_filename": "tests/local/shared-everything-threads/i31.wast",
+  "commands": [
+    {
+      "type": "module",
+      "line": 1,
+      "filename": "i31.0.wasm"
+    }
+  ]
+}
\ No newline at end of file
diff --git a/tests/snapshots/local/shared-everything-threads/i31.wast/0.print b/tests/snapshots/local/shared-everything-threads/i31.wast/0.print
new file mode 100644
index 00000000..d9d704e3
--- /dev/null
+++ b/tests/snapshots/local/shared-everything-threads/i31.wast/0.print
@@ -0,0 +1,7 @@
+(module
+  (type (;0;) (func (result (ref null (shared i31)))))
+  (func (;0;) (type 0) (result (ref null (shared i31)))
+    i32.const 0
+    ref.i31_shared
+  )
+)