From 0fb1e12a05e37676e7454eba1c4c0f361b4a12a3 Mon Sep 17 00:00:00 2001 From: Sebastian Poeplau Date: Thu, 3 Jul 2025 17:02:40 +0200 Subject: [PATCH] Preserve the .debug_gdb_scripts section Make sure that compiler and linker don't optimize the section's contents away by adding the global holding the data to "llvm.used". This eliminates the need for a volatile load in the main shim; since the LLVM codegen backend is the only implementer of the corresponding trait function, remove it entirely. Pretty printers in dylib dependencies are now emitted by the main crate instead of the dylib; apart from matching how rlibs are handled, this approach has the advantage that `omit_gdb_pretty_printer_section` keeps working with dylib dependencies. --- compiler/rustc_codegen_gcc/src/debuginfo.rs | 7 +-- compiler/rustc_codegen_llvm/src/base.rs | 12 ++--- .../rustc_codegen_llvm/src/debuginfo/gdb.rs | 49 +++++++------------ .../rustc_codegen_llvm/src/debuginfo/mod.rs | 34 ++++++------- compiler/rustc_codegen_ssa/src/base.rs | 12 +---- .../rustc_codegen_ssa/src/traits/debuginfo.rs | 3 +- src/tools/compiletest/src/directives.rs | 1 + tests/codegen/gdb_debug_script_load.rs | 26 ++-------- tests/debuginfo/embedded-visualizer.rs | 1 + tests/run-make/symbols-all-mangled/rmake.rs | 35 +++++++------ 10 files changed, 71 insertions(+), 109 deletions(-) diff --git a/compiler/rustc_codegen_gcc/src/debuginfo.rs b/compiler/rustc_codegen_gcc/src/debuginfo.rs index 4c8585192a1b1..51b2cd31fe612 100644 --- a/compiler/rustc_codegen_gcc/src/debuginfo.rs +++ b/compiler/rustc_codegen_gcc/src/debuginfo.rs @@ -36,10 +36,6 @@ impl<'a, 'gcc, 'tcx> DebugInfoBuilderMethods for Builder<'a, 'gcc, 'tcx> { _variable_alloca.set_location(_dbg_loc); } - fn insert_reference_to_gdb_debug_scripts_section_global(&mut self) { - // TODO(antoyo): insert reference to gdb debug scripts section global. - } - /// FIXME(tempdragon): Currently, this function is not yet implemented. It seems that the /// debug name and the mangled name should both be included in the LValues. /// Besides, a function to get the rvalue type(m_is_lvalue) should also be included. @@ -254,7 +250,8 @@ impl<'gcc, 'tcx> DebugInfoCodegenMethods<'tcx> for CodegenCx<'gcc, 'tcx> { // TODO(antoyo): implement. } - fn debuginfo_finalize(&self) { + fn debuginfo_finalize(&mut self) { + // TODO: emit section `.debug_gdb_scripts`. 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 5dda836988c81..d7da03bf490fd 100644 --- a/compiler/rustc_codegen_llvm/src/base.rs +++ b/compiler/rustc_codegen_llvm/src/base.rs @@ -109,11 +109,16 @@ 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 next. + // also be added to the `llvm.compiler.used` variable, created below. 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); @@ -130,11 +135,6 @@ 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 49ee96a41d696..c54bba3a90631 100644 --- a/compiler/rustc_codegen_llvm/src/debuginfo/gdb.rs +++ b/compiler/rustc_codegen_llvm/src/debuginfo/gdb.rs @@ -1,5 +1,7 @@ // .debug_gdb_scripts binary section. +use std::ffi::CString; + use rustc_attr_data_structures::{AttributeKind, find_attr}; use rustc_codegen_ssa::base::collect_debugger_visualizers_transitive; use rustc_codegen_ssa::traits::*; @@ -8,31 +10,21 @@ use rustc_middle::bug; use rustc_middle::middle::debugger_visualizer::DebuggerVisualizerType; use rustc_session::config::{CrateType, DebugInfo}; -use crate::builder::Builder; use crate::common::CodegenCx; use crate::llvm; use crate::value::Value; -/// Inserts a side-effect free instruction sequence that makes sure that the -/// .debug_gdb_scripts global is referenced, so it isn't removed by the linker. -pub(crate) fn insert_reference_to_gdb_debug_scripts_section_global(bx: &mut Builder<'_, '_, '_>) { - if needs_gdb_debug_scripts_section(bx) { - let gdb_debug_scripts_section = get_or_insert_gdb_debug_scripts_section_global(bx); - // Load just the first byte as that's all that's necessary to force - // LLVM to keep around the reference to the global. - let volatile_load_instruction = bx.volatile_load(bx.type_i8(), gdb_debug_scripts_section); - unsafe { - llvm::LLVMSetAlignment(volatile_load_instruction, 1); - } - } -} - /// Allocates the global variable responsible for the .debug_gdb_scripts binary /// section. pub(crate) fn get_or_insert_gdb_debug_scripts_section_global<'ll>( - cx: &CodegenCx<'ll, '_>, + cx: &mut CodegenCx<'ll, '_>, ) -> &'ll Value { - let c_section_var_name = c"__rustc_debug_gdb_scripts_section__"; + 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 section_var_name = c_section_var_name.to_str().unwrap(); let section_var = unsafe { llvm::LLVMGetNamedGlobal(cx.llmod, c_section_var_name.as_ptr()) }; @@ -79,6 +71,8 @@ pub(crate) fn get_or_insert_gdb_debug_scripts_section_global<'ll>( // This should make sure that the whole section is not larger than // the string it contains. Otherwise we get a warning from GDB. llvm::LLVMSetAlignment(section_var, 1); + // Make sure that the linker doesn't optimize the global away. + cx.add_used_global(section_var); section_var } }) @@ -88,17 +82,10 @@ pub(crate) fn needs_gdb_debug_scripts_section(cx: &CodegenCx<'_, '_>) -> bool { let omit_gdb_pretty_printer_section = find_attr!(cx.tcx.hir_krate_attrs(), AttributeKind::OmitGdbPrettyPrinterSection); - // 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. + // 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::Dylib - | CrateType::Cdylib - | CrateType::Staticlib - | CrateType::Sdylib => { + 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 @@ -109,9 +96,11 @@ pub(crate) fn needs_gdb_debug_scripts_section(cx: &CodegenCx<'_, '_>) -> bool { // want to slow down the common case. false } - 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. + 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 } }); diff --git a/compiler/rustc_codegen_llvm/src/debuginfo/mod.rs b/compiler/rustc_codegen_llvm/src/debuginfo/mod.rs index 5ca2505cec43b..1e3d4275a3fc3 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, is_node_local_to_unit}; +use self::utils::{DIB, create_DIArray, debug_context, is_node_local_to_unit}; use crate::builder::Builder; use crate::common::{AsCCharPtr, CodegenCx}; use crate::llvm; @@ -131,20 +131,22 @@ impl<'ll, 'tcx> CodegenUnitDebugContext<'ll, 'tcx> { } /// Creates any deferred debug metadata nodes -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); - } +pub(crate) fn finalize(cx: &mut CodegenCx<'_, '_>) { + if cx.dbg_cx.is_none() { + return; + } + + debug!("finalize"); - dbg_cx.finalize(cx.sess()); + 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); } + + debug_context(cx).finalize(cx.sess()); } impl<'ll> Builder<'_, 'll, '_> { @@ -215,10 +217,6 @@ impl<'ll> DebugInfoBuilderMethods for Builder<'_, 'll, '_> { } } - fn insert_reference_to_gdb_debug_scripts_section_global(&mut self) { - gdb::insert_reference_to_gdb_debug_scripts_section_global(self) - } - fn set_var_name(&mut self, value: &'ll Value, name: &str) { // Avoid wasting time if LLVM value names aren't even enabled. if self.sess().fewer_names() { @@ -614,7 +612,7 @@ impl<'ll, 'tcx> DebugInfoCodegenMethods<'tcx> for CodegenCx<'ll, 'tcx> { metadata::extend_scope_to_file(self, scope_metadata, file) } - fn debuginfo_finalize(&self) { + fn debuginfo_finalize(&mut self) { finalize(self) } diff --git a/compiler/rustc_codegen_ssa/src/base.rs b/compiler/rustc_codegen_ssa/src/base.rs index 18581f854b664..47d20780d28d4 100644 --- a/compiler/rustc_codegen_ssa/src/base.rs +++ b/compiler/rustc_codegen_ssa/src/base.rs @@ -528,8 +528,6 @@ pub fn maybe_create_entry_wrapper<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>>( let llbb = Bx::append_block(cx, llfn, "top"); let mut bx = Bx::build(cx, llbb); - bx.insert_reference_to_gdb_debug_scripts_section_global(); - let isize_ty = cx.type_isize(); let ptr_ty = cx.type_ptr(); let (arg_argc, arg_argv) = get_argc_argv(&mut bx); @@ -609,15 +607,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/compiler/rustc_codegen_ssa/src/traits/debuginfo.rs b/compiler/rustc_codegen_ssa/src/traits/debuginfo.rs index b9d4950e0ad36..e5dfe0b2cda48 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(&self); + fn debuginfo_finalize(&mut self); // FIXME(eddyb) find a common convention for all of the debuginfo-related // names (choose between `dbg`, `debug`, `debuginfo`, `debug_info` etc.). @@ -81,6 +81,5 @@ pub trait DebugInfoBuilderMethods: BackendTypes { ); fn set_dbg_loc(&mut self, dbg_loc: Self::DILocation); fn clear_dbg_loc(&mut self); - fn insert_reference_to_gdb_debug_scripts_section_global(&mut self); fn set_var_name(&mut self, value: Self::Value, name: &str); } diff --git a/src/tools/compiletest/src/directives.rs b/src/tools/compiletest/src/directives.rs index 6bf968a3132ef..ff032c207901f 100644 --- a/src/tools/compiletest/src/directives.rs +++ b/src/tools/compiletest/src/directives.rs @@ -831,6 +831,7 @@ 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/codegen/gdb_debug_script_load.rs b/tests/codegen/gdb_debug_script_load.rs index 3e92eba10b121..cc7170460ed32 100644 --- a/tests/codegen/gdb_debug_script_load.rs +++ b/tests/codegen/gdb_debug_script_load.rs @@ -4,34 +4,14 @@ //@ ignore-wasm //@ ignore-emscripten -//@ compile-flags: -g -C no-prepopulate-passes -Cpanic=abort +//@ compile-flags: -g -Cpanic=abort -#![feature(lang_items)] #![no_std] +#![no_main] #[panic_handler] fn panic_handler(_: &core::panic::PanicInfo) -> ! { loop {} } -#[no_mangle] -extern "C" fn rust_eh_personality() { - loop {} -} - -// 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__ - -#[lang = "start"] -fn lang_start( - _main: fn() -> T, - _argc: isize, - _argv: *const *const u8, - _sigpipe: u8, -) -> isize { - return 0; -} - -fn main() {} +// CHECK: @llvm.used = {{.+}} @__rustc_debug_gdb_scripts_section diff --git a/tests/debuginfo/embedded-visualizer.rs b/tests/debuginfo/embedded-visualizer.rs index cbd8691394d54..878de39075ba9 100644 --- a/tests/debuginfo/embedded-visualizer.rs +++ b/tests/debuginfo/embedded-visualizer.rs @@ -1,6 +1,7 @@ //@ compile-flags:-g //@ ignore-lldb //@ ignore-windows-gnu: #128981 +//@ ignore-i586-unknown-linux-gnu: linker too old in CI // === CDB TESTS ================================================================================== diff --git a/tests/run-make/symbols-all-mangled/rmake.rs b/tests/run-make/symbols-all-mangled/rmake.rs index 2cf579758002a..e30bef9858079 100644 --- a/tests/run-make/symbols-all-mangled/rmake.rs +++ b/tests/run-make/symbols-all-mangled/rmake.rs @@ -35,13 +35,7 @@ fn symbols_check_archive(path: &str) { continue; // All compiler-builtins symbols must remain unmangled } - 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. + if symbol_ok_everywhere(name) { continue; } @@ -71,13 +65,7 @@ fn symbols_check(path: &str) { continue; } - 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. + if symbol_ok_everywhere(name) { continue; } @@ -88,3 +76,22 @@ 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; +}