diff --git a/ghostscope-dwarf/src/core/types.rs b/ghostscope-dwarf/src/core/types.rs index c95edf79..140dd087 100644 --- a/ghostscope-dwarf/src/core/types.rs +++ b/ghostscope-dwarf/src/core/types.rs @@ -84,8 +84,74 @@ pub struct IndexEntry { /// For variables: vec![(address, address)] if static /// Empty vec if no address (e.g., types, inlined functions without concrete instances) pub address_ranges: Vec<(u64, u64)>, - /// Optional DW_AT_entry_pc for inline/call site DIEs (single-point locations) + /// Optional raw DW_AT_entry_pc for inline/call site DIEs. + /// Callers should prefer `validated_entry_pc()` when selecting probe PCs. pub entry_pc: Option, + /// Explicit function role for addressable function-like DIEs. + pub function_kind: FunctionDieKind, +} + +impl IndexEntry { + pub fn function_kind(&self) -> FunctionDieKind { + if self.function_kind != FunctionDieKind::NotFunction { + return self.function_kind; + } + + match self.tag { + gimli::constants::DW_TAG_inlined_subroutine => FunctionDieKind::InlineInstance, + gimli::constants::DW_TAG_subprogram => { + if self.flags.is_inline_instance { + FunctionDieKind::InlineInstance + } else if !self.address_ranges.is_empty() || self.entry_pc.is_some() { + FunctionDieKind::ConcreteSubprogram + } else { + FunctionDieKind::AbstractSubprogram + } + } + _ => FunctionDieKind::NotFunction, + } + } + + /// True when this DIE is a concrete DW_TAG_inlined_subroutine instance. + pub fn is_inline_instance(&self) -> bool { + self.function_kind() == FunctionDieKind::InlineInstance + } + + /// True when this DIE is a concrete, addressable subprogram body. + pub fn is_concrete_subprogram(&self) -> bool { + self.function_kind() == FunctionDieKind::ConcreteSubprogram + } + + /// Return entry_pc when it is usable as this DIE's own entry address. + /// + /// Most DIEs with an entry_pc also carry ranges, and some producers emit + /// caller-side setup PCs that do not belong to the inline instance itself. + /// Reject those out-of-range PCs. However, DWARF can also encode + /// single-point inline/call-site scopes using only entry_pc and no ranges; + /// in that shape the point entry_pc is the only addressable location and + /// should be preserved. + pub fn validated_entry_pc(&self) -> Option { + self.entry_pc.filter(|pc| { + self.address_ranges.is_empty() + || self + .address_ranges + .iter() + .any(|(start, end)| *start <= *pc && *pc < *end) + }) + } +} + +#[derive(Debug, Clone, Copy, PartialEq, Eq, Default)] +pub enum FunctionDieKind { + #[default] + NotFunction, + /// Addressless DW_TAG_subprogram, typically an abstract definition or + /// declaration-like node used only for origin/specification metadata. + AbstractSubprogram, + /// Concrete out-of-line DW_TAG_subprogram with executable code ranges. + ConcreteSubprogram, + /// Concrete DW_TAG_inlined_subroutine instance inside a caller. + InlineInstance, } /// Index flags (inspired by GDB's cooked_index_flag_enum) @@ -95,8 +161,10 @@ pub struct IndexFlags { pub is_static: bool, /// True if this is the program's main function pub is_main: bool, - /// True if this is an inline function - pub is_inline: bool, + /// True if this DIE is a concrete DW_TAG_inlined_subroutine instance. + pub is_inline_instance: bool, + /// True if this DIE carries DW_AT_inline (or inherits that declaration). + pub has_inline_attribute: bool, /// True if this entry uses the linkage name pub is_linkage: bool, /// True if this is just a type declaration (not definition) diff --git a/ghostscope-dwarf/src/index/lightweight_index.rs b/ghostscope-dwarf/src/index/lightweight_index.rs index b43bcaea..7c2668f1 100644 --- a/ghostscope-dwarf/src/index/lightweight_index.rs +++ b/ghostscope-dwarf/src/index/lightweight_index.rs @@ -386,9 +386,9 @@ impl LightweightIndex { tracing::trace!("Found {} entries for function '{}'", entries.len(), name); for entry in &entries { - let display_addr = if entry.flags.is_inline { + let display_addr = if entry.is_inline_instance() { entry - .entry_pc + .validated_entry_pc() .or_else(|| entry.address_ranges.first().map(|(start, _)| *start)) } else { entry.address_ranges.first().map(|(start, _)| *start) @@ -396,10 +396,11 @@ impl LightweightIndex { if let Some(addr) = display_addr { tracing::trace!( - " - {} at 0x{:x} (inline={}, {} ranges)", + " - {} at 0x{:x} (role={:?}, inline={}, {} ranges)", entry.name, addr, - entry.flags.is_inline, + entry.function_kind(), + entry.is_inline_instance(), entry.address_ranges.len() ); } diff --git a/ghostscope-dwarf/src/module/data.rs b/ghostscope-dwarf/src/module/data.rs index afc04ddd..c382899c 100644 --- a/ghostscope-dwarf/src/module/data.rs +++ b/ghostscope-dwarf/src/module/data.rs @@ -22,7 +22,7 @@ mod file_selection_scoring { } use crate::{ - binary::{empty_dwarf_reader, try_load_debug_file, DwarfData, MappedFile}, + binary::{empty_dwarf_reader, try_load_debug_file, DwarfData, DwarfReader, MappedFile}, core::{mapping::ModuleMapping, GlobalVariableInfo, Result, SectionType, SourceLocation}, index::{ BlockIndex, BlockIndexBuilder, CfiIndex, FunctionBlocks, LightweightIndex, @@ -30,7 +30,10 @@ use crate::{ }, parser::{CompilationUnit, ExpressionEvaluator, SourceFile}, resolver::{ChainSpec, OnDemandResolver}, - semantics::{resolve_attr_with_unit_origins, resolve_name_with_origins}, + semantics::{ + range_contains_pc, resolve_attr_with_unit_origins, resolve_name_with_origins, + resolve_origin_entry, + }, }; use gimli::Reader; use object::{Object, ObjectSection, ObjectSegment}; @@ -1802,16 +1805,20 @@ impl ModuleData { /// /// Semantics /// - Inline (DW_TAG_inlined_subroutine): - /// Return exactly one address per inline DIE, using the inline instance - /// "start" (low_pc) semantics. Implemented as the minimum of all range - /// starts when ranges exist; otherwise fall back to entry_pc. Intentionally - /// do not scan for is_stmt here to preserve entry-like behavior and keep - /// entry-view locations available. + /// Return exactly one address per inline DIE. Prefer DW_AT_entry_pc only + /// when it falls inside the DIE's own ranges; some producers point + /// entry_pc at caller-side setup code instead of the inlined body. If the + /// validated entry_pc is missing, preserve the first DWARF-emitted range + /// start so hot/cold partitioning does not drift to a lower-address cold + /// fragment; only fall back to the minimum range start as a final + /// recovery path. Intentionally do not scan for is_stmt here to preserve + /// entry-like behavior and keep entry-view locations available. /// - Non-inline (DW_TAG_subprogram): /// For each selected executable range, return the first executable address - /// after the prologue (prologue-skip). If any formal parameter uses - /// DW_OP_entry_value, prefer the true entry (range start) to preserve - /// entry context. When DW_AT_ranges contains partitioned hot/cold code, + /// after the prologue (prologue-skip). If any formal parameter's active + /// location at the selected probe PC uses DW_OP_entry_value, prefer the + /// true entry (range start) to preserve entry context. When DW_AT_ranges + /// contains partitioned hot/cold code, /// prefer the range containing DW_AT_entry_pc; otherwise preserve the /// first DWARF-emitted range because compilers typically list the entry/ /// hot partition first even when a later address sort would put `.cold` @@ -1822,72 +1829,100 @@ impl ModuleData { /// computations to ensure stable behavior across compilers/toolchains. fn compute_addresses_for_entry(&self, entry: &crate::core::IndexEntry) -> Vec { let mut out = Vec::new(); - if entry.flags.is_inline { - // Debug: print ranges & entry_pc once per inline entry - let mut ranges = entry.address_ranges.clone(); - ranges.sort_unstable_by_key(|(s, _)| *s); - if !ranges.is_empty() { - let parts: Vec = ranges - .iter() - .map(|(s, e)| format!("(0x{s:x},0x{e:x})")) - .collect(); - let epc_dbg = entry - .entry_pc - .map(|v| format!("0x{v:x}")) - .unwrap_or("None".to_string()); - let rlen = ranges.len(); - let rlist = parts.join(", "); - tracing::debug!( - "Inline '{}' entry_pc={epc_dbg} ranges({rlen}): [{rlist}]", - entry.name - ); - } else { - let epc_dbg = entry - .entry_pc - .map(|v| format!("0x{v:x}")) - .unwrap_or("None".to_string()); - tracing::debug!("Inline '{}' has no ranges; entry_pc={epc_dbg}", entry.name); - } - - let low_pc = ranges.iter().map(|(s, _)| *s).min(); - if let Some(addr) = low_pc.or(entry.entry_pc) { - tracing::debug!("Inline '{}' selected=0x{addr:x} (low_pc)", entry.name); - out.push(addr); - } else { - tracing::warn!( - "Inline entry has no usable address (no ranges/entry_pc): unit_off={:?}, die_off={:?}", - entry.unit_offset, - entry.die_offset - ); - } - } else { - // If function parameters use DW_OP_entry_value, prefer the true entry (no prologue skip) - let prefer_entry = self.function_uses_entry_value(entry).unwrap_or(false); - let nranges = Self::selected_non_inline_ranges(entry); - for (start, _end) in &nranges { - let addr = if prefer_entry { - *start - } else { - self.line_mapping.find_first_executable_address(*start) - }; - if prefer_entry { + match entry.function_kind() { + crate::core::FunctionDieKind::InlineInstance => { + // Debug: print ranges & entry_pc once per inline entry + let mut ranges = entry.address_ranges.clone(); + ranges.sort_unstable_by_key(|(s, _)| *s); + if !ranges.is_empty() { + let parts: Vec = ranges + .iter() + .map(|(s, e)| format!("(0x{s:x},0x{e:x})")) + .collect(); + let epc_dbg = entry + .entry_pc + .map(|v| format!("0x{v:x}")) + .unwrap_or("None".to_string()); + let rlen = ranges.len(); + let rlist = parts.join(", "); tracing::debug!( - "Non-inline '{}' entry_value=true, using entry start=0x{start:x}", + "Inline '{}' entry_pc={epc_dbg} ranges({rlen}): [{rlist}]", entry.name ); } else { - let off = addr.saturating_sub(*start); - tracing::debug!( - "Non-inline '{}' start=0x{start:x} first_exec=0x{addr:x} (+0x{off:x})", - entry.name + let epc_dbg = entry + .entry_pc + .map(|v| format!("0x{v:x}")) + .unwrap_or("None".to_string()); + tracing::debug!("Inline '{}' has no ranges; entry_pc={epc_dbg}", entry.name); + } + + if let Some(addr) = Self::selected_inline_address(entry) { + tracing::debug!("Inline '{}' selected=0x{addr:x}", entry.name); + out.push(addr); + } else { + tracing::warn!( + "Inline entry has no usable address (no ranges/entry_pc): unit_off={:?}, die_off={:?}", + entry.unit_offset, + entry.die_offset ); } - out.push(addr); } + crate::core::FunctionDieKind::ConcreteSubprogram => { + let nranges = Self::selected_non_inline_ranges(entry); + for (start, end) in &nranges { + let candidate = { + let first_exec = self.line_mapping.find_first_executable_address(*start); + Self::selected_non_inline_probe_address(*start, *end, first_exec) + }; + // Only force the true entry when the location active at the + // probe PC already relies on DW_OP_entry_value. Some optimized + // functions switch to entry_value later in the body, but still + // have stable register locations at the first executable + // instruction after the prologue. + let prefer_entry = self + .function_uses_entry_value_at(entry, candidate) + .unwrap_or(false); + let addr = if prefer_entry { *start } else { candidate }; + if prefer_entry { + tracing::debug!( + "Non-inline '{}' entry_value active at 0x{candidate:x}, using entry start=0x{start:x}", + entry.name, + ); + } else { + let off = addr.saturating_sub(*start); + tracing::debug!( + "Non-inline '{}' start=0x{start:x} first_exec=0x{addr:x} (+0x{off:x})", + entry.name + ); + if addr == *start { + tracing::debug!( + "Non-inline '{}' kept entry start because prologue-skip candidate escaped range [0x{start:x}, 0x{end:x})", + entry.name + ); + } + } + out.push(addr); + } + } + crate::core::FunctionDieKind::AbstractSubprogram => { + tracing::debug!( + "Skipping abstract subprogram '{}' with no concrete code ranges", + entry.name + ); + } + crate::core::FunctionDieKind::NotFunction => {} } out } + fn selected_inline_address(entry: &crate::core::IndexEntry) -> Option { + let first_start = entry.address_ranges.first().map(|(start, _)| *start); + let low_pc = entry.address_ranges.iter().map(|(start, _)| *start).min(); + + entry.validated_entry_pc().or(first_start).or(low_pc) + } + fn selected_non_inline_ranges(entry: &crate::core::IndexEntry) -> Vec<(u64, u64)> { let ranges = entry.address_ranges.clone(); if ranges.len() <= 1 { @@ -1907,8 +1942,21 @@ impl ModuleData { vec![ranges[0]] } - /// Check if this subprogram uses DW_OP_entry_value in any formal parameter location - fn function_uses_entry_value(&self, idx_entry: &crate::core::IndexEntry) -> Result { + fn selected_non_inline_probe_address(start: u64, end: u64, candidate: u64) -> u64 { + if start <= candidate && candidate < end { + candidate + } else { + start + } + } + + /// Check if this subprogram uses DW_OP_entry_value for any formal parameter + /// location active at the given PC. + fn function_uses_entry_value_at( + &self, + idx_entry: &crate::core::IndexEntry, + pc: u64, + ) -> Result { let dwarf = self.resolver.dwarf_ref(); let header = dwarf .unit_header(idx_entry.unit_offset) @@ -1919,38 +1967,313 @@ impl ModuleData { let entry = unit .entry(idx_entry.die_offset) .map_err(|e| anyhow::anyhow!("entry load error: {}", e))?; + Self::subprogram_uses_entry_value_at(dwarf, &unit, &entry, pc) + } + + #[cfg(test)] + fn subprogram_uses_entry_value( + dwarf: &gimli::Dwarf, + unit: &gimli::Unit, + entry: &gimli::DebuggingInformationEntry, + ) -> Result { + let mut visited = HashSet::with_capacity(4); + if let Some(entry_abs) = entry.offset().to_debug_info_offset(&unit.header) { + visited.insert(entry_abs); + } + + Self::subprogram_uses_entry_value_inner(dwarf, unit, entry, &mut visited) + } + + fn subprogram_uses_entry_value_at( + dwarf: &gimli::Dwarf, + unit: &gimli::Unit, + entry: &gimli::DebuggingInformationEntry, + pc: u64, + ) -> Result { + let mut visited = HashSet::with_capacity(4); + if let Some(entry_abs) = entry.offset().to_debug_info_offset(&unit.header) { + visited.insert(entry_abs); + } + + Self::subprogram_uses_entry_value_at_inner(dwarf, unit, entry, pc, &mut visited) + } + + #[cfg(test)] + fn subprogram_uses_entry_value_inner( + dwarf: &gimli::Dwarf, + unit: &gimli::Unit, + entry: &gimli::DebuggingInformationEntry, + visited: &mut HashSet, + ) -> Result { if entry.tag() != gimli::constants::DW_TAG_subprogram { return Ok(false); } + if let Some(uses_entry_value) = + Self::direct_formal_parameters_entry_value_state(unit, entry)? + { + return Ok(uses_entry_value); + } + + for origin_attr in [ + gimli::constants::DW_AT_abstract_origin, + gimli::constants::DW_AT_specification, + ] { + if let Some(value) = entry.attr_value(origin_attr) { + if let Some((origin_abs, origin_unit, origin_entry)) = + resolve_origin_entry(dwarf, unit, value) + .map_err(|e| anyhow::anyhow!("origin resolution error: {}", e))? + { + if visited.insert(origin_abs) + && Self::subprogram_uses_entry_value_inner( + dwarf, + &origin_unit, + &origin_entry, + visited, + )? + { + return Ok(true); + } + } + } + } + + Ok(false) + } + + fn subprogram_uses_entry_value_at_inner( + dwarf: &gimli::Dwarf, + unit: &gimli::Unit, + entry: &gimli::DebuggingInformationEntry, + pc: u64, + visited: &mut HashSet, + ) -> Result { + if entry.tag() != gimli::constants::DW_TAG_subprogram { + return Ok(false); + } + + if let Some(uses_entry_value) = + Self::direct_formal_parameters_entry_value_state_at_pc(dwarf, unit, entry, pc)? + { + return Ok(uses_entry_value); + } + + for origin_attr in [ + gimli::constants::DW_AT_abstract_origin, + gimli::constants::DW_AT_specification, + ] { + if let Some(value) = entry.attr_value(origin_attr) { + if let Some((origin_abs, origin_unit, origin_entry)) = + resolve_origin_entry(dwarf, unit, value) + .map_err(|e| anyhow::anyhow!("origin resolution error: {}", e))? + { + if visited.insert(origin_abs) + && Self::subprogram_uses_entry_value_at_inner( + dwarf, + &origin_unit, + &origin_entry, + pc, + visited, + )? + { + return Ok(true); + } + } + } + } + + Ok(false) + } + + #[cfg(test)] + fn direct_formal_parameters_entry_value_state( + unit: &gimli::Unit, + entry: &gimli::DebuggingInformationEntry, + ) -> Result> { + let mut saw_parameter = false; + if let Ok(mut tree) = unit.entries_tree(Some(entry.offset())) { if let Ok(root) = tree.root() { let mut children = root.children(); while let Ok(Some(child)) = children.next() { let e = child.entry(); - if e.tag() == gimli::constants::DW_TAG_formal_parameter { - if let Ok(Some(gimli::AttributeValue::Exprloc(expr))) = - resolve_attr_with_unit_origins( - e, - &unit, - gimli::constants::DW_AT_location, - ) - { - // Parse ops and look for EntryValue - let mut expression = gimli::Expression(expr.0); - while let Ok(op) = - gimli::Operation::parse(&mut expression.0, unit.encoding()) - { - if matches!(op, gimli::Operation::EntryValue { .. }) { - return Ok(true); - } - } + if e.tag() != gimli::constants::DW_TAG_formal_parameter { + continue; + } + saw_parameter = true; + + if let Ok(Some(gimli::AttributeValue::Exprloc(expr))) = + resolve_attr_with_unit_origins(e, unit, gimli::constants::DW_AT_location) + { + if Self::expression_uses_entry_value(unit, gimli::Expression(expr.0)) { + return Ok(Some(true)); } } } } } - Ok(false) + + if saw_parameter { + Ok(Some(false)) + } else { + Ok(None) + } + } + + fn direct_formal_parameters_entry_value_state_at_pc( + dwarf: &gimli::Dwarf, + unit: &gimli::Unit, + entry: &gimli::DebuggingInformationEntry, + pc: u64, + ) -> Result> { + let mut saw_parameter = false; + + if let Ok(mut tree) = unit.entries_tree(Some(entry.offset())) { + if let Ok(root) = tree.root() { + let mut children = root.children(); + while let Ok(Some(child)) = children.next() { + let e = child.entry(); + if e.tag() != gimli::constants::DW_TAG_formal_parameter { + continue; + } + saw_parameter = true; + + if let Ok(Some(value)) = + resolve_attr_with_unit_origins(e, unit, gimli::constants::DW_AT_location) + { + if Self::attribute_uses_entry_value_at_pc(dwarf, unit, value, pc)? { + return Ok(Some(true)); + } + } + } + } + } + + if saw_parameter { + Ok(Some(false)) + } else { + Ok(None) + } + } + + fn attribute_uses_entry_value_at_pc( + dwarf: &gimli::Dwarf, + unit: &gimli::Unit, + value: gimli::AttributeValue, + pc: u64, + ) -> Result { + match value { + gimli::AttributeValue::Exprloc(expr) => Ok(Self::expression_uses_entry_value( + unit, + gimli::Expression(expr.0), + )), + gimli::AttributeValue::LocationListsRef(offset) => { + Self::location_list_uses_entry_value_at_pc( + dwarf, + unit, + gimli::LocationListsOffset(offset.0), + pc, + ) + } + gimli::AttributeValue::SecOffset(offset) => Self::location_list_uses_entry_value_at_pc( + dwarf, + unit, + gimli::LocationListsOffset(offset), + pc, + ), + _ => Ok(false), + } + } + + fn location_list_uses_entry_value_at_pc( + dwarf: &gimli::Dwarf, + unit: &gimli::Unit, + offset: gimli::LocationListsOffset, + pc: u64, + ) -> Result { + let mut raw_locations = match dwarf.raw_locations(unit, offset) { + Ok(iter) => iter, + Err(_) => return Ok(false), + }; + + let mut base_address = unit.low_pc; + let mut default_location_uses_entry_value = None; + while let Some(raw_entry) = raw_locations + .next() + .map_err(|e| anyhow::anyhow!("raw location list iteration error: {:?}", e))? + { + match raw_entry { + gimli::RawLocListEntry::BaseAddress { addr } => { + base_address = addr; + } + gimli::RawLocListEntry::BaseAddressx { addr } => { + if let Ok(resolved) = dwarf.address(unit, addr) { + base_address = resolved; + } + } + gimli::RawLocListEntry::StartLength { + begin, + length, + data, + } => { + if range_contains_pc(begin, begin.wrapping_add(length), pc) { + return Ok(Self::expression_uses_entry_value(unit, data)); + } + } + gimli::RawLocListEntry::StartEnd { begin, end, data } => { + if range_contains_pc(begin, end, pc) { + return Ok(Self::expression_uses_entry_value(unit, data)); + } + } + gimli::RawLocListEntry::OffsetPair { begin, end, data } + | gimli::RawLocListEntry::AddressOrOffsetPair { begin, end, data } => { + let start = base_address.wrapping_add(begin); + let end_addr = base_address.wrapping_add(end); + if range_contains_pc(start, end_addr, pc) { + return Ok(Self::expression_uses_entry_value(unit, data)); + } + } + gimli::RawLocListEntry::StartxLength { + begin, + length, + data, + } => { + if let Ok(start) = dwarf.address(unit, begin) { + if range_contains_pc(start, start.wrapping_add(length), pc) { + return Ok(Self::expression_uses_entry_value(unit, data)); + } + } + } + gimli::RawLocListEntry::StartxEndx { begin, end, data } => { + if let (Ok(start), Ok(end_addr)) = + (dwarf.address(unit, begin), dwarf.address(unit, end)) + { + if range_contains_pc(start, end_addr, pc) { + return Ok(Self::expression_uses_entry_value(unit, data)); + } + } + } + gimli::RawLocListEntry::DefaultLocation { data } => { + default_location_uses_entry_value = + Some(Self::expression_uses_entry_value(unit, data)); + } + } + } + + Ok(default_location_uses_entry_value.unwrap_or(false)) + } + + fn expression_uses_entry_value( + unit: &gimli::Unit, + mut expression: gimli::Expression, + ) -> bool { + while let Ok(op) = gimli::Operation::parse(&mut expression.0, unit.encoding()) { + if matches!(op, gimli::Operation::EntryValue { .. }) { + return true; + } + } + + false } /// Lookup function addresses by any of: DW_AT_name, linkage name, or demangled name @@ -2425,8 +2748,14 @@ impl ModuleData { #[cfg(test)] mod tests { use super::ModuleData; - use crate::core::{IndexEntry, IndexFlags}; + use crate::binary::{dwarf_reader_from_arc, DwarfReader}; + use crate::core::{FunctionDieKind, IndexEntry, IndexFlags}; use gimli::constants; + use gimli::write::{ + Address, AttributeValue as WriteAttributeValue, Dwarf as WriteDwarf, EndianVec, + Expression as WriteExpression, LineProgram, Location, LocationList, Sections, Unit, + }; + use gimli::{Format, Register}; use std::sync::Arc; fn subprogram_entry(ranges: &[(u64, u64)], entry_pc: Option) -> IndexEntry { @@ -2439,7 +2768,285 @@ mod tests { language: None, address_ranges: ranges.to_vec(), entry_pc, + function_kind: FunctionDieKind::ConcreteSubprogram, + } + } + + fn inline_entry(ranges: &[(u64, u64)], entry_pc: Option) -> IndexEntry { + let mut entry = subprogram_entry(ranges, entry_pc); + entry.tag = constants::DW_TAG_inlined_subroutine; + entry.flags.is_inline_instance = true; + entry.function_kind = FunctionDieKind::InlineInstance; + entry + } + + fn build_origin_backed_entry_value_fixture( + origin_attr: gimli::DwAt, + ) -> gimli::Dwarf { + let encoding = gimli::Encoding { + format: Format::Dwarf32, + version: 4, + address_size: 8, + }; + + let mut dwarf = WriteDwarf::new(); + let unit_id = dwarf.units.add(Unit::new(encoding, LineProgram::none())); + let unit = dwarf.units.get_mut(unit_id); + let root = unit.root(); + + let origin_id = unit.add(root, constants::DW_TAG_subprogram); + unit.get_mut(origin_id).set( + constants::DW_AT_name, + WriteAttributeValue::String(b"entry_value_target".to_vec()), + ); + + let origin_param_id = unit.add(origin_id, constants::DW_TAG_formal_parameter); + let mut inner = WriteExpression::new(); + inner.op_reg(Register(5)); + let mut origin_param_loc = WriteExpression::new(); + origin_param_loc.op_entry_value(inner); + unit.get_mut(origin_param_id).set( + constants::DW_AT_location, + WriteAttributeValue::Exprloc(origin_param_loc), + ); + + let concrete_id = unit.add(root, constants::DW_TAG_subprogram); + unit.get_mut(concrete_id) + .set(origin_attr, WriteAttributeValue::UnitRef(origin_id)); + + let mut sections = Sections::new(EndianVec::new(gimli::LittleEndian)); + dwarf.write(&mut sections).unwrap(); + + let dwarf_sections: gimli::DwarfSections> = gimli::DwarfSections::load(|id| { + Ok::<_, gimli::Error>( + sections + .get(id) + .map(|section| section.slice().to_vec()) + .unwrap_or_default(), + ) + }) + .unwrap(); + + dwarf_sections + .borrow(|section| dwarf_reader_from_arc(Arc::<[u8]>::from(section.as_slice()))) + } + + fn build_origin_backed_entry_value_override_fixture( + origin_attr: gimli::DwAt, + ) -> gimli::Dwarf { + let encoding = gimli::Encoding { + format: Format::Dwarf32, + version: 4, + address_size: 8, + }; + + let mut dwarf = WriteDwarf::new(); + let unit_id = dwarf.units.add(Unit::new(encoding, LineProgram::none())); + let unit = dwarf.units.get_mut(unit_id); + let root = unit.root(); + + let origin_id = unit.add(root, constants::DW_TAG_subprogram); + unit.get_mut(origin_id).set( + constants::DW_AT_name, + WriteAttributeValue::String(b"entry_value_override_target".to_vec()), + ); + + let origin_param_id = unit.add(origin_id, constants::DW_TAG_formal_parameter); + let mut inner = WriteExpression::new(); + inner.op_reg(Register(5)); + let mut origin_param_loc = WriteExpression::new(); + origin_param_loc.op_entry_value(inner); + unit.get_mut(origin_param_id).set( + constants::DW_AT_location, + WriteAttributeValue::Exprloc(origin_param_loc), + ); + + let concrete_id = unit.add(root, constants::DW_TAG_subprogram); + unit.get_mut(concrete_id) + .set(origin_attr, WriteAttributeValue::UnitRef(origin_id)); + + let concrete_param_id = unit.add(concrete_id, constants::DW_TAG_formal_parameter); + let concrete_param = unit.get_mut(concrete_param_id); + concrete_param.set( + constants::DW_AT_abstract_origin, + WriteAttributeValue::UnitRef(origin_param_id), + ); + let mut concrete_param_loc = WriteExpression::new(); + concrete_param_loc.op_reg(Register(6)); + concrete_param.set( + constants::DW_AT_location, + WriteAttributeValue::Exprloc(concrete_param_loc), + ); + + let mut sections = Sections::new(EndianVec::new(gimli::LittleEndian)); + dwarf.write(&mut sections).unwrap(); + + let dwarf_sections: gimli::DwarfSections> = gimli::DwarfSections::load(|id| { + Ok::<_, gimli::Error>( + sections + .get(id) + .map(|section| section.slice().to_vec()) + .unwrap_or_default(), + ) + }) + .unwrap(); + + dwarf_sections + .borrow(|section| dwarf_reader_from_arc(Arc::<[u8]>::from(section.as_slice()))) + } + + fn build_origin_backed_entry_value_range_fixture( + origin_attr: gimli::DwAt, + ) -> gimli::Dwarf { + let encoding = gimli::Encoding { + format: Format::Dwarf32, + version: 5, + address_size: 8, + }; + + let mut dwarf = WriteDwarf::new(); + let unit_id = dwarf.units.add(Unit::new(encoding, LineProgram::none())); + let unit = dwarf.units.get_mut(unit_id); + let root = unit.root(); + + let origin_id = unit.add(root, constants::DW_TAG_subprogram); + unit.get_mut(origin_id).set( + constants::DW_AT_name, + WriteAttributeValue::String(b"entry_value_range_target".to_vec()), + ); + + let origin_param_id = unit.add(origin_id, constants::DW_TAG_formal_parameter); + let mut direct_loc = WriteExpression::new(); + direct_loc.op_reg(Register(5)); + let mut inner = WriteExpression::new(); + inner.op_reg(Register(5)); + let mut entry_value_loc = WriteExpression::new(); + entry_value_loc.op_entry_value(inner); + let loc_id = unit.locations.add(LocationList(vec![ + Location::StartEnd { + begin: Address::Constant(0x1470), + end: Address::Constant(0x1477), + data: direct_loc, + }, + Location::StartEnd { + begin: Address::Constant(0x1477), + end: Address::Constant(0x147b), + data: entry_value_loc, + }, + ])); + unit.get_mut(origin_param_id).set( + constants::DW_AT_location, + WriteAttributeValue::LocationListRef(loc_id), + ); + + let concrete_id = unit.add(root, constants::DW_TAG_subprogram); + unit.get_mut(concrete_id) + .set(origin_attr, WriteAttributeValue::UnitRef(origin_id)); + + let mut sections = Sections::new(EndianVec::new(gimli::LittleEndian)); + dwarf.write(&mut sections).unwrap(); + + let dwarf_sections: gimli::DwarfSections> = gimli::DwarfSections::load(|id| { + Ok::<_, gimli::Error>( + sections + .get(id) + .map(|section| section.slice().to_vec()) + .unwrap_or_default(), + ) + }) + .unwrap(); + + dwarf_sections + .borrow(|section| dwarf_reader_from_arc(Arc::<[u8]>::from(section.as_slice()))) + } + + fn build_origin_backed_default_location_entry_value_fixture( + origin_attr: gimli::DwAt, + ) -> gimli::Dwarf { + let encoding = gimli::Encoding { + format: Format::Dwarf32, + version: 5, + address_size: 8, + }; + + let mut dwarf = WriteDwarf::new(); + let unit_id = dwarf.units.add(Unit::new(encoding, LineProgram::none())); + let unit = dwarf.units.get_mut(unit_id); + let root = unit.root(); + + let origin_id = unit.add(root, constants::DW_TAG_subprogram); + unit.get_mut(origin_id).set( + constants::DW_AT_name, + WriteAttributeValue::String(b"entry_value_default_location_target".to_vec()), + ); + + let origin_param_id = unit.add(origin_id, constants::DW_TAG_formal_parameter); + let mut direct_loc = WriteExpression::new(); + direct_loc.op_reg(Register(5)); + let mut inner = WriteExpression::new(); + inner.op_reg(Register(5)); + let mut default_entry_value_loc = WriteExpression::new(); + default_entry_value_loc.op_entry_value(inner); + let loc_id = unit.locations.add(LocationList(vec![ + Location::DefaultLocation { + data: default_entry_value_loc, + }, + Location::StartEnd { + begin: Address::Constant(0x1470), + end: Address::Constant(0x147b), + data: direct_loc, + }, + ])); + unit.get_mut(origin_param_id).set( + constants::DW_AT_location, + WriteAttributeValue::LocationListRef(loc_id), + ); + + let concrete_id = unit.add(root, constants::DW_TAG_subprogram); + unit.get_mut(concrete_id) + .set(origin_attr, WriteAttributeValue::UnitRef(origin_id)); + + let mut sections = Sections::new(EndianVec::new(gimli::LittleEndian)); + dwarf.write(&mut sections).unwrap(); + + let dwarf_sections: gimli::DwarfSections> = gimli::DwarfSections::load(|id| { + Ok::<_, gimli::Error>( + sections + .get(id) + .map(|section| section.slice().to_vec()) + .unwrap_or_default(), + ) + }) + .unwrap(); + + dwarf_sections + .borrow(|section| dwarf_reader_from_arc(Arc::<[u8]>::from(section.as_slice()))) + } + + fn first_unit(dwarf: &gimli::Dwarf) -> gimli::Unit { + let mut units = dwarf.units(); + let header = units.next().unwrap().unwrap(); + dwarf.unit(header).unwrap() + } + + fn find_subprogram_with_origin_attr( + unit: &gimli::Unit, + origin_attr: gimli::DwAt, + ) -> gimli::UnitOffset { + let mut tree = unit.entries_tree(None).unwrap(); + let root = tree.root().unwrap(); + let mut children = root.children(); + + while let Some(child) = children.next().unwrap() { + let entry = child.entry(); + if entry.tag() == constants::DW_TAG_subprogram + && entry.attr_value(origin_attr).is_some() + { + return entry.offset(); + } } + + panic!("failed to find subprogram with origin attr {origin_attr:?}"); } #[test] @@ -2482,4 +3089,258 @@ mod tests { vec![(0x100, 0x110)], ); } + + #[test] + fn selected_non_inline_probe_address_clamps_prologue_skip_to_function_range() { + // Regression scenario: + // The hot/cold fix intentionally prologue-skips non-inline functions by + // asking the line table for the first executable PC after range start. + // On optimized binaries the line table can sometimes return a PC from + // the next function entirely. If we trust that blindly, function-level + // tracing attaches to a sibling symbol and silently misses the target. + // + // This locks in the clamp: a candidate outside [start, end) must fall + // back to the function's own start, while an in-range candidate is kept. + assert_eq!( + ModuleData::selected_non_inline_probe_address(0x1470, 0x147b, 0x14f2), + 0x1470 + ); + assert_eq!( + ModuleData::selected_non_inline_probe_address(0x1470, 0x147b, 0x1474), + 0x1474 + ); + } + + #[test] + fn selected_inline_address_prefers_entry_pc_over_cold_min_range() { + // Regression scenario: + // A DW_TAG_inlined_subroutine can carry multiple ranges, including a + // lower-address cold fragment. Using min(range.start) would incorrectly + // place the probe on the cold block even when DW_AT_entry_pc points at + // the real hot entry into the inlined body. + // + // This test mirrors the CGPsend shape and ensures entry_pc wins. + let entry = inline_entry( + &[ + (0x8eb12b, 0x8eb139), + (0x8eb150, 0x8eb157), + (0x8eb16a, 0x8eb1b0), + (0x76e798, 0x76e7a2), + ], + Some(0x8eb16a), + ); + + assert_eq!(ModuleData::selected_inline_address(&entry), Some(0x8eb16a)); + } + + #[test] + fn selected_inline_address_without_entry_pc_keeps_first_emitted_hot_range() { + // Regression scenario: + // Some inline DIEs omit DW_AT_entry_pc but still emit ranges in + // compiler order, with the hot fragment first and a lower-address cold + // fragment later. Sorting and taking min(range.start) reintroduces the + // same cold-placement bug. + // + // This keeps the fallback policy stable: if entry_pc is missing, prefer + // the first emitted range before considering a pure minimum. + let entry = inline_entry( + &[ + (0x8eb12b, 0x8eb139), + (0x8eb150, 0x8eb157), + (0x76e798, 0x76e7a2), + ], + None, + ); + + assert_eq!(ModuleData::selected_inline_address(&entry), Some(0x8eb12b)); + } + + #[test] + fn selected_inline_address_ignores_entry_pc_outside_inline_ranges() { + // Regression scenario: + // Some optimized GCC builds emit an inlined_subroutine whose + // DW_AT_entry_pc points at the caller-side setup block, while the + // inline DIE's own ranges only cover the actual inlined body. Trusting + // entry_pc unconditionally selects a PC where the inline parameters are + // not in scope and later eBPF compilation fails with "Variable not in + // scope". + // + // This mirrors the container CI failure shape: entry_pc=0x1215 but the + // inline instance itself only covers [0x1289, 0x1293). A correct + // selection must stay inside the inline DIE's own ranges. + let entry = inline_entry(&[(0x1289, 0x1293)], Some(0x1215)); + + assert_eq!(ModuleData::selected_inline_address(&entry), Some(0x1289)); + } + + #[test] + fn selected_inline_address_keeps_entry_pc_only_point_scopes() { + // Regression scenario: + // Some inline/call-site DIEs are encoded as a single point with only + // DW_AT_entry_pc and no ranges at all. Those scopes are still + // addressable elsewhere in the DWARF pipeline, so inline address + // selection must not drop them just because there is no range to + // validate against. + let entry = inline_entry(&[], Some(0x1289)); + + assert_eq!(ModuleData::selected_inline_address(&entry), Some(0x1289)); + } + + #[test] + fn subprogram_uses_entry_value_via_abstract_origin_parameters() { + // Regression scenario: + // After concrete out-of-line subprograms stopped inheriting is_inline, + // they began using the non-inline address path again. That path needs to + // know when parameter recovery depends on DW_OP_entry_value so it can + // preserve the true entry PC instead of prologue-skipping. + // + // Optimized DWARF may place the formal parameters only on the abstract + // origin, with the concrete subprogram inheriting them via + // DW_AT_abstract_origin. This test ensures that inherited parameter DIEs + // are still consulted for entry_value detection. + let dwarf = build_origin_backed_entry_value_fixture(constants::DW_AT_abstract_origin); + let unit = first_unit(&dwarf); + let concrete_offset = + find_subprogram_with_origin_attr(&unit, constants::DW_AT_abstract_origin); + let concrete = unit.entry(concrete_offset).unwrap(); + + assert_eq!( + ModuleData::direct_formal_parameters_entry_value_state(&unit, &concrete).unwrap(), + None, + "concrete DIE should not expose direct parameter children in this fixture" + ); + assert!( + ModuleData::subprogram_uses_entry_value(&dwarf, &unit, &concrete).unwrap(), + "entry_value should be discovered through DW_AT_abstract_origin" + ); + } + + #[test] + fn subprogram_uses_entry_value_via_specification_parameters() { + // Same as the abstract-origin case above, but for compilers that route + // concrete subprograms through DW_AT_specification instead. Both origin + // chains must preserve entry_value-driven entry selection. + let dwarf = build_origin_backed_entry_value_fixture(constants::DW_AT_specification); + let unit = first_unit(&dwarf); + let concrete_offset = + find_subprogram_with_origin_attr(&unit, constants::DW_AT_specification); + let concrete = unit.entry(concrete_offset).unwrap(); + + assert_eq!( + ModuleData::direct_formal_parameters_entry_value_state(&unit, &concrete).unwrap(), + None, + "concrete DIE should not expose direct parameter children in this fixture" + ); + assert!( + ModuleData::subprogram_uses_entry_value(&dwarf, &unit, &concrete).unwrap(), + "entry_value should be discovered through DW_AT_specification" + ); + } + + #[test] + fn subprogram_uses_entry_value_does_not_override_concrete_parameter_locations() { + // Regression scenario: + // A concrete optimized subprogram may have its own formal_parameter + // children that override the abstract origin's DW_AT_location, often via + // DW_AT_abstract_origin on the parameter DIE itself. In that shape, the + // concrete child is authoritative and origin-level entry_value must not + // force prefer_entry=true. + // + // This test ensures direct concrete parameter locations win over the + // origin's location expression. + let dwarf = + build_origin_backed_entry_value_override_fixture(constants::DW_AT_abstract_origin); + let unit = first_unit(&dwarf); + let concrete_offset = + find_subprogram_with_origin_attr(&unit, constants::DW_AT_abstract_origin); + let concrete = unit.entry(concrete_offset).unwrap(); + + assert_eq!( + ModuleData::direct_formal_parameters_entry_value_state(&unit, &concrete).unwrap(), + Some(false), + "concrete DIE should treat its own parameter children as authoritative" + ); + assert!( + !ModuleData::subprogram_uses_entry_value(&dwarf, &unit, &concrete).unwrap(), + "origin-level entry_value must not override concrete parameter locations" + ); + } + + #[test] + fn subprogram_uses_entry_value_at_pc_only_when_active_location_uses_it() { + // Regression scenario: + // The original entry_value check was too coarse: if any loclist segment + // used DW_OP_entry_value anywhere in the function, we forced the probe + // back to the raw entry. That breaks functions like optimized + // calculate_something, where the first executable instruction still has + // direct register locations and entry_value only appears later. + // + // This test builds that exact shape in miniature and verifies the new + // rule: only the location expression active at the candidate probe PC + // may trigger prefer_entry=true. + let dwarf = build_origin_backed_entry_value_range_fixture(constants::DW_AT_abstract_origin); + let unit = first_unit(&dwarf); + let concrete_offset = + find_subprogram_with_origin_attr(&unit, constants::DW_AT_abstract_origin); + let concrete = unit.entry(concrete_offset).unwrap(); + + assert!( + !ModuleData::subprogram_uses_entry_value_at(&dwarf, &unit, &concrete, 0x1474).unwrap(), + "entry_value should not force the true entry while the active location is still a direct register" + ); + assert!( + ModuleData::subprogram_uses_entry_value_at(&dwarf, &unit, &concrete, 0x1478).unwrap(), + "entry_value should still be detected once the active location range switches to it" + ); + } + + #[test] + fn subprogram_uses_entry_value_at_pc_respects_concrete_parameter_overrides() { + // This complements the test above: even with PC-sensitive loclist + // evaluation, we must still honor concrete parameter overrides before + // walking up to abstract origins/specifications. Otherwise an origin + // loclist with entry_value could reclassify a concrete out-of-line body + // that already exposed usable direct-register parameter locations. + let dwarf = + build_origin_backed_entry_value_override_fixture(constants::DW_AT_abstract_origin); + let unit = first_unit(&dwarf); + let concrete_offset = + find_subprogram_with_origin_attr(&unit, constants::DW_AT_abstract_origin); + let concrete = unit.entry(concrete_offset).unwrap(); + + assert!( + !ModuleData::subprogram_uses_entry_value_at(&dwarf, &unit, &concrete, 0x1478).unwrap(), + "concrete parameter locations must remain authoritative at the selected probe PC" + ); + } + + #[test] + fn subprogram_uses_entry_value_at_pc_prefers_specific_loclist_ranges_over_default_location() { + // Regression scenario: + // DWARF5 loclists may start with DW_LLE_default_location and then + // override it with a later range-specific entry. gimli::locations() + // normalizes the default to [0, u64::MAX), which can mask the later + // specific range and incorrectly report entry_value everywhere. + // + // This fixture keeps entry_value in the default location but switches + // to a direct register location for [0x1470, 0x147b). The specific + // range must win at PCs inside that span, while the default still + // applies outside it. + let dwarf = build_origin_backed_default_location_entry_value_fixture( + constants::DW_AT_abstract_origin, + ); + let unit = first_unit(&dwarf); + let concrete_offset = + find_subprogram_with_origin_attr(&unit, constants::DW_AT_abstract_origin); + let concrete = unit.entry(concrete_offset).unwrap(); + + assert!( + !ModuleData::subprogram_uses_entry_value_at(&dwarf, &unit, &concrete, 0x1474).unwrap(), + "the specific direct-register range should override the default-location entry_value" + ); + assert!( + ModuleData::subprogram_uses_entry_value_at(&dwarf, &unit, &concrete, 0x1500).unwrap(), + "outside the specific range, the default-location entry_value should still apply" + ); + } } diff --git a/ghostscope-dwarf/src/parser/fast_parser.rs b/ghostscope-dwarf/src/parser/fast_parser.rs index b41f577a..29291dbe 100644 --- a/ghostscope-dwarf/src/parser/fast_parser.rs +++ b/ghostscope-dwarf/src/parser/fast_parser.rs @@ -5,7 +5,7 @@ use crate::{ binary::DwarfReader, core::{ demangle::{demangle_by_lang, demangled_leaf}, - IndexEntry, Result, + FunctionDieKind, IndexEntry, Result, }, index::{ directory_from_index, resolve_file_path, LightweightFileIndex, LightweightIndex, @@ -21,11 +21,23 @@ use tracing::debug; #[derive(Clone, Default)] struct FunctionMetadata { name: Option, - is_inline: bool, + has_inline_attribute: bool, is_linkage_name: bool, is_external: Option, } +#[derive(Clone)] +struct FunctionEntrySeed { + die_offset: gimli::UnitOffset, + tag: gimli::DwTag, + unit_offset: gimli::DebugInfoOffset, + flags: crate::core::IndexFlags, + language: Option, + address_ranges: Vec<(u64, u64)>, + entry_pc: Option, + function_kind: FunctionDieKind, +} + /// Compilation unit information with associated directories and files. #[derive(Debug, Clone)] pub(crate) struct CompilationUnit { @@ -132,6 +144,14 @@ impl<'a> DwarfParser<'a> { &mut visited, )?; if let Some(name) = metadata.name.clone() { + let address_ranges = + self.extract_address_ranges(self.dwarf, unit, entry)?; + let entry_pc_cached = self.extract_entry_pc(entry)?; + let function_kind = Self::classify_function_kind( + entry.tag(), + &address_ranges, + entry_pc_cached, + ); let is_main = self.is_main_function(entry, &name).unwrap_or(false); let is_static = metadata .is_external @@ -140,51 +160,30 @@ impl<'a> DwarfParser<'a> { let flags = crate::core::IndexFlags { is_static, is_main, - is_inline: metadata.is_inline, + is_inline_instance: function_kind == FunctionDieKind::InlineInstance, + has_inline_attribute: metadata.has_inline_attribute, is_linkage: metadata.is_linkage_name, ..Default::default() }; - let address_ranges = - self.extract_address_ranges(self.dwarf, unit, entry)?; - let entry_pc_cached = self.extract_entry_pc(entry)?; - let index_entry = IndexEntry { - name: std::sync::Arc::from(name.as_str()), + let linkage_name = self + .extract_linkage_name(self.dwarf, unit, entry)? + .map(|(linkage_name, _)| linkage_name); + let seed = FunctionEntrySeed { die_offset: entry.offset(), - unit_offset, tag: entry.tag(), + unit_offset, flags, language: cu_language, - address_ranges: address_ranges.clone(), + address_ranges, entry_pc: entry_pc_cached, + function_kind, }; - shard - .functions - .entry(name.clone()) - .or_default() - .push(index_entry); - if let Some((linkage_name, _)) = - self.extract_linkage_name(self.dwarf, unit, entry)? - { - if linkage_name != metadata.name.clone().unwrap_or_default() { - let mut alias_flags = flags; - alias_flags.is_linkage = true; - let index_entry_linkage = IndexEntry { - name: std::sync::Arc::from(linkage_name.as_str()), - die_offset: entry.offset(), - unit_offset, - tag: entry.tag(), - flags: alias_flags, - language: cu_language, - address_ranges: address_ranges.clone(), - entry_pc: entry_pc_cached, - }; - shard - .functions - .entry(linkage_name) - .or_default() - .push(index_entry_linkage); - } - } + Self::push_function_entries( + &mut shard.functions, + &name, + linkage_name, + &seed, + ); } } gimli::constants::DW_TAG_inlined_subroutine => { @@ -197,57 +196,44 @@ impl<'a> DwarfParser<'a> { &mut visited, )?; if let Some(name) = metadata.name.clone() { + let address_ranges = + self.extract_address_ranges(self.dwarf, unit, entry)?; + let entry_pc_cached = self.extract_entry_pc(entry)?; + let function_kind = Self::classify_function_kind( + entry.tag(), + &address_ranges, + entry_pc_cached, + ); let is_static = metadata .is_external .map(|external| !external) .unwrap_or(false); let flags = crate::core::IndexFlags { is_static, - is_inline: true, + is_inline_instance: function_kind == FunctionDieKind::InlineInstance, + has_inline_attribute: metadata.has_inline_attribute, is_linkage: metadata.is_linkage_name, ..Default::default() }; - let address_ranges = - self.extract_address_ranges(self.dwarf, unit, entry)?; - let entry_pc_cached = self.extract_entry_pc(entry)?; - let index_entry = IndexEntry { - name: std::sync::Arc::from(name.as_str()), + let linkage_name = self + .extract_linkage_name(self.dwarf, unit, entry)? + .map(|(linkage_name, _)| linkage_name); + let seed = FunctionEntrySeed { die_offset: entry.offset(), - unit_offset, tag: entry.tag(), + unit_offset, flags, language: cu_language, - address_ranges: address_ranges.clone(), + address_ranges, entry_pc: entry_pc_cached, + function_kind, }; - shard - .functions - .entry(name.clone()) - .or_default() - .push(index_entry); - if let Some((linkage_name, _)) = - self.extract_linkage_name(self.dwarf, unit, entry)? - { - if linkage_name != metadata.name.clone().unwrap_or_default() { - let mut alias_flags = flags; - alias_flags.is_linkage = true; - let index_entry_linkage = IndexEntry { - name: std::sync::Arc::from(linkage_name.as_str()), - die_offset: entry.offset(), - unit_offset, - tag: entry.tag(), - flags: alias_flags, - language: cu_language, - address_ranges: address_ranges.clone(), - entry_pc: entry_pc_cached, - }; - shard - .functions - .entry(linkage_name) - .or_default() - .push(index_entry_linkage); - } - } + Self::push_function_entries( + &mut shard.functions, + &name, + linkage_name, + &seed, + ); } } gimli::constants::DW_TAG_variable => { @@ -353,6 +339,7 @@ impl<'a> DwarfParser<'a> { language: cu_language, address_ranges: var_ranges.clone(), entry_pc: None, + function_kind: FunctionDieKind::NotFunction, }; tracing::trace!( "Registering variable alias '{}' (linkage={}, lang={:?}, die={:?})", @@ -389,6 +376,7 @@ impl<'a> DwarfParser<'a> { language: cu_language, address_ranges: Vec::new(), entry_pc: None, + function_kind: FunctionDieKind::NotFunction, }; shard.types.entry(name).or_default().push(index_entry); } @@ -403,6 +391,61 @@ impl<'a> DwarfParser<'a> { Self { dwarf } } + fn classify_function_kind( + tag: gimli::DwTag, + address_ranges: &[(u64, u64)], + entry_pc: Option, + ) -> FunctionDieKind { + match tag { + gimli::constants::DW_TAG_inlined_subroutine => FunctionDieKind::InlineInstance, + gimli::constants::DW_TAG_subprogram => { + if !address_ranges.is_empty() || entry_pc.is_some() { + FunctionDieKind::ConcreteSubprogram + } else { + FunctionDieKind::AbstractSubprogram + } + } + _ => FunctionDieKind::NotFunction, + } + } + + fn build_function_index_entry(name: &str, seed: &FunctionEntrySeed) -> IndexEntry { + IndexEntry { + name: std::sync::Arc::from(name), + die_offset: seed.die_offset, + unit_offset: seed.unit_offset, + tag: seed.tag, + flags: seed.flags, + language: seed.language, + address_ranges: seed.address_ranges.clone(), + entry_pc: seed.entry_pc, + function_kind: seed.function_kind, + } + } + + fn push_function_entries( + functions: &mut HashMap>, + name: &str, + linkage_name: Option, + seed: &FunctionEntrySeed, + ) { + functions + .entry(name.to_owned()) + .or_default() + .push(Self::build_function_index_entry(name, seed)); + + if let Some(linkage_name) = linkage_name.filter(|linkage_name| linkage_name != name) { + let mut alias_seed = seed.clone(); + let mut alias_flags = alias_seed.flags; + alias_flags.is_linkage = true; + alias_seed.flags = alias_flags; + functions + .entry(linkage_name.clone()) + .or_default() + .push(Self::build_function_index_entry(&linkage_name, &alias_seed)); + } + } + fn extract_attr_string( dwarf: &gimli::Dwarf, unit: &gimli::Unit, @@ -446,7 +489,9 @@ impl<'a> DwarfParser<'a> { Ok(None) } - fn extract_inline_flag(entry: &gimli::DebuggingInformationEntry) -> Result { + fn extract_inline_attribute( + entry: &gimli::DebuggingInformationEntry, + ) -> Result { if let Some(attr) = entry.attr(gimli::constants::DW_AT_inline) { if let gimli::AttributeValue::Inline(inline_attr) = attr.value() { return Ok(inline_attr == gimli::DW_INL_inlined @@ -481,7 +526,7 @@ impl<'a> DwarfParser<'a> { metadata.is_linkage_name = is_linkage; } - metadata.is_inline = Self::extract_inline_flag(entry)?; + metadata.has_inline_attribute = Self::extract_inline_attribute(entry)?; metadata.is_external = Self::bool_attr(entry, gimli::constants::DW_AT_external)?; @@ -497,7 +542,6 @@ impl<'a> DwarfParser<'a> { if metadata.name.is_none() { metadata.name = origin_metadata.name.clone(); } - metadata.is_inline |= origin_metadata.is_inline; if metadata.is_external.is_none() { metadata.is_external = origin_metadata.is_external; } @@ -1240,6 +1284,71 @@ mod tests { .borrow(|section| dwarf_reader_from_arc(Arc::<[u8]>::from(section.as_slice()))) } + fn build_inline_origin_fixture() -> gimli::Dwarf { + let encoding = gimli::Encoding { + format: Format::Dwarf32, + version: 4, + address_size: 8, + }; + + let mut dwarf = WriteDwarf::new(); + let unit_id = dwarf.units.add(Unit::new(encoding, LineProgram::none())); + let unit = dwarf.units.get_mut(unit_id); + let root = unit.root(); + + let abstract_id = unit.add(root, gimli::constants::DW_TAG_subprogram); + let abstract_fn = unit.get_mut(abstract_id); + abstract_fn.set( + gimli::constants::DW_AT_name, + WriteAttributeValue::String(b"CGPsend".to_vec()), + ); + abstract_fn.set( + gimli::constants::DW_AT_inline, + WriteAttributeValue::Inline(gimli::DW_INL_inlined), + ); + abstract_fn.set( + gimli::constants::DW_AT_external, + WriteAttributeValue::Flag(true), + ); + + let concrete_id = unit.add(root, gimli::constants::DW_TAG_subprogram); + let concrete_fn = unit.get_mut(concrete_id); + concrete_fn.set( + gimli::constants::DW_AT_abstract_origin, + WriteAttributeValue::UnitRef(abstract_id), + ); + concrete_fn.set( + gimli::constants::DW_AT_low_pc, + WriteAttributeValue::Address(Address::Constant(0x8e97c0)), + ); + concrete_fn.set( + gimli::constants::DW_AT_high_pc, + WriteAttributeValue::Udata(0x420), + ); + + let inlined_id = unit.add(root, gimli::constants::DW_TAG_inlined_subroutine); + unit.get_mut(inlined_id).set( + gimli::constants::DW_AT_abstract_origin, + WriteAttributeValue::UnitRef(abstract_id), + ); + + let mut sections = Sections::new(EndianVec::new(LittleEndian)); + dwarf.write(&mut sections).unwrap(); + + let dwarf_sections: gimli::DwarfSections> = gimli::DwarfSections::load(|id| { + Ok::<_, gimli::Error>( + sections + .get(id) + .map(|section| section.slice().to_vec()) + .unwrap_or_default(), + ) + }) + .unwrap(); + + dwarf_sections + .borrow(|section| dwarf_reader_from_arc(Arc::<[u8]>::from(section.as_slice()))) + } + #[test] fn parse_debug_info_skips_stack_value_address_locals_from_global_index() { let dwarf = build_variable_index_fixture(); @@ -1269,4 +1378,80 @@ mod tests { "address-valued optimized local must not be indexed as a global: {optimized_local:?}" ); } + + #[test] + fn parse_debug_info_keeps_concrete_abstract_origin_subprogram_non_inline() { + // Regression scenario: + // GCC/Clang can emit all three DIE shapes for one logical function: + // 1. an abstract DW_TAG_subprogram marked DW_AT_inline, + // 2. a concrete out-of-line DW_TAG_subprogram with DW_AT_abstract_origin, + // 3. one or more DW_TAG_inlined_subroutine instances. + // + // The bug was that merge_from_origin copied the abstract function's + // inline attribute from (1) onto (2) and downstream code treated that + // as if the concrete body were an inline instance. + // Once that happened, the concrete body was routed through the inline + // address-selection path and could pick the wrong cold-partition PC. + // + // This test keeps the synthetic DIE graph minimal and asserts the parser + // preserves the intended split: + // - abstract definition stays inline + // - concrete out-of-line body stays non-inline + // - inlined_subroutine instance stays inline + let dwarf = build_inline_origin_fixture(); + let parser = DwarfParser { dwarf: &dwarf }; + + let result = parser.parse_debug_info("synthetic").unwrap(); + let entries = result + .lightweight_index + .find_dies_by_function_name("CGPsend"); + + let concrete_entries: Vec<_> = entries + .iter() + .copied() + .filter(|entry| { + entry.function_kind() == crate::core::FunctionDieKind::ConcreteSubprogram + }) + .collect(); + let abstract_entries: Vec<_> = entries + .iter() + .copied() + .filter(|entry| { + entry.function_kind() == crate::core::FunctionDieKind::AbstractSubprogram + }) + .collect(); + let inlined_entries: Vec<_> = entries + .iter() + .copied() + .filter(|entry| entry.function_kind() == crate::core::FunctionDieKind::InlineInstance) + .collect(); + + assert_eq!( + concrete_entries.len(), + 1, + "concrete out-of-line subprogram should stay non-inline: {entries:?}" + ); + assert_eq!( + abstract_entries.len(), + 1, + "only the abstract inline definition should carry the inline flag: {entries:?}" + ); + assert_eq!( + inlined_entries.len(), + 1, + "expected one inlined subroutine instance: {entries:?}" + ); + assert!( + inlined_entries[0].is_inline_instance(), + "DW_TAG_inlined_subroutine must remain an inline instance: {entries:?}" + ); + assert!( + !concrete_entries[0].flags.has_inline_attribute, + "concrete out-of-line body should not inherit the abstract inline attribute: {entries:?}" + ); + assert!( + abstract_entries[0].flags.has_inline_attribute, + "abstract definition should retain its original DW_AT_inline attribute: {entries:?}" + ); + } } diff --git a/ghostscope-dwarf/src/planner.rs b/ghostscope-dwarf/src/planner.rs index ebfe36ad..32c91ddc 100644 --- a/ghostscope-dwarf/src/planner.rs +++ b/ghostscope-dwarf/src/planner.rs @@ -365,7 +365,7 @@ impl<'dwarf> AccessPlanner<'dwarf> { mod tests { use super::*; use crate::binary::dwarf_reader_from_arc; - use crate::core::{IndexEntry, IndexFlags}; + use crate::core::{FunctionDieKind, IndexEntry, IndexFlags}; use crate::index::{LightweightIndex, TypeNameIndex}; use gimli::constants; use gimli::write::{ @@ -493,6 +493,7 @@ mod tests { language: None, address_ranges: Vec::new(), entry_pc: None, + function_kind: FunctionDieKind::NotFunction, }], ); let type_index = Arc::new(TypeNameIndex::build_from_lightweight( @@ -608,6 +609,7 @@ mod tests { language: None, address_ranges: Vec::new(), entry_pc: None, + function_kind: FunctionDieKind::NotFunction, }], ); let type_index = Arc::new(TypeNameIndex::build_from_lightweight(