diff --git a/e2e-tests/tests/fixtures/static_scope_program/Makefile b/e2e-tests/tests/fixtures/static_scope_program/Makefile index dafd233f..aae20f47 100644 --- a/e2e-tests/tests/fixtures/static_scope_program/Makefile +++ b/e2e-tests/tests/fixtures/static_scope_program/Makefile @@ -2,15 +2,19 @@ CC ?= gcc CFLAGS ?= -Wall -Wextra -g -O0 BINARY ?= static_scope_program OBJ ?= $(BINARY).o +EXTRA_OBJ = $(OBJ:.o=_other.o) all: $(BINARY) -$(BINARY): $(OBJ) +$(BINARY): $(OBJ) $(EXTRA_OBJ) $(CC) $(CFLAGS) -o $@ $^ $(OBJ): static_scope_program.c $(CC) $(CFLAGS) -c -o $@ $< +$(EXTRA_OBJ): other.c + $(CC) $(CFLAGS) -c -o $@ $< + clean: rm -f *.o static_scope_program static_scope_program_clang_dwarf5 diff --git a/e2e-tests/tests/fixtures/static_scope_program/other.c b/e2e-tests/tests/fixtures/static_scope_program/other.c new file mode 100644 index 00000000..6325418d --- /dev/null +++ b/e2e-tests/tests/fixtures/static_scope_program/other.c @@ -0,0 +1,6 @@ +static struct { int other; int common; } cfg = {99, 2}; +int binding_state = 11; + +__attribute__((noinline)) int binding_scope_two(void) { + return cfg.other + cfg.common; +} diff --git a/e2e-tests/tests/fixtures/static_scope_program/static_scope_program.c b/e2e-tests/tests/fixtures/static_scope_program/static_scope_program.c index 8ba88ff5..d64f56eb 100644 --- a/e2e-tests/tests/fixtures/static_scope_program/static_scope_program.c +++ b/e2e-tests/tests/fixtures/static_scope_program/static_scope_program.c @@ -20,9 +20,16 @@ static int bump_counters(int seed) { return function_scope_static_counter + file_scope_static_counter + regular_local; } +static int binding_scope_one(void); +static int binding_scope_unrelated(void); +extern int binding_scope_two(void); +extern int binding_state; + int main(void) { while (1) { int snapshot = bump_counters(2); + snapshot += binding_scope_one() + binding_scope_two(); + snapshot += binding_scope_unrelated(); if (snapshot == -1) { return 1; } @@ -30,3 +37,15 @@ int main(void) { } return 0; } + +static struct { int own; int common; } cfg = {11, 1}; + +__attribute__((noinline)) static int binding_scope_one(void) { + return cfg.own + cfg.common + binding_state; +} + +__attribute__((noinline)) static int binding_scope_unrelated(void) { + static int binding_state = 999; + static struct { int own; int common; } cfg = {777, 778}; + return binding_state + cfg.own + cfg.common; +} diff --git a/e2e-tests/tests/static_scope_execution.rs b/e2e-tests/tests/static_scope_execution.rs index 2232e1d4..8b94771e 100644 --- a/e2e-tests/tests/static_scope_execution.rs +++ b/e2e-tests/tests/static_scope_execution.rs @@ -4,6 +4,62 @@ use common::{fixture_compiler_available, init, FixtureCompiler, FIXTURES}; use std::path::Path; use std::time::Duration; +#[tokio::test] +async fn test_static_global_binding_uses_the_current_compilation_unit() -> anyhow::Result<()> { + init(); + let binary = FIXTURES.get_test_binary("static_scope_program")?; + let target = spawn_static_scope_program(&binary).await?; + let script = r#" +trace binding_scope_one { print "FIRST_CU:{}:{}:{}", cfg.own, cfg.common, binding_state; } +trace binding_scope_two { print "SECOND_CU:{}:{}", cfg.other, cfg.common; } +trace binding_scope_unrelated { print "LOCAL_SCOPE:{}:{}:{}", cfg.own, cfg.common, binding_state; } +"#; + let (code, stdout, stderr) = run_ghostscope_with_script_for_target(script, 3, &target).await?; + target.terminate().await?; + assert_eq!(code, 0, "stderr={stderr} stdout={stdout}"); + assert!(stdout.contains("FIRST_CU:11:1:11"), "{stdout}"); + assert!(stdout.contains("SECOND_CU:99:2"), "{stdout}"); + assert!(stdout.contains("LOCAL_SCOPE:777:778:999"), "{stdout}"); + Ok(()) +} + +#[tokio::test] +async fn test_global_binding_rejects_static_local_outside_its_scope() -> anyhow::Result<()> { + init(); + let binary = FIXTURES.get_test_binary("static_scope_program")?; + let target = spawn_static_scope_program(&binary).await?; + let (code, stdout, stderr) = run_ghostscope_with_script_for_target( + r#"trace binding_scope_one { print "WRONG_LOCAL:{}", function_scope_static_counter; }"#, + 3, + &target, + ) + .await?; + target.terminate().await?; + assert_ne!(code, 0, "stderr={stderr} stdout={stdout}"); + assert!(!stdout.contains("WRONG_LOCAL:"), "{stdout}"); + assert!(stderr.contains("function_scope_static_counter"), "{stderr}"); + Ok(()) +} + +#[tokio::test] +async fn test_invalid_static_field_cannot_bind_a_different_compilation_unit() -> anyhow::Result<()> +{ + init(); + let binary = FIXTURES.get_test_binary("static_scope_program")?; + let target = spawn_static_scope_program(&binary).await?; + let (code, stdout, stderr) = run_ghostscope_with_script_for_target( + r#"trace binding_scope_one { print "WRONG_SCOPE:{}", cfg.other; }"#, + 3, + &target, + ) + .await?; + target.terminate().await?; + assert_ne!(code, 0, "stderr={stderr} stdout={stdout}"); + assert!(!stdout.contains("WRONG_SCOPE:"), "{stdout}"); + assert!(stderr.contains("other"), "{stderr}"); + Ok(()) +} + async fn run_ghostscope_with_script_for_target( script_content: &str, timeout_secs: u64, diff --git a/ghostscope-compiler/src/ebpf/dwarf_bridge.rs b/ghostscope-compiler/src/ebpf/dwarf_bridge.rs index 9a725a44..58af23cd 100644 --- a/ghostscope-compiler/src/ebpf/dwarf_bridge.rs +++ b/ghostscope-compiler/src/ebpf/dwarf_bridge.rs @@ -1522,7 +1522,11 @@ impl<'ctx, 'dw> EbpfContext<'ctx, 'dw> { } if let Some((_global_module, plan)) = analyzer - .plan_global_access_read_plan(&prefer_module, var_name, &VariableAccessPath::default()) + .plan_global_access_read_plan_at_address( + &module_address, + var_name, + &VariableAccessPath::default(), + ) .map_err(|err| CodeGenError::DwarfError(err.to_string()))? { debug!("Found DWARF global '{}' via variable read plan", var_name); @@ -1608,7 +1612,7 @@ impl<'ctx, 'dw> EbpfContext<'ctx, 'dw> { } if let Some((_module_path, plan)) = analyzer - .plan_global_access_read_plan(&prefer_module, base_name, access_path) + .plan_global_access_read_plan_at_address(&module_address, base_name, access_path) .map_err(|err| CodeGenError::DwarfError(err.to_string()))? { debug!("Found DWARF global access '{path_text}' via variable read plan"); diff --git a/ghostscope-dwarf/src/analyzer/plan_global.rs b/ghostscope-dwarf/src/analyzer/plan_global.rs index 99e35de3..c042f629 100644 --- a/ghostscope-dwarf/src/analyzer/plan_global.rs +++ b/ghostscope-dwarf/src/analyzer/plan_global.rs @@ -1,27 +1,28 @@ use super::DwarfAnalyzer; use crate::{ core::{GlobalVariableInfo, Provenance, Result}, - semantics::{VariableAccessPath, VariableReadPlan}, + semantics::{PcContext, VariableAccessPath, VariableReadPlan}, }; use std::path::{Path, PathBuf}; impl DwarfAnalyzer { - pub(super) fn select_unambiguous_global_plan( + pub(super) fn select_unambiguous_global_binding( base: &str, - mut candidates: Vec<(PathBuf, VariableReadPlan)>, - ) -> Result> { + mut candidates: Vec<(PathBuf, GlobalVariableInfo)>, + ) -> Result> { match candidates.len() { 0 => Ok(None), 1 => Ok(candidates.pop()), count => { let details = candidates .iter() - .map(|(module_path, plan)| { - let declaration = plan - .declaration - .map(|die| format!(" cu={} die=0x{:x}", die.cu.0, die.offset)) - .unwrap_or_default(); - format!("{}{}", module_path.display(), declaration) + .map(|(module_path, info)| { + format!( + "{} cu={} die=0x{:x}", + module_path.display(), + info.unit_offset.0, + info.die_offset.0 + ) }) .collect::>() .join(", "); @@ -32,19 +33,19 @@ impl DwarfAnalyzer { } } - pub(super) fn select_global_plan_with_preferred_module( + pub(super) fn select_global_binding_with_preferred_module( base: &str, prefer_module: &Path, - candidates: Vec<(PathBuf, VariableReadPlan)>, - ) -> Result> { + candidates: Vec<(PathBuf, GlobalVariableInfo)>, + ) -> Result> { let (preferred, fallback): (Vec<_>, Vec<_>) = candidates .into_iter() .partition(|(module_path, _)| module_path == prefer_module); if !preferred.is_empty() { - return Self::select_unambiguous_global_plan(base, preferred); + return Self::select_unambiguous_global_binding(base, preferred); } - Self::select_unambiguous_global_plan(base, fallback) + Self::select_unambiguous_global_binding(base, fallback) } /// Find global/static variables by name across all loaded modules @@ -74,71 +75,82 @@ impl DwarfAnalyzer { results } - /// Plan a global/static source-level access path as a neutral read plan. + /// Bind a global/static declaration before projecting its access path. + /// Without a PC, duplicate declarations in the preferred module are ambiguous. pub fn plan_global_access_read_plan( &self, - prefer_module: &PathBuf, + prefer_module: &Path, base: &str, path: &VariableAccessPath, ) -> Result> { - let matches = self.find_global_variables_by_name(base); - if matches.is_empty() { - return Ok(None); - } + self.plan_global_access_read_plan_in_scope(prefer_module, None, base, path) + } - let mut ordered: Vec<(PathBuf, GlobalVariableInfo)> = Vec::new(); - for (module_path, info) in matches.iter() { - if *module_path == *prefer_module { - ordered.push((module_path.clone(), info.clone())); - } - } - for (module_path, info) in matches.into_iter() { - if module_path != *prefer_module { - ordered.push((module_path, info)); - } - } + /// Resolve global names in the compilation unit of the traced instruction. + /// An invalid field on that declaration must not select another CU's variable. + pub fn plan_global_access_read_plan_at_address( + &self, + address: &crate::ModuleAddress, + base: &str, + path: &VariableAccessPath, + ) -> Result> { + let context = self.resolve_pc(address)?; + self.plan_global_access_read_plan_in_scope(&address.module_path, Some(&context), base, path) + } - let mut direct_matches = Vec::new(); - let mut last_error = None; - for (module_path, info) in ordered { - let base_plan = match self.resolve_variable_read_plan_by_offsets_in_module( - &module_path, - info.unit_offset, - info.die_offset, - Provenance::Synthesized { - detail: "global access".to_string(), - }, - ) { - Ok(plan) => plan, - Err(err) => { - last_error = Some(err); - continue; - } + fn plan_global_access_read_plan_in_scope( + &self, + prefer_module: &Path, + context: Option<&PcContext>, + base: &str, + path: &VariableAccessPath, + ) -> Result> { + let mut matches = self.find_global_variables_by_name(base); + let mut has_unknown_scope = false; + if let Some(context) = context { + // The global index also lists static locals. A CU preference must not + // promote a declaration belonging to another function or lexical block. + matches.retain(|(module_path, info)| { + let visibility = self.module_id_for_path(module_path).and_then(|module| { + self.modules + .get(module_path)? + .global_variable_visibility(info, module, context) + }); + has_unknown_scope |= visibility.is_none(); + visibility != Some(false) + }); + } + let prefer_cu = context.filter(|_| !has_unknown_scope).and_then(|context| { + context + .inline_chain + .last() + .map(|frame| frame.abstract_origin.unwrap_or(frame.concrete_die).cu) + .or(context.cu) + }); + if let Some(cu) = prefer_cu { + let in_scope = |(module_path, info): &(PathBuf, GlobalVariableInfo)| { + Self::module_paths_equivalent(module_path, prefer_module) + && info.unit_offset.0 as u64 == u64::from(cu.0) }; - - match self.plan_access_path_with_type_completion(&module_path, base_plan, path) { - Ok(plan) => direct_matches.push((module_path, plan)), - Err(primary_error) => { - if Self::is_value_backed_aggregate_access_error(&primary_error) { - return Err(primary_error); - } - last_error = Some(primary_error); - } + if matches.iter().any(in_scope) { + matches.retain(in_scope); } } - - if !direct_matches.is_empty() { - return Self::select_global_plan_with_preferred_module( - base, - prefer_module, - direct_matches, - ); - } - - if let Some(err) = last_error { - return Err(err); - } - Ok(None) + let Some((module_path, info)) = + Self::select_global_binding_with_preferred_module(base, prefer_module, matches)? + else { + return Ok(None); + }; + let base_plan = self.resolve_variable_read_plan_by_offsets_in_module( + &module_path, + info.unit_offset, + info.die_offset, + Provenance::Synthesized { + detail: "global access".to_string(), + }, + )?; + let plan = self.plan_access_path_with_type_completion(&module_path, base_plan, path)?; + Ok(Some((module_path, plan))) } fn resolve_variable_read_plan_by_offsets_in_module>( diff --git a/ghostscope-dwarf/src/analyzer/plan_pc.rs b/ghostscope-dwarf/src/analyzer/plan_pc.rs index 6c4253c0..d05d7c4c 100644 --- a/ghostscope-dwarf/src/analyzer/plan_pc.rs +++ b/ghostscope-dwarf/src/analyzer/plan_pc.rs @@ -2,9 +2,8 @@ use super::DwarfAnalyzer; use crate::{ core::{demangle::RustSymbolHashDisplay, ModuleAddress, Provenance, Result}, semantics::{ - AddressSpaceInfo, FunctionParameter, PcContext, PcLineInfo, PcRange, PlanError, - VariableAccessPath, VariableAccessSegment, VariableReadPlan, VisibleVariable, - VisibleVariablesResult, + AddressSpaceInfo, FunctionParameter, PcContext, PcLineInfo, PcRange, VariableAccessPath, + VariableAccessSegment, VariableReadPlan, VisibleVariable, VisibleVariablesResult, }, }; use std::path::Path; @@ -193,11 +192,6 @@ impl DwarfAnalyzer { )) } - pub(super) fn is_value_backed_aggregate_access_error(err: &anyhow::Error) -> bool { - err.downcast_ref::() - .is_some_and(PlanError::is_value_backed_aggregate_access) - } - pub(super) fn read_plan_from_variable( variable: crate::parser::ParsedVariable, provenance: Provenance, diff --git a/ghostscope-dwarf/src/analyzer/tests.rs b/ghostscope-dwarf/src/analyzer/tests.rs index 30bf9bb6..37280c9f 100644 --- a/ghostscope-dwarf/src/analyzer/tests.rs +++ b/ghostscope-dwarf/src/analyzer/tests.rs @@ -1,34 +1,19 @@ use super::DwarfAnalyzer; use crate::{ - core::{AddressExpr, Availability, Provenance, VariableLocation}, - semantics::{VariableReadPlan, VisibleVariable}, + core::{Availability, VariableLocation}, + semantics::VisibleVariable, RuntimeTextSymbol, }; use std::path::{Path, PathBuf}; -fn global_plan(name: &str, address: u64) -> VariableReadPlan { - VariableReadPlan { +fn global_binding(name: &str, address: u64) -> crate::GlobalVariableInfo { + crate::GlobalVariableInfo { name: name.to_string(), - type_name: "int".to_string(), - access_path: crate::VariableAccessPath::default(), - module_path: None, - dwarf_type: Some(crate::TypeInfo::BaseType { - name: "int".to_string(), - size: 4, - encoding: gimli::constants::DW_ATE_signed.0 as u16, - }), - declaration: None, - type_id: None, - location: VariableLocation::Address(AddressExpr::constant(address)), - availability: Availability::Available, - scope_depth: 0, - is_parameter: false, - is_artificial: false, - pc_range: None, - inline_context: None, - provenance: Provenance::Synthesized { - detail: "test".to_string(), - }, + lexical_scope: None, + link_address: Some(address), + section: None, + unit_offset: gimli::DebugInfoOffset(0), + die_offset: gimli::UnitOffset(address as usize), } } @@ -98,11 +83,11 @@ fn variable_selection_keeps_inner_match_over_outer_diagnostic() { #[test] fn global_plan_selection_rejects_ambiguous_matches() { - let err = DwarfAnalyzer::select_unambiguous_global_plan( + let err = DwarfAnalyzer::select_unambiguous_global_binding( "state", vec![ - (PathBuf::from("/tmp/a"), global_plan("state", 0x1000)), - (PathBuf::from("/tmp/b"), global_plan("state", 0x2000)), + (PathBuf::from("/tmp/a"), global_binding("state", 0x1000)), + (PathBuf::from("/tmp/b"), global_binding("state", 0x2000)), ], ) .expect_err("multiple global candidates should be ambiguous"); @@ -113,9 +98,9 @@ fn global_plan_selection_rejects_ambiguous_matches() { #[test] fn global_plan_selection_accepts_single_match() { - let selected = DwarfAnalyzer::select_unambiguous_global_plan( + let selected = DwarfAnalyzer::select_unambiguous_global_binding( "state", - vec![(PathBuf::from("/tmp/a"), global_plan("state", 0x1000))], + vec![(PathBuf::from("/tmp/a"), global_binding("state", 0x1000))], ) .expect("single global candidate should be accepted") .expect("single global candidate should be returned"); @@ -126,33 +111,39 @@ fn global_plan_selection_accepts_single_match() { #[test] fn global_plan_selection_prefers_current_module_match() { - let selected = DwarfAnalyzer::select_global_plan_with_preferred_module( + let selected = DwarfAnalyzer::select_global_binding_with_preferred_module( "state", Path::new("/tmp/current"), vec![ - (PathBuf::from("/tmp/other"), global_plan("state", 0x2000)), - (PathBuf::from("/tmp/current"), global_plan("state", 0x1000)), + (PathBuf::from("/tmp/other"), global_binding("state", 0x2000)), + ( + PathBuf::from("/tmp/current"), + global_binding("state", 0x1000), + ), ], ) .expect("current module candidate should be accepted") .expect("current module candidate should be returned"); assert_eq!(selected.0, PathBuf::from("/tmp/current")); - assert_eq!( - selected.1.location, - VariableLocation::Address(AddressExpr::constant(0x1000)) - ); + assert_eq!(selected.1.link_address, Some(0x1000)); } #[test] fn global_plan_selection_rejects_ambiguous_current_module_matches() { - let err = DwarfAnalyzer::select_global_plan_with_preferred_module( + let err = DwarfAnalyzer::select_global_binding_with_preferred_module( "state", Path::new("/tmp/current"), vec![ - (PathBuf::from("/tmp/current"), global_plan("state", 0x1000)), - (PathBuf::from("/tmp/current"), global_plan("state", 0x1004)), - (PathBuf::from("/tmp/other"), global_plan("state", 0x2000)), + ( + PathBuf::from("/tmp/current"), + global_binding("state", 0x1000), + ), + ( + PathBuf::from("/tmp/current"), + global_binding("state", 0x1004), + ), + (PathBuf::from("/tmp/other"), global_binding("state", 0x2000)), ], ) .expect_err("duplicate current-module candidates should be ambiguous"); diff --git a/ghostscope-dwarf/src/core/types.rs b/ghostscope-dwarf/src/core/types.rs index c0485dfe..1228d67c 100644 --- a/ghostscope-dwarf/src/core/types.rs +++ b/ghostscope-dwarf/src/core/types.rs @@ -55,6 +55,8 @@ pub struct IndexEntry { pub die_offset: gimli::UnitOffset, /// Compilation unit offset (gimli native type) pub unit_offset: gimli::DebugInfoOffset, + /// Nearest function/inline/lexical-block owner for a function-local variable. + pub lexical_scope: Option, /// DWARF tag (gimli native type) pub tag: gimli::DwTag, /// Index flags (inspired by GDB's cooked_index_flag) @@ -173,6 +175,8 @@ pub enum SectionType { #[derive(Debug, Clone)] pub struct GlobalVariableInfo { pub name: String, + /// Nearest function/inline/lexical-block owner, when nested in a function. + pub lexical_scope: Option, /// Link-time address from DWARF location (if available) pub link_address: Option, /// Best-effort section classification based on ELF section headers diff --git a/ghostscope-dwarf/src/objfile/function_lookup.rs b/ghostscope-dwarf/src/objfile/function_lookup.rs index 876696ea..9d244d85 100644 --- a/ghostscope-dwarf/src/objfile/function_lookup.rs +++ b/ghostscope-dwarf/src/objfile/function_lookup.rs @@ -932,6 +932,7 @@ mod tests { name: Arc::from("CGPsend"), die_offset: gimli::UnitOffset(0), unit_offset: gimli::DebugInfoOffset(0), + lexical_scope: None, tag: constants::DW_TAG_subprogram, flags: IndexFlags::default(), language: None, @@ -1514,6 +1515,7 @@ mod tests { name: Arc::::from(mangled.as_str()), die_offset: gimli::UnitOffset(0), unit_offset: gimli::DebugInfoOffset(0), + lexical_scope: None, tag: constants::DW_TAG_subprogram, flags: IndexFlags { is_linkage: true, @@ -1552,6 +1554,7 @@ mod tests { name: Arc::::from(mangled.as_str()), die_offset: gimli::UnitOffset(0), unit_offset: gimli::DebugInfoOffset(0), + lexical_scope: None, tag: constants::DW_TAG_subprogram, flags: IndexFlags { is_linkage: true, @@ -1578,6 +1581,7 @@ mod tests { name: Arc::::from(name), die_offset: gimli::UnitOffset(0), unit_offset: gimli::DebugInfoOffset(0), + lexical_scope: None, tag: constants::DW_TAG_subprogram, flags: IndexFlags { is_linkage: true, @@ -1677,6 +1681,7 @@ mod tests { name: Arc::::from(mangled.as_str()), die_offset: gimli::UnitOffset(0), unit_offset: gimli::DebugInfoOffset(0), + lexical_scope: None, tag: constants::DW_TAG_subprogram, flags: IndexFlags { is_linkage: true, diff --git a/ghostscope-dwarf/src/objfile/globals.rs b/ghostscope-dwarf/src/objfile/globals.rs index 13cfc03b..a153b826 100644 --- a/ghostscope-dwarf/src/objfile/globals.rs +++ b/ghostscope-dwarf/src/objfile/globals.rs @@ -43,6 +43,7 @@ impl LoadedObjfile { let section = link_address.and_then(|addr| self.classify_section(&obj, addr)); out.push(GlobalVariableInfo { name: name.to_string(), + lexical_scope: entry.lexical_scope, link_address, section, die_offset: entry.die_offset, @@ -101,6 +102,7 @@ impl LoadedObjfile { let link_address = e.representative_addr; out.push(GlobalVariableInfo { name: e.name.to_string(), + lexical_scope: e.lexical_scope, link_address, section: None, die_offset: e.die_offset, @@ -121,6 +123,7 @@ impl LoadedObjfile { out.push(GlobalVariableInfo { name: e.name.to_string(), + lexical_scope: e.lexical_scope, link_address, section, die_offset: e.die_offset, diff --git a/ghostscope-dwarf/src/objfile/variables.rs b/ghostscope-dwarf/src/objfile/variables.rs index 37ed23b4..e14c4e3b 100644 --- a/ghostscope-dwarf/src/objfile/variables.rs +++ b/ghostscope-dwarf/src/objfile/variables.rs @@ -250,6 +250,73 @@ impl LoadedObjfile { Some(false) } + /// Check lexical ownership without evaluating storage. `None` means DWARF + /// does not provide enough scope identity to establish visibility. + pub(crate) fn global_variable_visibility( + &self, + info: &crate::GlobalVariableInfo, + module: crate::ModuleId, + context: &crate::PcContext, + ) -> Option { + let Some(scope_offset) = info.lexical_scope else { + return Some(true); + }; + let scope = die_ref(module, info.unit_offset, scope_offset); + let active_scopes = context + .function + .map(|function| function.declaration) + .into_iter() + .chain(context.lexical_scopes.iter().map(|scope| scope.die)) + .chain(context.inline_chain.iter().flat_map(|frame| { + std::iter::once(frame.concrete_die).chain(frame.abstract_origin) + })); + for active in active_scopes.filter(|active| active.module == module) { + if active == scope { + return Some(true); + } + // Optimized scopes can leave their variables on an abstract DIE. + let unit = self + .unit(gimli::DebugInfoOffset(active.cu.0 as usize)) + .ok()?; + let entry = unit.entry(gimli::UnitOffset(active.offset as usize)).ok()?; + for attr in [gimli::DW_AT_abstract_origin, gimli::DW_AT_specification] { + if let Some(value) = entry.attr_value(attr) { + if let Some((_, origin_unit, origin_entry)) = + resolve_origin_entry(self.dwarf(), &unit, value).ok()? + { + if origin_unit.header.debug_info_offset() == Some(info.unit_offset) + && origin_entry.offset() == scope_offset + { + return Some(true); + } + } + } + } + } + + let unit = self.unit(info.unit_offset).ok()?; + let owner = unit.entry(scope_offset).ok()?; + let has_pc = [ + gimli::DW_AT_low_pc, + gimli::DW_AT_ranges, + gimli::DW_AT_entry_pc, + ] + .iter() + .any(|attr| owner.attr_value(*attr).is_some()); + // Clang can place an inline static under an anonymous, addressless + // subprogram with no link to the inline origin. Keep it as an unresolved + // candidate; its CU alone must not make it win a name collision. + if !has_pc + && (owner.tag() == gimli::DW_TAG_lexical_block + || resolve_name_with_origins(self.dwarf(), &unit, &owner) + .ok()? + .is_none()) + { + return None; + } + Some(false) + } + pub(crate) fn resolve_pc_scopes( &self, module: crate::ModuleId, @@ -1012,6 +1079,7 @@ mod tests { name: Arc::from("Foo"), die_offset: full_struct_off, unit_offset: full_cu_off, + lexical_scope: None, tag: constants::DW_TAG_structure_type, flags: IndexFlags::default(), language: None, diff --git a/ghostscope-dwarf/src/parser/fast_parser/mod.rs b/ghostscope-dwarf/src/parser/fast_parser/mod.rs index 64001f7e..4c324847 100644 --- a/ghostscope-dwarf/src/parser/fast_parser/mod.rs +++ b/ghostscope-dwarf/src/parser/fast_parser/mod.rs @@ -304,7 +304,7 @@ impl<'a> DwarfParser<'a> { let mut shard = InfoShard::default(); let mut entries = unit.entries_raw(None)?; let mut metadata_cache: HashMap = HashMap::new(); - let mut tag_stack: Vec = Vec::new(); + let mut tag_stack: Vec<(gimli::DwTag, gimli::UnitOffset)> = Vec::new(); while !entries.is_empty() { let d: usize = entries.next_depth() as usize; let entry_offset = entries.next_offset(); @@ -331,7 +331,7 @@ impl<'a> DwarfParser<'a> { // Most DIEs are not indexable. Skip their attributes without materializing // a full DebuggingInformationEntry so template-heavy CUs stay cheaper. entries.skip_attributes(abbrev.attributes())?; - tag_stack.push(tag); + tag_stack.push((tag, entry_offset)); continue; } @@ -437,7 +437,7 @@ impl<'a> DwarfParser<'a> { let var_addr = self.extract_variable_address_from_raw(unit, &raw_attrs)?; let is_static_symbol = Self::is_static_variable_symbol_from_raw(&raw_attrs, var_addr); - let in_function_scope = tag_stack.iter().any(|t| { + let in_function_scope = tag_stack.iter().any(|(t, _)| { *t == gimli::constants::DW_TAG_subprogram || *t == gimli::constants::DW_TAG_inlined_subroutine }); @@ -448,7 +448,7 @@ impl<'a> DwarfParser<'a> { tag_stack ); // Skip local variables - tag_stack.push(tag); + tag_stack.push((tag, entry_offset)); continue; } else if in_function_scope { // Rust (and some C compilers) sometimes nest file-scoped statics under the @@ -466,7 +466,7 @@ impl<'a> DwarfParser<'a> { "Skipping variable at {:?} (declaration-only DIE)", entry_offset ); - tag_stack.push(tag); + tag_stack.push((tag, entry_offset)); continue; } let mut collected_names: Vec<(Arc, bool)> = Vec::new(); @@ -500,7 +500,7 @@ impl<'a> DwarfParser<'a> { entry_offset, cu_language ); - tag_stack.push(tag); + tag_stack.push((tag, entry_offset)); continue; } @@ -508,6 +508,19 @@ impl<'a> DwarfParser<'a> { is_static: is_static_symbol, ..Default::default() }; + let lexical_scope = in_function_scope + .then(|| { + tag_stack.iter().rev().find_map(|(tag, offset)| { + matches!( + *tag, + gimli::DW_TAG_subprogram + | gimli::DW_TAG_inlined_subroutine + | gimli::DW_TAG_lexical_block + ) + .then_some(*offset) + }) + }) + .flatten(); for (name, is_linkage_alias) in collected_names { let mut entry_flags = flags; @@ -516,6 +529,7 @@ impl<'a> DwarfParser<'a> { name: Arc::clone(&name), die_offset: entry_offset, unit_offset, + lexical_scope, tag, flags: entry_flags, language: cu_language, @@ -551,6 +565,7 @@ impl<'a> DwarfParser<'a> { name: Arc::clone(&name), die_offset: entry_offset, unit_offset, + lexical_scope: None, tag, flags, language: cu_language, @@ -563,7 +578,7 @@ impl<'a> DwarfParser<'a> { } _ => {} } - tag_stack.push(tag); + tag_stack.push((tag, entry_offset)); } Ok(shard) } @@ -594,6 +609,7 @@ impl<'a> DwarfParser<'a> { name, die_offset: seed.die_offset, unit_offset: seed.unit_offset, + lexical_scope: None, tag: seed.tag, flags: seed.flags, language: seed.language, @@ -1557,6 +1573,7 @@ impl<'a> DwarfParser<'a> { name: Arc::from(name.as_str()), die_offset, unit_offset, + lexical_scope: None, tag, flags: crate::core::IndexFlags { is_main: name == "main" || name == "_main", diff --git a/ghostscope-dwarf/src/semantics/variable_plan/mod.rs b/ghostscope-dwarf/src/semantics/variable_plan/mod.rs index e4c29b02..33e35c46 100644 --- a/ghostscope-dwarf/src/semantics/variable_plan/mod.rs +++ b/ghostscope-dwarf/src/semantics/variable_plan/mod.rs @@ -312,12 +312,6 @@ pub enum PlanError { UnsupportedDereference { location: VariableLocation }, } -impl PlanError { - pub fn is_value_backed_aggregate_access(&self) -> bool { - matches!(self, PlanError::ValueBackedAggregateOffset { .. }) - } -} - #[derive(Debug, Clone, Copy, PartialEq, Eq)] enum ElementIndexContext { AccessPath, diff --git a/ghostscope-dwarf/src/semantics/variable_plan/tests.rs b/ghostscope-dwarf/src/semantics/variable_plan/tests.rs index c22216e5..a0235baf 100644 --- a/ghostscope-dwarf/src/semantics/variable_plan/tests.rs +++ b/ghostscope-dwarf/src/semantics/variable_plan/tests.rs @@ -714,7 +714,7 @@ fn field_access_rejects_value_backed_aggregates() { .expect_err("value-backed aggregate field access should fail"); assert!( err.downcast_ref::() - .is_some_and(PlanError::is_value_backed_aggregate_access), + .is_some_and(|error| matches!(error, PlanError::ValueBackedAggregateOffset { .. })), "unexpected error: {err}" ); } @@ -745,7 +745,7 @@ fn array_index_rejects_value_backed_aggregates() { .expect_err("value-backed aggregate array access should fail"); assert!( err.downcast_ref::() - .is_some_and(PlanError::is_value_backed_aggregate_access), + .is_some_and(|error| matches!(error, PlanError::ValueBackedAggregateOffset { .. })), "unexpected error: {err}" ); } diff --git a/ghostscope-dwarf/tests/global_binding.rs b/ghostscope-dwarf/tests/global_binding.rs new file mode 100644 index 00000000..34b04ea2 --- /dev/null +++ b/ghostscope-dwarf/tests/global_binding.rs @@ -0,0 +1,247 @@ +use ghostscope_dwarf::{ + AddressExpr, DwarfAnalyzer, VariableAccessPath, VariableAccessSegment, VariableLocation, +}; +use object::{Object, ObjectSymbol}; + +#[tokio::test] +async fn global_binding_precedes_projection_and_prefers_the_pc_compilation_unit() { + let dir = tempfile::tempdir().unwrap(); + let first = dir.path().join("first.c"); + let second = dir.path().join("second.c"); + let binary = dir.path().join("globals"); + std::fs::write( + &first, + "static struct { int own; int common; } cfg = {11, 1};\n\ + int tick(void) { return cfg.own; }\n\ + int main(void) { return tick(); }\n", + ) + .unwrap(); + std::fs::write( + &second, + "static struct { int other; int common; } cfg = {99, 2};\n\ + int other_tick(void) { return cfg.other; }\n", + ) + .unwrap(); + let output = std::process::Command::new("cc") + .args(["-g", "-O0"]) + .args([&first, &second]) + .arg("-o") + .arg(&binary) + .output() + .unwrap(); + assert!(output.status.success(), "{output:?}"); + let analyzer = DwarfAnalyzer::from_exec_path(&binary).await.unwrap(); + let field = + |name: &str| VariableAccessPath::new(vec![VariableAccessSegment::Field(name.to_string())]); + // Only one of the two structs has `other`; that must not resolve ambiguity. + let error = analyzer + .plan_global_access_read_plan(&binary, "cfg", &field("other")) + .unwrap_err(); + assert!(error.to_string().contains("Ambiguous global 'cfg'")); + + for (function, own, foreign) in [("tick", "own", "other"), ("other_tick", "other", "own")] { + let pc = analyzer.lookup_function_addresses(function).remove(0); + let context = analyzer.resolve_pc(&pc).unwrap(); + let (_, plan) = analyzer + .plan_global_access_read_plan_at_address(&pc, "cfg", &field(own)) + .unwrap() + .unwrap(); + assert_eq!(Some(plan.declaration.unwrap().cu), context.cu); + assert!(analyzer + .plan_global_access_read_plan_at_address(&pc, "cfg", &field(foreign)) + .is_err()); + let (_, common) = analyzer + .plan_global_access_read_plan_at_address(&pc, "cfg", &field("common")) + .unwrap() + .unwrap(); + assert_eq!(common.declaration, plan.declaration); + } +} + +#[tokio::test] +async fn global_binding_excludes_static_locals_outside_the_pc_scope() { + let dir = tempfile::tempdir().unwrap(); + let first = dir.path().join("first.c"); + let second = dir.path().join("second.c"); + let binary = dir.path().join("globals"); + std::fs::write( + &first, + "extern int state;\n\ + int unrelated(void) { static int state = 999; static int hidden = 7; return state + hidden; }\n\ + int tick(void) { return state; }\n\ + int scoped(int enabled) {\n\ + if (enabled) {\n\ + static int state = 333;\n\ + return state; /* inside */\n\ + }\n\ + return state; /* outside */\n\ + }\n\ + int main(void) { return tick() + unrelated(); }\n", + ) + .unwrap(); + std::fs::write(&second, "int state = 11;\n").unwrap(); + let output = std::process::Command::new("cc") + .args(["-g", "-O0"]) + .args([&first, &second]) + .arg("-o") + .arg(&binary) + .output() + .unwrap(); + assert!(output.status.success(), "{output:?}"); + + let bytes = std::fs::read(&binary).unwrap(); + let object = object::File::parse(bytes.as_slice()).unwrap(); + let global_address = object + .symbols() + .find(|symbol| symbol.name() == Ok("state")) + .unwrap() + .address(); + let analyzer = DwarfAnalyzer::from_exec_path(&binary).await.unwrap(); + let path = VariableAccessPath::default(); + let pc = analyzer.lookup_function_addresses("tick").remove(0); + let context = analyzer.resolve_pc(&pc).unwrap(); + assert!(analyzer + .plan_variable_by_name(&context, "state") + .unwrap() + .is_none()); + let (_, plan) = analyzer + .plan_global_access_read_plan_at_address(&pc, "state", &path) + .unwrap() + .unwrap(); + assert_eq!( + plan.location, + VariableLocation::Address(AddressExpr::constant(global_address)) + ); + assert!(analyzer + .plan_global_access_read_plan_at_address(&pc, "hidden", &path) + .unwrap() + .is_none()); + + // Discovery still includes static locals, and their own scope can read them. + assert_eq!(analyzer.find_global_variables_by_name("state").len(), 3); + assert!(analyzer + .plan_global_access_read_plan(&binary, "hidden", &path) + .unwrap() + .is_some()); + let local_pc = analyzer.lookup_function_addresses("unrelated").remove(0); + let local_context = analyzer.resolve_pc(&local_pc).unwrap(); + let local = analyzer + .plan_variable_by_name(&local_context, "state") + .unwrap() + .unwrap(); + assert_ne!(local.location, plan.location); + let (_, local_global) = analyzer + .plan_global_access_read_plan_at_address(&local_pc, "state", &path) + .unwrap() + .unwrap(); + assert_eq!(local_global.location, local.location); + + for (marker, is_global) in [("/* inside */", false), ("/* outside */", true)] { + let line = std::fs::read_to_string(&first) + .unwrap() + .lines() + .position(|line| line.contains(marker)) + .unwrap() as u32 + + 1; + let addresses = analyzer.lookup_addresses_by_source_line(first.to_str().unwrap(), line); + assert!(!addresses.is_empty()); + for address in addresses { + let (_, scoped) = analyzer + .plan_global_access_read_plan_at_address(&address, "state", &path) + .unwrap() + .unwrap(); + assert_eq!(scoped.location == plan.location, is_global, "{marker}"); + } + } +} + +#[tokio::test] +async fn inline_static_binding_preserves_origin_and_unknown_scope_candidates() { + for compiler in ["cc", "clang"] { + if compiler == "clang" + && !std::process::Command::new(compiler) + .arg("--version") + .output() + .is_ok_and(|output| output.status.success()) + { + eprintln!("Skipping inline static binding with unavailable clang"); + continue; + } + for collision in [false, true] { + let dir = tempfile::tempdir().unwrap(); + let first = dir.path().join("first.c"); + let second = dir.path().join("second.c"); + let binary = dir.path().join("inline"); + std::fs::write( + &first, + "static inline __attribute__((always_inline)) int inc(void) {\n\ + static volatile int count = 4;\n\ + count++;\n\ + return count;\n\ + }\n\ + __attribute__((noinline)) int tick(void) { return inc(); }\n\ + int main(void) { return tick(); }\n", + ) + .unwrap(); + std::fs::write( + &second, + if collision { + "int count = 99;\n" + } else { + "int unrelated = 99;\n" + }, + ) + .unwrap(); + let output = std::process::Command::new(compiler) + .args(["-g", "-O2"]) + .args([&first, &second]) + .arg("-o") + .arg(&binary) + .output() + .unwrap(); + assert!(output.status.success(), "{compiler}: {output:?}"); + let analyzer = DwarfAnalyzer::from_exec_path(&binary).await.unwrap(); + let pc = analyzer.lookup_function_addresses("inc").remove(0); + let context = analyzer.resolve_pc(&pc).unwrap(); + assert!(!context.inline_chain.is_empty()); + let count = analyzer + .find_global_variables_by_name("count") + .into_iter() + .find(|(_, info)| info.lexical_scope.is_some()) + .unwrap() + .1; + let result = analyzer.plan_global_access_read_plan_at_address( + &pc, + "count", + &VariableAccessPath::default(), + ); + match result { + Ok(Some((_, plan))) => { + assert_eq!(plan.availability, ghostscope_dwarf::Availability::Available); + assert_eq!( + plan.location, + VariableLocation::Address(AddressExpr::constant( + count.link_address.unwrap() + )) + ); + } + Err(error) if collision => { + assert!(error.to_string().starts_with("Ambiguous global 'count'")); + } + other => panic!("{compiler}, collision={collision}: {other:?}"), + } + if collision && compiler == "clang" { + // Clang's anonymous owner cannot establish a CU preference. + assert!(analyzer + .plan_global_access_read_plan_at_address( + &pc, + "count", + &VariableAccessPath::default(), + ) + .unwrap_err() + .to_string() + .starts_with("Ambiguous global 'count'")); + } + } + } +}