maxjhandsome
/
llvm-project
forked from OSchip/llvm-project
1
0
Fork 0
Code Issues Pull Requests Packages Releases Wiki Activity

[DebugInfo] Skip ODRUniquing for mismatched tags 

Otherwise, ODRUniquing would map some member method/variable MDNodes
to have enum type DIScope, resulting in invalid debug info and bad
DWARF.

- Add a Verifier check that when a 'scope:' operand is an ODR type that is not an enum.
- Makes ODRUniquing apply to only ODR types with the same tag so that the debuginfo/DWARF is well-formed.

Reviewed By: probinson, aprantl

Differential Revision: https://reviews.llvm.org/D111770
This commit is contained in:
Yuanfang Chen 2021-10-26 13:58:43 -07:00
parent 2887d9fd86
commit 7c3fa52785
5 changed files with 124 additions and 34 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

9
llvm/lib/IR/DebugInfoMetadata.cpp
View File

@ -640,6 +640,9 @@ DICompositeType *DICompositeType::buildODRType(
VTableHolder, TemplateParams, &Identifier, Discriminator,
DataLocation, Associated, Allocated, Rank, Annotations);
if (CT->getTag() != Tag)
return nullptr;
// Only mutate CT if it's a forward declaration and the new operands aren't.
assert(CT->getRawIdentifier() == &Identifier && "Wrong ODR identifier?");
if (!CT->isForwardDecl() || (Flags & DINode::FlagFwdDecl))
@ -672,12 +675,16 @@ DICompositeType *DICompositeType::getODRType(
if (!Context.isODRUniquingDebugTypes())
return nullptr;
auto *&CT = (*Context.pImpl->DITypeMap)[&Identifier];
if (!CT)
if (!CT) {
CT = DICompositeType::getDistinct(
Context, Tag, Name, File, Line, Scope, BaseType, SizeInBits,
AlignInBits, OffsetInBits, Flags, Elements, RuntimeLang, VTableHolder,
TemplateParams, &Identifier, Discriminator, DataLocation, Associated,
Allocated, Rank, Annotations);
} else {
if (CT->getTag() != Tag)
return nullptr;
}
return CT;
}

21
llvm/lib/IR/Verifier.cpp
View File

@ -549,6 +549,8 @@ private:
void verifyFunctionAttrs(FunctionType *FT, AttributeList Attrs,
const Value *V, bool IsIntrinsic);
void verifyFunctionMetadata(ArrayRef<std::pair<unsigned, MDNode *>> MDs);
template <typename T>
void verifyODRTypeAsScopeOperand(const MDNode &MD, T * = nullptr);
void visitConstantExprsRecursively(const Constant *EntryC);
void visitConstantExpr(const ConstantExpr *CE);
@ -839,6 +841,19 @@ void Verifier::visitNamedMDNode(const NamedMDNode &NMD) {
}
}
template <typename T>
void Verifier::verifyODRTypeAsScopeOperand(const MDNode &MD, T *) {
if (isa<T>(MD)) {
if (auto *N = dyn_cast_or_null<DICompositeType>(cast<T>(MD).getScope()))
// Of all the supported tags for DICompositeType(see visitDICompositeType)
// we know that enum type cannot be a scope.
AssertDI(N->getTag() != dwarf::DW_TAG_enumeration_type,
"enum type is not a scope; check enum type ODR "
"violation",
N, &MD);
}
}
void Verifier::visitMDNode(const MDNode &MD, AreDebugLocsAllowed AllowLocs) {
// Only visit each node once. Metadata can be mutually recursive, so this
// avoids infinite recursion here, as well as being an optimization.
@ -848,6 +863,12 @@ void Verifier::visitMDNode(const MDNode &MD, AreDebugLocsAllowed AllowLocs) {
Assert(&MD.getContext() == &Context,
"MDNode context does not match Module context!", &MD);
// Makes sure when a scope operand is a ODR type, the ODR type uniquing does
// not create invalid debug metadata.
// TODO: check that the non-ODR-type scope operand is valid.
verifyODRTypeAsScopeOperand<DIType>(MD);
verifyODRTypeAsScopeOperand<DILocalScope>(MD);
switch (MD.getMetadataID()) {
default:
llvm_unreachable("Invalid MDNode subclass");

47
llvm/test/Linker/debug-info-bad-enum.ll Normal file
View File

@ -0,0 +1,47 @@
; Check that ODR type uniquing does not create invalid debug metadata.
; RUN: split-file %s %t
; RUN: llvm-link -S -o - %t/a.ll %t/b.ll 2>&1 | FileCheck %s
; CHECK-DAG: distinct !DICompileUnit({{.*}}, enums: ![[ENUMLIST:[0-9]+]],
; CHECK-DAG: ![[ENUMLIST]] = !{![[ENUM:[0-9]+]]}
; CHECK-DAG: ![[ENUM]] = distinct !DICompositeType(tag: DW_TAG_enumeration_type, {{.*}}, identifier: "_ZTS5Stage"
; CHECK-DAG: distinct !DICompileUnit({{.*}}, retainedTypes: ![[RETAIN:[0-9]+]],
; CHECK-DAG: ![[RETAIN]] = !{![[VAR:[0-9]+]]}
; CHECK-DAG: ![[CLASS:[0-9]+]] = distinct !DICompositeType(tag: DW_TAG_class_type, {{.*}}, identifier: "_ZTS5Stage"
; CHECK-DAG: ![[VAR]] = !DIDerivedType(tag: DW_TAG_member, name: "Var", scope: ![[CLASS]],
; CHECK-NOT: enum type is not a scope; check enum type ODR violation
;--- a.ll
target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
target triple = "x86_64-unknown-linux-gnu"
!llvm.dbg.cu = !{!0}
!llvm.module.flags = !{!9}
!0 = distinct !DICompileUnit(language: DW_LANG_C_plus_plus_14, file: !1, producer: "clang", isOptimized: true, emissionKind: FullDebug, enums: !2, splitDebugInlining: false, nameTableKind: None)
!1 = !DIFile(filename: "a.cpp", directory: "file")
!2 = !{!3}
!3 = !DICompositeType(tag: DW_TAG_enumeration_type, name: "Stage", file: !1, line: 3, baseType: !4, size: 32, elements: !5, identifier: "_ZTS5Stage")
!4 = !DIBasicType(name: "unsigned int", size: 32, encoding: DW_ATE_unsigned)
!5 = !{!6}
!6 = !DIEnumerator(name: "A1", value: 0, isUnsigned: true)
!9 = !{i32 2, !"Debug Info Version", i32 3}
;--- b.ll
target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
target triple = "x86_64-unknown-linux-gnu"
!llvm.dbg.cu = !{!2}
!llvm.module.flags = !{!11}
!2 = distinct !DICompileUnit(language: DW_LANG_C_plus_plus_14, file: !3, producer: "clang", isOptimized: true, emissionKind: FullDebug, retainedTypes: !10, splitDebugInlining: false, nameTableKind: None)
!3 = !DIFile(filename: "b.cpp", directory: "file")
!5 = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed)
!6 = !DIDerivedType(tag: DW_TAG_member, name: "Var", scope: !8, file: !3, line: 5, baseType: !5, flags: DIFlagStaticMember)
!8 = distinct !DICompositeType(tag: DW_TAG_class_type, name: "Stage", file: !3, line: 3, size: 64, flags: DIFlagTypePassByValue, elements: !9, identifier: "_ZTS5Stage")
!9 = !{!6}
!10 = !{!6}
!11 = !{i32 2, !"Debug Info Version", i32 3}

16
llvm/test/Verifier/dbg-invalid-enum-as-scope.ll Normal file
View File

@ -0,0 +1,16 @@
; RUN: llvm-as -disable-output <%s 2>&1 | FileCheck %s
; CHECK: enum type is not a scope; check enum type ODR violation
; CHECK: warning: ignoring invalid debug info
!llvm.module.flags = !{!0}
!0 = !{i32 2, !"Debug Info Version", i32 3}
!llvm.dbg.cu = !{!1}
!1 = distinct !DICompileUnit(language: DW_LANG_C99, file: !2, producer: "clang", isOptimized: false, runtimeVersion: 0, emissionKind: FullDebug, retainedTypes: !13, enums: !3)
!2 = !DIFile(filename: "file.c", directory: "dir")
!3 = !{!4}
!4 = distinct !DICompositeType(tag: DW_TAG_enumeration_type, name: "Stage", file: !2, line: 3, baseType: !10, size: 32, elements: !11, identifier: "_ZTS5Stage")
!6 = !DIDerivedType(tag: DW_TAG_member, name: "Var", scope: !4, file: !2, line: 5, baseType: !10)
!10 = !DIBasicType(name: "unsigned int", size: 32, encoding: DW_ATE_unsigned)
!11 = !{!12}
!12 = !DIEnumerator(name: "A1", value: 0, isUnsigned: true)
!13 = !{!6}

65
llvm/unittests/IR/DebugTypeODRUniquingTest.cpp
View File

@ -81,38 +81,38 @@ TEST(DebugTypeODRUniquingTest, buildODRType) {
EXPECT_EQ(dwarf::DW_TAG_class_type, CT.getTag());
// Update with another forward decl. This should be a no-op.
EXPECT_EQ(&CT,
DICompositeType::buildODRType(
Context, UUID, dwarf::DW_TAG_structure_type, nullptr, nullptr,
0, nullptr, nullptr, 0, 0, 0, DINode::FlagFwdDecl, nullptr, 0,
nullptr, nullptr, nullptr, nullptr, nullptr, nullptr, nullptr,
nullptr));
EXPECT_EQ(dwarf::DW_TAG_class_type, CT.getTag());
EXPECT_EQ(&CT, DICompositeType::buildODRType(
Context, UUID, dwarf::DW_TAG_class_type, nullptr, nullptr,
0, nullptr, nullptr, 0, 0, 0, DINode::FlagFwdDecl, nullptr,
0, nullptr, nullptr, nullptr, nullptr, nullptr, nullptr,
nullptr, nullptr));
EXPECT_FALSE(DICompositeType::buildODRType(
Context, UUID, dwarf::DW_TAG_structure_type, nullptr, nullptr, 0, nullptr,
nullptr, 0, 0, 0, DINode::FlagFwdDecl, nullptr, 0, nullptr, nullptr,
nullptr, nullptr, nullptr, nullptr, nullptr, nullptr));
// Update with a definition. This time we should see a change.
EXPECT_EQ(&CT,
DICompositeType::buildODRType(
Context, UUID, dwarf::DW_TAG_structure_type, nullptr, nullptr,
0, nullptr, nullptr, 0, 0, 0, DINode::FlagZero, nullptr, 0,
nullptr, nullptr, nullptr, nullptr, nullptr, nullptr, nullptr,
nullptr));
EXPECT_EQ(dwarf::DW_TAG_structure_type, CT.getTag());
EXPECT_EQ(&CT, DICompositeType::buildODRType(
Context, UUID, dwarf::DW_TAG_class_type, nullptr, nullptr,
0, nullptr, nullptr, 0, 0, 0, DINode::FlagZero, nullptr, 0,
nullptr, nullptr, nullptr, nullptr, nullptr, nullptr,
nullptr, nullptr));
EXPECT_FALSE(CT.isForwardDecl());
// Further updates should be ignored.
EXPECT_EQ(&CT,
DICompositeType::buildODRType(
Context, UUID, dwarf::DW_TAG_class_type, nullptr, nullptr, 0,
nullptr, nullptr, 0, 0, 0, DINode::FlagFwdDecl, nullptr, 0,
nullptr, nullptr, nullptr, nullptr, nullptr, nullptr, nullptr,
nullptr));
EXPECT_EQ(dwarf::DW_TAG_structure_type, CT.getTag());
EXPECT_EQ(&CT,
DICompositeType::buildODRType(
Context, UUID, dwarf::DW_TAG_class_type, nullptr, nullptr, 0,
nullptr, nullptr, 0, 0, 0, DINode::FlagZero, nullptr, 0,
nullptr, nullptr, nullptr, nullptr, nullptr, nullptr, nullptr,
nullptr));
EXPECT_EQ(dwarf::DW_TAG_structure_type, CT.getTag());
EXPECT_EQ(&CT, DICompositeType::buildODRType(
Context, UUID, dwarf::DW_TAG_class_type, nullptr, nullptr,
0, nullptr, nullptr, 0, 0, 0, DINode::FlagFwdDecl, nullptr,
0, nullptr, nullptr, nullptr, nullptr, nullptr, nullptr,
nullptr, nullptr));
EXPECT_FALSE(CT.isForwardDecl());
EXPECT_EQ(&CT, DICompositeType::buildODRType(
Context, UUID, dwarf::DW_TAG_class_type, nullptr, nullptr,
111u, nullptr, nullptr, 0, 0, 0, DINode::FlagZero, nullptr,
0, nullptr, nullptr, nullptr, nullptr, nullptr, nullptr,
nullptr, nullptr));
EXPECT_NE(111u, CT.getLine());
}
TEST(DebugTypeODRUniquingTest, buildODRTypeFields) {
@ -136,7 +136,6 @@ TEST(DebugTypeODRUniquingTest, buildODRTypeFields) {
DO_FOR_FIELD(VTableHolder) \
DO_FOR_FIELD(TemplateParams)
#define FOR_EACH_INLINEFIELD() \
DO_FOR_FIELD(Tag) \
DO_FOR_FIELD(Line) \
DO_FOR_FIELD(SizeInBits) \
DO_FOR_FIELD(AlignInBits) \
@ -155,10 +154,10 @@ TEST(DebugTypeODRUniquingTest, buildODRTypeFields) {
// Replace all the fields with new values that are distinct from each other.
EXPECT_EQ(&CT,
DICompositeType::buildODRType(
Context, UUID, Tag, Name, File, Line, Scope, BaseType,
SizeInBits, AlignInBits, OffsetInBits, DINode::FlagArtificial,
Elements, RuntimeLang, VTableHolder, TemplateParams, nullptr,
nullptr, nullptr, nullptr, nullptr, nullptr));
Context, UUID, 0, Name, File, Line, Scope, BaseType, SizeInBits,
AlignInBits, OffsetInBits, DINode::FlagArtificial, Elements,
RuntimeLang, VTableHolder, TemplateParams, nullptr, nullptr,
nullptr, nullptr, nullptr, nullptr));
// Confirm that all the right fields got updated.
#define DO_FOR_FIELD(X) EXPECT_EQ(X, CT.getRaw##X());