Skip over unsupported instructions instead of panicking

crates/cli-support/Cargo.toml

@@ -28,3 +28,6 @@ wasm-bindgen-wasm-interpreter = { path = "../wasm-interpreter", version = '=0.2.
 wit-text = "0.8.0"
 wit-walrus = "0.6.0"
 wit-validator = "0.2.0"
+
+[dev-dependencies]
+wat = "1.0"

crates/cli-support/src/descriptor.rs

@@ -108,14 +108,17 @@ pub enum VectorKind {
 }
 
 impl Descriptor {
-    pub fn decode(mut data: &[u32]) -> Descriptor {
-        let descriptor = Descriptor::_decode(&mut data, false);
-        assert!(data.is_empty(), "remaining data {:?}", data);
-        descriptor
+    /// Decodes `data` into a descriptor, or returns `None` if it's invalid.
+    pub fn decode(mut data: &[u32]) -> Option<Descriptor> {
+        let descriptor = Descriptor::_decode(&mut data, false)?;
+        if !data.is_empty() {
+            return None;
+        }
+        Some(descriptor)
     }
 
-    fn _decode(data: &mut &[u32], clamped: bool) -> Descriptor {
-        match get(data) {
+    fn _decode(data: &mut &[u32], clamped: bool) -> Option<Descriptor> {
+        Some(match get(data)? {
             I8 => Descriptor::I8,
             I16 => Descriptor::I16,
             I32 => Descriptor::I32,
@@ -128,31 +131,31 @@ impl Descriptor {
             F32 => Descriptor::F32,
             F64 => Descriptor::F64,
             BOOLEAN => Descriptor::Boolean,
-            FUNCTION => Descriptor::Function(Box::new(Function::decode(data))),
-            CLOSURE => Descriptor::Closure(Box::new(Closure::decode(data))),
-            REF => Descriptor::Ref(Box::new(Descriptor::_decode(data, clamped))),
-            REFMUT => Descriptor::RefMut(Box::new(Descriptor::_decode(data, clamped))),
-            SLICE => Descriptor::Slice(Box::new(Descriptor::_decode(data, clamped))),
-            VECTOR => Descriptor::Vector(Box::new(Descriptor::_decode(data, clamped))),
-            OPTIONAL => Descriptor::Option(Box::new(Descriptor::_decode(data, clamped))),
-            RESULT => Descriptor::Result(Box::new(Descriptor::_decode(data, clamped))),
+            FUNCTION => Descriptor::Function(Box::new(Function::decode(data)?)),
+            CLOSURE => Descriptor::Closure(Box::new(Closure::decode(data)?)),
+            REF => Descriptor::Ref(Box::new(Descriptor::_decode(data, clamped)?)),
+            REFMUT => Descriptor::RefMut(Box::new(Descriptor::_decode(data, clamped)?)),
+            SLICE => Descriptor::Slice(Box::new(Descriptor::_decode(data, clamped)?)),
+            VECTOR => Descriptor::Vector(Box::new(Descriptor::_decode(data, clamped)?)),
+            OPTIONAL => Descriptor::Option(Box::new(Descriptor::_decode(data, clamped)?)),
+            RESULT => Descriptor::Result(Box::new(Descriptor::_decode(data, clamped)?)),
             CACHED_STRING => Descriptor::CachedString,
             STRING => Descriptor::String,
             EXTERNREF => Descriptor::Externref,
-            ENUM => Descriptor::Enum { hole: get(data) },
+            ENUM => Descriptor::Enum { hole: get(data)? },
             RUST_STRUCT => {
-                let name = get_string(data);
+                let name = get_string(data)?;
                 Descriptor::RustStruct(name)
             }
             NAMED_EXTERNREF => {
-                let name = get_string(data);
+                let name = get_string(data)?;
                 Descriptor::NamedExternref(name)
             }
             CHAR => Descriptor::Char,
             UNIT => Descriptor::Unit,
-            CLAMPED => Descriptor::_decode(data, true),
-            other => panic!("unknown descriptor: {}", other),
-        }
+            CLAMPED => Descriptor::_decode(data, true)?,
+            _ => return None,
+        })
     }
 
     pub fn unwrap_function(self) -> Function {
@@ -204,45 +207,48 @@ impl Descriptor {
     }
 }
 
-fn get(a: &mut &[u32]) -> u32 {
-    let ret = a[0];
+fn get(a: &mut &[u32]) -> Option<u32> {
+    let ret = *a.get(0)?;
     *a = &a[1..];
-    ret
+    Some(ret)
 }
 
-fn get_string(data: &mut &[u32]) -> String {
-    (0..get(data))
-        .map(|_| char::from_u32(get(data)).unwrap())
+fn get_string(data: &mut &[u32]) -> Option<String> {
+    (0..get(data)?)
+        .map(|_| char::from_u32(get(data)?))
         .collect()
 }
 
 impl Closure {
-    fn decode(data: &mut &[u32]) -> Closure {
-        let shim_idx = get(data);
-        let dtor_idx = get(data);
-        let mutable = get(data) == REFMUT;
-        assert_eq!(get(data), FUNCTION);
-        Closure {
+    fn decode(data: &mut &[u32]) -> Option<Closure> {
+        let shim_idx = get(data)?;
+        let dtor_idx = get(data)?;
+        let mutable = get(data)? == REFMUT;
+        if get(data)? != FUNCTION {
+            return None;
+        }
+
+        Some(Closure {
             shim_idx,
             dtor_idx,
             mutable,
-            function: Function::decode(data),
-        }
+            function: Function::decode(data)?,
+        })
     }
 }
 
 impl Function {
-    fn decode(data: &mut &[u32]) -> Function {
-        let shim_idx = get(data);
-        let arguments = (0..get(data))
+    fn decode(data: &mut &[u32]) -> Option<Function> {
+        let shim_idx = get(data)?;
+        let arguments = (0..get(data)?)
             .map(|_| Descriptor::_decode(data, false))
-            .collect::<Vec<_>>();
-        Function {
+            .collect::<Option<Vec<_>>>()?;
+        Some(Function {
             arguments,
             shim_idx,
-            ret: Descriptor::_decode(data, false),
-            inner_ret: Some(Descriptor::_decode(data, false)),
-        }
+            ret: Descriptor::_decode(data, false)?,
+            inner_ret: Some(Descriptor::_decode(data, false)?),
+        })
     }
 }
 

crates/cli-support/src/descriptors.rs

@@ -14,9 +14,10 @@ use crate::descriptor::{Closure, Descriptor};
 use anyhow::Error;
 use std::borrow::Cow;
 use std::collections::{HashMap, HashSet};
+use std::fmt::Write;
 use walrus::ImportId;
 use walrus::{CustomSection, FunctionId, Module, TypedCustomSectionId};
-use wasm_bindgen_wasm_interpreter::Interpreter;
+use wasm_bindgen_wasm_interpreter::{Interpreter, Skip};
 
 #[derive(Default, Debug)]
 pub struct WasmBindgenDescriptorsSection {
@@ -40,6 +41,31 @@ pub fn execute(module: &mut Module) -> Result<WasmBindgenDescriptorsSectionId, E
     Ok(module.customs.add(section))
 }
 
+/// Returns an error for an invalid descriptor.
+///
+/// `skipped` is the list of functions that were skipped while retrieving the
+/// descriptor from the wasm module, which are likely the cause of the error if
+/// present.
+fn descriptor_error(descriptor: &[u32], skipped: &[Skip]) -> anyhow::Error {
+    let mut msg = format!("invalid descriptor: {descriptor:?}");
+
+    if !skipped.is_empty() {
+        writeln!(
+            msg,
+            "\n\n\
+            Some functions containing unsupported instructions were skipped while\n\
+            intepreting the wasm module, which may have caused this:"
+        )
+        .unwrap();
+
+        for skip in skipped {
+            writeln!(msg, "    {skip}").unwrap();
+        }
+    }
+
+    anyhow::Error::msg(msg)
+}
+
 impl WasmBindgenDescriptorsSection {
     fn execute_exports(
         &mut self,
@@ -56,9 +82,10 @@ impl WasmBindgenDescriptorsSection {
                 walrus::ExportItem::Function(id) => id,
                 _ => panic!("{} export not a function", export.name),
             };
-            if let Some(d) = interpreter.interpret_descriptor(id, module) {
+            if let Some((d, skipped)) = interpreter.interpret_descriptor(id, module) {
                 let name = &export.name[prefix.len()..];
-                let descriptor = Descriptor::decode(d);
+                let descriptor =
+                    Descriptor::decode(d).ok_or_else(|| descriptor_error(d, &skipped))?;
                 self.descriptors.insert(name.to_string(), descriptor);
             }
             to_remove.push(export.id());
@@ -96,10 +123,14 @@ impl WasmBindgenDescriptorsSection {
             };
             dfs_in_order(&mut find, local, local.entry_block());
             if find.found {
-                let descriptor = interpreter
+                let (descriptor, skipped) = interpreter
                     .interpret_closure_descriptor(id, module, &mut element_removal_list)
                     .unwrap();
-                func_to_descriptor.insert(id, Descriptor::decode(descriptor));
+                func_to_descriptor.insert(
+                    id,
+                    Descriptor::decode(descriptor)
+                        .ok_or_else(|| descriptor_error(descriptor, &skipped))?,
+                );
             }
         }
 
@@ -179,3 +210,47 @@ impl CustomSection for WasmBindgenDescriptorsSection {
         panic!("shouldn't emit custom sections just yet");
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use super::WasmBindgenDescriptorsSectionId;
+
+    fn execute(wat: &str) -> anyhow::Result<WasmBindgenDescriptorsSectionId> {
+        let wasm = wat::parse_str(wat).unwrap();
+        let mut module = walrus::Module::from_buffer(&wasm).unwrap();
+        super::execute(&mut module)
+    }
+
+    #[test]
+    fn unsupported_instruction() {
+        let result = execute(
+            r#"
+            (module
+                (import "__wbindgen_placeholder__" "__wbindgen_describe"
+                (func $__wbindgen_describe (param i32)))
+
+                (func $bar
+                    ;; We don't support `block`, so this will cause an error.
+                    block
+                    end
+
+                    ;; Which means this descriptor won't go through.
+                    i32.const 0
+                    call $__wbindgen_describe
+                )
+
+                (func $foo
+                    ;; Call into a nested function `bar`, since an unsupported instruction in the
+                    ;; root function we're calling panics immediately.
+                    call $bar
+                )
+
+                (export "__wbindgen_describe_foo" (func $foo))
+            )
+            "#,
+        );
+        let err = result.unwrap_err();
+        assert!(err.to_string().contains("invalid descriptor: []"));
+        assert!(err.to_string().contains("Block"));
+    }
+}