[PATCH] D137067: [DebugInfo][Metadata] Make AllEnumTypes holding TrackingMDNodeRef

[Kristina Bessonova via Phabricator via cfe-commits]

krisb created this revision.
krisb added reviewers: ellis, aprantl, dblaikie.
Herald added a subscriber: hiraditya.
Herald added a project: All.
krisb requested review of this revision.
Herald added projects: clang, LLVM.
Herald added subscribers: llvm-commits, cfe-commits.

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.

Once resolved the temporary, we may 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-uniqueing it, which may return a new pointer in a case of 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).


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D137067

Files:
  clang/test/CodeGenCXX/debug-info-enum-metadata-collision.cpp
  llvm/include/llvm/IR/DIBuilder.h
  llvm/lib/IR/DIBuilder.cpp


Index: llvm/lib/IR/DIBuilder.cpp
===================================================================
--- llvm/lib/IR/DIBuilder.cpp
+++ llvm/lib/IR/DIBuilder.cpp
@@ -84,7 +84,9 @@
   }
 
   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 @@
       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;
 }
Index: llvm/include/llvm/IR/DIBuilder.h
===================================================================
--- llvm/include/llvm/IR/DIBuilder.h
+++ llvm/include/llvm/IR/DIBuilder.h
@@ -48,7 +48,7 @@
     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;
Index: clang/test/CodeGenCXX/debug-info-enum-metadata-collision.cpp
===================================================================
--- /dev/null
+++ clang/test/CodeGenCXX/debug-info-enum-metadata-collision.cpp
@@ -0,0 +1,30 @@
+// 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();
+};
+template <typename Typename1> struct Struct2 {
+  void function1() { (void)Struct1<Typename1>::enumValue1; }
+};
+void function2() {
+  struct Struct3 {};
+  Struct2<Struct3> variable1;
+  variable1.function1();
+}
+void function3() {
+  struct Struct3 {};
+  Struct2<Struct3> variable1;
+  variable1.function1();
+}


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D137067.471946.patch
Type: text/x-patch
Size: 2820 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20221031/abc048ed/attachment-0001.bin>