Debugging: Keep a component's breakpoints in the module they were set in

crates/cranelift/src/compiled_function.rs

@@ -6,8 +6,8 @@ use cranelift_codegen::{
     binemit::Reloc, ir, isa::unwind::CfaUnwindInfo, isa::unwind::UnwindInfo,
 };
 use wasmtime_environ::{
-    FilePos, FrameStateSlotBuilder, InstructionAddressMap, ModulePC, PrimaryMap, TrapInformation,
-    Tunables,
+    FilePos, FrameStateSlotBuilder, FuncKey, InstructionAddressMap, ModulePC, PrimaryMap,
+    StaticModuleIndex, TrapInformation, Tunables,
 };
 
 #[derive(Debug, Clone, PartialEq, Eq, Default)]
@@ -68,8 +68,10 @@ pub struct CompiledFunction {
     metadata: CompiledFunctionMetadata,
     /// Debug metadata for the top-level function's state slot.
     pub debug_slot_descriptor: Option<FrameStateSlotBuilder>,
-    /// Debug breakpoint patches: module-relative Wasm PC, offset range in buffer.
-    pub breakpoint_patch_points: Vec<(ModulePC, Range<u32>)>,
+    /// Debug breakpoint patches: the module whose code it is (which
+    /// differs from this function's own module if inlined from
+    /// another), module-relative Wasm PC, offset range in buffer.
+    pub breakpoint_patch_points: Vec<(StaticModuleIndex, ModulePC, Range<u32>)>,
 }
 
 impl CompiledFunction {
@@ -117,19 +119,37 @@ impl CompiledFunction {
             assert_eq!(tag.offset, patchable_callsite.ret_addr);
 
             // Tag format used by our Wasm-to-CLIF format is
-            // (stackslot, wasm_pc, stack_shape). Taking the
-            // second-to-last tag will get the innermost Wasm PC (if
-            // there are multiple nested frames due to inlining).
+            // (stackslot, wasm_pc, stack_shape). Taking the last
+            // triple will get the innermost frame (if there are
+            // multiple nested frames due to inlining): its Wasm PC,
+            // and through its state slot, the module that PC is in.
             assert!(tag.tags.len() >= 3);
-            let ir::DebugTag::User(wasm_pc_raw) = tag.tags[tag.tags.len() - 2] else {
+            let &[
+                ir::DebugTag::StackSlot(slot),
+                ir::DebugTag::User(wasm_pc_raw),
+                ir::DebugTag::User(_stack_shape),
+            ] = &tag.tags[tag.tags.len() - 3..]
+            else {
                 panic!("invalid tag")
             };
+            let func_key = self
+                .buffer
+                .frame_layout()
+                .expect("breakpoint in a function without a frame layout")
+                .stackslots[slot]
+                .key
+                .expect("Key must be present on stackslot used as state slot")
+                .bits();
+            let (module, _) = FuncKey::from_raw_u64(func_key).unwrap_defined_wasm_function();
 
             let patchable_start = patchable_callsite.ret_addr - patchable_callsite.len;
             let patchable_end = patchable_callsite.ret_addr;
 
-            self.breakpoint_patch_points
-                .push((ModulePC::new(wasm_pc_raw), patchable_start..patchable_end));
+            self.breakpoint_patch_points.push((
+                module,
+                ModulePC::new(wasm_pc_raw),
+                patchable_start..patchable_end,
+            ));
 
             tags.next();
             patchable_callsites.next();
@@ -252,8 +272,10 @@ impl CompiledFunction {
 
     /// Returns an iterator over breakpoint patches for this function.
     ///
-    /// Each tuple is (wasm PC, buffer offset range).
-    pub fn breakpoint_patches(&self) -> impl Iterator<Item = (ModulePC, Range<u32>)> + '_ {
+    /// Each tuple is (module, wasm PC, buffer offset range).
+    pub fn breakpoint_patches(
+        &self,
+    ) -> impl Iterator<Item = (StaticModuleIndex, ModulePC, Range<u32>)> + '_ {
         self.breakpoint_patch_points.iter().cloned()
     }
 }

crates/cranelift/src/compiler.rs

@@ -736,7 +736,7 @@ impl wasmtime_environ::Compiler for Compiler {
             }
         }
 
-        let mut breakpoint_table: Vec<(ModulePC, Range<u32>)> = Vec::new();
+        let mut breakpoint_table: Vec<(StaticModuleIndex, ModulePC, Range<u32>)> = Vec::new();
         let mut nop_units = None;
 
         let mut ret = Vec::with_capacity(funcs.len());
@@ -799,7 +799,7 @@ impl wasmtime_environ::Compiler for Compiler {
         // may appear in multiple functions (due to inlining) so it is
         // only now that we can aggregate all appearances of a PC
         // together for breakpoint metadata indexed by that PC.
-        breakpoint_table.sort_by_key(|(wasm_pc, _text_range)| *wasm_pc);
+        breakpoint_table.sort_by_key(|(_module, wasm_pc, _text_range)| *wasm_pc);
 
         builder.finish(|text| {
             if !breakpoint_table.is_empty() {
@@ -817,11 +817,11 @@ impl wasmtime_environ::Compiler for Compiler {
                     }
                 };
 
-                for (wasm_pc, text_range) in &breakpoint_table {
+                for (module, wasm_pc, text_range) in &breakpoint_table {
                     let start = usize::try_from(text_range.start).unwrap();
                     let end = usize::try_from(text_range.end).unwrap();
                     let text = &mut text[start..end];
-                    frame_tables.add_breakpoint_patch(*wasm_pc, text_range.start, text);
+                    frame_tables.add_breakpoint_patch(*module, *wasm_pc, text_range.start, text);
                     fill_with_nops(text);
                 }
             }
@@ -1888,13 +1888,13 @@ fn clif_to_env_frame_tables<'a>(
 /// Wasmtime's serialized metadata.
 fn clif_to_env_breakpoints(
     range: Range<u64>,
-    breakpoint_patches: impl Iterator<Item = (ModulePC, Range<u32>)>,
-    patch_table: &mut Vec<(ModulePC, Range<u32>)>,
+    breakpoint_patches: impl Iterator<Item = (StaticModuleIndex, ModulePC, Range<u32>)>,
+    patch_table: &mut Vec<(StaticModuleIndex, ModulePC, Range<u32>)>,
 ) -> Result<()> {
-    patch_table.extend(breakpoint_patches.map(|(wasm_pc, offset_range)| {
+    patch_table.extend(breakpoint_patches.map(|(module, wasm_pc, offset_range)| {
         let start = offset_range.start + u32::try_from(range.start).unwrap();
         let end = offset_range.end + u32::try_from(range.start).unwrap();
-        (wasm_pc, start..end)
+        (module, wasm_pc, start..end)
     }));
     Ok(())
 }

crates/environ/src/compile/frame_table.rs

@@ -10,7 +10,7 @@
 
 use crate::{
     FrameInstPos, FrameStackShape, FrameStateSlotOffset, FrameTableDescriptorIndex, FrameValType,
-    FuncKey, ModulePC, WasmHeapTopType, WasmValType, prelude::*,
+    FuncKey, ModulePC, StaticModuleIndex, WasmHeapTopType, WasmValType, prelude::*,
 };
 use object::{LittleEndian, U32};
 use std::collections::{HashMap, hash_map::Entry};
@@ -212,7 +212,9 @@ impl FrameStateSlotBuilder {
 /// - `num_progpoints` times:
 ///   - progpoint descriptor offset: u32
 /// - `num_breakpoints` times:
-///    - Wasm PC: u32 (sorted order; may repeat)
+///    - static module index of the Wasm PC: u32
+/// - `num_breakpoints` times:
+///    - Wasm PC, relative to that module: u32 (sorted order; may repeat)
 /// - `num_breakpoints` times:
 ///    - patch offset in text: u32
 /// - `num_breakpoints` times:
@@ -243,6 +245,7 @@ pub struct FrameTableBuilder {
     progpoint_descriptor_offsets: Vec<U32<LittleEndian>>,
     progpoint_descriptor_data: Vec<U32<LittleEndian>>,
 
+    breakpoint_modules: Vec<U32<LittleEndian>>,
     breakpoint_pcs: Vec<U32<LittleEndian>>,
     breakpoint_patch_offsets: Vec<U32<LittleEndian>>,
     breakpoint_patch_data_ends: Vec<U32<LittleEndian>>,
@@ -334,13 +337,18 @@ impl FrameTableBuilder {
         }
     }
 
-    /// Add one breakpoint patch.
+    /// Add one breakpoint patch, for a Wasm PC in the given module.
+    ///
+    /// Patches must be added in order of Wasm PC.
     pub fn add_breakpoint_patch(
         &mut self,
+        module: StaticModuleIndex,
         wasm_pc: ModulePC,
         patch_start_native_pc: u32,
         patch: &[u8],
     ) {
+        self.breakpoint_modules
+            .push(U32::new(LittleEndian, module.as_u32()));
         self.breakpoint_pcs
             .push(U32::new(LittleEndian, wasm_pc.raw()));
         self.breakpoint_patch_offsets
@@ -381,6 +389,7 @@ impl FrameTableBuilder {
         f(object::bytes_of_slice(&self.frame_descriptor_fp_offsets));
         f(object::bytes_of_slice(&self.progpoint_pcs));
         f(object::bytes_of_slice(&self.progpoint_descriptor_offsets));
+        f(object::bytes_of_slice(&self.breakpoint_modules));
         f(object::bytes_of_slice(&self.breakpoint_pcs));
         f(object::bytes_of_slice(&self.breakpoint_patch_offsets));
         f(object::bytes_of_slice(&self.breakpoint_patch_data_ends));

crates/environ/src/frame_table.rs

@@ -4,7 +4,7 @@
 //! section in a compiled artifact as produced by
 //! [`crate::compile::FrameTableBuilder`].
 
-use crate::{FuncKey, ModulePC};
+use crate::{FuncKey, ModulePC, StaticModuleIndex};
 use alloc::vec::Vec;
 use object::{Bytes, LittleEndian, U32};
 
@@ -53,6 +53,7 @@ pub struct FrameTable<'a> {
     progpoint_descriptor_offsets: &'a [U32<LittleEndian>],
     progpoint_descriptor_data: &'a [U32<LittleEndian>],
 
+    breakpoint_modules: &'a [U32<LittleEndian>],
     breakpoint_pcs: &'a [U32<LittleEndian>],
     breakpoint_patch_offsets: &'a [U32<LittleEndian>],
     breakpoint_patch_data_ends: &'a [U32<LittleEndian>],
@@ -115,6 +116,9 @@ impl<'a> FrameTable<'a> {
         let (progpoint_descriptor_offsets, data) =
             object::slice_from_bytes::<U32<LittleEndian>>(data, num_progpoint_descriptors)
                 .map_err(|_| anyhow::anyhow!("Unable to read progpoint descriptor offset slice"))?;
+        let (breakpoint_modules, data) =
+            object::slice_from_bytes::<U32<LittleEndian>>(data, num_breakpoints)
+                .map_err(|_| anyhow::anyhow!("Unable to read breakpoint module slice"))?;
         let (breakpoint_pcs, data) =
             object::slice_from_bytes::<U32<LittleEndian>>(data, num_breakpoints)
                 .map_err(|_| anyhow::anyhow!("Unable to read breakpoint PC slice"))?;
@@ -146,6 +150,7 @@ impl<'a> FrameTable<'a> {
             progpoint_pcs,
             progpoint_descriptor_offsets,
             progpoint_descriptor_data,
+            breakpoint_modules,
             breakpoint_pcs,
             breakpoint_patch_offsets,
             breakpoint_patch_data_ends,
@@ -345,14 +350,19 @@ impl<'a> FrameTable<'a> {
 
     /// Return an iterator over all breakpoint patches.
     ///
-    /// Returned tuples are (module-relative Wasm PC, breakpoint data).
+    /// Returned tuples are (module, module-relative Wasm PC, breakpoint
+    /// data). The module is the one whose Wasm code the breakpoint is
+    /// in, even if that code was inlined into another module's
+    /// function.
     pub fn breakpoint_patches(
         &self,
-    ) -> impl Iterator<Item = (ModulePC, FrameTableBreakpointData<'_>)> + '_ {
+    ) -> impl Iterator<Item = (StaticModuleIndex, ModulePC, FrameTableBreakpointData<'_>)> + '_
+    {
         self.breakpoint_pcs.iter().enumerate().map(|(i, wasm_pc)| {
+            let module = StaticModuleIndex::from_u32(self.breakpoint_modules[i].get(LittleEndian));
             let wasm_pc = ModulePC::new(wasm_pc.get(LittleEndian));
             let data = self.breakpoint_patch(i);
-            (wasm_pc, data)
+            (module, wasm_pc, data)
         })
     }
 }

crates/environ/src/module_artifacts.rs

@@ -431,19 +431,6 @@ impl CompiledFunctionsTable {
         if loc.is_empty() { None } else { Some(loc) }
     }
 
-    /// Get the range of the text section that holds all of `namespace`'s
-    /// functions, if it has any.
-    ///
-    /// A namespace's functions are laid out together, so no other function
-    /// falls inside this range.
-    pub fn text_range(&self, namespace: FuncKeyNamespace) -> Option<Range<u32>> {
-        let ns_idx = self.namespace_index(namespace)?;
-        let Range { start, end } = self.func_loc_range(ns_idx);
-        let first = self.func_locs.get(start)?;
-        let last = &self.func_locs[FuncLocIndex::from_u32(end.as_u32() - 1)];
-        Some(first.start..last.start + last.length)
-    }
-
     fn src_loc_index(&self, key: FuncKey) -> Option<SrcLocIndex> {
         let (key_ns, key_index) = key.into_parts();
         if !Self::has_src_locs(key_ns.kind()) {

crates/wasmtime/src/runtime/debug.rs

@@ -1241,9 +1241,9 @@ impl<'a> BreakpointState<'a> {
             .expect("Frame table must be present when guest-debug is enabled");
         // A component's modules share one code image, and so its frame table: leave the
         // other modules' code to them, or this module's breakpoints decide theirs.
-        let own_code = module.compiled_module().defined_funcs_text_range();
-        for (wasm_pc, patch) in table.breakpoint_patches() {
-            if !own_code.contains(&patch.offset) {
+        let index = module.compiled_module().module_index();
+        for (patch_module, wasm_pc, patch) in table.breakpoint_patches() {
+            if patch_module != index {
                 continue;
             }
             let key = BreakpointKey::from_raw(&module, wasm_pc);

crates/wasmtime/src/runtime/instantiate.rs

@@ -90,7 +90,7 @@ impl CompiledModule {
         &self.module
     }
 
-    fn module_index(&self) -> StaticModuleIndex {
+    pub(crate) fn module_index(&self) -> StaticModuleIndex {
         self.module.module_index
     }
 
@@ -143,23 +143,6 @@ impl CompiledModule {
         self.engine_code.raw_wasm_to_array_trampoline_data(range)
     }
 
-    /// Returns the range of the text section that holds this module's
-    /// defined functions.
-    ///
-    /// A component's modules share one text section, so this excludes the
-    /// other modules' functions.
-    pub fn defined_funcs_text_range(&self) -> Range<usize> {
-        let namespace =
-            FuncKey::DefinedWasmFunction(self.module_index(), DefinedFuncIndex::from_u32(0))
-                .namespace();
-        match self.index.text_range(namespace) {
-            Some(range) => {
-                usize::try_from(range.start).unwrap()..usize::try_from(range.end).unwrap()
-            }
-            None => 0..0,
-        }
-    }
-
     /// Lookups a defined function by a program counter value.
     ///
     /// Returns the defined function index and the relative address of

src/commands/objdump.rs

@@ -152,7 +152,7 @@ impl ObjdumpCommand {
         let mut breakpoints = frame_table_descriptors
             .iter()
             .flat_map(|ftd| ftd.breakpoint_patches())
-            .map(|(wasm_pc, patch)| (wasm_pc, patch.offset, SmallVec::from(patch.enable)))
+            .map(|(_module, wasm_pc, patch)| (wasm_pc, patch.offset, SmallVec::from(patch.enable)))
             .collect::<Vec<_>>();
         breakpoints.sort_by_key(|(_wasm_pc, native_offset, _patch)| *native_offset);
         let breakpoints: Box<dyn Iterator<Item = _>> = Box::new(breakpoints.into_iter());