Repository: bytecodealliance/wasm-tools
Pull request #1667 (2024-07-12 14:29:32)

Update some documentation about longer names
Adding some follow-up documentation to #1650


Patch:
diff --git a/Cargo.lock b/Cargo.lock
index 2d39c9f5..7edaef94 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -1699,6 +1699,7 @@ dependencies = [
  "addr2line",
  "anyhow",
  "arbitrary",
+ "bitflags",
  "clap",
  "clap_complete",
  "cpp_demangle",
diff --git a/Cargo.toml b/Cargo.toml
index 1ac79b10..5a84f926 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -115,6 +115,7 @@ termcolor = { workspace = true }
 # Dependencies of `validate`
 wasmparser = { workspace = true, optional = true, features = ['validate'] }
 rayon = { workspace = true, optional = true }
+bitflags = { workspace = true, optional = true }
 
 # Dependencies of `print`
 wasmprinter = { workspace = true }
@@ -201,7 +202,7 @@ default = [
 ]
 
 # Each subcommand is gated behind a feature and lists the dependencies it needs
-validate = ['dep:wasmparser', 'rayon', 'dep:addr2line', 'dep:gimli']
+validate = ['dep:wasmparser', 'rayon', 'dep:addr2line', 'dep:gimli', 'dep:bitflags']
 print = []
 parse = []
 smith = ['wasm-smith', 'arbitrary', 'dep:serde', 'dep:serde_derive', 'dep:serde_json']
diff --git a/crates/wasmparser/src/binary_reader.rs b/crates/wasmparser/src/binary_reader.rs
index 7f77662d..c93187df 100644
--- a/crates/wasmparser/src/binary_reader.rs
+++ b/crates/wasmparser/src/binary_reader.rs
@@ -697,7 +697,9 @@ impl<'a> BinaryReader<'a> {
     }
 
     /// Reads a WebAssembly string from the module.
+    ///
     /// # Errors
+    ///
     /// If `BinaryReader` has less than up to four bytes remaining, the string's
     /// length exceeds the remaining bytes, the string's length exceeds
     /// `limits::MAX_WASM_STRING_SIZE`, or the string contains invalid utf-8.
@@ -713,6 +715,10 @@ impl<'a> BinaryReader<'a> {
     }
 
     /// Reads a unlimited WebAssembly string from the module.
+    ///
+    /// Note that this is similar to [`BinaryReader::read_string`] except that
+    /// it will not limit the size of the returned string by
+    /// `limits::MAX_WASM_STRING_SIZE`.
     pub fn read_unlimited_string(&mut self) -> Result<&'a str> {
         let len = self.read_var_u32()? as usize;
         return self.internal_read_string(len);
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 ef192acb..d53211ad 100644
--- a/crates/wasmparser/src/readers/core/names.rs
+++ b/crates/wasmparser/src/readers/core/names.rs
@@ -33,6 +33,9 @@ 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()?;
+        // This seems to match what browsers do where they don't limit the
+        // length of names in the `name` section while they do limit the names
+        // in the import and export section for example.
         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/addr2line.rs b/src/addr2line.rs
index f81986ee..1be023c5 100644
--- a/src/addr2line.rs
+++ b/src/addr2line.rs
@@ -107,7 +107,7 @@ impl<'a> Addr2lineModules<'a> {
                 Some(start) => addr
                     .checked_sub(start)
                     .context("address is before the beginning of the text section")?,
-                None => bail!("no code section found in module"),
+                None => return Ok(None),
             }
         };
 
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..b94596b5 100644
--- a/src/bin/wasm-tools/validate.rs
+++ b/src/bin/wasm-tools/validate.rs
@@ -1,5 +1,6 @@
 use addr2line::LookupResult;
 use anyhow::{anyhow, Context, Result};
+use bitflags::Flags;
 use rayon::prelude::*;
 use std::fmt::Write;
 use std::mem;
@@ -171,37 +172,9 @@ impl Opts {
 fn parse_features(arg: &str) -> Result<WasmFeatures> {
     let mut ret = WasmFeatures::default();
 
-    const FEATURES: &[(&str, WasmFeatures)] = &[
-        ("reference-types", WasmFeatures::REFERENCE_TYPES),
-        ("function-references", WasmFeatures::FUNCTION_REFERENCES),
-        ("simd", WasmFeatures::SIMD),
-        ("threads", WasmFeatures::THREADS),
-        (
-            "shared-everything-threads",
-            WasmFeatures::SHARED_EVERYTHING_THREADS,
-        ),
-        ("bulk-memory", WasmFeatures::BULK_MEMORY),
-        ("multi-value", WasmFeatures::MULTI_VALUE),
-        ("tail-call", WasmFeatures::TAIL_CALL),
-        ("component-model", WasmFeatures::COMPONENT_MODEL),
-        (
-            "component-model-values",
-            WasmFeatures::COMPONENT_MODEL_VALUES,
-        ),
-        ("multi-memory", WasmFeatures::MULTI_MEMORY),
-        ("exception-handling", WasmFeatures::EXCEPTIONS),
-        ("memory64", WasmFeatures::MEMORY64),
-        ("extended-const", WasmFeatures::EXTENDED_CONST),
-        ("floats", WasmFeatures::FLOATS),
-        (
-            "saturating-float-to-int",
-            WasmFeatures::SATURATING_FLOAT_TO_INT,
-        ),
-        ("sign-extension", WasmFeatures::SIGN_EXTENSION),
-        ("mutable-global", WasmFeatures::MUTABLE_GLOBAL),
-        ("relaxed-simd", WasmFeatures::RELAXED_SIMD),
-        ("gc", WasmFeatures::GC),
-    ];
+    fn flag_name(flag: &bitflags::Flag<WasmFeatures>) -> String {
+        flag.name().to_lowercase().replace('_', "-")
+    }
 
     for part in arg.split(',').map(|s| s.trim()).filter(|s| !s.is_empty()) {
         let (enable, part) = if let Some(part) = part.strip_prefix("-") {
@@ -211,28 +184,27 @@ fn parse_features(arg: &str) -> Result<WasmFeatures> {
         };
         match part {
             "all" => {
-                for (name, feature) in FEATURES {
-                    // don't count this under "all" for now.
-                    if *name == "deterministic" {
-                        continue;
-                    }
-                    ret.set(*feature, enable);
+                for flag in WasmFeatures::FLAGS.iter() {
+                    ret.set(*flag.value(), enable);
                 }
             }
 
             name => {
-                let (_, feature) = FEATURES.iter().find(|(n, _)| *n == name).ok_or_else(|| {
-                    anyhow!(
-                        "unknown feature `{}`\nValid features: {}",
-                        name,
-                        FEATURES
-                            .iter()
-                            .map(|(name, _)| *name)
-                            .collect::<Vec<_>>()
-                            .join(", "),
-                    )
-                })?;
-                ret.set(*feature, enable);
+                let flag = WasmFeatures::FLAGS
+                    .iter()
+                    .find(|f| flag_name(f) == name)
+                    .ok_or_else(|| {
+                        anyhow!(
+                            "unknown feature `{}`\nValid features: {}",
+                            name,
+                            WasmFeatures::FLAGS
+                                .iter()
+                                .map(flag_name)
+                                .collect::<Vec<_>>()
+                                .join(", "),
+                        )
+                    })?;
+                ret.set(*flag.value(), enable);
             }
         }
     }


Test patch:
diff --git a/tests/cli/validate-features.wat b/tests/cli/validate-features.wat
new file mode 100644
index 00000000..3aad684f
--- /dev/null
+++ b/tests/cli/validate-features.wat
@@ -0,0 +1,5 @@
+;; FAIL: validate --features=-all %
+
+(module
+  (import "x" "y" (global v128))
+)
diff --git a/tests/cli/validate-features.wat.stderr b/tests/cli/validate-features.wat.stderr
new file mode 100644
index 00000000..45ead379
--- /dev/null
+++ b/tests/cli/validate-features.wat.stderr
@@ -0,0 +1,1 @@
+error: SIMD support is not enabled (at offset 0xb)
diff --git a/tests/cli/validate-features2.wat b/tests/cli/validate-features2.wat
new file mode 100644
index 00000000..4bf1c135
--- /dev/null
+++ b/tests/cli/validate-features2.wat
@@ -0,0 +1,6 @@
+;; RUN: validate --features=-all,simd %
+
+(module
+  (import "x" "y" (global v128))
+)
+
diff --git a/tests/cli/validate-unknown-features.wat b/tests/cli/validate-unknown-features.wat
new file mode 100644
index 00000000..daebd5c6
--- /dev/null
+++ b/tests/cli/validate-unknown-features.wat
@@ -0,0 +1,4 @@
+;; FAIL: validate --features=unknown %
+
+(module)
+
diff --git a/tests/cli/validate-unknown-features.wat.stderr b/tests/cli/validate-unknown-features.wat.stderr
new file mode 100644
index 00000000..3a72311f
--- /dev/null
+++ b/tests/cli/validate-unknown-features.wat.stderr
@@ -0,0 +1,4 @@
+error: invalid value 'unknown' for '--features <FEATURES>': unknown feature `unknown`
+Valid features: mutable-global, saturating-float-to-int, sign-extension, reference-types, multi-value, bulk-memory, simd, relaxed-simd, threads, shared-everything-threads, tail-call, floats, multi-memory, exceptions, memory64, extended-const, component-model, function-references, memory-control, gc, custom-page-sizes, component-model-values, component-model-nested-names, component-model-more-flags, legacy-exceptions
+
+For more information, try '--help'.
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))
+)