From 270c1a4d24ba4b244037c3fa1651d17a5f77eae4 Mon Sep 17 00:00:00 2001 From: bjorn3 <17426603+bjorn3@users.noreply.github.com> Date: Wed, 6 Aug 2025 18:00:58 +0000 Subject: [PATCH 1/2] Revert "Embed GDB pretty printers in rlibs and dylibs" This reverts commit b4d923cea0509933b1fb859930cb20784251f9be. --- .../rustc_codegen_llvm/src/debuginfo/gdb.rs | 43 ++++++++++++++----- compiler/rustc_codegen_ssa/src/base.rs | 10 +---- .../src/directives/directive_names.rs | 1 - tests/debuginfo/embedded-visualizer.rs | 2 - 4 files changed, 33 insertions(+), 23 deletions(-) diff --git a/compiler/rustc_codegen_llvm/src/debuginfo/gdb.rs b/compiler/rustc_codegen_llvm/src/debuginfo/gdb.rs index fccd32dec9537..b3e978be5701f 100644 --- a/compiler/rustc_codegen_llvm/src/debuginfo/gdb.rs +++ b/compiler/rustc_codegen_llvm/src/debuginfo/gdb.rs @@ -1,13 +1,13 @@ // .debug_gdb_scripts binary section. -use std::collections::BTreeSet; use std::ffi::CString; +use rustc_codegen_ssa::base::collect_debugger_visualizers_transitive; use rustc_codegen_ssa::traits::*; use rustc_hir::def_id::LOCAL_CRATE; use rustc_middle::bug; use rustc_middle::middle::debugger_visualizer::DebuggerVisualizerType; -use rustc_session::config::DebugInfo; +use rustc_session::config::{CrateType, DebugInfo}; use crate::builder::Builder; use crate::common::CodegenCx; @@ -51,14 +51,10 @@ pub(crate) fn get_or_insert_gdb_debug_scripts_section_global<'ll>( // Next, add the pretty printers that were specified via the `#[debugger_visualizer]` // attribute. - let visualizers = cx - .tcx - .debugger_visualizers(LOCAL_CRATE) - .iter() - .filter(|visualizer| { - visualizer.visualizer_type == DebuggerVisualizerType::GdbPrettyPrinter - }) - .collect::>(); + let visualizers = collect_debugger_visualizers_transitive( + cx.tcx, + DebuggerVisualizerType::GdbPrettyPrinter, + ); let crate_name = cx.tcx.crate_name(LOCAL_CRATE); for (index, visualizer) in visualizers.iter().enumerate() { // The initial byte `4` instructs GDB that the following pretty printer @@ -95,5 +91,30 @@ pub(crate) fn get_or_insert_gdb_debug_scripts_section_global<'ll>( } pub(crate) fn needs_gdb_debug_scripts_section(cx: &CodegenCx<'_, '_>) -> bool { - cx.sess().opts.debuginfo != DebugInfo::None && cx.sess().target.emit_debug_gdb_scripts + // We collect pretty printers transitively for all crates, so we make sure + // that the section is only emitted for leaf crates. + let embed_visualizers = cx.tcx.crate_types().iter().any(|&crate_type| match crate_type { + CrateType::Executable | CrateType::Cdylib | CrateType::Staticlib | CrateType::Sdylib => { + // These are crate types for which we will embed pretty printers since they + // are treated as leaf crates. + true + } + CrateType::ProcMacro => { + // We could embed pretty printers for proc macro crates too but it does not + // seem like a good default, since this is a rare use case and we don't + // want to slow down the common case. + false + } + CrateType::Rlib | CrateType::Dylib => { + // Don't embed pretty printers for these crate types; the compiler + // can see the `#[debug_visualizer]` attributes when using the + // library, and emitting `.debug_gdb_scripts` regardless would + // break `#![omit_gdb_pretty_printer_section]`. + false + } + }); + + cx.sess().opts.debuginfo != DebugInfo::None + && cx.sess().target.emit_debug_gdb_scripts + && embed_visualizers } diff --git a/compiler/rustc_codegen_ssa/src/base.rs b/compiler/rustc_codegen_ssa/src/base.rs index b4556ced0b3fb..eb21468650c76 100644 --- a/compiler/rustc_codegen_ssa/src/base.rs +++ b/compiler/rustc_codegen_ssa/src/base.rs @@ -609,15 +609,7 @@ pub fn collect_debugger_visualizers_transitive( ) -> BTreeSet { tcx.debugger_visualizers(LOCAL_CRATE) .iter() - .chain( - tcx.crates(()) - .iter() - .filter(|&cnum| { - let used_crate_source = tcx.used_crate_source(*cnum); - used_crate_source.rlib.is_some() || used_crate_source.rmeta.is_some() - }) - .flat_map(|&cnum| tcx.debugger_visualizers(cnum)), - ) + .chain(tcx.crates(()).iter().flat_map(|&cnum| tcx.debugger_visualizers(cnum))) .filter(|visualizer| visualizer.visualizer_type == visualizer_type) .cloned() .collect::>() diff --git a/src/tools/compiletest/src/directives/directive_names.rs b/src/tools/compiletest/src/directives/directive_names.rs index 099058bf0511a..f7955429d8369 100644 --- a/src/tools/compiletest/src/directives/directive_names.rs +++ b/src/tools/compiletest/src/directives/directive_names.rs @@ -68,7 +68,6 @@ pub(crate) const KNOWN_DIRECTIVE_NAMES: &[&str] = &[ "ignore-gnu", "ignore-haiku", "ignore-horizon", - "ignore-i586-unknown-linux-gnu", "ignore-i686-pc-windows-gnu", "ignore-i686-pc-windows-msvc", "ignore-illumos", diff --git a/tests/debuginfo/embedded-visualizer.rs b/tests/debuginfo/embedded-visualizer.rs index 12d87be7c66a0..cbd8691394d54 100644 --- a/tests/debuginfo/embedded-visualizer.rs +++ b/tests/debuginfo/embedded-visualizer.rs @@ -1,8 +1,6 @@ //@ compile-flags:-g //@ ignore-lldb //@ ignore-windows-gnu: #128981 -//@ ignore-musl: linker too old in CI -//@ ignore-i586-unknown-linux-gnu: linker too old in CI // === CDB TESTS ================================================================================== From e02cc40ec96457ab563273a69d314418cea7eb84 Mon Sep 17 00:00:00 2001 From: bjorn3 <17426603+bjorn3@users.noreply.github.com> Date: Wed, 6 Aug 2025 18:01:07 +0000 Subject: [PATCH 2/2] Revert "Preserve the .debug_gdb_scripts section" This reverts commit 868bdde25b030e0b71a29a5dbc04a891036e702e. --- compiler/rustc_codegen_gcc/src/debuginfo.rs | 3 +- compiler/rustc_codegen_llvm/src/base.rs | 12 +++---- .../rustc_codegen_llvm/src/debuginfo/gdb.rs | 30 ++++++++-------- .../rustc_codegen_llvm/src/debuginfo/mod.rs | 36 ++++++++----------- compiler/rustc_codegen_ssa/src/base.rs | 10 +++++- .../rustc_codegen_ssa/src/traits/debuginfo.rs | 2 +- tests/codegen-llvm/gdb_debug_script_load.rs | 4 +-- tests/run-make/symbols-all-mangled/rmake.rs | 35 ++++++++---------- 8 files changed, 60 insertions(+), 72 deletions(-) diff --git a/compiler/rustc_codegen_gcc/src/debuginfo.rs b/compiler/rustc_codegen_gcc/src/debuginfo.rs index 4c0b643952320..4c8585192a1b1 100644 --- a/compiler/rustc_codegen_gcc/src/debuginfo.rs +++ b/compiler/rustc_codegen_gcc/src/debuginfo.rs @@ -254,8 +254,7 @@ impl<'gcc, 'tcx> DebugInfoCodegenMethods<'tcx> for CodegenCx<'gcc, 'tcx> { // TODO(antoyo): implement. } - fn debuginfo_finalize(&mut self) { - // TODO: emit section `.debug_gdb_scripts`. + fn debuginfo_finalize(&self) { self.context.set_debug_info(true) } diff --git a/compiler/rustc_codegen_llvm/src/base.rs b/compiler/rustc_codegen_llvm/src/base.rs index d7da03bf490fd..5dda836988c81 100644 --- a/compiler/rustc_codegen_llvm/src/base.rs +++ b/compiler/rustc_codegen_llvm/src/base.rs @@ -109,16 +109,11 @@ pub(crate) fn compile_codegen_unit( } // Finalize code coverage by injecting the coverage map. Note, the coverage map will - // also be added to the `llvm.compiler.used` variable, created below. + // also be added to the `llvm.compiler.used` variable, created next. if cx.sess().instrument_coverage() { cx.coverageinfo_finalize(); } - // Finalize debuginfo. This adds to `llvm.used`, created below. - if cx.sess().opts.debuginfo != DebugInfo::None { - cx.debuginfo_finalize(); - } - // Create the llvm.used and llvm.compiler.used variables. if !cx.used_statics.is_empty() { cx.create_used_variable_impl(c"llvm.used", &cx.used_statics); @@ -135,6 +130,11 @@ pub(crate) fn compile_codegen_unit( llvm::LLVMDeleteGlobal(old_g); } } + + // Finalize debuginfo + if cx.sess().opts.debuginfo != DebugInfo::None { + cx.debuginfo_finalize(); + } } ModuleCodegen::new_regular(cgu_name.to_string(), llvm_module) diff --git a/compiler/rustc_codegen_llvm/src/debuginfo/gdb.rs b/compiler/rustc_codegen_llvm/src/debuginfo/gdb.rs index b3e978be5701f..6eb7042da6171 100644 --- a/compiler/rustc_codegen_llvm/src/debuginfo/gdb.rs +++ b/compiler/rustc_codegen_llvm/src/debuginfo/gdb.rs @@ -1,7 +1,5 @@ // .debug_gdb_scripts binary section. -use std::ffi::CString; - use rustc_codegen_ssa::base::collect_debugger_visualizers_transitive; use rustc_codegen_ssa::traits::*; use rustc_hir::def_id::LOCAL_CRATE; @@ -33,12 +31,7 @@ pub(crate) fn insert_reference_to_gdb_debug_scripts_section_global(bx: &mut Buil pub(crate) fn get_or_insert_gdb_debug_scripts_section_global<'ll>( cx: &CodegenCx<'ll, '_>, ) -> &'ll Value { - let c_section_var_name = CString::new(format!( - "__rustc_debug_gdb_scripts_section_{}_{:08x}", - cx.tcx.crate_name(LOCAL_CRATE), - cx.tcx.stable_crate_id(LOCAL_CRATE), - )) - .unwrap(); + let c_section_var_name = c"__rustc_debug_gdb_scripts_section__"; let section_var_name = c_section_var_name.to_str().unwrap(); let section_var = unsafe { llvm::LLVMGetNamedGlobal(cx.llmod, c_section_var_name.as_ptr()) }; @@ -91,10 +84,17 @@ pub(crate) fn get_or_insert_gdb_debug_scripts_section_global<'ll>( } pub(crate) fn needs_gdb_debug_scripts_section(cx: &CodegenCx<'_, '_>) -> bool { - // We collect pretty printers transitively for all crates, so we make sure - // that the section is only emitted for leaf crates. + // To ensure the section `__rustc_debug_gdb_scripts_section__` will not create + // ODR violations at link time, this section will not be emitted for rlibs since + // each rlib could produce a different set of visualizers that would be embedded + // in the `.debug_gdb_scripts` section. For that reason, we make sure that the + // section is only emitted for leaf crates. let embed_visualizers = cx.tcx.crate_types().iter().any(|&crate_type| match crate_type { - CrateType::Executable | CrateType::Cdylib | CrateType::Staticlib | CrateType::Sdylib => { + CrateType::Executable + | CrateType::Dylib + | CrateType::Cdylib + | CrateType::Staticlib + | CrateType::Sdylib => { // These are crate types for which we will embed pretty printers since they // are treated as leaf crates. true @@ -105,11 +105,9 @@ pub(crate) fn needs_gdb_debug_scripts_section(cx: &CodegenCx<'_, '_>) -> bool { // want to slow down the common case. false } - CrateType::Rlib | CrateType::Dylib => { - // Don't embed pretty printers for these crate types; the compiler - // can see the `#[debug_visualizer]` attributes when using the - // library, and emitting `.debug_gdb_scripts` regardless would - // break `#![omit_gdb_pretty_printer_section]`. + CrateType::Rlib => { + // As per the above description, embedding pretty printers for rlibs could + // lead to ODR violations so we skip this crate type as well. false } }); diff --git a/compiler/rustc_codegen_llvm/src/debuginfo/mod.rs b/compiler/rustc_codegen_llvm/src/debuginfo/mod.rs index c911435967c3e..6cbf2dbf7d3fd 100644 --- a/compiler/rustc_codegen_llvm/src/debuginfo/mod.rs +++ b/compiler/rustc_codegen_llvm/src/debuginfo/mod.rs @@ -30,7 +30,7 @@ use tracing::debug; use self::metadata::{UNKNOWN_COLUMN_NUMBER, UNKNOWN_LINE_NUMBER, file_metadata, type_di_node}; use self::namespace::mangled_name_of_instance; -use self::utils::{DIB, create_DIArray, debug_context, is_node_local_to_unit}; +use self::utils::{DIB, create_DIArray, is_node_local_to_unit}; use crate::builder::Builder; use crate::common::{AsCCharPtr, CodegenCx}; use crate::llvm; @@ -131,28 +131,20 @@ impl<'ll, 'tcx> CodegenUnitDebugContext<'ll, 'tcx> { } /// Creates any deferred debug metadata nodes -pub(crate) fn finalize(cx: &mut CodegenCx<'_, '_>) { - if cx.dbg_cx.is_none() { - return; - } - - debug!("finalize"); - - if gdb::needs_gdb_debug_scripts_section(cx) { - // Add a .debug_gdb_scripts section to this compile-unit. This will - // cause GDB to try and load the gdb_load_rust_pretty_printers.py file, - // which activates the Rust pretty printers for binary this section is - // contained in. - let section_var = gdb::get_or_insert_gdb_debug_scripts_section_global(cx); +pub(crate) fn finalize(cx: &CodegenCx<'_, '_>) { + if let Some(dbg_cx) = &cx.dbg_cx { + debug!("finalize"); + + if gdb::needs_gdb_debug_scripts_section(cx) { + // Add a .debug_gdb_scripts section to this compile-unit. This will + // cause GDB to try and load the gdb_load_rust_pretty_printers.py file, + // which activates the Rust pretty printers for binary this section is + // contained in. + gdb::get_or_insert_gdb_debug_scripts_section_global(cx); + } - // Make sure that the linker doesn't optimize the global away. Adding - // it to `llvm.used` has the advantage that it works even in no_std - // binaries, where we don't have a main shim and thus don't emit a - // volatile load to preserve the global. - cx.add_used_global(section_var); + dbg_cx.finalize(cx.sess()); } - - debug_context(cx).finalize(cx.sess()); } impl<'ll> Builder<'_, 'll, '_> { @@ -622,7 +614,7 @@ impl<'ll, 'tcx> DebugInfoCodegenMethods<'tcx> for CodegenCx<'ll, 'tcx> { metadata::extend_scope_to_file(self, scope_metadata, file) } - fn debuginfo_finalize(&mut self) { + fn debuginfo_finalize(&self) { finalize(self) } diff --git a/compiler/rustc_codegen_ssa/src/base.rs b/compiler/rustc_codegen_ssa/src/base.rs index eb21468650c76..b4556ced0b3fb 100644 --- a/compiler/rustc_codegen_ssa/src/base.rs +++ b/compiler/rustc_codegen_ssa/src/base.rs @@ -609,7 +609,15 @@ pub fn collect_debugger_visualizers_transitive( ) -> BTreeSet { tcx.debugger_visualizers(LOCAL_CRATE) .iter() - .chain(tcx.crates(()).iter().flat_map(|&cnum| tcx.debugger_visualizers(cnum))) + .chain( + tcx.crates(()) + .iter() + .filter(|&cnum| { + let used_crate_source = tcx.used_crate_source(*cnum); + used_crate_source.rlib.is_some() || used_crate_source.rmeta.is_some() + }) + .flat_map(|&cnum| tcx.debugger_visualizers(cnum)), + ) .filter(|visualizer| visualizer.visualizer_type == visualizer_type) .cloned() .collect::>() diff --git a/compiler/rustc_codegen_ssa/src/traits/debuginfo.rs b/compiler/rustc_codegen_ssa/src/traits/debuginfo.rs index 30a3bd5abe492..b9d4950e0ad36 100644 --- a/compiler/rustc_codegen_ssa/src/traits/debuginfo.rs +++ b/compiler/rustc_codegen_ssa/src/traits/debuginfo.rs @@ -50,7 +50,7 @@ pub trait DebugInfoCodegenMethods<'tcx>: BackendTypes { scope_metadata: Self::DIScope, file: &SourceFile, ) -> Self::DIScope; - fn debuginfo_finalize(&mut self); + fn debuginfo_finalize(&self); // FIXME(eddyb) find a common convention for all of the debuginfo-related // names (choose between `dbg`, `debug`, `debuginfo`, `debug_info` etc.). diff --git a/tests/codegen-llvm/gdb_debug_script_load.rs b/tests/codegen-llvm/gdb_debug_script_load.rs index 90f2be41cf2ac..3e92eba10b121 100644 --- a/tests/codegen-llvm/gdb_debug_script_load.rs +++ b/tests/codegen-llvm/gdb_debug_script_load.rs @@ -9,8 +9,6 @@ #![feature(lang_items)] #![no_std] -// CHECK: @llvm.used = {{.+}} @__rustc_debug_gdb_scripts_section - #[panic_handler] fn panic_handler(_: &core::panic::PanicInfo) -> ! { loop {} @@ -24,7 +22,7 @@ extern "C" fn rust_eh_personality() { // Needs rustc to generate `main` as that's where the magic load is inserted. // IOW, we cannot write this test with `#![no_main]`. // CHECK-LABEL: @main -// CHECK: load volatile i8, {{.+}} @__rustc_debug_gdb_scripts_section +// CHECK: load volatile i8, {{.+}} @__rustc_debug_gdb_scripts_section__ #[lang = "start"] fn lang_start( diff --git a/tests/run-make/symbols-all-mangled/rmake.rs b/tests/run-make/symbols-all-mangled/rmake.rs index e30bef9858079..2cf579758002a 100644 --- a/tests/run-make/symbols-all-mangled/rmake.rs +++ b/tests/run-make/symbols-all-mangled/rmake.rs @@ -35,7 +35,13 @@ fn symbols_check_archive(path: &str) { continue; // All compiler-builtins symbols must remain unmangled } - if symbol_ok_everywhere(name) { + if name.contains("rust_eh_personality") { + continue; // Unfortunately LLVM doesn't allow us to mangle this symbol + } + + if name.contains(".llvm.") { + // Starting in LLVM 21 we get various implementation-detail functions which + // contain .llvm. that are not a problem. continue; } @@ -65,7 +71,13 @@ fn symbols_check(path: &str) { continue; } - if symbol_ok_everywhere(name) { + if name.contains("rust_eh_personality") { + continue; // Unfortunately LLVM doesn't allow us to mangle this symbol + } + + if name.contains(".llvm.") { + // Starting in LLVM 21 we get various implementation-detail functions which + // contain .llvm. that are not a problem. continue; } @@ -76,22 +88,3 @@ fn symbols_check(path: &str) { fn strip_underscore_if_apple(symbol: &str) -> &str { if cfg!(target_vendor = "apple") { symbol.strip_prefix("_").unwrap() } else { symbol } } - -fn symbol_ok_everywhere(name: &str) -> bool { - if name.contains("rust_eh_personality") { - return true; // Unfortunately LLVM doesn't allow us to mangle this symbol - } - - if name.contains(".llvm.") { - // Starting in LLVM 21 we get various implementation-detail functions which - // contain .llvm. that are not a problem. - return true; - } - - if name.starts_with("__rustc_debug_gdb_scripts_section") { - // These symbols are fine; they're made unique by the crate ID. - return true; - } - - return false; -}