[llvm] 50921a2 - [FuzzMutate] Fix a bug in `connectToSink` which might invalidate the whole module.

[Peter Rong via llvm-commits]

Author: Peter Rong
Date: 2022-11-29T13:07:26-08:00
New Revision: 50921a2174405fbac04a4c9aaeb6993153f05703

URL: https://github.com/llvm/llvm-project/commit/50921a2174405fbac04a4c9aaeb6993153f05703
DIFF: https://github.com/llvm/llvm-project/commit/50921a2174405fbac04a4c9aaeb6993153f05703.diff

LOG: [FuzzMutate] Fix a bug in `connectToSink` which might invalidate the whole module.

`connectToSink` uses a value by putting it in a future instruction.
It will replace the operand of a future instruction with the current value.

However, if current value is an `Instruction` and put into a switch case, the module is invalid.
We fix that by only connecting to Br/Switch's condition, and don't touch other operands.

Will have other strategies to mutate other Br/Switch operands to be patched once this patch is passed

Reviewed By: arsenm

Differential Revision: https://reviews.llvm.org/D138890

Added: 
    

Modified: 
    llvm/lib/FuzzMutate/RandomIRBuilder.cpp
    llvm/unittests/FuzzMutate/RandomIRBuilderTest.cpp

Removed: 
    


################################################################################
diff  --git a/llvm/lib/FuzzMutate/RandomIRBuilder.cpp b/llvm/lib/FuzzMutate/RandomIRBuilder.cpp
index 9ac31ebccb99f..0aa73340e1f8d 100644
--- a/llvm/lib/FuzzMutate/RandomIRBuilder.cpp
+++ b/llvm/lib/FuzzMutate/RandomIRBuilder.cpp
@@ -72,6 +72,7 @@ Value *RandomIRBuilder::newSource(BasicBlock &BB, ArrayRef<Instruction *> Insts,
 
 static bool isCompatibleReplacement(const Instruction *I, const Use &Operand,
                                     const Value *Replacement) {
+  unsigned int OperandNo = Operand.getOperandNo();
   if (Operand->getType() != Replacement->getType())
     return false;
   switch (I->getOpcode()) {
@@ -80,13 +81,21 @@ static bool isCompatibleReplacement(const Instruction *I, const Use &Operand,
   case Instruction::ExtractValue:
     // TODO: We could potentially validate these, but for now just leave indices
     // alone.
-    if (Operand.getOperandNo() >= 1)
+    if (OperandNo >= 1)
       return false;
     break;
   case Instruction::InsertValue:
   case Instruction::InsertElement:
   case Instruction::ShuffleVector:
-    if (Operand.getOperandNo() >= 2)
+    if (OperandNo >= 2)
+      return false;
+    break;
+  // For Br/Switch, we only try to modify the 1st Operand (condition).
+  // Modify other operands, like switch case may accidently change case from
+  // ConstantInt to a register, which is illegal.
+  case Instruction::Switch:
+  case Instruction::Br:
+    if (OperandNo >= 1)
       return false;
     break;
   default:

diff  --git a/llvm/unittests/FuzzMutate/RandomIRBuilderTest.cpp b/llvm/unittests/FuzzMutate/RandomIRBuilderTest.cpp
index 95f01abf6576c..95d4a383a0012 100644
--- a/llvm/unittests/FuzzMutate/RandomIRBuilderTest.cpp
+++ b/llvm/unittests/FuzzMutate/RandomIRBuilderTest.cpp
@@ -13,6 +13,7 @@
 #include "llvm/FuzzMutate/IRMutator.h"
 #include "llvm/FuzzMutate/OpDescriptor.h"
 #include "llvm/FuzzMutate/Operations.h"
+#include "llvm/FuzzMutate/Random.h"
 #include "llvm/IR/Constants.h"
 #include "llvm/IR/Instructions.h"
 #include "llvm/IR/LLVMContext.h"
@@ -292,4 +293,49 @@ TEST(RandomIRBuilderTest, SwiftError) {
   }
 }
 
+TEST(RandomIRBuilderTest, dontConnectToSwitch) {
+  // Check that we never put anything into switch's case branch
+  // If we accidently put a variable, the module is invalid.
+  LLVMContext Ctx;
+  const char *SourceCode = "\n\
+    define void @test(i1 %C1, i1 %C2, i32 %I, i32 %J) { \n\
+    Entry:  \n\
+      %I.1 = add i32 %I, 42 \n\
+      %J.1 = add i32 %J, 42 \n\
+      %IJ = add i32 %I, %J \n\
+      switch i32 %I, label %Default [ \n\
+        i32 1, label %OnOne  \n\
+      ] \n\
+    Default:  \n\
+      %CIEqJ = icmp eq i32 %I.1, %J.1 \n\
+      %CISltJ = icmp slt i32 %I.1, %J.1 \n\
+      %CAnd = and i1 %C1, %C2 \n\
+      br i1 %CIEqJ, label %Default, label %Exit \n\
+    OnOne:  \n\
+      br i1 %C1, label %OnOne, label %Exit \n\
+    Exit:  \n\
+      ret void \n\
+    }";
+
+  std::vector<Type *> Types = {Type::getInt32Ty(Ctx), Type::getInt1Ty(Ctx)};
+  RandomIRBuilder IB(Seed, Types);
+  for (int i = 0; i < 20; i++) {
+    std::unique_ptr<Module> M = parseAssembly(SourceCode, Ctx);
+    Function &F = *M->getFunction("test");
+    auto RS = makeSampler(IB.Rand, make_pointer_range(F));
+    BasicBlock *BB = RS.getSelection();
+    SmallVector<Instruction *, 32> Insts;
+    for (auto I = BB->getFirstInsertionPt(), E = BB->end(); I != E; ++I)
+      Insts.push_back(&*I);
+    if (Insts.size() < 2)
+      continue;
+    // Choose an instruction and connect to later operations.
+    size_t IP = uniform<size_t>(IB.Rand, 1, Insts.size() - 1);
+    Instruction *Inst = Insts[IP - 1];
+    auto ConnectAfter = makeArrayRef(Insts).slice(IP);
+    IB.connectToSink(*BB, ConnectAfter, Inst);
+    ASSERT_FALSE(verifyModule(*M, &errs()));
+  }
+}
+
 } // namespace