diff options
-rw-r--r-- | include/clang/Analysis/ProgramPoint.h | 1 | ||||
-rw-r--r-- | include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h | 18 | ||||
-rw-r--r-- | include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitor.h | 28 | ||||
-rw-r--r-- | lib/StaticAnalyzer/Checkers/DereferenceChecker.cpp | 1 | ||||
-rw-r--r-- | lib/StaticAnalyzer/Checkers/ReturnUndefChecker.cpp | 1 | ||||
-rw-r--r-- | lib/StaticAnalyzer/Checkers/UndefBranchChecker.cpp | 1 | ||||
-rw-r--r-- | lib/StaticAnalyzer/Checkers/UndefResultChecker.cpp | 1 | ||||
-rw-r--r-- | lib/StaticAnalyzer/Checkers/UndefinedAssignmentChecker.cpp | 1 | ||||
-rw-r--r-- | lib/StaticAnalyzer/Core/BugReporter.cpp | 258 | ||||
-rw-r--r-- | lib/StaticAnalyzer/Core/BugReporterVisitors.cpp | 55 | ||||
-rw-r--r-- | test/Analysis/diagnostics/undef-value-caller.c | 379 | ||||
-rw-r--r-- | test/Analysis/diagnostics/undef-value-param.c | 451 | ||||
-rw-r--r-- | test/Analysis/diagnostics/undef-value-param.m | 476 |
13 files changed, 1324 insertions, 347 deletions
diff --git a/include/clang/Analysis/ProgramPoint.h b/include/clang/Analysis/ProgramPoint.h index 5de06cd3a5..e66022c876 100644 --- a/include/clang/Analysis/ProgramPoint.h +++ b/include/clang/Analysis/ProgramPoint.h @@ -461,6 +461,7 @@ public: }; /// Represents a point when we begin processing an inlined call. +/// CallEnter uses the caller's location context. class CallEnter : public ProgramPoint { public: CallEnter(const Stmt *stmt, const StackFrameContext *calleeCtx, diff --git a/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h b/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h index 48393a379b..c8581e2e23 100644 --- a/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h +++ b/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h @@ -24,6 +24,7 @@ #include "llvm/ADT/ilist_node.h" #include "llvm/ADT/ImmutableSet.h" #include "llvm/ADT/DenseSet.h" +#include "llvm/ADT/SmallSet.h" namespace clang { @@ -95,6 +96,10 @@ protected: /// for multiple PathDiagnosticConsumers. llvm::SmallVector<Regions *, 2> interestingRegions; + /// A set of location contexts that correspoind to call sites which should be + /// considered "interesting". + llvm::SmallSet<const LocationContext *, 2> InterestingLocationContexts; + /// A set of custom visitors which generate "event" diagnostics at /// interesting points in the path. VisitorList Callbacks; @@ -172,10 +177,12 @@ public: void markInteresting(SymbolRef sym); void markInteresting(const MemRegion *R); void markInteresting(SVal V); + void markInteresting(const LocationContext *LC); bool isInteresting(SymbolRef sym); bool isInteresting(const MemRegion *R); bool isInteresting(SVal V); + bool isInteresting(const LocationContext *LC); unsigned getConfigurationChangeToken() const { return ConfigurationChangeToken; @@ -342,6 +349,11 @@ private: /// A vector of BugReports for tracking the allocated pointers and cleanup. std::vector<BugReportEquivClass *> EQClassesVector; + /// A map from PathDiagnosticPiece to the LocationContext of the inlined + /// function call it represents. + llvm::DenseMap<const PathDiagnosticCallPiece*, + const LocationContext*> LocationContextMap; + protected: BugReporter(BugReporterData& d, Kind k) : BugTypes(F.getEmptySet()), kind(k), D(d) {} @@ -382,6 +394,8 @@ public: PathDiagnosticConsumer &PC, ArrayRef<BugReport *> &bugReports) {} + bool RemoveUneededCalls(PathPieces &pieces, BugReport *R); + void Register(BugType *BT); /// \brief Add the given report to the set of reports tracked by BugReporter. @@ -411,6 +425,10 @@ public: static bool classof(const BugReporter* R) { return true; } + void addCallPieceLocationContextPair(const PathDiagnosticCallPiece *C, + const LocationContext *LC) { + LocationContextMap[C] = LC; + } private: llvm::StringMap<BugType *> StrBugTypes; diff --git a/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitor.h b/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitor.h index e82b5233d7..f3206964e3 100644 --- a/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitor.h +++ b/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitor.h @@ -222,7 +222,33 @@ public: const ExplodedNode *N, llvm::Optional<bool> &prunable); }; - + +/// \brief When a region containing undefined value or '0' value is passed +/// as an argument in a call, marks the call as interesting. +/// +/// As a result, BugReporter will not prune the path through the function even +/// if the region's contents are not modified/accessed by the call. +class UndefOrNullArgVisitor + : public BugReporterVisitorImpl<UndefOrNullArgVisitor> { + + /// The interesting memory region this visitor is tracking. + const MemRegion *R; + +public: + UndefOrNullArgVisitor(const MemRegion *InR) : R(InR) {} + + virtual void Profile(llvm::FoldingSetNodeID &ID) const { + static int Tag = 0; + ID.AddPointer(&Tag); + ID.AddPointer(R); + } + + PathDiagnosticPiece *VisitNode(const ExplodedNode *N, + const ExplodedNode *PrevN, + BugReporterContext &BRC, + BugReport &BR); +}; + namespace bugreporter { void trackNullOrUndefValue(const ExplodedNode *N, const Stmt *S, BugReport &R); diff --git a/lib/StaticAnalyzer/Checkers/DereferenceChecker.cpp b/lib/StaticAnalyzer/Checkers/DereferenceChecker.cpp index cb0a3ce3e3..00128de2b6 100644 --- a/lib/StaticAnalyzer/Checkers/DereferenceChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/DereferenceChecker.cpp @@ -192,7 +192,6 @@ void DereferenceChecker::checkLocation(SVal l, bool isLoad, const Stmt* S, new BugReport(*BT_undef, BT_undef->getDescription(), N); bugreporter::trackNullOrUndefValue(N, bugreporter::GetDerefExpr(N), *report); - report->disablePathPruning(); C.EmitReport(report); } return; diff --git a/lib/StaticAnalyzer/Checkers/ReturnUndefChecker.cpp b/lib/StaticAnalyzer/Checkers/ReturnUndefChecker.cpp index 4abda42dd3..7e95199685 100644 --- a/lib/StaticAnalyzer/Checkers/ReturnUndefChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/ReturnUndefChecker.cpp @@ -53,7 +53,6 @@ void ReturnUndefChecker::checkPreStmt(const ReturnStmt *RS, BugReport *report = new BugReport(*BT, BT->getDescription(), N); - report->disablePathPruning(); report->addRange(RetE->getSourceRange()); bugreporter::trackNullOrUndefValue(N, RetE, *report); diff --git a/lib/StaticAnalyzer/Checkers/UndefBranchChecker.cpp b/lib/StaticAnalyzer/Checkers/UndefBranchChecker.cpp index 8a97b057b5..a50152eb5a 100644 --- a/lib/StaticAnalyzer/Checkers/UndefBranchChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/UndefBranchChecker.cpp @@ -101,7 +101,6 @@ void UndefBranchChecker::checkBranchCondition(const Stmt *Condition, BugReport *R = new BugReport(*BT, BT->getDescription(), N); bugreporter::trackNullOrUndefValue(N, Ex, *R); R->addRange(Ex->getSourceRange()); - R->disablePathPruning(); Ctx.EmitReport(R); } diff --git a/lib/StaticAnalyzer/Checkers/UndefResultChecker.cpp b/lib/StaticAnalyzer/Checkers/UndefResultChecker.cpp index 605f6775f9..bbfff0cdc3 100644 --- a/lib/StaticAnalyzer/Checkers/UndefResultChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/UndefResultChecker.cpp @@ -81,7 +81,6 @@ void UndefResultChecker::checkPostStmt(const BinaryOperator *B, else bugreporter::trackNullOrUndefValue(N, B, *report); - report->disablePathPruning(); C.EmitReport(report); } } diff --git a/lib/StaticAnalyzer/Checkers/UndefinedAssignmentChecker.cpp b/lib/StaticAnalyzer/Checkers/UndefinedAssignmentChecker.cpp index 8ae75eaaea..021364bde8 100644 --- a/lib/StaticAnalyzer/Checkers/UndefinedAssignmentChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/UndefinedAssignmentChecker.cpp @@ -80,7 +80,6 @@ void UndefinedAssignmentChecker::checkBind(SVal location, SVal val, R->addRange(ex->getSourceRange()); bugreporter::trackNullOrUndefValue(N, ex, *R); } - R->disablePathPruning(); C.EmitReport(R); } diff --git a/lib/StaticAnalyzer/Core/BugReporter.cpp b/lib/StaticAnalyzer/Core/BugReporter.cpp index 571baecd72..1866a27f72 100644 --- a/lib/StaticAnalyzer/Core/BugReporter.cpp +++ b/lib/StaticAnalyzer/Core/BugReporter.cpp @@ -121,7 +121,7 @@ GetCurrentOrNextStmt(const ExplodedNode *N) { /// Recursively scan through a path and prune out calls and macros pieces /// that aren't needed. Return true if afterwards the path contains /// "interesting stuff" which means it should be pruned from the parent path. -static bool RemoveUneededCalls(PathPieces &pieces) { +bool BugReporter::RemoveUneededCalls(PathPieces &pieces, BugReport *R) { bool containsSomethingInteresting = false; const unsigned N = pieces.size(); @@ -134,16 +134,22 @@ static bool RemoveUneededCalls(PathPieces &pieces) { switch (piece->getKind()) { case PathDiagnosticPiece::Call: { PathDiagnosticCallPiece *call = cast<PathDiagnosticCallPiece>(piece); + // Check if the location context is interesting. + assert(LocationContextMap.count(call)); + if (R->isInteresting(LocationContextMap[call])) { + containsSomethingInteresting = true; + break; + } // Recursively clean out the subclass. Keep this call around if // it contains any informative diagnostics. - if (!RemoveUneededCalls(call->path)) + if (!RemoveUneededCalls(call->path, R)) continue; containsSomethingInteresting = true; break; } case PathDiagnosticPiece::Macro: { PathDiagnosticMacroPiece *macro = cast<PathDiagnosticMacroPiece>(piece); - if (!RemoveUneededCalls(macro->subPieces)) + if (!RemoveUneededCalls(macro->subPieces, R)) continue; containsSomethingInteresting = true; break; @@ -430,55 +436,60 @@ static void GenerateMinimalPathDiagnostic(PathDiagnostic& PD, NextNode = GetPredecessorNode(N); ProgramPoint P = N->getLocation(); - - if (const CallExitEnd *CE = dyn_cast<CallExitEnd>(&P)) { - PathDiagnosticCallPiece *C = - PathDiagnosticCallPiece::construct(N, *CE, SMgr); - PD.getActivePath().push_front(C); - PD.pushActivePath(&C->path); - CallStack.push_back(StackDiagPair(C, N)); - continue; - } - - if (const CallEnter *CE = dyn_cast<CallEnter>(&P)) { - // Flush all locations, and pop the active path. - bool VisitedEntireCall = PD.isWithinCall(); - PD.popActivePath(); - - // Either we just added a bunch of stuff to the top-level path, or - // we have a previous CallExitEnd. If the former, it means that the - // path terminated within a function call. We must then take the - // current contents of the active path and place it within - // a new PathDiagnosticCallPiece. - PathDiagnosticCallPiece *C; - if (VisitedEntireCall) { - C = cast<PathDiagnosticCallPiece>(PD.getActivePath().front()); - } else { - const Decl *Caller = CE->getLocationContext()->getDecl(); - C = PathDiagnosticCallPiece::construct(PD.getActivePath(), Caller); + + do { + if (const CallExitEnd *CE = dyn_cast<CallExitEnd>(&P)) { + PathDiagnosticCallPiece *C = + PathDiagnosticCallPiece::construct(N, *CE, SMgr); + GRBugReporter& BR = PDB.getBugReporter(); + BR.addCallPieceLocationContextPair(C, CE->getCalleeContext()); + PD.getActivePath().push_front(C); + PD.pushActivePath(&C->path); + CallStack.push_back(StackDiagPair(C, N)); + break; } - C->setCallee(*CE, SMgr); - if (!CallStack.empty()) { - assert(CallStack.back().first == C); - CallStack.pop_back(); + if (const CallEnter *CE = dyn_cast<CallEnter>(&P)) { + // Flush all locations, and pop the active path. + bool VisitedEntireCall = PD.isWithinCall(); + PD.popActivePath(); + + // Either we just added a bunch of stuff to the top-level path, or + // we have a previous CallExitEnd. If the former, it means that the + // path terminated within a function call. We must then take the + // current contents of the active path and place it within + // a new PathDiagnosticCallPiece. + PathDiagnosticCallPiece *C; + if (VisitedEntireCall) { + C = cast<PathDiagnosticCallPiece>(PD.getActivePath().front()); + } else { + const Decl *Caller = CE->getLocationContext()->getDecl(); + C = PathDiagnosticCallPiece::construct(PD.getActivePath(), Caller); + GRBugReporter& BR = PDB.getBugReporter(); + BR.addCallPieceLocationContextPair(C, CE->getCalleeContext()); + } + + C->setCallee(*CE, SMgr); + if (!CallStack.empty()) { + assert(CallStack.back().first == C); + CallStack.pop_back(); + } + break; } - continue; - } - if (const BlockEdge *BE = dyn_cast<BlockEdge>(&P)) { - const CFGBlock *Src = BE->getSrc(); - const CFGBlock *Dst = BE->getDst(); - const Stmt *T = Src->getTerminator(); + if (const BlockEdge *BE = dyn_cast<BlockEdge>(&P)) { + const CFGBlock *Src = BE->getSrc(); + const CFGBlock *Dst = BE->getDst(); + const Stmt *T = Src->getTerminator(); - if (!T) - continue; + if (!T) + break; - PathDiagnosticLocation Start = - PathDiagnosticLocation::createBegin(T, SMgr, - N->getLocationContext()); + PathDiagnosticLocation Start = + PathDiagnosticLocation::createBegin(T, SMgr, + N->getLocationContext()); - switch (T->getStmtClass()) { + switch (T->getStmtClass()) { default: break; @@ -487,16 +498,16 @@ static void GenerateMinimalPathDiagnostic(PathDiagnostic& PD, const Stmt *S = GetNextStmt(N); if (!S) - continue; + break; std::string sbuf; llvm::raw_string_ostream os(sbuf); const PathDiagnosticLocation &End = PDB.getEnclosingStmtLocation(S); os << "Control jumps to line " - << End.asLocation().getExpansionLineNumber(); - PD.getActivePath().push_front(new PathDiagnosticControlFlowPiece(Start, End, - os.str())); + << End.asLocation().getExpansionLineNumber(); + PD.getActivePath().push_front(new PathDiagnosticControlFlowPiece( + Start, End, os.str())); break; } @@ -509,52 +520,52 @@ static void GenerateMinimalPathDiagnostic(PathDiagnostic& PD, PathDiagnosticLocation End(S, SMgr, LC); switch (S->getStmtClass()) { - default: - os << "No cases match in the switch statement. " - "Control jumps to line " - << End.asLocation().getExpansionLineNumber(); - break; - case Stmt::DefaultStmtClass: - os << "Control jumps to the 'default' case at line " - << End.asLocation().getExpansionLineNumber(); - break; - - case Stmt::CaseStmtClass: { - os << "Control jumps to 'case "; - const CaseStmt *Case = cast<CaseStmt>(S); - const Expr *LHS = Case->getLHS()->IgnoreParenCasts(); - - // Determine if it is an enum. - bool GetRawInt = true; - - if (const DeclRefExpr *DR = dyn_cast<DeclRefExpr>(LHS)) { - // FIXME: Maybe this should be an assertion. Are there cases - // were it is not an EnumConstantDecl? - const EnumConstantDecl *D = + default: + os << "No cases match in the switch statement. " + "Control jumps to line " + << End.asLocation().getExpansionLineNumber(); + break; + case Stmt::DefaultStmtClass: + os << "Control jumps to the 'default' case at line " + << End.asLocation().getExpansionLineNumber(); + break; + + case Stmt::CaseStmtClass: { + os << "Control jumps to 'case "; + const CaseStmt *Case = cast<CaseStmt>(S); + const Expr *LHS = Case->getLHS()->IgnoreParenCasts(); + + // Determine if it is an enum. + bool GetRawInt = true; + + if (const DeclRefExpr *DR = dyn_cast<DeclRefExpr>(LHS)) { + // FIXME: Maybe this should be an assertion. Are there cases + // were it is not an EnumConstantDecl? + const EnumConstantDecl *D = dyn_cast<EnumConstantDecl>(DR->getDecl()); - if (D) { - GetRawInt = false; - os << *D; - } + if (D) { + GetRawInt = false; + os << *D; } + } - if (GetRawInt) - os << LHS->EvaluateKnownConstInt(PDB.getASTContext()); + if (GetRawInt) + os << LHS->EvaluateKnownConstInt(PDB.getASTContext()); - os << ":' at line " - << End.asLocation().getExpansionLineNumber(); - break; - } + os << ":' at line " + << End.asLocation().getExpansionLineNumber(); + break; } - PD.getActivePath().push_front(new PathDiagnosticControlFlowPiece(Start, End, - os.str())); + } + PD.getActivePath().push_front(new PathDiagnosticControlFlowPiece( + Start, End, os.str())); } else { os << "'Default' branch taken. "; const PathDiagnosticLocation &End = PDB.ExecutionContinues(os, N); - PD.getActivePath().push_front(new PathDiagnosticControlFlowPiece(Start, End, - os.str())); + PD.getActivePath().push_front(new PathDiagnosticControlFlowPiece( + Start, End, os.str())); } break; @@ -565,12 +576,12 @@ static void GenerateMinimalPathDiagnostic(PathDiagnostic& PD, std::string sbuf; llvm::raw_string_ostream os(sbuf); PathDiagnosticLocation End = PDB.ExecutionContinues(os, N); - PD.getActivePath().push_front(new PathDiagnosticControlFlowPiece(Start, End, - os.str())); + PD.getActivePath().push_front(new PathDiagnosticControlFlowPiece( + Start, End, os.str())); break; } - // Determine control-flow for ternary '?'. + // Determine control-flow for ternary '?'. case Stmt::BinaryConditionalOperatorClass: case Stmt::ConditionalOperatorClass: { std::string sbuf; @@ -587,12 +598,12 @@ static void GenerateMinimalPathDiagnostic(PathDiagnostic& PD, if (const Stmt *S = End.asStmt()) End = PDB.getEnclosingStmtLocation(S); - PD.getActivePath().push_front(new PathDiagnosticControlFlowPiece(Start, End, - os.str())); + PD.getActivePath().push_front(new PathDiagnosticControlFlowPiece( + Start, End, os.str())); break; } - // Determine control-flow for short-circuited '&&' and '||'. + // Determine control-flow for short-circuited '&&' and '||'. case Stmt::BinaryOperatorClass: { if (!PDB.supportsLogicalOpControlFlow()) break; @@ -609,16 +620,16 @@ static void GenerateMinimalPathDiagnostic(PathDiagnostic& PD, os << "false"; PathDiagnosticLocation End(B->getLHS(), SMgr, LC); PathDiagnosticLocation Start = - PathDiagnosticLocation::createOperatorLoc(B, SMgr); - PD.getActivePath().push_front(new PathDiagnosticControlFlowPiece(Start, End, - os.str())); + PathDiagnosticLocation::createOperatorLoc(B, SMgr); + PD.getActivePath().push_front(new PathDiagnosticControlFlowPiece( + Start, End, os.str())); } else { os << "true"; PathDiagnosticLocation Start(B->getLHS(), SMgr, LC); PathDiagnosticLocation End = PDB.ExecutionContinues(N); - PD.getActivePath().push_front(new PathDiagnosticControlFlowPiece(Start, End, - os.str())); + PD.getActivePath().push_front(new PathDiagnosticControlFlowPiece( + Start, End, os.str())); } } else { @@ -629,16 +640,16 @@ static void GenerateMinimalPathDiagnostic(PathDiagnostic& PD, os << "false"; PathDiagnosticLocation Start(B->getLHS(), SMgr, LC); PathDiagnosticLocation End = PDB.ExecutionContinues(N); - PD.getActivePath().push_front(new PathDiagnosticControlFlowPiece(Start, End, - os.str())); + PD.getActivePath().push_front(new PathDiagnosticControlFlowPiece( + Start, End, os.str())); } else { os << "true"; PathDiagnosticLocation End(B->getLHS(), SMgr, LC); PathDiagnosticLocation Start = - PathDiagnosticLocation::createOperatorLoc(B, SMgr); - PD.getActivePath().push_front(new PathDiagnosticControlFlowPiece(Start, End, - os.str())); + PathDiagnosticLocation::createOperatorLoc(B, SMgr); + PD.getActivePath().push_front(new PathDiagnosticControlFlowPiece( + Start, End, os.str())); } } @@ -656,8 +667,8 @@ static void GenerateMinimalPathDiagnostic(PathDiagnostic& PD, if (const Stmt *S = End.asStmt()) End = PDB.getEnclosingStmtLocation(S); - PD.getActivePath().push_front(new PathDiagnosticControlFlowPiece(Start, End, - os.str())); + PD.getActivePath().push_front(new PathDiagnosticControlFlowPiece( + Start, End, os.str())); } else { PathDiagnosticLocation End = PDB.ExecutionContinues(N); @@ -665,8 +676,8 @@ static void GenerateMinimalPathDiagnostic(PathDiagnostic& PD, if (const Stmt *S = End.asStmt()) End = PDB.getEnclosingStmtLocation(S); - PD.getActivePath().push_front(new PathDiagnosticControlFlowPiece(Start, End, - "Loop condition is false. Exiting loop")); + PD.getActivePath().push_front(new PathDiagnosticControlFlowPiece( + Start, End, "Loop condition is false. Exiting loop")); } break; @@ -683,16 +694,16 @@ static void GenerateMinimalPathDiagnostic(PathDiagnostic& PD, if (const Stmt *S = End.asStmt()) End = PDB.getEnclosingStmtLocation(S); - PD.getActivePath().push_front(new PathDiagnosticControlFlowPiece(Start, End, - os.str())); + PD.getActivePath().push_front(new PathDiagnosticControlFlowPiece( + Start, End, os.str())); } else { PathDiagnosticLocation End = PDB.ExecutionContinues(N); if (const Stmt *S = End.asStmt()) End = PDB.getEnclosingStmtLocation(S); - PD.getActivePath().push_front(new PathDiagnosticControlFlowPiece(Start, End, - "Loop condition is true. Entering loop body")); + PD.getActivePath().push_front(new PathDiagnosticControlFlowPiece( + Start, End, "Loop condition is true. Entering loop body")); } break; @@ -705,16 +716,17 @@ static void GenerateMinimalPathDiagnostic(PathDiagnostic& PD, End = PDB.getEnclosingStmtLocation(S); if (*(Src->succ_begin()+1) == Dst) - PD.getActivePath().push_front(new PathDiagnosticControlFlowPiece(Start, End, - "Taking false branch")); + PD.getActivePath().push_front(new PathDiagnosticControlFlowPiece( + Start, End, "Taking false branch")); else - PD.getActivePath().push_front(new PathDiagnosticControlFlowPiece(Start, End, - "Taking true branch")); + PD.getActivePath().push_front(new PathDiagnosticControlFlowPiece( + Start, End, "Taking true branch")); break; } + } } - } + } while(0); if (NextNode) { // Add diagnostic pieces from custom visitors. @@ -1166,6 +1178,8 @@ static void GenerateExtensivePathDiagnostic(PathDiagnostic& PD, PathDiagnosticCallPiece *C = PathDiagnosticCallPiece::construct(N, *CE, SM); + GRBugReporter& BR = PDB.getBugReporter(); + BR.addCallPieceLocationContextPair(C, CE->getCalleeContext()); EB.addEdge(C->callReturn, true); EB.flushLocations(); @@ -1202,6 +1216,8 @@ static void GenerateExtensivePathDiagnostic(PathDiagnostic& PD, } else { const Decl *Caller = CE->getLocationContext()->getDecl(); C = PathDiagnosticCallPiece::construct(PD.getActivePath(), Caller); + GRBugReporter& BR = PDB.getBugReporter(); + BR.addCallPieceLocationContextPair(C, CE->getCalleeContext()); } C->setCallee(*CE, SM); @@ -1414,6 +1430,12 @@ void BugReport::markInteresting(SVal V) { markInteresting(V.getAsSymbol()); } +void BugReport::markInteresting(const LocationContext *LC) { + if (!LC) + return; + InterestingLocationContexts.insert(LC); +} + bool BugReport::isInteresting(SVal V) { return isInteresting(V.getAsRegion()) || isInteresting(V.getAsSymbol()); } @@ -1438,6 +1460,12 @@ bool BugReport::isInteresting(const MemRegion *R) { return false; } +bool BugReport::isInteresting(const LocationContext *LC) { + if (!LC) + return false; + return InterestingLocationContexts.count(LC); +} + void BugReport::lazyInitializeInterestingSets() { if (interestingSymbols.empty()) { interestingSymbols.push_back(new Symbols()); @@ -1911,7 +1939,7 @@ void GRBugReporter::GeneratePathDiagnostic(PathDiagnostic& PD, // Finally, prune the diagnostic path of uninteresting stuff. if (R->shouldPrunePath()) { - bool hasSomethingInteresting = RemoveUneededCalls(PD.getMutablePieces()); + bool hasSomethingInteresting = RemoveUneededCalls(PD.getMutablePieces(), R); assert(hasSomethingInteresting); (void) hasSomethingInteresting; } diff --git a/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp b/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp index a1b7e305e1..521b727395 100644 --- a/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp +++ b/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp @@ -17,6 +17,7 @@ #include "clang/AST/ExprObjC.h" #include "clang/StaticAnalyzer/Core/BugReporter/BugReporter.h" #include "clang/StaticAnalyzer/Core/BugReporter/PathDiagnostic.h" +#include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h" #include "clang/StaticAnalyzer/Core/PathSensitive/ExplodedGraph.h" #include "clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h" #include "clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h" @@ -478,6 +479,7 @@ void bugreporter::trackNullOrUndefValue(const ExplodedNode *N, const Stmt *S, SVal V = state->getRawSVal(loc::MemRegionVal(R)); report.markInteresting(R); report.markInteresting(V); + report.addVisitor(new UndefOrNullArgVisitor(R)); // If the contents are symbolic, find out when they became null. if (V.getAsLocSymbol()) { @@ -503,6 +505,8 @@ void bugreporter::trackNullOrUndefValue(const ExplodedNode *N, const Stmt *S, // Is it a symbolic value? if (loc::MemRegionVal *L = dyn_cast<loc::MemRegionVal>(&V)) { const MemRegion *Base = L->getRegion()->getBaseRegion(); + report.addVisitor(new UndefOrNullArgVisitor(Base)); + if (isa<SymbolicRegion>(Base)) { report.markInteresting(Base); report.addVisitor(new TrackConstraintBRVisitor(loc::MemRegionVal(Base), @@ -950,3 +954,54 @@ ConditionBRVisitor::VisitTrueTest(const Expr *Cond, return event; } +PathDiagnosticPiece * +UndefOrNullArgVisitor::VisitNode(const ExplodedNode *N, + const ExplodedNode *PrevN, + BugReporterContext &BRC, + BugReport &BR) { + + ProgramStateRef State = N->getState(); + ProgramPoint ProgLoc = N->getLocation(); + + // We are only interested in visiting CallEnter nodes. + CallEnter *CEnter = dyn_cast<CallEnter>(&ProgLoc); + if (!CEnter) + return 0; + + // Check if one of the arguments is the region the visitor is tracking. + CallEventManager &CEMgr = BRC.getStateManager().getCallEventManager(); + CallEventRef<> Call = CEMgr.getCaller(CEnter->getCalleeContext(), State); + unsigned Idx = 0; + for (CallEvent::param_iterator I = Call->param_begin(), + E = Call->param_end(); I != E; ++I, ++Idx) { + const MemRegion *ArgReg = Call->getArgSVal(Idx).getAsRegion(); + + // Are we tracking the argument? + if ( !ArgReg || ArgReg != R) + return 0; + + // Check the function parameter type. + const ParmVarDecl *ParamDecl = *I; + assert(ParamDecl && "Formal parameter has no decl?"); + QualType T = ParamDecl->getType(); + + if (!(T->isAnyPointerType() || T->isReferenceType())) { + // Function can only change the value passed in by address. + return 0; + } + + // If it is a const pointer value, the function does not intend to + // change the value. + if (T->getPointeeType().isConstQualified()) + return 0; + + // Mark the call site (LocationContext) as interesting if the value of the + // argument is undefined or '0'/'NULL'. + SVal BoundVal = State->getSVal(ArgReg); + if (BoundVal.isUndef() || BoundVal.isZeroConstant()) { + BR.markInteresting(CEnter->getCalleeContext()); + return 0; + } + } + return 0; +} diff --git a/test/Analysis/diagnostics/undef-value-caller.c b/test/Analysis/diagnostics/undef-value-caller.c index 928839d040..627b334971 100644 --- a/test/Analysis/diagnostics/undef-value-caller.c +++ b/test/Analysis/diagnostics/undef-value-caller.c @@ -4,235 +4,162 @@ #include "undef-value-callee.h" // This code used to cause a crash since we were not adding fileID of the header to the plist diagnostic. -// Note, in the future, we do not even need to step into this callee since it does not influence the result. + int test_calling_unimportant_callee(int argc, char *argv[]) { int x; callee(); return x; // expected-warning {{Undefined or garbage value returned to caller}} } -// CHECK: <?xml version="1.0" encoding="UTF-8"?> -// CHECK: <plist version="1.0"> -// CHECK: <dict> -// CHECK: <key>files</key> -// CHECK: <array> -// CHECK: </array> -// CHECK: <key>diagnostics</key> -// CHECK: <array> -// CHECK: <dict> -// CHECK: <key>path</key> -// CHECK: <array> -// CHECK: <dict> -// CHECK: <key>kind</key><string>event</string> -// CHECK: <key>location</key> -// CHECK: <dict> -// CHECK: <key>line</key><integer>9</integer> -// CHECK: <key>col</key><integer>3</integer> -// CHECK: <key>file</key><integer>0</integer> -// CHECK: </dict> -// CHECK: <key>ranges</key> -// CHECK: <array> -// CHECK: <array> -// CHECK: <dict> -// CHECK: <key>line</key><integer>9</integer> -// CHECK: <key>col</key><integer>3</integer> -// CHECK: <key>file</key><integer>0</integer> -// CHECK: </dict> -// CHECK: <dict> -// CHECK: <key>line</key><integer>9</integer> -// CHECK: <key>col</key><integer>7</integer> -// CHECK: <key>file</key><integer>0</integer> -// CHECK: </dict> -// CHECK: </array> -// CHECK: </array> -// CHECK: <key>depth</key><integer>0</integer> -// CHECK: <key>extended_message</key> -// CHECK: <string>Variable 'x' declared without an initial value</string> -// CHECK: <key>message</key> -// CHECK: <string>Variable 'x' declared without an initial value</string> -// CHECK: </dict> -// CHECK: <dict> -// CHECK: <key>kind</key><string>control</string> -// CHECK: <key>edges</key> -// CHECK: <array> -// CHECK: <dict> -// CHECK: <key>start</key> -// CHECK: <array> -// CHECK: <dict> -// CHECK: <key>line</key><integer>9</integer> -// CHECK: <key>col</key><integer>3</integer> -// CHECK: <key>file</key><integer>0</integer> -// CHECK: </dict> -// CHECK: <dict> -// CHECK: <key>line</key><integer>9</integer> -// CHECK: <key>col</key><integer>5</integer> -// CHECK: <key>file</key><integer>0</integer> -// CHECK: </dict> -// CHECK: </array> -// CHECK: <key>end</key> |