diff --git a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td --- a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td +++ b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td @@ -1267,6 +1267,12 @@ Released, Hide>, CmdLineOption, + CmdLineOption> LocAndVals, + const LocationContext *LCtx, PointerEscapeKind Kind, + const CallEvent *Call) override; + + ProgramStateRef + processPointerEscapedOnBind(ProgramStateRef State, + SVal Loc, SVal Val, + const LocationContext *LCtx); + /// Call PointerEscape callback when a value escapes as a result of /// region invalidation. /// \param[in] ITraits Specifies invalidation traits for regions/symbols. @@ -628,9 +634,10 @@ RegionAndSymbolInvalidationTraits &ITraits) override; /// A simple wrapper when you only need to notify checkers of pointer-escape - /// of a single value. - ProgramStateRef escapeValue(ProgramStateRef State, SVal V, - PointerEscapeKind K) const; + /// of some values. + ProgramStateRef escapeValues(ProgramStateRef State, ArrayRef Vs, + PointerEscapeKind K, + const CallEvent *Call = nullptr) const; public: // FIXME: 'tag' should be removed, and a LocationContext should be used diff --git a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/SubEngine.h b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/SubEngine.h --- a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/SubEngine.h +++ b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/SubEngine.h @@ -15,6 +15,7 @@ #include "clang/Analysis/ProgramPoint.h" #include "clang/StaticAnalyzer/Core/PathSensitive/SVals.h" #include "clang/StaticAnalyzer/Core/PathSensitive/Store.h" +#include "clang/StaticAnalyzer/Core/CheckerManager.h" namespace clang { @@ -148,8 +149,10 @@ return processRegionChanges(state, nullptr, MR, MR, LCtx, nullptr); } - virtual ProgramStateRef - processPointerEscapedOnBind(ProgramStateRef State, SVal Loc, SVal Val, const LocationContext *LCtx) = 0; + virtual ProgramStateRef processPointerEscapedOnBind( + ProgramStateRef State, ArrayRef> LocAndVals, + const LocationContext *LCtx, PointerEscapeKind Kind, + const CallEvent *Call) = 0; virtual ProgramStateRef notifyCheckersOfPointerEscape(ProgramStateRef State, diff --git a/clang/lib/StaticAnalyzer/Checkers/AnalysisOrderChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/AnalysisOrderChecker.cpp --- a/clang/lib/StaticAnalyzer/Checkers/AnalysisOrderChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/AnalysisOrderChecker.cpp @@ -40,6 +40,7 @@ check::EndFunction, check::NewAllocator, check::Bind, + check::PointerEscape, check::RegionChanges, check::LiveSymbols> { @@ -165,6 +166,15 @@ llvm::errs() << "RegionChanges\n"; return State; } + + ProgramStateRef checkPointerEscape(ProgramStateRef State, + const InvalidatedSymbols &Escaped, + const CallEvent *Call, + PointerEscapeKind Kind) const { + if (isCallbackEnabled(State, "PointerEscape")) + llvm::errs() << "PointerEscape\n"; + return State; + } }; } // end anonymous namespace diff --git a/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp b/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp --- a/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp +++ b/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp @@ -1172,13 +1172,16 @@ } } -ProgramStateRef ExprEngine::escapeValue(ProgramStateRef State, SVal V, - PointerEscapeKind K) const { +ProgramStateRef ExprEngine::escapeValues(ProgramStateRef State, + ArrayRef Vs, + PointerEscapeKind K, + const CallEvent *Call) const { class CollectReachableSymbolsCallback final : public SymbolVisitor { - InvalidatedSymbols Symbols; + InvalidatedSymbols &Symbols; public: - explicit CollectReachableSymbolsCallback(ProgramStateRef) {} + explicit CollectReachableSymbolsCallback(InvalidatedSymbols &Symbols) + : Symbols(Symbols) {} const InvalidatedSymbols &getSymbols() const { return Symbols; } @@ -1187,11 +1190,13 @@ return true; } }; + InvalidatedSymbols Symbols; + CollectReachableSymbolsCallback CallBack(Symbols); + for (SVal V : Vs) + State->scanReachableSymbols(V, CallBack); - const CollectReachableSymbolsCallback &Scanner = - State->scanReachableSymbols(V); return getCheckerManager().runCheckersForPointerEscape( - State, Scanner.getSymbols(), /*CallEvent*/ nullptr, K, nullptr); + State, CallBack.getSymbols(), Call, K, nullptr); } void ExprEngine::Visit(const Stmt *S, ExplodedNode *Pred, @@ -1484,7 +1489,7 @@ for (auto Child : Ex->children()) { assert(Child); SVal Val = State->getSVal(Child, LCtx); - State = escapeValue(State, Val, PSK_EscapeOther); + State = escapeValues(State, Val, PSK_EscapeOther); } Bldr2.generateNode(S, N, State); @@ -2685,33 +2690,52 @@ // destructor. We won't see the destructor during analysis, but it's there. // (4) We are binding to a MemRegion with stack storage that the store // does not understand. +ProgramStateRef ExprEngine::processPointerEscapedOnBind( + ProgramStateRef State, ArrayRef> LocAndVals, + const LocationContext *LCtx, PointerEscapeKind Kind, + const CallEvent *Call) { + SmallVector Escaped; + for (const std::pair &LocAndVal : LocAndVals) { + // Cases (1) and (2). + const MemRegion *MR = LocAndVal.first.getAsRegion(); + if (!MR || !MR->hasStackStorage()) { + Escaped.push_back(LocAndVal.second); + continue; + } + + // Case (3). + if (const auto *VR = dyn_cast(MR->getBaseRegion())) + if (VR->hasStackParametersStorage() && VR->getStackFrame()->inTopFrame()) + if (const auto *RD = VR->getValueType()->getAsCXXRecordDecl()) + if (!RD->hasTrivialDestructor()) { + Escaped.push_back(LocAndVal.second); + continue; + } + + // Case (4): in order to test that, generate a new state with the binding + // added. If it is the same state, then it escapes (since the store cannot + // represent the binding). + // Do this only if we know that the store is not supposed to generate the + // same state. + SVal StoredVal = State->getSVal(MR); + if (StoredVal != LocAndVal.second) + if (State == + (State->bindLoc(loc::MemRegionVal(MR), LocAndVal.second, LCtx))) + Escaped.push_back(LocAndVal.second); + } + + if (Escaped.empty()) + return State; + + return escapeValues(State, Escaped, Kind, Call); +} + ProgramStateRef ExprEngine::processPointerEscapedOnBind(ProgramStateRef State, SVal Loc, SVal Val, const LocationContext *LCtx) { - - // Cases (1) and (2). - const MemRegion *MR = Loc.getAsRegion(); - if (!MR || !MR->hasStackStorage()) - return escapeValue(State, Val, PSK_EscapeOnBind); - - // Case (3). - if (const auto *VR = dyn_cast(MR->getBaseRegion())) - if (VR->hasStackParametersStorage() && VR->getStackFrame()->inTopFrame()) - if (const auto *RD = VR->getValueType()->getAsCXXRecordDecl()) - if (!RD->hasTrivialDestructor()) - return escapeValue(State, Val, PSK_EscapeOnBind); - - // Case (4): in order to test that, generate a new state with the binding - // added. If it is the same state, then it escapes (since the store cannot - // represent the binding). - // Do this only if we know that the store is not supposed to generate the - // same state. - SVal StoredVal = State->getSVal(MR); - if (StoredVal != Val) - if (State == (State->bindLoc(loc::MemRegionVal(MR), Val, LCtx))) - return escapeValue(State, Val, PSK_EscapeOnBind); - - return State; + std::pair LocAndVal(Loc, Val); + return processPointerEscapedOnBind(State, LocAndVal, LCtx, PSK_EscapeOnBind, + nullptr); } ProgramStateRef diff --git a/clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp b/clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp --- a/clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp +++ b/clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp @@ -102,8 +102,8 @@ state = state->BindExpr(B, LCtx, Result); } else { // If we cannot evaluate the operation escape the operands. - state = escapeValue(state, LeftV, PSK_EscapeOther); - state = escapeValue(state, RightV, PSK_EscapeOther); + state = escapeValues(state, LeftV, PSK_EscapeOther); + state = escapeValues(state, RightV, PSK_EscapeOther); } Bldr.generateNode(B, *it, state); @@ -275,7 +275,7 @@ V = evalMinus(V); state = state->BindExpr(CastE, LCtx, V); if (V.isUnknown() && !OrigV.isUnknown()) { - state = escapeValue(state, OrigV, PSK_EscapeOther); + state = escapeValues(state, OrigV, PSK_EscapeOther); } Bldr.generateNode(CastE, Pred, state); diff --git a/clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp b/clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp --- a/clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp +++ b/clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp @@ -10,6 +10,7 @@ // //===----------------------------------------------------------------------===// +#include "clang/AST/Decl.h" #include "clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h" #include "PrettyStackTraceLocationContext.h" #include "clang/AST/CXXInheritance.h" @@ -592,9 +593,45 @@ for (auto I : dstCallEvaluated) finishArgumentConstruction(dstArgumentCleanup, I, Call); - // Finally, run any post-call checks. - getCheckerManager().runCheckersForPostCall(Dst, dstArgumentCleanup, + ExplodedNodeSet dstPostCall; + getCheckerManager().runCheckersForPostCall(dstPostCall, dstArgumentCleanup, Call, *this); + + // Escaping symbols conjured during invalidating the regions above. + // Note that, for inlined calls the nodes were put back into the worklist, + // so we can assume that every node belongs to a conservative call at this + // point. + + // Run pointerEscape callback with the newly conjured symbols. + SmallVector, 8> Escaped; + for (auto I : dstPostCall) { + NodeBuilder B(I, Dst, *currBldrCtx); + ProgramStateRef State = I->getState(); + Escaped.clear(); + { + unsigned Arg = -1; + for (const ParmVarDecl *PVD : Call.parameters()) { + ++Arg; + QualType ParamTy = PVD->getType(); + if (ParamTy.isNull() || + (!ParamTy->isPointerType() && !ParamTy->isReferenceType())) + continue; + QualType Pointee = ParamTy->getPointeeType(); + if (Pointee.isConstQualified() || Pointee->isVoidType()) + continue; + if (const MemRegion *MR = Call.getArgSVal(Arg).getAsRegion()) + Escaped.emplace_back(loc::MemRegionVal(MR), State->getSVal(MR, Pointee)); + } + } + + State = processPointerEscapedOnBind(State, Escaped, I->getLocationContext(), + PSK_EscapeOutParameters, &Call); + + if (State == I->getState()) + Dst.insert(I); + else + B.generateNode(I->getLocation(), State, I); + } } ProgramStateRef ExprEngine::bindReturnValue(const CallEvent &Call, @@ -670,8 +707,7 @@ // Conservatively evaluate call by invalidating regions and binding // a conjured return value. void ExprEngine::conservativeEvalCall(const CallEvent &Call, NodeBuilder &Bldr, - ExplodedNode *Pred, - ProgramStateRef State) { + ExplodedNode *Pred, ProgramStateRef State) { State = Call.invalidateRegions(currBldrCtx->blockCount(), State); State = bindReturnValue(Call, Pred->getLocationContext(), State); diff --git a/clang/test/Analysis/analyzer-config.c b/clang/test/Analysis/analyzer-config.c --- a/clang/test/Analysis/analyzer-config.c +++ b/clang/test/Analysis/analyzer-config.c @@ -37,6 +37,7 @@ // CHECK-NEXT: debug.AnalysisOrder:EndFunction = false // CHECK-NEXT: debug.AnalysisOrder:LiveSymbols = false // CHECK-NEXT: debug.AnalysisOrder:NewAllocator = false +// CHECK-NEXT: debug.AnalysisOrder:PointerEscape = false // CHECK-NEXT: debug.AnalysisOrder:PostCall = false // CHECK-NEXT: debug.AnalysisOrder:PostStmtArraySubscriptExpr = false // CHECK-NEXT: debug.AnalysisOrder:PostStmtCXXNewExpr = false @@ -97,4 +98,4 @@ // CHECK-NEXT: unroll-loops = false // CHECK-NEXT: widen-loops = false // CHECK-NEXT: [stats] -// CHECK-NEXT: num-entries = 94 +// CHECK-NEXT: num-entries = 95 diff --git a/clang/test/Analysis/expr-inspection.c b/clang/test/Analysis/expr-inspection.c --- a/clang/test/Analysis/expr-inspection.c +++ b/clang/test/Analysis/expr-inspection.c @@ -50,5 +50,5 @@ void test_field_dumps(struct S s, struct S *p) { clang_analyzer_dump_pointer(&s.x); // expected-warning{{&s.x}} - clang_analyzer_dump_pointer(&p->x); // expected-warning{{&SymRegion{reg_$0}.x}} + clang_analyzer_dump_pointer(&p->x); // expected-warning{{&SymRegion{reg_$1}.x}} } diff --git a/clang/test/Analysis/pointer-escape-on-conservative-calls.c b/clang/test/Analysis/pointer-escape-on-conservative-calls.c new file mode 100644 --- /dev/null +++ b/clang/test/Analysis/pointer-escape-on-conservative-calls.c @@ -0,0 +1,13 @@ +// RUN: %clang_cc1 -analyze -analyzer-checker=debug.AnalysisOrder -analyzer-config debug.AnalysisOrder:PointerEscape=true -analyzer-config debug.AnalysisOrder:PostCall=true %s 2>&1 | FileCheck %s + + +void f(int *); +int *getMem(); + +int main() { + f(getMem()); + return 0; +} + +// CHECK: PostCall (f) +// CHECK-NEXT: PointerEscape