diff --git a/bootstrap.example.toml b/bootstrap.example.toml index 418562609a9b9..8229538ca3ce0 100644 --- a/bootstrap.example.toml +++ b/bootstrap.example.toml @@ -343,10 +343,12 @@ # The path to (or name of) the GDB executable to use. This is only used for # executing the debuginfo test suite. +# Set this to "discover" to automatically discover GDB from the environment. #build.gdb = "gdb" # The path to (or name of) the LLDB executable to use. This is only used for # executing the debuginfo test suite. +# Set this to "discover" to automatically discover LLDB from the environment. #build.lldb = "lldb" # The node.js executable to use. Note that this is only used for the emscripten diff --git a/src/bootstrap/src/core/build_steps/test.rs b/src/bootstrap/src/core/build_steps/test.rs index 44bedaf878e04..093a45c1e39f6 100644 --- a/src/bootstrap/src/core/build_steps/test.rs +++ b/src/bootstrap/src/core/build_steps/test.rs @@ -2544,7 +2544,7 @@ Please disable assertions with `rust.debug-assertions = false`. if let Some(debuggers::Gdb { gdb }) = debuggers::discover_gdb(builder, android.as_ref()) { - cmd.arg("--gdb").arg(gdb.as_ref()); + cmd.arg("--gdb").arg(gdb); } if let Some(debuggers::Lldb { lldb_exe, lldb_version }) = diff --git a/src/bootstrap/src/core/config/config.rs b/src/bootstrap/src/core/config/config.rs index 74be2409570f8..8ba141d15a0bc 100644 --- a/src/bootstrap/src/core/config/config.rs +++ b/src/bootstrap/src/core/config/config.rs @@ -49,8 +49,9 @@ use crate::core::config::toml::target::{ DefaultLinuxLinkerOverride, Target, TomlTarget, default_linux_linker_overrides, }; use crate::core::config::{ - CompilerBuiltins, CompressDebuginfo, DebuginfoLevel, DryRun, GccCiMode, LlvmLibunwind, Merge, - OverrideAllocator, ReplaceOpt, RustcLto, SplitDebuginfo, StringOrBool, threads_from_config, + CompilerBuiltins, CompressDebuginfo, DebuggerPath, DebuginfoLevel, DryRun, GccCiMode, + LlvmLibunwind, Merge, OverrideAllocator, ReplaceOpt, RustcLto, SplitDebuginfo, StringOrBool, + threads_from_config, }; use crate::core::download::{ DownloadContext, download_beta_toolchain, is_download_ci_available, maybe_download_rustfmt, @@ -284,8 +285,8 @@ pub struct Config { pub codegen_tests: bool, pub nodejs: Option, pub yarn: Option, - pub gdb: Option, - pub lldb: Option, + pub gdb: Option, + pub lldb: Option, pub python: Option, pub windows_rc: Option, pub reuse: Option, @@ -1457,7 +1458,7 @@ NOTE: Please add `--stage 2` to your command line, or if you're sure you want to free_args: flags_free_args, full_bootstrap: build_full_bootstrap.unwrap_or(false), gcc_ci_mode, - gdb: build_gdb.map(PathBuf::from), + gdb: build_gdb, host_target, hosts, in_tree_gcc_info, @@ -1478,7 +1479,7 @@ NOTE: Please add `--stage 2` to your command line, or if you're sure you want to libgccjit_libs_dir: gcc_libgccjit_libs_dir, library_docs_private_items: build_library_docs_private_items.unwrap_or(false), lld_enabled, - lldb: build_lldb.map(PathBuf::from), + lldb: build_lldb, llvm_allow_old_toolchain: llvm_allow_old_toolchain.unwrap_or(false), llvm_assertions, llvm_bitcode_linker_enabled: rust_llvm_bitcode_linker.unwrap_or(false), diff --git a/src/bootstrap/src/core/config/mod.rs b/src/bootstrap/src/core/config/mod.rs index 6d7cca4897379..8a198a0d9436f 100644 --- a/src/bootstrap/src/core/config/mod.rs +++ b/src/bootstrap/src/core/config/mod.rs @@ -508,6 +508,27 @@ pub enum GccCiMode { DownloadFromCi, } +#[derive(Clone, Debug, PartialEq)] +pub enum DebuggerPath { + /// Use a debugger at this path + Path(PathBuf), + /// Try to automatically discover a version of a debugger from the environment + Discover, +} + +impl<'d> Deserialize<'d> for DebuggerPath { + fn deserialize(deserializer: D) -> Result>::Error> + where + D: serde::Deserializer<'d>, + { + let value = String::deserialize(deserializer)?; + match value.as_str() { + "discover" => Ok(Self::Discover), + path => Ok(Self::Path(PathBuf::from(path))), + } + } +} + pub fn threads_from_config(v: u32) -> u32 { match v { 0 => std::thread::available_parallelism().map_or(1, std::num::NonZeroUsize::get) as u32, diff --git a/src/bootstrap/src/core/config/tests.rs b/src/bootstrap/src/core/config/tests.rs index 91327057a8183..d84315d87e45a 100644 --- a/src/bootstrap/src/core/config/tests.rs +++ b/src/bootstrap/src/core/config/tests.rs @@ -11,7 +11,7 @@ use clap::CommandFactory; use super::flags::Flags; use super::toml::change_id::ChangeIdWrapper; use super::toml::rust::parse_codegen_backends; -use super::{Config, RUSTC_IF_UNCHANGED_ALLOWED_PATHS}; +use super::{Config, DebuggerPath, RUSTC_IF_UNCHANGED_ALLOWED_PATHS}; use crate::ChangeId; use crate::core::build_steps::clippy::{LintConfig, get_clippy_rules_in_order}; use crate::core::build_steps::llvm::LLVM_INVALIDATION_PATHS; @@ -111,7 +111,11 @@ fn override_toml() { crate::core::config::RustcLto::Fat, "setting string value without quotes" ); - assert_eq!(config.gdb, Some("bar".into()), "setting string value with quotes"); + assert_eq!( + config.gdb, + Some(DebuggerPath::Path("bar".into())), + "setting string value with quotes" + ); assert!(!config.deny_warnings, "setting boolean value"); assert_eq!( config.optimized_compiler_builtins, diff --git a/src/bootstrap/src/core/config/toml/build.rs b/src/bootstrap/src/core/config/toml/build.rs index 877dde676c605..9259dfd167fc4 100644 --- a/src/bootstrap/src/core/config/toml/build.rs +++ b/src/bootstrap/src/core/config/toml/build.rs @@ -11,7 +11,7 @@ use std::collections::HashMap; use serde::{Deserialize, Deserializer}; use crate::core::config::toml::ReplaceOpt; -use crate::core::config::{CompilerBuiltins, Merge, StringOrBool}; +use crate::core::config::{CompilerBuiltins, DebuggerPath, Merge, StringOrBool}; use crate::{HashSet, PathBuf, define_config, exit}; define_config! { @@ -33,8 +33,8 @@ define_config! { library_docs_private_items: Option = "library-docs-private-items", docs_minification: Option = "docs-minification", submodules: Option = "submodules", - gdb: Option = "gdb", - lldb: Option = "lldb", + gdb: Option = "gdb", + lldb: Option = "lldb", nodejs: Option = "nodejs", npm: Option = "npm", // unused, present for compatibility yarn: Option = "yarn", diff --git a/src/bootstrap/src/core/debuggers/gdb.rs b/src/bootstrap/src/core/debuggers/gdb.rs index 2eb441ee98523..3c2f7b8328461 100644 --- a/src/bootstrap/src/core/debuggers/gdb.rs +++ b/src/bootstrap/src/core/debuggers/gdb.rs @@ -1,39 +1,36 @@ -use std::borrow::Cow; -use std::path::Path; +use std::path::PathBuf; use crate::core::android; use crate::core::builder::Builder; +use crate::core::config::DebuggerPath; use crate::utils::exec::BootstrapCommand; -pub(crate) struct Gdb<'a> { - pub(crate) gdb: Cow<'a, Path>, +pub(crate) struct Gdb { + pub(crate) gdb: PathBuf, } -pub(crate) fn discover_gdb<'a>( - builder: &'a Builder<'_>, +pub(crate) fn discover_gdb( + builder: &Builder<'_>, android: Option<&android::Android>, -) -> Option> { +) -> 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 }); + match &builder.config.gdb { + Some(DebuggerPath::Path(path)) => { + return Some(Gdb { gdb: path.clone() }); + } + Some(DebuggerPath::Discover) => {} + None => return None, } // 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(), + let gdb = match android { + Some(android::Android { android_cross_path, .. }) => android_cross_path.join("bin/gdb"), + None => PathBuf::from("gdb"), }; // Check whether an ambient gdb exists, by running `gdb --version`. let output = { - let mut gdb_command = BootstrapCommand::new(gdb.as_ref()).allow_failure(); + let mut gdb_command = BootstrapCommand::new(&gdb).allow_failure(); gdb_command.arg("--version"); gdb_command.run_capture(builder) }; diff --git a/src/bootstrap/src/core/debuggers/lldb.rs b/src/bootstrap/src/core/debuggers/lldb.rs index 9cf8b40d751e5..5137d191244d5 100644 --- a/src/bootstrap/src/core/debuggers/lldb.rs +++ b/src/bootstrap/src/core/debuggers/lldb.rs @@ -1,6 +1,7 @@ use std::path::PathBuf; use crate::core::builder::Builder; +use crate::core::config::DebuggerPath; use crate::utils::exec::command; pub(crate) struct Lldb { @@ -9,17 +10,17 @@ pub(crate) struct Lldb { } 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")); + // If a path to a LLDB binary was provided, it has to exist and return some version, to avoid + // silent failures. + let (lldb_exe, explicitly_set_lldb) = match &builder.config.lldb { + Some(DebuggerPath::Path(path)) => (path.clone(), true), + Some(DebuggerPath::Discover) => (PathBuf::from("lldb"), false), + None => return None, + }; let mut cmd = command(&lldb_exe); cmd.arg("--version"); - // If a path to a LLDB binary was provided, it has to exist and return some version, to avoid - // silent failures. - let explicitly_set_lldb = builder.config.lldb.is_some(); if !explicitly_set_lldb { cmd = cmd.allow_failure(); } diff --git a/src/bootstrap/src/core/sanity.rs b/src/bootstrap/src/core/sanity.rs index 92b1f35b64f2f..51ece21324c3b 100644 --- a/src/bootstrap/src/core/sanity.rs +++ b/src/bootstrap/src/core/sanity.rs @@ -16,7 +16,7 @@ use std::{env, fs}; use crate::builder::{Builder, Kind}; use crate::core::build_steps::tool; -use crate::core::config::{CompilerBuiltins, Target}; +use crate::core::config::{CompilerBuiltins, DebuggerPath, Target}; use crate::utils::exec::command; use crate::{Build, Subcommand, t}; @@ -187,12 +187,10 @@ than building it. .map(|p| cmd_finder.must_have(p)) .or_else(|| cmd_finder.maybe_have("yarn")); - build.config.gdb = build - .config - .gdb - .take() - .map(|p| cmd_finder.must_have(p)) - .or_else(|| cmd_finder.maybe_have("gdb")); + build.config.gdb = build.config.gdb.take().map(|p| match p { + DebuggerPath::Discover => DebuggerPath::Discover, + DebuggerPath::Path(path) => DebuggerPath::Path(cmd_finder.must_have(path)), + }); build.config.reuse = build .config diff --git a/src/bootstrap/src/utils/change_tracker.rs b/src/bootstrap/src/utils/change_tracker.rs index 4252f2683b3d9..e5f28d7e57efb 100644 --- a/src/bootstrap/src/utils/change_tracker.rs +++ b/src/bootstrap/src/utils/change_tracker.rs @@ -651,4 +651,9 @@ pub const CONFIG_CHANGE_HISTORY: &[ChangeInfo] = &[ severity: ChangeSeverity::Info, summary: "A new `build.sde` configuration option has been added to support intrinsic-test.", }, + ChangeInfo { + change_id: 159455, + severity: ChangeSeverity::Warning, + summary: "GDB and LLDB are no longer automatically discovered from the environment. If you want to use path discovery for them, you can opt in using `build.gdb = \"discover\"` or `build.lldb = \"discover\"`.", + }, ]; diff --git a/src/ci/run.sh b/src/ci/run.sh index 339d9ffa55faf..194723eb3252e 100755 --- a/src/ci/run.sh +++ b/src/ci/run.sh @@ -140,6 +140,11 @@ else # No need to compress debuginfo for tests, and we do not want to depend on zlib RUST_CONFIGURE_ARGS="$RUST_CONFIGURE_ARGS --set rust.compress-debuginfo=off" + # Enable GDB/LLDB automatic discovery on CI + # Ideally, we should later change this to provide the paths explicitly + RUST_CONFIGURE_ARGS="$RUST_CONFIGURE_ARGS --set build.gdb=discover" + RUST_CONFIGURE_ARGS="$RUST_CONFIGURE_ARGS --set build.lldb=discover" + # We almost always want debug assertions enabled, but sometimes this takes too # long for too little benefit, so we just turn them off. if [ "$NO_DEBUG_ASSERTIONS" = "" ]; then