Auto merge of #89597 - michaelwoerister:improve-vtable-debuginfo, r=wesleywiser

Create more accurate debuginfo for vtables.

Before this PR all vtables would have the same name (`"vtable"`) in debuginfo. Now they get an unambiguous name that identifies the implementing type and the trait that is being implemented.

This is only one of several possible improvements:
- This PR describes vtables as arrays of `*const u8` pointers. It would nice to describe them as structs where function pointer is represented by a field with a name indicative of the method it maps to. However, this requires coming up with a naming scheme that avoids clashes between methods with the same name (which is possible if the vtable contains multiple traits).
- The PR does not update the debuginfo we generate for the vtable-pointer field in a fat `dyn` pointer. Right now there does not seem to be an easy way of getting ahold of a vtable-layout without also knowing the concrete self-type of a trait object.

r? `@wesleywiser`

Author: bors

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<PolyExistentialTraitRef<'tcx>>, _vtable: Self::Value) {
         // TODO(antoyo)
     }
 

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,
 };
@@ -29,8 +29,9 @@ use rustc_index::vec::{Idx, IndexVec};
 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_query_system::ich::NodeIdHashingMode;
 use rustc_session::config::{self, DebugInfo};
@@ -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<ty::PolyExistentialTraitRef<'tcx>>,
+) -> &'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<ty::PolyExistentialTraitRef<'tcx>>,
+    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),

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<ty::PolyExistentialTraitRef<'tcx>>,
+        vtable: Self::Value,
+    ) {
+        metadata::create_vtable_metadata(self, ty, trait_ref, vtable)
     }
 
     fn extend_scope_to_file(

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:
+///
+/// `<path::to::SomeType as path::to::SomeTrait>::{vtable}`
+///
+/// or, when generating C++-like names:
+///
+/// `impl$<path::to::SomeType, path::to::SomeTrait>::vtable$`
+pub fn compute_debuginfo_vtable_name<'tcx>(
+    tcx: TyCtxt<'tcx>,
+    t: Ty<'tcx>,
+    trait_ref: Option<ty::PolyExistentialTraitRef<'tcx>>,
+) -> 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 {

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
 }

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<PolyExistentialTraitRef<'tcx>>,
+        vtable: Self::Value,
+    );
 
     /// Creates the function-specific debug context.
     ///

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: "<debug_vtable::Foo as debug_vtable::SomeTrait>::{vtable}"
+// MSVC-LABEL: !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$"
+// CHECK: !DISubrange(count: 4
+
+// NONMSVC-LABEL: !DIGlobalVariable(name: "<debug_vtable::Foo as _>::{vtable}"
+// MSVC-LABEL: !DIGlobalVariable(name: "impl$<debug_vtable::Foo, _>::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<T, U> {
+    fn method1(&self) -> (T, U);
+}
+
+impl SomeTraitWithGenerics<u64, i8> 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<u64, i8> = x;
+    (y.method1(), z.method1(), x as &dyn Send)
+}