[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.

llvm-svn: 150513
This commit is contained in:
Anna Zaks 2012-02-14 21:55:24 +00:00
parent 78f815d3a4
commit 3d34834bb0
11 changed files with 161 additions and 74 deletions

View File

@ -265,9 +265,10 @@ class RegionChanges {
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,

View File

@ -52,6 +52,19 @@ public:
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 @@ public:
/// 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 @@ public:
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;

View File

@ -212,7 +212,8 @@ public:
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,

View File

@ -97,19 +97,20 @@ public:
/// 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.

View File

@ -64,7 +64,8 @@ public:
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 @@ ProgramStateRef
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;

View File

@ -184,13 +184,27 @@ public:
/// 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;
}

View File

@ -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 @@ public:
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 @@ public:
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 @@ public:
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 @@ private:
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 @@ public:
};
} // 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 @@ void MallocChecker::initIdentifierInfo(CheckerContext &C) const {
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 @@ void MallocChecker::checkPostStmt(const CallExpr *CE, CheckerContext &C) const {
}
}
}
// 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 @@ bool MallocChecker::checkEscape(SymbolRef Sym, const Stmt *S,
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 @@ ProgramStateRef MallocChecker::evalAssume(ProgramStateRef state,
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 @@ MallocChecker::checkRegionChanges(ProgramStateRef state,
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 *

View File

@ -2439,7 +2439,8 @@ public:
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 @@ ProgramStateRef
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;

View File

@ -413,14 +413,15 @@ ProgramStateRef
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;
}

View File

@ -176,9 +176,10 @@ ProgramStateRef
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,

View File

@ -179,7 +179,7 @@ ProgramState::invalidateRegionsImpl(ArrayRef<const MemRegion *> Regions,
= 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 =