[llvm] 4ecb2b8 - [DebugInfo][Metadata] Make AllEnumTypes holding TrackingMDNodeRef

[Kristina Bessonova via llvm-commits]

Author: Kristina Bessonova
Date: 2022-11-03T10:28:38+02:00
New Revision: 4ecb2b8ef6be69b55d46ac274f3b7a7103219f98

URL: https://github.com/llvm/llvm-project/commit/4ecb2b8ef6be69b55d46ac274f3b7a7103219f98
DIFF: https://github.com/llvm/llvm-project/commit/4ecb2b8ef6be69b55d46ac274f3b7a7103219f98.diff

LOG: [DebugInfo][Metadata] Make AllEnumTypes holding TrackingMDNodeRef

Having AllEnumtypes to be a vector of TrackingMDNodeRef makes it possible
to reflect changes in metadata in the vector if they took place before DIBuilder
being finalized.

Otherwise, we end up with heap-use-after-free because AllEnumTypes contains
metadata that no longer valid.

Consider a case where we have a class containing a definition of a enum,
so this enum has the class as a scope. For some reason (doesn't matter for
the current issue), we create a temporary debug metadata for this class, and
then resolve it while finalizing CGDebugInfo.

In the case of collision during uniqifying the temporary, we then need
to replace its uses with a new pointer. If a temporary's user is unique
(this is the enum mentioned above), we may need re-uniquefying it,
which may return a new pointer in the case of another collision.
If so, the pointer we stored in AllEnumTypes vector become dangling.
Making AllEnumTypes hodling TrackingMDNodeRef should solve this problem
(see debug-info-enum-metadata-collision.cpp test for details).

Reviewed By: dblaikie

Differential Revision: https://reviews.llvm.org/D137067

Added: 
    clang/test/CodeGenCXX/debug-info-enum-metadata-collision.cpp

Modified: 
    llvm/include/llvm/IR/DIBuilder.h
    llvm/lib/IR/DIBuilder.cpp

Removed: 
    


################################################################################
diff  --git a/clang/test/CodeGenCXX/debug-info-enum-metadata-collision.cpp b/clang/test/CodeGenCXX/debug-info-enum-metadata-collision.cpp
new file mode 100644
index 0000000000000..dd27acd0a77c5
--- /dev/null
+++ b/clang/test/CodeGenCXX/debug-info-enum-metadata-collision.cpp
@@ -0,0 +1,25 @@
+// RUN: %clang_cc1 -triple %itanium_abi_triple -emit-llvm -debug-info-kind=constructor %s -o - | FileCheck %s
+
+// Test that clang doesn't crash while resolving temporary debug metadata of
+// a record with collisions in the record's enum users.
+
+// CHECK:      !DICompositeType(tag: DW_TAG_enumeration_type,
+// CHECK-SAME:                  scope: [[SCOPE:![0-9]+]]
+// CHECK-SAME:                  elements: [[ELEMENTS:![0-9]+]]
+// CHECK:      [[SCOPE]] = !DICompositeType(tag: DW_TAG_structure_type
+// CHECK-SAME:                              name: "Struct1<Struct3>"
+// CHECK:      [[ELEMENTS]] = !{[[ELEMENT:![0-9]+]]}
+// CHECK:      [[ELEMENT]] = !DIEnumerator(name: "enumValue1"
+
+template <typename> struct Struct1 {
+  enum { enumValue1 };
+  Struct1();
+};
+void function2() {
+  struct Struct3 {};
+  int i = Struct1<Struct3>::enumValue1;
+}
+void function3() {
+  struct Struct3 {};
+  int i = Struct1<Struct3>::enumValue1;
+}

diff  --git a/llvm/include/llvm/IR/DIBuilder.h b/llvm/include/llvm/IR/DIBuilder.h
index dfb65ce341296..61fa4d8f3b9fd 100644
--- a/llvm/include/llvm/IR/DIBuilder.h
+++ b/llvm/include/llvm/IR/DIBuilder.h
@@ -48,7 +48,7 @@ namespace llvm {
     Function *LabelFn;       ///< llvm.dbg.label
     Function *AddrFn;        ///< llvm.dbg.addr
 
-    SmallVector<Metadata *, 4> AllEnumTypes;
+    SmallVector<TrackingMDNodeRef, 4> AllEnumTypes;
     /// Track the RetainTypes, since they can be updated later on.
     SmallVector<TrackingMDNodeRef, 4> AllRetainTypes;
     SmallVector<Metadata *, 4> AllSubprograms;

diff  --git a/llvm/lib/IR/DIBuilder.cpp b/llvm/lib/IR/DIBuilder.cpp
index fada07ac383ae..76d7ade09a88c 100644
--- a/llvm/lib/IR/DIBuilder.cpp
+++ b/llvm/lib/IR/DIBuilder.cpp
@@ -84,7 +84,9 @@ void DIBuilder::finalize() {
   }
 
   if (!AllEnumTypes.empty())
-    CUNode->replaceEnumTypes(MDTuple::get(VMContext, AllEnumTypes));
+    CUNode->replaceEnumTypes(MDTuple::get(
+        VMContext, SmallVector<Metadata *, 16>(AllEnumTypes.begin(),
+                                               AllEnumTypes.end())));
 
   SmallVector<Metadata *, 16> RetainValues;
   // Declarations and definitions of the same type may be retained. Some
@@ -556,7 +558,7 @@ DICompositeType *DIBuilder::createEnumerationType(
       getNonCompileUnitScope(Scope), UnderlyingType, SizeInBits, AlignInBits, 0,
       IsScoped ? DINode::FlagEnumClass : DINode::FlagZero, Elements, 0, nullptr,
       nullptr, UniqueIdentifier);
-  AllEnumTypes.push_back(CTy);
+  AllEnumTypes.emplace_back(CTy);
   trackIfUnresolved(CTy);
   return CTy;
 }