[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/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 *