forked from OSchip/llvm-project
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. llvm-svn: 167813
This commit is contained in:
parent
ab72f9763f
commit
67291b90f9
|
@ -107,7 +107,7 @@ class MallocChecker : public Checker<check::DeadSymbols,
|
|||
check::PreStmt<CallExpr>,
|
||||
check::PostStmt<CallExpr>,
|
||||
check::PostStmt<BlockExpr>,
|
||||
check::PreObjCMessage,
|
||||
check::PostObjCMessage,
|
||||
check::Location,
|
||||
check::Bind,
|
||||
eval::Assume,
|
||||
|
@ -135,7 +135,7 @@ public:
|
|||
|
||||
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 @@ private:
|
|||
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 @@ private:
|
|||
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 @@ static bool isFreeWhenDoneSetToZero(const ObjCMethodCall &Call) {
|
|||
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 @@ void MallocChecker::checkPreObjCMessage(const ObjCMethodCall &Call,
|
|||
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,44 @@ ProgramStateRef MallocChecker::FreeMemAux(CheckerContext &C,
|
|||
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);
|
||||
}
|
||||
|
||||
/// Check if the previous call to free on the given symbol failed.
|
||||
///
|
||||
/// For example, if free failed, returns true. In addition, cleans out the
|
||||
/// state from the corresponding failure info. Retuns the cleaned out state
|
||||
/// and the corresponding return value symbol.
|
||||
static std::pair<bool, ProgramStateRef>
|
||||
checkAndCleanFreeFailedInfo(ProgramStateRef State,
|
||||
SymbolRef Sym, const SymbolRef *Ret) {
|
||||
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);
|
||||
State = State->remove<FreeReturnValue>(Sym);
|
||||
return std::pair<bool, ProgramStateRef>(FreeFailed.isConstrainedTrue(),
|
||||
State);
|
||||
}
|
||||
return std::pair<bool, ProgramStateRef>(false, State);
|
||||
}
|
||||
|
||||
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 +666,7 @@ ProgramStateRef MallocChecker::FreeMemAux(CheckerContext &C,
|
|||
|
||||
// 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 +715,17 @@ ProgramStateRef MallocChecker::FreeMemAux(CheckerContext &C,
|
|||
return 0;
|
||||
|
||||
SymbolRef Sym = SR->getSymbol();
|
||||
const RefState *RS = state->get<RegionState>(Sym);
|
||||
const RefState *RS = State->get<RegionState>(Sym);
|
||||
bool FailedToFree = false;
|
||||
const SymbolRef *RetStatusSymbolPtr = 0;
|
||||
llvm::tie(FailedToFree, State) =
|
||||
checkAndCleanFreeFailedInfo(State, Sym, RetStatusSymbolPtr);
|
||||
|
||||
// Check double free.
|
||||
if (RS && (RS->isReleased() || RS->isRelinquished())) {
|
||||
if (RS &&
|
||||
(RS->isReleased() || RS->isRelinquished()) &&
|
||||
!FailedToFree) {
|
||||
|
||||
if (ExplodedNode *N = C.generateSink()) {
|
||||
if (!BT_DoubleFree)
|
||||
BT_DoubleFree.reset(
|
||||
|
@ -696,6 +735,8 @@ ProgramStateRef MallocChecker::FreeMemAux(CheckerContext &C,
|
|||
"Attempt to free non-owned memory"), N);
|
||||
R->addRange(ArgExpr->getSourceRange());
|
||||
R->markInteresting(Sym);
|
||||
if (RetStatusSymbolPtr)
|
||||
R->markInteresting(*RetStatusSymbolPtr);
|
||||
R->addVisitor(new MallocBugVisitor(Sym));
|
||||
C.emitReport(R);
|
||||
}
|
||||
|
@ -704,10 +745,21 @@ ProgramStateRef MallocChecker::FreeMemAux(CheckerContext &C,
|
|||
|
||||
ReleasedAllocated = (RS != 0);
|
||||
|
||||
// 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 +1116,15 @@ void MallocChecker::checkDeadSymbols(SymbolReaper &SymReaper,
|
|||
}
|
||||
}
|
||||
|
||||
// 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()) {
|
||||
|
|
|
@ -222,3 +222,50 @@ void noCrashOnVariableArgumentSelector() {
|
|||
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);}
|
||||
}
|
||||
|
||||
void test12365078_false_negative_no_malloc(unichar *characters) {
|
||||
NSString *string = [[NSString alloc] initWithCharactersNoCopy:characters length:12 freeWhenDone:1];
|
||||
if (!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);
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
Loading…
Reference in New Issue