fix PR5009 by making CGSCCPM realize that a call was devirtualized
[oota-llvm.git] / lib / Analysis / Lint.cpp
index 19945179f4d363464464798e0fa252e72e979d9f..25d4f9571dab37a2dff46ab5a552708a3005fa38 100644 (file)
@@ -20,7 +20,7 @@
 // Another limitation is that it assumes all code will be executed. A store
 // through a null pointer in a basic block which is never reached is harmless,
 // but this pass will warn about it anyway.
-
+//
 // Optimization passes may make conditions that this pass checks for more or
 // less obvious. If an optimization pass appears to be introducing a warning,
 // it may be that the optimization pass is merely exposing an existing
 #include "llvm/Support/Debug.h"
 #include "llvm/Support/InstVisitor.h"
 #include "llvm/Support/raw_ostream.h"
+#include "llvm/ADT/STLExtras.h"
 using namespace llvm;
 
 namespace {
+  namespace MemRef {
+    static unsigned Read     = 1;
+    static unsigned Write    = 2;
+    static unsigned Callee   = 4;
+    static unsigned Branchee = 8;
+  }
+
   class Lint : public FunctionPass, public InstVisitor<Lint> {
     friend class InstVisitor<Lint>;
 
+    void visitFunction(Function &F);
+
     void visitCallSite(CallSite CS);
     void visitMemoryReference(Instruction &I, Value *Ptr, unsigned Align,
-                              const Type *Ty);
+                              const Type *Ty, unsigned Flags);
 
-    void visitInstruction(Instruction &I);
     void visitCallInst(CallInst &I);
     void visitInvokeInst(InvokeInst &I);
     void visitReturnInst(ReturnInst &I);
     void visitLoadInst(LoadInst &I);
     void visitStoreInst(StoreInst &I);
+    void visitXor(BinaryOperator &I);
+    void visitSub(BinaryOperator &I);
+    void visitLShr(BinaryOperator &I);
+    void visitAShr(BinaryOperator &I);
+    void visitShl(BinaryOperator &I);
     void visitSDiv(BinaryOperator &I);
     void visitUDiv(BinaryOperator &I);
     void visitSRem(BinaryOperator &I);
@@ -70,6 +84,9 @@ namespace {
     void visitAllocaInst(AllocaInst &I);
     void visitVAArgInst(VAArgInst &I);
     void visitIndirectBrInst(IndirectBrInst &I);
+    void visitExtractElementInst(ExtractElementInst &I);
+    void visitInsertElementInst(InsertElementInst &I);
+    void visitUnreachableInst(UnreachableInst &I);
 
   public:
     Module *Mod;
@@ -165,7 +182,11 @@ bool Lint::runOnFunction(Function &F) {
   return false;
 }
 
-void Lint::visitInstruction(Instruction &I) {
+void Lint::visitFunction(Function &F) {
+  // This isn't undefined behavior, it's just a little unusual, and it's a
+  // fairly common mistake to neglect to name a function.
+  Assert1(F.hasName() || F.hasLocalLinkage(),
+          "Unusual: Unnamed function with non-local linkage", &F);
 }
 
 void Lint::visitCallSite(CallSite CS) {
@@ -173,11 +194,12 @@ void Lint::visitCallSite(CallSite CS) {
   Value *Callee = CS.getCalledValue();
 
   // TODO: Check function alignment?
-  visitMemoryReference(I, Callee, 0, 0);
+  visitMemoryReference(I, Callee, 0, 0, MemRef::Callee);
 
   if (Function *F = dyn_cast<Function>(Callee->stripPointerCasts())) {
     Assert1(CS.getCallingConv() == F->getCallingConv(),
-            "Caller and callee calling convention differ", &I);
+            "Undefined behavior: Caller and callee calling convention differ",
+            &I);
 
     const FunctionType *FT = F->getFunctionType();
     unsigned NumActualArgs = unsigned(CS.arg_end()-CS.arg_begin());
@@ -185,7 +207,8 @@ void Lint::visitCallSite(CallSite CS) {
     Assert1(FT->isVarArg() ?
               FT->getNumParams() <= NumActualArgs :
               FT->getNumParams() == NumActualArgs,
-            "Call argument count mismatches callee argument count", &I);
+            "Undefined behavior: Call argument count mismatches callee "
+            "argument count", &I);
       
     // TODO: Check argument types (in case the callee was casted)
 
@@ -206,9 +229,14 @@ void Lint::visitCallSite(CallSite CS) {
 
     case Intrinsic::memcpy: {
       MemCpyInst *MCI = cast<MemCpyInst>(&I);
-      visitMemoryReference(I, MCI->getSource(), MCI->getAlignment(), 0);
-      visitMemoryReference(I, MCI->getDest(), MCI->getAlignment(), 0);
-
+      visitMemoryReference(I, MCI->getSource(), MCI->getAlignment(), 0,
+                           MemRef::Write);
+      visitMemoryReference(I, MCI->getDest(), MCI->getAlignment(), 0,
+                           MemRef::Read);
+
+      // Check that the memcpy arguments don't overlap. The AliasAnalysis API
+      // isn't expressive enough for what we really want to do. Known partial
+      // overlap is not distinguished from the case where nothing is known.
       unsigned Size = 0;
       if (const ConstantInt *Len =
             dyn_cast<ConstantInt>(MCI->getLength()->stripPointerCasts()))
@@ -216,34 +244,44 @@ void Lint::visitCallSite(CallSite CS) {
           Size = Len->getValue().getZExtValue();
       Assert1(AA->alias(MCI->getSource(), Size, MCI->getDest(), Size) !=
               AliasAnalysis::MustAlias,
-              "memcpy source and destination overlap", &I);
+              "Undefined behavior: memcpy source and destination overlap", &I);
       break;
     }
     case Intrinsic::memmove: {
       MemMoveInst *MMI = cast<MemMoveInst>(&I);
-      visitMemoryReference(I, MMI->getSource(), MMI->getAlignment(), 0);
-      visitMemoryReference(I, MMI->getDest(), MMI->getAlignment(), 0);
+      visitMemoryReference(I, MMI->getSource(), MMI->getAlignment(), 0,
+                           MemRef::Write);
+      visitMemoryReference(I, MMI->getDest(), MMI->getAlignment(), 0,
+                           MemRef::Read);
       break;
     }
     case Intrinsic::memset: {
       MemSetInst *MSI = cast<MemSetInst>(&I);
-      visitMemoryReference(I, MSI->getDest(), MSI->getAlignment(), 0);
+      visitMemoryReference(I, MSI->getDest(), MSI->getAlignment(), 0,
+                           MemRef::Write);
       break;
     }
 
     case Intrinsic::vastart:
-      visitMemoryReference(I, CS.getArgument(0), 0, 0);
+      Assert1(I.getParent()->getParent()->isVarArg(),
+              "Undefined behavior: va_start called in a non-varargs function",
+              &I);
+
+      visitMemoryReference(I, CS.getArgument(0), 0, 0,
+                           MemRef::Read | MemRef::Write);
       break;
     case Intrinsic::vacopy:
-      visitMemoryReference(I, CS.getArgument(0), 0, 0);
-      visitMemoryReference(I, CS.getArgument(1), 0, 0);
+      visitMemoryReference(I, CS.getArgument(0), 0, 0, MemRef::Write);
+      visitMemoryReference(I, CS.getArgument(1), 0, 0, MemRef::Read);
       break;
     case Intrinsic::vaend:
-      visitMemoryReference(I, CS.getArgument(0), 0, 0);
+      visitMemoryReference(I, CS.getArgument(0), 0, 0,
+                           MemRef::Read | MemRef::Write);
       break;
 
     case Intrinsic::stackrestore:
-      visitMemoryReference(I, CS.getArgument(0), 0, 0);
+      visitMemoryReference(I, CS.getArgument(0), 0, 0,
+                           MemRef::Read);
       break;
     }
 }
@@ -259,18 +297,43 @@ void Lint::visitInvokeInst(InvokeInst &I) {
 void Lint::visitReturnInst(ReturnInst &I) {
   Function *F = I.getParent()->getParent();
   Assert1(!F->doesNotReturn(),
-          "Return statement in function with noreturn attribute", &I);
+          "Unusual: Return statement in function with noreturn attribute",
+          &I);
 }
 
 // TODO: Add a length argument and check that the reference is in bounds
-// TODO: Add read/write/execute flags and check for writing to read-only
-//       memory or jumping to suspicious writeable memory
 void Lint::visitMemoryReference(Instruction &I,
-                                Value *Ptr, unsigned Align, const Type *Ty) {
-  Assert1(!isa<ConstantPointerNull>(Ptr->getUnderlyingObject()),
-          "Null pointer dereference", &I);
-  Assert1(!isa<UndefValue>(Ptr->getUnderlyingObject()),
-          "Undef pointer dereference", &I);
+                                Value *Ptr, unsigned Align, const Type *Ty,
+                                unsigned Flags) {
+  Value *UnderlyingObject = Ptr->getUnderlyingObject();
+  Assert1(!isa<ConstantPointerNull>(UnderlyingObject),
+          "Undefined behavior: Null pointer dereference", &I);
+  Assert1(!isa<UndefValue>(UnderlyingObject),
+          "Undefined behavior: Undef pointer dereference", &I);
+
+  if (Flags & MemRef::Write) {
+    if (const GlobalVariable *GV = dyn_cast<GlobalVariable>(UnderlyingObject))
+      Assert1(!GV->isConstant(),
+              "Undefined behavior: Write to read-only memory", &I);
+    Assert1(!isa<Function>(UnderlyingObject) &&
+            !isa<BlockAddress>(UnderlyingObject),
+            "Undefined behavior: Write to text section", &I);
+  }
+  if (Flags & MemRef::Read) {
+    Assert1(!isa<Function>(UnderlyingObject),
+            "Unusual: Load from function body", &I);
+    Assert1(!isa<BlockAddress>(UnderlyingObject),
+            "Undefined behavior: Load from block address", &I);
+  }
+  if (Flags & MemRef::Callee) {
+    Assert1(!isa<BlockAddress>(UnderlyingObject),
+            "Undefined behavior: Call to block address", &I);
+  }
+  if (Flags & MemRef::Branchee) {
+    Assert1(!isa<Constant>(UnderlyingObject) ||
+            isa<BlockAddress>(UnderlyingObject),
+            "Undefined behavior: Branch to non-blockaddress", &I);
+  }
 
   if (TD) {
     if (Align == 0 && Ty) Align = TD->getABITypeAlignment(Ty);
@@ -281,21 +344,58 @@ void Lint::visitMemoryReference(Instruction &I,
                    KnownZero(BitWidth, 0), KnownOne(BitWidth, 0);
       ComputeMaskedBits(Ptr, Mask, KnownZero, KnownOne, TD);
       Assert1(!(KnownOne & APInt::getLowBitsSet(BitWidth, Log2_32(Align))),
-              "Memory reference address is misaligned", &I);
+              "Undefined behavior: Memory reference address is misaligned", &I);
     }
   }
 }
 
 void Lint::visitLoadInst(LoadInst &I) {
-  visitMemoryReference(I, I.getPointerOperand(), I.getAlignment(), I.getType());
+  visitMemoryReference(I, I.getPointerOperand(), I.getAlignment(), I.getType(),
+                       MemRef::Read);
 }
 
 void Lint::visitStoreInst(StoreInst &I) {
   visitMemoryReference(I, I.getPointerOperand(), I.getAlignment(),
-                  I.getOperand(0)->getType());
+                  I.getOperand(0)->getType(), MemRef::Write);
+}
+
+void Lint::visitXor(BinaryOperator &I) {
+  Assert1(!isa<UndefValue>(I.getOperand(0)) ||
+          !isa<UndefValue>(I.getOperand(1)),
+          "Undefined result: xor(undef, undef)", &I);
+}
+
+void Lint::visitSub(BinaryOperator &I) {
+  Assert1(!isa<UndefValue>(I.getOperand(0)) ||
+          !isa<UndefValue>(I.getOperand(1)),
+          "Undefined result: sub(undef, undef)", &I);
+}
+
+void Lint::visitLShr(BinaryOperator &I) {
+  if (ConstantInt *CI =
+        dyn_cast<ConstantInt>(I.getOperand(1)->stripPointerCasts()))
+    Assert1(CI->getValue().ult(cast<IntegerType>(I.getType())->getBitWidth()),
+            "Undefined result: Shift count out of range", &I);
+}
+
+void Lint::visitAShr(BinaryOperator &I) {
+  if (ConstantInt *CI =
+        dyn_cast<ConstantInt>(I.getOperand(1)->stripPointerCasts()))
+    Assert1(CI->getValue().ult(cast<IntegerType>(I.getType())->getBitWidth()),
+            "Undefined result: Shift count out of range", &I);
+}
+
+void Lint::visitShl(BinaryOperator &I) {
+  if (ConstantInt *CI =
+        dyn_cast<ConstantInt>(I.getOperand(1)->stripPointerCasts()))
+    Assert1(CI->getValue().ult(cast<IntegerType>(I.getType())->getBitWidth()),
+            "Undefined result: Shift count out of range", &I);
 }
 
 static bool isZero(Value *V, TargetData *TD) {
+  // Assume undef could be zero.
+  if (isa<UndefValue>(V)) return true;
+
   unsigned BitWidth = cast<IntegerType>(V->getType())->getBitWidth();
   APInt Mask = APInt::getAllOnesValue(BitWidth),
                KnownZero(BitWidth, 0), KnownOne(BitWidth, 0);
@@ -304,34 +404,61 @@ static bool isZero(Value *V, TargetData *TD) {
 }
 
 void Lint::visitSDiv(BinaryOperator &I) {
-  Assert1(!isZero(I.getOperand(1), TD), "Division by zero", &I);
+  Assert1(!isZero(I.getOperand(1), TD),
+          "Undefined behavior: Division by zero", &I);
 }
 
 void Lint::visitUDiv(BinaryOperator &I) {
-  Assert1(!isZero(I.getOperand(1), TD), "Division by zero", &I);
+  Assert1(!isZero(I.getOperand(1), TD),
+          "Undefined behavior: Division by zero", &I);
 }
 
 void Lint::visitSRem(BinaryOperator &I) {
-  Assert1(!isZero(I.getOperand(1), TD), "Division by zero", &I);
+  Assert1(!isZero(I.getOperand(1), TD),
+          "Undefined behavior: Division by zero", &I);
 }
 
 void Lint::visitURem(BinaryOperator &I) {
-  Assert1(!isZero(I.getOperand(1), TD), "Division by zero", &I);
+  Assert1(!isZero(I.getOperand(1), TD),
+          "Undefined behavior: Division by zero", &I);
 }
 
 void Lint::visitAllocaInst(AllocaInst &I) {
   if (isa<ConstantInt>(I.getArraySize()))
     // This isn't undefined behavior, it's just an obvious pessimization.
     Assert1(&I.getParent()->getParent()->getEntryBlock() == I.getParent(),
-            "Static alloca outside of entry block", &I);
+            "Pessimization: Static alloca outside of entry block", &I);
 }
 
 void Lint::visitVAArgInst(VAArgInst &I) {
-  visitMemoryReference(I, I.getOperand(0), 0, 0);
+  visitMemoryReference(I, I.getOperand(0), 0, 0,
+                       MemRef::Read | MemRef::Write);
 }
 
 void Lint::visitIndirectBrInst(IndirectBrInst &I) {
-  visitMemoryReference(I, I.getAddress(), 0, 0);
+  visitMemoryReference(I, I.getAddress(), 0, 0, MemRef::Branchee);
+}
+
+void Lint::visitExtractElementInst(ExtractElementInst &I) {
+  if (ConstantInt *CI =
+        dyn_cast<ConstantInt>(I.getIndexOperand()->stripPointerCasts()))
+    Assert1(CI->getValue().ult(I.getVectorOperandType()->getNumElements()),
+            "Undefined result: extractelement index out of range", &I);
+}
+
+void Lint::visitInsertElementInst(InsertElementInst &I) {
+  if (ConstantInt *CI =
+        dyn_cast<ConstantInt>(I.getOperand(2)->stripPointerCasts()))
+    Assert1(CI->getValue().ult(I.getType()->getNumElements()),
+            "Undefined result: insertelement index out of range", &I);
+}
+
+void Lint::visitUnreachableInst(UnreachableInst &I) {
+  // This isn't undefined behavior, it's merely suspicious.
+  Assert1(&I == I.getParent()->begin() ||
+          prior(BasicBlock::iterator(&I))->mayHaveSideEffects(),
+          "Unusual: unreachable immediately preceded by instruction without "
+          "side effects", &I);
 }
 
 //===----------------------------------------------------------------------===//