Skip to content

Commit 2b1122e

Browse files
jmorsejmorse
authored
[DebugInfo][RemoveDIs] Use iterator-insertion in unittests and fuzzer (#102015)
These are the final few places in LLVM that use instruction pointers to insert instructions -- use iterators instead, which is needed for debug-info correctness in the future. Most of this is a gentle scattering of getIterator calls or not deref-then-addrofing iterators. libfuzzer does require a storage change to keep built instruction positions in a container though. The unit-test changes are very straightforwards. This leaves us in a position where libfuzzer can't fuzz on either of debug-info records, however I don't believe that fuzzing of debug-info is in scope for the library.
1 parent 29817a9 commit 2b1122e

File tree

12 files changed

+155
-134
lines changed

12 files changed

+155
-134
lines changed

‎llvm/include/llvm/FuzzMutate/OpDescriptor.h

+1-1
Original file line numberDiff line numberDiff line change
@@ -89,7 +89,7 @@ class SourcePred {
8989
struct OpDescriptor {
9090
unsigned Weight;
9191
SmallVector<SourcePred, 2> SourcePreds;
92-
std::function<Value *(ArrayRef<Value *>, Instruction *)> BuilderFunc;
92+
std::function<Value *(ArrayRef<Value *>, BasicBlock::iterator)> BuilderFunc;
9393
};
9494

9595
static inline SourcePred onlyType(Type *Only) {

‎llvm/lib/FuzzMutate/IRMutator.cpp

+5-5
Original file line numberDiff line numberDiff line change
@@ -148,7 +148,7 @@ void InjectorIRStrategy::mutate(BasicBlock &BB, RandomIRBuilder &IB) {
148148
for (const auto &Pred : ArrayRef(OpDesc->SourcePreds).slice(1))
149149
Srcs.push_back(IB.findOrCreateSource(BB, InstsBefore, Srcs, Pred));
150150

151-
if (Value *Op = OpDesc->BuilderFunc(Srcs, Insts[IP])) {
151+
if (Value *Op = OpDesc->BuilderFunc(Srcs, Insts[IP]->getIterator())) {
152152
// Find a sink and wire up the results of the operation.
153153
IB.connectToSink(BB, InstsAfter, Op);
154154
}
@@ -388,9 +388,9 @@ void InsertFunctionStrategy::mutate(BasicBlock &BB, RandomIRBuilder &IB) {
388388
}
389389
bool isRetVoid = (F->getReturnType() == Type::getVoidTy(M->getContext()));
390390
auto BuilderFunc = [FTy, F, isRetVoid](ArrayRef<Value *> Srcs,
391-
Instruction *Inst) {
391+
BasicBlock::iterator InsertPt) {
392392
StringRef Name = isRetVoid ? nullptr : "C";
393-
CallInst *Call = CallInst::Create(FTy, F, Srcs, Name, Inst);
393+
CallInst *Call = CallInst::Create(FTy, F, Srcs, Name, InsertPt);
394394
// Don't return this call inst if it return void as it can't be sinked.
395395
return isRetVoid ? nullptr : Call;
396396
};
@@ -414,7 +414,7 @@ void InsertFunctionStrategy::mutate(BasicBlock &BB, RandomIRBuilder &IB) {
414414
Srcs.push_back(IB.findOrCreateSource(BB, InstsBefore, Srcs, Pred));
415415
}
416416

417-
if (Value *Op = BuilderFunc(Srcs, Insts[IP])) {
417+
if (Value *Op = BuilderFunc(Srcs, Insts[IP]->getIterator())) {
418418
// Find a sink and wire up the results of the operation.
419419
IB.connectToSink(BB, InstsAfter, Op);
420420
}
@@ -543,7 +543,7 @@ void InsertPHIStrategy::mutate(BasicBlock &BB, RandomIRBuilder &IB) {
543543
if (&BB == &BB.getParent()->getEntryBlock())
544544
return;
545545
Type *Ty = IB.randomType();
546-
PHINode *PHI = PHINode::Create(Ty, llvm::pred_size(&BB), "", &BB.front());
546+
PHINode *PHI = PHINode::Create(Ty, llvm::pred_size(&BB), "", BB.begin());
547547

548548
// Use a map to make sure the same incoming basic block has the same value.
549549
DenseMap<BasicBlock *, Value *> IncomingValues;

‎llvm/lib/FuzzMutate/Operations.cpp

+30-24
Original file line numberDiff line numberDiff line change
@@ -98,25 +98,25 @@ void llvm::describeFuzzerVectorOps(std::vector<fuzzerop::OpDescriptor> &Ops) {
9898
}
9999

100100
OpDescriptor llvm::fuzzerop::selectDescriptor(unsigned Weight) {
101-
auto buildOp = [](ArrayRef<Value *> Srcs, Instruction *Inst) {
102-
return SelectInst::Create(Srcs[0], Srcs[1], Srcs[2], "S", Inst);
101+
auto buildOp = [](ArrayRef<Value *> Srcs, BasicBlock::iterator InsertPt) {
102+
return SelectInst::Create(Srcs[0], Srcs[1], Srcs[2], "S", InsertPt);
103103
};
104104
return {Weight,
105105
{boolOrVecBoolType(), matchFirstLengthWAnyType(), matchSecondType()},
106106
buildOp};
107107
}
108108

109109
OpDescriptor llvm::fuzzerop::fnegDescriptor(unsigned Weight) {
110-
auto buildOp = [](ArrayRef<Value *> Srcs, Instruction *Inst) {
111-
return UnaryOperator::Create(Instruction::FNeg, Srcs[0], "F", Inst);
110+
auto buildOp = [](ArrayRef<Value *> Srcs, BasicBlock::iterator InsertPt) {
111+
return UnaryOperator::Create(Instruction::FNeg, Srcs[0], "F", InsertPt);
112112
};
113113
return {Weight, {anyFloatOrVecFloatType()}, buildOp};
114114
}
115115

116116
OpDescriptor llvm::fuzzerop::binOpDescriptor(unsigned Weight,
117117
Instruction::BinaryOps Op) {
118-
auto buildOp = [Op](ArrayRef<Value *> Srcs, Instruction *Inst) {
119-
return BinaryOperator::Create(Op, Srcs[0], Srcs[1], "B", Inst);
118+
auto buildOp = [Op](ArrayRef<Value *> Srcs, BasicBlock::iterator InsertPt) {
119+
return BinaryOperator::Create(Op, Srcs[0], Srcs[1], "B", InsertPt);
120120
};
121121
switch (Op) {
122122
case Instruction::Add:
@@ -148,8 +148,9 @@ OpDescriptor llvm::fuzzerop::binOpDescriptor(unsigned Weight,
148148
OpDescriptor llvm::fuzzerop::cmpOpDescriptor(unsigned Weight,
149149
Instruction::OtherOps CmpOp,
150150
CmpInst::Predicate Pred) {
151-
auto buildOp = [CmpOp, Pred](ArrayRef<Value *> Srcs, Instruction *Inst) {
152-
return CmpInst::Create(CmpOp, Pred, Srcs[0], Srcs[1], "C", Inst);
151+
auto buildOp = [CmpOp, Pred](ArrayRef<Value *> Srcs,
152+
BasicBlock::iterator InsertPt) {
153+
return CmpInst::Create(CmpOp, Pred, Srcs[0], Srcs[1], "C", InsertPt);
153154
};
154155

155156
switch (CmpOp) {
@@ -163,9 +164,10 @@ OpDescriptor llvm::fuzzerop::cmpOpDescriptor(unsigned Weight,
163164
}
164165

165166
OpDescriptor llvm::fuzzerop::splitBlockDescriptor(unsigned Weight) {
166-
auto buildSplitBlock = [](ArrayRef<Value *> Srcs, Instruction *Inst) {
167-
BasicBlock *Block = Inst->getParent();
168-
BasicBlock *Next = Block->splitBasicBlock(Inst, "BB");
167+
auto buildSplitBlock = [](ArrayRef<Value *> Srcs,
168+
BasicBlock::iterator InsertPt) {
169+
BasicBlock *Block = InsertPt->getParent();
170+
BasicBlock *Next = Block->splitBasicBlock(InsertPt, "BB");
169171

170172
// If it was an exception handling block, we are done.
171173
if (Block->isEHPad())
@@ -174,7 +176,8 @@ OpDescriptor llvm::fuzzerop::splitBlockDescriptor(unsigned Weight) {
174176
// Loop back on this block by replacing the unconditional forward branch
175177
// with a conditional with a backedge.
176178
if (Block != &Block->getParent()->getEntryBlock()) {
177-
BranchInst::Create(Block, Next, Srcs[0], Block->getTerminator());
179+
BranchInst::Create(Block, Next, Srcs[0],
180+
Block->getTerminator()->getIterator());
178181
Block->getTerminator()->eraseFromParent();
179182

180183
// We need values for each phi in the block. Since there isn't a good way
@@ -193,12 +196,12 @@ OpDescriptor llvm::fuzzerop::splitBlockDescriptor(unsigned Weight) {
193196
}
194197

195198
OpDescriptor llvm::fuzzerop::gepDescriptor(unsigned Weight) {
196-
auto buildGEP = [](ArrayRef<Value *> Srcs, Instruction *Inst) {
199+
auto buildGEP = [](ArrayRef<Value *> Srcs, BasicBlock::iterator InsertPt) {
197200
// TODO: It would be better to generate a random type here, rather than
198201
// generating a random value and picking its type.
199202
Type *Ty = Srcs[1]->getType();
200203
auto Indices = ArrayRef(Srcs).drop_front(2);
201-
return GetElementPtrInst::Create(Ty, Srcs[0], Indices, "G", Inst);
204+
return GetElementPtrInst::Create(Ty, Srcs[0], Indices, "G", InsertPt);
202205
};
203206
// TODO: Handle aggregates and vectors
204207
// TODO: Support multiple indices.
@@ -239,10 +242,11 @@ static SourcePred validExtractValueIndex() {
239242
}
240243

241244
OpDescriptor llvm::fuzzerop::extractValueDescriptor(unsigned Weight) {
242-
auto buildExtract = [](ArrayRef<Value *> Srcs, Instruction *Inst) {
245+
auto buildExtract = [](ArrayRef<Value *> Srcs,
246+
BasicBlock::iterator InsertPt) {
243247
// TODO: It's pretty inefficient to shuffle this all through constants.
244248
unsigned Idx = cast<ConstantInt>(Srcs[1])->getZExtValue();
245-
return ExtractValueInst::Create(Srcs[0], {Idx}, "E", Inst);
249+
return ExtractValueInst::Create(Srcs[0], {Idx}, "E", InsertPt);
246250
};
247251
// TODO: Should we handle multiple indices?
248252
return {Weight, {anyAggregateType(), validExtractValueIndex()}, buildExtract};
@@ -298,10 +302,10 @@ static SourcePred validInsertValueIndex() {
298302
}
299303

300304
OpDescriptor llvm::fuzzerop::insertValueDescriptor(unsigned Weight) {
301-
auto buildInsert = [](ArrayRef<Value *> Srcs, Instruction *Inst) {
305+
auto buildInsert = [](ArrayRef<Value *> Srcs, BasicBlock::iterator InsertPt) {
302306
// TODO: It's pretty inefficient to shuffle this all through constants.
303307
unsigned Idx = cast<ConstantInt>(Srcs[2])->getZExtValue();
304-
return InsertValueInst::Create(Srcs[0], Srcs[1], {Idx}, "I", Inst);
308+
return InsertValueInst::Create(Srcs[0], Srcs[1], {Idx}, "I", InsertPt);
305309
};
306310
return {
307311
Weight,
@@ -310,16 +314,17 @@ OpDescriptor llvm::fuzzerop::insertValueDescriptor(unsigned Weight) {
310314
}
311315

312316
OpDescriptor llvm::fuzzerop::extractElementDescriptor(unsigned Weight) {
313-
auto buildExtract = [](ArrayRef<Value *> Srcs, Instruction *Inst) {
314-
return ExtractElementInst::Create(Srcs[0], Srcs[1], "E", Inst);
317+
auto buildExtract = [](ArrayRef<Value *> Srcs,
318+
BasicBlock::iterator InsertPt) {
319+
return ExtractElementInst::Create(Srcs[0], Srcs[1], "E", InsertPt);
315320
};
316321
// TODO: Try to avoid undefined accesses.
317322
return {Weight, {anyVectorType(), anyIntType()}, buildExtract};
318323
}
319324

320325
OpDescriptor llvm::fuzzerop::insertElementDescriptor(unsigned Weight) {
321-
auto buildInsert = [](ArrayRef<Value *> Srcs, Instruction *Inst) {
322-
return InsertElementInst::Create(Srcs[0], Srcs[1], Srcs[2], "I", Inst);
326+
auto buildInsert = [](ArrayRef<Value *> Srcs, BasicBlock::iterator InsertPt) {
327+
return InsertElementInst::Create(Srcs[0], Srcs[1], Srcs[2], "I", InsertPt);
323328
};
324329
// TODO: Try to avoid undefined accesses.
325330
return {Weight,
@@ -343,8 +348,9 @@ static SourcePred validShuffleVectorIndex() {
343348
}
344349

345350
OpDescriptor llvm::fuzzerop::shuffleVectorDescriptor(unsigned Weight) {
346-
auto buildShuffle = [](ArrayRef<Value *> Srcs, Instruction *Inst) {
347-
return new ShuffleVectorInst(Srcs[0], Srcs[1], Srcs[2], "S", Inst);
351+
auto buildShuffle = [](ArrayRef<Value *> Srcs,
352+
BasicBlock::iterator InsertPt) {
353+
return new ShuffleVectorInst(Srcs[0], Srcs[1], Srcs[2], "S", InsertPt);
348354
};
349355
return {Weight,
350356
{anyVectorType(), matchFirstType(), validShuffleVectorIndex()},

‎llvm/lib/FuzzMutate/RandomIRBuilder.cpp

+9-8
Original file line numberDiff line numberDiff line change
@@ -69,9 +69,9 @@ AllocaInst *RandomIRBuilder::createStackMemory(Function *F, Type *Ty,
6969
BasicBlock *EntryBB = &F->getEntryBlock();
7070
DataLayout DL(F->getParent());
7171
AllocaInst *Alloca = new AllocaInst(Ty, DL.getAllocaAddrSpace(), "A",
72-
&*EntryBB->getFirstInsertionPt());
72+
EntryBB->getFirstInsertionPt());
7373
if (Init)
74-
new StoreInst(Init, Alloca, Alloca->getNextNode());
74+
new StoreInst(Init, Alloca, std::next(Alloca->getIterator()));
7575
return Alloca;
7676
}
7777

@@ -165,7 +165,7 @@ Value *RandomIRBuilder::findOrCreateSource(BasicBlock &BB,
165165
Type *Ty = GV->getValueType();
166166
LoadInst *LoadGV = nullptr;
167167
if (BB.getTerminator()) {
168-
LoadGV = new LoadInst(Ty, GV, "LGV", &*BB.getFirstInsertionPt());
168+
LoadGV = new LoadInst(Ty, GV, "LGV", BB.getFirstInsertionPt());
169169
} else {
170170
LoadGV = new LoadInst(Ty, GV, "LGV", &BB);
171171
}
@@ -213,7 +213,7 @@ Value *RandomIRBuilder::newSource(BasicBlock &BB, ArrayRef<Instruction *> Insts,
213213
}
214214
// Pick the type independently.
215215
Type *AccessTy = RS.getSelection()->getType();
216-
auto *NewLoad = new LoadInst(AccessTy, Ptr, "L", &*IP);
216+
auto *NewLoad = new LoadInst(AccessTy, Ptr, "L", IP);
217217

218218
// Only sample this load if it really matches the descriptor
219219
if (Pred.matches(Srcs, NewLoad))
@@ -231,7 +231,8 @@ Value *RandomIRBuilder::newSource(BasicBlock &BB, ArrayRef<Instruction *> Insts,
231231
Function *F = BB.getParent();
232232
AllocaInst *Alloca = createStackMemory(F, Ty, newSrc);
233233
if (BB.getTerminator()) {
234-
newSrc = new LoadInst(Ty, Alloca, /*ArrLen,*/ "L", BB.getTerminator());
234+
newSrc = new LoadInst(Ty, Alloca, /*ArrLen,*/ "L",
235+
BB.getTerminator()->getIterator());
235236
} else {
236237
newSrc = new LoadInst(Ty, Alloca, /*ArrLen,*/ "L", &BB);
237238
}
@@ -325,7 +326,7 @@ Instruction *RandomIRBuilder::connectToSink(BasicBlock &BB,
325326
for (BasicBlock *Dom : Dominators) {
326327
for (Instruction &I : *Dom) {
327328
if (isa<PointerType>(I.getType()))
328-
return new StoreInst(V, &I, Insts.back());
329+
return new StoreInst(V, &I, Insts.back()->getIterator());
329330
}
330331
}
331332
break;
@@ -351,7 +352,7 @@ Instruction *RandomIRBuilder::connectToSink(BasicBlock &BB,
351352
Module *M = BB.getParent()->getParent();
352353
auto [GV, DidCreate] =
353354
findOrCreateGlobalVariable(M, {}, fuzzerop::onlyType(V->getType()));
354-
return new StoreInst(V, GV, Insts.back());
355+
return new StoreInst(V, GV, Insts.back()->getIterator());
355356
}
356357
case EndOfValueSink:
357358
default:
@@ -373,7 +374,7 @@ Instruction *RandomIRBuilder::newSink(BasicBlock &BB,
373374
}
374375
}
375376

376-
return new StoreInst(V, Ptr, Insts.back());
377+
return new StoreInst(V, Ptr, Insts.back()->getIterator());
377378
}
378379

379380
Value *RandomIRBuilder::findPointer(BasicBlock &BB,

‎llvm/tools/llvm-reduce/deltas/ReduceOperandBundles.cpp

+2-1
Original file line numberDiff line numberDiff line change
@@ -88,7 +88,8 @@ static void maybeRewriteCallWithDifferentBundles(
8888
});
8989

9090
// Finally actually replace the bundles on the call.
91-
CallBase *NewCall = CallBase::Create(OrigCall, NewBundles, OrigCall);
91+
CallBase *NewCall =
92+
CallBase::Create(OrigCall, NewBundles, OrigCall->getIterator());
9293
OrigCall->replaceAllUsesWith(NewCall);
9394
OrigCall->eraseFromParent();
9495
}

0 commit comments

Comments
 (0)