Let replaceVTableHolder accept any type

[tromey]

Summary

In Rust, a trait can be implemented for any type, and if a trait
object pointer is used for the type, then a virtual table will be
emitted for that trait/type combination.

We would like debuggers to be able to inspect trait objects, which
requires finding the concrete type associated with a given vtable.

This patch changes LLVM so that any type can be passed to
replaceVTableHolder. This allows the Rust compiler to emit the needed
debug info -- associating a vtable with the concrete type for which it
was emitted.

This is a DWARF extension: DWARF only specifies the meaning of
DW_AT_containing_type in one specific situation. This style of DWARF
extension is routine, though, and LLVM already has one such case for
DW_AT_containing_type.

[JDevlieghere]

test/DebugInfo/Generic/containing-type-extension.ll

9

You can omit the regex at the end of the line, FileCheck will look for the substring, similar to grep.

10

You should use CHECK-NEXT: or add something like CHECK-NOT: DW_TAG between your CHECK lines to ensure that they actually belong to this DW_TAG_structure_type.

[aprantl]

test/DebugInfo/Generic/containing-type-extension.ll

8

you probably want a CHECK-NOT: TAG here to avoid matching beyond the current tag
alternatively you can use llvm-dwarfdump --name vtable

20

we usually strip out all non-essential attributes (everything in quotes) from testcases.

25

Why not use DW_LANG_Rust and an actual example?

unittests/IR/MetadataTest.cpp

1317

Maybe you can specifically call out Rust as a user here.

[tromey]

test/DebugInfo/Generic/containing-type-extension.ll

25

It seemed better to me to have a minimal example; but I can make the DW_LANG_Rust change pretty easily.

[tromey]

Address review comments

[aprantl]

test/DebugInfo/Generic/containing-type-extension.ll

25

My point was: it would be helpful to have actual Rust debug info code in the testcase to serve as a reminder hat kind of use-cases we need to support to guide future development/refactoring of LLVM. Having a minimal testcase is of course good, too, but I think in this case it would be nice to have an example of actual well-formed Rust debug info highlighting what Rust needs (and how it is different from clang). Can you generate a similarly small testcase from Rust?

[tromey]

test/DebugInfo/Generic/containing-type-extension.ll

25

Sure, I can. The one in the patch was made by minimizing a rustc-generated file. In one real .ll I have:

!3 = !DICompositeType(tag: DW_TAG_structure_type, name: "vtable", file: !2, size: 64, align: 64, flags: DIFlagArtificial, elements: !4, vtableHolder: !5, identifier: "vtable")
...
!5 = !DIBasicType(name: "f64", size: 64, encoding: DW_ATE_float)

... which I think is essentially the same as what's in the current test:

!8 = !DIBasicType(tag: DW_TAG_base_type, name: "int", size: 32, align: 32, encoding: DW_ATE_signed)
...
!12 = !DICompositeType(tag: DW_TAG_structure_type, name: "vtable", size: 8, align: 8, elements: !2, identifier: "vtable", vtableHolder: !8)

[tromey]

test/DebugInfo/Generic/containing-type-extension.ll

6

This comment is wrong - copied from a different test. I'll fix it.

[tromey]

Add a test based on rustc output

[JDevlieghere]

Looks good to me with these few nits, but let's wait for Adrian's approval before landing.

test/DebugInfo/Generic/containing-type-extension-rust.ll

9 ↗

(On Diff #121340)

You should also add CHECK-NOT: TAG below this line if the DW_AT_name should belong to the same DW_TAG_structure_type

test/DebugInfo/Generic/containing-type-extension.ll

10

Same here.

[tromey]

Thanks for the reviews. I don't have commit access, so could someone please check this in for me?

include/llvm/IR/DIBuilder.h

public:
     /// \param DL           Debug info location.
     /// \param InsertBefore Location for the new intrinsic.
     Instruction *insertDbgValueIntrinsic(llvm::Value *Val,
                                          DILocalVariable *VarInfo,
                                          DIExpression *Expr,
                                          const DILocation *DL,
                                          Instruction *InsertBefore);
 
-    /// Replace the vtable holder in the given composite type.
+    /// Replace the vtable holder in the given type.
     ///
     /// If this creates a self reference, it may orphan some unresolved cycles
     /// in the operands of \c T, so \a DIBuilder needs to track that.
     void replaceVTableHolder(DICompositeType *&T,
-                             DICompositeType *VTableHolder);
+                             DIType *VTableHolder);
 
     /// Replace arrays on a composite type.
     ///
     /// If \c T is resolved, but the arrays aren't -- which can happen if \c T
     /// has a self-reference -- \a DIBuilder needs to track the array to
     /// resolve cycles.
     void replaceArrays(DICompositeType *&T, DINodeArray Elements,
                        DINodeArray TParams = DINodeArray());

lib/CodeGen/AsmPrinter/DwarfUnit.cpp

for (const auto *Element : Elements) {
       }
     }
 
     if (CTy->isAppleBlockExtension())
       addFlag(Buffer, dwarf::DW_AT_APPLE_block);
 
     // This is outside the DWARF spec, but GDB expects a DW_AT_containing_type
     // inside C++ composite types to point to the base class with the vtable.
-    if (auto *ContainingType =
-            dyn_cast_or_null<DICompositeType>(resolve(CTy->getVTableHolder())))
+    // Rust uses DW_AT_containing_type to link a vtable to the type
+    // for which it was created.
+    if (auto *ContainingType = resolve(CTy->getVTableHolder()))
       addDIEEntry(Buffer, dwarf::DW_AT_containing_type,
                   *getOrCreateTypeDIE(ContainingType));
 
     if (CTy->isObjcClassComplete())
       addFlag(Buffer, dwarf::DW_AT_APPLE_objc_complete_type);
 
     // Add template parameters to a class, structure or union types.
     // FIXME: The support isn't in the metadata for this yet.

lib/IR/DIBuilder.cpp

Value *Args[] = {getDbgIntrinsicValueImpl(VMContext, V),
                    MetadataAsValue::get(VMContext, VarInfo),
                    MetadataAsValue::get(VMContext, Expr)};
 
   IRBuilder<> B = getIRBForDbgInsertion(DL, InsertBB, InsertBefore);
   return B.CreateCall(ValueFn, Args);
 }
 
 void DIBuilder::replaceVTableHolder(DICompositeType *&T,
-                                    DICompositeType *VTableHolder) {
+                                    DIType *VTableHolder) {
   {
     TypedTrackingMDRef<DICompositeType> N(T);
     N->replaceVTableHolder(VTableHolder);
     T = N.get();
   }
 
   // If this didn't create a self-reference, just return.
   if (T != VTableHolder)

test/DebugInfo/Generic/containing-type-extension.ll

+; REQUIRES: object-emission
+
+; RUN: %llc_dwarf -O0 -filetype=obj < %s > %t
+; RUN: llvm-dwarfdump -v -debug-info %t | FileCheck %s
+
+; Make sure we correctly handle context of a subprogram being a type identifier.
+; CHECK: [[SP:.*]]: DW_TAG_structure_type
+; CHECK-NOT: TAG
+; CHECK: DW_AT_containing_type [DW_FORM_ref4]
+; CHECK: DW_AT_name [DW_FORM_strp] {{.*}}= "vtable")
+
+; The code doesn't actually matter.
+define i32 @main() #0 !dbg !4 {
+entry:
+  %retval = alloca i32, align 4
+  store i32 0, i32* %retval
+  ret i32 0, !dbg !10
+}
+
+attributes #0 = { nounwind uwtable }
+
+!llvm.dbg.cu = !{!0}
+!llvm.module.flags = !{!9, !11}
+
+!0 = distinct !DICompileUnit(language: DW_LANG_Rust, producer: "clang version 3.4 (trunk 185475)", isOptimized: false, emissionKind: FullDebug, file: !1, enums: !2, retainedTypes: !2, globals: !15, imports: !2)
+!1 = !DIFile(filename: "CodeGen/dwarf-version.c", directory: "test")
+!2 = !{}
+!4 = distinct !DISubprogram(name: "main", line: 6, isLocal: false, isDefinition: true, virtualIndex: 6, flags: DIFlagPrototyped | DIFlagMainSubprogram, isOptimized: false, unit: !0, scopeLine: 6, file: !1, scope: !5, type: !6, variables: !2)
+!5 = !DIFile(filename: "CodeGen/dwarf-version.c", directory: "test")
+!6 = !DISubroutineType(types: !7)
+!7 = !{!8}
+!8 = !DIBasicType(tag: DW_TAG_base_type, name: "int", size: 32, align: 32, encoding: DW_ATE_signed)
+!9 = !{i32 2, !"Dwarf Version", i32 4}
+!10 = !DILocation(line: 7, scope: !4)
+!11 = !{i32 1, !"Debug Info Version", i32 3}
+!12 = !DICompositeType(tag: DW_TAG_structure_type, name: "vtable", size: 8, align: 8, elements: !2, identifier: "vtable", vtableHolder: !8)
+!13 = !DIGlobalVariableExpression(var: !14, expr: !DIExpression())
+!14 = !DIGlobalVariable(name: "vtable", linkageName: "vtable", scope: null, file: !1, line: 1, type: !12, isLocal: true, isDefinition: true)
+!15 = !{!13}

unittests/IR/MetadataTest.cpp

TEST_F(DICompositeTypeTest, replaceOperands) {
   EXPECT_EQ(Elements, N->getElements().get());
   N->replaceElements(nullptr);
   EXPECT_EQ(nullptr, N->getElements().get());
 
   DIType *VTableHolder = getCompositeType();
   EXPECT_EQ(nullptr, N->getVTableHolder());
   N->replaceVTableHolder(VTableHolder);
   EXPECT_EQ(VTableHolder, N->getVTableHolder());
+  // As an extension, the containing type can be anything.  This is
+  // used by Rust to associate vtables with their concrete type.
+  DIType *BasicType = getBasicType("basic");
+  N->replaceVTableHolder(BasicType);
+  EXPECT_EQ(BasicType, N->getVTableHolder());
   N->replaceVTableHolder(nullptr);
   EXPECT_EQ(nullptr, N->getVTableHolder());
 
   auto *TemplateParams = MDTuple::getDistinct(Context, None);
   EXPECT_EQ(nullptr, N->getTemplateParams().get());
   N->replaceTemplateParams(TemplateParams);
   EXPECT_EQ(TemplateParams, N->getTemplateParams().get());
   N->replaceTemplateParams(nullptr);