From 7593c50d431fdec34d9da2b820ce236f57757435 Mon Sep 17 00:00:00 2001 From: David Wood Date: Mon, 14 Feb 2022 03:39:32 +0000 Subject: [PATCH 1/4] bootstrap: add split-debuginfo config Replace `run-dysutil` option with more general `split-debuginfo` option that works on all platforms. Signed-off-by: David Wood --- config.toml.example | 29 +++++++++++++++++------ src/bootstrap/builder.rs | 20 +++++++--------- src/bootstrap/config.rs | 50 ++++++++++++++++++++++++++++++++++++++-- 3 files changed, 78 insertions(+), 21 deletions(-) diff --git a/config.toml.example b/config.toml.example index 6e53d9b442f16..b3946b67b0d4e 100644 --- a/config.toml.example +++ b/config.toml.example @@ -473,13 +473,28 @@ changelog-seen = 2 # FIXME(#61117): Some tests fail when this option is enabled. #debuginfo-level-tests = 0 -# Whether to run `dsymutil` on Apple platforms to gather debug info into .dSYM -# bundles. `dsymutil` adds time to builds for no clear benefit, and also makes -# it more difficult for debuggers to find debug info. The compiler currently -# defaults to running `dsymutil` to preserve its historical default, but when -# compiling the compiler itself, we skip it by default since we know it's safe -# to do so in that case. -#run-dsymutil = false +# Should rustc be build with split debuginfo? Default is platform dependent. +# Valid values are the same as those accepted by `-C split-debuginfo` +# (`off`/`unpacked`/`packed`). +# +# On Linux, packed split debuginfo is used by default, which splits debuginfo +# into a separate `rustc.dwp` file. Split DWARF on Linux results in lower +# linking times (there's less debuginfo for the linker to process), +# `split-debuginfo` is enabled on default for Linux. Unpacked debuginfo could +# technically work too, but the cost of running the DWARF packager is marginal +# and results in debuginfo being in a single file. +# +# On Apple platforms, unpacked split debuginfo is used by default. Unpacked +# debuginfo does not run `dsymutil`, which packages debuginfo from disparate +# object files into a single `.dSYM` file. `dsymutil` adds time to builds for +# no clear benefit, and also makes it more difficult for debuggers to find +# debug info. The compiler currently defaults to running `dsymutil` to preserve +# its historical default, but when compiling the compiler itself, we skip it by +# default since we know it's safe to do so in that case. +# +# On Windows platforms, packed debuginfo is the only supported option, +# producing a `.pdb` file. +#split-debuginfo = if linux { packed } else if windows { packed } else if apple { unpacked } # Whether or not `panic!`s generate backtraces (RUST_BACKTRACE) #backtrace = true diff --git a/src/bootstrap/builder.rs b/src/bootstrap/builder.rs index 0276d15a5b472..02d4dedec5fbe 100644 --- a/src/bootstrap/builder.rs +++ b/src/bootstrap/builder.rs @@ -14,7 +14,7 @@ use std::time::{Duration, Instant}; use crate::cache::{Cache, Interned, INTERNER}; use crate::check; use crate::compile; -use crate::config::TargetSelection; +use crate::config::{SplitDebuginfo, TargetSelection}; use crate::dist; use crate::doc; use crate::flags::{Color, Subcommand}; @@ -1365,18 +1365,14 @@ impl<'a> Builder<'a> { }, ); - // `dsymutil` adds time to builds on Apple platforms for no clear benefit, and also makes - // it more difficult for debuggers to find debug info. The compiler currently defaults to - // running `dsymutil` to preserve its historical default, but when compiling the compiler - // itself, we skip it by default since we know it's safe to do so in that case. - // See https://github.com/rust-lang/rust/issues/79361 for more info on this flag. - if target.contains("apple") { - if self.config.rust_run_dsymutil { - rustflags.arg("-Csplit-debuginfo=packed"); - } else { - rustflags.arg("-Csplit-debuginfo=unpacked"); - } + if target.contains("linux") || target.contains("windows") { + rustflags.arg("-Zunstable-options"); } + match self.config.rust_split_debuginfo { + SplitDebuginfo::Packed => rustflags.arg("-Csplit-debuginfo=packed"), + SplitDebuginfo::Unpacked => rustflags.arg("-Csplit-debuginfo=unpacked"), + SplitDebuginfo::Off => rustflags.arg("-Csplit-debuginfo=off"), + }; if self.config.cmd.bless() { // Bless `expect!` tests. diff --git a/src/bootstrap/config.rs b/src/bootstrap/config.rs index d7c29f6900a53..a0c260780554d 100644 --- a/src/bootstrap/config.rs +++ b/src/bootstrap/config.rs @@ -130,7 +130,7 @@ pub struct Config { pub rust_debuginfo_level_std: u32, pub rust_debuginfo_level_tools: u32, pub rust_debuginfo_level_tests: u32, - pub rust_run_dsymutil: bool, + pub rust_split_debuginfo: SplitDebuginfo, pub rust_rpath: bool, pub rustc_parallel: bool, pub rustc_default_linker: Option, @@ -221,6 +221,46 @@ impl FromStr for LlvmLibunwind { } } +#[derive(Copy, Clone, PartialEq, Eq, PartialOrd, Ord, Hash)] +pub enum SplitDebuginfo { + Packed, + Unpacked, + Off, +} + +impl Default for SplitDebuginfo { + fn default() -> Self { + SplitDebuginfo::Off + } +} + +impl std::str::FromStr for SplitDebuginfo { + type Err = (); + + fn from_str(s: &str) -> Result { + match s { + "packed" => Ok(SplitDebuginfo::Packed), + "unpacked" => Ok(SplitDebuginfo::Unpacked), + "off" => Ok(SplitDebuginfo::Off), + _ => Err(()), + } + } +} + +impl SplitDebuginfo { + /// Returns the default `-Csplit-debuginfo` value for the current target. See the comment for + /// `rust.split-debuginfo` in `config.toml.example`. + fn default_for_platform(target: &str) -> Self { + if target.contains("apple") { + SplitDebuginfo::Unpacked + } else if target.contains("windows") { + SplitDebuginfo::Packed + } else { + SplitDebuginfo::Unpacked + } + } +} + #[derive(Copy, Clone, Default, PartialEq, Eq, PartialOrd, Ord, Hash)] pub struct TargetSelection { pub triple: Interned, @@ -586,6 +626,7 @@ define_config! { debuginfo_level_std: Option = "debuginfo-level-std", debuginfo_level_tools: Option = "debuginfo-level-tools", debuginfo_level_tests: Option = "debuginfo-level-tests", + split_debuginfo: Option = "split-debuginfo", run_dsymutil: Option = "run-dsymutil", backtrace: Option = "backtrace", incremental: Option = "incremental", @@ -992,7 +1033,12 @@ impl Config { debuginfo_level_std = rust.debuginfo_level_std; debuginfo_level_tools = rust.debuginfo_level_tools; debuginfo_level_tests = rust.debuginfo_level_tests; - config.rust_run_dsymutil = rust.run_dsymutil.unwrap_or(false); + config.rust_split_debuginfo = rust + .split_debuginfo + .as_deref() + .map(SplitDebuginfo::from_str) + .map(|v| v.expect("invalid value for rust.split_debuginfo")) + .unwrap_or(SplitDebuginfo::default_for_platform(&config.build.triple)); optimize = rust.optimize; ignore_git = rust.ignore_git; config.rust_new_symbol_mangling = rust.new_symbol_mangling; From 1d207bbd51c6fee3d232782896f337b7d5f0c6bd Mon Sep 17 00:00:00 2001 From: David Wood Date: Sun, 3 Apr 2022 06:35:49 +0100 Subject: [PATCH 2/4] bootstrap: disable split dwarf by default Signed-off-by: David Wood --- config.toml.example | 9 ++------- src/bootstrap/config.rs | 2 +- 2 files changed, 3 insertions(+), 8 deletions(-) diff --git a/config.toml.example b/config.toml.example index b3946b67b0d4e..dd886879b145d 100644 --- a/config.toml.example +++ b/config.toml.example @@ -477,12 +477,7 @@ changelog-seen = 2 # Valid values are the same as those accepted by `-C split-debuginfo` # (`off`/`unpacked`/`packed`). # -# On Linux, packed split debuginfo is used by default, which splits debuginfo -# into a separate `rustc.dwp` file. Split DWARF on Linux results in lower -# linking times (there's less debuginfo for the linker to process), -# `split-debuginfo` is enabled on default for Linux. Unpacked debuginfo could -# technically work too, but the cost of running the DWARF packager is marginal -# and results in debuginfo being in a single file. +# On Linux, split debuginfo is disabled by default. # # On Apple platforms, unpacked split debuginfo is used by default. Unpacked # debuginfo does not run `dsymutil`, which packages debuginfo from disparate @@ -494,7 +489,7 @@ changelog-seen = 2 # # On Windows platforms, packed debuginfo is the only supported option, # producing a `.pdb` file. -#split-debuginfo = if linux { packed } else if windows { packed } else if apple { unpacked } +#split-debuginfo = if linux { off } else if windows { packed } else if apple { unpacked } # Whether or not `panic!`s generate backtraces (RUST_BACKTRACE) #backtrace = true diff --git a/src/bootstrap/config.rs b/src/bootstrap/config.rs index a0c260780554d..f273fb42215e9 100644 --- a/src/bootstrap/config.rs +++ b/src/bootstrap/config.rs @@ -256,7 +256,7 @@ impl SplitDebuginfo { } else if target.contains("windows") { SplitDebuginfo::Packed } else { - SplitDebuginfo::Unpacked + SplitDebuginfo::Off } } } From b786345347d6abf0b2053dfdc70e501940277e62 Mon Sep 17 00:00:00 2001 From: David Wood Date: Thu, 14 Apr 2022 11:47:26 +0100 Subject: [PATCH 3/4] ssa: don't pack debuginfo on windows not only msvc Small fix that prevents `thorin` from running on platforms where it definitely shouldn't be running. Signed-off-by: David Wood --- compiler/rustc_codegen_ssa/src/back/link.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/rustc_codegen_ssa/src/back/link.rs b/compiler/rustc_codegen_ssa/src/back/link.rs index cf32d558d4a51..4d20ed841d47c 100644 --- a/compiler/rustc_codegen_ssa/src/back/link.rs +++ b/compiler/rustc_codegen_ssa/src/back/link.rs @@ -987,7 +987,7 @@ fn link_natively<'a, B: ArchiveBuilder<'a>>( // On MSVC packed debug information is produced by the linker itself so // there's no need to do anything else here. - SplitDebuginfo::Packed if sess.target.is_like_msvc => {} + SplitDebuginfo::Packed if sess.target.is_like_windows => {} // ... and otherwise we're processing a `*.dwp` packed dwarf file. // From 65cc0ad455634ad75be88781fa373400cfd337a0 Mon Sep 17 00:00:00 2001 From: David Wood Date: Tue, 19 Apr 2022 03:32:30 +0100 Subject: [PATCH 4/4] bootstrap: non-bootstrap windows split debuginfo Temporarily, only enable split debuginfo on Windows if not building with the boostrap compiler as there is a bug that isn't fixed in the bootstrap compiler which would result in `thorin` being run on Windows. Signed-off-by: David Wood --- src/bootstrap/builder.rs | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/src/bootstrap/builder.rs b/src/bootstrap/builder.rs index 02d4dedec5fbe..8c68f14072772 100644 --- a/src/bootstrap/builder.rs +++ b/src/bootstrap/builder.rs @@ -1365,14 +1365,18 @@ impl<'a> Builder<'a> { }, ); - if target.contains("linux") || target.contains("windows") { - rustflags.arg("-Zunstable-options"); + // FIXME(davidtwco): #[cfg(not(bootstrap))] - #95612 needs to be in the bootstrap compiler + // for this conditional to be removed. + if !target.contains("windows") || compiler.stage >= 1 { + if target.contains("linux") || target.contains("windows") { + rustflags.arg("-Zunstable-options"); + } + match self.config.rust_split_debuginfo { + SplitDebuginfo::Packed => rustflags.arg("-Csplit-debuginfo=packed"), + SplitDebuginfo::Unpacked => rustflags.arg("-Csplit-debuginfo=unpacked"), + SplitDebuginfo::Off => rustflags.arg("-Csplit-debuginfo=off"), + }; } - match self.config.rust_split_debuginfo { - SplitDebuginfo::Packed => rustflags.arg("-Csplit-debuginfo=packed"), - SplitDebuginfo::Unpacked => rustflags.arg("-Csplit-debuginfo=unpacked"), - SplitDebuginfo::Off => rustflags.arg("-Csplit-debuginfo=off"), - }; if self.config.cmd.bless() { // Bless `expect!` tests.