[llvm-commits] [llvm] r96282 - in /llvm/trunk: lib/VMCore/Verifier.cpp unittests/VMCore/VerifierTest.cpp

[Chandler Carruth]

On Mon, Feb 15, 2010 at 4:42 PM, Nick Lewycky <nicholas at mxc.ca> wrote:
> Chris Lattner wrote:
>> Isn't a unit test for this completely overkill?
>
> Testing methodology-type people would tell you that the real problem is
> that the rest of the verifier is lacking tests ...
>
> ... but really, writing this test case is what led to the fixes in
> r96272, r96273 and r96275. Even if we didn't test this in particular, we
> should probably have a unit test that F->dump() and FPM.run(F) on
> functions that have no module owners still work.
>
> Ultimately I don't really care.

FWIW, I like it. The cost is so low that even just verifying that
someone hasn't typoed something egregiously seems to justify it. But
I'm one of those testing methodology types... ;]

>
> Nick
>
>>
>> -Chris
>>
>> On Feb 15, 2010, at 2:09 PM, Nick Lewycky wrote:
>>
>>> Author: nicholas
>>> Date: Mon Feb 15 16:09:09 2010
>>> New Revision: 96282
>>>
>>> URL: http://llvm.org/viewvc/llvm-project?rev=96282&view=rev
>>> Log:
>>> Teach the verifier to check the condition on a branch and ensure that it has
>>> 'i1' type.
>>>
>>> Added:
>>>     llvm/trunk/unittests/VMCore/VerifierTest.cpp
>>> Modified:
>>>     llvm/trunk/lib/VMCore/Verifier.cpp
>>>
>>> Modified: llvm/trunk/lib/VMCore/Verifier.cpp
>>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/VMCore/Verifier.cpp?rev=96282&r1=96281&r2=96282&view=diff
>>>
>>> ==============================================================================
>>> --- llvm/trunk/lib/VMCore/Verifier.cpp (original)
>>> +++ llvm/trunk/lib/VMCore/Verifier.cpp Mon Feb 15 16:09:09 2010
>>> @@ -317,6 +317,7 @@
>>>      void visitStoreInst(StoreInst&SI);
>>>      void visitInstruction(Instruction&I);
>>>      void visitTerminatorInst(TerminatorInst&I);
>>> +    void visitBranchInst(BranchInst&BI);
>>>      void visitReturnInst(ReturnInst&RI);
>>>      void visitSwitchInst(SwitchInst&SI);
>>>      void visitSelectInst(SelectInst&SI);
>>> @@ -749,6 +750,14 @@
>>>    visitInstruction(I);
>>> }
>>>
>>> +void Verifier::visitBranchInst(BranchInst&BI) {
>>> +  if (BI.isConditional()) {
>>> +    Assert2(BI.getCondition()->getType()->isIntegerTy(1),
>>> +            "Branch condition is not 'i1' type!",&BI, BI.getCondition());
>>> +  }
>>> +  visitTerminatorInst(BI);
>>> +}
>>> +
>>> void Verifier::visitReturnInst(ReturnInst&RI) {
>>>    Function *F = RI.getParent()->getParent();
>>>    unsigned N = RI.getNumOperands();
>>>
>>> Added: llvm/trunk/unittests/VMCore/VerifierTest.cpp
>>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/VMCore/VerifierTest.cpp?rev=96282&view=auto
>>>
>>> ==============================================================================
>>> --- llvm/trunk/unittests/VMCore/VerifierTest.cpp (added)
>>> +++ llvm/trunk/unittests/VMCore/VerifierTest.cpp Mon Feb 15 16:09:09 2010
>>> @@ -0,0 +1,44 @@
>>> +//===- llvm/unittest/VMCore/VerifierTest.cpp - Verifier unit tests --------===//
>>> +//
>>> +//                     The LLVM Compiler Infrastructure
>>> +//
>>> +// This file is distributed under the University of Illinois Open Source
>>> +// License. See LICENSE.TXT for details.
>>> +//
>>> +//===----------------------------------------------------------------------===//
>>> +
>>> +#include "llvm/Constants.h"
>>> +#include "llvm/DerivedTypes.h"
>>> +#include "llvm/Function.h"
>>> +#include "llvm/Instructions.h"
>>> +#include "llvm/LLVMContext.h"
>>> +#include "llvm/Analysis/Verifier.h"
>>> +#include "gtest/gtest.h"
>>> +
>>> +namespace llvm {
>>> +namespace {
>>> +
>>> +TEST(VerifierTest, Branch_i1) {
>>> +  LLVMContext&C = getGlobalContext();
>>> +  FunctionType *FTy = FunctionType::get(Type::getVoidTy(C), /*isVarArg=*/false);
>>> +  Function *F = Function::Create(FTy, GlobalValue::ExternalLinkage);
>>> +  BasicBlock *Entry = BasicBlock::Create(C, "entry", F);
>>> +  BasicBlock *Exit = BasicBlock::Create(C, "exit", F);
>>> +  ReturnInst::Create(C, Exit);
>>> +
>>> +  // To avoid triggering an assertion in BranchInst::Create, we first create
>>> +  // a branch with an 'i1' condition ...
>>> +
>>> +  Constant *False = ConstantInt::getFalse(C);
>>> +  BranchInst *BI = BranchInst::Create(Exit, Exit, False, Entry);
>>> +
>>> +  // ... then use setOperand to redirect it to a value of different type.
>>> +
>>> +  Constant *Zero32 = ConstantInt::get(IntegerType::get(C, 32), 0);
>>> +  BI->setOperand(0, Zero32);
>>> +
>>> +  EXPECT_TRUE(verifyFunction(*F, ReturnStatusAction));
>>> +}
>>> +
>>> +}
>>> +}
>>>
>>>
>>> _______________________________________________
>>> llvm-commits mailing list
>>> llvm-commits at cs.uiuc.edu
>>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>>
>>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>