Skip to content

Commit c9906a6

Browse files
jmmartinezkzhuravl
authored andcommitted
[HeterogeneousDwarf] Keep track of GlobalVariables DIFragments using WeakVH
In DwarfDebug::beginModule, the compiler stores for each fragment its associated GlobalVariable in a DenseMap. However, nothing prevents the GlobalVariable from being removed during a later pass, invalidating the reference in the map. In the attached test case, AMDGPULowerLDS removes LDS variables without any use, which resulted in an use-after-free access. The WeakVH works as a reference to the GlobalVariable, however, when the global is erased a callback sets the WeakVH to invalid. Change-Id: If1627f548441f07ab32454ece2beefb518ab2aa2
1 parent 74a46e2 commit c9906a6

File tree

7 files changed

+79
-41
lines changed

7 files changed

+79
-41
lines changed

llvm/lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -204,7 +204,7 @@ DIE *DwarfCompileUnit::getOrCreateGlobalVariableDIE(
204204

205205
DIE *DwarfCompileUnit::getOrCreateGlobalVariableDIE(
206206
const DILifetime &Lifetime,
207-
const DenseMap<DIFragment *, const GlobalVariable *> &GVFragmentMap) {
207+
const DwarfDebug::GVFragmentMapTy &GVFragmentMap) {
208208

209209
const DIGlobalVariable *GV = dyn_cast<DIGlobalVariable>(Lifetime.getObject());
210210

@@ -462,7 +462,7 @@ void DwarfCompileUnit::addLocationAttribute(
462462

463463
void DwarfCompileUnit::addLocationAttribute(
464464
DIE *VariableDIE, const DIGlobalVariable *GV, const DILifetime &Lifetime,
465-
const DenseMap<DIFragment *, const GlobalVariable *> &GVFragmentMap) {
465+
const DwarfDebug::GVFragmentMapTy &GVFragmentMap) {
466466
// FIXME: Determine when this is appropriate, considering the existing
467467
// implementation.
468468
bool AddToAccelTable = true;
@@ -505,7 +505,7 @@ DIE *DwarfCompileUnit::getOrCreateCommonBlock(
505505

506506
DIE *DwarfCompileUnit::getOrCreateCommonBlock(
507507
const DICommonBlock *CB, const DILifetime &Lifetime,
508-
const DenseMap<DIFragment *, const GlobalVariable *> &GVFragmentMap) {
508+
const DwarfDebug::GVFragmentMapTy &GVFragmentMap) {
509509
// Check for pre-existence.
510510
if (DIE *NDie = getDIE(CB))
511511
return NDie;

llvm/lib/CodeGen/AsmPrinter/DwarfCompileUnit.h

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -165,21 +165,21 @@ class DwarfCompileUnit final : public DwarfUnit {
165165

166166
DIE *getOrCreateGlobalVariableDIE(
167167
const DILifetime &Lifetime,
168-
const DenseMap<DIFragment *, const GlobalVariable *> &GVFragmentMap);
168+
const DwarfDebug::GVFragmentMapTy &GVFragmentMap);
169169

170170
DIE *getOrCreateCommonBlock(const DICommonBlock *CB,
171171
ArrayRef<GlobalExpr> GlobalExprs);
172172

173-
DIE *getOrCreateCommonBlock(
174-
const DICommonBlock *CB, const DILifetime &Lifetime,
175-
const DenseMap<DIFragment *, const GlobalVariable *> &GVFragmentMap);
173+
DIE *getOrCreateCommonBlock(const DICommonBlock *CB,
174+
const DILifetime &Lifetime,
175+
const DwarfDebug::GVFragmentMapTy &GVFragmentMap);
176176

177177
void addLocationAttribute(DIE *ToDIE, const DIGlobalVariable *GV,
178178
ArrayRef<GlobalExpr> GlobalExprs);
179179

180-
void addLocationAttribute(
181-
DIE *ToDIE, const DIGlobalVariable *GV, const DILifetime &Lifetime,
182-
const DenseMap<DIFragment *, const GlobalVariable *> &GVFragmentMap);
180+
void addLocationAttribute(DIE *ToDIE, const DIGlobalVariable *GV,
181+
const DILifetime &Lifetime,
182+
const DwarfDebug::GVFragmentMapTy &GVFragmentMap);
183183

184184
/// addLabelAddress - Add a dwarf label attribute data and value using
185185
/// either DW_FORM_addr or DW_FORM_GNU_addr_index.

llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1177,7 +1177,7 @@ void DwarfDebug::beginModule(Module *M) {
11771177
SingleCU = NumDebugCUs == 1;
11781178
DenseMap<DIGlobalVariable *, SmallVector<DwarfCompileUnit::GlobalExpr, 1>>
11791179
GVMap;
1180-
for (const GlobalVariable &Global : M->globals()) {
1180+
for (GlobalVariable &Global : M->globals()) {
11811181
// To support the "inlining" of GV-fragments as an optimization, we record
11821182
// the referrer for each such fragment.
11831183
if (llvm::isHeterogeneousDebug(*M)) {
@@ -2003,10 +2003,10 @@ void DwarfDebug::collectEntityInfo(DwarfCompileUnit &TheCU,
20032003
Entry.finalize(*Asm, List, BT, TheCU);
20042004
}
20052005

2006-
const Module &M = *Asm->MF->getFunction().getParent();
2007-
DenseMap<DICompileUnit *, SmallVector<DILifetime *>> AddCULifetimeMap;
2006+
Module &M = *Asm->MF->getFunction().getParent();
2007+
DenseMap<DICompileUnit *, SmallVector<DILifetime *>> AddCULifetimeMap;
20082008
if (isHeterogeneousDebug(M)) {
2009-
for (const GlobalVariable &Global : M.globals()) {
2009+
for (GlobalVariable &Global : M.globals()) {
20102010
if (DIFragment *F = Global.getDbgDef()) {
20112011
GVFragmentMap[F] = &Global;
20122012
}

llvm/lib/CodeGen/AsmPrinter/DwarfDebug.h

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -492,7 +492,11 @@ class DwarfDebug : public DebugHandlerBase {
492492
/// Avoid using DW_OP_convert due to consumer incompatibilities.
493493
bool EnableOpConvert;
494494

495-
DenseMap<DIFragment *, const GlobalVariable *> GVFragmentMap;
495+
public:
496+
using GVFragmentMapTy = DenseMap<DIFragment *, WeakVH>;
497+
498+
private:
499+
GVFragmentMapTy GVFragmentMap;
496500
DenseMap<DISubprogram *, SmallVector<DILifetime *>> SPLifetimeMap;
497501
DenseSet<DILifetime *> ProcessedLifetimes;
498502

llvm/lib/CodeGen/AsmPrinter/DwarfExpression.cpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -842,10 +842,10 @@ bool DwarfExprAST::tryInlineArgObject(DIObject *ArgObject) {
842842
auto *Fragment = dyn_cast<DIFragment>(ArgObject);
843843
if (!Fragment)
844844
return false;
845-
const auto GV = GVFragmentMap->find(Fragment);
846-
if (GV == GVFragmentMap->end())
845+
const auto GVRef = GVFragmentMap->find(Fragment);
846+
if (GVRef == GVFragmentMap->end() || !GVRef->getSecond())
847847
return false;
848-
const GlobalVariable *Global = GV->getSecond();
848+
const GlobalVariable *Global = cast<GlobalVariable>(GVRef->getSecond());
849849
// FIXME(KZHURAVL): This depends on the target and address space
850850
// semantics. For AMDGPU, address space 3 is lds/local/shared.
851851
// Need to replace this with a target hook!

llvm/lib/CodeGen/AsmPrinter/DwarfExpression.h

Lines changed: 22 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
#define LLVM_LIB_CODEGEN_ASMPRINTER_DWARFEXPRESSION_H
1515

1616
#include "ByteStreamer.h"
17+
#include "DwarfDebug.h"
1718
#include "llvm/ADT/ArrayRef.h"
1819
#include "llvm/ADT/SmallVector.h"
1920
#include "llvm/IR/DebugInfoMetadata.h"
@@ -513,7 +514,7 @@ class DwarfExprAST {
513514
// An `std::optional<const DenseMap<_, _>&>` where `nullptr` represents
514515
// `None`. Only present and applicable as part of an optimization for
515516
// DIFragments which refer to global variable fragments.
516-
const DenseMap<DIFragment *, const GlobalVariable *> *GVFragmentMap;
517+
const DwarfDebug::GVFragmentMapTy *GVFragmentMap;
517518
std::unique_ptr<DwarfExprAST::Node> Root;
518519
// FIXME(KZHURAVL): This is a temporary boolean variable that indicates
519520
// whether the lowering of this expression is supported or not. If the
@@ -586,10 +587,10 @@ class DwarfExprAST {
586587
virtual void emitDwarfLabelDelta(const MCSymbol *Hi, const MCSymbol *Lo) = 0;
587588

588589
public:
589-
DwarfExprAST(
590-
const AsmPrinter &AP, const TargetRegisterInfo *TRI, DwarfCompileUnit &CU,
591-
const DILifetime &Lifetime, const MachineOperand *Referrer,
592-
const DenseMap<DIFragment *, const GlobalVariable *> *GVFragmentMap)
590+
DwarfExprAST(const AsmPrinter &AP, const TargetRegisterInfo *TRI,
591+
DwarfCompileUnit &CU, const DILifetime &Lifetime,
592+
const MachineOperand *Referrer,
593+
const DwarfDebug::GVFragmentMapTy *GVFragmentMap)
593594
: AP(AP), TRI(TRI), CU(CU), Lifetime(Lifetime), Referrer(Referrer),
594595
GVFragmentMap(GVFragmentMap) {
595596
buildDIExprAST();
@@ -610,24 +611,23 @@ class DebugLocDwarfExprAST final : DwarfExprAST {
610611
void emitDwarfOpAddrx(unsigned Index) override;
611612
void emitDwarfLabelDelta(const MCSymbol *Hi, const MCSymbol *Lo) override;
612613

613-
DebugLocDwarfExprAST(
614-
const AsmPrinter &AP, const TargetRegisterInfo *TRI, DwarfCompileUnit &CU,
615-
BufferByteStreamer &BS, const DILifetime &Lifetime,
616-
const MachineOperand *Referrer,
617-
const DenseMap<DIFragment *, const GlobalVariable *> *GVFragmentMap)
614+
DebugLocDwarfExprAST(const AsmPrinter &AP, const TargetRegisterInfo *TRI,
615+
DwarfCompileUnit &CU, BufferByteStreamer &BS,
616+
const DILifetime &Lifetime,
617+
const MachineOperand *Referrer,
618+
const DwarfDebug::GVFragmentMapTy *GVFragmentMap)
618619
: DwarfExprAST(AP, TRI, CU, Lifetime, Referrer, GVFragmentMap),
619620
OutBS(BS) {}
620621

621-
public:
622+
public:
622623
DebugLocDwarfExprAST(const AsmPrinter &AP, const TargetRegisterInfo &TRI,
623624
DwarfCompileUnit &CU, BufferByteStreamer &BS,
624625
const DILifetime &Lifetime,
625626
const MachineOperand &Referrer)
626627
: DebugLocDwarfExprAST(AP, &TRI, CU, BS, Lifetime, &Referrer, nullptr) {}
627-
DebugLocDwarfExprAST(
628-
const AsmPrinter &AP, DwarfCompileUnit &CU, BufferByteStreamer &BS,
629-
const DILifetime &Lifetime,
630-
const DenseMap<DIFragment *, const GlobalVariable *> &GVFragmentMap)
628+
DebugLocDwarfExprAST(const AsmPrinter &AP, DwarfCompileUnit &CU,
629+
BufferByteStreamer &BS, const DILifetime &Lifetime,
630+
const DwarfDebug::GVFragmentMapTy &GVFragmentMap)
631631
: DebugLocDwarfExprAST(AP, nullptr, CU, BS, Lifetime, nullptr,
632632
&GVFragmentMap) {}
633633
DebugLocDwarfExprAST(const DebugLocDwarfExprAST &) = delete;
@@ -653,10 +653,10 @@ class DIEDwarfExprAST final : DwarfExprAST {
653653
void emitDwarfOpAddrx(unsigned Index) override;
654654
void emitDwarfLabelDelta(const MCSymbol *Hi, const MCSymbol *Lo) override;
655655

656-
DIEDwarfExprAST(
657-
const AsmPrinter &AP, const TargetRegisterInfo *TRI, DwarfCompileUnit &CU,
658-
DIELoc &DIE, const DILifetime &Lifetime, const MachineOperand *Referrer,
659-
const DenseMap<DIFragment *, const GlobalVariable *> *GVFragmentMap)
656+
DIEDwarfExprAST(const AsmPrinter &AP, const TargetRegisterInfo *TRI,
657+
DwarfCompileUnit &CU, DIELoc &DIE, const DILifetime &Lifetime,
658+
const MachineOperand *Referrer,
659+
const DwarfDebug::GVFragmentMapTy *GVFragmentMap)
660660
: DwarfExprAST(AP, TRI, CU, Lifetime, Referrer, GVFragmentMap),
661661
OutDIE(DIE) {}
662662

@@ -665,10 +665,9 @@ class DIEDwarfExprAST final : DwarfExprAST {
665665
DwarfCompileUnit &CU, DIELoc &DIE, const DILifetime &Lifetime,
666666
const MachineOperand &Referrer)
667667
: DIEDwarfExprAST(AP, &TRI, CU, DIE, Lifetime, &Referrer, nullptr) {}
668-
DIEDwarfExprAST(
669-
const AsmPrinter &AP, DwarfCompileUnit &CU, DIELoc &DIE,
670-
const DILifetime &Lifetime,
671-
const DenseMap<DIFragment *, const GlobalVariable *> &GVFragmentMap)
668+
DIEDwarfExprAST(const AsmPrinter &AP, DwarfCompileUnit &CU, DIELoc &DIE,
669+
const DILifetime &Lifetime,
670+
const DwarfDebug::GVFragmentMapTy &GVFragmentMap)
672671
: DIEDwarfExprAST(AP, nullptr, CU, DIE, Lifetime, nullptr,
673672
&GVFragmentMap) {}
674673
DIEDwarfExprAST(const DIEDwarfExprAST &) = delete;
Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
; RUN: llc -O0 -mtriple=amdgcn-amd-amdhsa -mcpu=gfx906 -verify-machineinstrs -filetype=obj %s -o - | llvm-dwarfdump -v -debug-info - | FileCheck %s
2+
; CHECK: DW_TAG_compile_unit
3+
; CHECK: DW_TAG_subprogram
4+
; CHECK: DW_TAG_variable
5+
; CHECK: DW_AT_name [DW_FORM_strx1] (indexed ({{[0-9]+}}) string = "unused")
6+
; CHECK: DW_AT_LLVM_memory_space [DW_FORM_data4] (DW_MSPACE_LLVM_group)
7+
; CHECK: DW_AT_location [DW_FORM_exprloc] (<empty>)
8+
; CHECK-NOT: DW_TAG_subprogram
9+
; CHECK-NOT: DW_TAG_variable
10+
11+
@unused = external addrspace(3) global [4 x float], !dbg.def !0
12+
13+
define amdgpu_kernel void @foo() !dbg !1 {
14+
ret void
15+
}
16+
17+
!llvm.module.flags = !{!11, !12}
18+
!llvm.dbg.cu = !{!2}
19+
!llvm.dbg.retainedNodes = !{!5}
20+
21+
!0 = distinct !DIFragment()
22+
!1 = distinct !DISubprogram(name: "foo", linkageName: "foo", scope: !2, file: !3, line: 1, type: !13, scopeLine: 1, flags: DIFlagPrototyped, spFlags: DISPFlagDefinition, unit: !2)
23+
!2 = distinct !DICompileUnit(language: DW_LANG_C_plus_plus_14, file: !3, producer: "clang", isOptimized: false, runtimeVersion: 0, emissionKind: FullDebug, enums: !4, retainedTypes: !4, imports: !4, splitDebugInlining: false, nameTableKind: None)
24+
!3 = !DIFile(filename: "file.cpp", directory: "/dir")
25+
!4 = !{}
26+
!5 = distinct !DILifetime(object: !6, location: !DIExpr(DIOpArg(0, ptr addrspace(3)), DIOpDeref([4 x float])), argObjects: {!0})
27+
!6 = distinct !DIGlobalVariable(name: "unused", scope: !1, file: !3, line: 1, type: !7, isLocal: false, isDefinition: true, memorySpace: DW_MSPACE_LLVM_group)
28+
!7 = !DICompositeType(tag: DW_TAG_array_type, baseType: !8, size: 128, elements: !9)
29+
!8 = !DIBasicType(name: "float", size: 32, encoding: DW_ATE_float)
30+
!9 = !{!10}
31+
!10 = !DISubrange(count: 4)
32+
!11 = !{i32 7, !"Dwarf Version", i32 5}
33+
!12 = !{i32 2, !"Debug Info Version", i32 4}
34+
!13 = distinct !DISubroutineType(types: !14)
35+
!14 = !{null}

0 commit comments

Comments
 (0)