[IR Verifier] didn't check if switch case is constant, align IR Verifier's check with LLParser.

[Peter]

Summary

If a programmer incorrectly Switch->setOperand() and Verifier will pass, causing problems when dumping this Module
This patch aligns SwitchInst's check with LLParser. It also includes a test to justify the patch.

[arsenm]

llvm/lib/IR/Verifier.cpp

2882

Is this missing a check for a consistent number of operands, or is that hidden somewhere else?

[Peter]

llvm/lib/IR/Verifier.cpp

2882

I don't think there is a check for operand number consistency.

The Index * 2 + 2 is getCaseValue()'s default behavior. I believe SwitchInst manages operand number consistency when you addCase().

[Peter]

format

[arsenm]

llvm/lib/IR/Verifier.cpp

2882

But the verifier's job is to ensure like that is followed. Not sure there's another way to bypass that

[Peter]

llvm/lib/IR/Verifier.cpp

2882

I reviewed the SwitchInst definition again and couldn't find a bypass. Will merge tomorrow.

llvm/lib/IR/Verifier.cpp

 
 void Verifier::visitSwitchInst(SwitchInst &SI) {
   Check(SI.getType()->isVoidTy(), "Switch must have void result type!", &SI);
   // Check to make sure that all of the constants in the switch instruction
   // have the same type as the switched-on value.
   Type *SwitchTy = SI.getCondition()->getType();
   SmallPtrSet<ConstantInt*, 32> Constants;
   for (auto &Case : SI.cases()) {
+    Check(isa<ConstantInt>(SI.getOperand(Case.getCaseIndex() * 2 + 2)),
+          "Case value is not a constant integer.", &SI);
     Check(Case.getCaseValue()->getType() == SwitchTy,
           "Switch constants must all be same type as switch value!", &SI);
     Check(Constants.insert(Case.getCaseValue()).second,
           "Duplicate integer as switch case", &SI, Case.getCaseValue());
   }
 
   visitTerminator(SI);
 }

llvm/unittests/IR/VerifierTest.cpp

TEST(VerifierTest, AttributesWrongContext) {
   FunctionType *FTy2 =
       FunctionType::get(Type::getVoidTy(C2), /*isVarArg=*/false);
   Function *F2 = Function::Create(FTy2, Function::ExternalLinkage, "foo", M2);
   F2->copyAttributesFrom(F1);
 
   EXPECT_TRUE(verifyFunction(*F2));
 }
 
+TEST(VerifierTest, SwitchInst) {
+  LLVMContext C;
+  Module M("M", C);
+  IntegerType *Int32Ty = Type::getInt32Ty(C);
+  FunctionType *FTy = FunctionType::get(Type::getVoidTy(C), {Int32Ty, Int32Ty},
+                                        /*isVarArg=*/false);
+  Function *F = Function::Create(FTy, Function::ExternalLinkage, "foo", M);
+  BasicBlock *Entry = BasicBlock::Create(C, "entry", F);
+  BasicBlock *Default = BasicBlock::Create(C, "default", F);
+  BasicBlock *OnOne = BasicBlock::Create(C, "on_one", F);
+  BasicBlock *OnTwo = BasicBlock::Create(C, "on_two", F);
+
+  BasicBlock *Exit = BasicBlock::Create(C, "exit", F);
+
+  BranchInst::Create(Exit, Default);
+  BranchInst::Create(Exit, OnTwo);
+  BranchInst::Create(Exit, OnOne);
+  ReturnInst::Create(C, Exit);
+
+  Value *Cond = F->getArg(0);
+  SwitchInst *Switch = SwitchInst::Create(Cond, Default, 2, Entry);
+  Switch->addCase(ConstantInt::get(Int32Ty, 1), OnOne);
+  Switch->addCase(ConstantInt::get(Int32Ty, 2), OnTwo);
+
+  EXPECT_FALSE(verifyFunction(*F));
+  // set one case value to function argument.
+  Switch->setOperand(2, F->getArg(1));
+  EXPECT_TRUE(verifyFunction(*F));
+}
+
 } // end anonymous namespace
 } // end namespace llvm