From 61c5a6d644e9e5c1995c33a2b07ab251702848ac Mon Sep 17 00:00:00 2001 From: Michael Woerister Date: Wed, 6 Oct 2021 14:26:50 +0200 Subject: [PATCH] Create more accurate debuginfo for vtables. Before this commit all vtables would have the same name "vtable" in debuginfo. Now they get a name that identifies the implementing type and the trait that is being implemented. --- compiler/rustc_codegen_gcc/src/debuginfo.rs | 4 +- .../src/debuginfo/metadata.rs | 77 +++++++++++-------- .../rustc_codegen_llvm/src/debuginfo/mod.rs | 9 ++- .../src/debuginfo/type_names.rs | 56 ++++++++++++++ compiler/rustc_codegen_ssa/src/meth.rs | 2 +- .../rustc_codegen_ssa/src/traits/debuginfo.rs | 9 ++- src/test/codegen/debug-vtable.rs | 47 +++++++++++ src/test/codegen/vtabletype.rs | 21 ----- 8 files changed, 163 insertions(+), 62 deletions(-) create mode 100644 src/test/codegen/debug-vtable.rs delete mode 100644 src/test/codegen/vtabletype.rs diff --git a/compiler/rustc_codegen_gcc/src/debuginfo.rs b/compiler/rustc_codegen_gcc/src/debuginfo.rs index 4d3b4f04badec..31959fa19c588 100644 --- a/compiler/rustc_codegen_gcc/src/debuginfo.rs +++ b/compiler/rustc_codegen_gcc/src/debuginfo.rs @@ -2,7 +2,7 @@ use gccjit::RValue; use rustc_codegen_ssa::mir::debuginfo::{FunctionDebugContext, VariableKind}; use rustc_codegen_ssa::traits::{DebugInfoBuilderMethods, DebugInfoMethods}; use rustc_middle::mir; -use rustc_middle::ty::{Instance, Ty}; +use rustc_middle::ty::{Instance, PolyExistentialTraitRef, Ty}; use rustc_span::{SourceFile, Span, Symbol}; use rustc_target::abi::Size; use rustc_target::abi::call::FnAbi; @@ -31,7 +31,7 @@ impl<'a, 'gcc, 'tcx> DebugInfoBuilderMethods for Builder<'a, 'gcc, 'tcx> { } impl<'gcc, 'tcx> DebugInfoMethods<'tcx> for CodegenCx<'gcc, 'tcx> { - fn create_vtable_metadata(&self, _ty: Ty<'tcx>, _vtable: Self::Value) { + fn create_vtable_metadata(&self, _ty: Ty<'tcx>, _trait_ref: Option>, _vtable: Self::Value) { // TODO(antoyo) } diff --git a/compiler/rustc_codegen_llvm/src/debuginfo/metadata.rs b/compiler/rustc_codegen_llvm/src/debuginfo/metadata.rs index f6ec5e6395f84..51c2a926a9cf0 100644 --- a/compiler/rustc_codegen_llvm/src/debuginfo/metadata.rs +++ b/compiler/rustc_codegen_llvm/src/debuginfo/metadata.rs @@ -2,7 +2,7 @@ use self::MemberDescriptionFactory::*; use self::RecursiveTypeDescription::*; use super::namespace::mangled_name_of_instance; -use super::type_names::compute_debuginfo_type_name; +use super::type_names::{compute_debuginfo_type_name, compute_debuginfo_vtable_name}; use super::utils::{ create_DIArray, debug_context, get_namespace_for_item, is_node_local_to_unit, DIB, }; @@ -30,8 +30,9 @@ use rustc_middle::ich::NodeIdHashingMode; use rustc_middle::mir::{self, GeneratorLayout}; use rustc_middle::ty::layout::{self, IntegerExt, LayoutOf, PrimitiveExt, TyAndLayout}; use rustc_middle::ty::subst::GenericArgKind; -use rustc_middle::ty::Instance; -use rustc_middle::ty::{self, AdtKind, GeneratorSubsts, ParamEnv, Ty, TyCtxt}; +use rustc_middle::ty::{ + self, AdtKind, GeneratorSubsts, Instance, ParamEnv, Ty, TyCtxt, COMMON_VTABLE_ENTRIES, +}; use rustc_middle::{bug, span_bug}; use rustc_session::config::{self, DebugInfo}; use rustc_span::symbol::Symbol; @@ -2591,11 +2592,45 @@ pub fn create_global_var_metadata(cx: &CodegenCx<'ll, '_>, def_id: DefId, global } } +/// Generates LLVM debuginfo for a vtable. +fn vtable_type_metadata( + cx: &CodegenCx<'ll, 'tcx>, + ty: Ty<'tcx>, + poly_trait_ref: Option>, +) -> &'ll DIType { + let tcx = cx.tcx; + + let vtable_entries = if let Some(poly_trait_ref) = poly_trait_ref { + let trait_ref = poly_trait_ref.with_self_ty(tcx, ty); + let trait_ref = tcx.erase_regions(trait_ref); + + tcx.vtable_entries(trait_ref) + } else { + 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); + + type_metadata(cx, vtable_type, rustc_span::DUMMY_SP) +} + /// Creates debug information for the given vtable, which is for the /// given type. /// /// Adds the created metadata nodes directly to the crate's IR. -pub fn create_vtable_metadata(cx: &CodegenCx<'ll, 'tcx>, ty: Ty<'tcx>, vtable: &'ll Value) { +pub fn create_vtable_metadata( + cx: &CodegenCx<'ll, 'tcx>, + ty: Ty<'tcx>, + poly_trait_ref: Option>, + vtable: &'ll Value, +) { if cx.dbg_cx.is_none() { return; } @@ -2605,42 +2640,16 @@ pub fn create_vtable_metadata(cx: &CodegenCx<'ll, 'tcx>, ty: Ty<'tcx>, vtable: & return; } - let type_metadata = type_metadata(cx, ty, rustc_span::DUMMY_SP); + let vtable_name = compute_debuginfo_vtable_name(cx.tcx, ty, poly_trait_ref); + let vtable_type = vtable_type_metadata(cx, ty, poly_trait_ref); unsafe { - // `LLVMRustDIBuilderCreateStructType()` wants an empty array. A null - // pointer will lead to hard to trace and debug LLVM assertions - // later on in `llvm/lib/IR/Value.cpp`. - let empty_array = create_DIArray(DIB(cx), &[]); - let name = "vtable"; - - // Create a new one each time. We don't want metadata caching - // here, because each vtable will refer to a unique containing - // type. - let vtable_type = llvm::LLVMRustDIBuilderCreateStructType( - DIB(cx), - NO_SCOPE_METADATA, - name.as_ptr().cast(), - name.len(), - unknown_file_metadata(cx), - UNKNOWN_LINE_NUMBER, - Size::ZERO.bits(), - cx.tcx.data_layout.pointer_align.abi.bits() as u32, - DIFlags::FlagArtificial, - None, - empty_array, - 0, - Some(type_metadata), - name.as_ptr().cast(), - name.len(), - ); - let linkage_name = ""; llvm::LLVMRustDIBuilderCreateStaticVariable( DIB(cx), NO_SCOPE_METADATA, - name.as_ptr().cast(), - name.len(), + vtable_name.as_ptr().cast(), + vtable_name.len(), linkage_name.as_ptr().cast(), linkage_name.len(), unknown_file_metadata(cx), diff --git a/compiler/rustc_codegen_llvm/src/debuginfo/mod.rs b/compiler/rustc_codegen_llvm/src/debuginfo/mod.rs index 894320a798285..1f1bd73c7d035 100644 --- a/compiler/rustc_codegen_llvm/src/debuginfo/mod.rs +++ b/compiler/rustc_codegen_llvm/src/debuginfo/mod.rs @@ -550,8 +550,13 @@ impl DebugInfoMethods<'tcx> for CodegenCx<'ll, 'tcx> { unsafe { llvm::LLVMRustDIBuilderCreateDebugLocation(line, col, scope, inlined_at) } } - fn create_vtable_metadata(&self, ty: Ty<'tcx>, vtable: Self::Value) { - metadata::create_vtable_metadata(self, ty, vtable) + fn create_vtable_metadata( + &self, + ty: Ty<'tcx>, + trait_ref: Option>, + vtable: Self::Value, + ) { + metadata::create_vtable_metadata(self, ty, trait_ref, vtable) } fn extend_scope_to_file( diff --git a/compiler/rustc_codegen_ssa/src/debuginfo/type_names.rs b/compiler/rustc_codegen_ssa/src/debuginfo/type_names.rs index e842f5e9391c8..6fe346064fc4e 100644 --- a/compiler/rustc_codegen_ssa/src/debuginfo/type_names.rs +++ b/compiler/rustc_codegen_ssa/src/debuginfo/type_names.rs @@ -446,6 +446,62 @@ fn push_debuginfo_type_name<'tcx>( } } +/// Computes a name for the global variable storing a vtable. +/// +/// The name is of the form: +/// +/// `::{vtable}` +/// +/// or, when generating C++-like names: +/// +/// `impl$::vtable$` +pub fn compute_debuginfo_vtable_name<'tcx>( + tcx: TyCtxt<'tcx>, + t: Ty<'tcx>, + trait_ref: Option>, +) -> String { + let cpp_like_names = cpp_like_names(tcx); + + let mut vtable_name = String::with_capacity(64); + + if cpp_like_names { + vtable_name.push_str("impl$<"); + } else { + vtable_name.push('<'); + } + + let mut visited = FxHashSet::default(); + push_debuginfo_type_name(tcx, t, true, &mut vtable_name, &mut visited); + + if cpp_like_names { + vtable_name.push_str(", "); + } else { + vtable_name.push_str(" as "); + } + + if let Some(trait_ref) = trait_ref { + push_item_name(tcx, trait_ref.skip_binder().def_id, true, &mut vtable_name); + visited.clear(); + push_generic_params_internal( + tcx, + trait_ref.skip_binder().substs, + &mut vtable_name, + &mut visited, + ); + } else { + vtable_name.push_str("_"); + } + + push_close_angle_bracket(cpp_like_names, &mut vtable_name); + + let suffix = if cpp_like_names { "::vtable$" } else { "::{vtable}" }; + + vtable_name.reserve_exact(suffix.len()); + vtable_name.push_str(suffix); + + vtable_name +} + pub fn push_item_name(tcx: TyCtxt<'tcx>, def_id: DefId, qualified: bool, output: &mut String) { let def_key = tcx.def_key(def_id); if qualified { diff --git a/compiler/rustc_codegen_ssa/src/meth.rs b/compiler/rustc_codegen_ssa/src/meth.rs index efeec5b728413..2b3c1af445ace 100644 --- a/compiler/rustc_codegen_ssa/src/meth.rs +++ b/compiler/rustc_codegen_ssa/src/meth.rs @@ -78,7 +78,7 @@ pub fn get_vtable<'tcx, Cx: CodegenMethods<'tcx>>( let align = cx.data_layout().pointer_align.abi; let vtable = cx.static_addr_of(vtable_const, align, Some("vtable")); - cx.create_vtable_metadata(ty, vtable); + cx.create_vtable_metadata(ty, trait_ref, vtable); cx.vtables().borrow_mut().insert((ty, trait_ref), vtable); vtable } diff --git a/compiler/rustc_codegen_ssa/src/traits/debuginfo.rs b/compiler/rustc_codegen_ssa/src/traits/debuginfo.rs index 3e66d711d2ef5..e700afc448f28 100644 --- a/compiler/rustc_codegen_ssa/src/traits/debuginfo.rs +++ b/compiler/rustc_codegen_ssa/src/traits/debuginfo.rs @@ -1,13 +1,18 @@ use super::BackendTypes; use crate::mir::debuginfo::{FunctionDebugContext, VariableKind}; use rustc_middle::mir; -use rustc_middle::ty::{Instance, Ty}; +use rustc_middle::ty::{Instance, PolyExistentialTraitRef, Ty}; use rustc_span::{SourceFile, Span, Symbol}; use rustc_target::abi::call::FnAbi; use rustc_target::abi::Size; pub trait DebugInfoMethods<'tcx>: BackendTypes { - fn create_vtable_metadata(&self, ty: Ty<'tcx>, vtable: Self::Value); + fn create_vtable_metadata( + &self, + ty: Ty<'tcx>, + trait_ref: Option>, + vtable: Self::Value, + ); /// Creates the function-specific debug context. /// diff --git a/src/test/codegen/debug-vtable.rs b/src/test/codegen/debug-vtable.rs new file mode 100644 index 0000000000000..851d68da5ee62 --- /dev/null +++ b/src/test/codegen/debug-vtable.rs @@ -0,0 +1,47 @@ +// 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: "::{vtable}" +// MSVC-LABEL: !DIGlobalVariable(name: "impl$::vtable$" +// NONMSVC: !DIDerivedType(tag: DW_TAG_pointer_type, name: "*const ()", +// MSVC: !DIDerivedType(tag: DW_TAG_pointer_type, name: "ptr_const$ >", +// CHECK: !DISubrange(count: 5 + +// NONMSVC-LABEL: !DIGlobalVariable(name: ">::{vtable}" +// MSVC-LABEL: !DIGlobalVariable(name: "impl$ >::vtable$" +// CHECK: !DISubrange(count: 4 + +// NONMSVC-LABEL: !DIGlobalVariable(name: "::{vtable}" +// MSVC-LABEL: !DIGlobalVariable(name: "impl$::vtable$" +// CHECK: !DISubrange(count: 3 + +#![crate_type = "lib"] + +pub struct Foo; + +pub trait SomeTrait { + fn method1(&self) -> u32; + fn method2(&self) -> u32; +} + +impl SomeTrait for Foo { + fn method1(&self) -> u32 { 1 } + fn method2(&self) -> u32 { 2 } +} + +pub trait SomeTraitWithGenerics { + fn method1(&self) -> (T, U); +} + +impl SomeTraitWithGenerics for Foo { + fn method1(&self) -> (u64, i8) { (1, 2) } +} + +pub fn foo(x: &Foo) -> (u32, (u64, i8), &dyn Send) { + let y: &dyn SomeTrait = x; + let z: &dyn SomeTraitWithGenerics = x; + (y.method1(), z.method1(), x as &dyn Send) +} diff --git a/src/test/codegen/vtabletype.rs b/src/test/codegen/vtabletype.rs deleted file mode 100644 index 82d65b101b06d..0000000000000 --- a/src/test/codegen/vtabletype.rs +++ /dev/null @@ -1,21 +0,0 @@ -// This test depends on a patch that was committed to upstream LLVM -// after 5.0, then backported to the Rust LLVM fork. - -// ignore-windows -// ignore-macos - -// compile-flags: -g -C no-prepopulate-passes - -// CHECK-LABEL: @main -// CHECK: {{.*}}DICompositeType{{.*}}name: "vtable",{{.*}}vtableHolder:{{.*}} - -pub trait T { -} - -impl T for f64 { -} - -pub fn main() { - let d = 23.0f64; - let td = &d as &T; -}