threads: add validation for shared calls (#1736)

* threads: add validation for shared calls

The shared-everything-threads [proposal] prevents `shared` contexts
(e.g., in a `shared` function) from accessing unshared objects. This
change adds validation to prevent `shared`-to-unshared calls.

[proposal]: https://github.com/WebAssembly/shared-everything-threads

* threads: refactor and add shared access checks

This checks shareability of other kinds of shared objects (minus
`shared` memories).

* Replace `type_of_function` with `type_index_of_function`

`WasmModuleResources` previously retrieved the entire `FuncType` but
this obscured the sharedness of the function in question, since the
shared flag is held a level up in `CompositeType`. By replacing
`WasmModuleResources::type_of_function` with a more low-level
`WasmModuleResources::type_index_of_function`, we can reuse the existing
helper functions that retrieve a `FuncType` from a `CompositeType` and
do the necessary sharedness checks.

* Clean up `OperatorValidator::new_func`

Author: abrown

crates/wasmparser/src/resources.rs

@@ -50,14 +50,11 @@ pub trait WasmModuleResources {
     /// The sub type must be canonicalized.
     fn sub_type_at(&self, type_index: u32) -> Option<&SubType>;
 
-    /// Returns the type id associated with the given function
-    /// index.
+    /// Returns the type ID associated with the given function index.
     fn type_id_of_function(&self, func_idx: u32) -> Option<CoreTypeId>;
 
-    /// Returns the `FuncType` associated with the given function index.
-    ///
-    /// The function type must be canonicalized.
-    fn type_of_function(&self, func_idx: u32) -> Option<&FuncType>;
+    /// Returns the type index associated with the given function index.
+    fn type_index_of_function(&self, func_index: u32) -> Option<u32>;
 
     /// Returns the element type at the given index.
     ///
@@ -153,8 +150,8 @@ where
     fn type_id_of_function(&self, func_idx: u32) -> Option<CoreTypeId> {
         T::type_id_of_function(self, func_idx)
     }
-    fn type_of_function(&self, func_idx: u32) -> Option<&FuncType> {
-        T::type_of_function(self, func_idx)
+    fn type_index_of_function(&self, func_idx: u32) -> Option<u32> {
+        T::type_index_of_function(self, func_idx)
     }
     fn check_heap_type(&self, t: &mut HeapType, offset: usize) -> Result<(), BinaryReaderError> {
         T::check_heap_type(self, t, offset)
@@ -210,8 +207,8 @@ where
         T::type_id_of_function(self, func_idx)
     }
 
-    fn type_of_function(&self, func_idx: u32) -> Option<&FuncType> {
-        T::type_of_function(self, func_idx)
+    fn type_index_of_function(&self, func_idx: u32) -> Option<u32> {
+        T::type_index_of_function(self, func_idx)
     }
 
     fn check_heap_type(&self, t: &mut HeapType, offset: usize) -> Result<(), BinaryReaderError> {

crates/wasmparser/src/validator/core.rs

@@ -1331,9 +1331,8 @@ impl WasmModuleResources for OperatorValidatorResources<'_> {
         self.module.types.get(*type_index as usize).copied()
     }
 
-    fn type_of_function(&self, at: u32) -> Option<&FuncType> {
-        let type_index = self.module.functions.get(at as usize)?;
-        Some(self.sub_type_at(*type_index)?.composite_type.unwrap_func())
+    fn type_index_of_function(&self, at: u32) -> Option<u32> {
+        self.module.functions.get(at as usize).copied()
     }
 
     fn check_heap_type(&self, t: &mut HeapType, offset: usize) -> Result<()> {
@@ -1407,9 +1406,8 @@ impl WasmModuleResources for ValidatorResources {
         self.0.types.get(type_index as usize).copied()
     }
 
-    fn type_of_function(&self, at: u32) -> Option<&FuncType> {
-        let type_index = *self.0.functions.get(at as usize)?;
-        Some(self.sub_type_at(type_index)?.composite_type.unwrap_func())
+    fn type_index_of_function(&self, at: u32) -> Option<u32> {
+        self.0.functions.get(at as usize).copied()
     }
 
     fn check_heap_type(&self, t: &mut HeapType, offset: usize) -> Result<()> {

crates/wasmparser/src/validator/func.rs

@@ -276,7 +276,7 @@ mod tests {
         fn type_id_of_function(&self, _at: u32) -> Option<CoreTypeId> {
             todo!()
         }
-        fn type_of_function(&self, _func_idx: u32) -> Option<&crate::FuncType> {
+        fn type_index_of_function(&self, _at: u32) -> Option<u32> {
             todo!()
         }
         fn check_heap_type(&self, _t: &mut HeapType, _offset: usize) -> Result<()> {

crates/wasmparser/src/validator/operators.rs

@@ -54,6 +54,9 @@ pub(crate) struct OperatorValidator {
     /// Offset of the `end` instruction which emptied the `control` stack, which
     /// must be the end of the function.
     end_which_emptied_control: Option<usize>,
+
+    /// Whether validation is happening in a shared context.
+    shared: bool,
 }
 
 // No science was performed in the creation of this number, feel free to change
@@ -274,6 +277,7 @@ impl OperatorValidator {
             operands,
             control,
             end_which_emptied_control: None,
+            shared: false,
         }
     }
 
@@ -300,17 +304,30 @@ impl OperatorValidator {
             unreachable: false,
             init_height: 0,
         });
-        let params = OperatorValidatorTemp {
-            // This offset is used by the `func_type_at` and `inputs`.
+
+        // Retrieve the function's type via index (`ty`); the `offset` is
+        // necessary due to `sub_type_at`'s error messaging.
+        let sub_ty = OperatorValidatorTemp {
             offset,
             inner: &mut ret,
             resources,
         }
-        .func_type_at(ty)?
-        .params();
-        for ty in params {
-            ret.locals.define(1, *ty);
-            ret.local_inits.push(true);
+        .sub_type_at(ty)?;
+
+        // Set up the function's locals.
+        if let CompositeInnerType::Func(func_ty) = &sub_ty.composite_type.inner {
+            for ty in func_ty.params() {
+                ret.locals.define(1, *ty);
+                ret.local_inits.push(true);
+            }
+        } else {
+            bail!(offset, "expected func type at index {ty}, found {sub_ty}")
+        }
+
+        // If we're in a shared function, ensure we do not access unshared
+        // objects.
+        if sub_ty.composite_type.shared {
+            ret.shared = true;
         }
         Ok(ret)
     }
@@ -912,14 +929,13 @@ where
     /// Returns the corresponding function type for the `func` item located at
     /// `function_index`.
     fn type_of_function(&self, function_index: u32) -> Result<&'resources FuncType> {
-        match self.resources.type_of_function(function_index) {
-            Some(f) => Ok(f),
-            None => {
-                bail!(
-                    self.offset,
-                    "unknown function {function_index}: function index out of bounds",
-                )
-            }
+        if let Some(type_index) = self.resources.type_index_of_function(function_index) {
+            self.func_type_at(type_index)
+        } else {
+            bail!(
+                self.offset,
+                "unknown function {function_index}: function index out of bounds",
+            )
         }
     }
 
@@ -971,7 +987,7 @@ where
     /// table.
     ///
     /// The return value of this function is the function type behind
-    /// `type_index` which must then be passedt o `check_{call,return_call}_ty`.
+    /// `type_index` which must then be passed to `check_{call,return_call}_ty`.
     fn check_call_indirect_ty(
         &mut self,
         type_index: u32,
@@ -1302,6 +1318,12 @@ where
     fn struct_type_at(&self, at: u32) -> Result<&'resources StructType> {
         let sub_ty = self.sub_type_at(at)?;
         if let CompositeInnerType::Struct(struct_ty) = &sub_ty.composite_type.inner {
+            if self.inner.shared && !sub_ty.composite_type.shared {
+                bail!(
+                    self.offset,
+                    "shared functions cannot access unshared structs",
+                );
+            }
             Ok(struct_ty)
         } else {
             bail!(
@@ -1342,6 +1364,12 @@ where
     fn array_type_at(&self, at: u32) -> Result<FieldType> {
         let sub_ty = self.sub_type_at(at)?;
         if let CompositeInnerType::Array(array_ty) = &sub_ty.composite_type.inner {
+            if self.inner.shared && !sub_ty.composite_type.shared {
+                bail!(
+                    self.offset,
+                    "shared functions cannot access unshared arrays",
+                );
+            }
             Ok(array_ty.0)
         } else {
             bail!(
@@ -1365,6 +1393,12 @@ where
     fn func_type_at(&self, at: u32) -> Result<&'resources FuncType> {
         let sub_ty = self.sub_type_at(at)?;
         if let CompositeInnerType::Func(func_ty) = &sub_ty.composite_type.inner {
+            if self.inner.shared && !sub_ty.composite_type.shared {
+                bail!(
+                    self.offset,
+                    "shared functions cannot access unshared functions",
+                );
+            }
             Ok(func_ty)
         } else {
             bail!(
@@ -1382,6 +1416,12 @@ where
 
     fn global_type_at(&self, at: u32) -> Result<GlobalType> {
         if let Some(ty) = self.resources.global_at(at) {
+            if self.inner.shared && !ty.shared {
+                bail!(
+                    self.offset,
+                    "shared functions cannot access unshared globals",
+                );
+            }
             Ok(ty)
         } else {
             bail!(self.offset, "unknown global: global index out of bounds");
@@ -1391,7 +1431,15 @@ where
     /// Validates that the `table` is valid and returns the type it points to.
     fn table_type_at(&self, table: u32) -> Result<TableType> {
         match self.resources.table_at(table) {
-            Some(ty) => Ok(ty),
+            Some(ty) => {
+                if self.inner.shared && !ty.shared {
+                    bail!(
+                        self.offset,
+                        "shared functions cannot access unshared tables",
+                    );
+                }
+                Ok(ty)
+            }
             None => bail!(
                 self.offset,
                 "unknown table {table}: table index out of bounds"

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

@@ -0,0 +1,64 @@
+;; Check that shared functions are valid WAT.
+(module
+  (type $f (shared (func)))
+  (func (import "spectest" "shared-func") (type $f))
+  (func (type $f))
+)
+
+;; Check that unshared functions cannot be called from shared functions.
+(assert_invalid
+  (module
+    (type $shared (shared (func)))
+    (type $unshared (func))
+    (func (type $shared)
+      call $unshared)
+    (func $unshared (type $unshared))
+  )
+  "shared functions cannot access unshared functions")
+
+;; Check that unshared globals cannot be accessed from shared functions.
+(assert_invalid
+  (module
+    (global $unshared_global i32 (i32.const 0))
+    (type $shared_func (shared (func)))
+    (func (type $shared_func)
+      global.get $unshared_global
+      drop)
+    )
+  "shared functions cannot access unshared globals")
+
+;; Check that unshared tables cannot be accessed from shared functions.
+(assert_invalid
+  (module
+    (table $unshared_table 1 anyref)
+    (type $shared_func (shared (func)))
+    (func (type $shared_func)
+      i32.const 0
+      table.get $unshared_table
+      drop)
+    )
+  "shared functions cannot access unshared tables")
+
+;; Check that unshared arrays cannot be accessed from shared functions.
+(assert_invalid
+  (module
+    (type $unshared_array (array anyref))
+    (type $shared_func (shared (func)))
+    (func (type $shared_func)
+      (array.new $unshared_array (ref.null any) (i32.const 0))
+      (array.get $unshared_array (i32.const 0))
+      drop)
+    )
+  "shared functions cannot access unshared arrays")
+
+;; Check that unshared structs cannot be accessed from shared functions.
+(assert_invalid
+  (module
+    (type $unshared_struct (struct (field i32)))
+    (type $shared_func (shared (func)))
+    (func (type $shared_func)
+      (struct.new $unshared_struct (i32.const 0))
+      (struct.get $unshared_struct 0)
+      drop)
+    )
+  "shared functions cannot access unshared structs")

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

@@ -1,5 +1,3 @@
-;; Check shared attributes for global heap types.
-
 ;; `func` references.
 (module
   ;; Imported (long/short forms, mut, null).

tests/snapshots/local/shared-everything-threads/funcs.wast.json

@@ -0,0 +1,45 @@
+{
+  "source_filename": "tests/local/shared-everything-threads/funcs.wast",
+  "commands": [
+    {
+      "type": "module",
+      "line": 2,
+      "filename": "funcs.0.wasm"
+    },
+    {
+      "type": "assert_invalid",
+      "line": 10,
+      "filename": "funcs.1.wasm",
+      "text": "shared functions cannot access unshared functions",
+      "module_type": "binary"
+    },
+    {
+      "type": "assert_invalid",
+      "line": 21,
+      "filename": "funcs.2.wasm",
+      "text": "shared functions cannot access unshared globals",
+      "module_type": "binary"
+    },
+    {
+      "type": "assert_invalid",
+      "line": 32,
+      "filename": "funcs.3.wasm",
+      "text": "shared functions cannot access unshared tables",
+      "module_type": "binary"
+    },
+    {
+      "type": "assert_invalid",
+      "line": 44,
+      "filename": "funcs.4.wasm",
+      "text": "shared functions cannot access unshared arrays",
+      "module_type": "binary"
+    },
+    {
+      "type": "assert_invalid",
+      "line": 56,
+      "filename": "funcs.5.wasm",
+      "text": "shared functions cannot access unshared structs",
+      "module_type": "binary"
+    }
+  ]
+}

tests/snapshots/local/shared-everything-threads/funcs.wast/0.print

@@ -0,0 +1,5 @@
+(module
+  (type $f (;0;) (shared(func)))
+  (import "spectest" "shared-func" (func (;0;) (type $f)))
+  (func (;1;) (type $f))
+)