Skip to content

cg_llvm: Use constants for DWARF opcodes, instead of FFI calls #135115

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Jan 6, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Cargo.lock
Original file line number Diff line number Diff line change
Expand Up @@ -3499,6 +3499,7 @@ name = "rustc_codegen_llvm"
version = "0.0.0"
dependencies = [
"bitflags",
"gimli 0.30.0",
"itertools",
"libc",
"measureme",
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_codegen_llvm/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ test = false
[dependencies]
# tidy-alphabetical-start
bitflags = "2.4.1"
gimli = "0.30"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

mmm, three versions of gimli in the repo... should fix that...

Copy link
Contributor Author

@Zalathar Zalathar Jan 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, it looks like we have:

  • 0.31 (latest) used by run-make-support, which actually needs it
  • 0.30 used by most stuff, which can probably be bumped to 0.31
  • 0.28 used by an old addr2line, which is pulled in by anyhow's backtrace feature, which is perhaps not even needed nowadays?
    • also pulled in by color-eyre

itertools = "0.12"
libc = "0.2"
measureme = "11"
Expand Down
37 changes: 37 additions & 0 deletions compiler/rustc_codegen_llvm/src/debuginfo/dwarf_const.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
//! 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);

// 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;
27 changes: 6 additions & 21 deletions compiler/rustc_codegen_llvm/src/debuginfo/metadata.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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::{
Expand Down Expand Up @@ -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;

Expand All @@ -90,8 +75,6 @@ type SmallVec<T> = smallvec::SmallVec<[T; 16]>;
mod enums;
mod type_map;

pub(crate) use type_map::TypeMap;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I moved this to where the other imports are, so that it doesn't trick rust-analyzer into adding new imports in the wrong place.


/// 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 {
Expand Down Expand Up @@ -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,
)
}
})
Expand Down Expand Up @@ -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() => {
Expand Down Expand Up @@ -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(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down
20 changes: 11 additions & 9 deletions compiler/rustc_codegen_llvm/src/debuginfo/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -47,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)]
Expand Down Expand Up @@ -152,29 +157,26 @@ impl<'ll> DebugInfoBuilderMethods for Builder<'_, 'll, '_> {
indirect_offsets: &[Size],
fragment: Option<Range<Size>>,
) {
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);
}
Expand Down
3 changes: 0 additions & 3 deletions compiler/rustc_codegen_llvm/src/llvm/ffi.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
16 changes: 4 additions & 12 deletions compiler/rustc_llvm/llvm-wrapper/RustWrapper.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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<llvm::Type>(Ty)->print(OS);
Expand Down
Loading