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: Make sure that type names for closure and generator environments are unique in debuginfo. #93154

Merged
merged 1 commit into from
Feb 2, 2022
Merged

debuginfo: Make sure that type names for closure and generator environments are unique in debuginfo. #93154

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
37 changes: 20 additions & 17 deletions compiler/rustc_codegen_llvm/src/debuginfo/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ use rustc_data_structures::sync::Lrc;
use rustc_hir::def_id::{DefId, DefIdMap};
use rustc_index::vec::IndexVec;
use rustc_middle::mir;
use rustc_middle::ty::layout::{HasTyCtxt, LayoutOf};
use rustc_middle::ty::layout::LayoutOf;
use rustc_middle::ty::subst::{GenericArgKind, SubstsRef};
use rustc_middle::ty::{self, Instance, ParamEnv, Ty, TypeFoldable};
use rustc_session::config::{self, DebugInfo};
Expand Down Expand Up @@ -307,9 +307,11 @@ impl<'ll, 'tcx> DebugInfoMethods<'tcx> for CodegenCx<'ll, 'tcx> {
fn_abi: &FnAbi<'tcx, Ty<'tcx>>,
maybe_definition_llfn: Option<&'ll Value>,
) -> &'ll DIScope {
let tcx = self.tcx;

let def_id = instance.def_id();
let containing_scope = get_containing_scope(self, instance);
let span = self.tcx.def_span(def_id);
let span = tcx.def_span(def_id);
let loc = self.lookup_debug_loc(span.lo());
let file_metadata = file_metadata(self, &loc.file);

Expand All @@ -319,16 +321,24 @@ impl<'ll, 'tcx> DebugInfoMethods<'tcx> for CodegenCx<'ll, 'tcx> {
};

let mut name = String::new();
type_names::push_item_name(self.tcx(), def_id, false, &mut name);
type_names::push_item_name(tcx, def_id, false, &mut name);

// Find the enclosing function, in case this is a closure.
let enclosing_fn_def_id = self.tcx().typeck_root_def_id(def_id);
let enclosing_fn_def_id = tcx.typeck_root_def_id(def_id);

// We look up the generics of the enclosing function and truncate the substs
// to their length in order to cut off extra stuff that might be in there for
// closures or generators.
let generics = tcx.generics_of(enclosing_fn_def_id);
let substs = instance.substs.truncate_to(tcx, generics);

type_names::push_generic_params(
tcx,
tcx.normalize_erasing_regions(ty::ParamEnv::reveal_all(), substs),
&mut name,
);

// Get_template_parameters() will append a `<...>` clause to the function
// name if necessary.
let generics = self.tcx().generics_of(enclosing_fn_def_id);
let substs = instance.substs.truncate_to(self.tcx(), generics);
let template_parameters = get_template_parameters(self, generics, substs, &mut name);
let template_parameters = get_template_parameters(self, generics, substs);

let linkage_name = &mangled_name_of_instance(self, instance).name;
// Omit the linkage_name if it is the same as subprogram name.
Expand All @@ -350,7 +360,7 @@ impl<'ll, 'tcx> DebugInfoMethods<'tcx> for CodegenCx<'ll, 'tcx> {
if self.sess().opts.optimize != config::OptLevel::No {
spflags |= DISPFlags::SPFlagOptimized;
}
if let Some((id, _)) = self.tcx.entry_fn(()) {
if let Some((id, _)) = tcx.entry_fn(()) {
if id == def_id {
spflags |= DISPFlags::SPFlagMainSubprogram;
}
Expand Down Expand Up @@ -429,14 +439,7 @@ impl<'ll, 'tcx> DebugInfoMethods<'tcx> for CodegenCx<'ll, 'tcx> {
cx: &CodegenCx<'ll, 'tcx>,
generics: &ty::Generics,
substs: SubstsRef<'tcx>,
name_to_append_suffix_to: &mut String,
) -> &'ll DIArray {
type_names::push_generic_params(
cx.tcx,
cx.tcx.normalize_erasing_regions(ty::ParamEnv::reveal_all(), substs),
name_to_append_suffix_to,
);

if substs.types().next().is_none() {
return create_DIArray(DIB(cx), &[]);
}
Expand Down
110 changes: 74 additions & 36 deletions compiler/rustc_codegen_ssa/src/debuginfo/type_names.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,9 @@

use rustc_data_structures::fx::FxHashSet;
use rustc_data_structures::stable_hasher::{HashStable, StableHasher};
use rustc_hir as hir;
use rustc_hir::def_id::DefId;
use rustc_hir::definitions::{DefPathData, DefPathDataName, DisambiguatedDefPathData};
use rustc_hir::{AsyncGeneratorKind, GeneratorKind, Mutability};
use rustc_middle::ty::layout::IntegerExt;
use rustc_middle::ty::subst::{GenericArgKind, SubstsRef};
use rustc_middle::ty::{self, AdtDef, ExistentialProjection, Ty, TyCtxt};
Expand Down Expand Up @@ -102,14 +102,14 @@ fn push_debuginfo_type_name<'tcx>(
ty::RawPtr(ty::TypeAndMut { ty: inner_type, mutbl }) => {
if cpp_like_debuginfo {
match mutbl {
hir::Mutability::Not => output.push_str("ptr_const$<"),
hir::Mutability::Mut => output.push_str("ptr_mut$<"),
Mutability::Not => output.push_str("ptr_const$<"),
Mutability::Mut => output.push_str("ptr_mut$<"),
}
} else {
output.push('*');
match mutbl {
hir::Mutability::Not => output.push_str("const "),
hir::Mutability::Mut => output.push_str("mut "),
Mutability::Not => output.push_str("const "),
Mutability::Mut => output.push_str("mut "),
}
}

Expand All @@ -131,8 +131,8 @@ fn push_debuginfo_type_name<'tcx>(
output.push_str(mutbl.prefix_str());
} else if !is_slice_or_str {
match mutbl {
hir::Mutability::Not => output.push_str("ref$<"),
hir::Mutability::Mut => output.push_str("ref_mut$<"),
Mutability::Not => output.push_str("ref$<"),
Mutability::Mut => output.push_str("ref_mut$<"),
}
}

Expand Down Expand Up @@ -345,14 +345,39 @@ fn push_debuginfo_type_name<'tcx>(
// processing
visited.remove(t);
}
ty::Closure(def_id, ..) | ty::Generator(def_id, ..) => {
let key = tcx.def_key(def_id);
ty::Closure(def_id, substs) | ty::Generator(def_id, substs, ..) => {
// Name will be "{closure_env#0}<T1, T2, ...>", "{generator_env#0}<T1, T2, ...>", or
// "{async_fn_env#0}<T1, T2, ...>", etc.
let def_key = tcx.def_key(def_id);

if qualified {
let parent_def_id = DefId { index: key.parent.unwrap(), ..def_id };
let parent_def_id = DefId { index: def_key.parent.unwrap(), ..def_id };
push_item_name(tcx, parent_def_id, true, output);
output.push_str("::");
}
push_unqualified_item_name(tcx, def_id, key.disambiguated_data, output);

let mut label = String::with_capacity(20);
write!(&mut label, "{}_env", generator_kind_label(tcx.generator_kind(def_id))).unwrap();

push_disambiguated_special_name(
&label,
def_key.disambiguated_data.disambiguator,
cpp_like_debuginfo,
output,
);

// We also need to add the generic arguments of the async fn/generator or
// the enclosing function (for closures or async blocks), so that we end
// up with a unique name for every instantiation.

// Find the generics of the enclosing function, as defined in the source code.
let enclosing_fn_def_id = tcx.typeck_root_def_id(def_id);
let generics = tcx.generics_of(enclosing_fn_def_id);

// Truncate the substs to the length of the above generics. This will cut off
// anything closure- or generator-specific.
let substs = substs.truncate_to(tcx, generics);
push_generic_params_internal(tcx, substs, output, visited);
}
// Type parameters from polymorphized functions.
ty::Param(_) => {
Expand Down Expand Up @@ -509,6 +534,29 @@ pub fn push_item_name(tcx: TyCtxt<'_>, def_id: DefId, qualified: bool, output: &
push_unqualified_item_name(tcx, def_id, def_key.disambiguated_data, output);
}

fn generator_kind_label(generator_kind: Option<GeneratorKind>) -> &'static str {
match generator_kind {
Some(GeneratorKind::Async(AsyncGeneratorKind::Block)) => "async_block",
Some(GeneratorKind::Async(AsyncGeneratorKind::Closure)) => "async_closure",
Some(GeneratorKind::Async(AsyncGeneratorKind::Fn)) => "async_fn",
Some(GeneratorKind::Gen) => "generator",
None => "closure",
}
}

fn push_disambiguated_special_name(
label: &str,
disambiguator: u32,
cpp_like_debuginfo: bool,
output: &mut String,
) {
if cpp_like_debuginfo {
write!(output, "{}${}", label, disambiguator).unwrap();
} else {
write!(output, "{{{}#{}}}", label, disambiguator).unwrap();
}
}

fn push_unqualified_item_name(
tcx: TyCtxt<'_>,
def_id: DefId,
Expand All @@ -519,42 +567,32 @@ fn push_unqualified_item_name(
DefPathData::CrateRoot => {
output.push_str(tcx.crate_name(def_id.krate).as_str());
}
DefPathData::ClosureExpr if tcx.generator_kind(def_id).is_some() => {
let key = match tcx.generator_kind(def_id).unwrap() {
hir::GeneratorKind::Async(hir::AsyncGeneratorKind::Block) => "async_block",
hir::GeneratorKind::Async(hir::AsyncGeneratorKind::Closure) => "async_closure",
hir::GeneratorKind::Async(hir::AsyncGeneratorKind::Fn) => "async_fn",
hir::GeneratorKind::Gen => "generator",
};
// Generators look like closures, but we want to treat them differently
// in the debug info.
if cpp_like_debuginfo(tcx) {
write!(output, "{}${}", key, disambiguated_data.disambiguator).unwrap();
} else {
write!(output, "{{{}#{}}}", key, disambiguated_data.disambiguator).unwrap();
}
DefPathData::ClosureExpr => {
let label = generator_kind_label(tcx.generator_kind(def_id));

push_disambiguated_special_name(
label,
disambiguated_data.disambiguator,
cpp_like_debuginfo(tcx),
output,
);
}
_ => match disambiguated_data.data.name() {
DefPathDataName::Named(name) => {
output.push_str(name.as_str());
}
DefPathDataName::Anon { namespace } => {
if cpp_like_debuginfo(tcx) {
write!(output, "{}${}", namespace, disambiguated_data.disambiguator).unwrap();
} else {
write!(output, "{{{}#{}}}", namespace, disambiguated_data.disambiguator)
.unwrap();
}
push_disambiguated_special_name(
namespace.as_str(),
disambiguated_data.disambiguator,
cpp_like_debuginfo(tcx),
output,
);
}
},
};
}

// Pushes the generic parameters in the given `InternalSubsts` to the output string.
// This ignores region parameters, since they can't reliably be
// reconstructed for items from non-local crates. For local crates, this
// would be possible but with inlining and LTO we have to use the least
// common denominator - otherwise we would run into conflicts.
michaelwoerister marked this conversation as resolved.
Show resolved Hide resolved
fn push_generic_params_internal<'tcx>(
tcx: TyCtxt<'tcx>,
substs: SubstsRef<'tcx>,
Expand Down
2 changes: 1 addition & 1 deletion src/test/codegen/async-fn-debug-msvc.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ async fn async_fn_test() {
// FIXME: No way to reliably check the filename.

// CHECK-DAG: [[ASYNC_FN:!.*]] = !DINamespace(name: "async_fn_test"
// CHECK-DAG: [[GEN:!.*]] = !DICompositeType(tag: DW_TAG_union_type, name: "async_fn$0"
// CHECK-DAG: [[GEN:!.*]] = !DICompositeType(tag: DW_TAG_union_type, name: "async_fn_env$0"
// CHECK: {{!.*}} = !DIDerivedType(tag: DW_TAG_member, name: "variant0", scope: [[GEN]],
// For brevity, we only check the struct name and members of the last variant.
// CHECK-SAME: file: [[FILE:![0-9]*]], line: 11,
Expand Down
2 changes: 1 addition & 1 deletion src/test/codegen/async-fn-debug.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ async fn async_fn_test() {
// FIXME: No way to reliably check the filename.

// CHECK-DAG: [[ASYNC_FN:!.*]] = !DINamespace(name: "async_fn_test"
// CHECK-DAG: [[GEN:!.*]] = !DICompositeType(tag: DW_TAG_structure_type, name: "{async_fn#0}", scope: [[ASYNC_FN]]
// CHECK-DAG: [[GEN:!.*]] = !DICompositeType(tag: DW_TAG_structure_type, name: "{async_fn_env#0}", scope: [[ASYNC_FN]]
// CHECK: [[VARIANT:!.*]] = !DICompositeType(tag: DW_TAG_variant_part, scope: [[ASYNC_FN]],
// CHECK-NOT: flags: DIFlagArtificial
// CHECK-SAME: discriminator: [[DISC:![0-9]*]]
Expand Down
53 changes: 39 additions & 14 deletions src/test/codegen/debug-vtable.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,25 +1,36 @@
// compile-flags: -Cdebuginfo=2 -Copt-level=0 -Ccodegen-units=1
// ignore-tidy-linelength

// This test checks the debuginfo for the expected 3 vtables is generated for correct names and number
// of entries.

// NONMSVC-LABEL: !DIGlobalVariable(name: "<debug_vtable::Foo as debug_vtable::SomeTrait>::{vtable}"
// MSVC-LABEL: !DIGlobalVariable(name: "impl$<debug_vtable::Foo, debug_vtable::SomeTrait>::vtable$"
// Use the v0 symbol mangling scheme to codegen order independent of rustc version.
// Unnamed items like shims are generated in lexicographical order of their symbol name and in the
// legacy mangling scheme rustc version and generic parameters are both hashed into a single part
// of the name, thus randomizing item order with respect to rustc version.

// compile-flags: -Cdebuginfo=2 -Copt-level=0 -Csymbol-mangling-version=v0
// ignore-tidy-linelength

// NONMSVC: !DIGlobalVariable(name: "<debug_vtable::Foo as debug_vtable::SomeTrait>::{vtable}"
// MSVC: !DIGlobalVariable(name: "impl$<debug_vtable::Foo, debug_vtable::SomeTrait>::vtable$"
// NONMSVC: !DIDerivedType(tag: DW_TAG_pointer_type, name: "*const ()",
// MSVC: !DIDerivedType(tag: DW_TAG_pointer_type, name: "ptr_const$<tuple$<> >",
// CHECK: !DISubrange(count: 5

// NONMSVC-LABEL: !DIGlobalVariable(name: "<debug_vtable::Foo as debug_vtable::SomeTraitWithGenerics<u64, i8>>::{vtable}"
// MSVC-LABEL: !DIGlobalVariable(name: "impl$<debug_vtable::Foo, debug_vtable::SomeTraitWithGenerics<u64,i8> >::vtable$"
// NONMSVC: !DIGlobalVariable(name: "<debug_vtable::Foo as debug_vtable::SomeTraitWithGenerics<u64, i8>>::{vtable}"
// MSVC: !DIGlobalVariable(name: "impl$<debug_vtable::Foo, debug_vtable::SomeTraitWithGenerics<u64,i8> >::vtable$"
// CHECK: !DISubrange(count: 4

// NONMSVC-LABEL: !DIGlobalVariable(name: "<debug_vtable::Foo as _>::{vtable}"
// MSVC-LABEL: !DIGlobalVariable(name: "impl$<debug_vtable::Foo, _>::vtable$"
// NONMSVC: !DIGlobalVariable(name: "<debug_vtable::Foo as _>::{vtable}"
// MSVC: !DIGlobalVariable(name: "impl$<debug_vtable::Foo, _>::vtable$"
// CHECK: !DISubrange(count: 3

// NONMSVC-LABEL: !DIGlobalVariable(name: "<debug_vtable::bar::{closure#0} as core::ops::function::FnOnce<(core::option::Option<&dyn core::ops::function::Fn<(), Output=()>>)>>::{vtable}"
// MSVC-LABEL: !DIGlobalVariable(name: "impl$<debug_vtable::bar::closure$0, core::ops::function::FnOnce<tuple$<enum$<core::option::Option<ref$<dyn$<core::ops::function::Fn<tuple$<>,assoc$<Output,tuple$<> > > > > >, {{.*}}, {{.*}}, Some> > > >::vtable$"
// NONMSVC: !DIGlobalVariable(name: "<debug_vtable::bar::{closure_env#0} as core::ops::function::FnOnce<(core::option::Option<&dyn core::ops::function::Fn<(), Output=()>>)>>::{vtable}"
// MSVC: !DIGlobalVariable(name: "impl$<debug_vtable::bar::closure_env$0, core::ops::function::FnOnce<tuple$<enum$<core::option::Option<ref$<dyn$<core::ops::function::Fn<tuple$<>,assoc$<Output,tuple$<> > > > > >, {{.*}}, {{.*}}, Some> > > >::vtable$"

// NONMSVC: !DIGlobalVariable(name: "<debug_vtable::generic_closure::{closure_env#0}<bool> as core::ops::function::FnOnce<()>>::{vtable}"
// MSVC: !DIGlobalVariable(name: "impl$<debug_vtable::generic_closure::closure_env$0<bool>, core::ops::function::FnOnce<tuple$<> > >::vtable$
Copy link
Member

Choose a reason for hiding this comment

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

I was going to comment on the tuple$<> thing which seems like it could break natvis but I see we already filed #89704 for that.


// NONMSVC: !DIGlobalVariable(name: "<debug_vtable::generic_closure::{closure_env#0}<u32> as core::ops::function::FnOnce<()>>::{vtable}"
// MSVC: !DIGlobalVariable(name: "impl$<debug_vtable::generic_closure::closure_env$0<u32>, core::ops::function::FnOnce<tuple$<> > >::vtable$

#![crate_type = "lib"]

Expand All @@ -31,16 +42,22 @@ pub trait SomeTrait {
}

impl SomeTrait for Foo {
fn method1(&self) -> u32 { 1 }
fn method2(&self) -> u32 { 2 }
fn method1(&self) -> u32 {
1
}
fn method2(&self) -> u32 {
2
}
}

pub trait SomeTraitWithGenerics<T, U> {
fn method1(&self) -> (T, U);
}

impl SomeTraitWithGenerics<u64, i8> for Foo {
fn method1(&self) -> (u64, i8) { (1, 2) }
fn method1(&self) -> (u64, i8) {
(1, 2)
}
}

pub fn foo(x: &Foo) -> (u32, (u64, i8), &dyn Send) {
Expand All @@ -55,3 +72,11 @@ pub fn foo(x: &Foo) -> (u32, (u64, i8), &dyn Send) {
pub fn bar() -> Box<dyn FnOnce(Option<&dyn Fn()>)> {
Box::new(|_x: Option<&dyn Fn()>| {})
}

fn generic_closure<T: 'static>(x: T) -> Box<dyn FnOnce() -> T> {
Box::new(move || x)
}

pub fn instantiate_generic_closures() -> (Box<dyn FnOnce() -> u32>, Box<dyn FnOnce() -> bool>) {
(generic_closure(1u32), generic_closure(false))
}
Loading