From e26710610490da43b30e66747c713b382fe62794 Mon Sep 17 00:00:00 2001 From: Zalathar Date: Sun, 5 Jan 2025 11:38:43 +1100 Subject: [PATCH 1/3] Use gimli to get the values of DWARF constants needed by codegen The `gimli` crate is already a dependency of `thorin-dwp`, which is already a dependency of `rustc_codegen_ssa`. --- Cargo.lock | 1 + compiler/rustc_codegen_llvm/Cargo.toml | 1 + .../src/debuginfo/dwarf_const.rs | 29 +++++++++++++++++++ .../src/debuginfo/metadata.rs | 27 ++++------------- .../src/debuginfo/metadata/enums/cpp_like.rs | 7 +++-- .../rustc_codegen_llvm/src/debuginfo/mod.rs | 1 + 6 files changed, 42 insertions(+), 24 deletions(-) create mode 100644 compiler/rustc_codegen_llvm/src/debuginfo/dwarf_const.rs diff --git a/Cargo.lock b/Cargo.lock index a1365c2d202d2..caf7bf15fb27c 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3499,6 +3499,7 @@ name = "rustc_codegen_llvm" version = "0.0.0" dependencies = [ "bitflags", + "gimli 0.30.0", "itertools", "libc", "measureme", diff --git a/compiler/rustc_codegen_llvm/Cargo.toml b/compiler/rustc_codegen_llvm/Cargo.toml index 689986d642d90..c44d1a5e5c220 100644 --- a/compiler/rustc_codegen_llvm/Cargo.toml +++ b/compiler/rustc_codegen_llvm/Cargo.toml @@ -9,6 +9,7 @@ test = false [dependencies] # tidy-alphabetical-start bitflags = "2.4.1" +gimli = "0.30" itertools = "0.12" libc = "0.2" measureme = "11" diff --git a/compiler/rustc_codegen_llvm/src/debuginfo/dwarf_const.rs b/compiler/rustc_codegen_llvm/src/debuginfo/dwarf_const.rs new file mode 100644 index 0000000000000..f05422644041e --- /dev/null +++ b/compiler/rustc_codegen_llvm/src/debuginfo/dwarf_const.rs @@ -0,0 +1,29 @@ +//! Definitions of various DWARF-related constants. + +use libc::c_uint; + +/// Helper macro to let us redeclare gimli's constants as our own constants +/// with a different type, with less risk of copy-paste errors. +macro_rules! declare_constant { + ( + $name:ident : $type:ty + ) => { + #[allow(non_upper_case_globals)] + pub(crate) const $name: $type = ::gimli::constants::$name.0 as $type; + + // Assert that as-cast probably hasn't changed the value. + const _: () = assert!($name as i128 == ::gimli::constants::$name.0 as i128); + }; +} + +declare_constant!(DW_TAG_const_type: c_uint); + +// DWARF languages. +declare_constant!(DW_LANG_Rust: c_uint); + +// DWARF attribute type encodings. +declare_constant!(DW_ATE_boolean: c_uint); +declare_constant!(DW_ATE_float: c_uint); +declare_constant!(DW_ATE_signed: c_uint); +declare_constant!(DW_ATE_unsigned: c_uint); +declare_constant!(DW_ATE_UTF: c_uint); diff --git a/compiler/rustc_codegen_llvm/src/debuginfo/metadata.rs b/compiler/rustc_codegen_llvm/src/debuginfo/metadata.rs index 12411b21ba32c..88e43e1c678ad 100644 --- a/compiler/rustc_codegen_llvm/src/debuginfo/metadata.rs +++ b/compiler/rustc_codegen_llvm/src/debuginfo/metadata.rs @@ -22,6 +22,7 @@ use rustc_target::spec::DebuginfoKind; use smallvec::smallvec; use tracing::{debug, instrument}; +pub(crate) use self::type_map::TypeMap; use self::type_map::{DINodeCreationResult, Stub, UniqueTypeId}; use super::CodegenUnitDebugContext; use super::namespace::mangled_name_of_instance; @@ -30,6 +31,7 @@ use super::utils::{ DIB, create_DIArray, debug_context, get_namespace_for_item, is_node_local_to_unit, }; use crate::common::{AsCCharPtr, CodegenCx}; +use crate::debuginfo::dwarf_const; use crate::debuginfo::metadata::type_map::build_type_with_children; use crate::debuginfo::utils::{WidePtrKind, wide_pointer_kind}; use crate::llvm::debuginfo::{ @@ -59,23 +61,6 @@ impl fmt::Debug for llvm::Metadata { } } -// From DWARF 5. -// See http://www.dwarfstd.org/ShowIssue.php?issue=140129.1. -const DW_LANG_RUST: c_uint = 0x1c; -#[allow(non_upper_case_globals)] -const DW_ATE_boolean: c_uint = 0x02; -#[allow(non_upper_case_globals)] -const DW_ATE_float: c_uint = 0x04; -#[allow(non_upper_case_globals)] -const DW_ATE_signed: c_uint = 0x05; -#[allow(non_upper_case_globals)] -const DW_ATE_unsigned: c_uint = 0x07; -#[allow(non_upper_case_globals)] -const DW_ATE_UTF: c_uint = 0x10; - -#[allow(non_upper_case_globals)] -const DW_TAG_const_type: c_uint = 0x26; - pub(super) const UNKNOWN_LINE_NUMBER: c_uint = 0; pub(super) const UNKNOWN_COLUMN_NUMBER: c_uint = 0; @@ -90,8 +75,6 @@ type SmallVec = smallvec::SmallVec<[T; 16]>; mod enums; mod type_map; -pub(crate) use type_map::TypeMap; - /// Returns from the enclosing function if the type debuginfo node with the given /// unique ID can be found in the type map. macro_rules! return_if_di_node_created_in_meantime { @@ -522,7 +505,7 @@ fn recursion_marker_type_di_node<'ll, 'tcx>(cx: &CodegenCx<'ll, 'tcx>) -> &'ll D name.as_c_char_ptr(), name.len(), cx.tcx.data_layout.pointer_size.bits(), - DW_ATE_unsigned, + dwarf_const::DW_ATE_unsigned, ) } }) @@ -781,6 +764,8 @@ fn build_basic_type_di_node<'ll, 'tcx>( // .natvis visualizers (and perhaps other existing native debuggers?) let cpp_like_debuginfo = cpp_like_debuginfo(cx.tcx); + use dwarf_const::{DW_ATE_UTF, DW_ATE_boolean, DW_ATE_float, DW_ATE_signed, DW_ATE_unsigned}; + let (name, encoding) = match t.kind() { ty::Never => ("!", DW_ATE_unsigned), ty::Tuple(elements) if elements.is_empty() => { @@ -961,7 +946,7 @@ pub(crate) fn build_compile_unit_di_node<'ll, 'tcx>( let unit_metadata = llvm::LLVMRustDIBuilderCreateCompileUnit( debug_context.builder, - DW_LANG_RUST, + dwarf_const::DW_LANG_Rust, compile_unit_file, producer.as_c_char_ptr(), producer.len(), diff --git a/compiler/rustc_codegen_llvm/src/debuginfo/metadata/enums/cpp_like.rs b/compiler/rustc_codegen_llvm/src/debuginfo/metadata/enums/cpp_like.rs index dd83714614dab..a72e205c9b249 100644 --- a/compiler/rustc_codegen_llvm/src/debuginfo/metadata/enums/cpp_like.rs +++ b/compiler/rustc_codegen_llvm/src/debuginfo/metadata/enums/cpp_like.rs @@ -12,12 +12,13 @@ use rustc_middle::ty::{self, AdtDef, CoroutineArgs, CoroutineArgsExt, Ty}; use smallvec::smallvec; use crate::common::{AsCCharPtr, CodegenCx}; +use crate::debuginfo::dwarf_const::DW_TAG_const_type; use crate::debuginfo::metadata::enums::DiscrResult; use crate::debuginfo::metadata::type_map::{self, Stub, UniqueTypeId}; use crate::debuginfo::metadata::{ - DINodeCreationResult, DW_TAG_const_type, NO_GENERICS, NO_SCOPE_METADATA, SmallVec, - UNKNOWN_LINE_NUMBER, build_field_di_node, file_metadata, file_metadata_from_def_id, - size_and_align_of, type_di_node, unknown_file_metadata, visibility_di_flags, + DINodeCreationResult, NO_GENERICS, NO_SCOPE_METADATA, SmallVec, UNKNOWN_LINE_NUMBER, + build_field_di_node, file_metadata, file_metadata_from_def_id, size_and_align_of, type_di_node, + unknown_file_metadata, visibility_di_flags, }; use crate::debuginfo::utils::DIB; use crate::llvm::debuginfo::{DIFile, DIFlags, DIType}; diff --git a/compiler/rustc_codegen_llvm/src/debuginfo/mod.rs b/compiler/rustc_codegen_llvm/src/debuginfo/mod.rs index fae698bea2a6b..1c941cd92dcb5 100644 --- a/compiler/rustc_codegen_llvm/src/debuginfo/mod.rs +++ b/compiler/rustc_codegen_llvm/src/debuginfo/mod.rs @@ -39,6 +39,7 @@ use crate::llvm::debuginfo::{ use crate::value::Value; mod create_scope_map; +mod dwarf_const; mod gdb; pub(crate) mod metadata; mod namespace; From 1b62645418c40b663c1726131837d68fa575f15c Mon Sep 17 00:00:00 2001 From: Zalathar Date: Sun, 5 Jan 2025 12:29:37 +1100 Subject: [PATCH 2/3] Use constants for DWARF opcodes, instead of FFI calls --- .../src/debuginfo/dwarf_const.rs | 8 ++++++++ compiler/rustc_codegen_llvm/src/debuginfo/mod.rs | 15 ++++++--------- compiler/rustc_codegen_llvm/src/llvm/ffi.rs | 3 --- compiler/rustc_llvm/llvm-wrapper/RustWrapper.cpp | 16 ++++------------ 4 files changed, 18 insertions(+), 24 deletions(-) diff --git a/compiler/rustc_codegen_llvm/src/debuginfo/dwarf_const.rs b/compiler/rustc_codegen_llvm/src/debuginfo/dwarf_const.rs index f05422644041e..408429152223d 100644 --- a/compiler/rustc_codegen_llvm/src/debuginfo/dwarf_const.rs +++ b/compiler/rustc_codegen_llvm/src/debuginfo/dwarf_const.rs @@ -27,3 +27,11 @@ declare_constant!(DW_ATE_float: c_uint); declare_constant!(DW_ATE_signed: c_uint); declare_constant!(DW_ATE_unsigned: c_uint); declare_constant!(DW_ATE_UTF: c_uint); + +// DWARF expression operators. +declare_constant!(DW_OP_deref: u64); +declare_constant!(DW_OP_plus_uconst: u64); +/// Defined by LLVM in `llvm/include/llvm/BinaryFormat/Dwarf.h`. +/// Double-checked by a static assertion in `RustWrapper.cpp`. +#[allow(non_upper_case_globals)] +pub(crate) const DW_OP_LLVM_fragment: u64 = 0x1000; diff --git a/compiler/rustc_codegen_llvm/src/debuginfo/mod.rs b/compiler/rustc_codegen_llvm/src/debuginfo/mod.rs index 1c941cd92dcb5..cb18f3cd78bf8 100644 --- a/compiler/rustc_codegen_llvm/src/debuginfo/mod.rs +++ b/compiler/rustc_codegen_llvm/src/debuginfo/mod.rs @@ -153,29 +153,26 @@ impl<'ll> DebugInfoBuilderMethods for Builder<'_, 'll, '_> { indirect_offsets: &[Size], fragment: Option>, ) { + use dwarf_const::{DW_OP_LLVM_fragment, DW_OP_deref, DW_OP_plus_uconst}; + // Convert the direct and indirect offsets and fragment byte range to address ops. - // FIXME(eddyb) use `const`s instead of getting the values via FFI, - // the values should match the ones in the DWARF standard anyway. - let op_deref = || unsafe { llvm::LLVMRustDIBuilderCreateOpDeref() }; - let op_plus_uconst = || unsafe { llvm::LLVMRustDIBuilderCreateOpPlusUconst() }; - let op_llvm_fragment = || unsafe { llvm::LLVMRustDIBuilderCreateOpLLVMFragment() }; let mut addr_ops = SmallVec::<[u64; 8]>::new(); if direct_offset.bytes() > 0 { - addr_ops.push(op_plus_uconst()); + addr_ops.push(DW_OP_plus_uconst); addr_ops.push(direct_offset.bytes() as u64); } for &offset in indirect_offsets { - addr_ops.push(op_deref()); + addr_ops.push(DW_OP_deref); if offset.bytes() > 0 { - addr_ops.push(op_plus_uconst()); + addr_ops.push(DW_OP_plus_uconst); addr_ops.push(offset.bytes() as u64); } } if let Some(fragment) = fragment { // `DW_OP_LLVM_fragment` takes as arguments the fragment's // offset and size, both of them in bits. - addr_ops.push(op_llvm_fragment()); + addr_ops.push(DW_OP_LLVM_fragment); addr_ops.push(fragment.start.bits() as u64); addr_ops.push((fragment.end - fragment.start).bits() as u64); } diff --git a/compiler/rustc_codegen_llvm/src/llvm/ffi.rs b/compiler/rustc_codegen_llvm/src/llvm/ffi.rs index 21be66cb29b69..bb324ee682cdb 100644 --- a/compiler/rustc_codegen_llvm/src/llvm/ffi.rs +++ b/compiler/rustc_codegen_llvm/src/llvm/ffi.rs @@ -2177,9 +2177,6 @@ unsafe extern "C" { Location: &'a DILocation, BD: c_uint, ) -> Option<&'a DILocation>; - pub fn LLVMRustDIBuilderCreateOpDeref() -> u64; - pub fn LLVMRustDIBuilderCreateOpPlusUconst() -> u64; - pub fn LLVMRustDIBuilderCreateOpLLVMFragment() -> u64; pub fn LLVMRustWriteTypeToString(Type: &Type, s: &RustString); pub fn LLVMRustWriteValueToString(value_ref: &Value, s: &RustString); diff --git a/compiler/rustc_llvm/llvm-wrapper/RustWrapper.cpp b/compiler/rustc_llvm/llvm-wrapper/RustWrapper.cpp index ca62483b0aa2b..dd72ea2497f7c 100644 --- a/compiler/rustc_llvm/llvm-wrapper/RustWrapper.cpp +++ b/compiler/rustc_llvm/llvm-wrapper/RustWrapper.cpp @@ -54,6 +54,10 @@ using namespace llvm; using namespace llvm::sys; using namespace llvm::object; +// This opcode is an LLVM detail that could hypothetically change (?), so +// verify that the hard-coded value in `dwarf_const.rs` still agrees with LLVM. +static_assert(dwarf::DW_OP_LLVM_fragment == 0x1000); + // LLVMAtomicOrdering is already an enum - don't create another // one. static AtomicOrdering fromRust(LLVMAtomicOrdering Ordering) { @@ -1397,18 +1401,6 @@ LLVMRustDILocationCloneWithBaseDiscriminator(LLVMMetadataRef Location, return wrap(NewLoc.has_value() ? NewLoc.value() : nullptr); } -extern "C" uint64_t LLVMRustDIBuilderCreateOpDeref() { - return dwarf::DW_OP_deref; -} - -extern "C" uint64_t LLVMRustDIBuilderCreateOpPlusUconst() { - return dwarf::DW_OP_plus_uconst; -} - -extern "C" uint64_t LLVMRustDIBuilderCreateOpLLVMFragment() { - return dwarf::DW_OP_LLVM_fragment; -} - extern "C" void LLVMRustWriteTypeToString(LLVMTypeRef Ty, RustStringRef Str) { auto OS = RawRustStringOstream(Str); unwrap(Ty)->print(OS); From f50721ebadf8e2c2940bc9334926dc20f87ff22d Mon Sep 17 00:00:00 2001 From: Zalathar Date: Sun, 5 Jan 2025 12:31:41 +1100 Subject: [PATCH 3/3] Explain why the `DW_TAG_*` constants remain as-is for now --- compiler/rustc_codegen_llvm/src/debuginfo/mod.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/compiler/rustc_codegen_llvm/src/debuginfo/mod.rs b/compiler/rustc_codegen_llvm/src/debuginfo/mod.rs index cb18f3cd78bf8..755f4816acf98 100644 --- a/compiler/rustc_codegen_llvm/src/debuginfo/mod.rs +++ b/compiler/rustc_codegen_llvm/src/debuginfo/mod.rs @@ -48,6 +48,10 @@ mod utils; use self::create_scope_map::compute_mir_scopes; pub(crate) use self::metadata::build_global_var_di_node; +// FIXME(Zalathar): These `DW_TAG_*` constants are fake values that were +// removed from LLVM in 2015, and are only used by our own `RustWrapper.cpp` +// to decide which C++ API to call. Instead, we should just have two separate +// FFI functions and choose the correct one on the Rust side. #[allow(non_upper_case_globals)] const DW_TAG_auto_variable: c_uint = 0x100; #[allow(non_upper_case_globals)]