diff --git a/e2e-tests/tests/cpp_script_execution.rs b/e2e-tests/tests/cpp_script_execution.rs index 806be034..766ad7d6 100644 --- a/e2e-tests/tests/cpp_script_execution.rs +++ b/e2e-tests/tests/cpp_script_execution.rs @@ -6,6 +6,8 @@ use common::{init, FIXTURES}; const CPP_NESTED_MEMBER_TRACE_LINE: u32 = 44; const CPP_SIBLING_BLOCK_MEMBER_TRACE_LINE: u32 = 72; +const CPP_INHERITED_MEMBER_TRACE_LINE: u32 = 81; +const CPP_AMBIGUOUS_INHERITED_MEMBER_TRACE_LINE: u32 = 90; async fn compile_cpp_complex_script( script: &str, @@ -129,6 +131,90 @@ trace {}:{CPP_SIBLING_BLOCK_MEMBER_TRACE_LINE} {{ Ok(()) } +#[tokio::test] +async fn test_cpp_inherited_member_access() -> anyhow::Result<()> { + init(); + + let binary_path = FIXTURES.get_test_binary("cpp_complex_program")?; + let source_path = binary_path + .parent() + .ok_or_else(|| anyhow::anyhow!("cpp_complex_program has no parent directory"))? + .join("main.cpp"); + let script = format!( + r#" +trace {}:{CPP_INHERITED_MEMBER_TRACE_LINE} {{ + print "INHERITED:{{}}", d.inherited; +}} +"#, + source_path.display() + ); + + let compiled = compile_cpp_complex_script(&script).await?; + assert!( + !compiled.uprobe_configs.is_empty(), + "expected inherited d.inherited access to compile; target_info={} failed_targets={:?}", + compiled.target_info, + compiled.failed_targets + ); + + let target = spawn_cpp_complex_program().await?; + let (exit_code, stdout, stderr) = + run_ghostscope_with_script_for_target(&script, 4, &target).await?; + target.terminate().await?; + assert_eq!(exit_code, 0, "stderr={stderr} stdout={stdout}"); + assert!( + stdout.contains("INHERITED:222"), + "Expected inherited base member value. STDOUT: {stdout}" + ); + + Ok(()) +} + +#[tokio::test] +async fn test_cpp_ambiguous_inherited_member_access_is_rejected() -> anyhow::Result<()> { + init(); + + let binary_path = FIXTURES.get_test_binary("cpp_complex_program")?; + let source_path = binary_path + .parent() + .ok_or_else(|| anyhow::anyhow!("cpp_complex_program has no parent directory"))? + .join("main.cpp"); + let script = format!( + r#" +trace {}:{CPP_AMBIGUOUS_INHERITED_MEMBER_TRACE_LINE} {{ + print d.conflict; +}} +"#, + source_path.display() + ); + + let message = match compile_cpp_complex_script(&script).await { + Ok(compiled) => { + assert!( + compiled.uprobe_configs.is_empty(), + "ambiguous inherited member access should not compile: {compiled:?}" + ); + assert!( + !compiled.failed_targets.is_empty(), + "ambiguous inherited member access should report a failed target: {compiled:?}" + ); + compiled + .failed_targets + .iter() + .map(|target| target.error_message.as_str()) + .collect::>() + .join("\n") + } + Err(error) => error.to_string(), + }; + assert!( + message.contains("Ambiguous member 'conflict'"), + "unexpected compile error: {message}" + ); + + Ok(()) +} + #[tokio::test] async fn test_cpp_script_print_globals() -> anyhow::Result<()> { init(); diff --git a/e2e-tests/tests/dwarf_index_regressions.rs b/e2e-tests/tests/dwarf_index_regressions.rs index 829811a2..ea78088a 100644 --- a/e2e-tests/tests/dwarf_index_regressions.rs +++ b/e2e-tests/tests/dwarf_index_regressions.rs @@ -117,6 +117,36 @@ fn assert_native_index_queries( Ok(()) } +fn assert_type_unit_inherited_member_queries( + analyzer: &ghostscope_dwarf::DwarfAnalyzer, + target: &Path, + producer: &str, +) -> anyhow::Result<()> { + let derived = analyzer + .resolve_c_style_semantic_type_spec_in_module(target, "Derived") + .with_context(|| format!("{producer} type units did not resolve Derived"))?; + let layout = ghostscope_dwarf::member_layout(&derived.summary, "inherited") + .with_context(|| format!("{producer} type units dropped Base::inherited"))?; + assert_eq!(layout.offset, 0, "unexpected {producer} inherited offset"); + + let projection = analyzer.project_resolved_type( + &derived, + &ghostscope_dwarf::VariableAccessSegment::Field("inherited".to_string()), + Some(target), + )?; + assert_eq!( + projection.layout, + ghostscope_dwarf::TypeProjectionLayout::Member { offset: 0 }, + "unexpected {producer} inherited projection" + ); + assert_eq!( + projection.resolved_type.summary.type_name(), + "int", + "unexpected {producer} inherited member type" + ); + Ok(()) +} + async fn spawn_inline_callsite_program( binary_path: &Path, ) -> anyhow::Result { @@ -905,6 +935,11 @@ async fn test_gdb_index_resolves_function_type_and_global_lazily() -> anyhow::Re .resolve_struct_type_shallow_by_name("Outer") .context(".debug_names did not resolve Outer from its type unit")?; assert_eq!(outer_type.size(), 16, "unexpected Outer size"); + assert_type_unit_inherited_member_queries( + &debug_names_type_analyzer, + &debug_names_type_units, + "clang++", + )?; let gdb_type_units = temp_dir.path().join("cpp-complex.gdb-type-units"); fs::copy(&debug_names_type_units, &gdb_type_units)?; @@ -936,6 +971,29 @@ async fn test_gdb_index_resolves_function_type_and_global_lazily() -> anyhow::Re } } + if command_available("g++") { + let cpp_source = Path::new(env!("CARGO_MANIFEST_DIR")) + .join("tests/fixtures/cpp_complex_program/main.cpp"); + let gcc_type_units = temp_dir.path().join("cpp-complex.gcc-type-units"); + run_command( + StdCommand::new("g++") + .arg("-gdwarf-5") + .arg("-fdebug-types-section") + .arg("-O0") + .arg(&cpp_source) + .arg("-o") + .arg(&gcc_type_units), + "g++ type-unit build", + )?; + anyhow::ensure!( + dwarf_has_type_unit(&gcc_type_units)?, + "g++ did not emit a DWARF5 type unit" + ); + let gcc_type_analyzer = + ghostscope_dwarf::DwarfAnalyzer::from_exec_path(&gcc_type_units).await?; + assert_type_unit_inherited_member_queries(&gcc_type_analyzer, &gcc_type_units, "g++")?; + } + if command_available("gcc") && command_available("objcopy") { let fixture_dir = source .parent() diff --git a/e2e-tests/tests/fixtures/cpp_complex_program/main.cpp b/e2e-tests/tests/fixtures/cpp_complex_program/main.cpp index 5d4309f3..e61e28bb 100644 --- a/e2e-tests/tests/fixtures/cpp_complex_program/main.cpp +++ b/e2e-tests/tests/fixtures/cpp_complex_program/main.cpp @@ -74,6 +74,23 @@ __attribute__((noinline)) int sibling_block_member_probe(int x) { return block_path_sink; } +struct Base { int inherited; }; +struct Derived : Base { int own; }; + +__attribute__((noinline)) int inherited_member_probe(Derived* d) { + volatile int sink = d->inherited + d->own; + return sink; +} + +struct LeftBase { int conflict; }; +struct RightBase { int conflict; }; +struct AmbiguousDerived : LeftBase, RightBase {}; + +__attribute__((noinline)) int ambiguous_inherited_member_probe(AmbiguousDerived* d) { + volatile int sink = d->LeftBase::conflict + d->RightBase::conflict; + return sink; +} + // Variables purposely ending with ::h and ::h264 to validate demangled leaf handling int h = 5; int h264 = 7; @@ -87,6 +104,12 @@ static void touch_globals() { int main() { ns1::Foo f; + ns1::Derived derived; + derived.inherited = 222; + derived.own = 333; + ns1::AmbiguousDerived ambiguous; + ambiguous.LeftBase::conflict = 444; + ambiguous.RightBase::conflict = 555; int acc = 0; for (int i = 0; i < 50000; ++i) { acc += f.bar(i); @@ -94,6 +117,8 @@ int main() { acc += ns1::add(1.5, 2.5); acc += ns1::nested_member_probe(i); acc += ns1::sibling_block_member_probe(i); + acc += ns1::inherited_member_probe(&derived); + acc += ns1::ambiguous_inherited_member_probe(&ambiguous); touch_globals(); std::this_thread::sleep_for(std::chrono::milliseconds(1000)); } diff --git a/ghostscope-dwarf/src/objfile/type_context.rs b/ghostscope-dwarf/src/objfile/type_context.rs index 61779c2f..84b981c7 100644 --- a/ghostscope-dwarf/src/objfile/type_context.rs +++ b/ghostscope-dwarf/src/objfile/type_context.rs @@ -4,7 +4,8 @@ use super::{variables::complete_aggregate_declaration_entry, LoadedObjfile}; use crate::{ core::Result, semantics::{ - resolve_attr_with_unit_origins, resolve_name_with_origins, resolve_type_ref_with_origins, + resolve_attr_with_unit_origins, resolve_name_with_origins, resolve_type_definition_loc, + resolve_type_definition_ref_with_origins, resolve_type_ref_with_origins, CompilationUnitMetadata, ProducerInfo, SourceLanguage, TypeLoc, VariableAccessSegment, }, CuId, ModuleId, TypeId, @@ -123,6 +124,10 @@ fn normalize_type_loc( ) -> Result> { let mut visited = HashSet::new(); for _ in 0..MAX_TYPE_REFERENCE_DEPTH { + let Some(definition_loc) = resolve_type_definition_loc(dwarf, loc)? else { + return Ok(None); + }; + loc = definition_loc; if !visited.insert((loc.cu_off.0, loc.die_off.0)) { return Err(anyhow::anyhow!( "cycle while resolving type DIE at {:?}:{:?}", @@ -198,6 +203,35 @@ fn projected_member_type_loc( }; } + projected_aggregate_member_type_loc( + dwarf, + type_name_index, + aggregate_loc, + field, + &HashSet::new(), + 0, + ) +} + +fn projected_aggregate_member_type_loc( + dwarf: &gimli::Dwarf, + type_name_index: &crate::index::TypeNameIndex, + loc: TypeLoc, + field: &str, + visited: &HashSet<(usize, usize)>, + inheritance_depth: usize, +) -> Result> { + if inheritance_depth >= MAX_TYPE_REFERENCE_DEPTH { + return Ok(None); + } + let Some(aggregate_loc) = normalize_type_loc(dwarf, type_name_index, loc)? else { + return Ok(None); + }; + let mut current_path = visited.clone(); + if !current_path.insert((aggregate_loc.cu_off.0, aggregate_loc.die_off.0)) { + return Ok(None); + } + let header = dwarf.unit_header(aggregate_loc.cu_off)?; let unit = dwarf.unit(header)?; let entry = unit.entry(aggregate_loc.die_off)?; @@ -211,23 +245,51 @@ fn projected_member_type_loc( let mut tree = unit.entries_tree(Some(entry.offset()))?; let root = tree.root()?; let mut children = root.children(); + let mut base_classes = Vec::new(); while let Some(child) = children.next()? { let member = child.entry(); - if member.tag() != gimli::DW_TAG_member { - continue; - } - let Some(name_attribute) = member.attr(gimli::DW_AT_name) else { - continue; - }; - let name_reader = dwarf.attr_string(&unit, name_attribute.value())?; - let name = name_reader.to_string_lossy()?; - if name.as_ref() != field { - continue; + if member.tag() == gimli::DW_TAG_member { + if resolve_name_with_origins(dwarf, &unit, member)?.as_deref() == Some(field) { + return resolve_type_ref_with_origins(dwarf, member, &unit); + } + } else if member.tag() == gimli::DW_TAG_inheritance { + if let Some(base_class) = + resolve_type_definition_ref_with_origins(dwarf, member, &unit)? + { + base_classes.push(base_class); + } } - return resolve_type_ref_with_origins(dwarf, member, &unit); } - Ok(None) + let mut inherited_member = None; + for base_class in base_classes { + if let Some(member) = projected_aggregate_member_type_loc( + dwarf, + type_name_index, + base_class, + field, + ¤t_path, + inheritance_depth + 1, + )? { + if inherited_member.is_some() { + let kind = match entry.tag() { + gimli::DW_TAG_class_type => "class", + gimli::DW_TAG_union_type => "union", + _ => "struct", + }; + let type_name = resolve_name_with_origins(dwarf, &unit, &entry)? + .unwrap_or_else(|| "".to_string()); + return Err(crate::TypeLayoutError::AmbiguousMember { + kind, + type_name, + field: field.to_string(), + } + .into()); + } + inherited_member = Some(member); + } + } + Ok(inherited_member) } fn variant_member_type_loc( @@ -569,6 +631,8 @@ mod tests { int: TypeLoc, pair_pointer: TypeLoc, int_array: TypeLoc, + derived: TypeLoc, + ambiguous_derived: TypeLoc, } fn build_fixture() -> Fixture { @@ -640,6 +704,84 @@ mod tests { let int_array = unit.add(root, gimli::DW_TAG_array_type); unit.get_mut(int_array) .set(gimli::DW_AT_type, WriteAttributeValue::UnitRef(int)); + + let base = unit.add(root, gimli::DW_TAG_structure_type); + unit.get_mut(base).set( + gimli::DW_AT_name, + WriteAttributeValue::String(b"Base".to_vec()), + ); + unit.get_mut(base) + .set(gimli::DW_AT_byte_size, WriteAttributeValue::Data1(8)); + let inherited = unit.add(base, gimli::DW_TAG_member); + unit.get_mut(inherited).set( + gimli::DW_AT_name, + WriteAttributeValue::String(b"inherited".to_vec()), + ); + unit.get_mut(inherited) + .set(gimli::DW_AT_type, WriteAttributeValue::UnitRef(int)); + unit.get_mut(inherited).set( + gimli::DW_AT_data_member_location, + WriteAttributeValue::Udata(4), + ); + + let derived = unit.add(root, gimli::DW_TAG_class_type); + unit.get_mut(derived).set( + gimli::DW_AT_name, + WriteAttributeValue::String(b"Derived".to_vec()), + ); + unit.get_mut(derived) + .set(gimli::DW_AT_byte_size, WriteAttributeValue::Data1(20)); + let inheritance = unit.add(derived, gimli::DW_TAG_inheritance); + unit.get_mut(inheritance) + .set(gimli::DW_AT_type, WriteAttributeValue::UnitRef(base)); + unit.get_mut(inheritance).set( + gimli::DW_AT_data_member_location, + WriteAttributeValue::Udata(8), + ); + let own = unit.add(derived, gimli::DW_TAG_member); + unit.get_mut(own).set( + gimli::DW_AT_name, + WriteAttributeValue::String(b"own".to_vec()), + ); + unit.get_mut(own) + .set(gimli::DW_AT_type, WriteAttributeValue::UnitRef(int)); + unit.get_mut(own).set( + gimli::DW_AT_data_member_location, + WriteAttributeValue::Udata(16), + ); + + let other_base = unit.add(root, gimli::DW_TAG_structure_type); + unit.get_mut(other_base).set( + gimli::DW_AT_name, + WriteAttributeValue::String(b"OtherBase".to_vec()), + ); + unit.get_mut(other_base) + .set(gimli::DW_AT_byte_size, WriteAttributeValue::Data1(4)); + let other_inherited = unit.add(other_base, gimli::DW_TAG_member); + unit.get_mut(other_inherited).set( + gimli::DW_AT_name, + WriteAttributeValue::String(b"inherited".to_vec()), + ); + unit.get_mut(other_inherited) + .set(gimli::DW_AT_type, WriteAttributeValue::UnitRef(int)); + + let ambiguous_derived = unit.add(root, gimli::DW_TAG_class_type); + unit.get_mut(ambiguous_derived).set( + gimli::DW_AT_name, + WriteAttributeValue::String(b"AmbiguousDerived".to_vec()), + ); + unit.get_mut(ambiguous_derived) + .set(gimli::DW_AT_byte_size, WriteAttributeValue::Data1(12)); + let first_inheritance = unit.add(ambiguous_derived, gimli::DW_TAG_inheritance); + unit.get_mut(first_inheritance) + .set(gimli::DW_AT_type, WriteAttributeValue::UnitRef(base)); + let second_inheritance = unit.add(ambiguous_derived, gimli::DW_TAG_inheritance); + unit.get_mut(second_inheritance) + .set(gimli::DW_AT_type, WriteAttributeValue::UnitRef(other_base)); + unit.get_mut(second_inheritance).set( + gimli::DW_AT_data_member_location, + WriteAttributeValue::Udata(8), + ); } let mut sections = Sections::new(EndianVec::new(LittleEndian)); @@ -663,6 +805,7 @@ mod tests { let mut int = None; let mut pair_pointer = None; let mut int_array = None; + let mut classes = Vec::new(); let mut entries = unit.entries(); while let Some(entry) = entries.next_dfs().unwrap() { let loc = TypeLoc { @@ -674,11 +817,13 @@ mod tests { gimli::DW_TAG_structure_type => structures.push(loc), gimli::DW_TAG_pointer_type => pair_pointer = Some(loc), gimli::DW_TAG_array_type => int_array = Some(loc), + gimli::DW_TAG_class_type => classes.push(loc), _ => {} } } - assert_eq!(structures.len(), 2); + assert_eq!(structures.len(), 4); + assert_eq!(classes.len(), 2); Fixture { dwarf, cu: CuId(cu_off.0 as u32), @@ -687,6 +832,8 @@ mod tests { int: int.unwrap(), pair_pointer: pair_pointer.unwrap(), int_array: int_array.unwrap(), + derived: classes[0], + ambiguous_derived: classes[1], } } @@ -927,6 +1074,86 @@ mod tests { assert_eq!(element, Some(fixture.int)); } + #[test] + fn follows_inheritance_for_member_type_and_layout() { + let fixture = build_fixture(); + let types = TypeNameIndex::default(); + + let inherited = projected_type_loc( + &fixture.dwarf, + &types, + fixture.derived, + &VariableAccessSegment::Field("inherited".to_string()), + ) + .unwrap(); + assert_eq!(inherited, Some(fixture.int)); + + let header = fixture.dwarf.unit_header(fixture.derived.cu_off).unwrap(); + let unit = fixture.dwarf.unit(header).unwrap(); + let summary = crate::parser::DetailedParser::resolve_type_shallow_at_offset( + &fixture.dwarf, + &unit, + fixture.derived.die_off, + SourceLanguage::Cpp, + ) + .unwrap(); + let crate::TypeInfo::StructType { members, .. } = summary else { + panic!("expected derived class summary"); + }; + + assert_eq!( + members + .iter() + .find(|member| member.name == "inherited") + .map(|member| member.offset), + Some(12) + ); + assert_eq!( + members + .iter() + .find(|member| member.name == "own") + .map(|member| member.offset), + Some(16) + ); + } + + #[test] + fn rejects_ambiguous_inherited_member_paths() { + let fixture = build_fixture(); + let types = TypeNameIndex::default(); + + let projected_error = projected_type_loc( + &fixture.dwarf, + &types, + fixture.ambiguous_derived, + &VariableAccessSegment::Field("inherited".to_string()), + ) + .expect_err("two inherited paths should be ambiguous"); + assert!(matches!( + projected_error.downcast_ref::(), + Some(crate::TypeLayoutError::AmbiguousMember { field, .. }) + if field == "inherited" + )); + + let header = fixture + .dwarf + .unit_header(fixture.ambiguous_derived.cu_off) + .unwrap(); + let unit = fixture.dwarf.unit(header).unwrap(); + let summary = crate::parser::DetailedParser::resolve_type_shallow_at_offset( + &fixture.dwarf, + &unit, + fixture.ambiguous_derived.die_off, + SourceLanguage::Cpp, + ) + .unwrap(); + assert!(matches!( + crate::member_layout(&summary, "inherited"), + Err(crate::TypeLayoutError::AmbiguousMember { field, .. }) + if field == "inherited" + )); + } + #[test] fn resolves_template_type_parameter_by_dwarf_order() { let fixture = build_fixture(); diff --git a/ghostscope-dwarf/src/parser/detailed_parser.rs b/ghostscope-dwarf/src/parser/detailed_parser.rs index 0a8154e3..99832925 100644 --- a/ghostscope-dwarf/src/parser/detailed_parser.rs +++ b/ghostscope-dwarf/src/parser/detailed_parser.rs @@ -12,7 +12,8 @@ use crate::{ index::{CfiIndex, FunctionBlocks}, parser::ExpressionEvaluator, semantics::{ - resolve_name_with_origins, resolve_type_ref_in_same_unit_with_origins, VisibleVariable, + resolve_name_with_origins, resolve_type_definition_ref_with_origins, + resolve_type_ref_in_same_unit_with_origins, VisibleVariable, }, TypeInfo, }; @@ -171,6 +172,66 @@ impl DetailedParser { }) } + fn static_data_member_offset( + entry: &gimli::DebuggingInformationEntry, + unit: &gimli::Unit, + reason: &'static str, + ) -> Option { + let Some(location) = entry.attr(gimli::DW_AT_data_member_location) else { + return Some(0); + }; + + match location.value() { + gimli::AttributeValue::Exprloc(expression) => expr_errors::downgrade_optional_to_none( + DwarfExprMode::ConstOffset, + crate::dwarf_expr::const_eval::eval_const_offset(&expression, unit.encoding()), + reason, + ), + value => attr_u64(value), + } + } + + fn parse_inherited_members( + dwarf: &gimli::Dwarf, + unit: &gimli::Unit, + inheritance: &gimli::DebuggingInformationEntry, + inheritance_depth: usize, + ) -> Option> { + if inheritance + .attr(gimli::DW_AT_virtuality) + .is_some_and(|attribute| { + !matches!( + attribute.value(), + gimli::AttributeValue::Virtuality(gimli::DW_VIRTUALITY_none) + ) + }) + { + return None; + } + + let base_offset = + Self::static_data_member_offset(inheritance, unit, "non-virtual base class layout")?; + let base_loc = resolve_type_definition_ref_with_origins(dwarf, inheritance, unit) + .ok() + .flatten()?; + let base_header = dwarf.unit_header(base_loc.cu_off).ok()?; + let base_unit = dwarf.unit(base_header).ok()?; + let base_type = Self::resolve_type_shallow_at_offset_impl_with_depth( + dwarf, + &base_unit, + base_loc.die_off, + inheritance_depth + 1, + )?; + let mut members = match base_type { + TypeInfo::StructType { members, .. } | TypeInfo::VariantType { members, .. } => members, + _ => return None, + }; + for member in &mut members { + member.offset = base_offset.checked_add(member.offset)?; + } + Some(members) + } + fn discriminant_is_unsigned(type_info: &TypeInfo) -> bool { match type_info { TypeInfo::BaseType { encoding, .. } => { @@ -412,10 +473,26 @@ impl DetailedParser { } fn resolve_type_shallow_at_offset_impl( + dwarf: &gimli::Dwarf, + unit: &gimli::Unit, + type_offset: gimli::UnitOffset, + ) -> Option { + Self::resolve_type_shallow_at_offset_impl_with_depth(dwarf, unit, type_offset, 0) + } + + fn resolve_type_shallow_at_offset_impl_with_depth( dwarf: &gimli::Dwarf, unit: &gimli::Unit, mut type_offset: gimli::UnitOffset, + inheritance_depth: usize, ) -> Option { + const MAX_INHERITANCE_DEPTH: usize = 64; + if inheritance_depth >= MAX_INHERITANCE_DEPTH { + return Some(TypeInfo::UnknownType { + name: "".to_string(), + }); + } + let mut visited = std::collections::HashSet::new(); // Strip typedef/qualifiers chain but keep last typedef name if it's the canonical alias let mut alias_name: Option = None; @@ -629,8 +706,9 @@ impl DetailedParser { byte_size = sz; } } - // Collect only direct member DIEs + // Direct members shadow inherited members, regardless of DIE order. let mut members: Vec = Vec::new(); + let mut inherited_members: Vec = Vec::new(); let mut variant_parts: Vec = Vec::new(); if let Ok(mut tree) = unit.entries_tree(Some(entry.offset())) { if let Ok(root) = tree.root() { @@ -745,6 +823,15 @@ impl DetailedParser { bit_offset, bit_size, }); + } else if ce.tag() == gimli::DW_TAG_inheritance { + if let Some(base_members) = Self::parse_inherited_members( + dwarf, + unit, + ce, + inheritance_depth, + ) { + inherited_members.extend(base_members); + } } else if ce.tag() == DW_TAG_VARIANT_PART { if let Some(part) = Self::parse_variant_part_at_offset(dwarf, unit, ce.offset()) @@ -755,6 +842,14 @@ impl DetailedParser { } } } + let direct_member_names = members + .iter() + .map(|member| member.name.as_str()) + .collect::>(); + inherited_members + .retain(|member| !direct_member_names.contains(member.name.as_str())); + // Keep duplicate inherited names so lookup can reject ambiguous base paths. + members.extend(inherited_members); // Post-process: infer array total_size/element_count when missing (from next member offset or struct size) if !members.is_empty() { // Pre-build sorted offsets diff --git a/ghostscope-dwarf/src/semantics/mod.rs b/ghostscope-dwarf/src/semantics/mod.rs index 07e9f85b..7306b3e7 100644 --- a/ghostscope-dwarf/src/semantics/mod.rs +++ b/ghostscope-dwarf/src/semantics/mod.rs @@ -32,6 +32,7 @@ pub use type_layout::{ strip_type_aliases, IndexableElementLayout, MemberLayout, TypeLayoutError, }; pub(crate) use types::{ + resolve_type_definition_loc, resolve_type_definition_ref_with_origins, resolve_type_ref_in_same_unit_with_origins, resolve_type_ref_with_origins, TypeLoc, }; pub use unwind_plan::{ diff --git a/ghostscope-dwarf/src/semantics/type_layout.rs b/ghostscope-dwarf/src/semantics/type_layout.rs index 0f3d747e..a001c8cd 100644 --- a/ghostscope-dwarf/src/semantics/type_layout.rs +++ b/ghostscope-dwarf/src/semantics/type_layout.rs @@ -24,6 +24,13 @@ pub enum TypeLayoutError { members: String, }, + #[error("Ambiguous member '{field}' in {kind} '{type_name}'")] + AmbiguousMember { + kind: &'static str, + type_name: String, + field: String, + }, + #[error("member access requires struct or union type, got '{type_name}'")] InvalidMemberBase { type_name: String }, } @@ -60,28 +67,41 @@ pub fn is_pointer_or_array_type(ty: &TypeInfo) -> bool { pub fn member_layout(ty: &TypeInfo, field: &str) -> Result { match strip_type_aliases(ty) { - TypeInfo::StructType { name, members, .. } => members - .iter() - .find(|member| member.name == field) - .map(|member| MemberLayout { - offset: member.offset, - member_type: member.member_type.clone(), - }) - .ok_or_else(|| unknown_member_error("struct", name, field, members)), - TypeInfo::UnionType { name, members, .. } => members - .iter() - .find(|member| member.name == field) - .map(|member| MemberLayout { - offset: member.offset, - member_type: member.member_type.clone(), - }) - .ok_or_else(|| unknown_member_error("union", name, field, members)), + TypeInfo::StructType { name, members, .. } => { + unique_member_layout("struct", name, field, members) + } + TypeInfo::UnionType { name, members, .. } => { + unique_member_layout("union", name, field, members) + } other => Err(TypeLayoutError::InvalidMemberBase { type_name: other.type_name(), }), } } +fn unique_member_layout( + kind: &'static str, + type_name: &str, + field: &str, + members: &[crate::StructMember], +) -> Result { + let mut matches = members.iter().filter(|member| member.name == field); + let Some(member) = matches.next() else { + return Err(unknown_member_error(kind, type_name, field, members)); + }; + if matches.next().is_some() { + return Err(TypeLayoutError::AmbiguousMember { + kind, + type_name: type_name.to_string(), + field: field.to_string(), + }); + } + Ok(MemberLayout { + offset: member.offset, + member_type: member.member_type.clone(), + }) +} + pub fn indexable_element_layout(ty: &TypeInfo) -> Option { match strip_type_aliases(ty) { TypeInfo::ArrayType { element_type, .. } => Some(IndexableElementLayout { @@ -181,4 +201,37 @@ mod tests { let element = indexable_element_layout(&pointer_type).expect("pointer element layout"); assert_eq!(element.stride, 4); } + + #[test] + fn duplicate_member_names_are_ambiguous() { + let duplicate = TypeInfo::StructType { + name: "Derived".to_string(), + size: 8, + members: vec![ + StructMember { + name: "value".to_string(), + member_type: signed_int(), + offset: 0, + bit_offset: None, + bit_size: None, + }, + StructMember { + name: "value".to_string(), + member_type: signed_int(), + offset: 4, + bit_offset: None, + bit_size: None, + }, + ], + }; + + assert!(matches!( + member_layout(&duplicate, "value"), + Err(TypeLayoutError::AmbiguousMember { + kind: "struct", + type_name, + field, + }) if type_name == "Derived" && field == "value" + )); + } } diff --git a/ghostscope-dwarf/src/semantics/types.rs b/ghostscope-dwarf/src/semantics/types.rs index 920523b6..0fb7e73f 100644 --- a/ghostscope-dwarf/src/semantics/types.rs +++ b/ghostscope-dwarf/src/semantics/types.rs @@ -40,6 +40,34 @@ fn resolve_debug_info_ref( Ok(None) } +fn type_unit_loc_by_signature( + dwarf: &gimli::Dwarf, + signature: gimli::DebugTypeSignature, +) -> crate::core::Result> { + let mut units = dwarf.units(); + while let Some(header) = units.next()? { + let type_offset = match header.type_() { + gimli::UnitType::Type { + type_signature, + type_offset, + } + | gimli::UnitType::SplitType { + type_signature, + type_offset, + } if type_signature == signature => type_offset, + _ => continue, + }; + let cu_off = header + .debug_info_offset() + .ok_or_else(|| anyhow::anyhow!("type unit missing debug_info offset"))?; + return Ok(Some(TypeLoc { + cu_off, + die_off: type_offset, + })); + } + Ok(None) +} + fn type_loc_from_attr_value( dwarf: &gimli::Dwarf, unit: &gimli::Unit, @@ -57,10 +85,58 @@ fn type_loc_from_attr_value( } Ok(resolve_debug_info_ref(dwarf, debug_info_off)?.map(|(_, loc)| loc)) } + gimli::AttributeValue::DebugTypesRef(signature) => { + type_unit_loc_by_signature(dwarf, signature) + } _ => Ok(None), } } +/// Follow a declaration DIE's `DW_AT_signature` to its DWARF5 type-unit definition. +/// +/// A producer may use a unit-relative reference to a signature-bearing declaration +/// instead of referencing the type-unit signature directly with `DW_FORM_ref_sig8`. +pub(crate) fn resolve_type_definition_loc( + dwarf: &gimli::Dwarf, + mut loc: TypeLoc, +) -> crate::core::Result> { + const MAX_SIGNATURE_DEPTH: usize = 64; + let mut visited = std::collections::HashSet::new(); + + for _ in 0..MAX_SIGNATURE_DEPTH { + if !visited.insert((loc.cu_off.0, loc.die_off.0)) { + return Ok(None); + } + + let header = dwarf.unit_header(loc.cu_off)?; + let unit = dwarf.unit(header)?; + let entry = unit.entry(loc.die_off)?; + let Some(value) = entry.attr_value(gimli::constants::DW_AT_signature) else { + return Ok(Some(loc)); + }; + let gimli::AttributeValue::DebugTypesRef(signature) = value else { + return Ok(Some(loc)); + }; + let Some(definition) = type_unit_loc_by_signature(dwarf, signature)? else { + return Ok(None); + }; + loc = definition; + } + + Ok(None) +} + +pub(crate) fn resolve_type_definition_ref_with_origins( + dwarf: &gimli::Dwarf, + entry: &gimli::DebuggingInformationEntry, + unit: &gimli::Unit, +) -> crate::core::Result> { + let Some(loc) = resolve_type_ref_with_origins(dwarf, entry, unit)? else { + return Ok(None); + }; + resolve_type_definition_loc(dwarf, loc) +} + pub(crate) fn resolve_type_ref_with_origins( dwarf: &gimli::Dwarf, entry: &gimli::DebuggingInformationEntry, diff --git a/ghostscope-dwarf/src/semantics/variable_plan/mod.rs b/ghostscope-dwarf/src/semantics/variable_plan/mod.rs index 9b01083f..e4c29b02 100644 --- a/ghostscope-dwarf/src/semantics/variable_plan/mod.rs +++ b/ghostscope-dwarf/src/semantics/variable_plan/mod.rs @@ -269,6 +269,13 @@ pub enum PlanError { members: String, }, + #[error("Ambiguous member '{field}' in {kind} '{type_name}'")] + AmbiguousMember { + kind: &'static str, + type_name: String, + field: String, + }, + #[error("Tuple index '.{index}' requires DWARF type identity for language dispatch")] TupleIndexMissingTypeIdentity { index: u32 }, @@ -571,6 +578,16 @@ impl VariableReadPlan { members, } .into(), + TypeLayoutError::AmbiguousMember { + kind, + type_name, + field, + } => PlanError::AmbiguousMember { + kind, + type_name, + field, + } + .into(), TypeLayoutError::InvalidMemberBase { type_name } => { anyhow::anyhow!("member '{field}' not found on type '{type_name}'") }