Skip to content
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

cg_llvm: Replace some DIBuilder wrappers with LLVM-C API bindings (part 2) #136632

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open
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
115 changes: 115 additions & 0 deletions compiler/rustc_codegen_llvm/src/debuginfo/di_builder.rs
Copy link
Member

Choose a reason for hiding this comment

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

this is where all the action is, then, since this makes-safe a few things...

Original file line number Diff line number Diff line change
@@ -0,0 +1,115 @@
//! Safe wrappers for [`DIBuilder`] FFI functions.

use libc::c_uint;
use rustc_abi::{Align, Size};

use crate::llvm::debuginfo::{DIBuilder, DIFlags};
use crate::llvm::{self, Metadata};

impl<'ll> DIBuilder<'ll> {
pub(crate) fn create_subroutine_type(
&self,
parameter_types: &[Option<&'ll Metadata>],
flags: DIFlags,
) -> &'ll Metadata {
unsafe {
llvm::LLVMDIBuilderCreateSubroutineType(
self,
Comment on lines +15 to +17
Copy link
Member

Choose a reason for hiding this comment

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

Okay, so, it's somewhat hard to audit the safe wrappers if I don't know what safety constraints they are filling. I can guess, but I shouldn't have to. Either there should be safety comments on the wrapper decls or they should be on this block, so that future additions to these functions don't fuck up and change the parts of the code that have to be a certain way.

Copy link
Member

Choose a reason for hiding this comment

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

like it could be, idk, instead of exhaustively listing param relationships

// SAFETY: this call is safe if we know the DIBuilder is live, which we do because
// we have it by-ref, and the rest is just translating slices to two params

None, // ("File"; unused)
parameter_types.as_ptr(),
parameter_types.len() as c_uint,
flags,
)
}
}

pub(crate) fn create_union_type(
&self,
scope: Option<&'ll Metadata>,
name: &str,
file_metadata: &'ll Metadata,
line_number: c_uint,
size: Size,
align: Align,
flags: DIFlags,
elements: &[&'ll Metadata],
unique_id: &str,
) -> &'ll Metadata {
unsafe {
llvm::LLVMDIBuilderCreateUnionType(
self,
scope,
name.as_ptr(),
name.len(),
file_metadata,
line_number,
size.bits(),
align.bits() as u32,
flags,
elements.as_ptr(),
elements.len() as c_uint,
0, // ("Objective-C runtime version"; default is 0)
unique_id.as_ptr(),
unique_id.len(),
)
}
}

pub(crate) fn create_array_type(
&self,
size: Size,
align: Align,
element_type: &'ll Metadata,
subscripts: &[&'ll Metadata],
) -> &'ll Metadata {
unsafe {
llvm::LLVMDIBuilderCreateArrayType(
self,
size.bits(),
align.bits() as u32,
element_type,
subscripts.as_ptr(),
subscripts.len() as c_uint,
)
}
}

pub(crate) fn create_basic_type(
&self,
name: &str,
size: Size,
dwarf_type_encoding: u32,
flags: DIFlags,
) -> &'ll Metadata {
unsafe {
llvm::LLVMDIBuilderCreateBasicType(
self,
name.as_ptr(),
name.len(),
size.bits(),
dwarf_type_encoding,
flags,
)
}
}

pub(crate) fn create_pointer_type(
&self,
pointee_type: &'ll Metadata,
size: Size,
align: Align,
name: &str,
) -> &'ll Metadata {
unsafe {
llvm::LLVMDIBuilderCreatePointerType(
self,
pointee_type,
size.bits(),
align.bits() as u32,
0, // ("DWARF address space"; default is 0)
name.as_ptr(),
name.len(),
)
}
}
}
117 changes: 34 additions & 83 deletions compiler/rustc_codegen_llvm/src/debuginfo/metadata.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,7 @@ use self::type_map::{DINodeCreationResult, Stub, UniqueTypeId};
use super::CodegenUnitDebugContext;
use super::namespace::mangled_name_of_instance;
use super::type_names::{compute_debuginfo_type_name, compute_debuginfo_vtable_name};
use super::utils::{
DIB, create_DIArray, debug_context, get_namespace_for_item, is_node_local_to_unit,
};
use super::utils::{DIB, 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;
Expand Down Expand Up @@ -114,19 +112,9 @@ fn build_fixed_size_array_di_node<'ll, 'tcx>(
.try_to_target_usize(cx.tcx)
.expect("expected monomorphic const in codegen") as c_longlong;

let subrange =
unsafe { Some(llvm::LLVMRustDIBuilderGetOrCreateSubrange(DIB(cx), 0, upper_bound)) };
let subrange = unsafe { llvm::LLVMRustDIBuilderGetOrCreateSubrange(DIB(cx), 0, upper_bound) };

let subscripts = create_DIArray(DIB(cx), &[subrange]);
let di_node = unsafe {
llvm::LLVMRustDIBuilderCreateArrayType(
DIB(cx),
size.bits(),
align.bits() as u32,
element_type_di_node,
subscripts,
)
};
let di_node = DIB(cx).create_array_type(size, align, element_type_di_node, &[subrange]);

DINodeCreationResult::new(di_node, false)
}
Expand Down Expand Up @@ -168,17 +156,12 @@ fn build_pointer_or_reference_di_node<'ll, 'tcx>(
"ptr_type={ptr_type}, pointee_type={pointee_type}",
);

let di_node = unsafe {
llvm::LLVMRustDIBuilderCreatePointerType(
DIB(cx),
pointee_type_di_node,
data_layout.pointer_size.bits(),
data_layout.pointer_align.abi.bits() as u32,
0, // Ignore DWARF address space.
ptr_type_debuginfo_name.as_c_char_ptr(),
ptr_type_debuginfo_name.len(),
)
};
let di_node = DIB(cx).create_pointer_type(
pointee_type_di_node,
data_layout.pointer_size,
data_layout.pointer_align.abi,
&ptr_type_debuginfo_name,
);

DINodeCreationResult { di_node, already_stored_in_typemap: false }
}
Expand Down Expand Up @@ -226,17 +209,12 @@ fn build_pointer_or_reference_di_node<'ll, 'tcx>(

// The data pointer type is a regular, thin pointer, regardless of whether this
// is a slice or a trait object.
let data_ptr_type_di_node = unsafe {
llvm::LLVMRustDIBuilderCreatePointerType(
DIB(cx),
pointee_type_di_node,
addr_field.size.bits(),
addr_field.align.abi.bits() as u32,
0, // Ignore DWARF address space.
std::ptr::null(),
0,
)
};
let data_ptr_type_di_node = DIB(cx).create_pointer_type(
pointee_type_di_node,
addr_field.size,
addr_field.align.abi,
"",
);

smallvec![
build_field_di_node(
Expand Down Expand Up @@ -311,12 +289,7 @@ fn build_subroutine_type_di_node<'ll, 'tcx>(

debug_context(cx).type_map.unique_id_to_di_node.borrow_mut().remove(&unique_type_id);

let fn_di_node = unsafe {
llvm::LLVMRustDIBuilderCreateSubroutineType(
DIB(cx),
create_DIArray(DIB(cx), &signature_di_nodes[..]),
)
};
let fn_di_node = DIB(cx).create_subroutine_type(&signature_di_nodes, DIFlags::FlagZero);

// This is actually a function pointer, so wrap it in pointer DI.
let name = compute_debuginfo_type_name(cx.tcx, fn_ty, false);
Expand All @@ -325,17 +298,7 @@ fn build_subroutine_type_di_node<'ll, 'tcx>(
ty::FnPtr(..) => (cx.tcx.data_layout.pointer_size, cx.tcx.data_layout.pointer_align.abi),
_ => unreachable!(),
};
let di_node = unsafe {
llvm::LLVMRustDIBuilderCreatePointerType(
DIB(cx),
fn_di_node,
size.bits(),
align.bits() as u32,
0, // Ignore DWARF address space.
name.as_c_char_ptr(),
name.len(),
)
};
let di_node = DIB(cx).create_pointer_type(fn_di_node, size, align, &name);

DINodeCreationResult::new(di_node, false)
}
Expand Down Expand Up @@ -487,26 +450,22 @@ pub(crate) fn type_di_node<'ll, 'tcx>(cx: &CodegenCx<'ll, 'tcx>, t: Ty<'tcx>) ->
// FIXME(mw): Cache this via a regular UniqueTypeId instead of an extra field in the debug context.
fn recursion_marker_type_di_node<'ll, 'tcx>(cx: &CodegenCx<'ll, 'tcx>) -> &'ll DIType {
*debug_context(cx).recursion_marker_type.get_or_init(move || {
unsafe {
// The choice of type here is pretty arbitrary -
// anything reading the debuginfo for a recursive
// type is going to see *something* weird - the only
// question is what exactly it will see.
//
// FIXME: the name `<recur_type>` does not fit the naming scheme
// of other types.
//
// FIXME: it might make sense to use an actual pointer type here
// so that debuggers can show the address.
let name = "<recur_type>";
llvm::LLVMRustDIBuilderCreateBasicType(
DIB(cx),
name.as_c_char_ptr(),
name.len(),
cx.tcx.data_layout.pointer_size.bits(),
dwarf_const::DW_ATE_unsigned,
)
}
// The choice of type here is pretty arbitrary -
// anything reading the debuginfo for a recursive
// type is going to see *something* weird - the only
// question is what exactly it will see.
//
// FIXME: the name `<recur_type>` does not fit the naming scheme
// of other types.
//
// FIXME: it might make sense to use an actual pointer type here
// so that debuggers can show the address.
DIB(cx).create_basic_type(
"<recur_type>",
cx.tcx.data_layout.pointer_size,
dwarf_const::DW_ATE_unsigned,
DIFlags::FlagZero,
)
})
}

Expand Down Expand Up @@ -788,15 +747,7 @@ fn build_basic_type_di_node<'ll, 'tcx>(
_ => bug!("debuginfo::build_basic_type_di_node - `t` is invalid type"),
};

let ty_di_node = unsafe {
llvm::LLVMRustDIBuilderCreateBasicType(
DIB(cx),
name.as_c_char_ptr(),
name.len(),
cx.size_of(t).bits(),
encoding,
)
};
let ty_di_node = DIB(cx).create_basic_type(name, cx.size_of(t), encoding, DIFlags::FlagZero);

if !cpp_like_debuginfo {
return DINodeCreationResult::new(ty_di_node, false);
Expand Down
28 changes: 11 additions & 17 deletions compiler/rustc_codegen_llvm/src/debuginfo/metadata/type_map.rs
Original file line number Diff line number Diff line change
Expand Up @@ -226,23 +226,17 @@ pub(super) fn stub<'ll, 'tcx>(
)
}
}
Stub::Union => unsafe {
llvm::LLVMRustDIBuilderCreateUnionType(
DIB(cx),
containing_scope,
name.as_c_char_ptr(),
name.len(),
file_metadata,
line_number,
size.bits(),
align.bits() as u32,
flags,
Some(empty_array),
0,
unique_type_id_str.as_c_char_ptr(),
unique_type_id_str.len(),
)
},
Stub::Union => DIB(cx).create_union_type(
containing_scope,
name,
file_metadata,
line_number,
size,
align,
flags,
&[],
&unique_type_id_str,
),
};
StubInfo { metadata, unique_type_id }
}
Expand Down
11 changes: 6 additions & 5 deletions compiler/rustc_codegen_llvm/src/debuginfo/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ use crate::llvm::debuginfo::{
use crate::value::Value;

mod create_scope_map;
mod di_builder;
mod dwarf_const;
mod gdb;
pub(crate) mod metadata;
Expand Down Expand Up @@ -325,9 +326,9 @@ impl<'ll, 'tcx> DebugInfoCodegenMethods<'tcx> for CodegenCx<'ll, 'tcx> {
let loc = self.lookup_debug_loc(span.lo());
let file_metadata = file_metadata(self, &loc.file);

let function_type_metadata = unsafe {
let function_type_metadata = {
let fn_signature = get_function_signature(self, fn_abi);
llvm::LLVMRustDIBuilderCreateSubroutineType(DIB(self), fn_signature)
DIB(self).create_subroutine_type(&fn_signature, DIFlags::FlagZero)
};

let mut name = String::with_capacity(64);
Expand Down Expand Up @@ -420,9 +421,9 @@ impl<'ll, 'tcx> DebugInfoCodegenMethods<'tcx> for CodegenCx<'ll, 'tcx> {
fn get_function_signature<'ll, 'tcx>(
cx: &CodegenCx<'ll, 'tcx>,
fn_abi: &FnAbi<'tcx, Ty<'tcx>>,
) -> &'ll DIArray {
) -> Vec<Option<&'ll llvm::Metadata>> {
if cx.sess().opts.debuginfo != DebugInfo::Full {
return create_DIArray(DIB(cx), &[]);
return vec![];
}

let mut signature = Vec::with_capacity(fn_abi.args.len() + 1);
Expand Down Expand Up @@ -463,7 +464,7 @@ impl<'ll, 'tcx> DebugInfoCodegenMethods<'tcx> for CodegenCx<'ll, 'tcx> {
.extend(fn_abi.args.iter().map(|arg| Some(type_di_node(cx, arg.layout.ty))));
}

create_DIArray(DIB(cx), &signature[..])
signature
}

fn get_template_parameters<'ll, 'tcx>(
Expand Down
Loading
Loading