[analyzer] Make Malloc Checker optimistic in presence of inlining.
(In response of Ted's review of r150112.)
This moves the logic which checked if a symbol escapes through a
parameter to invalidateRegionCallback (instead of post CallExpr visit.)
To accommodate the change, added a CallOrObjCMessage parameter to
checkRegionChanges callback.
git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@150513 91177308-0d34-0410-b5e6-96231b3b80d8
diff --git a/include/clang/StaticAnalyzer/Core/Checker.h b/include/clang/StaticAnalyzer/Core/Checker.h
index 1e90555..76d8c15 100644
--- a/include/clang/StaticAnalyzer/Core/Checker.h
+++ b/include/clang/StaticAnalyzer/Core/Checker.h
@@ -265,9 +265,10 @@
ProgramStateRef state,
const StoreManager::InvalidatedSymbols *invalidated,
ArrayRef<const MemRegion *> Explicits,
- ArrayRef<const MemRegion *> Regions) {
+ ArrayRef<const MemRegion *> Regions,
+ const CallOrObjCMessage *Call) {
return ((const CHECKER *)checker)->checkRegionChanges(state, invalidated,
- Explicits, Regions);
+ Explicits, Regions, Call);
}
template <typename CHECKER>
static bool _wantsRegionChangeUpdate(void *checker,
diff --git a/include/clang/StaticAnalyzer/Core/CheckerManager.h b/include/clang/StaticAnalyzer/Core/CheckerManager.h
index a404657..fa22f53 100644
--- a/include/clang/StaticAnalyzer/Core/CheckerManager.h
+++ b/include/clang/StaticAnalyzer/Core/CheckerManager.h
@@ -52,6 +52,19 @@
template <typename T> class CheckerFn;
+template <typename RET, typename P1, typename P2, typename P3, typename P4,
+ typename P5>
+class CheckerFn<RET(P1, P2, P3, P4, P5)> {
+ typedef RET (*Func)(void *, P1, P2, P3, P4, P5);
+ Func Fn;
+public:
+ CheckerBase *Checker;
+ CheckerFn(CheckerBase *checker, Func fn) : Fn(fn), Checker(checker) { }
+ RET operator()(P1 p1, P2 p2, P3 p3, P4 p4, P5 p5) const {
+ return Fn(Checker, p1, p2, p3, p4, p5);
+ }
+};
+
template <typename RET, typename P1, typename P2, typename P3, typename P4>
class CheckerFn<RET(P1, P2, P3, P4)> {
typedef RET (*Func)(void *, P1, P2, P3, P4);
@@ -269,11 +282,14 @@
/// For example, in the case of a function call, these would be arguments.
/// \param Regions The transitive closure of accessible regions,
/// i.e. all regions that may have been touched by this change.
+ /// \param The call expression wrapper if the regions are invalidated by a
+ /// call.
ProgramStateRef
runCheckersForRegionChanges(ProgramStateRef state,
const StoreManager::InvalidatedSymbols *invalidated,
ArrayRef<const MemRegion *> ExplicitRegions,
- ArrayRef<const MemRegion *> Regions);
+ ArrayRef<const MemRegion *> Regions,
+ const CallOrObjCMessage *Call);
/// \brief Run checkers for handling assumptions on symbolic values.
ProgramStateRef runCheckersForEvalAssume(ProgramStateRef state,
@@ -349,8 +365,9 @@
typedef CheckerFn<ProgramStateRef (ProgramStateRef,
const StoreManager::InvalidatedSymbols *symbols,
- ArrayRef<const MemRegion *> ExplicitRegions,
- ArrayRef<const MemRegion *> Regions)>
+ ArrayRef<const MemRegion *> ExplicitRegions,
+ ArrayRef<const MemRegion *> Regions,
+ const CallOrObjCMessage *Call)>
CheckRegionChangesFunc;
typedef CheckerFn<bool (ProgramStateRef)> WantsRegionChangeUpdateFunc;
diff --git a/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h b/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
index f1a5d88..b7c4958 100644
--- a/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
+++ b/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
@@ -212,7 +212,8 @@
processRegionChanges(ProgramStateRef state,
const StoreManager::InvalidatedSymbols *invalidated,
ArrayRef<const MemRegion *> ExplicitRegions,
- ArrayRef<const MemRegion *> Regions);
+ ArrayRef<const MemRegion *> Regions,
+ const CallOrObjCMessage *Call);
/// printState - Called by ProgramStateManager to print checker-specific data.
void printState(raw_ostream &Out, ProgramStateRef State,
diff --git a/include/clang/StaticAnalyzer/Core/PathSensitive/SubEngine.h b/include/clang/StaticAnalyzer/Core/PathSensitive/SubEngine.h
index d252da1..0fb04b8 100644
--- a/include/clang/StaticAnalyzer/Core/PathSensitive/SubEngine.h
+++ b/include/clang/StaticAnalyzer/Core/PathSensitive/SubEngine.h
@@ -97,19 +97,20 @@
/// region change should trigger a processRegionChanges update.
virtual bool wantsRegionChangeUpdate(ProgramStateRef state) = 0;
- /// processRegionChanges - Called by ProgramStateManager whenever a change is made
- /// to the store. Used to update checkers that track region values.
+ /// processRegionChanges - Called by ProgramStateManager whenever a change is
+ /// made to the store. Used to update checkers that track region values.
virtual ProgramStateRef
processRegionChanges(ProgramStateRef state,
const StoreManager::InvalidatedSymbols *invalidated,
ArrayRef<const MemRegion *> ExplicitRegions,
- ArrayRef<const MemRegion *> Regions) = 0;
+ ArrayRef<const MemRegion *> Regions,
+ const CallOrObjCMessage *Call) = 0;
inline ProgramStateRef
processRegionChange(ProgramStateRef state,
const MemRegion* MR) {
- return processRegionChanges(state, 0, MR, MR);
+ return processRegionChanges(state, 0, MR, MR, 0);
}
/// printState - Called by ProgramStateManager to print checker-specific data.
diff --git a/lib/StaticAnalyzer/Checkers/CStringChecker.cpp b/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
index 748fc43..dcc62a6 100644
--- a/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
+++ b/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
@@ -64,7 +64,8 @@
checkRegionChanges(ProgramStateRef state,
const StoreManager::InvalidatedSymbols *,
ArrayRef<const MemRegion *> ExplicitRegions,
- ArrayRef<const MemRegion *> Regions) const;
+ ArrayRef<const MemRegion *> Regions,
+ const CallOrObjCMessage *Call) const;
typedef void (CStringChecker::*FnCheck)(CheckerContext &,
const CallExpr *) const;
@@ -1807,7 +1808,8 @@
CStringChecker::checkRegionChanges(ProgramStateRef state,
const StoreManager::InvalidatedSymbols *,
ArrayRef<const MemRegion *> ExplicitRegions,
- ArrayRef<const MemRegion *> Regions) const {
+ ArrayRef<const MemRegion *> Regions,
+ const CallOrObjCMessage *Call) const {
CStringLength::EntryMap Entries = state->get<CStringLength>();
if (Entries.isEmpty())
return state;
diff --git a/lib/StaticAnalyzer/Checkers/CheckerDocumentation.cpp b/lib/StaticAnalyzer/Checkers/CheckerDocumentation.cpp
index cd31ae3..a5f9d30 100644
--- a/lib/StaticAnalyzer/Checkers/CheckerDocumentation.cpp
+++ b/lib/StaticAnalyzer/Checkers/CheckerDocumentation.cpp
@@ -184,13 +184,27 @@
/// check::LiveSymbols
void checkLiveSymbols(ProgramStateRef State, SymbolReaper &SR) const {}
- /// check::RegionChanges
+
bool wantsRegionChangeUpdate(ProgramStateRef St) const { return true; }
+
+ /// check::RegionChanges
+ /// Allows tracking regions which get invalidated.
+ /// \param state The current program state.
+ /// \param invalidated A set of all symbols potentially touched by the change.
+ /// \param ExplicitRegions The regions explicitly requested for invalidation.
+ /// For example, in the case of a function call, these would be arguments.
+ /// \param Regions The transitive closure of accessible regions,
+ /// i.e. all regions that may have been touched by this change.
+ /// \param The call expression wrapper if the regions are invalidated by a
+ /// call, 0 otherwise.
+ /// Note, in order to be notified, the checker should also implement
+ /// wantsRegionChangeUpdate callback.
ProgramStateRef
checkRegionChanges(ProgramStateRef State,
const StoreManager::InvalidatedSymbols *,
ArrayRef<const MemRegion *> ExplicitRegions,
- ArrayRef<const MemRegion *> Regions) const {
+ ArrayRef<const MemRegion *> Regions,
+ const CallOrObjCMessage *Call) const {
return State;
}
diff --git a/lib/StaticAnalyzer/Checkers/MallocChecker.cpp b/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
index 7cbb49e..d57e8a9 100644
--- a/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
+++ b/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
@@ -17,6 +17,7 @@
#include "clang/StaticAnalyzer/Core/CheckerManager.h"
#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
#include "clang/StaticAnalyzer/Core/BugReporter/BugType.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/ObjCMessage.h"
#include "clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h"
#include "clang/StaticAnalyzer/Core/PathSensitive/ProgramStateTrait.h"
#include "clang/StaticAnalyzer/Core/PathSensitive/SymbolManager.h"
@@ -69,6 +70,7 @@
class MallocChecker : public Checker<check::DeadSymbols,
check::EndPath,
check::PreStmt<ReturnStmt>,
+ check::PreStmt<CallExpr>,
check::PostStmt<CallExpr>,
check::Location,
check::Bind,
@@ -94,8 +96,7 @@
ChecksFilter Filter;
- void initIdentifierInfo(CheckerContext &C) const;
-
+ void checkPreStmt(const CallExpr *S, CheckerContext &C) const;
void checkPostStmt(const CallExpr *CE, CheckerContext &C) const;
void checkDeadSymbols(SymbolReaper &SymReaper, CheckerContext &C) const;
void checkEndPath(CheckerContext &C) const;
@@ -110,12 +111,19 @@
checkRegionChanges(ProgramStateRef state,
const StoreManager::InvalidatedSymbols *invalidated,
ArrayRef<const MemRegion *> ExplicitRegions,
- ArrayRef<const MemRegion *> Regions) const;
+ ArrayRef<const MemRegion *> Regions,
+ const CallOrObjCMessage *Call) const;
bool wantsRegionChangeUpdate(ProgramStateRef state) const {
return true;
}
private:
+ void initIdentifierInfo(ASTContext &C) const;
+
+ /// Check if this is one of the functions which can allocate/reallocate memory
+ /// pointed to by one of its arguments.
+ bool isMemFunction(const FunctionDecl *FD, ASTContext &C) const;
+
static void MallocMem(CheckerContext &C, const CallExpr *CE);
static void MallocMemReturnsAttr(CheckerContext &C, const CallExpr *CE,
const OwnershipAttr* Att);
@@ -144,6 +152,10 @@
bool checkUseAfterFree(SymbolRef Sym, CheckerContext &C,
const Stmt *S = 0) const;
+ /// Check if the function is not known to us. So, for example, we could
+ /// conservatively assume it can free/reallocate it's pointer arguments.
+ bool hasUnknownBehavior(const FunctionDecl *FD, ProgramStateRef State) const;
+
static bool SummarizeValue(raw_ostream &os, SVal V);
static bool SummarizeRegion(raw_ostream &os, const MemRegion *MR);
void ReportBadFree(CheckerContext &C, SVal ArgVal, SourceRange range) const;
@@ -220,8 +232,7 @@
};
} // end anonymous namespace
-void MallocChecker::initIdentifierInfo(CheckerContext &C) const {
- ASTContext &Ctx = C.getASTContext();
+void MallocChecker::initIdentifierInfo(ASTContext &Ctx) const {
if (!II_malloc)
II_malloc = &Ctx.Idents.get("malloc");
if (!II_free)
@@ -232,11 +243,31 @@
II_calloc = &Ctx.Idents.get("calloc");
}
+bool MallocChecker::isMemFunction(const FunctionDecl *FD, ASTContext &C) const {
+ initIdentifierInfo(C);
+ IdentifierInfo *FunI = FD->getIdentifier();
+ if (!FunI)
+ return false;
+
+ // TODO: Add more here : ex: reallocf!
+ if (FunI == II_malloc || FunI == II_free ||
+ FunI == II_realloc || FunI == II_calloc)
+ return true;
+
+ if (Filter.CMallocOptimistic && FD->hasAttrs() &&
+ FD->specific_attr_begin<OwnershipAttr>() !=
+ FD->specific_attr_end<OwnershipAttr>())
+ return true;
+
+
+ return false;
+}
+
void MallocChecker::checkPostStmt(const CallExpr *CE, CheckerContext &C) const {
const FunctionDecl *FD = C.getCalleeDecl(CE);
if (!FD)
return;
- initIdentifierInfo(C);
+ initIdentifierInfo(C.getASTContext());
if (FD->getIdentifier() == II_malloc) {
MallocMem(C, CE);
@@ -278,42 +309,6 @@
}
}
}
-
- // Check use after free, when a freed pointer is passed to a call.
- ProgramStateRef State = C.getState();
- for (CallExpr::const_arg_iterator I = CE->arg_begin(),
- E = CE->arg_end(); I != E; ++I) {
- const Expr *A = *I;
- if (A->getType().getTypePtr()->isAnyPointerType()) {
- SymbolRef Sym = State->getSVal(A, C.getLocationContext()).getAsSymbol();
- if (!Sym)
- continue;
- if (checkUseAfterFree(Sym, C, A))
- return;
- }
- }
-
- // The pointer might escape through a function call.
- // TODO: This should be rewritten to take into account inlining.
- if (Filter.CMallocPessimistic) {
- SourceLocation FLoc = FD->getLocation();
- // We assume that the pointers cannot escape through calls to system
- // functions.
- if (C.getSourceManager().isInSystemHeader(FLoc))
- return;
-
- ProgramStateRef State = C.getState();
- for (CallExpr::const_arg_iterator I = CE->arg_begin(),
- E = CE->arg_end(); I != E; ++I) {
- const Expr *A = *I;
- if (A->getType().getTypePtr()->isAnyPointerType()) {
- SymbolRef Sym = State->getSVal(A, C.getLocationContext()).getAsSymbol();
- if (!Sym)
- continue;
- checkEscape(Sym, A, C);
- }
- }
- }
}
void MallocChecker::MallocMem(CheckerContext &C, const CallExpr *CE) {
@@ -802,6 +797,25 @@
return false;
}
+void MallocChecker::checkPreStmt(const CallExpr *CE, CheckerContext &C) const {
+ if (isMemFunction(C.getCalleeDecl(CE), C.getASTContext()))
+ return;
+
+ // Check use after free, when a freed pointer is passed to a call.
+ ProgramStateRef State = C.getState();
+ for (CallExpr::const_arg_iterator I = CE->arg_begin(),
+ E = CE->arg_end(); I != E; ++I) {
+ const Expr *A = *I;
+ if (A->getType().getTypePtr()->isAnyPointerType()) {
+ SymbolRef Sym = State->getSVal(A, C.getLocationContext()).getAsSymbol();
+ if (!Sym)
+ continue;
+ if (checkUseAfterFree(Sym, C, A))
+ return;
+ }
+ }
+}
+
void MallocChecker::checkPreStmt(const ReturnStmt *S, CheckerContext &C) const {
const Expr *E = S->getRetValue();
if (!E)
@@ -926,22 +940,54 @@
return state;
}
+// Check if the function is not known to us. So, for example, we could
+// conservatively assume it can free/reallocate it's pointer arguments.
+// (We assume that the pointers cannot escape through calls to system
+// functions not handled by this checker.)
+bool MallocChecker::hasUnknownBehavior(const FunctionDecl *FD,
+ ProgramStateRef State) const {
+ ASTContext &ASTC = State->getStateManager().getContext();
+
+ // If it's one of the allocation functions we can reason about, we model it's
+ // behavior explicitly.
+ if (isMemFunction(FD, ASTC)) {
+ return false;
+ }
+
+ // If it's a system call, we know it does not free the memory.
+ SourceManager &SM = ASTC.getSourceManager();
+ if (SM.isInSystemHeader(FD->getLocation())) {
+ return false;
+ }
+
+ // Otherwise, assume that the function can free memory.
+ return true;
+}
+
// If the symbol we are tracking is invalidated, but not explicitly (ex: the &p
// escapes, when we are tracking p), do not track the symbol as we cannot reason
// about it anymore.
ProgramStateRef
-MallocChecker::checkRegionChanges(ProgramStateRef state,
+MallocChecker::checkRegionChanges(ProgramStateRef State,
const StoreManager::InvalidatedSymbols *invalidated,
ArrayRef<const MemRegion *> ExplicitRegions,
- ArrayRef<const MemRegion *> Regions) const {
+ ArrayRef<const MemRegion *> Regions,
+ const CallOrObjCMessage *Call) const {
if (!invalidated)
- return state;
-
+ return State;
llvm::SmallPtrSet<SymbolRef, 8> WhitelistedSymbols;
- for (ArrayRef<const MemRegion *>::iterator I = ExplicitRegions.begin(),
- E = ExplicitRegions.end(); I != E; ++I) {
- if (const SymbolicRegion *SR = (*I)->StripCasts()->getAs<SymbolicRegion>())
- WhitelistedSymbols.insert(SR->getSymbol());
+
+ const FunctionDecl *FD = (Call ? dyn_cast<FunctionDecl>(Call->getDecl()) : 0);
+
+ // If it's a call which might free or reallocate memory, we assume that all
+ // regions (explicit and implicit) escaped. Otherwise, whitelist explicit
+ // pointers; we still can track them.
+ if (!(FD && hasUnknownBehavior(FD, State))) {
+ for (ArrayRef<const MemRegion *>::iterator I = ExplicitRegions.begin(),
+ E = ExplicitRegions.end(); I != E; ++I) {
+ if (const SymbolicRegion *R = (*I)->StripCasts()->getAs<SymbolicRegion>())
+ WhitelistedSymbols.insert(R->getSymbol());
+ }
}
for (StoreManager::InvalidatedSymbols::const_iterator I=invalidated->begin(),
@@ -949,10 +995,11 @@
SymbolRef sym = *I;
if (WhitelistedSymbols.count(sym))
continue;
- // Don't track the symbol.
- state = state->remove<RegionState>(sym);
+ // The symbol escaped.
+ if (const RefState *RS = State->get<RegionState>(sym))
+ State = State->set<RegionState>(sym, RefState::getEscaped(RS->getStmt()));
}
- return state;
+ return State;
}
PathDiagnosticPiece *
diff --git a/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp b/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp
index 6678a0b..96cc2f4 100644
--- a/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp
+++ b/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp
@@ -2439,7 +2439,8 @@
checkRegionChanges(ProgramStateRef state,
const StoreManager::InvalidatedSymbols *invalidated,
ArrayRef<const MemRegion *> ExplicitRegions,
- ArrayRef<const MemRegion *> Regions) const;
+ ArrayRef<const MemRegion *> Regions,
+ const CallOrObjCMessage *Call) const;
bool wantsRegionChangeUpdate(ProgramStateRef state) const {
return true;
@@ -3303,7 +3304,8 @@
RetainCountChecker::checkRegionChanges(ProgramStateRef state,
const StoreManager::InvalidatedSymbols *invalidated,
ArrayRef<const MemRegion *> ExplicitRegions,
- ArrayRef<const MemRegion *> Regions) const {
+ ArrayRef<const MemRegion *> Regions,
+ const CallOrObjCMessage *Call) const {
if (!invalidated)
return state;
diff --git a/lib/StaticAnalyzer/Core/CheckerManager.cpp b/lib/StaticAnalyzer/Core/CheckerManager.cpp
index 732a821..cb4da7c 100644
--- a/lib/StaticAnalyzer/Core/CheckerManager.cpp
+++ b/lib/StaticAnalyzer/Core/CheckerManager.cpp
@@ -413,14 +413,15 @@
CheckerManager::runCheckersForRegionChanges(ProgramStateRef state,
const StoreManager::InvalidatedSymbols *invalidated,
ArrayRef<const MemRegion *> ExplicitRegions,
- ArrayRef<const MemRegion *> Regions) {
+ ArrayRef<const MemRegion *> Regions,
+ const CallOrObjCMessage *Call) {
for (unsigned i = 0, e = RegionChangesCheckers.size(); i != e; ++i) {
// If any checker declares the state infeasible (or if it starts that way),
// bail out.
if (!state)
return NULL;
state = RegionChangesCheckers[i].CheckFn(state, invalidated,
- ExplicitRegions, Regions);
+ ExplicitRegions, Regions, Call);
}
return state;
}
diff --git a/lib/StaticAnalyzer/Core/ExprEngine.cpp b/lib/StaticAnalyzer/Core/ExprEngine.cpp
index 9e5d6a9..b0ed181 100644
--- a/lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ b/lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -176,9 +176,10 @@
ExprEngine::processRegionChanges(ProgramStateRef state,
const StoreManager::InvalidatedSymbols *invalidated,
ArrayRef<const MemRegion *> Explicits,
- ArrayRef<const MemRegion *> Regions) {
+ ArrayRef<const MemRegion *> Regions,
+ const CallOrObjCMessage *Call) {
return getCheckerManager().runCheckersForRegionChanges(state, invalidated,
- Explicits, Regions);
+ Explicits, Regions, Call);
}
void ExprEngine::printState(raw_ostream &Out, ProgramStateRef State,
diff --git a/lib/StaticAnalyzer/Core/ProgramState.cpp b/lib/StaticAnalyzer/Core/ProgramState.cpp
index 459bf83..f52369e 100644
--- a/lib/StaticAnalyzer/Core/ProgramState.cpp
+++ b/lib/StaticAnalyzer/Core/ProgramState.cpp
@@ -179,7 +179,7 @@
= Mgr.StoreMgr->invalidateRegions(getStore(), Regions, E, Count, IS,
Call, &Invalidated);
ProgramStateRef newState = makeWithStore(newStore);
- return Eng->processRegionChanges(newState, &IS, Regions, Invalidated);
+ return Eng->processRegionChanges(newState, &IS, Regions, Invalidated, Call);
}
const StoreRef &newStore =
