threads: add validation for global constant expressions

Previously, it was possible to use an unshared global to initialize a
shared global, table element. This change makes that invalid by
propagating sharedness in to the constant expression validator.

Author: abrown

crates/wasmparser/src/validator/core.rs

@@ -133,7 +133,7 @@ impl ModuleState {
     ) -> Result<()> {
         self.module
             .check_global_type(&mut global.ty, features, types, offset)?;
-        self.check_const_expr(&global.init_expr, global.ty.content_type, features, types)?;
+        self.check_const_expr(&global.init_expr, global.ty.content_type, global.ty.shared, features, types)?;
         self.module.assert_mut().globals.push(global.ty);
         Ok(())
     }
@@ -162,7 +162,7 @@ impl ModuleState {
                          the function-references proposal"
                     );
                 }
-                self.check_const_expr(expr, table.ty.element_type.into(), features, types)?;
+                self.check_const_expr(expr, table.ty.element_type.into(), table.ty.shared, features, types)?;
             }
         }
         self.module.assert_mut().tables.push(table.ty);
@@ -182,8 +182,8 @@ impl ModuleState {
                 memory_index,
                 offset_expr,
             } => {
-                let ty = self.module.memory_at(memory_index, offset)?.index_type();
-                self.check_const_expr(&offset_expr, ty, features, types)
+                let ty = self.module.memory_at(memory_index, offset)?;
+                self.check_const_expr(&offset_expr, ty.index_type(), ty.shared, features, types)
             }
         }
     }
@@ -195,8 +195,8 @@ impl ModuleState {
         types: &TypeList,
         offset: usize,
     ) -> Result<()> {
-        // the `funcref` value type is allowed all the way back to the MVP, so
-        // don't check it here
+        // The `funcref` value type is allowed all the way back to the MVP, so
+        // don't check it here.
         let element_ty = match &mut e.items {
             crate::ElementItems::Functions(_) => RefType::FUNC,
             crate::ElementItems::Expressions(ty, _) => {
@@ -221,8 +221,7 @@ impl ModuleState {
                         offset,
                     ));
                 }
-
-                self.check_const_expr(&offset_expr, table.index_type(), features, types)?;
+                self.check_const_expr(&offset_expr, table.index_type(), table.shared, features, types)?;
             }
             ElementKind::Passive | ElementKind::Declared => {
                 if !features.bulk_memory() {
@@ -256,8 +255,18 @@ impl ModuleState {
             }
             crate::ElementItems::Expressions(ty, reader) => {
                 validate_count(reader.count())?;
+                let shared = match ty.heap_type() {
+                    HeapType::Abstract { shared, .. } => shared,
+                    HeapType::Concrete(unpacked_index) => {
+                        if let Some(id) = unpacked_index.as_core_type_id() {
+                            types[id].composite_type.shared
+                        } else {
+                            todo!()
+                        }
+                    }
+                };
                 for expr in reader {
-                    self.check_const_expr(&expr?, ValType::Ref(ty), features, types)?;
+                    self.check_const_expr(&expr?, ValType::Ref(ty), shared, features, types)?;
                 }
             }
         }
@@ -269,6 +278,7 @@ impl ModuleState {
         &mut self,
         expr: &ConstExpr<'_>,
         expected_ty: ValType,
+        shared: bool,
         features: &WasmFeatures,
         types: &TypeList,
     ) -> Result<()> {
@@ -286,6 +296,7 @@ impl ModuleState {
                 module: &mut self.module,
             },
             features,
+            shared,
         };
 
         let mut ops = expr.get_operators_reader();
@@ -309,6 +320,7 @@ impl ModuleState {
             resources: OperatorValidatorResources<'a>,
             order: Order,
             features: &'a WasmFeatures,
+            shared: bool,
         }
 
         impl VisitConstOperator<'_> {
@@ -374,6 +386,12 @@ impl ModuleState {
                         self.offset,
                     ));
                 }
+                if self.shared && !global.shared {
+                    return Err(BinaryReaderError::new(
+                        "invalid type: constant expression must be shared",
+                        self.offset,
+                    ));
+                }
                 Ok(())
             }
 

tests/local/shared-everything-threads/globals.wast

@@ -26,6 +26,18 @@
   (global (shared mut v128) (v128.const i64x2 0 0))
 )
 
+;; Check that we can only use shared globals to initialize other shared globals.
+(module
+  (global $a (shared i32) (i32.const 0))
+  (global $b (shared i32) (global.get $a))
+)
+(assert_invalid
+  (module
+    (global $a i32 (i32.const 0))
+    (global $b (shared i32) (global.get $a))
+  )
+  "invalid type")
+
 (assert_malformed
   (module quote "(global (mut shared i64) (i64.const -1))")
   "unexpected token")

tests/local/shared-everything-threads/tables.wast

@@ -42,6 +42,23 @@
     (table shared 0 (ref $t)))
   "shared tables must have a shared element type")
 
+;; A shared table must be initialized from shared objects.
+(module
+  (type $f (shared (func)))
+  (global $g (shared (ref null $f)) (ref.null $f))
+  (table $t shared 1 (ref null $f))
+  (elem (ref null $f) (global.get $g))
+)
+(assert_invalid
+  (module
+    (type $f (shared (func)))
+    (global $g (ref null $f) (ref.null $f))
+    (table $t shared 1 (ref null $f))
+    ;; When we initialize a shared element, everything must be shared, including
+    ;; the used global.
+    (elem (ref null $f) (global.get $g)))
+  "invalid type")
+
 ;; Check `table.atomic.*` instructions.
 (module (;eq;)
   (table $a (import "spectest" "table_eq") shared 1 (ref null (shared eq)))