Repository: bytecodealliance/wasm-tools
Pull request #1969 (2025-01-08 16:02:08)

Fail deterministically on invalid modules
Inspired by bytecodealliance/wasmtime#9947 this helps keep the output of `wasm-tools validate` more deterministic by ensuring that the first error is reported instead of any error within a module.


Patch:
diff --git a/crates/wasmparser/src/validator/component.rs b/crates/wasmparser/src/validator/component.rs
index ed70e597..96994f33 100644
--- a/crates/wasmparser/src/validator/component.rs
+++ b/crates/wasmparser/src/validator/component.rs
@@ -2970,7 +2970,6 @@ impl ComponentState {
                 match self.core_instance_export(instance_index, name, types, offset)? {
                     $expected(ty) => {
                         self.$collection.push(*ty);
-                        Ok(())
                     }
                     _ => {
                         bail!(
@@ -2992,7 +2991,7 @@ impl ComponentState {
                     "functions",
                     offset,
                 )?;
-                push_module_export!(EntityType::Func, core_funcs, "function")
+                push_module_export!(EntityType::Func, core_funcs, "function");
             }
             ExternalKind::Table => {
                 check_max(
@@ -3002,7 +3001,21 @@ impl ComponentState {
                     "tables",
                     offset,
                 )?;
-                push_module_export!(EntityType::Table, core_tables, "table")
+                push_module_export!(EntityType::Table, core_tables, "table");
+
+                let ty = self.core_tables.last().unwrap();
+                if ty.table64 {
+                    bail!(
+                        offset,
+                        "64-bit tables are not compatible with components yet"
+                    );
+                }
+                if ty.shared {
+                    bail!(
+                        offset,
+                        "shared tables are not compatible with components yet"
+                    );
+                }
             }
             ExternalKind::Memory => {
                 check_max(
@@ -3012,7 +3025,21 @@ impl ComponentState {
                     "memories",
                     offset,
                 )?;
-                push_module_export!(EntityType::Memory, core_memories, "memory")
+                push_module_export!(EntityType::Memory, core_memories, "memory");
+
+                let ty = self.core_memories.last().unwrap();
+                if ty.memory64 {
+                    bail!(
+                        offset,
+                        "64-bit linear memories are not compatible with components yet"
+                    );
+                }
+                if ty.shared {
+                    bail!(
+                        offset,
+                        "shared linear memories are not compatible with components yet"
+                    );
+                }
             }
             ExternalKind::Global => {
                 check_max(
@@ -3022,7 +3049,7 @@ impl ComponentState {
                     "globals",
                     offset,
                 )?;
-                push_module_export!(EntityType::Global, core_globals, "global")
+                push_module_export!(EntityType::Global, core_globals, "global");
             }
             ExternalKind::Tag => {
                 check_max(
@@ -3032,9 +3059,11 @@ impl ComponentState {
                     "tags",
                     offset,
                 )?;
-                push_module_export!(EntityType::Tag, core_tags, "tag")
+                push_module_export!(EntityType::Tag, core_tags, "tag");
             }
         }
+
+        Ok(())
     }
 
     fn alias_instance_export(
diff --git a/src/bin/wasm-tools/validate.rs b/src/bin/wasm-tools/validate.rs
index 399c97d4..dc44f6de 100644
--- a/src/bin/wasm-tools/validate.rs
+++ b/src/bin/wasm-tools/validate.rs
@@ -144,21 +144,31 @@ impl Opts {
         }
         log::info!("module structure validated in {:?}", start.elapsed());
 
-        // After we've validate the entire wasm module we'll use `rayon` to iterate
-        // over all functions in parallel and perform parallel validation of the
-        // input wasm module.
+        // After we've validate the entire wasm module we'll use `rayon` to
+        // iterate over all functions in parallel and perform parallel
+        // validation of the input wasm module.
+        //
+        // Note that validation results for each function are collected into a
+        // vector to ensure that in the case of multiple errors the first is
+        // always reported. Otherwise `rayon` does not guarantee the order that
+        // failures show up in.
         let start = Instant::now();
-        functions_to_validate.into_par_iter().try_for_each_init(
-            FuncValidatorAllocations::default,
-            |allocs, (to_validate, body)| -> Result<_> {
-                let mut validator = to_validate.into_validator(mem::take(allocs));
-                validator
-                    .validate(&body)
-                    .with_context(|| format!("func {} failed to validate", validator.index()))?;
-                *allocs = validator.into_allocations();
-                Ok(())
-            },
-        )?;
+        functions_to_validate
+            .into_par_iter()
+            .map_init(
+                FuncValidatorAllocations::default,
+                |allocs, (to_validate, body)| -> Result<_> {
+                    let mut validator = to_validate.into_validator(mem::take(allocs));
+                    validator.validate(&body).with_context(|| {
+                        format!("func {} failed to validate", validator.index())
+                    })?;
+                    *allocs = validator.into_allocations();
+                    Ok(())
+                },
+            )
+            .collect::<Vec<_>>()
+            .into_iter()
+            .collect::<Result<Vec<_>>>()?;
         log::info!("functions validated in {:?}", start.elapsed());
         Ok(())
     }


Test patch:
diff --git a/tests/cli/multiple-failures.wat b/tests/cli/multiple-failures.wat
new file mode 100644
index 00000000..214afb23
--- /dev/null
+++ b/tests/cli/multiple-failures.wat
@@ -0,0 +1,105 @@
+;; FAIL: validate %
+
+(module
+  (func (result i32))
+  (func (result i32))
+  (func (result i32))
+  (func (result i32))
+  (func (result i32))
+  (func (result i32))
+  (func (result i32))
+  (func (result i32))
+  (func (result i32))
+  (func (result i32))
+  (func (result i32))
+  (func (result i32))
+  (func (result i32))
+  (func (result i32))
+  (func (result i32))
+  (func (result i32))
+  (func (result i32))
+  (func (result i32))
+  (func (result i32))
+  (func (result i32))
+  (func (result i32))
+  (func (result i32))
+  (func (result i32))
+  (func (result i32))
+  (func (result i32))
+  (func (result i32))
+  (func (result i32))
+  (func (result i32))
+  (func (result i32))
+  (func (result i32))
+  (func (result i32))
+  (func (result i32))
+  (func (result i32))
+  (func (result i32))
+  (func (result i32))
+  (func (result i32))
+  (func (result i32))
+  (func (result i32))
+  (func (result i32))
+  (func (result i32))
+  (func (result i32))
+  (func (result i32))
+  (func (result i32))
+  (func (result i32))
+  (func (result i32))
+  (func (result i32))
+  (func (result i32))
+  (func (result i32))
+  (func (result i32))
+  (func (result i32))
+  (func (result i32))
+  (func (result i32))
+  (func (result i32))
+  (func (result i32))
+  (func (result i32))
+  (func (result i32))
+  (func (result i32))
+  (func (result i32))
+  (func (result i32))
+  (func (result i32))
+  (func (result i32))
+  (func (result i32))
+  (func (result i32))
+  (func (result i32))
+  (func (result i32))
+  (func (result i32))
+  (func (result i32))
+  (func (result i32))
+  (func (result i32))
+  (func (result i32))
+  (func (result i32))
+  (func (result i32))
+  (func (result i32))
+  (func (result i32))
+  (func (result i32))
+  (func (result i32))
+  (func (result i32))
+  (func (result i32))
+  (func (result i32))
+  (func (result i32))
+  (func (result i32))
+  (func (result i32))
+  (func (result i32))
+  (func (result i32))
+  (func (result i32))
+  (func (result i32))
+  (func (result i32))
+  (func (result i32))
+  (func (result i32))
+  (func (result i32))
+  (func (result i32))
+  (func (result i32))
+  (func (result i32))
+  (func (result i32))
+  (func (result i32))
+  (func (result i32))
+  (func (result i32))
+  (func (result i32))
+  (func (result i32))
+  (func (result i32))
+  (func (result i32))
+)
diff --git a/tests/cli/multiple-failures.wat.stderr b/tests/cli/multiple-failures.wat.stderr
new file mode 100644
index 00000000..7e52e8a8
--- /dev/null
+++ b/tests/cli/multiple-failures.wat.stderr
@@ -0,0 +1,4 @@
+error: func 0 failed to validate
+
+Caused by:
+    0: type mismatch: expected i32 but nothing on stack (at offset 0x7d)
diff --git a/tests/local/component-model/memory64.wast b/tests/local/component-model/memory64.wast
index 612e4b72..55534ee1 100644
--- a/tests/local/component-model/memory64.wast
+++ b/tests/local/component-model/memory64.wast
@@ -19,3 +19,21 @@
     (core instance $a (instantiate $A (with "" (instance $b))))
   )
   "mismatch in index type used for memories")
+
+(assert_invalid
+  (component
+    (core module $A
+      (memory (export "m") i64 1))
+    (core instance $A (instantiate $A))
+    (alias core export $A "m" (core memory $m))
+  )
+  "64-bit linear memories are not compatible with components yet")
+
+(assert_invalid
+  (component
+    (core module $A
+      (table (export "m") i64 1 funcref))
+    (core instance $A (instantiate $A))
+    (alias core export $A "m" (core table $m))
+  )
+  "64-bit tables are not compatible with components yet")
diff --git a/tests/local/component-model/shared-everything-threads/not-accepted.wast b/tests/local/component-model/shared-everything-threads/not-accepted.wast
new file mode 100644
index 00000000..ebc19a90
--- /dev/null
+++ b/tests/local/component-model/shared-everything-threads/not-accepted.wast
@@ -0,0 +1,18 @@
+(assert_invalid
+  (component
+    (core module $A
+      (memory (export "m") 1 2 shared))
+    (core instance $A (instantiate $A))
+    (alias core export $A "m" (core memory $m))
+  )
+  "shared linear memories are not compatible with components yet")
+
+(assert_invalid
+  (component
+    (core module $A
+      (table (export "m") shared 1 2 (ref null (shared func)))
+    )
+    (core instance $A (instantiate $A))
+    (alias core export $A "m" (core table $m))
+  )
+  "shared tables are not compatible with components yet")
diff --git a/tests/snapshots/local/component-model/memory64.wast.json b/tests/snapshots/local/component-model/memory64.wast.json
index 976d3a44..36a0c647 100644
--- a/tests/snapshots/local/component-model/memory64.wast.json
+++ b/tests/snapshots/local/component-model/memory64.wast.json
@@ -14,6 +14,20 @@
       "filename": "memory64.1.wasm",
       "module_type": "binary",
       "text": "mismatch in index type used for memories"
+    },
+    {
+      "type": "assert_invalid",
+      "line": 24,
+      "filename": "memory64.2.wasm",
+      "module_type": "binary",
+      "text": "64-bit linear memories are not compatible with components yet"
+    },
+    {
+      "type": "assert_invalid",
+      "line": 33,
+      "filename": "memory64.3.wasm",
+      "module_type": "binary",
+      "text": "64-bit tables are not compatible with components yet"
     }
   ]
 }
\ No newline at end of file
diff --git a/tests/snapshots/local/component-model/shared-everything-threads/not-accepted.wast.json b/tests/snapshots/local/component-model/shared-everything-threads/not-accepted.wast.json
new file mode 100644
index 00000000..d0634bce
--- /dev/null
+++ b/tests/snapshots/local/component-model/shared-everything-threads/not-accepted.wast.json
@@ -0,0 +1,19 @@
+{
+  "source_filename": "tests/local/component-model/shared-everything-threads/not-accepted.wast",
+  "commands": [
+    {
+      "type": "assert_invalid",
+      "line": 2,
+      "filename": "not-accepted.0.wasm",
+      "module_type": "binary",
+      "text": "shared linear memories are not compatible with components yet"
+    },
+    {
+      "type": "assert_invalid",
+      "line": 11,
+      "filename": "not-accepted.1.wasm",
+      "module_type": "binary",
+      "text": "shared tables are not compatible with components yet"
+    }
+  ]
+}
\ No newline at end of file