Repository: bytecodealliance/wasm-tools
Pull request #1661 (2024-07-11 13:50:51)

Bring back legacy exception handling opcodes
In this MR I bring back legacy exception handling opcodes, in reference to this report: https://github.com/bytecodealliance/wasm-tools/issues/1653.

@alexcrichton Two things:
- [x] I have prepared the simplest wasm file possible which uses all the deprecated opcodes (see `check_legacy_exception_handling_opcodes` test function),
could you please help me run the validator with this data?

- [x] Since this is behind the `legacy-exception-handling` feature, tests should be run with the `cargo test --all-features` flag, or I can add this feature to default list.
Given this, how should I do it to make sense?

trzeciak: @alexcrichton Okay, I changed 'cargo feature' to WasmFeatures and I added simple wat test file. I also changed some names and description, I think it's ready for review.

Patch:
diff --git a/crates/wasmparser/src/binary_reader.rs b/crates/wasmparser/src/binary_reader.rs
index 4df00290..7f77662d 100644
--- a/crates/wasmparser/src/binary_reader.rs
+++ b/crates/wasmparser/src/binary_reader.rs
@@ -688,6 +688,14 @@ impl<'a> BinaryReader<'a> {
         Ok(Ieee64(value))
     }
 
+    /// (internal) Reads a fixed-size WebAssembly string from the module.
+    fn internal_read_string(&mut self, len: usize) -> Result<&'a str> {
+        let bytes = self.read_bytes(len)?;
+        str::from_utf8(bytes).map_err(|_| {
+            BinaryReaderError::new("malformed UTF-8 encoding", self.original_position() - 1)
+        })
+    }
+
     /// Reads a WebAssembly string from the module.
     /// # Errors
     /// If `BinaryReader` has less than up to four bytes remaining, the string's
@@ -701,10 +709,13 @@ impl<'a> BinaryReader<'a> {
                 self.original_position() - 1,
             ));
         }
-        let bytes = self.read_bytes(len)?;
-        str::from_utf8(bytes).map_err(|_| {
-            BinaryReaderError::new("malformed UTF-8 encoding", self.original_position() - 1)
-        })
+        return self.internal_read_string(len);
+    }
+
+    /// Reads a unlimited WebAssembly string from the module.
+    pub fn read_unlimited_string(&mut self) -> Result<&'a str> {
+        let len = self.read_var_u32()? as usize;
+        return self.internal_read_string(len);
     }
 
     #[cold]
diff --git a/crates/wasmparser/src/features.rs b/crates/wasmparser/src/features.rs
index 6478cf9c..be5e1464 100644
--- a/crates/wasmparser/src/features.rs
+++ b/crates/wasmparser/src/features.rs
@@ -148,6 +148,13 @@ define_wasm_features! {
         pub component_model_nested_names: COMPONENT_MODEL_NESTED_NAMES(1 << 22) = false;
         /// Support for more than 32 flags per-type in the component model.
         pub component_model_more_flags: COMPONENT_MODEL_MORE_FLAGS(1 << 23) = false;
+        /// The WebAssembly legacy exception handling proposal (phase 1)
+        ///
+        /// # Note
+        ///
+        /// Support this feature as long as all leading browsers also support it
+        /// https://github.com/WebAssembly/exception-handling/blob/main/proposals/exception-handling/legacy/Exceptions.md
+        pub legacy_exceptions: LEGACY_EXCEPTIONS(1 << 24) = false;
     }
 }
 
diff --git a/crates/wasmparser/src/lib.rs b/crates/wasmparser/src/lib.rs
index 06b1a9ad..18183a48 100644
--- a/crates/wasmparser/src/lib.rs
+++ b/crates/wasmparser/src/lib.rs
@@ -158,11 +158,11 @@ macro_rules! for_each_operator {
             @exceptions Throw { tag_index: u32 } => visit_throw
             @exceptions ThrowRef => visit_throw_ref
             // Deprecated old instructions from the exceptions proposal
-            @exceptions Try { blockty: $crate::BlockType } => visit_try
-            @exceptions Catch { tag_index: u32 } => visit_catch
-            @exceptions Rethrow { relative_depth: u32 } => visit_rethrow
-            @exceptions Delegate { relative_depth: u32 } => visit_delegate
-            @exceptions CatchAll => visit_catch_all
+            @legacy_exceptions Try { blockty: $crate::BlockType } => visit_try
+            @legacy_exceptions Catch { tag_index: u32 } => visit_catch
+            @legacy_exceptions Rethrow { relative_depth: u32 } => visit_rethrow
+            @legacy_exceptions Delegate { relative_depth: u32 } => visit_delegate
+            @legacy_exceptions CatchAll => visit_catch_all
             @mvp End => visit_end
             @mvp Br { relative_depth: u32 } => visit_br
             @mvp BrIf { relative_depth: u32 } => visit_br_if
diff --git a/crates/wasmparser/src/readers/core/names.rs b/crates/wasmparser/src/readers/core/names.rs
index 986e8950..ef192acb 100644
--- a/crates/wasmparser/src/readers/core/names.rs
+++ b/crates/wasmparser/src/readers/core/names.rs
@@ -33,7 +33,7 @@ pub struct Naming<'a> {
 impl<'a> FromReader<'a> for Naming<'a> {
     fn from_reader(reader: &mut BinaryReader<'a>) -> Result<Self> {
         let index = reader.read_var_u32()?;
-        let name = reader.read_string()?;
+        let name = reader.read_unlimited_string()?;
         Ok(Naming { index, name })
     }
 }
diff --git a/crates/wasmparser/src/validator/operators.rs b/crates/wasmparser/src/validator/operators.rs
index e0f2c7ae..824fe093 100644
--- a/crates/wasmparser/src/validator/operators.rs
+++ b/crates/wasmparser/src/validator/operators.rs
@@ -121,6 +121,24 @@ pub enum FrameKind {
     ///
     /// This belongs to the Wasm exception handling proposal.
     TryTable,
+    /// A Wasm legacy `try` control block.
+    ///
+    /// # Note
+    ///
+    /// See: `WasmFeatures::legacy_exceptions` Note in `crates/wasmparser/src/features.rs`
+    LegacyTry,
+    /// A Wasm legacy `catch` control block.
+    ///
+    /// # Note
+    ///
+    /// See: `WasmFeatures::legacy_exceptions` Note in `crates/wasmparser/src/features.rs`
+    LegacyCatch,
+    /// A Wasm legacy `catch_all` control block.
+    ///
+    /// # Note
+    ///
+    /// See: `WasmFeatures::legacy_exceptions` Note in `crates/wasmparser/src/features.rs`
+    LegacyCatchAll,
 }
 
 struct OperatorValidatorTemp<'validator, 'resources, T> {
@@ -1322,6 +1340,7 @@ macro_rules! validate_proposal {
     (desc function_references) => ("function references");
     (desc memory_control) => ("memory control");
     (desc gc) => ("gc");
+    (desc legacy_exceptions) => ("legacy exceptions");
 }
 
 impl<'a, T> VisitOperator<'a> for WasmProposalValidator<'_, '_, T>
@@ -1486,21 +1505,6 @@ where
         self.unreachable()?;
         Ok(())
     }
-    fn visit_try(&mut self, _: BlockType) -> Self::Output {
-        bail!(self.offset, "unimplemented validation of deprecated opcode")
-    }
-    fn visit_catch(&mut self, _: u32) -> Self::Output {
-        bail!(self.offset, "unimplemented validation of deprecated opcode")
-    }
-    fn visit_rethrow(&mut self, _: u32) -> Self::Output {
-        bail!(self.offset, "unimplemented validation of deprecated opcode")
-    }
-    fn visit_delegate(&mut self, _: u32) -> Self::Output {
-        bail!(self.offset, "unimplemented validation of deprecated opcode")
-    }
-    fn visit_catch_all(&mut self) -> Self::Output {
-        bail!(self.offset, "unimplemented validation of deprecated opcode")
-    }
     fn visit_end(&mut self) -> Self::Output {
         let mut frame = self.pop_ctrl()?;
 
@@ -4124,6 +4128,80 @@ where
         self.pop_operand(Some(ValType::Ref(RefType::I31REF)))?;
         self.push_operand(ValType::I32)
     }
+    fn visit_try(&mut self, mut ty: BlockType) -> Self::Output {
+        self.check_block_type(&mut ty)?;
+        for ty in self.params(ty)?.rev() {
+            self.pop_operand(Some(ty))?;
+        }
+        self.push_ctrl(FrameKind::LegacyTry, ty)?;
+        Ok(())
+    }
+    fn visit_catch(&mut self, index: u32) -> Self::Output {
+        let frame = self.pop_ctrl()?;
+        if frame.kind != FrameKind::LegacyTry && frame.kind != FrameKind::LegacyCatch {
+            bail!(self.offset, "catch found outside of an `try` block");
+        }
+        // Start a new frame and push `exnref` value.
+        let height = self.operands.len();
+        let init_height = self.inits.len();
+        self.control.push(Frame {
+            kind: FrameKind::LegacyCatch,
+            block_type: frame.block_type,
+            height,
+            unreachable: false,
+            init_height,
+        });
+        // Push exception argument types.
+        let ty = self.tag_at(index)?;
+        for ty in ty.params() {
+            self.push_operand(*ty)?;
+        }
+        Ok(())
+    }
+    fn visit_rethrow(&mut self, relative_depth: u32) -> Self::Output {
+        // This is not a jump, but we need to check that the `rethrow`
+        // targets an actual `catch` to get the exception.
+        let (_, kind) = self.jump(relative_depth)?;
+        if kind != FrameKind::LegacyCatch && kind != FrameKind::LegacyCatchAll {
+            bail!(
+                self.offset,
+                "invalid rethrow label: target was not a `catch` block"
+            );
+        }
+        self.unreachable()?;
+        Ok(())
+    }
+    fn visit_delegate(&mut self, relative_depth: u32) -> Self::Output {
+        let frame = self.pop_ctrl()?;
+        if frame.kind != FrameKind::LegacyTry {
+            bail!(self.offset, "delegate found outside of an `try` block");
+        }
+        // This operation is not a jump, but we need to check the
+        // depth for validity
+        let _ = self.jump(relative_depth)?;
+        for ty in self.results(frame.block_type)? {
+            self.push_operand(ty)?;
+        }
+        Ok(())
+    }
+    fn visit_catch_all(&mut self) -> Self::Output {
+        let frame = self.pop_ctrl()?;
+        if frame.kind == FrameKind::LegacyCatchAll {
+            bail!(self.offset, "only one catch_all allowed per `try` block");
+        } else if frame.kind != FrameKind::LegacyTry && frame.kind != FrameKind::LegacyCatch {
+            bail!(self.offset, "catch_all found outside of a `try` block");
+        }
+        let height = self.operands.len();
+        let init_height = self.inits.len();
+        self.control.push(Frame {
+            kind: FrameKind::LegacyCatchAll,
+            block_type: frame.block_type,
+            height,
+            unreachable: false,
+            init_height,
+        });
+        Ok(())
+    }
 }
 
 #[derive(Clone, Debug)]
diff --git a/src/bin/wasm-tools/component.rs b/src/bin/wasm-tools/component.rs
index 1b15b2f5..adc24f30 100644
--- a/src/bin/wasm-tools/component.rs
+++ b/src/bin/wasm-tools/component.rs
@@ -648,11 +648,18 @@ impl WitOpts {
                         dir.join("deps")
                     };
                     let packages_with_same_name = &names[&pkg.name.name];
+                    let packages_with_same_namespace = packages_with_same_name[&pkg.name.namespace];
                     let stem = if packages_with_same_name.len() == 1 {
-                        pkg.name.name.clone()
+                        if packages_with_same_namespace == 1 {
+                            pkg.name.name.clone()
+                        } else {
+                            pkg.name
+                                .version
+                                .as_ref()
+                                .map(|ver| format!("{}@{}", pkg.name.name, ver))
+                                .unwrap_or_else(|| pkg.name.name.clone())
+                        }
                     } else {
-                        let packages_with_same_namespace =
-                            packages_with_same_name[&pkg.name.namespace];
                         if packages_with_same_namespace == 1 {
                             format!("{}:{}", pkg.name.namespace, pkg.name.name)
                         } else {
diff --git a/src/bin/wasm-tools/validate.rs b/src/bin/wasm-tools/validate.rs
index c0268ccb..fe402b13 100644
--- a/src/bin/wasm-tools/validate.rs
+++ b/src/bin/wasm-tools/validate.rs
@@ -201,6 +201,7 @@ fn parse_features(arg: &str) -> Result<WasmFeatures> {
         ("mutable-global", WasmFeatures::MUTABLE_GLOBAL),
         ("relaxed-simd", WasmFeatures::RELAXED_SIMD),
         ("gc", WasmFeatures::GC),
+        ("legacy-exceptions", WasmFeatures::LEGACY_EXCEPTIONS),
     ];
 
     for part in arg.split(',').map(|s| s.trim()).filter(|s| !s.is_empty()) {


Test patch:
diff --git a/tests/cli/wit-directory-output-in-deps-folder-with-multiple-version.wit b/tests/cli/wit-directory-output-in-deps-folder-with-multiple-version.wit
new file mode 100644
index 00000000..980d99db
--- /dev/null
+++ b/tests/cli/wit-directory-output-in-deps-folder-with-multiple-version.wit
@@ -0,0 +1,16 @@
+// RUN: component wit % --out-dir %tmpdir | component wit %tmpdir
+
+package a:b@0.2.0 {
+  interface foo {}
+}
+
+package a:b {
+  interface foo {}
+}
+
+package a:c {
+  world a {
+    import a:b/foo@0.2.0;
+    import a:b/foo;
+  }
+}
\ No newline at end of file
diff --git a/tests/cli/wit-directory-output-in-deps-folder-with-multiple-version.wit.stdout b/tests/cli/wit-directory-output-in-deps-folder-with-multiple-version.wit.stdout
new file mode 100644
index 00000000..f306ee9c
--- /dev/null
+++ b/tests/cli/wit-directory-output-in-deps-folder-with-multiple-version.wit.stdout
@@ -0,0 +1,20 @@
+package a:b {
+  interface foo {
+  }
+
+}
+
+
+package a:b@0.2.0 {
+  interface foo {
+  }
+
+}
+
+
+package a:c {
+  world a {
+    import a:b/foo@0.2.0;
+    import a:b/foo;
+  }
+}
diff --git a/tests/local/legacy-exceptions.wat b/tests/local/legacy-exceptions.wat
new file mode 100644
index 00000000..f923a69a
--- /dev/null
+++ b/tests/local/legacy-exceptions.wat
@@ -0,0 +1,17 @@
+;; --enable-legacy-exceptions
+(module
+  (type (;0;) (func))
+  (func (;0;) (type 0)
+    try ;; label = @1
+      try ;; label = @2
+        try ;; label = @3
+          throw 0
+        catch_all
+          rethrow 0 (;@3;)
+        end
+      delegate 0 (;@2;)
+    catch 0
+    end
+  )
+  (tag (;0;) (type 0))
+)
diff --git a/tests/roundtrip.rs b/tests/roundtrip.rs
index 6f1b64c7..4a8e4f2b 100644
--- a/tests/roundtrip.rs
+++ b/tests/roundtrip.rs
@@ -579,7 +579,8 @@ impl TestState {
             & !WasmFeatures::SHARED_EVERYTHING_THREADS
             & !WasmFeatures::COMPONENT_MODEL
             & !WasmFeatures::COMPONENT_MODEL_NESTED_NAMES
-            & !WasmFeatures::COMPONENT_MODEL_MORE_FLAGS;
+            & !WasmFeatures::COMPONENT_MODEL_MORE_FLAGS
+            & !WasmFeatures::LEGACY_EXCEPTIONS;
         for part in test.iter().filter_map(|t| t.to_str()) {
             match part {
                 "testsuite" => {
@@ -604,6 +605,7 @@ impl TestState {
                 }
                 "simd" => features.insert(WasmFeatures::SIMD),
                 "exception-handling" => features.insert(WasmFeatures::EXCEPTIONS),
+                "legacy-exceptions.wat" => features.insert(WasmFeatures::LEGACY_EXCEPTIONS),
                 "tail-call" => features.insert(WasmFeatures::TAIL_CALL),
                 "memory64" => features.insert(WasmFeatures::MEMORY64),
                 "component-model" => features.insert(WasmFeatures::COMPONENT_MODEL),
@@ -655,11 +657,13 @@ fn error_matches(error: &str, message: &str) -> bool {
         || message == "alignment must be a power of two"
         || message == "i32 constant out of range"
         || message == "constant expression required"
+        || message == "legacy exceptions support is not enabled"
     {
         return error.contains("expected ")
             || error.contains("constant out of range")
             || error.contains("extra tokens remaining")
-            || error.contains("unimplemented validation of deprecated opcode");
+            || error.contains("unimplemented validation of deprecated opcode")
+            || error.contains("legacy exceptions support is not enabled");
     }
 
     if message == "illegal character" {
diff --git a/tests/snapshots/local/legacy-exceptions.wat.print b/tests/snapshots/local/legacy-exceptions.wat.print
new file mode 100644
index 00000000..8ae9aa1a
--- /dev/null
+++ b/tests/snapshots/local/legacy-exceptions.wat.print
@@ -0,0 +1,16 @@
+(module
+  (type (;0;) (func))
+  (func (;0;) (type 0)
+    try ;; label = @1
+      try ;; label = @2
+        try ;; label = @3
+          throw 0
+        catch_all
+          rethrow 0 (;@3;)
+        end
+      delegate 0 (;@1;)
+    catch 0
+    end
+  )
+  (tag (;0;) (type 0))
+)