From 2044a5f51b12960ee2df64f62275bcf4598bd258 Mon Sep 17 00:00:00 2001 From: Zalathar Date: Fri, 24 Oct 2025 17:03:26 +1100 Subject: [PATCH 1/3] Separate debugger discovery from debugger version-query --- src/tools/compiletest/src/debuggers.rs | 32 +++++++++++++------------- src/tools/compiletest/src/lib.rs | 7 +++--- 2 files changed, 20 insertions(+), 19 deletions(-) diff --git a/src/tools/compiletest/src/debuggers.rs b/src/tools/compiletest/src/debuggers.rs index 8afe3289fa45a..93f27fb90951c 100644 --- a/src/tools/compiletest/src/debuggers.rs +++ b/src/tools/compiletest/src/debuggers.rs @@ -103,22 +103,19 @@ fn find_cdb(target: &str) -> Option { } /// Returns Path to CDB -pub(crate) fn analyze_cdb( - cdb: Option, - target: &str, -) -> (Option, Option<[u16; 4]>) { +pub(crate) fn discover_cdb(cdb: Option, target: &str) -> Option { let cdb = cdb.map(Utf8PathBuf::from).or_else(|| find_cdb(target)); + cdb +} +pub(crate) fn query_cdb_version(cdb: &Utf8Path) -> Option<[u16; 4]> { let mut version = None; - if let Some(cdb) = cdb.as_ref() { - if let Ok(output) = Command::new(cdb).arg("/version").output() { - if let Some(first_line) = String::from_utf8_lossy(&output.stdout).lines().next() { - version = extract_cdb_version(&first_line); - } + if let Ok(output) = Command::new(cdb).arg("/version").output() { + if let Some(first_line) = String::from_utf8_lossy(&output.stdout).lines().next() { + version = extract_cdb_version(&first_line); } } - - (cdb, version) + version } pub(crate) fn extract_cdb_version(full_version_line: &str) -> Option<[u16; 4]> { @@ -132,12 +129,11 @@ pub(crate) fn extract_cdb_version(full_version_line: &str) -> Option<[u16; 4]> { Some([major, minor, patch, build]) } -/// Returns (Path to GDB, GDB Version) -pub(crate) fn analyze_gdb( +pub(crate) fn discover_gdb( gdb: Option, target: &str, android_cross_path: &Utf8Path, -) -> (Option, Option) { +) -> Option { #[cfg(not(windows))] const GDB_FALLBACK: &str = "gdb"; #[cfg(windows)] @@ -159,6 +155,10 @@ pub(crate) fn analyze_gdb( Some(ref s) => s.to_owned(), }; + Some(gdb) +} + +pub(crate) fn query_gdb_version(gdb: &str) -> Option { let mut version_line = None; if let Ok(output) = Command::new(&gdb).arg("--version").output() { if let Some(first_line) = String::from_utf8_lossy(&output.stdout).lines().next() { @@ -168,10 +168,10 @@ pub(crate) fn analyze_gdb( let version = match version_line { Some(line) => extract_gdb_version(&line), - None => return (None, None), + None => return None, }; - (Some(gdb), version) + version } pub(crate) fn extract_gdb_version(full_version_line: &str) -> Option { diff --git a/src/tools/compiletest/src/lib.rs b/src/tools/compiletest/src/lib.rs index b524017e4dadd..89f5623c1fc99 100644 --- a/src/tools/compiletest/src/lib.rs +++ b/src/tools/compiletest/src/lib.rs @@ -258,10 +258,11 @@ fn parse_config(args: Vec) -> Config { let target = opt_str2(matches.opt_str("target")); let android_cross_path = opt_path(matches, "android-cross-path"); // FIXME: `cdb_version` is *derived* from cdb, but it's *not* technically a config! - let (cdb, cdb_version) = debuggers::analyze_cdb(matches.opt_str("cdb"), &target); + let cdb = debuggers::discover_cdb(matches.opt_str("cdb"), &target); + let cdb_version = cdb.as_deref().and_then(debuggers::query_cdb_version); // FIXME: `gdb_version` is *derived* from gdb, but it's *not* technically a config! - let (gdb, gdb_version) = - debuggers::analyze_gdb(matches.opt_str("gdb"), &target, &android_cross_path); + let gdb = debuggers::discover_gdb(matches.opt_str("gdb"), &target, &android_cross_path); + let gdb_version = gdb.as_deref().and_then(debuggers::query_gdb_version); // FIXME: `lldb_version` is *derived* from lldb, but it's *not* technically a config! let lldb_version = matches.opt_str("lldb-version").as_deref().and_then(debuggers::extract_lldb_version); From e9f360330d86f8ffd9cee7c71a83b12027d436d7 Mon Sep 17 00:00:00 2001 From: Zalathar Date: Sat, 25 Oct 2025 20:42:50 +1100 Subject: [PATCH 2/3] Only pass debugger-related flags for `debuginfo` tests --- src/bootstrap/src/core/build_steps/test.rs | 64 ++++++++++++---------- 1 file changed, 35 insertions(+), 29 deletions(-) diff --git a/src/bootstrap/src/core/build_steps/test.rs b/src/bootstrap/src/core/build_steps/test.rs index e1b3ff92a07b8..9a6b47bdf3a1a 100644 --- a/src/bootstrap/src/core/build_steps/test.rs +++ b/src/bootstrap/src/core/build_steps/test.rs @@ -2078,27 +2078,43 @@ Please disable assertions with `rust.debug-assertions = false`. cmd.arg("--python").arg(builder.python()); - if let Some(ref gdb) = builder.config.gdb { - cmd.arg("--gdb").arg(gdb); - } - - let lldb_exe = builder.config.lldb.clone().unwrap_or_else(|| PathBuf::from("lldb")); - let lldb_version = command(&lldb_exe) - .allow_failure() - .arg("--version") - .run_capture(builder) - .stdout_if_ok() - .and_then(|v| if v.trim().is_empty() { None } else { Some(v) }); - if let Some(ref vers) = lldb_version { - cmd.arg("--lldb-version").arg(vers); - let lldb_python_dir = command(&lldb_exe) + // FIXME(#148099): Currently we set these Android-related flags in all + // modes, even though they should only be needed in "debuginfo" mode, + // because the GDB-discovery code in compiletest currently assumes that + // `--android-cross-path` is always set for Android targets. + cmd.arg("--adb-path").arg("adb"); + cmd.arg("--adb-test-dir").arg(ADB_TEST_DIR); + if target.contains("android") && !builder.config.dry_run() { + // Assume that cc for this target comes from the android sysroot + cmd.arg("--android-cross-path") + .arg(builder.cc(target).parent().unwrap().parent().unwrap()); + } else { + cmd.arg("--android-cross-path").arg(""); + } + + if mode == "debuginfo" { + if let Some(ref gdb) = builder.config.gdb { + cmd.arg("--gdb").arg(gdb); + } + + let lldb_exe = builder.config.lldb.clone().unwrap_or_else(|| PathBuf::from("lldb")); + let lldb_version = command(&lldb_exe) .allow_failure() - .arg("-P") - .run_capture_stdout(builder) + .arg("--version") + .run_capture(builder) .stdout_if_ok() - .map(|p| p.lines().next().expect("lldb Python dir not found").to_string()); - if let Some(ref dir) = lldb_python_dir { - cmd.arg("--lldb-python-dir").arg(dir); + .and_then(|v| if v.trim().is_empty() { None } else { Some(v) }); + if let Some(ref vers) = lldb_version { + cmd.arg("--lldb-version").arg(vers); + let lldb_python_dir = command(&lldb_exe) + .allow_failure() + .arg("-P") + .run_capture_stdout(builder) + .stdout_if_ok() + .map(|p| p.lines().next().expect("lldb Python dir not found").to_string()); + if let Some(ref dir) = lldb_python_dir { + cmd.arg("--lldb-python-dir").arg(dir); + } } } @@ -2332,16 +2348,6 @@ Please disable assertions with `rust.debug-assertions = false`. cmd.env("RUST_TEST_TMPDIR", builder.tempdir()); - cmd.arg("--adb-path").arg("adb"); - cmd.arg("--adb-test-dir").arg(ADB_TEST_DIR); - if target.contains("android") && !builder.config.dry_run() { - // Assume that cc for this target comes from the android sysroot - cmd.arg("--android-cross-path") - .arg(builder.cc(target).parent().unwrap().parent().unwrap()); - } else { - cmd.arg("--android-cross-path").arg(""); - } - if builder.config.cmd.rustfix_coverage() { cmd.arg("--rustfix-coverage"); } From bcfbd7401d76b6c3455376e584402cc2a6b22dce Mon Sep 17 00:00:00 2001 From: Zalathar Date: Sat, 25 Oct 2025 20:34:51 +1100 Subject: [PATCH 3/3] Extract some discovery code for debuginfo tests into submodules --- src/bootstrap/src/core/build_steps/test.rs | 42 +++++++-------------- src/bootstrap/src/core/debuggers/android.rs | 24 ++++++++++++ src/bootstrap/src/core/debuggers/gdb.rs | 13 +++++++ src/bootstrap/src/core/debuggers/lldb.rs | 32 ++++++++++++++++ src/bootstrap/src/core/debuggers/mod.rs | 10 +++++ src/bootstrap/src/core/mod.rs | 1 + 6 files changed, 93 insertions(+), 29 deletions(-) create mode 100644 src/bootstrap/src/core/debuggers/android.rs create mode 100644 src/bootstrap/src/core/debuggers/gdb.rs create mode 100644 src/bootstrap/src/core/debuggers/lldb.rs create mode 100644 src/bootstrap/src/core/debuggers/mod.rs diff --git a/src/bootstrap/src/core/build_steps/test.rs b/src/bootstrap/src/core/build_steps/test.rs index 9a6b47bdf3a1a..57dedcccf98c8 100644 --- a/src/bootstrap/src/core/build_steps/test.rs +++ b/src/bootstrap/src/core/build_steps/test.rs @@ -29,6 +29,7 @@ use crate::core::builder::{ }; use crate::core::config::TargetSelection; use crate::core::config::flags::{Subcommand, get_completion, top_level_help}; +use crate::core::debuggers; use crate::utils::build_stamp::{self, BuildStamp}; use crate::utils::exec::{BootstrapCommand, command}; use crate::utils::helpers::{ @@ -38,8 +39,6 @@ use crate::utils::helpers::{ use crate::utils::render_tests::{add_flags_and_try_run_tests, try_run_tests}; use crate::{CLang, CodegenBackendKind, DocTests, GitRepo, Mode, PathSet, envify}; -const ADB_TEST_DIR: &str = "/data/local/tmp/work"; - /// Runs `cargo test` on various internal tools used by bootstrap. #[derive(Debug, Clone, PartialEq, Eq, Hash)] pub struct CrateBootstrap { @@ -2082,39 +2081,24 @@ Please disable assertions with `rust.debug-assertions = false`. // modes, even though they should only be needed in "debuginfo" mode, // because the GDB-discovery code in compiletest currently assumes that // `--android-cross-path` is always set for Android targets. - cmd.arg("--adb-path").arg("adb"); - cmd.arg("--adb-test-dir").arg(ADB_TEST_DIR); - if target.contains("android") && !builder.config.dry_run() { - // Assume that cc for this target comes from the android sysroot - cmd.arg("--android-cross-path") - .arg(builder.cc(target).parent().unwrap().parent().unwrap()); - } else { - cmd.arg("--android-cross-path").arg(""); + if let Some(debuggers::Android { adb_path, adb_test_dir, android_cross_path }) = + debuggers::discover_android(builder, target) + { + cmd.arg("--adb-path").arg(adb_path); + cmd.arg("--adb-test-dir").arg(adb_test_dir); + cmd.arg("--android-cross-path").arg(android_cross_path); } if mode == "debuginfo" { - if let Some(ref gdb) = builder.config.gdb { + if let Some(debuggers::Gdb { gdb }) = debuggers::discover_gdb(builder) { cmd.arg("--gdb").arg(gdb); } - let lldb_exe = builder.config.lldb.clone().unwrap_or_else(|| PathBuf::from("lldb")); - let lldb_version = command(&lldb_exe) - .allow_failure() - .arg("--version") - .run_capture(builder) - .stdout_if_ok() - .and_then(|v| if v.trim().is_empty() { None } else { Some(v) }); - if let Some(ref vers) = lldb_version { - cmd.arg("--lldb-version").arg(vers); - let lldb_python_dir = command(&lldb_exe) - .allow_failure() - .arg("-P") - .run_capture_stdout(builder) - .stdout_if_ok() - .map(|p| p.lines().next().expect("lldb Python dir not found").to_string()); - if let Some(ref dir) = lldb_python_dir { - cmd.arg("--lldb-python-dir").arg(dir); - } + if let Some(debuggers::Lldb { lldb_version, lldb_python_dir }) = + debuggers::discover_lldb(builder) + { + cmd.arg("--lldb-version").arg(lldb_version); + cmd.arg("--lldb-python-dir").arg(lldb_python_dir); } } diff --git a/src/bootstrap/src/core/debuggers/android.rs b/src/bootstrap/src/core/debuggers/android.rs new file mode 100644 index 0000000000000..b1ad9ca555fbd --- /dev/null +++ b/src/bootstrap/src/core/debuggers/android.rs @@ -0,0 +1,24 @@ +use std::path::PathBuf; + +use crate::core::builder::Builder; +use crate::core::config::TargetSelection; + +pub(crate) struct Android { + pub(crate) adb_path: &'static str, + pub(crate) adb_test_dir: &'static str, + pub(crate) android_cross_path: PathBuf, +} + +pub(crate) fn discover_android(builder: &Builder<'_>, target: TargetSelection) -> Option { + let adb_path = "adb"; + // See . + let adb_test_dir = "/data/local/tmp/work"; + + let android_cross_path = if target.contains("android") && !builder.config.dry_run() { + builder.cc(target).parent().unwrap().parent().unwrap().to_owned() + } else { + PathBuf::new() + }; + + Some(Android { adb_path, adb_test_dir, android_cross_path }) +} diff --git a/src/bootstrap/src/core/debuggers/gdb.rs b/src/bootstrap/src/core/debuggers/gdb.rs new file mode 100644 index 0000000000000..ddad0909e4f07 --- /dev/null +++ b/src/bootstrap/src/core/debuggers/gdb.rs @@ -0,0 +1,13 @@ +use std::path::Path; + +use crate::core::builder::Builder; + +pub(crate) struct Gdb<'a> { + pub(crate) gdb: &'a Path, +} + +pub(crate) fn discover_gdb<'a>(builder: &'a Builder<'_>) -> Option> { + let gdb = builder.config.gdb.as_deref()?; + + Some(Gdb { gdb }) +} diff --git a/src/bootstrap/src/core/debuggers/lldb.rs b/src/bootstrap/src/core/debuggers/lldb.rs new file mode 100644 index 0000000000000..66ab45573d6bb --- /dev/null +++ b/src/bootstrap/src/core/debuggers/lldb.rs @@ -0,0 +1,32 @@ +use std::path::PathBuf; + +use crate::core::builder::Builder; +use crate::utils::exec::command; + +pub(crate) struct Lldb { + pub(crate) lldb_version: String, + pub(crate) lldb_python_dir: String, +} + +pub(crate) fn discover_lldb(builder: &Builder<'_>) -> Option { + // FIXME(#148361): We probably should not be picking up whatever arbitrary + // lldb happens to be in the user's path, and instead require some kind of + // explicit opt-in or configuration. + let lldb_exe = builder.config.lldb.clone().unwrap_or_else(|| PathBuf::from("lldb")); + + let lldb_version = command(&lldb_exe) + .allow_failure() + .arg("--version") + .run_capture(builder) + .stdout_if_ok() + .and_then(|v| if v.trim().is_empty() { None } else { Some(v) })?; + + let lldb_python_dir = command(&lldb_exe) + .allow_failure() + .arg("-P") + .run_capture_stdout(builder) + .stdout_if_ok() + .map(|p| p.lines().next().expect("lldb Python dir not found").to_string())?; + + Some(Lldb { lldb_version, lldb_python_dir }) +} diff --git a/src/bootstrap/src/core/debuggers/mod.rs b/src/bootstrap/src/core/debuggers/mod.rs new file mode 100644 index 0000000000000..011ce4081a43e --- /dev/null +++ b/src/bootstrap/src/core/debuggers/mod.rs @@ -0,0 +1,10 @@ +//! Code for discovering debuggers and debugger-related configuration, so that +//! it can be passed to compiletest when running debuginfo tests. + +pub(crate) use self::android::{Android, discover_android}; +pub(crate) use self::gdb::{Gdb, discover_gdb}; +pub(crate) use self::lldb::{Lldb, discover_lldb}; + +mod android; +mod gdb; +mod lldb; diff --git a/src/bootstrap/src/core/mod.rs b/src/bootstrap/src/core/mod.rs index 9e18d6704d4fe..f27a09c81c55b 100644 --- a/src/bootstrap/src/core/mod.rs +++ b/src/bootstrap/src/core/mod.rs @@ -1,6 +1,7 @@ pub(crate) mod build_steps; pub(crate) mod builder; pub(crate) mod config; +pub(crate) mod debuggers; pub(crate) mod download; pub(crate) mod metadata; pub(crate) mod sanity;