Don't re-parse wasm for debuginfo (#2085)

* Don't re-parse wasm for debuginfo

This commit updates debuginfo parsing to happen during the main
translation of the original wasm module. This avoid re-parsing the wasm
module twice (at least the section-level headers). Additionally this
ties debuginfo directly to a `ModuleTranslation` which makes it easier
to process debuginfo for nested modules in the upcoming module linking
proposal.

The changes here are summarized by taking the `read_debuginfo` function
and merging it with the main module translation that happens which is
driven by cranelift. Some new hooks were added to the module environment
trait to support this, but most of it was integrating with existing hooks.

* Fix tests in debug crate

crates/debug/src/transform/address_transform.rs

@@ -1,4 +1,3 @@
-use crate::WasmFileInfo;
 use gimli::write;
 use more_asserts::assert_le;
 use std::collections::BTreeMap;
@@ -6,6 +5,7 @@ use std::iter::FromIterator;
 use wasmtime_environ::entity::{EntityRef, PrimaryMap};
 use wasmtime_environ::ir::SourceLoc;
 use wasmtime_environ::wasm::DefinedFuncIndex;
+use wasmtime_environ::WasmFileInfo;
 use wasmtime_environ::{FunctionAddressMap, ModuleAddressMap};
 
 pub type GeneratedAddress = usize;
@@ -602,11 +602,11 @@ impl AddressTransform {
 #[cfg(test)]
 mod tests {
     use super::{build_function_lookup, get_wasm_code_offset, AddressTransform};
-    use crate::read_debuginfo::WasmFileInfo;
     use gimli::write::Address;
     use std::iter::FromIterator;
     use wasmtime_environ::entity::PrimaryMap;
     use wasmtime_environ::ir::SourceLoc;
+    use wasmtime_environ::WasmFileInfo;
     use wasmtime_environ::{FunctionAddressMap, InstructionAddressMap, ModuleAddressMap};
 
     #[test]
@@ -724,7 +724,7 @@ mod tests {
                 path: None,
                 code_section_offset: 1,
                 imported_func_count: 0,
-                funcs: Box::new([]),
+                funcs: Vec::new(),
             },
         );
 

crates/debug/src/transform/expression.rs

@@ -683,9 +683,9 @@ mod tests {
     }
 
     fn create_mock_address_transform() -> AddressTransform {
-        use crate::read_debuginfo::WasmFileInfo;
         use wasmtime_environ::entity::PrimaryMap;
         use wasmtime_environ::ir::SourceLoc;
+        use wasmtime_environ::WasmFileInfo;
         use wasmtime_environ::{FunctionAddressMap, InstructionAddressMap};
         let mut module_map = PrimaryMap::new();
         let code_section_offset: u32 = 100;
@@ -709,7 +709,7 @@ mod tests {
         });
         let fi = WasmFileInfo {
             code_section_offset: code_section_offset.into(),
-            funcs: Box::new([]),
+            funcs: Vec::new(),
             imported_func_count: 0,
             path: None,
         };

crates/debug/src/transform/mod.rs

@@ -2,7 +2,6 @@ use self::refs::DebugInfoRefsMap;
 use self::simulate::generate_simulated_dwarf;
 use self::unit::clone_unit;
 use crate::gc::build_dependencies;
-use crate::DebugInfoData;
 use anyhow::Error;
 use gimli::{
     write, DebugAddr, DebugLine, DebugLineStr, DebugStr, DebugStrOffsets, LocationLists,
@@ -11,6 +10,7 @@ use gimli::{
 use std::collections::HashSet;
 use thiserror::Error;
 use wasmtime_environ::isa::TargetIsa;
+use wasmtime_environ::DebugInfoData;
 use wasmtime_environ::{ModuleAddressMap, ModuleVmctxInfo, ValueLabelsRanges};
 
 pub use address_transform::AddressTransform;

crates/debug/src/transform/simulate.rs

@@ -1,18 +1,19 @@
 use super::expression::{CompiledExpression, FunctionFrameInfo};
 use super::utils::{add_internal_types, append_vmctx_info, get_function_frame_info};
 use super::AddressTransform;
-use crate::read_debuginfo::WasmFileInfo;
 use anyhow::{Context, Error};
 use gimli::write;
 use gimli::{self, LineEncoding};
 use std::collections::{HashMap, HashSet};
 use std::path::PathBuf;
+use std::sync::atomic::{AtomicUsize, Ordering::SeqCst};
+use wasmparser::Type as WasmType;
 use wasmtime_environ::entity::EntityRef;
-use wasmtime_environ::wasm::{get_vmctx_value_label, DefinedFuncIndex};
-use wasmtime_environ::{ModuleVmctxInfo, ValueLabelsRanges};
-
-pub use crate::read_debuginfo::{DebugInfoData, FunctionMetadata, WasmType};
 use wasmtime_environ::isa::TargetIsa;
+use wasmtime_environ::wasm::{get_vmctx_value_label, DefinedFuncIndex};
+use wasmtime_environ::WasmFileInfo;
+use wasmtime_environ::{DebugInfoData, FunctionMetadata};
+use wasmtime_environ::{ModuleVmctxInfo, ValueLabelsRanges};
 
 const PRODUCER_NAME: &str = "wasmtime";
 
@@ -87,7 +88,7 @@ fn generate_line_info(
     Ok(out_program)
 }
 
-fn check_invalid_chars_in_name(s: String) -> Option<String> {
+fn check_invalid_chars_in_name(s: &str) -> Option<&str> {
     if s.contains('\x00') {
         None
     } else {
@@ -96,16 +97,13 @@ fn check_invalid_chars_in_name(s: String) -> Option<String> {
 }
 
 fn autogenerate_dwarf_wasm_path(di: &DebugInfoData) -> PathBuf {
+    static NEXT_ID: AtomicUsize = AtomicUsize::new(0);
     let module_name = di
         .name_section
-        .as_ref()
-        .and_then(|ns| ns.module_name.to_owned())
+        .module_name
         .and_then(check_invalid_chars_in_name)
-        .unwrap_or_else(|| unsafe {
-            static mut GEN_ID: u32 = 0;
-            GEN_ID += 1;
-            format!("<gen-{}>", GEN_ID)
-        });
+        .map(|s| s.to_string())
+        .unwrap_or_else(|| format!("<gen-{}>", NEXT_ID.fetch_add(1, SeqCst)));
     let path = format!("/<wasm-module>/{}.wasm", module_name);
     PathBuf::from(path)
 }
@@ -195,7 +193,7 @@ fn generate_vars(
     scope_ranges: &[(u64, u64)],
     wasm_types: &WasmTypesDieRefs,
     func_meta: &FunctionMetadata,
-    locals_names: Option<&HashMap<u32, String>>,
+    locals_names: Option<&HashMap<u32, &str>>,
     out_strings: &mut write::StringTable,
     isa: &dyn TargetIsa,
 ) -> Result<(), Error> {
@@ -253,7 +251,7 @@ fn generate_vars(
 
             let name_id = match locals_names
                 .and_then(|m| m.get(&(var_index as u32)))
-                .and_then(|s| check_invalid_chars_in_name(s.to_owned()))
+                .and_then(|s| check_invalid_chars_in_name(s))
             {
                 Some(n) => out_strings.add(assert_dwarf_str!(n)),
                 None => out_strings.add(format!("var{}", var_index)),
@@ -297,14 +295,8 @@ pub fn generate_simulated_dwarf(
         .and_then(check_invalid_chars_in_path)
         .unwrap_or_else(|| autogenerate_dwarf_wasm_path(di));
 
-    let (func_names, locals_names) = if let Some(ref name_section) = di.name_section {
-        (
-            Some(&name_section.func_names),
-            Some(&name_section.locals_names),
-        )
-    } else {
-        (None, None)
-    };
+    let func_names = &di.name_section.func_names;
+    let locals_names = &di.name_section.locals_names;
     let imported_func_count = di.wasm_file.imported_func_count;
 
     let (unit, root_id, name_id) = {
@@ -376,8 +368,8 @@ pub fn generate_simulated_dwarf(
 
         let func_index = imported_func_count + (index as u32);
         let id = match func_names
-            .and_then(|m| m.get(&func_index))
-            .and_then(|s| check_invalid_chars_in_name(s.to_owned()))
+            .get(&func_index)
+            .and_then(|s| check_invalid_chars_in_name(s))
         {
             Some(n) => out_strings.add(assert_dwarf_str!(n)),
             None => out_strings.add(format!("wasm-function[{}]", func_index)),
@@ -407,7 +399,7 @@ pub fn generate_simulated_dwarf(
                 &[(source_range.0, source_range.1)],
                 &wasm_types,
                 &di.wasm_file.funcs[index],
-                locals_names.and_then(|m| m.get(&(index as u32))),
+                locals_names.get(&(index as u32)),
                 out_strings,
                 isa,
             )?;