Repository: bytecodealliance/wasm-tools
Pull request #1350 (2024-01-03 18:19:08)

Use `bitflags` for symbol flags
Follow-up on https://github.com/bytecodealliance/wasm-tools/pull/1348#issuecomment-1874687578.


Patch:
diff --git a/Cargo.lock b/Cargo.lock
index 3538d9a0..c7d7d412 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -1925,6 +1925,7 @@ name = "wasmparser"
 version = "0.118.1"
 dependencies = [
  "anyhow",
+ "bitflags 2.4.1",
  "criterion",
  "env_logger",
  "indexmap 2.1.0",
diff --git a/crates/wasmparser/Cargo.toml b/crates/wasmparser/Cargo.toml
index 7410a2af..00031a80 100644
--- a/crates/wasmparser/Cargo.toml
+++ b/crates/wasmparser/Cargo.toml
@@ -13,6 +13,7 @@ edition.workspace = true
 exclude = ["benches/*.wasm"]
 
 [dependencies]
+bitflags = "2.4.1"
 indexmap = { workspace = true }
 semver = { workspace = true }
 
diff --git a/crates/wasmparser/src/readers/core/dylink0.rs b/crates/wasmparser/src/readers/core/dylink0.rs
index 3bdda39f..671022ea 100644
--- a/crates/wasmparser/src/readers/core/dylink0.rs
+++ b/crates/wasmparser/src/readers/core/dylink0.rs
@@ -1,4 +1,4 @@
-use crate::{BinaryReader, Result, Subsection, Subsections};
+use crate::{BinaryReader, Result, Subsection, Subsections, SymbolFlags};
 use std::ops::Range;
 
 /// Parser for the dynamic linking `dylink.0` custom section.
@@ -36,11 +36,7 @@ pub struct MemInfo {
 #[derive(Debug)]
 pub struct ExportInfo<'a> {
     pub name: &'a str,
-
-    /// Note that these flags correspond to those described in
-    /// <https://github.com/WebAssembly/tool-conventions/blob/main/Linking.md>
-    /// with the `WASM_SYM_*` prefix.
-    pub flags: u32,
+    pub flags: SymbolFlags,
 }
 
 #[allow(missing_docs)]
@@ -48,11 +44,7 @@ pub struct ExportInfo<'a> {
 pub struct ImportInfo<'a> {
     pub module: &'a str,
     pub field: &'a str,
-
-    /// Note that these flags correspond to those described in
-    /// <https://github.com/WebAssembly/tool-conventions/blob/main/Linking.md>
-    /// with the `WASM_SYM_*` prefix.
-    pub flags: u32,
+    pub flags: SymbolFlags,
 }
 
 /// Possible subsections of the `dylink.0` custom section.
@@ -91,7 +83,7 @@ impl<'a> Subsection<'a> for Dylink0Subsection<'a> {
                     .map(|_| {
                         Ok(ExportInfo {
                             name: reader.read_string()?,
-                            flags: reader.read_var_u32()?,
+                            flags: reader.read()?,
                         })
                     })
                     .collect::<Result<_, _>>()?,
@@ -102,7 +94,7 @@ impl<'a> Subsection<'a> for Dylink0Subsection<'a> {
                         Ok(ImportInfo {
                             module: reader.read_string()?,
                             field: reader.read_string()?,
-                            flags: reader.read_var_u32()?,
+                            flags: reader.read()?,
                         })
                     })
                     .collect::<Result<_, _>>()?,
diff --git a/crates/wasmparser/src/readers/core/linking.rs b/crates/wasmparser/src/readers/core/linking.rs
index e5a83b3a..f70611e9 100644
--- a/crates/wasmparser/src/readers/core/linking.rs
+++ b/crates/wasmparser/src/readers/core/linking.rs
@@ -3,20 +3,64 @@ use crate::{
 };
 use std::ops::Range;
 
-/// A flag indicating that this is a weak symbol.
-pub const WASM_SYM_BINDING_WEAK: u32 = 1 << 0;
-/// A flag indicating that this is a local symbol (this is exclusive with [WASM_SYM_BINDING_WEAK]).
-pub const WASM_SYM_BINDING_LOCAL: u32 = 1 << 1;
-/// A flag indicating that this is a hidden symbol.
-pub const WASM_SYM_VISIBILITY_HIDDEN: u32 = 1 << 2;
-/// A flag indicating that this symbol is not defined.
-pub const WASM_SYM_UNDEFINED: u32 = 1 << 4;
-/// A flag indiciating that this symbol is intended to be exported from the wasm module to the host environment
-pub const WASM_SYM_EXPORTED: u32 = 1 << 5;
-/// A flag indiciating that this symbol uses an explicit symbol name, rather than reusing the name from a wasm import.
-pub const WASM_SYM_EXPLICIT_NAME: u32 = 1 << 6;
-/// A flag indicating that this symbol is intended to be included in the linker output, regardless of whether it is used by the program.
-pub const WASM_SYM_NO_STRIP: u32 = 1 << 7;
+bitflags::bitflags! {
+    /// Flags for WebAssembly symbols.
+    ///
+    /// These flags correspond to those described in
+    /// <https://github.com/WebAssembly/tool-conventions/blob/main/Linking.md>
+    /// with the `WASM_SYM_*` prefix.
+    #[repr(transparent)]
+    #[derive(Debug, Clone, Copy, Default, PartialEq, Eq, PartialOrd, Ord, Hash)]
+    pub struct SymbolFlags: u32 {
+        /* N.B.:
+            Newly added flags should be keep in sync with `print_dylink0_flags`
+            in `crates/wasmprinter/src/lib.rs`.
+        */
+        /// This is a weak symbol.
+        const BINDING_WEAK = 1 << 0;
+        /// This is a local symbol (this is exclusive with [BINDING_WEAK]).
+        const BINDING_LOCAL = 1 << 1;
+        /// This is a hidden symbol.
+        const VISIBILITY_HIDDEN = 1 << 2;
+        /// This symbol is not defined.
+        const UNDEFINED = 1 << 4;
+        /// This symbol is intended to be exported from the wasm module to the host environment.
+        const EXPORTED = 1 << 5;
+        /// This symbol uses an explicit symbol name, rather than reusing the name from a wasm import.
+        const EXPLICIT_NAME = 1 << 6;
+        /// This symbol is intended to be included in the linker output, regardless of whether it is used by the program.
+        const NO_STRIP = 1 << 7;
+        /// This symbol resides in thread local storage.
+        const TLS = 1 << 8;
+        /// This symbol represents an absolute address.
+        const ABSOLUTE = 1 << 9;
+    }
+
+    /// Flags for WebAssembly segments.
+    ///
+    /// These flags are defined by implementation at the time of writing:
+    /// <https://github.com/llvm/llvm-project/blob/llvmorg-17.0.6/llvm/include/llvm/BinaryFormat/Wasm.h#L391-L394>
+    #[repr(transparent)]
+    #[derive(Debug, Clone, Copy, Default, PartialEq, Eq, Hash)]
+    pub struct SegmentFlags: u32 {
+        /// The segment contains only null-terminated strings, which allows the linker to perform merging.
+        const STRINGS = 0x1;
+        /// The segment contains thread-local data.
+        const TLS = 0x2;
+    }
+}
+
+impl<'a> FromReader<'a> for SymbolFlags {
+    fn from_reader(reader: &mut BinaryReader<'a>) -> Result<Self> {
+        Ok(Self::from_bits_retain(reader.read_var_u32()?))
+    }
+}
+
+impl<'a> FromReader<'a> for SegmentFlags {
+    fn from_reader(reader: &mut BinaryReader<'a>) -> Result<Self> {
+        Ok(Self::from_bits_retain(reader.read_var_u32()?))
+    }
+}
 
 /// A reader for the `linking` custom section of a WebAssembly module.
 ///
@@ -43,14 +87,14 @@ pub struct Segment<'a> {
     /// The required alignment of the segment, encoded as a power of 2.
     pub alignment: u32,
     /// The flags for the segment.
-    pub flags: u32,
+    pub flags: SegmentFlags,
 }
 
 impl<'a> FromReader<'a> for Segment<'a> {
     fn from_reader(reader: &mut BinaryReader<'a>) -> Result<Self> {
         let name = reader.read_string()?;
         let alignment = reader.read_var_u32()?;
-        let flags = reader.read_var_u32()?;
+        let flags = reader.read()?;
         Ok(Self {
             name,
             alignment,
@@ -175,7 +219,7 @@ pub enum SymbolInfo<'a> {
     /// The symbol is a function.
     Func {
         /// The flags for the symbol.
-        flags: u32,
+        flags: SymbolFlags,
         /// The index of the function corresponding to this symbol.
         index: u32,
         /// The name for the function, if it is defined or uses an explicit name.
@@ -184,7 +228,7 @@ pub enum SymbolInfo<'a> {
     /// The symbol is a data symbol.
     Data {
         /// The flags for the symbol.
-        flags: u32,
+        flags: SymbolFlags,
         /// The name for the symbol.
         name: &'a str,
         /// The definition of the data symbol, if it is defined.
@@ -193,7 +237,7 @@ pub enum SymbolInfo<'a> {
     /// The symbol is a global.
     Global {
         /// The flags for the symbol.
-        flags: u32,
+        flags: SymbolFlags,
         /// The index of the global corresponding to this symbol.
         index: u32,
         /// The name for the global, if it is defined or uses an explicit name.
@@ -202,14 +246,14 @@ pub enum SymbolInfo<'a> {
     /// The symbol is a section.
     Section {
         /// The flags for the symbol.
-        flags: u32,
+        flags: SymbolFlags,
         /// The index of the function corresponding to this symbol.
         section: u32,
     },
     /// The symbol is an event.
     Event {
         /// The flags for the symbol.
-        flags: u32,
+        flags: SymbolFlags,
         /// The index of the event corresponding to this symbol.
         index: u32,
         /// The name for the event, if it is defined or uses an explicit name.
@@ -218,7 +262,7 @@ pub enum SymbolInfo<'a> {
     /// The symbol is a table.
     Table {
         /// The flags for the symbol.
-        flags: u32,
+        flags: SymbolFlags,
         /// The index of the table corresponding to this symbol.
         index: u32,
         /// The name for the table, if it is defined or uses an explicit name.
@@ -230,10 +274,10 @@ impl<'a> FromReader<'a> for SymbolInfo<'a> {
     fn from_reader(reader: &mut BinaryReader<'a>) -> Result<Self> {
         let offset = reader.original_position();
         let kind = reader.read_u8()?;
-        let flags = reader.read_var_u32()?;
+        let flags: SymbolFlags = reader.read()?;
 
-        let defined = (flags & WASM_SYM_UNDEFINED) == 0;
-        let explicit_name = (flags & WASM_SYM_EXPLICIT_NAME) != 0;
+        let defined = !flags.contains(SymbolFlags::UNDEFINED);
+        let explicit_name = flags.contains(SymbolFlags::EXPLICIT_NAME);
 
         const SYMTAB_FUNCTION: u8 = 0;
         const SYMTAB_DATA: u8 = 1;
@@ -283,7 +327,7 @@ impl<'a> FromReader<'a> for SymbolInfo<'a> {
     }
 }
 
-/// Represents a data symbol.
+/// Represents the metadata about a data symbol defined in the wasm file.
 #[derive(Debug, Copy, Clone)]
 pub struct DefinedDataSymbol {
     /// The index of the data segment.
diff --git a/crates/wasmprinter/src/lib.rs b/crates/wasmprinter/src/lib.rs
index 67034637..e011f842 100644
--- a/crates/wasmprinter/src/lib.rs
+++ b/crates/wasmprinter/src/lib.rs
@@ -2657,25 +2657,28 @@ impl Printer {
         Ok(())
     }
 
-    fn print_dylink0_flags(&mut self, mut flags: u32) -> Result<()> {
+    fn print_dylink0_flags(&mut self, mut flags: SymbolFlags) -> Result<()> {
         macro_rules! print_flag {
             ($($name:ident = $text:tt)*) => ({$(
-                if flags & wasmparser::$name != 0 {
-                    flags &= !wasmparser::$name;
+                if flags.contains(SymbolFlags::$name) {
+                    flags.remove(SymbolFlags::$name);
                     self.result.push_str(concat!(" ", $text));
                 }
             )*})
         }
+        // N.B.: Keep in sync with `parse_sym_flags` in `crates/wast/src/core/custom.rs`.
         print_flag! {
-            WASM_SYM_BINDING_WEAK = "binding-weak"
-            WASM_SYM_BINDING_LOCAL = "binding-local"
-            WASM_SYM_VISIBILITY_HIDDEN = "visibility-hidden"
-            WASM_SYM_UNDEFINED = "undefined"
-            WASM_SYM_EXPORTED = "exported"
-            WASM_SYM_EXPLICIT_NAME = "explicit-name"
-            WASM_SYM_NO_STRIP = "no-strip"
-        }
-        if flags != 0 {
+            BINDING_WEAK = "binding-weak"
+            BINDING_LOCAL = "binding-local"
+            VISIBILITY_HIDDEN = "visibility-hidden"
+            UNDEFINED = "undefined"
+            EXPORTED = "exported"
+            EXPLICIT_NAME = "explicit-name"
+            NO_STRIP = "no-strip"
+            TLS = "tls"
+            ABSOLUTE = "absolute"
+        }
+        if !flags.is_empty() {
             write!(self.result, " {:#x}", flags)?;
         }
         Ok(())
diff --git a/crates/wast/src/core/custom.rs b/crates/wast/src/core/custom.rs
index 4fe6c7c2..2205bf51 100644
--- a/crates/wast/src/core/custom.rs
+++ b/crates/wast/src/core/custom.rs
@@ -360,6 +360,7 @@ fn parse_sym_flags(parser: Parser<'_>) -> Result<u32> {
                 }
             })*};
         }
+        // N.B.: Keep in sync with `print_dylink0_flags` in `crates/wasmprinter/src/lib.rs`.
         parse_flags! {
             "binding-weak" = 1 << 0,
             "binding-local" = 1 << 1,
@@ -368,6 +369,8 @@ fn parse_sym_flags(parser: Parser<'_>) -> Result<u32> {
             "exported" = 1 << 5,
             "explicit-name" = 1 << 6,
             "no-strip" = 1 << 7,
+            "tls" = 1 << 8,
+            "absolute" = 1 << 9,
         }
         return Err(l.error());
     }
diff --git a/crates/wit-component/src/linking.rs b/crates/wit-component/src/linking.rs
index 56adb738..240197ca 100644
--- a/crates/wit-component/src/linking.rs
+++ b/crates/wit-component/src/linking.rs
@@ -39,7 +39,7 @@ use {
         Instruction as Ins, MemArg, MemorySection, MemoryType, Module, RawCustomSection, RefType,
         StartSection, TableSection, TableType, TypeSection, ValType,
     },
-    wasmparser::WASM_SYM_BINDING_WEAK,
+    wasmparser::SymbolFlags,
 };
 
 mod metadata;
@@ -892,7 +892,7 @@ fn resolve_symbols<'a>(
                             mutable: false,
                         }),
                     },
-                    flags: 0,
+                    flags: SymbolFlags::empty(),
                 },
             );
         }
@@ -924,7 +924,7 @@ fn resolve_symbols<'a>(
                                 results: Vec::new(),
                             }),
                         },
-                        flags: 0,
+                        flags: SymbolFlags::empty(),
                     },
                 ));
             }
@@ -1321,7 +1321,7 @@ impl Linker {
                 && (self.stub_missing_functions
                     || missing
                         .iter()
-                        .all(|(_, export)| 0 != (export.flags & WASM_SYM_BINDING_WEAK)))
+                        .all(|(_, export)| export.flags.contains(SymbolFlags::BINDING_WEAK)))
             {
                 self.stub_missing_functions = false;
                 self.libraries.push((
@@ -1335,7 +1335,7 @@ impl Linker {
                     "unresolved symbol(s):\n{}",
                     missing
                         .iter()
-                        .filter(|(_, export)| 0 == (export.flags & WASM_SYM_BINDING_WEAK))
+                        .filter(|(_, export)| !export.flags.contains(SymbolFlags::BINDING_WEAK))
                         .map(|(importer, export)| { format!("\t{importer} needs {}", export.key) })
                         .collect::<Vec<_>>()
                         .join("\n")
diff --git a/crates/wit-component/src/linking/metadata.rs b/crates/wit-component/src/linking/metadata.rs
index 725d27f8..954f3355 100644
--- a/crates/wit-component/src/linking/metadata.rs
+++ b/crates/wit-component/src/linking/metadata.rs
@@ -9,7 +9,7 @@ use {
     },
     wasmparser::{
         BinaryReader, BinaryReaderError, ExternalKind, FuncType, Parser, Payload, RefType,
-        Subsection, Subsections, TableType, TypeRef, ValType,
+        Subsection, Subsections, SymbolFlags, TableType, TypeRef, ValType,
     },
 };
 
@@ -131,7 +131,7 @@ pub struct Import<'a> {
     pub module: &'a str,
     pub name: &'a str,
     pub ty: Type,
-    pub flags: u32,
+    pub flags: SymbolFlags,
 }
 
 /// Represents a core Wasm export
@@ -151,7 +151,7 @@ impl<'a> fmt::Display for ExportKey<'a> {
 #[derive(Debug, Clone, PartialEq, Eq, Hash, PartialOrd, Ord)]
 pub struct Export<'a> {
     pub key: ExportKey<'a>,
-    pub flags: u32,
+    pub flags: SymbolFlags,
 }
 
 /// Represents a `WASM_DYLINK_MEM_INFO` value
@@ -212,7 +212,7 @@ pub struct Metadata<'a> {
     pub has_component_exports: bool,
 
     /// The functions imported from the `env` module, if any
-    pub env_imports: BTreeSet<(&'a str, (FunctionType, u32))>,
+    pub env_imports: BTreeSet<(&'a str, (FunctionType, SymbolFlags))>,
 
     /// The memory addresses imported from `GOT.mem`, if any
     pub memory_address_imports: BTreeSet<&'a str>,
@@ -230,14 +230,14 @@ pub struct Metadata<'a> {
 #[derive(Debug)]
 struct ExportInfo<'a> {
     name: &'a str,
-    flags: u32,
+    flags: SymbolFlags,
 }
 
 #[derive(Debug)]
 struct ImportInfo<'a> {
     module: &'a str,
     field: &'a str,
-    flags: u32,
+    flags: SymbolFlags,
 }
 
 #[derive(Debug)]
@@ -270,7 +270,7 @@ impl<'a> Subsection<'a> for DylinkSubsection<'a> {
                     .map(|_| {
                         Ok(ExportInfo {
                             name: reader.read_string()?,
-                            flags: reader.read_var_u32()?,
+                            flags: reader.read()?,
                         })
                     })
                     .collect::<Result<_, _>>()?,
@@ -281,7 +281,7 @@ impl<'a> Subsection<'a> for DylinkSubsection<'a> {
                         Ok(ImportInfo {
                             module: reader.read_string()?,
                             field: reader.read_string()?,
-                            flags: reader.read_var_u32()?,
+                            flags: reader.read()?,
                         })
                     })
                     .collect::<Result<_, _>>()?,
@@ -413,7 +413,10 @@ impl<'a> Metadata<'a> {
                                             FunctionType::try_from(
                                                 &types[usize::try_from(ty).unwrap()],
                                             )?,
-                                            *import_info.get(&("env", name)).unwrap_or(&0),
+                                            import_info
+                                                .get(&("env", name))
+                                                .copied()
+                                                .unwrap_or_default(),
                                         ),
                                     ));
                                 } else {
@@ -463,7 +466,10 @@ impl<'a> Metadata<'a> {
                                         bail!("unsupported import kind for {module}.{name}: {ty:?}",)
                                     }
                                 };
-                                let flags = *import_info.get(&(module, name)).unwrap_or(&0);
+                                let flags = import_info
+                                    .get(&(module, name))
+                                    .copied()
+                                    .unwrap_or_default();
                                 result.imports.insert(Import {
                                     module,
                                     name,
@@ -523,7 +529,8 @@ impl<'a> Metadata<'a> {
                                         )
                                     }
                                 };
-                                let flags = *export_info.get(&export.name).unwrap_or(&0);
+                                let flags =
+                                    export_info.get(&export.name).copied().unwrap_or_default();
                                 result.exports.insert(Export {
                                     key: ExportKey {
                                         name: export.name,


Test patch:
diff --git a/tests/snapshots/local/dylink0.wast/8.print b/tests/snapshots/local/dylink0.wast/8.print
index c11f43cb..88cadfad 100644
--- a/tests/snapshots/local/dylink0.wast/8.print
+++ b/tests/snapshots/local/dylink0.wast/8.print
@@ -1,6 +1,6 @@
 (module
   (@dylink.0
-    (export-info "a" binding-weak binding-local visibility-hidden undefined exported explicit-name no-strip 0xffffff08)
+    (export-info "a" binding-weak binding-local visibility-hidden undefined exported explicit-name no-strip tls absolute 0xfffffc08)
     (export-info "a" binding-local)
     (export-info "a" binding-weak binding-local undefined)
   )