Skip to content

Commit 19032bf

Browse files
authored
[aarch64][win] Update Called Globals info when updating Call Site info (#122762)
Fixes the "use after poison" issue introduced by #121516 (see <#121516 (comment)>). The root cause of this issue is that #121516 introduced "Called Global" information for call instructions modeling how "Call Site" info is stored in the machine function, HOWEVER it didn't copy the copy/move/erase operations for call site information. The fix is to rename and update the existing copy/move/erase functions so they also take care of Called Global info.
1 parent 283dca5 commit 19032bf

28 files changed

+176
-157
lines changed

llvm/include/llvm/CodeGen/MachineFunction.h

Lines changed: 20 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -354,11 +354,6 @@ class LLVM_ABI MachineFunction {
354354
/// a table of valid targets for Windows EHCont Guard.
355355
std::vector<MCSymbol *> CatchretTargets;
356356

357-
/// Mapping of call instruction to the global value and target flags that it
358-
/// calls, if applicable.
359-
DenseMap<const MachineInstr *, std::pair<const GlobalValue *, unsigned>>
360-
CalledGlobalsMap;
361-
362357
/// \name Exception Handling
363358
/// \{
364359

@@ -494,6 +489,11 @@ class LLVM_ABI MachineFunction {
494489
SmallVector<ArgRegPair, 1> ArgRegPairs;
495490
};
496491

492+
struct CalledGlobalInfo {
493+
const GlobalValue *Callee;
494+
unsigned TargetFlags;
495+
};
496+
497497
private:
498498
Delegate *TheDelegate = nullptr;
499499
GISelChangeObserver *Observer = nullptr;
@@ -506,6 +506,11 @@ class LLVM_ABI MachineFunction {
506506
/// instruction if debug entry value support is enabled.
507507
CallSiteInfoMap::iterator getCallSiteInfo(const MachineInstr *MI);
508508

509+
using CalledGlobalsMap = DenseMap<const MachineInstr *, CalledGlobalInfo>;
510+
/// Mapping of call instruction to the global value and target flags that it
511+
/// calls, if applicable.
512+
CalledGlobalsMap CalledGlobalsInfo;
513+
509514
// Callbacks for insertion and removal.
510515
void handleInsertion(MachineInstr &MI);
511516
void handleRemoval(MachineInstr &MI);
@@ -1189,22 +1194,20 @@ class LLVM_ABI MachineFunction {
11891194

11901195
/// Tries to get the global and target flags for a call site, if the
11911196
/// instruction is a call to a global.
1192-
std::pair<const GlobalValue *, unsigned>
1193-
tryGetCalledGlobal(const MachineInstr *MI) const {
1194-
return CalledGlobalsMap.lookup(MI);
1197+
CalledGlobalInfo tryGetCalledGlobal(const MachineInstr *MI) const {
1198+
return CalledGlobalsInfo.lookup(MI);
11951199
}
11961200

11971201
/// Notes the global and target flags for a call site.
1198-
void addCalledGlobal(const MachineInstr *MI,
1199-
std::pair<const GlobalValue *, unsigned> Details) {
1202+
void addCalledGlobal(const MachineInstr *MI, CalledGlobalInfo Details) {
12001203
assert(MI && "MI must not be null");
1201-
assert(Details.first && "Global must not be null");
1202-
CalledGlobalsMap.insert({MI, Details});
1204+
assert(Details.Callee && "Global must not be null");
1205+
CalledGlobalsInfo.insert({MI, Details});
12031206
}
12041207

12051208
/// Iterates over the full set of call sites and their associated globals.
12061209
auto getCalledGlobals() const {
1207-
return llvm::make_range(CalledGlobalsMap.begin(), CalledGlobalsMap.end());
1210+
return llvm::make_range(CalledGlobalsInfo.begin(), CalledGlobalsInfo.end());
12081211
}
12091212

12101213
/// \name Exception Handling
@@ -1383,7 +1386,7 @@ class LLVM_ABI MachineFunction {
13831386

13841387
/// Start tracking the arguments passed to the call \p CallI.
13851388
void addCallSiteInfo(const MachineInstr *CallI, CallSiteInfo &&CallInfo) {
1386-
assert(CallI->isCandidateForCallSiteEntry());
1389+
assert(CallI->isCandidateForAdditionalCallInfo());
13871390
bool Inserted =
13881391
CallSitesInfo.try_emplace(CallI, std::move(CallInfo)).second;
13891392
(void)Inserted;
@@ -1399,18 +1402,16 @@ class LLVM_ABI MachineFunction {
13991402

14001403
/// Erase the call site info for \p MI. It is used to remove a call
14011404
/// instruction from the instruction stream.
1402-
void eraseCallSiteInfo(const MachineInstr *MI);
1405+
void eraseAdditionalCallInfo(const MachineInstr *MI);
14031406
/// Copy the call site info from \p Old to \ New. Its usage is when we are
14041407
/// making a copy of the instruction that will be inserted at different point
14051408
/// of the instruction stream.
1406-
void copyCallSiteInfo(const MachineInstr *Old,
1407-
const MachineInstr *New);
1409+
void copyAdditionalCallInfo(const MachineInstr *Old, const MachineInstr *New);
14081410

14091411
/// Move the call site info from \p Old to \New call site info. This function
14101412
/// is used when we are replacing one call instruction with another one to
14111413
/// the same callee.
1412-
void moveCallSiteInfo(const MachineInstr *Old,
1413-
const MachineInstr *New);
1414+
void moveAdditionalCallInfo(const MachineInstr *Old, const MachineInstr *New);
14141415

14151416
unsigned getNewDebugInstrNum() {
14161417
return ++DebugInstrNumberingCount;

llvm/include/llvm/CodeGen/MachineInstr.h

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -957,13 +957,14 @@ class MachineInstr
957957
return hasProperty(MCID::Call, Type);
958958
}
959959

960-
/// Return true if this is a call instruction that may have an associated
961-
/// call site entry in the debug info.
962-
bool isCandidateForCallSiteEntry(QueryType Type = IgnoreBundle) const;
960+
/// Return true if this is a call instruction that may have an additional
961+
/// information associated with it.
962+
bool isCandidateForAdditionalCallInfo(QueryType Type = IgnoreBundle) const;
963+
963964
/// Return true if copying, moving, or erasing this instruction requires
964-
/// updating Call Site Info (see \ref copyCallSiteInfo, \ref moveCallSiteInfo,
965-
/// \ref eraseCallSiteInfo).
966-
bool shouldUpdateCallSiteInfo() const;
965+
/// updating additional call info (see \ref copyCallInfo, \ref moveCallInfo,
966+
/// \ref eraseCallInfo).
967+
bool shouldUpdateAdditionalCallInfo() const;
967968

968969
/// Returns true if the specified instruction stops control flow
969970
/// from executing the instruction immediately following it. Examples include

llvm/include/llvm/CodeGen/SelectionDAG.h

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -287,13 +287,14 @@ class SelectionDAG {
287287
SDDbgInfo *DbgInfo;
288288

289289
using CallSiteInfo = MachineFunction::CallSiteInfo;
290+
using CalledGlobalInfo = MachineFunction::CalledGlobalInfo;
290291

291292
struct NodeExtraInfo {
292293
CallSiteInfo CSInfo;
293294
MDNode *HeapAllocSite = nullptr;
294295
MDNode *PCSections = nullptr;
295296
MDNode *MMRA = nullptr;
296-
std::pair<const GlobalValue *, unsigned> CalledGlobal{};
297+
CalledGlobalInfo CalledGlobal{};
297298
bool NoMerge = false;
298299
};
299300
/// Out-of-line extra information for SDNodes.
@@ -2380,8 +2381,7 @@ class SelectionDAG {
23802381
SDEI[Node].CalledGlobal = {GV, OpFlags};
23812382
}
23822383
/// Return CalledGlobal associated with Node, or a nullopt if none exists.
2383-
std::optional<std::pair<const GlobalValue *, unsigned>>
2384-
getCalledGlobal(const SDNode *Node) {
2384+
std::optional<CalledGlobalInfo> getCalledGlobal(const SDNode *Node) {
23852385
auto I = SDEI.find(Node);
23862386
return I != SDEI.end()
23872387
? std::make_optional(std::move(I->second).CalledGlobal)

llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -918,7 +918,7 @@ void DwarfDebug::constructCallSiteEntryDIEs(const DISubprogram &SP,
918918

919919
// Skip instructions which aren't calls. Both calls and tail-calling jump
920920
// instructions (e.g TAILJMPd64) are classified correctly here.
921-
if (!MI.isCandidateForCallSiteEntry())
921+
if (!MI.isCandidateForAdditionalCallInfo())
922922
continue;
923923

924924
// Skip instructions marked as frame setup, as they are not interesting to
@@ -2019,7 +2019,7 @@ void DwarfDebug::beginInstruction(const MachineInstr *MI) {
20192019

20202020
// When describing calls, we need a label for the call instruction.
20212021
if (!NoDebug && SP->areAllCallsDescribed() &&
2022-
MI->isCandidateForCallSiteEntry(MachineInstr::AnyInBundle) &&
2022+
MI->isCandidateForAdditionalCallInfo(MachineInstr::AnyInBundle) &&
20232023
(!MI->hasDelaySlot() || delaySlotSupported(*MI))) {
20242024
const TargetInstrInfo *TII = MF.getSubtarget().getInstrInfo();
20252025
bool IsTail = TII->isTailCall(*MI);

llvm/lib/CodeGen/BranchFolding.cpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -165,10 +165,10 @@ void BranchFolder::RemoveDeadBlock(MachineBasicBlock *MBB) {
165165
// Avoid matching if this pointer gets reused.
166166
TriedMerging.erase(MBB);
167167

168-
// Update call site info.
168+
// Update call info.
169169
for (const MachineInstr &MI : *MBB)
170-
if (MI.shouldUpdateCallSiteInfo())
171-
MF->eraseCallSiteInfo(&MI);
170+
if (MI.shouldUpdateAdditionalCallInfo())
171+
MF->eraseAdditionalCallInfo(&MI);
172172

173173
// Remove the block.
174174
MF->erase(MBB);

llvm/lib/CodeGen/IfConversion.cpp

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1834,9 +1834,9 @@ bool IfConverter::IfConvertDiamondCommon(
18341834
}
18351835
while (NumDups1 != 0) {
18361836
// Since this instruction is going to be deleted, update call
1837-
// site info state if the instruction is call instruction.
1838-
if (DI2->shouldUpdateCallSiteInfo())
1839-
MBB2.getParent()->eraseCallSiteInfo(&*DI2);
1837+
// info state if the instruction is call instruction.
1838+
if (DI2->shouldUpdateAdditionalCallInfo())
1839+
MBB2.getParent()->eraseAdditionalCallInfo(&*DI2);
18401840

18411841
++DI2;
18421842
if (DI2 == MBB2.end())
@@ -1883,9 +1883,9 @@ bool IfConverter::IfConvertDiamondCommon(
18831883
--DI1;
18841884

18851885
// Since this instruction is going to be deleted, update call
1886-
// site info state if the instruction is call instruction.
1887-
if (DI1->shouldUpdateCallSiteInfo())
1888-
MBB1.getParent()->eraseCallSiteInfo(&*DI1);
1886+
// info state if the instruction is call instruction.
1887+
if (DI1->shouldUpdateAdditionalCallInfo())
1888+
MBB1.getParent()->eraseAdditionalCallInfo(&*DI1);
18891889

18901890
// skip dbg_value instructions
18911891
if (!DI1->isDebugInstr())
@@ -2169,9 +2169,9 @@ void IfConverter::CopyAndPredicateBlock(BBInfo &ToBBI, BBInfo &FromBBI,
21692169
break;
21702170

21712171
MachineInstr *MI = MF.CloneMachineInstr(&I);
2172-
// Make a copy of the call site info.
2173-
if (I.isCandidateForCallSiteEntry())
2174-
MF.copyCallSiteInfo(&I, MI);
2172+
// Make a copy of the call info.
2173+
if (I.isCandidateForAdditionalCallInfo())
2174+
MF.copyAdditionalCallInfo(&I, MI);
21752175

21762176
ToBBI.BB->insert(ToBBI.BB->end(), MI);
21772177
ToBBI.NonPredSize++;

llvm/lib/CodeGen/InlineSpiller.cpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -997,9 +997,9 @@ foldMemoryOperand(ArrayRef<std::pair<MachineInstr *, unsigned>> Ops,
997997
HSpiller.rmFromMergeableSpills(*MI, FI))
998998
--NumSpills;
999999
LIS.ReplaceMachineInstrInMaps(*MI, *FoldMI);
1000-
// Update the call site info.
1001-
if (MI->isCandidateForCallSiteEntry())
1002-
MI->getMF()->moveCallSiteInfo(MI, FoldMI);
1000+
// Update the call info.
1001+
if (MI->isCandidateForAdditionalCallInfo())
1002+
MI->getMF()->moveAdditionalCallInfo(MI, FoldMI);
10031003

10041004
// If we've folded a store into an instruction labelled with debug-info,
10051005
// record a substitution from the old operand to the memory operand. Handle

llvm/lib/CodeGen/LiveRangeEdit.cpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -253,9 +253,9 @@ bool LiveRangeEdit::foldAsLoad(LiveInterval *LI,
253253
return false;
254254
LLVM_DEBUG(dbgs() << " folded: " << *FoldMI);
255255
LIS.ReplaceMachineInstrInMaps(*UseMI, *FoldMI);
256-
// Update the call site info.
257-
if (UseMI->shouldUpdateCallSiteInfo())
258-
UseMI->getMF()->moveCallSiteInfo(UseMI, FoldMI);
256+
// Update the call info.
257+
if (UseMI->shouldUpdateAdditionalCallInfo())
258+
UseMI->getMF()->moveAdditionalCallInfo(UseMI, FoldMI);
259259
UseMI->eraseFromParent();
260260
DefMI->addRegisterDead(LI->reg(), nullptr);
261261
Dead.push_back(DefMI);

llvm/lib/CodeGen/MIRPrinter.cpp

Lines changed: 8 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -605,17 +605,14 @@ void MIRPrinter::convertCalledGlobals(yaml::MachineFunction &YMF,
605605
const MachineFunction &MF,
606606
MachineModuleSlotTracker &MST) {
607607
for (const auto &[CallInst, CG] : MF.getCalledGlobals()) {
608-
// If the call instruction was dropped, then we don't need to print it.
609-
auto BB = CallInst->getParent();
610-
if (BB) {
611-
yaml::MachineInstrLoc CallSite;
612-
CallSite.BlockNum = CallInst->getParent()->getNumber();
613-
CallSite.Offset = std::distance(CallInst->getParent()->instr_begin(),
614-
CallInst->getIterator());
615-
616-
yaml::CalledGlobal YamlCG{CallSite, CG.first->getName().str(), CG.second};
617-
YMF.CalledGlobals.push_back(YamlCG);
618-
}
608+
yaml::MachineInstrLoc CallSite;
609+
CallSite.BlockNum = CallInst->getParent()->getNumber();
610+
CallSite.Offset = std::distance(CallInst->getParent()->instr_begin(),
611+
CallInst->getIterator());
612+
613+
yaml::CalledGlobal YamlCG{CallSite, CG.Callee->getName().str(),
614+
CG.TargetFlags};
615+
YMF.CalledGlobals.push_back(YamlCG);
619616
}
620617

621618
// Sort by position of call instructions.

0 commit comments

Comments
 (0)