From a58340103c26c8701079afe0d4e0bf38d9db386a Mon Sep 17 00:00:00 2001 From: swananan Date: Sat, 13 Jun 2026 20:53:10 +0800 Subject: [PATCH] fix: prefer strict debuglink matches in loose mode Scan every .gnu_debuglink candidate before accepting a loose match so a later strict CRC/Build-ID match can still win over an earlier mismatched debug file. Add startup load report e2e coverage for a bad configured search-path candidate followed by the strict same-directory debuglink file. --- .../tests/startup_load_report_execution.rs | 101 +++++++++++++++++- ghostscope-dwarf/src/binary/debuglink.rs | 23 +++- 2 files changed, 118 insertions(+), 6 deletions(-) diff --git a/e2e-tests/tests/startup_load_report_execution.rs b/e2e-tests/tests/startup_load_report_execution.rs index de3f322..2610202 100644 --- a/e2e-tests/tests/startup_load_report_execution.rs +++ b/e2e-tests/tests/startup_load_report_execution.rs @@ -18,6 +18,7 @@ const FIXTURE_DEBUG_FILE: &str = "debug_source_report.debug"; const EMBEDDED_BINARY: &str = "debug_source_report_embedded"; const NO_DEBUGLINK_BINARY: &str = "debug_source_report_no_debuglink"; const NO_DWARF_DEBUGLINK_BINARY: &str = "debug_source_report_no_dwarf_debuglink"; +const NO_DWARF_DEBUG_FILE: &str = "debug_source_report_no_dwarf.debug"; const MISSING_BINARY: &str = "debug_source_report_missing"; const BAD_DEBUG_FILE: &str = "bad.debug"; const TEST_CONFIG: &str = r#" @@ -98,6 +99,56 @@ async fn test_startup_report_shows_debuglink_source() -> Result<()> { Ok(()) } +#[tokio::test] +#[serial_test::serial] +async fn test_loose_debuglink_search_prefers_later_strict_match() -> Result<()> { + init(); + + if !is_host_topology() { + println!("skipping startup load report e2e outside host->host topology"); + return Ok(()); + } + + let fixture = ensure_startup_report_fixture()?; + let bad_search_dir = TempDir::new().context("failed to create bad debug search dir")?; + fs::copy( + &fixture.no_dwarf_debug_file, + bad_search_dir.path().join(FIXTURE_DEBUG_FILE), + ) + .context("failed to seed bad debug search path")?; + let config = startup_report_config(&[bad_search_dir.path()]); + let run = run_startup_report_command_for_binary_with_config( + &fixture, + &fixture.binary, + &[OsString::from("--allow-loose-debug-match")], + &config, + )?; + + assert!( + run.status.success(), + "loose debuglink search-order run failed with status {}\n{}", + run.status, + run.output + ); + assert_output_contains(&run.output, "Startup load report:"); + assert_output_contains(&run.output, "\x1b[34mdebuglink:1\x1b[0m"); + assert_output_contains(&run.output, "module details:"); + assert_output_contains(&run.output, "\x1b[34mdebuglink\x1b[0m"); + assert_output_contains(&run.output, FIXTURE_DEBUG_FILE); + assert!( + !run.output.contains("\x1b[33mmissing:1\x1b[0m"), + "loose mode should prefer a later strict match over an earlier bad candidate\n{}", + run.output + ); + assert!( + !run.output.contains("\x1b[33mmissing DWARF:\x1b[0m"), + "strict debuglink fallback should avoid missing-DWARF output\n{}", + run.output + ); + + Ok(()) +} + #[tokio::test] #[serial_test::serial] async fn test_startup_report_shows_missing_source_without_module_details() -> Result<()> { @@ -255,6 +306,7 @@ struct StartupReportFixture { embedded_binary: PathBuf, no_debuglink_binary: PathBuf, no_dwarf_debuglink_binary: PathBuf, + no_dwarf_debug_file: PathBuf, missing_binary: PathBuf, debug_file: PathBuf, bad_debug_file: PathBuf, @@ -276,6 +328,7 @@ fn ensure_startup_report_fixture() -> Result { embedded_binary: dir.join(EMBEDDED_BINARY), no_debuglink_binary: dir.join(NO_DEBUGLINK_BINARY), no_dwarf_debuglink_binary: dir.join(NO_DWARF_DEBUGLINK_BINARY), + no_dwarf_debug_file: dir.join(NO_DWARF_DEBUG_FILE), missing_binary: dir.join(MISSING_BINARY), debug_file: dir.join(FIXTURE_DEBUG_FILE), bad_debug_file: dir.join(BAD_DEBUG_FILE), @@ -317,6 +370,15 @@ fn run_startup_report_command_for_binary( fixture: &StartupReportFixture, binary: &Path, extra_args: &[OsString], +) -> Result { + run_startup_report_command_for_binary_with_config(fixture, binary, extra_args, TEST_CONFIG) +} + +fn run_startup_report_command_for_binary_with_config( + fixture: &StartupReportFixture, + binary: &Path, + extra_args: &[OsString], + config: &str, ) -> Result { let sandbox = common::sandbox::SandboxHandle::default_ghostscope()?; let (program, sandbox_args) = sandbox.ghostscope_command()?; @@ -326,7 +388,7 @@ fn run_startup_report_command_for_binary( let temp_dir = TempDir::new().context("failed to create startup report temp dir")?; let config_path = temp_dir.path().join("ghostscope.toml"); - fs::write(&config_path, TEST_CONFIG).with_context(|| { + fs::write(&config_path, config).with_context(|| { format!( "failed to write startup report config {}", config_path.display() @@ -365,6 +427,43 @@ fn run_startup_report_command_for_binary( }) } +fn startup_report_config(debug_search_paths: &[&Path]) -> String { + let mut config = String::from( + r#" +[general] +enable_logging = false +enable_console_logging = false + +[script] +status = true +color = "auto" +"#, + ); + + if !debug_search_paths.is_empty() { + let paths = debug_search_paths + .iter() + .map(|path| format!("\"{}\"", toml_string(path))) + .collect::>() + .join(", "); + config.push_str(&format!("\n[dwarf]\nsearch_paths = [{paths}]\n")); + } + + config.push_str( + r#" +[dwarf.debuginfod] +enabled = "off" +"#, + ); + config +} + +fn toml_string(path: &Path) -> String { + path.to_string_lossy() + .replace('\\', "\\\\") + .replace('"', "\\\"") +} + fn startup_report_fixture_dir() -> PathBuf { Path::new(env!("CARGO_MANIFEST_DIR")) .join("tests") diff --git a/ghostscope-dwarf/src/binary/debuglink.rs b/ghostscope-dwarf/src/binary/debuglink.rs index 8bf6a92..c7f1416 100644 --- a/ghostscope-dwarf/src/binary/debuglink.rs +++ b/ghostscope-dwarf/src/binary/debuglink.rs @@ -29,8 +29,8 @@ use std::path::{Path, PathBuf}; /// System-wide debug directories are searched when the caller includes them in /// search_paths; the default GhostScope config includes common system paths. /// -/// Returns the path to the debug file if found and CRC matches -/// Also verifies build ID if present in both files +/// Returns the path to the debug file if a strict CRC/Build-ID match is found, +/// or if loose mode falls back to the first mismatched candidate. pub fn find_debug_file>( binary_path: P, user_search_paths: &[String], @@ -77,7 +77,10 @@ pub fn find_debug_file>( // Build search paths following GDB's strategy let search_paths = build_search_paths(binary_path, debug_filename, user_search_paths); - // Try each path and verify CRC + build ID + // Try each path and verify CRC + build ID. Strict matches always win, even + // in loose mode; only fall back to the first mismatched candidate after the + // full search list has been checked. + let mut first_loose_candidate = None; for candidate_path in search_paths { tracing::debug!("Checking debug file path: {}", candidate_path.display()); @@ -94,10 +97,12 @@ pub fn find_debug_file>( Ok(false) => { if allow_loose_debug_match { tracing::warn!( - "Debug file {} exists but verification failed; loose match enabled -> using it", + "Debug file {} exists but verification failed; loose match enabled -> retaining as fallback", candidate_path.display() ); - return Ok(Some(candidate_path)); + if first_loose_candidate.is_none() { + first_loose_candidate = Some(candidate_path); + } } else { tracing::error!( "Debug file {} exists but verification failed (CRC or Build-ID mismatch)", @@ -116,6 +121,14 @@ pub fn find_debug_file>( } } + if let Some(candidate_path) = first_loose_candidate { + tracing::warn!( + "No strict matching debug file found; loose match enabled -> using {}", + candidate_path.display() + ); + return Ok(Some(candidate_path)); + } + tracing::warn!( "Debug file '{}' not found in any standard location", debug_filename.display()