From 852aa0d2c5d2d1faf2d77b5aa3c0848068a342c5 Mon Sep 17 00:00:00 2001 From: Jordan Rose Date: Tue, 10 Jul 2012 22:07:52 +0000 Subject: [PATCH] [analyzer] Make CallEnter, CallExitBegin, and CallExitEnd not be StmtPoints These ProgramPoints are used in inlining calls, and not all calls have associated statements anymore. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@160021 91177308-0d34-0410-b5e6-96231b3b80d8 --- include/clang/Analysis/ProgramPoint.h | 25 ++++++---- .../Checkers/MacOSKeychainAPIChecker.cpp | 8 ++-- lib/StaticAnalyzer/Checkers/MallocChecker.cpp | 16 ++++--- .../Checkers/RetainCountChecker.cpp | 9 +++- lib/StaticAnalyzer/Core/BugReporter.cpp | 46 ++++++++++++------- lib/StaticAnalyzer/Core/CoreEngine.cpp | 6 +-- lib/StaticAnalyzer/Core/ExprEngine.cpp | 2 + .../Core/ExprEngineCallAndReturn.cpp | 36 ++++++++------- lib/StaticAnalyzer/Core/PathDiagnostic.cpp | 10 +++- 9 files changed, 102 insertions(+), 56 deletions(-) diff --git a/include/clang/Analysis/ProgramPoint.h b/include/clang/Analysis/ProgramPoint.h index 4dcf92e7f2..5de06cd3a5 100644 --- a/include/clang/Analysis/ProgramPoint.h +++ b/include/clang/Analysis/ProgramPoint.h @@ -49,12 +49,12 @@ public: PostStoreKind, PostConditionKind, PostLValueKind, + MinPostStmtKind = PostStmtKind, + MaxPostStmtKind = PostLValueKind, PostInitializerKind, CallEnterKind, CallExitBeginKind, CallExitEndKind, - MinPostStmtKind = PostStmtKind, - MaxPostStmtKind = CallExitEndKind, PreImplicitCallKind, PostImplicitCallKind, MinImplicitCallKind = PreImplicitCallKind, @@ -461,11 +461,11 @@ public: }; /// Represents a point when we begin processing an inlined call. -class CallEnter : public StmtPoint { +class CallEnter : public ProgramPoint { public: CallEnter(const Stmt *stmt, const StackFrameContext *calleeCtx, const LocationContext *callerCtx) - : StmtPoint(stmt, calleeCtx, CallEnterKind, callerCtx, 0) {} + : ProgramPoint(stmt, calleeCtx, CallEnterKind, callerCtx, 0) {} const Stmt *getCallExpr() const { return static_cast(getData1()); @@ -489,11 +489,11 @@ public: /// - Bind the return value /// - Run Remove dead bindings (to clean up the dead symbols from the callee). /// - CallExitEnd -class CallExitBegin : public StmtPoint { +class CallExitBegin : public ProgramPoint { public: // CallExitBegin uses the callee's location context. - CallExitBegin(const Stmt *S, const LocationContext *L) - : StmtPoint(S, 0, CallExitBeginKind, L, 0) {} + CallExitBegin(const StackFrameContext *L) + : ProgramPoint(0, CallExitBeginKind, L, 0) {} static bool classof(const ProgramPoint *Location) { return Location->getKind() == CallExitBeginKind; @@ -502,11 +502,16 @@ public: /// Represents a point when we finish the call exit sequence (for inlined call). /// \sa CallExitBegin -class CallExitEnd : public StmtPoint { +class CallExitEnd : public ProgramPoint { public: // CallExitEnd uses the caller's location context. - CallExitEnd(const Stmt *S, const LocationContext *L) - : StmtPoint(S, 0, CallExitEndKind, L, 0) {} + CallExitEnd(const StackFrameContext *CalleeCtx, + const LocationContext *CallerCtx) + : ProgramPoint(CalleeCtx, CallExitEndKind, CallerCtx, 0) {} + + const StackFrameContext *getCalleeContext() const { + return static_cast(getData1()); + } static bool classof(const ProgramPoint *Location) { return Location->getKind() == CallExitEndKind; diff --git a/lib/StaticAnalyzer/Checkers/MacOSKeychainAPIChecker.cpp b/lib/StaticAnalyzer/Checkers/MacOSKeychainAPIChecker.cpp index cb976e03a5..ac41f67d43 100644 --- a/lib/StaticAnalyzer/Checkers/MacOSKeychainAPIChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/MacOSKeychainAPIChecker.cpp @@ -528,9 +528,11 @@ MacOSKeychainAPIChecker::getAllocationSite(const ExplodedNode *N, } ProgramPoint P = AllocNode->getLocation(); - if (!isa(P)) - return 0; - return cast(P).getStmt(); + if (CallExitEnd *Exit = dyn_cast(&P)) + return Exit->getCalleeContext()->getCallSite(); + if (clang::PostStmt *PS = dyn_cast(&P)) + return PS->getStmt(); + return 0; } BugReport *MacOSKeychainAPIChecker:: diff --git a/lib/StaticAnalyzer/Checkers/MallocChecker.cpp b/lib/StaticAnalyzer/Checkers/MallocChecker.cpp index 8c2e5295ee..f9deb72336 100644 --- a/lib/StaticAnalyzer/Checkers/MallocChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/MallocChecker.cpp @@ -972,8 +972,10 @@ MallocChecker::getAllocationSite(const ExplodedNode *N, SymbolRef Sym, ProgramPoint P = AllocNode->getLocation(); const Stmt *AllocationStmt = 0; - if (isa(P)) - AllocationStmt = cast(P).getStmt(); + if (CallExitEnd *Exit = dyn_cast(&P)) + AllocationStmt = Exit->getCalleeContext()->getCallSite(); + else if (StmtPoint *SP = dyn_cast(&P)) + AllocationStmt = SP->getStmt(); return LeakInfo(AllocationStmt, ReferenceRegion); } @@ -1524,12 +1526,14 @@ MallocChecker::MallocBugVisitor::VisitNode(const ExplodedNode *N, // Retrieve the associated statement. ProgramPoint ProgLoc = N->getLocation(); - if (isa(ProgLoc)) - S = cast(ProgLoc).getStmt(); + if (StmtPoint *SP = dyn_cast(&ProgLoc)) + S = SP->getStmt(); + else if (CallExitEnd *Exit = dyn_cast(&ProgLoc)) + S = Exit->getCalleeContext()->getCallSite(); // If an assumption was made on a branch, it should be caught // here by looking at the state transition. - if (isa(ProgLoc)) { - const CFGBlock *srcBlk = cast(ProgLoc).getSrc(); + else if (BlockEdge *Edge = dyn_cast(&ProgLoc)) { + const CFGBlock *srcBlk = Edge->getSrc(); S = srcBlk->getTerminator(); } if (!S) diff --git a/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp b/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp index 541a8b7c15..a156f0dc45 100644 --- a/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp @@ -2352,8 +2352,15 @@ CFRefLeakReport::CFRefLeakReport(CFRefBug &D, const LangOptions &LOpts, GetAllocationSite(Ctx.getStateManager(), getErrorNode(), sym); // Get the SourceLocation for the allocation site. + // FIXME: This will crash the analyzer if an allocation comes from an + // implicit call. (Currently there are no such allocations in Cocoa, though.) + const Stmt *AllocStmt; ProgramPoint P = AllocNode->getLocation(); - const Stmt *AllocStmt = cast(P).getStmt(); + if (CallExitEnd *Exit = dyn_cast(&P)) + AllocStmt = Exit->getCalleeContext()->getCallSite(); + else + AllocStmt = cast(P).getStmt(); + assert(AllocStmt && "All allocations must come from explicit calls"); Location = PathDiagnosticLocation::createBegin(AllocStmt, SMgr, n->getLocationContext()); // Fill in the description of the bug. diff --git a/lib/StaticAnalyzer/Core/BugReporter.cpp b/lib/StaticAnalyzer/Core/BugReporter.cpp index 14fcb179cc..08588f60ab 100644 --- a/lib/StaticAnalyzer/Core/BugReporter.cpp +++ b/lib/StaticAnalyzer/Core/BugReporter.cpp @@ -48,6 +48,10 @@ static inline const Stmt *GetStmt(const ProgramPoint &P) { return SP->getStmt(); else if (const BlockEdge *BE = dyn_cast(&P)) return BE->getSrc()->getTerminator(); + else if (const CallEnter *CE = dyn_cast(&P)) + return CE->getCallExpr(); + else if (const CallExitEnd *CEE = dyn_cast(&P)) + return CEE->getCalleeContext()->getCallSite(); return 0; } @@ -1102,11 +1106,10 @@ static void reversePropagateInterestingSymbols(BugReport &R, const LocationContext *CalleeCtx, const LocationContext *CallerCtx) { - // FIXME: Handle CXXConstructExpr. - // FIXME: Handle calls to blocks. + // FIXME: Handle non-CallExpr-based CallEvents. const StackFrameContext *Callee = CalleeCtx->getCurrentStackFrame(); const Stmt *CallSite = Callee->getCallSite(); - if (const CallExpr *CE = dyn_cast(CallSite)) { + if (const CallExpr *CE = dyn_cast_or_null(CallSite)) { if (const FunctionDecl *FD = dyn_cast(CalleeCtx->getDecl())) { FunctionDecl::param_const_iterator PI = FD->param_begin(), PE = FD->param_end(); @@ -1149,17 +1152,24 @@ static void GenerateExtensivePathDiagnostic(PathDiagnostic& PD, if (const CallExitEnd *CE = dyn_cast(&P)) { const StackFrameContext *LCtx = - CE->getLocationContext()->getCurrentStackFrame(); - PathDiagnosticLocation Loc(CE->getStmt(), - PDB.getSourceManager(), - LCtx); - EB.addEdge(Loc, true); - EB.flushLocations(); - PathDiagnosticCallPiece *C = - PathDiagnosticCallPiece::construct(N, *CE, SM); - PD.getActivePath().push_front(C); - PD.pushActivePath(&C->path); - CallStack.push_back(StackDiagPair(C, N)); + CE->getLocationContext()->getCurrentStackFrame(); + // FIXME: This needs to handle implicit calls. + if (const Stmt *S = CE->getCalleeContext()->getCallSite()) { + if (const Expr *Ex = dyn_cast(S)) + reversePropagateIntererstingSymbols(*PDB.getBugReport(), IE, + N->getState().getPtr(), Ex, + N->getLocationContext()); + PathDiagnosticLocation Loc(S, + PDB.getSourceManager(), + LCtx); + EB.addEdge(Loc, true); + EB.flushLocations(); + PathDiagnosticCallPiece *C = + PathDiagnosticCallPiece::construct(N, *CE, SM); + PD.getActivePath().push_front(C); + PD.pushActivePath(&C->path); + CallStack.push_back(StackDiagPair(C, N)); + } break; } @@ -1191,8 +1201,12 @@ static void GenerateExtensivePathDiagnostic(PathDiagnostic& PD, const Decl * Caller = CE->getLocationContext()->getDecl(); C = PathDiagnosticCallPiece::construct(PD.getActivePath(), Caller); } - C->setCallee(*CE, SM); - EB.addContext(CE->getCallExpr()); + + // FIXME: We still need a location for implicit calls. + if (CE->getCallExpr()) { + C->setCallee(*CE, SM); + EB.addContext(CE->getCallExpr()); + } if (!CallStack.empty()) { assert(CallStack.back().first == C); diff --git a/lib/StaticAnalyzer/Core/CoreEngine.cpp b/lib/StaticAnalyzer/Core/CoreEngine.cpp index 689f05714f..d7a4baa0a8 100644 --- a/lib/StaticAnalyzer/Core/CoreEngine.cpp +++ b/lib/StaticAnalyzer/Core/CoreEngine.cpp @@ -265,7 +265,8 @@ void CoreEngine::dispatchWorkItem(ExplodedNode* Pred, ProgramPoint Loc, } default: assert(isa(Loc) || - isa(Loc)); + isa(Loc) || + isa(Loc)); HandlePostStmt(WU.getBlock(), WU.getIndex(), Pred); break; } @@ -539,10 +540,9 @@ ExplodedNode *CoreEngine::generateCallExitBeginNode(ExplodedNode *N) { // Create a CallExitBegin node and enqueue it. const StackFrameContext *LocCtx = cast(N->getLocationContext()); - const Stmt *CE = LocCtx->getCallSite(); // Use the the callee location context. - CallExitBegin Loc(CE, LocCtx); + CallExitBegin Loc(LocCtx); bool isNew; ExplodedNode *Node = G->getNode(Loc, N->getState(), false, &isNew); diff --git a/lib/StaticAnalyzer/Core/ExprEngine.cpp b/lib/StaticAnalyzer/Core/ExprEngine.cpp index a9387694ee..5e0a338084 100644 --- a/lib/StaticAnalyzer/Core/ExprEngine.cpp +++ b/lib/StaticAnalyzer/Core/ExprEngine.cpp @@ -1001,6 +1001,8 @@ bool ExprEngine::replayWithoutInlining(ExplodedNode *N, continue; if (isa(&L)) continue; + if (isa(&L)) + continue; if (const StmtPoint *SP = dyn_cast(&L)) if (SP->getStmt() == CE) continue; diff --git a/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp b/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp index cce55ea023..92497dbb1e 100644 --- a/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp +++ b/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp @@ -134,23 +134,25 @@ void ExprEngine::processCallExit(ExplodedNode *CEBNode) { const CFGBlock *Blk = 0; llvm::tie(LastSt, Blk) = getLastStmt(CEBNode); - // Step 2: generate node with binded return value: CEBNode -> BindedRetNode. + // Step 2: generate node with bound return value: CEBNode -> BindedRetNode. // If the callee returns an expression, bind its value to CallExpr. - if (const ReturnStmt *RS = dyn_cast_or_null(LastSt)) { - const LocationContext *LCtx = CEBNode->getLocationContext(); - SVal V = state->getSVal(RS, LCtx); - state = state->BindExpr(CE, callerCtx, V); - } + if (CE) { + if (const ReturnStmt *RS = dyn_cast_or_null(LastSt)) { + const LocationContext *LCtx = CEBNode->getLocationContext(); + SVal V = state->getSVal(RS, LCtx); + state = state->BindExpr(CE, callerCtx, V); + } - // Bind the constructed object value to CXXConstructExpr. - if (const CXXConstructExpr *CCE = dyn_cast(CE)) { - loc::MemRegionVal This = - svalBuilder.getCXXThis(CCE->getConstructor()->getParent(), calleeCtx); - SVal ThisV = state->getSVal(This); + // Bind the constructed object value to CXXConstructExpr. + if (const CXXConstructExpr *CCE = dyn_cast(CE)) { + loc::MemRegionVal This = + svalBuilder.getCXXThis(CCE->getConstructor()->getParent(), calleeCtx); + SVal ThisV = state->getSVal(This); - // Always bind the region to the CXXConstructExpr. - state = state->BindExpr(CCE, CEBNode->getLocationContext(), ThisV); + // Always bind the region to the CXXConstructExpr. + state = state->BindExpr(CCE, CEBNode->getLocationContext(), ThisV); + } } static SimpleProgramPointTag retValBindTag("ExprEngine : Bind Return Value"); @@ -186,7 +188,7 @@ void ExprEngine::processCallExit(ExplodedNode *CEBNode) { // Step 4: Generate the CallExit and leave the callee's context. // CleanedNodes -> CEENode - CallExitEnd Loc(CE, callerCtx); + CallExitEnd Loc(calleeCtx, callerCtx); bool isNew; ExplodedNode *CEENode = G.getNode(Loc, (*I)->getState(), false, &isNew); CEENode->addPredecessor(*I, G); @@ -202,7 +204,9 @@ void ExprEngine::processCallExit(ExplodedNode *CEBNode) { &Ctx); SaveAndRestore CBISave(currentStmtIdx, calleeCtx->getIndex()); - getCheckerManager().runCheckersForPostStmt(Dst, CEENode, CE, *this, true); + // FIXME: This needs to call PostCall. + if (CE) + getCheckerManager().runCheckersForPostStmt(Dst, CEENode, CE, *this, true); // Enqueue the next element in the block. for (ExplodedNodeSet::iterator PSI = Dst.begin(), PSE = Dst.end(); @@ -322,8 +326,8 @@ bool ExprEngine::inlineCall(ExplodedNodeSet &Dst, if (!ParentOfCallee) ParentOfCallee = CallerSFC; + // This may be NULL, but that's fine. const Expr *CallE = Call.getOriginExpr(); - assert(CallE && "It is not yet possible to have calls without statements"); // Construct a new stack frame for the callee. AnalysisDeclContext *CalleeADC = AMgr.getAnalysisDeclContext(D); diff --git a/lib/StaticAnalyzer/Core/PathDiagnostic.cpp b/lib/StaticAnalyzer/Core/PathDiagnostic.cpp index a1e662f129..bfb8bf6432 100644 --- a/lib/StaticAnalyzer/Core/PathDiagnostic.cpp +++ b/lib/StaticAnalyzer/Core/PathDiagnostic.cpp @@ -339,6 +339,9 @@ PathDiagnosticLocation else if (const PostStmt *PS = dyn_cast(&P)) { S = PS->getStmt(); } + else if (const CallExitEnd *CEE = dyn_cast(&P)) { + S = CEE->getCalleeContext()->getCallSite(); + } return PathDiagnosticLocation(S, SMng, P.getLocationContext()); } @@ -501,6 +504,10 @@ static PathDiagnosticLocation getLastStmtLoc(const ExplodedNode *N, ProgramPoint PP = N->getLocation(); if (const StmtPoint *SP = dyn_cast(&PP)) return PathDiagnosticLocation(SP->getStmt(), SM, PP.getLocationContext()); + // FIXME: Handle implicit calls. + if (const CallExitEnd *CEE = dyn_cast(&PP)) + if (const Stmt *CallSite = CEE->getCalleeContext()->getCallSite()) + return PathDiagnosticLocation(CallSite, SM, PP.getLocationContext()); if (N->pred_empty()) break; N = *N->pred_begin(); @@ -670,7 +677,8 @@ std::string StackHintGeneratorForSymbol::getMessage(const ExplodedNode *N){ const CallExitEnd *CExit = dyn_cast(&P); assert(CExit && "Stack Hints should be constructed at CallExitEnd points."); - const CallExpr *CE = dyn_cast_or_null(CExit->getStmt()); + const Stmt *CallSite = CExit->getCalleeContext()->getCallSite(); + const CallExpr *CE = dyn_cast_or_null(CallSite); if (!CE) return ""; -- 2.40.0