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.

Sign up for GitHub

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

Jump to bottom

debuginfo: Fix DW_AT_containing_type vtable debuginfo regression #93503

Merged
merged 3 commits into from
Feb 10, 2022
Merged

debuginfo: Fix DW_AT_containing_type vtable debuginfo regression #93503

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
f4799b8
debuginfo: Make some helper functions in rustc_codegen_llvm::debuginf…
michaelwoerister Jan 25, 2022
fc7f419
debuginfo: Bring back DW_AT_containing_type for vtables after it has …
michaelwoerister Jan 25, 2022
ed21805
debuginfo: Bring back DW_AT_containing_type for vtables -- address re…
michaelwoerister Feb 8, 2022
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
179 changes: 144 additions & 35 deletions compiler/rustc_codegen_llvm/src/debuginfo/metadata.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ use crate::value::Value;

use cstr::cstr;
use rustc_codegen_ssa::debuginfo::type_names::cpp_like_debuginfo;
use rustc_codegen_ssa::debuginfo::type_names::VTableNameKind;
use rustc_codegen_ssa::traits::*;
use rustc_data_structures::fingerprint::Fingerprint;
use rustc_data_structures::fx::FxHashMap;
Expand Down Expand Up @@ -276,6 +277,12 @@ impl<'ll, 'tcx> TypeMap<'ll, 'tcx> {
) -> String {
format!("{}_variant_part", self.get_unique_type_id_as_string(enum_type_id))
}

/// Gets the `UniqueTypeId` for the type of a vtable.
fn get_unique_type_id_of_vtable_type(&mut self, vtable_type_name: &str) -> UniqueTypeId {
let interner_key = self.unique_id_interner.intern(vtable_type_name);
interner_key
}
}

/// A description of some recursive type. It can either be already finished (as
Expand Down Expand Up @@ -351,14 +358,15 @@ impl<'ll, 'tcx> RecursiveTypeDescription<'ll, 'tcx> {

// ... then create the member descriptions ...
let member_descriptions = member_description_factory.create_member_descriptions(cx);
let type_params = compute_type_parameters(cx, unfinished_type);

// ... and attach them to the stub to complete it.
set_members_of_composite_type(
cx,
unfinished_type,
member_holding_stub,
member_descriptions,
None,
type_params,
);
MetadataCreationResult::new(metadata_stub, true)
}
Expand Down Expand Up @@ -983,7 +991,17 @@ fn foreign_type_metadata<'ll, 'tcx>(
debug!("foreign_type_metadata: {:?}", t);

let name = compute_debuginfo_type_name(cx.tcx, t, false);
create_struct_stub(cx, t, &name, unique_type_id, NO_SCOPE_METADATA, DIFlags::FlagZero)
let (size, align) = cx.size_and_align_of(t);
create_struct_stub(
cx,
size,
align,
&name,
unique_type_id,
NO_SCOPE_METADATA,
DIFlags::FlagZero,
None,
)
}

fn param_type_metadata<'ll, 'tcx>(cx: &CodegenCx<'ll, 'tcx>, t: Ty<'tcx>) -> &'ll DIType {
Expand Down Expand Up @@ -1299,14 +1317,17 @@ fn prepare_struct_metadata<'ll, 'tcx>(
};

let containing_scope = get_namespace_for_item(cx, struct_def_id);
let (size, align) = cx.size_and_align_of(struct_type);

let struct_metadata_stub = create_struct_stub(
cx,
struct_type,
size,
align,
&struct_name,
unique_type_id,
Some(containing_scope),
DIFlags::FlagZero,
None,
);

create_and_register_recursive_type_forward_declaration(
Expand Down Expand Up @@ -1398,15 +1419,18 @@ fn prepare_tuple_metadata<'ll, 'tcx>(
unique_type_id: UniqueTypeId,
containing_scope: Option<&'ll DIScope>,
) -> RecursiveTypeDescription<'ll, 'tcx> {
let (size, align) = cx.size_and_align_of(tuple_type);
let tuple_name = compute_debuginfo_type_name(cx.tcx, tuple_type, false);

let struct_stub = create_struct_stub(
cx,
tuple_type,
size,
align,
&tuple_name[..],
unique_type_id,
containing_scope,
DIFlags::FlagZero,
None,
);

create_and_register_recursive_type_forward_declaration(
Expand Down Expand Up @@ -1581,13 +1605,14 @@ impl<'ll, 'tcx> EnumMemberDescriptionFactory<'ll, 'tcx> {
describe_enum_variant(cx, self.layout, variant_info, self_metadata);

let member_descriptions = member_description_factory.create_member_descriptions(cx);
let type_params = compute_type_parameters(cx, self.enum_type);

set_members_of_composite_type(
cx,
self.enum_type,
variant_type_metadata,
member_descriptions,
Some(&self.common_members),
type_params,
);
vec![MemberDescription {
name: variant_info.variant_name(),
Expand Down Expand Up @@ -1648,13 +1673,14 @@ impl<'ll, 'tcx> EnumMemberDescriptionFactory<'ll, 'tcx> {

let member_descriptions =
member_desc_factory.create_member_descriptions(cx);
let type_params = compute_type_parameters(cx, self.enum_type);

set_members_of_composite_type(
cx,
self.enum_type,
variant_type_metadata,
member_descriptions,
Some(&self.common_members),
type_params,
);

MemberDescription {
Expand Down Expand Up @@ -1777,13 +1803,14 @@ impl<'ll, 'tcx> EnumMemberDescriptionFactory<'ll, 'tcx> {
);

let member_descriptions = member_desc_factory.create_member_descriptions(cx);
let type_params = compute_type_parameters(cx, self.enum_type);

set_members_of_composite_type(
cx,
self.enum_type,
variant_type_metadata,
member_descriptions,
Some(&self.common_members),
type_params,
);

let (size, align) =
Expand Down Expand Up @@ -1823,13 +1850,14 @@ impl<'ll, 'tcx> EnumMemberDescriptionFactory<'ll, 'tcx> {

let member_descriptions =
member_desc_factory.create_member_descriptions(cx);
let type_params = compute_type_parameters(cx, self.enum_type);

set_members_of_composite_type(
cx,
self.enum_type,
variant_type_metadata,
member_descriptions,
Some(&self.common_members),
type_params,
);

let niche_value = calculate_niche_value(i);
Expand Down Expand Up @@ -1965,13 +1993,18 @@ fn describe_enum_variant<'ll, 'tcx>(
.type_map
.borrow_mut()
.get_unique_type_id_of_enum_variant(cx, layout.ty, variant_name);

let (size, align) = cx.size_and_align_of(layout.ty);

create_struct_stub(
cx,
layout.ty,
size,
align,
variant_name,
unique_type_id,
Some(containing_scope),
DIFlags::FlagZero,
None,
)
});

Expand Down Expand Up @@ -2308,33 +2341,38 @@ fn composite_type_metadata<'ll, 'tcx>(
member_descriptions: Vec<MemberDescription<'ll>>,
containing_scope: Option<&'ll DIScope>,
) -> &'ll DICompositeType {
let (size, align) = cx.size_and_align_of(composite_type);

// Create the (empty) struct metadata node ...
let composite_type_metadata = create_struct_stub(
cx,
composite_type,
size,
align,
composite_type_name,
composite_type_unique_id,
containing_scope,
DIFlags::FlagZero,
None,
);

// ... and immediately create and add the member descriptions.
set_members_of_composite_type(
cx,
composite_type,
composite_type_metadata,
member_descriptions,
None,
compute_type_parameters(cx, composite_type),
);

composite_type_metadata
}

fn set_members_of_composite_type<'ll, 'tcx>(
cx: &CodegenCx<'ll, 'tcx>,
composite_type: Ty<'tcx>,
composite_type_metadata: &'ll DICompositeType,
member_descriptions: Vec<MemberDescription<'ll>>,
common_members: Option<&Vec<Option<&'ll DIType>>>,
type_params: &'ll DIArray,
) {
// In some rare cases LLVM metadata uniquing would lead to an existing type
// description being used instead of a new one created in
Expand All @@ -2361,13 +2399,12 @@ fn set_members_of_composite_type<'ll, 'tcx>(
member_metadata.extend(other_members.iter());
}

let type_params = compute_type_parameters(cx, composite_type);
unsafe {
let type_array = create_DIArray(DIB(cx), &member_metadata);
let field_array = create_DIArray(DIB(cx), &member_metadata);
llvm::LLVMRustDICompositeTypeReplaceArrays(
DIB(cx),
composite_type_metadata,
Some(type_array),
Some(field_array),
Some(type_params),
);
}
Expand Down Expand Up @@ -2420,14 +2457,14 @@ fn compute_type_parameters<'ll, 'tcx>(cx: &CodegenCx<'ll, 'tcx>, ty: Ty<'tcx>) -
/// with `set_members_of_composite_type()`.
fn create_struct_stub<'ll, 'tcx>(
cx: &CodegenCx<'ll, 'tcx>,
struct_type: Ty<'tcx>,
struct_type_name: &str,
size: Size,
align: Align,
type_name: &str,
unique_type_id: UniqueTypeId,
containing_scope: Option<&'ll DIScope>,
flags: DIFlags,
vtable_holder: Option<&'ll DIType>,
) -> &'ll DICompositeType {
let (struct_size, struct_align) = cx.size_and_align_of(struct_type);

let type_map = debug_context(cx).type_map.borrow();
let unique_type_id = type_map.get_unique_type_id_as_string(unique_type_id);

Expand All @@ -2440,17 +2477,17 @@ fn create_struct_stub<'ll, 'tcx>(
llvm::LLVMRustDIBuilderCreateStructType(
DIB(cx),
containing_scope,
struct_type_name.as_ptr().cast(),
struct_type_name.len(),
type_name.as_ptr().cast(),
type_name.len(),
unknown_file_metadata(cx),
UNKNOWN_LINE_NUMBER,
struct_size.bits(),
struct_align.bits() as u32,
size.bits(),
align.bits() as u32,
flags,
None,
empty_array,
0,
None,
vtable_holder,
unique_type_id.as_ptr().cast(),
unique_type_id.len(),
)
Expand Down Expand Up @@ -2556,6 +2593,14 @@ pub fn create_global_var_metadata<'ll>(cx: &CodegenCx<'ll, '_>, def_id: DefId, g
}

/// Generates LLVM debuginfo for a vtable.
///
/// The vtable type looks like a struct with a field for each function pointer and super-trait
/// pointer it contains (plus the `size` and `align` fields).
///
/// Except for `size`, `align`, and `drop_in_place`, the field names don't try to mirror
/// the name of the method they implement. This can be implemented in the future once there
/// is a proper disambiguation scheme for dealing with methods from different traits that have
/// the same name.
fn vtable_type_metadata<'ll, 'tcx>(
cx: &CodegenCx<'ll, 'tcx>,
ty: Ty<'tcx>,
Expand All @@ -2572,16 +2617,79 @@ fn vtable_type_metadata<'ll, 'tcx>(
COMMON_VTABLE_ENTRIES
};

// FIXME: We describe the vtable as an array of *const () pointers. The length of the array is
// correct - but we could create a more accurate description, e.g. by describing it
// as a struct where each field has a name that corresponds to the name of the method
// it points to.
// However, this is not entirely straightforward because there might be multiple
// methods with the same name if the vtable is for multiple traits. So for now we keep
// things simple instead of adding some ad-hoc disambiguation scheme.
let vtable_type = tcx.mk_array(tcx.mk_imm_ptr(tcx.types.unit), vtable_entries.len() as u64);
// All function pointers are described as opaque pointers. This could be improved in the future
// by describing them as actual function pointers.
let void_pointer_ty = tcx.mk_imm_ptr(tcx.types.unit);
let void_pointer_type_debuginfo = type_metadata(cx, void_pointer_ty);
let usize_debuginfo = type_metadata(cx, tcx.types.usize);
let (pointer_size, pointer_align) = cx.size_and_align_of(void_pointer_ty);
// If `usize` is not pointer-sized and -aligned then the size and alignment computations
// for the vtable as a whole would be wrong. Let's make sure this holds even on weird
// platforms.
assert_eq!(cx.size_and_align_of(tcx.types.usize), (pointer_size, pointer_align));

let vtable_type_name =
compute_debuginfo_vtable_name(cx.tcx, ty, poly_trait_ref, VTableNameKind::Type);
let unique_type_id = debug_context(cx)
.type_map
.borrow_mut()
.get_unique_type_id_of_vtable_type(&vtable_type_name);
let size = pointer_size * vtable_entries.len() as u64;

// This gets mapped to a DW_AT_containing_type attribute which allows GDB to correlate
// the vtable to the type it is for.
let vtable_holder = type_metadata(cx, ty);

let vtable_type_metadata = create_struct_stub(
cx,
size,
pointer_align,
&vtable_type_name,
unique_type_id,
NO_SCOPE_METADATA,
DIFlags::FlagArtificial,
Some(vtable_holder),
);

// Create a field for each entry in the vtable.
let fields: Vec<_> = vtable_entries
.iter()
.enumerate()
.filter_map(|(index, vtable_entry)| {
let (field_name, field_type) = match vtable_entry {
ty::VtblEntry::MetadataDropInPlace => {
("drop_in_place".to_string(), void_pointer_type_debuginfo)
}
ty::VtblEntry::Method(_) => {
// Note: This code does not try to give a proper name to each method
// because their might be multiple methods with the same name
wesleywiser marked this conversation as resolved.
Show resolved Hide resolved
// (coming from different traits).
(format!("__method{}", index), void_pointer_type_debuginfo)
}
ty::VtblEntry::TraitVPtr(_) => {
(format!("__super_trait_ptr{}", index), void_pointer_type_debuginfo)
Copy link
Member

Choose a reason for hiding this comment

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

This points to the super trait's vtable? It would be good (eventually) if we could set the type correctly here. I'm not sure if we might need to walk the tree of vtables for some reason ...

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, there is definitely room for improvement here. Types of the fn pointers as well. I'll add a comment about the super trait pointer.

}
ty::VtblEntry::MetadataAlign => ("align".to_string(), usize_debuginfo),
ty::VtblEntry::MetadataSize => ("size".to_string(), usize_debuginfo),
ty::VtblEntry::Vacant => return None,
};

Some(MemberDescription {
name: field_name,
type_metadata: field_type,
offset: pointer_size * index as u64,
size: pointer_size,
align: pointer_align,
flags: DIFlags::FlagZero,
discriminant: None,
source_info: None,
})
})
.collect();

type_metadata(cx, vtable_type)
let type_params = create_DIArray(DIB(cx), &[]);
set_members_of_composite_type(cx, vtable_type_metadata, fields, None, type_params);
vtable_type_metadata
}

/// Creates debug information for the given vtable, which is for the
Expand All @@ -2603,11 +2711,12 @@ pub fn create_vtable_metadata<'ll, 'tcx>(
return;
}

let vtable_name = compute_debuginfo_vtable_name(cx.tcx, ty, poly_trait_ref);
let vtable_name =
compute_debuginfo_vtable_name(cx.tcx, ty, poly_trait_ref, VTableNameKind::GlobalVariable);
let vtable_type = vtable_type_metadata(cx, ty, poly_trait_ref);
let linkage_name = "";

unsafe {
let linkage_name = "";
llvm::LLVMRustDIBuilderCreateStaticVariable(
DIB(cx),
NO_SCOPE_METADATA,
Expand Down
Loading