[analyzer] Malloc: Warn about use-after-free when memory ownership was
transfered with dataWithBytesNoCopy.
git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@158958 91177308-0d34-0410-b5e6-96231b3b80d8
diff --git a/lib/StaticAnalyzer/Checkers/MallocChecker.cpp b/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
index 3171c03..35c6073 100644
--- a/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
+++ b/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
@@ -96,6 +96,7 @@
check::PreStmt<CallExpr>,
check::PostStmt<CallExpr>,
check::PostStmt<BlockExpr>,
+ check::PreObjCMessage,
check::Location,
check::Bind,
eval::Assume,
@@ -123,6 +124,7 @@
void checkPreStmt(const CallExpr *S, CheckerContext &C) const;
void checkPostStmt(const CallExpr *CE, CheckerContext &C) const;
+ void checkPreObjCMessage(const ObjCMessage &Msg, CheckerContext &C) const;
void checkPostStmt(const BlockExpr *BE, CheckerContext &C) const;
void checkDeadSymbols(SymbolReaper &SymReaper, CheckerContext &C) const;
void checkEndPath(CheckerContext &C) const;
@@ -178,8 +180,12 @@
ProgramStateRef FreeMemAttr(CheckerContext &C, const CallExpr *CE,
const OwnershipAttr* Att) const;
ProgramStateRef FreeMemAux(CheckerContext &C, const CallExpr *CE,
- ProgramStateRef state, unsigned Num,
- bool Hold) const;
+ ProgramStateRef state, unsigned Num,
+ bool Hold) const;
+ ProgramStateRef FreeMemAux(CheckerContext &C, const Expr *Arg,
+ const Expr *ParentExpr,
+ ProgramStateRef state,
+ bool Hold) const;
ProgramStateRef ReallocMem(CheckerContext &C, const CallExpr *CE,
bool FreesMemOnFailure) const;
@@ -255,6 +261,15 @@
(S && S->isReleased()) && (!SPrev || !SPrev->isReleased()));
}
+ inline bool isRelinquished(const RefState *S, const RefState *SPrev,
+ const Stmt *Stmt) {
+ // Did not track -> relinquished. Other state (allocated) -> relinquished.
+ return (Stmt && (isa<CallExpr>(Stmt) || isa<ObjCMessageExpr>(Stmt) ||
+ isa<ObjCPropertyRefExpr>(Stmt)) &&
+ (S && S->isRelinquished()) &&
+ (!SPrev || !SPrev->isRelinquished()));
+ }
+
inline bool isReallocFailedCheck(const RefState *S, const RefState *SPrev,
const Stmt *Stmt) {
// If the expression is not a call, and the state change is
@@ -466,6 +481,37 @@
C.addTransition(State);
}
+static bool isFreeWhenDoneSetToZero(CallOrObjCMessage Call, Selector &S) {
+ for (unsigned i = 1; i < Call.getNumArgs(); ++i)
+ if (S.getNameForSlot(i).equals("freeWhenDone"))
+ if (Call.getArgSVal(i).isConstant(0))
+ return true;
+
+ return false;
+}
+
+void MallocChecker::checkPreObjCMessage(const ObjCMessage &Msg,
+ CheckerContext &C) const {
+ const ObjCMethodDecl *MD = Msg.getMethodDecl();
+ if (!MD)
+ return;
+
+ CallOrObjCMessage Call(Msg, C.getState(), C.getLocationContext());
+ Selector S = Msg.getSelector();
+
+ // 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];
+ // Unless 'freeWhenDone' param set to 0.
+ // TODO: Check that the memory was allocated with malloc.
+ if (S.getNameForSlot(0) == "dataWithBytesNoCopy" &&
+ !isFreeWhenDoneSetToZero(Call, S)){
+ unsigned int argIdx = 0;
+ C.addTransition(FreeMemAux(C, Call.getArg(argIdx),
+ Msg.getMessageExpr(), C.getState(), true));
+ }
+}
+
ProgramStateRef MallocChecker::MallocMemReturnsAttr(CheckerContext &C,
const CallExpr *CE,
const OwnershipAttr* Att) {
@@ -564,7 +610,15 @@
if (CE->getNumArgs() < (Num + 1))
return 0;
- const Expr *ArgExpr = CE->getArg(Num);
+ return FreeMemAux(C, CE->getArg(Num), CE, state, Hold);
+}
+
+ProgramStateRef MallocChecker::FreeMemAux(CheckerContext &C,
+ const Expr *ArgExpr,
+ const Expr *ParentExpr,
+ ProgramStateRef state,
+ bool Hold) const {
+
SVal ArgVal = state->getSVal(ArgExpr, C.getLocationContext());
if (!isa<DefinedOrUnknownSVal>(ArgVal))
return 0;
@@ -634,14 +688,14 @@
return 0;
// Check double free.
- // TODO: Split the 2 cases for better error messages.
if (RS->isReleased() || RS->isRelinquished()) {
if (ExplodedNode *N = C.generateSink()) {
if (!BT_DoubleFree)
BT_DoubleFree.reset(
new BugType("Double free", "Memory Error"));
BugReport *R = new BugReport(*BT_DoubleFree,
- "Attempt to free released memory", N);
+ (RS->isReleased() ? "Attempt to free released memory" :
+ "Attempt to free non-owned memory"), N);
R->addRange(ArgExpr->getSourceRange());
R->markInteresting(Sym);
R->addVisitor(new MallocBugVisitor(Sym));
@@ -652,8 +706,8 @@
// Normal free.
if (Hold)
- return state->set<RegionState>(Sym, RefState::getRelinquished(CE));
- return state->set<RegionState>(Sym, RefState::getReleased(CE));
+ return state->set<RegionState>(Sym, RefState::getRelinquished(ParentExpr));
+ return state->set<RegionState>(Sym, RefState::getReleased(ParentExpr));
}
bool MallocChecker::SummarizeValue(raw_ostream &os, SVal V) {
@@ -1381,7 +1435,7 @@
// White list the ObjC functions which do free memory.
// - Anything containing 'freeWhenDone' param set to 1.
// Ex: dataWithBytesNoCopy:length:freeWhenDone.
- for (unsigned i = 1; i < S.getNumArgs(); ++i) {
+ for (unsigned i = 1; i < Call->getNumArgs(); ++i) {
if (S.getNameForSlot(i).equals("freeWhenDone")) {
if (Call->getArgSVal(i).isConstant(1))
return false;
@@ -1452,9 +1506,14 @@
SymbolRef sym = *I;
if (WhitelistedSymbols.count(sym))
continue;
- // The symbol escaped.
- if (const RefState *RS = State->get<RegionState>(sym))
- State = State->set<RegionState>(sym, RefState::getEscaped(RS->getStmt()));
+ // The symbol escaped. Note, we assume that if the symbol is released,
+ // passing it out will result in a use after free. We also keep tracking
+ // relinquished symbols.
+ if (const RefState *RS = State->get<RegionState>(sym)) {
+ if (RS->isAllocated())
+ State = State->set<RegionState>(sym,
+ RefState::getEscaped(RS->getStmt()));
+ }
}
return State;
}
@@ -1514,6 +1573,9 @@
Msg = "Memory is released";
StackHint = new StackHintGeneratorForSymbol(Sym,
"Returned released memory");
+ } else if (isRelinquished(RS, RSPrev, S)) {
+ Msg = "Memory ownership is transfered";
+ StackHint = new StackHintGeneratorForSymbol(Sym, "");
} else if (isReallocFailedCheck(RS, RSPrev, S)) {
Mode = ReallocationFailed;
Msg = "Reallocation failed";
diff --git a/test/Analysis/malloc-annotations.c b/test/Analysis/malloc-annotations.c
index 15ce62d..089e531 100644
--- a/test/Analysis/malloc-annotations.c
+++ b/test/Analysis/malloc-annotations.c
@@ -124,11 +124,10 @@
}
// This case inflicts a possible double-free.
-// TODO: Better error message.
void af3() {
int *p = my_malloc(12);
my_hold(p);
- free(p); // expected-warning{{Attempt to free released memory}}
+ free(p); // expected-warning{{Attempt to free non-owned memory}}
}
int * af4() {
diff --git a/test/Analysis/malloc.mm b/test/Analysis/malloc.mm
index 855892d..23297ec 100644
--- a/test/Analysis/malloc.mm
+++ b/test/Analysis/malloc.mm
@@ -9,7 +9,6 @@
void testNSDatafFreeWhenDoneNoError(NSUInteger dataLength) {
unsigned char *data = (unsigned char *)malloc(42);
NSData *nsdata = [NSData dataWithBytesNoCopy:data length:dataLength];
- free(data); // no warning
}
void testNSDataFreeWhenDoneYES(NSUInteger dataLength) {
@@ -55,11 +54,17 @@
NSString *nsstr = [[NSString alloc] initWithCharactersNoCopy:data length:dataLength freeWhenDone:0]; // expected-warning{{leak}}
}
-// TODO: False Negative.
-void testNSDatafFreeWhenDoneFN(NSUInteger dataLength) {
- unsigned char *data = (unsigned char *)malloc(42);
- NSData *nsdata = [NSData dataWithBytesNoCopy:data length:dataLength freeWhenDone:1];
- free(data); // false negative
+void testRelinquished1() {
+ void *data = malloc(42);
+ NSData *nsdata = [NSData dataWithBytesNoCopy:data length:42 freeWhenDone:1];
+ free(data); // expected-warning {{Attempt to free non-owned memory}}
+}
+
+void testRelinquished2() {
+ void *data = malloc(42);
+ NSData *nsdata;
+ free(data);
+ [NSData dataWithBytesNoCopy:data length:42]; // expected-warning {{Attempt to free released memory}}
}
// Test CF/NS...NoCopy. PR12100: Pointers can escape when custom deallocators are provided.
