[FuzzMutate] Correctly insert sinks and sources around invoke instructions

[igor-laevsky]

Summary

Invoke instructions are terminators which produce results. They can match operand descriptor in which case we are going to insert load or store operation right after invoke . In this change I decided to exclude invokes from findPointer candidates altogether in order to avoid this kind of problems.

[bogner]

lib/FuzzMutate/RandomIRBuilder.cpp

156–157 ↗

(On Diff #125533)

Would isa<TerminatorInst> or whatever work here? I guess this is the only case, but it seems fine to be more general.

[igor-laevsky]

Hi Justin, I updated the diff to bail on all TerminatorInsts. I agree that it's better to be more general here.

llvm/trunk/lib/FuzzMutate/RandomIRBuilder.cpp

Value *RandomIRBuilder::newSource(BasicBlock &BB, ArrayRef<Instruction *> Insts,
   auto RS = makeSampler<Value *>(Rand);
   RS.sample(Pred.generate(Srcs, KnownTypes));
 
   // If we can find a pointer to load from, use it half the time.
   Value *Ptr = findPointer(BB, Insts, Srcs, Pred);
   if (Ptr) {
     // Create load from the chosen pointer
     auto IP = BB.getFirstInsertionPt();
-    if (auto *I = dyn_cast<Instruction>(Ptr))
+    if (auto *I = dyn_cast<Instruction>(Ptr)) {
       IP = ++I->getIterator();
+      assert(IP != BB.end() && "guaranteed by the findPointer");
+    }
     auto *NewLoad = new LoadInst(Ptr, "L", &*IP);
 
     // Only sample this load if it really matches the descriptor
     if (Pred.matches(Srcs, NewLoad))
       RS.sample(NewLoad, RS.totalWeight());
     else
       NewLoad->eraseFromParent();
   }
void RandomIRBuilder::newSink(BasicBlock &BB, ArrayRef<Instruction *> Insts,
 
   new StoreInst(V, Ptr, Insts.back());
 }
 
 Value *RandomIRBuilder::findPointer(BasicBlock &BB,
                                     ArrayRef<Instruction *> Insts,
                                     ArrayRef<Value *> Srcs, SourcePred Pred) {
   auto IsMatchingPtr = [&Srcs, &Pred](Instruction *Inst) {
+    // Invoke instructions sometimes produce valid pointers but currently
+    // we can't insert loads or stores from them
+    if (isa<TerminatorInst>(Inst))
+      return false;
+
     if (auto PtrTy = dyn_cast<PointerType>(Inst->getType()))
       // TODO: Check if this is horribly expensive.
       return Pred.matches(Srcs, UndefValue::get(PtrTy->getElementType()));
     return false;
   };
   if (auto RS = makeSampler(Rand, make_filter_range(Insts, IsMatchingPtr)))
     return RS.getSelection();
   return nullptr;
 }

llvm/trunk/unittests/FuzzMutate/RandomIRBuilderTest.cpp

TEST(RandomIRBuilderTest, InsertValueArray) {
   // Check that we can always pick the last two operands.
   for (int i = 0; i < 10; ++i) {
     Srcs[0] = Source;
     Srcs[1] = IB.findOrCreateSource(BB, {Source}, Srcs, Descr.SourcePreds[1]);
     IB.findOrCreateSource(BB, {}, Srcs, Descr.SourcePreds[2]);
   }
 }
 
+TEST(RandomIRBuilderTest, Invokes) {
+  // Check that we never generate load or store after invoke instruction
+
+  LLVMContext Ctx;
+  const char *SourceCode =
+      "declare i32* @f()"
+      "declare i32 @personality_function()"
+      "define i32* @test() personality i32 ()* @personality_function {\n"
+      "entry:\n"
+      "  %val = invoke i32* @f()\n"
+      "          to label %normal unwind label %exceptional\n"
+      "normal:\n"
+      "  ret i32* %val\n"
+      "exceptional:\n"
+      "  %landing_pad4 = landingpad token cleanup\n"
+      "  ret i32* undef\n"
+      "}";
+  auto M = parseAssembly(SourceCode, Ctx);
+
+
+  std::vector<Type *> Types = {Type::getInt8Ty(Ctx)};
+  RandomIRBuilder IB(Seed, Types);
+
+  // Get first basic block of the test function
+  Function &F = *M->getFunction("test");
+  BasicBlock &BB = *F.begin();
+
+  Instruction *Invoke = &*BB.begin();
+
+  // Find source but never insert new load after invoke
+  for (int i = 0; i < 10; ++i) {
+    (void)IB.findOrCreateSource(BB, {Invoke}, {}, fuzzerop::anyIntType());
+    ASSERT_TRUE(!verifyModule(*M, &errs()));
+  }
+}
+
 }