Move JIT debug image registration to CodeMemory (bytecodealliance#9470)

* Move JIT debug image registration to CodeMemory

JIT images correspond to ELF images, which may represent
multiple modules within a single component.

* Add the last ifdef

* Remove ManuallyDrop as per feedback

crates/environ/src/compile/module_artifacts.rs

@@ -237,7 +237,6 @@ impl<'a> ObjectBuilder<'a> {
             wasm_to_array_trampolines,
             func_names,
             meta: Metadata {
-                native_debug_info_present: self.tunables.generate_native_debuginfo,
                 has_unparsed_debuginfo,
                 code_section_offset: debuginfo.wasm_file.code_section_offset,
                 has_wasm_debuginfo: self.tunables.parse_wasm_debuginfo,

crates/environ/src/module_artifacts.rs

@@ -93,9 +93,6 @@ pub struct FunctionName {
 /// Metadata associated with a compiled ELF artifact.
 #[derive(Serialize, Deserialize)]
 pub struct Metadata {
-    /// Whether or not native debug information is available in `obj`
-    pub native_debug_info_present: bool,
-
     /// Whether or not the original wasm module contained debug information that
     /// we skipped and did not parse.
     pub has_unparsed_debuginfo: bool,

crates/wasmtime/src/runtime/code_memory.rs

@@ -2,7 +2,6 @@
 
 use crate::prelude::*;
 use crate::runtime::vm::{libcalls, MmapVec, UnwindRegistration};
-use core::mem::ManuallyDrop;
 use core::ops::Range;
 use object::endian::NativeEndian;
 use object::read::{elf::ElfFile64, Object, ObjectSection};
@@ -15,12 +14,14 @@ use wasmtime_jit_icache_coherence as icache_coherence;
 /// This type consumes ownership of a region of memory and will manage the
 /// executable permissions of the contained JIT code as necessary.
 pub struct CodeMemory {
-    // NB: these are `ManuallyDrop` because `unwind_registration` must be
-    // dropped first since it refers to memory owned by `mmap`.
-    mmap: ManuallyDrop<MmapVec>,
-    unwind_registration: ManuallyDrop<Option<UnwindRegistration>>,
+    mmap: MmapVec,
+    unwind_registration: Option<UnwindRegistration>,
+    #[cfg(feature = "debug-builtins")]
+    debug_registration: Option<crate::runtime::vm::GdbJitImageRegistration>,
     published: bool,
     enable_branch_protection: bool,
+    #[cfg(feature = "debug-builtins")]
+    has_native_debug_info: bool,
 
     relocations: Vec<(usize, obj::LibCall)>,
 
@@ -32,16 +33,15 @@ pub struct CodeMemory {
     address_map_data: Range<usize>,
     func_name_data: Range<usize>,
     info_data: Range<usize>,
-    dwarf: Range<usize>,
+    wasm_dwarf: Range<usize>,
 }
 
 impl Drop for CodeMemory {
     fn drop(&mut self) {
-        // Drop `unwind_registration` before `self.mmap`
-        unsafe {
-            ManuallyDrop::drop(&mut self.unwind_registration);
-            ManuallyDrop::drop(&mut self.mmap);
-        }
+        // Drop the registrations before `self.mmap` since they (implicitly) refer to it.
+        let _ = self.unwind_registration.take();
+        #[cfg(feature = "debug-builtins")]
+        let _ = self.debug_registration.take();
     }
 }
 
@@ -65,12 +65,14 @@ impl CodeMemory {
         let mut text = 0..0;
         let mut unwind = 0..0;
         let mut enable_branch_protection = None;
+        #[cfg(feature = "debug-builtins")]
+        let mut has_native_debug_info = false;
         let mut trap_data = 0..0;
         let mut wasm_data = 0..0;
         let mut address_map_data = 0..0;
         let mut func_name_data = 0..0;
         let mut info_data = 0..0;
-        let mut dwarf = 0..0;
+        let mut wasm_dwarf = 0..0;
         for section in obj.sections() {
             let data = section.data().err2anyhow()?;
             let name = section.name().err2anyhow()?;
@@ -124,23 +126,29 @@ impl CodeMemory {
                 obj::ELF_WASMTIME_TRAPS => trap_data = range,
                 obj::ELF_NAME_DATA => func_name_data = range,
                 obj::ELF_WASMTIME_INFO => info_data = range,
-                obj::ELF_WASMTIME_DWARF => dwarf = range,
+                obj::ELF_WASMTIME_DWARF => wasm_dwarf = range,
+                #[cfg(feature = "debug-builtins")]
+                ".debug_info" => has_native_debug_info = true,
 
                 _ => log::debug!("ignoring section {name}"),
             }
         }
         Ok(Self {
-            mmap: ManuallyDrop::new(mmap),
-            unwind_registration: ManuallyDrop::new(None),
+            mmap,
+            unwind_registration: None,
+            #[cfg(feature = "debug-builtins")]
+            debug_registration: None,
             published: false,
             enable_branch_protection: enable_branch_protection
                 .ok_or_else(|| anyhow!("missing `{}` section", obj::ELF_WASM_BTI))?,
+            #[cfg(feature = "debug-builtins")]
+            has_native_debug_info,
             text,
             unwind,
             trap_data,
             address_map_data,
             func_name_data,
-            dwarf,
+            wasm_dwarf,
             info_data,
             wasm_data,
             relocations,
@@ -162,8 +170,8 @@ impl CodeMemory {
 
     /// Returns the contents of the `ELF_WASMTIME_DWARF` section.
     #[inline]
-    pub fn dwarf(&self) -> &[u8] {
-        &self.mmap[self.dwarf.clone()]
+    pub fn wasm_dwarf(&self) -> &[u8] {
+        &self.mmap[self.wasm_dwarf.clone()]
     }
 
     /// Returns the data in the `ELF_NAME_DATA` section.
@@ -211,6 +219,7 @@ impl CodeMemory {
     ///
     /// * Change page protections from read/write to read/execute.
     /// * Register unwinding information with the OS
+    /// * Register this image with the debugger if native DWARF is present
     ///
     /// After this function executes all JIT code should be ready to execute.
     pub fn publish(&mut self) -> Result<()> {
@@ -268,6 +277,9 @@ impl CodeMemory {
             // runtime that there's unwinding information available for all
             // our just-published JIT functions.
             self.register_unwind_info()?;
+
+            #[cfg(feature = "debug-builtins")]
+            self.register_debug_image()?;
         }
 
         Ok(())
@@ -314,7 +326,24 @@ impl CodeMemory {
         let registration =
             UnwindRegistration::new(text.as_ptr(), unwind_info.as_ptr(), unwind_info.len())
                 .context("failed to create unwind info registration")?;
-        *self.unwind_registration = Some(registration);
+        self.unwind_registration = Some(registration);
+        Ok(())
+    }
+
+    #[cfg(feature = "debug-builtins")]
+    fn register_debug_image(&mut self) -> Result<()> {
+        if !self.has_native_debug_info {
+            return Ok(());
+        }
+
+        // TODO-DebugInfo: we're copying the whole image here, which is pretty wasteful.
+        // Use the existing memory by teaching code here about relocations in DWARF sections
+        // and anything else necessary that is done in "create_gdbjit_image" right now.
+        let image = self.mmap().to_vec();
+        let text: &[u8] = self.text();
+        let bytes = crate::debug::create_gdbjit_image(image, (text.as_ptr(), text.len()))?;
+        let reg = crate::runtime::vm::GdbJitImageRegistration::register(bytes);
+        self.debug_registration = Some(reg);
         Ok(())
     }
 

crates/wasmtime/src/runtime/instantiate.rs

@@ -21,8 +21,6 @@ pub struct CompiledModule {
     wasm_to_array_trampolines: Vec<(ModuleInternedTypeIndex, FunctionLoc)>,
     meta: Metadata,
     code_memory: Arc<CodeMemory>,
-    #[cfg(feature = "debug-builtins")]
-    dbg_jit_registration: Option<crate::runtime::vm::GdbJitImageRegistration>,
     /// A unique ID used to register this module with the engine.
     unique_id: CompiledModuleId,
     func_names: Vec<FunctionName>,
@@ -54,30 +52,19 @@ impl CompiledModule {
             module: Arc::new(info.module),
             funcs: info.funcs,
             wasm_to_array_trampolines: info.wasm_to_array_trampolines,
-            #[cfg(feature = "debug-builtins")]
-            dbg_jit_registration: None,
             code_memory,
             meta: info.meta,
             unique_id: CompiledModuleId::new(),
             func_names: info.func_names,
         };
-        ret.register_debug_and_profiling(profiler)?;
+        ret.register_profiling(profiler)?;
 
         Ok(ret)
     }
 
-    fn register_debug_and_profiling(&mut self, profiler: &dyn ProfilingAgent) -> Result<()> {
-        #[cfg(feature = "debug-builtins")]
-        if self.meta.native_debug_info_present {
-            let text = self.text();
-            let bytes = crate::debug::create_gdbjit_image(
-                self.mmap().to_vec(),
-                (text.as_ptr(), text.len()),
-            )
-            .context("failed to create jit image for gdb")?;
-            let reg = crate::runtime::vm::GdbJitImageRegistration::register(bytes);
-            self.dbg_jit_registration = Some(reg);
-        }
+    fn register_profiling(&mut self, profiler: &dyn ProfilingAgent) -> Result<()> {
+        // TODO-Bug?: "code_memory" is not exclusive for this module in the case of components,
+        // so we may be registering the same code range multiple times here.
         profiler.register_module(&self.code_memory.mmap()[..], &|addr| {
             let (idx, _) = self.func_by_text_offset(addr)?;
             let idx = self.module.func_index(idx);
@@ -279,7 +266,7 @@ impl CompiledModule {
                     let (_, range) = &self.meta.dwarf[i];
                     let start = range.start.try_into().ok()?;
                     let end = range.end.try_into().ok()?;
-                    self.code_memory().dwarf().get(start..end)
+                    self.code_memory().wasm_dwarf().get(start..end)
                 })
                 .unwrap_or(&[]);
             Ok(EndianSlice::new(data, gimli::LittleEndian))

tests/all/debug/lldb.rs

@@ -63,10 +63,6 @@ fn check_lldb_output(output: &str, directives: &str) -> Result<()> {
 
 #[test]
 #[ignore]
-#[cfg(all(
-    any(target_os = "linux", target_os = "macos"),
-    target_pointer_width = "64"
-))]
 pub fn test_debug_dwarf_lldb() -> Result<()> {
     let output = lldb_with_script(
         &[
@@ -104,10 +100,6 @@ check: exited with status
 
 #[test]
 #[ignore]
-#[cfg(all(
-    any(target_os = "linux", target_os = "macos"),
-    target_pointer_width = "64"
-))]
 pub fn test_debug_dwarf5_lldb() -> Result<()> {
     let output = lldb_with_script(
         &[
@@ -145,10 +137,6 @@ check: exited with status
 
 #[test]
 #[ignore]
-#[cfg(all(
-    any(target_os = "linux", target_os = "macos"),
-    target_pointer_width = "64"
-))]
 pub fn test_debug_split_dwarf4_lldb() -> Result<()> {
     let output = lldb_with_script(
         &[
@@ -186,10 +174,6 @@ check: exited with status
 
 #[test]
 #[ignore]
-#[cfg(all(
-    any(target_os = "linux", target_os = "macos"),
-    target_pointer_width = "64"
-))]
 pub fn test_debug_dwarf_ref() -> Result<()> {
     let output = lldb_with_script(
         &[
@@ -220,10 +204,6 @@ check: resuming
 
 #[test]
 #[ignore]
-#[cfg(all(
-    any(target_os = "linux", target_os = "macos"),
-    target_pointer_width = "64"
-))]
 pub fn test_debug_inst_offsets_are_correct_when_branches_are_removed() -> Result<()> {
     let output = lldb_with_script(
         &[
@@ -247,10 +227,6 @@ check: exited with status
 
 #[test]
 #[ignore]
-#[cfg(all(
-    any(target_os = "linux", target_os = "macos"),
-    target_pointer_width = "64"
-))]
 pub fn test_spilled_frame_base_is_accessible() -> Result<()> {
     let output = lldb_with_script(
         &[
@@ -305,10 +281,6 @@ int main()
  */
 #[test]
 #[ignore]
-#[cfg(all(
-    any(target_os = "linux", target_os = "macos"),
-    target_pointer_width = "64"
-))]
 pub fn test_debug_dwarf5_fission_lldb() -> Result<()> {
     let output = lldb_with_script(
         &[
@@ -341,10 +313,6 @@ check: exited with status = 0
     Ok(())
 }
 
-#[cfg(all(
-    any(target_os = "linux", target_os = "macos"),
-    target_pointer_width = "64"
-))]
 mod test_programs {
     use super::{check_lldb_output, lldb_with_script};
     use anyhow::Result;