Print custom sections by default in wasmprinter (#1449)

* Print custom sections by default in `wasmprinter`

The annotations proposal for WebAssembly added `@custom` syntax to the
text format of WebAssembly and while `wast` has supported parsing it for
quite some time now the `wasmprinter` half has not printed it. This was
mostly just for historical reasons, but this decision ends up causing
confusion for folks when they round-trip a binary through the text
format and are surprised when the result is different. An example of
this is that the `component-type*` custom sections for components are
lost when roundtripping and that can break the componentization process.

This commit updates to instead print these sections by default. That
does mean that many modules coming out of compilers may have lots of
DWARF information now printed, but this feels like a better default than
always ignoring them entirely.

It's worth noting here that `wasmprinter` and `wast` try to round-trip
the structure of a module but won't ever round trip a module
byte-for-byte. For example overlong LEB encodings will never be
reproduced. This means that while this commit will roundtrip custom
sections it won't guarantee that the custom section continues to work
because the offsets within the module when converted back to binary may
still be different.

* Fix some mutation tests to work on 32 and 64-bit

* Normalize WIT comments to `\n` from `\r\n`

Author: alexcrichton

crates/wasm-mutate/src/info.rs

@@ -317,18 +317,12 @@ impl<'a> ModuleInfo<'a> {
         assert!(src_idx < self.raw_sections.len());
         assert!(dest_idx < self.raw_sections.len());
         assert_ne!(src_idx, dest_idx);
+        let mut sections = self.raw_sections.clone();
+        sections.swap(src_idx, dest_idx);
         let mut module = wasm_encoder::Module::new();
-        self.raw_sections.iter().enumerate().for_each(|(i, s)| {
-            if src_idx < dest_idx && i == dest_idx {
-                module.section(&self.raw_sections[src_idx]);
-            }
-            if i != src_idx {
-                module.section(s);
-            }
-            if dest_idx < src_idx && i == dest_idx {
-                module.section(&self.raw_sections[src_idx]);
-            }
-        });
+        for section in sections {
+            module.section(&section);
+        }
         module
     }
 

crates/wasm-mutate/src/mutators/custom.rs

@@ -162,7 +162,7 @@ mod tests {
             AddCustomSectionMutator,
             r#"
                 (module
-                    (@custom "a" "b")
+                    (@custom "" "")
                 )
             "#,
         );
@@ -224,13 +224,13 @@ mod tests {
         crate::mutators::match_mutation(
             r#"
                 (module
-                    (@custom "name" "data")
+                    (@custom "custom-name" "data")
                 )
             "#,
             CustomSectionMutator,
             r#"
                 (module
-                    (@custom "n" "data")
+                    (@custom "custom-nam" "data")
                 )
             "#,
         );
@@ -241,15 +241,15 @@ mod tests {
         crate::mutators::match_mutation(
             r#"
             (module
-                (@custom "name" "data")
                 (@custom "name2" "data")
+                (@custom "name3" "data")
             )
             "#,
             ReorderCustomSectionMutator,
             r#"
             (module
+                (@custom "name3" "data")
                 (@custom "name2" "data")
-                (@custom "name" "data")
             )
             "#,
         )

crates/wasmprinter/src/lib.rs

@@ -124,6 +124,7 @@ struct State {
     name: Option<Naming>,
     core: CoreState,
     component: ComponentState,
+    custom_section_place: Option<&'static str>,
 }
 
 impl State {
@@ -133,6 +134,7 @@ impl State {
             name: None,
             core: CoreState::default(),
             component: ComponentState::default(),
+            custom_section_place: None,
         }
     }
 }
@@ -351,6 +353,7 @@ impl Printer {
                     match encoding {
                         Encoding::Module => {
                             states.push(State::new(Encoding::Module));
+                            states.last_mut().unwrap().custom_section_place = Some("before first");
                             if states.len() > 1 {
                                 self.start_group("core module");
                             } else {
@@ -414,7 +417,8 @@ impl Printer {
                         continue;
                     }
                     let cur = self.result.len();
-                    let err = match self.print_custom_section(c.clone()) {
+                    let state = states.last().unwrap();
+                    let err = match self.print_custom_section(state, c.clone()) {
                         Ok(()) => continue,
                         Err(e) => e,
                     };
@@ -429,13 +433,19 @@ impl Printer {
                         self.result.push_str(line);
                     }
                     self.newline(c.range().end);
+                    self.print_raw_custom_section(state, c)?;
+                }
+                Payload::TypeSection(s) => {
+                    self.update_custom_section_place(&mut states, "after type");
+                    self.print_types(states.last_mut().unwrap(), s)?;
                 }
-                Payload::TypeSection(s) => self.print_types(states.last_mut().unwrap(), s)?,
                 Payload::ImportSection(s) => {
+                    self.update_custom_section_place(&mut states, "after import");
                     Self::ensure_module(&states)?;
                     self.print_imports(states.last_mut().unwrap(), s)?
                 }
                 Payload::FunctionSection(reader) => {
+                    self.update_custom_section_place(&mut states, "after func");
                     Self::ensure_module(&states)?;
                     if mem::replace(&mut code_printed, true) {
                         bail!("function section appeared twice in module");
@@ -446,39 +456,47 @@ impl Printer {
                     self.print_code(states.last_mut().unwrap(), &code, reader)?;
                 }
                 Payload::TableSection(s) => {
+                    self.update_custom_section_place(&mut states, "after table");
                     Self::ensure_module(&states)?;
                     self.print_tables(states.last_mut().unwrap(), s)?
                 }
                 Payload::MemorySection(s) => {
+                    self.update_custom_section_place(&mut states, "after memory");
                     Self::ensure_module(&states)?;
                     self.print_memories(states.last_mut().unwrap(), s)?
                 }
                 Payload::TagSection(s) => {
+                    self.update_custom_section_place(&mut states, "after tag");
                     Self::ensure_module(&states)?;
                     self.print_tags(states.last_mut().unwrap(), s)?
                 }
                 Payload::GlobalSection(s) => {
+                    self.update_custom_section_place(&mut states, "after global");
                     Self::ensure_module(&states)?;
                     self.print_globals(states.last_mut().unwrap(), s)?
                 }
                 Payload::ExportSection(s) => {
+                    self.update_custom_section_place(&mut states, "after export");
                     Self::ensure_module(&states)?;
                     self.print_exports(states.last().unwrap(), s)?
                 }
                 Payload::StartSection { func, range } => {
+                    self.update_custom_section_place(&mut states, "after start");
                     Self::ensure_module(&states)?;
                     self.newline(range.start);
                     self.start_group("start ");
                     self.print_idx(&states.last().unwrap().core.func_names, func)?;
                     self.end_group();
                 }
                 Payload::ElementSection(s) => {
+                    self.update_custom_section_place(&mut states, "after element");
                     Self::ensure_module(&states)?;
                     self.print_elems(states.last_mut().unwrap(), s)?;
                 }
                 // printed with the `Function` section, so we
                 // skip this section
                 Payload::CodeSectionStart { size, .. } => {
+                    self.update_custom_section_place(&mut states, "after code");
                     Self::ensure_module(&states)?;
                     bytes = &bytes[size as usize..];
                     parser.skip_section();
@@ -489,6 +507,7 @@ impl Printer {
                     // not part of the text format
                 }
                 Payload::DataSection(s) => {
+                    self.update_custom_section_place(&mut states, "after data");
                     Self::ensure_module(&states)?;
                     self.print_data(states.last_mut().unwrap(), s)?;
                 }
@@ -577,6 +596,14 @@ impl Printer {
         Ok(())
     }
 
+    fn update_custom_section_place(&self, states: &mut Vec<State>, place: &'static str) {
+        if let Some(last) = states.last_mut() {
+            if let Some(prev) = &mut last.custom_section_place {
+                *prev = place;
+            }
+        }
+    }
+
     fn start_group(&mut self, name: &str) {
         self.result.push('(');
         self.result.push_str(name);
@@ -2661,8 +2688,15 @@ impl Printer {
         self.result.push(to_hex(byte & 0xf));
     }
 
-    fn print_custom_section(&mut self, section: CustomSectionReader<'_>) -> Result<()> {
+    fn print_custom_section(
+        &mut self,
+        state: &State,
+        section: CustomSectionReader<'_>,
+    ) -> Result<()> {
         match section.name() {
+            // For now `wasmprinter` has invented syntax for `producers` and
+            // `dylink.0` below to use in tests. Note that this syntax is not
+            // official at this time.
             "producers" => {
                 self.newline(section.range().start);
                 self.print_producers_section(ProducersSectionReader::new(
@@ -2677,10 +2711,35 @@ impl Printer {
                     section.data_offset(),
                 ))
             }
-            _ => Ok(()),
+
+            // These are parsed during `read_names_and_code` and are part of
+            // printing elsewhere, so don't print them.
+            "name" | "component-name" | "metadata.code.branch_hint" => Ok(()),
+
+            // Unknown custom sections get a `@custom` annotation printed.
+            _ => self.print_raw_custom_section(state, section),
         }
     }
 
+    fn print_raw_custom_section(
+        &mut self,
+        state: &State,
+        section: CustomSectionReader<'_>,
+    ) -> Result<()> {
+        self.newline(section.range().start);
+        self.start_group("@custom ");
+        self.print_str(section.name())?;
+        if let Some(place) = state.custom_section_place {
+            self.result.push_str(" (");
+            self.result.push_str(place);
+            self.result.push_str(")");
+        }
+        self.result.push_str(" ");
+        self.print_bytes(section.data())?;
+        self.end_group();
+        Ok(())
+    }
+
     fn print_producers_section(&mut self, section: ProducersSectionReader<'_>) -> Result<()> {
         self.start_group("@producers");
         for field in section {

crates/wit-component/src/encoding.rs

@@ -2127,7 +2127,7 @@ impl ComponentEncoder {
     }
 }
 
-#[cfg(test)]
+#[cfg(all(test, feature = "dummy-module"))]
 mod test {
     use crate::{dummy_module, embed_component_metadata};
 

crates/wit-component/tests/interfaces/console.wat

@@ -11,6 +11,7 @@
     )
   )
   (export (;1;) "console" (type 0))
+  (@custom "package-docs" "\00{}")
   (@producers
     (processed-by "wit-component" "$CARGO_PKG_VERSION")
   )

crates/wit-component/tests/interfaces/diamond-disambiguate.wat

@@ -63,6 +63,7 @@
     )
   )
   (export (;5;) "w1" (type 4))
+  (@custom "package-docs" "\00{}")
   (@producers
     (processed-by "wit-component" "$CARGO_PKG_VERSION")
   )

crates/wit-component/tests/interfaces/diamond.wat

@@ -100,6 +100,7 @@
     )
   )
   (export (;7;) "w3" (type 6))
+  (@custom "package-docs" "\00{}")
   (@producers
     (processed-by "wit-component" "$CARGO_PKG_VERSION")
   )

crates/wit-component/tests/interfaces/doc-comments.wat

@@ -66,6 +66,7 @@
     )
   )
   (export (;5;) "coverage-world" (type 4))
+  (@custom "package-docs" "\00{\22docs\22:\22package docs;\22,\22worlds\22:{\22coverage-world\22:{\22docs\22:\22world docs\22,\22interfaces\22:{\22i\22:{\22docs\22:\22world inline interface docs\22,\22funcs\22:{\22f\22:\22inline interface func docs\22},\22types\22:{\22t\22:{\22docs\22:\22inline interface typedef docs\22}}}},\22types\22:{\22t\22:{\22docs\22:\22world typedef docs\22}},\22funcs\22:{\22imp\22:\22world func import docs\22,\22exp\22:\22world func export docs\22}}},\22interfaces\22:{\22coverage-iface\22:{\22docs\22:\22interface docs\22,\22funcs\22:{\22[constructor]res\22:\22constructor docs\22,\22[method]res.m\22:\22method docs\22,\22[static]res.s\22:\22static func docs\22,\22f\22:\22interface func docs\22},\22types\22:{\22t\22:{\22docs\22:\22basic typedef docs\22},\22r\22:{\22docs\22:\22record typedef docs\22,\22items\22:{\22f1\22:\22record field docs\22}},\22fl\22:{\22items\22:{\22f1\22:\22flag docs\22}},\22v\22:{\22items\22:{\22c1\22:\22variant case docs\22}},\22e\22:{\22items\22:{\22c1\22:\22enum case docs\22}}}},\22other-comment-forms\22:{\22docs\22:\22other comment forms\5cn    multi-line block\22,\22funcs\22:{\22multiple-lines-split\22:\22one doc line\5cnnon-doc in the middle\5cnanother doc line\22,\22mixed-forms\22:\22mixed forms; line doc\5cnplus block doc\5cn      multi-line\22}}}}")
   (@producers
     (processed-by "wit-component" "$CARGO_PKG_VERSION")
   )

crates/wit-component/tests/interfaces/empty.wat

@@ -43,6 +43,7 @@
     )
   )
   (export (;5;) "actually-empty-world" (type 4))
+  (@custom "package-docs" "\00{}")
   (@producers
     (processed-by "wit-component" "$CARGO_PKG_VERSION")
   )

crates/wit-component/tests/interfaces/export-other-packages-interface.wat

@@ -16,6 +16,7 @@
     )
   )
   (export (;1;) "foo" (type 0))
+  (@custom "package-docs" "\00{}")
   (@producers
     (processed-by "wit-component" "$CARGO_PKG_VERSION")
   )