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();
+}
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();
+}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
