[llvm-branch-commits] [cfe-branch] r167870 - in /cfe/branches/release_32: ./ lib/StaticAnalyzer/Checkers/MallocChecker.cpp test/Analysis/malloc.mm test/SemaCXX/warn-unreachable.cpp
Anna Zaks
ganna at apple.com
Tue Nov 13 12:22:04 PST 2012
Author: zaks
Date: Tue Nov 13 14:22:03 2012
New Revision: 167870
URL: http://llvm.org/viewvc/llvm-project?rev=167870&view=rev
Log:
Fix a Malloc Checker FP by tracking return values from initWithCharacter
and other functions.
When these functions return null, the pointer is not freed by
them/ownership is not transfered. So we should allow the user to free
the pointer by calling another function when the return value is NULL.
Commits: 167813, 167814, 167868
Modified:
cfe/branches/release_32/ (props changed)
cfe/branches/release_32/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
cfe/branches/release_32/test/Analysis/malloc.mm
cfe/branches/release_32/test/SemaCXX/warn-unreachable.cpp (props changed)
Propchange: cfe/branches/release_32/
------------------------------------------------------------------------------
--- svn:mergeinfo (original)
+++ svn:mergeinfo Tue Nov 13 14:22:03 2012
@@ -1,3 +1,3 @@
/cfe/branches/type-system-rewrite:134693-134817
-/cfe/trunk:167749,167762,167780,167788,167790
+/cfe/trunk:167749,167762,167780,167788,167790,167813-167814,167868
/cfe/trunk/test/SemaTemplate:126920
Modified: cfe/branches/release_32/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/branches/release_32/lib/StaticAnalyzer/Checkers/MallocChecker.cpp?rev=167870&r1=167869&r2=167870&view=diff
==============================================================================
--- cfe/branches/release_32/lib/StaticAnalyzer/Checkers/MallocChecker.cpp (original)
+++ cfe/branches/release_32/lib/StaticAnalyzer/Checkers/MallocChecker.cpp Tue Nov 13 14:22:03 2012
@@ -107,7 +107,7 @@
check::PreStmt<CallExpr>,
check::PostStmt<CallExpr>,
check::PostStmt<BlockExpr>,
- check::PreObjCMessage,
+ check::PostObjCMessage,
check::Location,
check::Bind,
eval::Assume,
@@ -135,7 +135,7 @@
void checkPreStmt(const CallExpr *S, CheckerContext &C) const;
void checkPostStmt(const CallExpr *CE, CheckerContext &C) const;
- void checkPreObjCMessage(const ObjCMethodCall &Call, CheckerContext &C) const;
+ void checkPostObjCMessage(const ObjCMethodCall &Call, CheckerContext &C) const;
void checkPostStmt(const BlockExpr *BE, CheckerContext &C) const;
void checkDeadSymbols(SymbolReaper &SymReaper, CheckerContext &C) const;
void checkEndPath(CheckerContext &C) const;
@@ -193,12 +193,14 @@
ProgramStateRef FreeMemAux(CheckerContext &C, const CallExpr *CE,
ProgramStateRef state, unsigned Num,
bool Hold,
- bool &ReleasedAllocated) const;
+ bool &ReleasedAllocated,
+ bool ReturnsNullOnFailure = false) const;
ProgramStateRef FreeMemAux(CheckerContext &C, const Expr *Arg,
const Expr *ParentExpr,
- ProgramStateRef state,
+ ProgramStateRef State,
bool Hold,
- bool &ReleasedAllocated) const;
+ bool &ReleasedAllocated,
+ bool ReturnsNullOnFailure = false) const;
ProgramStateRef ReallocMem(CheckerContext &C, const CallExpr *CE,
bool FreesMemOnFailure) const;
@@ -341,6 +343,10 @@
REGISTER_MAP_WITH_PROGRAMSTATE(RegionState, SymbolRef, RefState)
REGISTER_MAP_WITH_PROGRAMSTATE(ReallocPairs, SymbolRef, ReallocPair)
+// A map from the freed symbol to the symbol representing the return value of
+// the free function.
+REGISTER_MAP_WITH_PROGRAMSTATE(FreeReturnValue, SymbolRef, SymbolRef)
+
namespace {
class StopTrackingCallback : public SymbolVisitor {
ProgramStateRef state;
@@ -492,8 +498,8 @@
return false;
}
-void MallocChecker::checkPreObjCMessage(const ObjCMethodCall &Call,
- CheckerContext &C) const {
+void MallocChecker::checkPostObjCMessage(const ObjCMethodCall &Call,
+ CheckerContext &C) const {
// If the first selector is dataWithBytesNoCopy, assume that the memory will
// be released with 'free' by the new object.
// Ex: [NSData dataWithBytesNoCopy:bytes length:10];
@@ -506,9 +512,12 @@
S.getNameForSlot(0) == "initWithCharactersNoCopy") &&
!isFreeWhenDoneSetToZero(Call)){
unsigned int argIdx = 0;
- C.addTransition(FreeMemAux(C, Call.getArgExpr(argIdx),
- Call.getOriginExpr(), C.getState(), true,
- ReleasedAllocatedMemory));
+ ProgramStateRef State = FreeMemAux(C, Call.getArgExpr(argIdx),
+ Call.getOriginExpr(), C.getState(), true,
+ ReleasedAllocatedMemory,
+ /* RetNullOnFailure*/ true);
+
+ C.addTransition(State);
}
}
@@ -609,21 +618,39 @@
ProgramStateRef state,
unsigned Num,
bool Hold,
- bool &ReleasedAllocated) const {
+ bool &ReleasedAllocated,
+ bool ReturnsNullOnFailure) const {
if (CE->getNumArgs() < (Num + 1))
return 0;
- return FreeMemAux(C, CE->getArg(Num), CE, state, Hold, ReleasedAllocated);
+ return FreeMemAux(C, CE->getArg(Num), CE, state, Hold,
+ ReleasedAllocated, ReturnsNullOnFailure);
+}
+
+/// Checks if the previous call to free on the given symbol failed - if free
+/// failed, returns true. Also, returns the corresponding return value symbol.
+bool didPreviousFreeFail(ProgramStateRef State,
+ SymbolRef Sym, SymbolRef &RetStatusSymbol) {
+ const SymbolRef *Ret = State->get<FreeReturnValue>(Sym);
+ if (Ret) {
+ assert(*Ret && "We should not store the null return symbol");
+ ConstraintManager &CMgr = State->getConstraintManager();
+ ConditionTruthVal FreeFailed = CMgr.isNull(State, *Ret);
+ RetStatusSymbol = *Ret;
+ return FreeFailed.isConstrainedTrue();
+ }
+ return false;
}
ProgramStateRef MallocChecker::FreeMemAux(CheckerContext &C,
const Expr *ArgExpr,
const Expr *ParentExpr,
- ProgramStateRef state,
+ ProgramStateRef State,
bool Hold,
- bool &ReleasedAllocated) const {
+ bool &ReleasedAllocated,
+ bool ReturnsNullOnFailure) const {
- SVal ArgVal = state->getSVal(ArgExpr, C.getLocationContext());
+ SVal ArgVal = State->getSVal(ArgExpr, C.getLocationContext());
if (!isa<DefinedOrUnknownSVal>(ArgVal))
return 0;
DefinedOrUnknownSVal location = cast<DefinedOrUnknownSVal>(ArgVal);
@@ -634,7 +661,7 @@
// The explicit NULL case, no operation is performed.
ProgramStateRef notNullState, nullState;
- llvm::tie(notNullState, nullState) = state->assume(location);
+ llvm::tie(notNullState, nullState) = State->assume(location);
if (nullState && !notNullState)
return 0;
@@ -683,10 +710,14 @@
return 0;
SymbolRef Sym = SR->getSymbol();
- const RefState *RS = state->get<RegionState>(Sym);
+ const RefState *RS = State->get<RegionState>(Sym);
+ SymbolRef PreviousRetStatusSymbol = 0;
// Check double free.
- if (RS && (RS->isReleased() || RS->isRelinquished())) {
+ if (RS &&
+ (RS->isReleased() || RS->isRelinquished()) &&
+ !didPreviousFreeFail(State, Sym, PreviousRetStatusSymbol)) {
+
if (ExplodedNode *N = C.generateSink()) {
if (!BT_DoubleFree)
BT_DoubleFree.reset(
@@ -696,6 +727,8 @@
"Attempt to free non-owned memory"), N);
R->addRange(ArgExpr->getSourceRange());
R->markInteresting(Sym);
+ if (PreviousRetStatusSymbol)
+ R->markInteresting(PreviousRetStatusSymbol);
R->addVisitor(new MallocBugVisitor(Sym));
C.emitReport(R);
}
@@ -704,10 +737,24 @@
ReleasedAllocated = (RS != 0);
+ // Clean out the info on previous call to free return info.
+ State = State->remove<FreeReturnValue>(Sym);
+
+ // Keep track of the return value. If it is NULL, we will know that free
+ // failed.
+ if (ReturnsNullOnFailure) {
+ SVal RetVal = C.getSVal(ParentExpr);
+ SymbolRef RetStatusSymbol = RetVal.getAsSymbol();
+ if (RetStatusSymbol) {
+ C.getSymbolManager().addSymbolDependency(Sym, RetStatusSymbol);
+ State = State->set<FreeReturnValue>(Sym, RetStatusSymbol);
+ }
+ }
+
// Normal free.
if (Hold)
- return state->set<RegionState>(Sym, RefState::getRelinquished(ParentExpr));
- return state->set<RegionState>(Sym, RefState::getReleased(ParentExpr));
+ return State->set<RegionState>(Sym, RefState::getRelinquished(ParentExpr));
+ return State->set<RegionState>(Sym, RefState::getReleased(ParentExpr));
}
bool MallocChecker::SummarizeValue(raw_ostream &os, SVal V) {
@@ -1064,6 +1111,15 @@
}
}
+ // Cleanup the FreeReturnValue Map.
+ FreeReturnValueTy FR = state->get<FreeReturnValue>();
+ for (FreeReturnValueTy::iterator I = FR.begin(), E = FR.end(); I != E; ++I) {
+ if (SymReaper.isDead(I->first) ||
+ SymReaper.isDead(I->second)) {
+ state = state->remove<FreeReturnValue>(I->first);
+ }
+ }
+
// Generate leak node.
ExplodedNode *N = C.getPredecessor();
if (!Errors.empty()) {
Modified: cfe/branches/release_32/test/Analysis/malloc.mm
URL: http://llvm.org/viewvc/llvm-project/cfe/branches/release_32/test/Analysis/malloc.mm?rev=167870&r1=167869&r2=167870&view=diff
==============================================================================
--- cfe/branches/release_32/test/Analysis/malloc.mm (original)
+++ cfe/branches/release_32/test/Analysis/malloc.mm Tue Nov 13 14:22:03 2012
@@ -222,3 +222,59 @@
NSMutableString *myString = [NSMutableString stringWithString:@"some text"];
[myString appendFormat:@"some text = %d", 3];
}
+
+void test12365078_check() {
+ unichar *characters = (unichar*)malloc(12);
+ NSString *string = [[NSString alloc] initWithCharactersNoCopy:characters length:12 freeWhenDone:1];
+ if (!string) free(characters); // no-warning
+}
+
+void test12365078_nocheck() {
+ unichar *characters = (unichar*)malloc(12);
+ NSString *string = [[NSString alloc] initWithCharactersNoCopy:characters length:12 freeWhenDone:1];
+}
+
+void test12365078_false_negative() {
+ unichar *characters = (unichar*)malloc(12);
+ NSString *string = [[NSString alloc] initWithCharactersNoCopy:characters length:12 freeWhenDone:1];
+ if (!string) {;}
+}
+
+void test12365078_no_malloc(unichar *characters) {
+ NSString *string = [[NSString alloc] initWithCharactersNoCopy:characters length:12 freeWhenDone:1];
+ if (!string) {free(characters);}
+}
+
+NSString *test12365078_no_malloc_returnValue(unichar *characters) {
+ NSString *string = [[NSString alloc] initWithCharactersNoCopy:characters length:12 freeWhenDone:1];
+ if (!string) {
+ return 0; // no-warning
+ }
+ return string;
+}
+
+void test12365078_nocheck_nomalloc(unichar *characters) {
+ NSString *string = [[NSString alloc] initWithCharactersNoCopy:characters length:12 freeWhenDone:1];
+ free(characters); // expected-warning {{Attempt to free non-owned memory}}
+}
+
+void test12365078_nested(unichar *characters) {
+ NSString *string = [[NSString alloc] initWithCharactersNoCopy:characters length:12 freeWhenDone:1];
+ if (!string) {
+ NSString *string2 = [[NSString alloc] initWithCharactersNoCopy:characters length:12 freeWhenDone:1];
+ if (!string2) {
+ NSString *string3 = [[NSString alloc] initWithCharactersNoCopy:characters length:12 freeWhenDone:1];
+ if (!string3) {
+ NSString *string4 = [[NSString alloc] initWithCharactersNoCopy:characters length:12 freeWhenDone:1];
+ if (!string4)
+ free(characters);
+ }
+ }
+ }
+}
+
+void test12365078_check_positive() {
+ unichar *characters = (unichar*)malloc(12);
+ NSString *string = [[NSString alloc] initWithCharactersNoCopy:characters length:12 freeWhenDone:1];
+ if (string) free(characters); // expected-warning{{Attempt to free non-owned memory}}
+}
Propchange: cfe/branches/release_32/test/SemaCXX/warn-unreachable.cpp
------------------------------------------------------------------------------
--- svn:mergeinfo (original)
+++ svn:mergeinfo Tue Nov 13 14:22:03 2012
@@ -1,2 +1,2 @@
/cfe/branches/type-system-rewrite/test/SemaCXX/warn-unreachable.cpp:134693-134817
-/cfe/trunk/test/SemaCXX/warn-unreachable.cpp:121961,167749,167762,167780,167788,167790
+/cfe/trunk/test/SemaCXX/warn-unreachable.cpp:121961,167749,167762,167780,167788,167790,167813-167814,167868
More information about the llvm-branch-commits
mailing list