From 85f7a6ed610933f32966483dc4aff2399928f3e9 Mon Sep 17 00:00:00 2001 From: Zalathar Date: Sun, 7 Dec 2025 13:40:38 +1100 Subject: [PATCH 1/2] Move Android-related discovery out of `core::debuggers` While some of this information is needed by debugger discovery, it is also needed by non-debuginfo tests, so the code doesn't belong in the `debuggers` module. --- src/bootstrap/src/core/{debuggers => }/android.rs | 0 src/bootstrap/src/core/build_steps/test.rs | 11 ++++------- src/bootstrap/src/core/debuggers/mod.rs | 2 -- src/bootstrap/src/core/mod.rs | 1 + 4 files changed, 5 insertions(+), 9 deletions(-) rename src/bootstrap/src/core/{debuggers => }/android.rs (100%) diff --git a/src/bootstrap/src/core/debuggers/android.rs b/src/bootstrap/src/core/android.rs similarity index 100% rename from src/bootstrap/src/core/debuggers/android.rs rename to src/bootstrap/src/core/android.rs diff --git a/src/bootstrap/src/core/build_steps/test.rs b/src/bootstrap/src/core/build_steps/test.rs index 7e46b85ff9af3..64c8c1e566139 100644 --- a/src/bootstrap/src/core/build_steps/test.rs +++ b/src/bootstrap/src/core/build_steps/test.rs @@ -29,7 +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::core::{android, debuggers}; use crate::utils::build_stamp::{self, BuildStamp}; use crate::utils::exec::{BootstrapCommand, command}; use crate::utils::helpers::{ @@ -2114,12 +2114,9 @@ Please disable assertions with `rust.debug-assertions = false`. builder.config.python.as_ref().expect("python is required for running rustdoc tests"), ); - // 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. - if let Some(debuggers::Android { adb_path, adb_test_dir, android_cross_path }) = - debuggers::discover_android(builder, target) + // Discover and set some flags related to running tests on Android targets. + if let Some(android::Android { adb_path, adb_test_dir, android_cross_path }) = + android::discover_android(builder, target) { cmd.arg("--adb-path").arg(adb_path); cmd.arg("--adb-test-dir").arg(adb_test_dir); diff --git a/src/bootstrap/src/core/debuggers/mod.rs b/src/bootstrap/src/core/debuggers/mod.rs index 011ce4081a43e..a11eda8e686e3 100644 --- a/src/bootstrap/src/core/debuggers/mod.rs +++ b/src/bootstrap/src/core/debuggers/mod.rs @@ -1,10 +1,8 @@ //! 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 f27a09c81c55b..4df2ed319da68 100644 --- a/src/bootstrap/src/core/mod.rs +++ b/src/bootstrap/src/core/mod.rs @@ -1,3 +1,4 @@ +pub(crate) mod android; pub(crate) mod build_steps; pub(crate) mod builder; pub(crate) mod config; From 8f35bd17cc4f88d3069a78d30890e1cfcf007ef1 Mon Sep 17 00:00:00 2001 From: Zalathar Date: Sat, 6 Dec 2025 22:22:09 +1100 Subject: [PATCH 2/2] Move ambient gdb discovery from compiletest to bootstrap --- src/bootstrap/src/core/android.rs | 6 ++- src/bootstrap/src/core/build_steps/test.rs | 10 ++--- src/bootstrap/src/core/debuggers/gdb.rs | 37 ++++++++++++++++-- src/tools/compiletest/src/common.rs | 2 +- src/tools/compiletest/src/debuggers.rs | 38 ------------------- src/tools/compiletest/src/lib.rs | 4 +- .../compiletest/src/runtest/debuginfo.rs | 8 ++-- 7 files changed, 51 insertions(+), 54 deletions(-) diff --git a/src/bootstrap/src/core/android.rs b/src/bootstrap/src/core/android.rs index b1ad9ca555fbd..60f7ca4dbcd0c 100644 --- a/src/bootstrap/src/core/android.rs +++ b/src/bootstrap/src/core/android.rs @@ -10,11 +10,15 @@ pub(crate) struct Android { } pub(crate) fn discover_android(builder: &Builder<'_>, target: TargetSelection) -> Option { + if !target.contains("android") { + return None; + } + 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() { + let android_cross_path = if !builder.config.dry_run() { builder.cc(target).parent().unwrap().parent().unwrap().to_owned() } else { PathBuf::new() diff --git a/src/bootstrap/src/core/build_steps/test.rs b/src/bootstrap/src/core/build_steps/test.rs index 64c8c1e566139..4237d39d74255 100644 --- a/src/bootstrap/src/core/build_steps/test.rs +++ b/src/bootstrap/src/core/build_steps/test.rs @@ -2115,17 +2115,17 @@ Please disable assertions with `rust.debug-assertions = false`. ); // Discover and set some flags related to running tests on Android targets. - if let Some(android::Android { adb_path, adb_test_dir, android_cross_path }) = - android::discover_android(builder, target) - { + let android = android::discover_android(builder, target); + if let Some(android::Android { adb_path, adb_test_dir, android_cross_path }) = &android { 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(debuggers::Gdb { gdb }) = debuggers::discover_gdb(builder) { - cmd.arg("--gdb").arg(gdb); + if let Some(debuggers::Gdb { gdb }) = debuggers::discover_gdb(builder, android.as_ref()) + { + cmd.arg("--gdb").arg(gdb.as_ref()); } if let Some(debuggers::Lldb { lldb_exe, lldb_version }) = diff --git a/src/bootstrap/src/core/debuggers/gdb.rs b/src/bootstrap/src/core/debuggers/gdb.rs index ddad0909e4f07..2eb441ee98523 100644 --- a/src/bootstrap/src/core/debuggers/gdb.rs +++ b/src/bootstrap/src/core/debuggers/gdb.rs @@ -1,13 +1,42 @@ +use std::borrow::Cow; use std::path::Path; +use crate::core::android; use crate::core::builder::Builder; +use crate::utils::exec::BootstrapCommand; pub(crate) struct Gdb<'a> { - pub(crate) gdb: &'a Path, + pub(crate) gdb: Cow<'a, Path>, } -pub(crate) fn discover_gdb<'a>(builder: &'a Builder<'_>) -> Option> { - let gdb = builder.config.gdb.as_deref()?; +pub(crate) fn discover_gdb<'a>( + builder: &'a Builder<'_>, + android: Option<&android::Android>, +) -> Option> { + // If there's an explicitly-configured gdb, use that. + if let Some(gdb) = builder.config.gdb.as_deref() { + // FIXME(Zalathar): Consider returning None if gdb is an empty string, + // as a way to explicitly disable ambient gdb discovery. + let gdb = Cow::Borrowed(gdb); + return Some(Gdb { gdb }); + } - Some(Gdb { gdb }) + // Otherwise, fall back to whatever gdb is sitting around in PATH. + // (That's the historical behavior, but maybe we should require opt-in?) + + let gdb: Cow<'_, Path> = match android { + Some(android::Android { android_cross_path, .. }) => { + android_cross_path.join("bin/gdb").into() + } + None => Path::new("gdb").into(), + }; + + // Check whether an ambient gdb exists, by running `gdb --version`. + let output = { + let mut gdb_command = BootstrapCommand::new(gdb.as_ref()).allow_failure(); + gdb_command.arg("--version"); + gdb_command.run_capture(builder) + }; + + if output.is_success() { Some(Gdb { gdb }) } else { None } } diff --git a/src/tools/compiletest/src/common.rs b/src/tools/compiletest/src/common.rs index d8472691afdf7..02b93593cbbd4 100644 --- a/src/tools/compiletest/src/common.rs +++ b/src/tools/compiletest/src/common.rs @@ -566,7 +566,7 @@ pub struct Config { /// /// FIXME: take a look at this; this is piggy-backing off of gdb code paths but only for /// `arm-linux-androideabi` target. - pub android_cross_path: Utf8PathBuf, + pub android_cross_path: Option, /// Extra parameter to run adb on `arm-linux-androideabi`. /// diff --git a/src/tools/compiletest/src/debuggers.rs b/src/tools/compiletest/src/debuggers.rs index 791c5f03c21a6..b0d0372454e2d 100644 --- a/src/tools/compiletest/src/debuggers.rs +++ b/src/tools/compiletest/src/debuggers.rs @@ -54,15 +54,6 @@ pub(crate) fn configure_lldb(config: &Config) -> Option> { Some(Arc::new(Config { debugger: Some(Debugger::Lldb), ..config.clone() })) } -/// Returns `true` if the given target is an Android target for the -/// purposes of GDB testing. -pub(crate) fn is_android_gdb_target(target: &str) -> bool { - matches!( - &target[..], - "arm-linux-androideabi" | "armv7-linux-androideabi" | "aarch64-linux-android" - ) -} - /// Returns `true` if the given target is a MSVC target for the purposes of CDB testing. fn is_pc_windows_msvc_target(target: &str) -> bool { target.ends_with("-pc-windows-msvc") @@ -129,35 +120,6 @@ pub(crate) fn extract_cdb_version(full_version_line: &str) -> Option<[u16; 4]> { Some([major, minor, patch, build]) } -pub(crate) fn discover_gdb( - gdb: Option, - target: &str, - android_cross_path: &Utf8Path, -) -> Option { - #[cfg(not(windows))] - const GDB_FALLBACK: &str = "gdb"; - #[cfg(windows)] - const GDB_FALLBACK: &str = "gdb.exe"; - - let fallback_gdb = || { - if is_android_gdb_target(target) { - let mut gdb_path = android_cross_path.to_string(); - gdb_path.push_str("/bin/gdb"); - gdb_path - } else { - GDB_FALLBACK.to_owned() - } - }; - - let gdb = match gdb { - None => fallback_gdb(), - Some(ref s) if s.is_empty() => fallback_gdb(), // may be empty if configure found no gdb - Some(ref s) => s.to_owned(), - }; - - Some(Utf8PathBuf::from(gdb)) -} - pub(crate) fn query_gdb_version(gdb: &Utf8Path) -> Option { let mut version_line = None; if let Ok(output) = Command::new(&gdb).arg("--version").output() { diff --git a/src/tools/compiletest/src/lib.rs b/src/tools/compiletest/src/lib.rs index acd0d70d081f1..f3bd467db3e41 100644 --- a/src/tools/compiletest/src/lib.rs +++ b/src/tools/compiletest/src/lib.rs @@ -256,12 +256,12 @@ fn parse_config(args: Vec) -> Config { } let target = opt_str2(matches.opt_str("target")); - let android_cross_path = opt_path(matches, "android-cross-path"); + let android_cross_path = matches.opt_str("android-cross-path").map(Utf8PathBuf::from); // FIXME: `cdb_version` is *derived* from cdb, but it's *not* technically a config! 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 = debuggers::discover_gdb(matches.opt_str("gdb"), &target, &android_cross_path); + let gdb = matches.opt_str("gdb").map(Utf8PathBuf::from); 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 = matches.opt_str("lldb").map(Utf8PathBuf::from); diff --git a/src/tools/compiletest/src/runtest/debuginfo.rs b/src/tools/compiletest/src/runtest/debuginfo.rs index 5ef4366b68be3..ac935910205b7 100644 --- a/src/tools/compiletest/src/runtest/debuginfo.rs +++ b/src/tools/compiletest/src/runtest/debuginfo.rs @@ -8,7 +8,7 @@ use tracing::debug; use super::debugger::DebuggerCommands; use super::{Debugger, Emit, ProcRes, TestCx, Truncated, WillExecute}; -use crate::debuggers::{extract_gdb_version, is_android_gdb_target}; +use crate::debuggers::extract_gdb_version; impl TestCx<'_> { pub(super) fn run_debuginfo_test(&self) { @@ -122,13 +122,15 @@ impl TestCx<'_> { let exe_file = self.make_exe_name(); let debugger_run_result; - if is_android_gdb_target(&self.config.target) { + // If bootstrap gave us an `--android-cross-path`, assume the target + // needs Android-specific handling. + if let Some(android_cross_path) = self.config.android_cross_path.as_deref() { cmds = cmds.replace("run", "continue"); // write debugger script let mut script_str = String::with_capacity(2048); script_str.push_str(&format!("set charset {}\n", Self::charset())); - script_str.push_str(&format!("set sysroot {}\n", &self.config.android_cross_path)); + script_str.push_str(&format!("set sysroot {android_cross_path}\n")); script_str.push_str(&format!("file {}\n", exe_file)); script_str.push_str("target remote :5039\n"); script_str.push_str(&format!(