diff options
Diffstat (limited to 'lib/Checker')
-rw-r--r-- | lib/Checker/BasicStore.cpp | 5 | ||||
-rw-r--r-- | lib/Checker/BugReporter.cpp | 19 | ||||
-rw-r--r-- | lib/Checker/BugReporterVisitors.cpp | 67 | ||||
-rw-r--r-- | lib/Checker/CallAndMessageChecker.cpp | 195 | ||||
-rw-r--r-- | lib/Checker/CheckDeadStores.cpp | 25 | ||||
-rw-r--r-- | lib/Checker/FlatStore.cpp | 4 | ||||
-rw-r--r-- | lib/Checker/GRExprEngine.cpp | 4 | ||||
-rw-r--r-- | lib/Checker/GRState.cpp | 3 | ||||
-rw-r--r-- | lib/Checker/RegionStore.cpp | 22 |
9 files changed, 275 insertions, 69 deletions
diff --git a/lib/Checker/BasicStore.cpp b/lib/Checker/BasicStore.cpp index 10136f3..7c53991 100644 --- a/lib/Checker/BasicStore.cpp +++ b/lib/Checker/BasicStore.cpp @@ -72,7 +72,9 @@ public: /// RemoveDeadBindings - Scans a BasicStore of 'state' for dead values. /// It updatees the GRState object in place with the values removed. - Store RemoveDeadBindings(Store store, Stmt* Loc, SymbolReaper& SymReaper, + Store RemoveDeadBindings(Store store, Stmt* Loc, + const StackFrameContext *LCtx, + SymbolReaper& SymReaper, llvm::SmallVectorImpl<const MemRegion*>& RegionRoots); void iterBindings(Store store, BindingsHandler& f); @@ -250,6 +252,7 @@ Store BasicStoreManager::Remove(Store store, Loc loc) { } Store BasicStoreManager::RemoveDeadBindings(Store store, Stmt* Loc, + const StackFrameContext *LCtx, SymbolReaper& SymReaper, llvm::SmallVectorImpl<const MemRegion*>& RegionRoots) { diff --git a/lib/Checker/BugReporter.cpp b/lib/Checker/BugReporter.cpp index 0cf593b..7272b34 100644 --- a/lib/Checker/BugReporter.cpp +++ b/lib/Checker/BugReporter.cpp @@ -36,6 +36,23 @@ BugReporterContext::~BugReporterContext() { if ((*I)->isOwnedByReporterContext()) delete *I; } +void BugReporterContext::addVisitor(BugReporterVisitor* visitor) { + if (!visitor) + return; + + llvm::FoldingSetNodeID ID; + visitor->Profile(ID); + void *InsertPos; + + if (CallbacksSet.FindNodeOrInsertPos(ID, InsertPos)) { + delete visitor; + return; + } + + CallbacksSet.InsertNode(visitor, InsertPos); + Callbacks = F.Add(visitor, Callbacks); +} + //===----------------------------------------------------------------------===// // Helper routines for walking the ExplodedGraph and fetching statements. //===----------------------------------------------------------------------===// @@ -1613,7 +1630,9 @@ void GRBugReporter::GeneratePathDiagnostic(PathDiagnostic& PD, else return; + // Register node visitors. R->registerInitialVisitors(PDB, N); + bugreporter::registerNilReceiverVisitor(PDB); switch (PDB.getGenerationScheme()) { case PathDiagnosticClient::Extensive: diff --git a/lib/Checker/BugReporterVisitors.cpp b/lib/Checker/BugReporterVisitors.cpp index 6cf41b1..1d6994b 100644 --- a/lib/Checker/BugReporterVisitors.cpp +++ b/lib/Checker/BugReporterVisitors.cpp @@ -92,6 +92,13 @@ public: FindLastStoreBRVisitor(SVal v, const MemRegion *r) : R(r), V(v), satisfied(false), StoreSite(0) {} + virtual void Profile(llvm::FoldingSetNodeID &ID) const { + static int tag = 0; + ID.AddPointer(&tag); + ID.AddPointer(R); + ID.Add(V); + } + PathDiagnosticPiece* VisitNode(const ExplodedNode *N, const ExplodedNode *PrevN, BugReporterContext& BRC) { @@ -129,8 +136,8 @@ public: return NULL; satisfied = true; - std::string sbuf; - llvm::raw_string_ostream os(sbuf); + llvm::SmallString<256> sbuf; + llvm::raw_svector_ostream os(sbuf); if (const PostStmt *PS = N->getLocationAs<PostStmt>()) { if (const DeclStmt *DS = PS->getStmtAs<DeclStmt>()) { @@ -239,6 +246,13 @@ public: TrackConstraintBRVisitor(DefinedSVal constraint, bool assumption) : Constraint(constraint), Assumption(assumption), isSatisfied(false) {} + void Profile(llvm::FoldingSetNodeID &ID) const { + static int tag = 0; + ID.AddPointer(&tag); + ID.AddBoolean(Assumption); + ID.Add(Constraint); + } + PathDiagnosticPiece* VisitNode(const ExplodedNode *N, const ExplodedNode *PrevN, BugReporterContext& BRC) { @@ -365,3 +379,52 @@ void clang::bugreporter::registerFindLastStore(BugReporterContext& BRC, BRC.addVisitor(new FindLastStoreBRVisitor(V, R)); } + + +namespace { +class NilReceiverVisitor : public BugReporterVisitor { +public: + NilReceiverVisitor() {} + + void Profile(llvm::FoldingSetNodeID &ID) const { + static int x = 0; + ID.AddPointer(&x); + } + + PathDiagnosticPiece* VisitNode(const ExplodedNode *N, + const ExplodedNode *PrevN, + BugReporterContext& BRC) { + + const PostStmt *P = N->getLocationAs<PostStmt>(); + if (!P) + return 0; + const ObjCMessageExpr *ME = P->getStmtAs<ObjCMessageExpr>(); + if (!ME) + return 0; + const Expr *Receiver = ME->getReceiver(); + if (!Receiver) + return 0; + const GRState *state = N->getState(); + const SVal &V = state->getSVal(Receiver); + const DefinedOrUnknownSVal *DV = dyn_cast<DefinedOrUnknownSVal>(&V); + if (!DV) + return 0; + state = state->Assume(*DV, true); + if (state) + return 0; + + // The receiver was nil, and hence the method was skipped. + // Register a BugReporterVisitor to issue a message telling us how + // the receiver was null. + bugreporter::registerTrackNullOrUndefValue(BRC, Receiver, N); + // Issue a message saying that the method was skipped. + PathDiagnosticLocation L(Receiver, BRC.getSourceManager()); + return new PathDiagnosticEventPiece(L, "No method actually called " + "because the receiver is nil"); + } +}; +} // end anonymous namespace + +void clang::bugreporter::registerNilReceiverVisitor(BugReporterContext &BRC) { + BRC.addVisitor(new NilReceiverVisitor()); +} diff --git a/lib/Checker/CallAndMessageChecker.cpp b/lib/Checker/CallAndMessageChecker.cpp index 9013c38..32cf753 100644 --- a/lib/Checker/CallAndMessageChecker.cpp +++ b/lib/Checker/CallAndMessageChecker.cpp @@ -24,7 +24,7 @@ namespace { class CallAndMessageChecker : public CheckerVisitor<CallAndMessageChecker> { BugType *BT_call_null; - BugType *BT_call_undef; + BugType *BT_call_undef; BugType *BT_call_arg; BugType *BT_msg_undef; BugType *BT_msg_arg; @@ -44,12 +44,20 @@ public: bool EvalNilReceiver(CheckerContext &C, const ObjCMessageExpr *ME); private: + bool PreVisitProcessArg(CheckerContext &C, const Expr *Ex, + const char *BT_desc, BugType *&BT); + void EmitBadCall(BugType *BT, CheckerContext &C, const CallExpr *CE); void EmitNilReceiverBug(CheckerContext &C, const ObjCMessageExpr *ME, ExplodedNode *N); - + void HandleNilReceiver(CheckerContext &C, const GRState *state, - const ObjCMessageExpr *ME); + const ObjCMessageExpr *ME); + + void LazyInit_BT(const char *desc, BugType *&BT) { + if (!BT) + BT = new BuiltinBug(desc); + } }; } // end anonymous namespace @@ -62,19 +70,127 @@ void CallAndMessageChecker::EmitBadCall(BugType *BT, CheckerContext &C, ExplodedNode *N = C.GenerateSink(); if (!N) return; - + EnhancedBugReport *R = new EnhancedBugReport(*BT, BT->getName(), N); R->addVisitorCreator(bugreporter::registerTrackNullOrUndefValue, bugreporter::GetCalleeExpr(N)); C.EmitReport(R); } -void CallAndMessageChecker::PreVisitCallExpr(CheckerContext &C, +bool CallAndMessageChecker::PreVisitProcessArg(CheckerContext &C, + const Expr *Ex, + const char *BT_desc, + BugType *&BT) { + + const SVal &V = C.getState()->getSVal(Ex); + + if (V.isUndef()) { + if (ExplodedNode *N = C.GenerateSink()) { + LazyInit_BT(BT_desc, BT); + + // Generate a report for this bug. + EnhancedBugReport *R = new EnhancedBugReport(*BT, BT->getName(), N); + R->addRange(Ex->getSourceRange()); + R->addVisitorCreator(bugreporter::registerTrackNullOrUndefValue, Ex); + C.EmitReport(R); + } + return true; + } + + if (const nonloc::LazyCompoundVal *LV = + dyn_cast<nonloc::LazyCompoundVal>(&V)) { + + class FindUninitializedField { + public: + llvm::SmallVector<const FieldDecl *, 10> FieldChain; + private: + ASTContext &C; + StoreManager &StoreMgr; + MemRegionManager &MrMgr; + Store store; + public: + FindUninitializedField(ASTContext &c, StoreManager &storeMgr, + MemRegionManager &mrMgr, Store s) + : C(c), StoreMgr(storeMgr), MrMgr(mrMgr), store(s) {} + + bool Find(const TypedRegion *R) { + QualType T = R->getValueType(C); + if (const RecordType *RT = T->getAsStructureType()) { + const RecordDecl *RD = RT->getDecl()->getDefinition(); + assert(RD && "Referred record has no definition"); + for (RecordDecl::field_iterator I = + RD->field_begin(), E = RD->field_end(); I!=E; ++I) { + const FieldRegion *FR = MrMgr.getFieldRegion(*I, R); + FieldChain.push_back(*I); + T = (*I)->getType(); + if (T->getAsStructureType()) { + if (Find(FR)) + return true; + } + else { + const SVal &V = StoreMgr.Retrieve(store, loc::MemRegionVal(FR)); + if (V.isUndef()) + return true; + } + FieldChain.pop_back(); + } + } + + return false; + } + }; + + const LazyCompoundValData *D = LV->getCVData(); + FindUninitializedField F(C.getASTContext(), + C.getState()->getStateManager().getStoreManager(), + C.getValueManager().getRegionManager(), + D->getStore()); + + if (F.Find(D->getRegion())) { + if (ExplodedNode *N = C.GenerateSink()) { + LazyInit_BT(BT_desc, BT); + llvm::SmallString<512> Str; + llvm::raw_svector_ostream os(Str); + os << "Passed-by-value struct argument contains uninitialized data"; + + if (F.FieldChain.size() == 1) + os << " (e.g., field: '" << F.FieldChain[0]->getNameAsString() + << "')"; + else { + os << " (e.g., via the field chain: '"; + bool first = true; + for (llvm::SmallVectorImpl<const FieldDecl *>::iterator + DI = F.FieldChain.begin(), DE = F.FieldChain.end(); DI!=DE;++DI){ + if (first) + first = false; + else + os << '.'; + os << (*DI)->getNameAsString(); + } + os << "')"; + } + + // Generate a report for this bug. + EnhancedBugReport *R = new EnhancedBugReport(*BT, os.str(), N); + R->addRange(Ex->getSourceRange()); + + // FIXME: enhance track back for uninitialized value for arbitrary + // memregions + C.EmitReport(R); + } + return true; + } + } + + return false; +} + +void CallAndMessageChecker::PreVisitCallExpr(CheckerContext &C, const CallExpr *CE){ - + const Expr *Callee = CE->getCallee()->IgnoreParens(); SVal L = C.getState()->getSVal(Callee); - + if (L.isUndef()) { if (!BT_call_undef) BT_call_undef = @@ -82,31 +198,20 @@ void CallAndMessageChecker::PreVisitCallExpr(CheckerContext &C, EmitBadCall(BT_call_undef, C, CE); return; } - + if (isa<loc::ConcreteInt>(L)) { if (!BT_call_null) BT_call_null = new BuiltinBug("Called function pointer is null (null dereference)"); EmitBadCall(BT_call_null, C, CE); - } - - for (CallExpr::const_arg_iterator I = CE->arg_begin(), E = CE->arg_end(); - I != E; ++I) { - if (C.getState()->getSVal(*I).isUndef()) { - if (ExplodedNode *N = C.GenerateSink()) { - if (!BT_call_arg) - BT_call_arg = new BuiltinBug("Pass-by-value argument in function call" - " is undefined"); - // Generate a report for this bug. - EnhancedBugReport *R = new EnhancedBugReport(*BT_call_arg, - BT_call_arg->getName(), N); - R->addRange((*I)->getSourceRange()); - R->addVisitorCreator(bugreporter::registerTrackNullOrUndefValue, *I); - C.EmitReport(R); - return; - } - } } + + for (CallExpr::const_arg_iterator I = CE->arg_begin(), E = CE->arg_end(); + I != E; ++I) + if (PreVisitProcessArg(C, *I, + "Pass-by-value argument in function call is" + " undefined", BT_call_arg)) + return; } void CallAndMessageChecker::PreVisitObjCMessageExpr(CheckerContext &C, @@ -132,23 +237,11 @@ void CallAndMessageChecker::PreVisitObjCMessageExpr(CheckerContext &C, // Check for any arguments that are uninitialized/undefined. for (ObjCMessageExpr::const_arg_iterator I = ME->arg_begin(), - E = ME->arg_end(); I != E; ++I) { - if (state->getSVal(*I).isUndef()) { - if (ExplodedNode *N = C.GenerateSink()) { - if (!BT_msg_arg) - BT_msg_arg = - new BuiltinBug("Pass-by-value argument in message expression" - " is undefined"); - // Generate a report for this bug. - EnhancedBugReport *R = new EnhancedBugReport(*BT_msg_arg, - BT_msg_arg->getName(), N); - R->addRange((*I)->getSourceRange()); - R->addVisitorCreator(bugreporter::registerTrackNullOrUndefValue, *I); - C.EmitReport(R); + E = ME->arg_end(); I != E; ++I) + if (PreVisitProcessArg(C, *I, + "Pass-by-value argument in message expression " + "is undefined", BT_msg_arg)) return; - } - } - } } bool CallAndMessageChecker::EvalNilReceiver(CheckerContext &C, @@ -160,24 +253,24 @@ bool CallAndMessageChecker::EvalNilReceiver(CheckerContext &C, void CallAndMessageChecker::EmitNilReceiverBug(CheckerContext &C, const ObjCMessageExpr *ME, ExplodedNode *N) { - + if (!BT_msg_ret) BT_msg_ret = new BuiltinBug("Receiver in message expression is " "'nil' and returns a garbage value"); - + llvm::SmallString<200> buf; llvm::raw_svector_ostream os(buf); os << "The receiver of message '" << ME->getSelector().getAsString() << "' is nil and returns a value of type '" << ME->getType().getAsString() << "' that will be garbage"; - + EnhancedBugReport *report = new EnhancedBugReport(*BT_msg_ret, os.str(), N); const Expr *receiver = ME->getReceiver(); report->addRange(receiver->getSourceRange()); - report->addVisitorCreator(bugreporter::registerTrackNullOrUndefValue, + report->addVisitorCreator(bugreporter::registerTrackNullOrUndefValue, receiver); - C.EmitReport(report); + C.EmitReport(report); } static bool SupportsNilWithFloatRet(const llvm::Triple &triple) { @@ -188,11 +281,11 @@ static bool SupportsNilWithFloatRet(const llvm::Triple &triple) { void CallAndMessageChecker::HandleNilReceiver(CheckerContext &C, const GRState *state, const ObjCMessageExpr *ME) { - + // Check the return type of the message expression. A message to nil will // return different values depending on the return type and the architecture. QualType RetTy = ME->getType(); - + ASTContext &Ctx = C.getASTContext(); CanQualType CanRetTy = Ctx.getCanonicalType(RetTy); @@ -216,7 +309,7 @@ void CallAndMessageChecker::HandleNilReceiver(CheckerContext &C, if (CanRetTy != Ctx.VoidTy && C.getPredecessor()->getParentMap().isConsumedExpr(ME)) { // Compute: sizeof(void *) and sizeof(return type) - const uint64_t voidPtrSize = Ctx.getTypeSize(Ctx.VoidPtrTy); + const uint64_t voidPtrSize = Ctx.getTypeSize(Ctx.VoidPtrTy); const uint64_t returnTypeSize = Ctx.getTypeSize(CanRetTy); if (voidPtrSize < returnTypeSize && @@ -247,6 +340,6 @@ void CallAndMessageChecker::HandleNilReceiver(CheckerContext &C, C.GenerateNode(state->BindExpr(ME, V)); return; } - + C.addTransition(state); } diff --git a/lib/Checker/CheckDeadStores.cpp b/lib/Checker/CheckDeadStores.cpp index 31f9390..d6ea187 100644 --- a/lib/Checker/CheckDeadStores.cpp +++ b/lib/Checker/CheckDeadStores.cpp @@ -220,16 +220,25 @@ public: if (E->isConstantInitializer(Ctx)) return; - // Special case: check for initializations from constant - // variables. - // - // e.g. extern const int MyConstant; - // int x = MyConstant; - // if (DeclRefExpr *DRE=dyn_cast<DeclRefExpr>(E->IgnoreParenCasts())) - if (VarDecl *VD = dyn_cast<VarDecl>(DRE->getDecl())) + if (VarDecl *VD = dyn_cast<VarDecl>(DRE->getDecl())) { + // Special case: check for initialization from constant + // variables. + // + // e.g. extern const int MyConstant; + // int x = MyConstant; + // if (VD->hasGlobalStorage() && - VD->getType().isConstQualified()) return; + VD->getType().isConstQualified()) + return; + // Special case: check for initialization from scalar + // parameters. This is often a form of defensive + // programming. Non-scalars are still an error since + // because it more likely represents an actual algorithmic + // bug. + if (isa<ParmVarDecl>(VD) && VD->getType()->isScalarType()) + return; + } Report(V, DeadInit, V->getLocation(), E->getSourceRange()); } diff --git a/lib/Checker/FlatStore.cpp b/lib/Checker/FlatStore.cpp index 07a54fb..2af9ffa 100644 --- a/lib/Checker/FlatStore.cpp +++ b/lib/Checker/FlatStore.cpp @@ -44,7 +44,9 @@ public: } SVal ArrayToPointer(Loc Array); - Store RemoveDeadBindings(Store store, Stmt* Loc, SymbolReaper& SymReaper, + Store RemoveDeadBindings(Store store, Stmt* Loc, + const StackFrameContext *LCtx, + SymbolReaper& SymReaper, llvm::SmallVectorImpl<const MemRegion*>& RegionRoots){ return store; } diff --git a/lib/Checker/GRExprEngine.cpp b/lib/Checker/GRExprEngine.cpp index e64ba94..3ace552 100644 --- a/lib/Checker/GRExprEngine.cpp +++ b/lib/Checker/GRExprEngine.cpp @@ -481,7 +481,9 @@ void GRExprEngine::ProcessStmt(CFGElement CE, GRStmtNodeBuilder& builder) { SymbolReaper SymReaper(BasePred->getLocationContext(), SymMgr); CleanedState = AMgr.shouldPurgeDead() - ? StateMgr.RemoveDeadBindings(EntryNode->getState(), CurrentStmt, SymReaper) + ? StateMgr.RemoveDeadBindings(EntryNode->getState(), CurrentStmt, + BasePred->getLocationContext()->getCurrentStackFrame(), + SymReaper) : EntryNode->getState(); // Process any special transfer function for dead symbols. diff --git a/lib/Checker/GRState.cpp b/lib/Checker/GRState.cpp index 97ede1d..2defbcd 100644 --- a/lib/Checker/GRState.cpp +++ b/lib/Checker/GRState.cpp @@ -35,6 +35,7 @@ GRStateManager::~GRStateManager() { const GRState* GRStateManager::RemoveDeadBindings(const GRState* state, Stmt* Loc, + const StackFrameContext *LCtx, SymbolReaper& SymReaper) { // This code essentially performs a "mark-and-sweep" of the VariableBindings. @@ -50,7 +51,7 @@ GRStateManager::RemoveDeadBindings(const GRState* state, Stmt* Loc, state, RegionRoots); // Clean up the store. - NewState.St = StoreMgr->RemoveDeadBindings(NewState.St, Loc, SymReaper, + NewState.St = StoreMgr->RemoveDeadBindings(NewState.St, Loc, LCtx, SymReaper, RegionRoots); return ConstraintMgr->RemoveDeadBindings(getPersistentState(NewState), diff --git a/lib/Checker/RegionStore.cpp b/lib/Checker/RegionStore.cpp index 307ef78..c2b702a 100644 --- a/lib/Checker/RegionStore.cpp +++ b/lib/Checker/RegionStore.cpp @@ -354,7 +354,9 @@ public: // Part of public interface to class. /// RemoveDeadBindings - Scans the RegionStore of 'state' for dead values. /// It returns a new Store with these values removed. - Store RemoveDeadBindings(Store store, Stmt* Loc, SymbolReaper& SymReaper, + Store RemoveDeadBindings(Store store, Stmt* Loc, + const StackFrameContext *LCtx, + SymbolReaper& SymReaper, llvm::SmallVectorImpl<const MemRegion*>& RegionRoots); const GRState *EnterStackFrame(const GRState *state, @@ -1678,12 +1680,14 @@ class RemoveDeadBindingsWorker : llvm::SmallVector<const SymbolicRegion*, 12> Postponed; SymbolReaper &SymReaper; Stmt *Loc; + const StackFrameContext *CurrentLCtx; + public: RemoveDeadBindingsWorker(RegionStoreManager &rm, GRStateManager &stateMgr, RegionBindings b, SymbolReaper &symReaper, - Stmt *loc) + Stmt *loc, const StackFrameContext *LCtx) : ClusterAnalysis<RemoveDeadBindingsWorker>(rm, stateMgr, b), - SymReaper(symReaper), Loc(loc) {} + SymReaper(symReaper), Loc(loc), CurrentLCtx(LCtx) {} // Called by ClusterAnalysis. void VisitAddedToCluster(const MemRegion *baseR, RegionCluster &C); @@ -1713,6 +1717,15 @@ void RemoveDeadBindingsWorker::VisitAddedToCluster(const MemRegion *baseR, return; } + + // CXXThisRegion in the current or parent location context is live. + if (const CXXThisRegion *TR = dyn_cast<CXXThisRegion>(baseR)) { + const StackArgumentsSpaceRegion *StackReg = + cast<StackArgumentsSpaceRegion>(TR->getSuperRegion()); + const StackFrameContext *RegCtx = StackReg->getStackFrame(); + if (RegCtx == CurrentLCtx || RegCtx->isParentOf(CurrentLCtx)) + AddToWorkList(TR, C); + } } void RemoveDeadBindingsWorker::VisitCluster(const MemRegion *baseR, @@ -1799,11 +1812,12 @@ bool RemoveDeadBindingsWorker::UpdatePostponed() { } Store RegionStoreManager::RemoveDeadBindings(Store store, Stmt* Loc, + const StackFrameContext *LCtx, SymbolReaper& SymReaper, llvm::SmallVectorImpl<const MemRegion*>& RegionRoots) { RegionBindings B = GetRegionBindings(store); - RemoveDeadBindingsWorker W(*this, StateMgr, B, SymReaper, Loc); + RemoveDeadBindingsWorker W(*this, StateMgr, B, SymReaper, Loc, LCtx); W.GenerateClusters(); // Enqueue the region roots onto the worklist. |