Skip to content

Commit

Permalink
Rollup merge of #111167 - cuviper:type-decl-disubprogram, r=michaelwo…
Browse files Browse the repository at this point in the history
…erister

debuginfo: split method declaration and definition

When we're adding a method to a type DIE, we only want a DW_AT_declaration
there, because LLVM LTO can't unify type definitions when a child DIE is a
full subprogram definition. Now the subprogram definition gets added at the
CU level with a specification link back to the abstract declaration.

Both GCC and Clang write debuginfo this way for C++ class methods.

Fixes #109730.
Fixes #109934.
  • Loading branch information
matthiaskrgr authored May 6, 2023
2 parents 8ec84dd + 10b69dd commit f440999
Show file tree
Hide file tree
Showing 5 changed files with 109 additions and 34 deletions.
85 changes: 51 additions & 34 deletions compiler/rustc_codegen_llvm/src/debuginfo/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -322,7 +322,7 @@ impl<'ll, 'tcx> DebugInfoMethods<'tcx> for CodegenCx<'ll, 'tcx> {
let tcx = self.tcx;

let def_id = instance.def_id();
let containing_scope = get_containing_scope(self, instance);
let (containing_scope, is_method) = get_containing_scope(self, instance);
let span = tcx.def_span(def_id);
let loc = self.lookup_debug_loc(span.lo());
let file_metadata = file_metadata(self, &loc.file);
Expand Down Expand Up @@ -378,8 +378,29 @@ impl<'ll, 'tcx> DebugInfoMethods<'tcx> for CodegenCx<'ll, 'tcx> {
}
}

unsafe {
return llvm::LLVMRustDIBuilderCreateFunction(
// When we're adding a method to a type DIE, we only want a DW_AT_declaration there, because
// LLVM LTO can't unify type definitions when a child DIE is a full subprogram definition.
// When we use this `decl` below, the subprogram definition gets created at the CU level
// with a DW_AT_specification pointing back to the type's declaration.
let decl = is_method.then(|| unsafe {
llvm::LLVMRustDIBuilderCreateMethod(
DIB(self),
containing_scope,
name.as_ptr().cast(),
name.len(),
linkage_name.as_ptr().cast(),
linkage_name.len(),
file_metadata,
loc.line,
function_type_metadata,
flags,
spflags & !DISPFlags::SPFlagDefinition,
template_parameters,
)
});

return unsafe {
llvm::LLVMRustDIBuilderCreateFunction(
DIB(self),
containing_scope,
name.as_ptr().cast(),
Expand All @@ -394,9 +415,9 @@ impl<'ll, 'tcx> DebugInfoMethods<'tcx> for CodegenCx<'ll, 'tcx> {
spflags,
maybe_definition_llfn,
template_parameters,
None,
);
}
decl,
)
};

fn get_function_signature<'ll, 'tcx>(
cx: &CodegenCx<'ll, 'tcx>,
Expand Down Expand Up @@ -493,14 +514,16 @@ impl<'ll, 'tcx> DebugInfoMethods<'tcx> for CodegenCx<'ll, 'tcx> {
names
}

/// Returns a scope, plus `true` if that's a type scope for "class" methods,
/// otherwise `false` for plain namespace scopes.
fn get_containing_scope<'ll, 'tcx>(
cx: &CodegenCx<'ll, 'tcx>,
instance: Instance<'tcx>,
) -> &'ll DIScope {
) -> (&'ll DIScope, bool) {
// First, let's see if this is a method within an inherent impl. Because
// if yes, we want to make the result subroutine DIE a child of the
// subroutine's self-type.
let self_type = cx.tcx.impl_of_method(instance.def_id()).and_then(|impl_def_id| {
if let Some(impl_def_id) = cx.tcx.impl_of_method(instance.def_id()) {
// If the method does *not* belong to a trait, proceed
if cx.tcx.trait_id_of_impl(impl_def_id).is_none() {
let impl_self_ty = cx.tcx.subst_and_normalize_erasing_regions(
Expand All @@ -511,39 +534,33 @@ impl<'ll, 'tcx> DebugInfoMethods<'tcx> for CodegenCx<'ll, 'tcx> {

// Only "class" methods are generally understood by LLVM,
// so avoid methods on other types (e.g., `<*mut T>::null`).
match impl_self_ty.kind() {
ty::Adt(def, ..) if !def.is_box() => {
// Again, only create type information if full debuginfo is enabled
if cx.sess().opts.debuginfo == DebugInfo::Full
&& !impl_self_ty.has_param()
{
Some(type_di_node(cx, impl_self_ty))
} else {
Some(namespace::item_namespace(cx, def.did()))
}
if let ty::Adt(def, ..) = impl_self_ty.kind() && !def.is_box() {
// Again, only create type information if full debuginfo is enabled
if cx.sess().opts.debuginfo == DebugInfo::Full && !impl_self_ty.has_param()
{
return (type_di_node(cx, impl_self_ty), true);
} else {
return (namespace::item_namespace(cx, def.did()), false);
}
_ => None,
}
} else {
// For trait method impls we still use the "parallel namespace"
// strategy
None
}
});
}

self_type.unwrap_or_else(|| {
namespace::item_namespace(
cx,
DefId {
krate: instance.def_id().krate,
index: cx
.tcx
.def_key(instance.def_id())
.parent
.expect("get_containing_scope: missing parent?"),
},
)
})
let scope = namespace::item_namespace(
cx,
DefId {
krate: instance.def_id().krate,
index: cx
.tcx
.def_key(instance.def_id())
.parent
.expect("get_containing_scope: missing parent?"),
},
);
(scope, false)
}
}

Expand Down
15 changes: 15 additions & 0 deletions compiler/rustc_codegen_llvm/src/llvm/ffi.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1987,6 +1987,21 @@ extern "C" {
Decl: Option<&'a DIDescriptor>,
) -> &'a DISubprogram;

pub fn LLVMRustDIBuilderCreateMethod<'a>(
Builder: &DIBuilder<'a>,
Scope: &'a DIDescriptor,
Name: *const c_char,
NameLen: size_t,
LinkageName: *const c_char,
LinkageNameLen: size_t,
File: &'a DIFile,
LineNo: c_uint,
Ty: &'a DIType,
Flags: DIFlags,
SPFlags: DISPFlags,
TParam: &'a DIArray,
) -> &'a DISubprogram;

pub fn LLVMRustDIBuilderCreateBasicType<'a>(
Builder: &DIBuilder<'a>,
Name: *const c_char,
Expand Down
22 changes: 22 additions & 0 deletions compiler/rustc_llvm/llvm-wrapper/RustWrapper.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -831,6 +831,28 @@ extern "C" LLVMMetadataRef LLVMRustDIBuilderCreateFunction(
return wrap(Sub);
}

extern "C" LLVMMetadataRef LLVMRustDIBuilderCreateMethod(
LLVMRustDIBuilderRef Builder, LLVMMetadataRef Scope,
const char *Name, size_t NameLen,
const char *LinkageName, size_t LinkageNameLen,
LLVMMetadataRef File, unsigned LineNo,
LLVMMetadataRef Ty, LLVMRustDIFlags Flags,
LLVMRustDISPFlags SPFlags, LLVMMetadataRef TParam) {
DITemplateParameterArray TParams =
DITemplateParameterArray(unwrap<MDTuple>(TParam));
DISubprogram::DISPFlags llvmSPFlags = fromRust(SPFlags);
DINode::DIFlags llvmFlags = fromRust(Flags);
DISubprogram *Sub = Builder->createMethod(
unwrapDI<DIScope>(Scope),
StringRef(Name, NameLen),
StringRef(LinkageName, LinkageNameLen),
unwrapDI<DIFile>(File), LineNo,
unwrapDI<DISubroutineType>(Ty),
0, 0, nullptr, // VTable params aren't used
llvmFlags, llvmSPFlags, TParams);
return wrap(Sub);
}

extern "C" LLVMMetadataRef LLVMRustDIBuilderCreateBasicType(
LLVMRustDIBuilderRef Builder, const char *Name, size_t NameLen,
uint64_t SizeInBits, unsigned Encoding) {
Expand Down
12 changes: 12 additions & 0 deletions tests/run-make/issue-109934-lto-debuginfo/Makefile
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
# ignore-cross-compile
include ../tools.mk

# With the upgrade to LLVM 16, this was getting:
#
# error: Cannot represent a difference across sections
#
# The error stemmed from DI function definitions under type scopes, fixed by
# only declaring in type scope and defining the subprogram elsewhere.

all:
$(RUSTC) lib.rs --test -C lto=fat -C debuginfo=2 -C incremental=$(TMPDIR)/inc-fat
9 changes: 9 additions & 0 deletions tests/run-make/issue-109934-lto-debuginfo/lib.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
extern crate alloc;

#[cfg(test)]
mod tests {
#[test]
fn something_alloc() {
assert_eq!(Vec::<u32>::new(), Vec::<u32>::new());
}
}

0 comments on commit f440999

Please sign in to comment.