[analyzer] Address Jordan's code review for r167813.

This simplifies logic, fixes a bug, and adds a test case.
Thanks Jordan!

llvm-svn: 167868
This commit is contained in:
Anna Zaks 2012-11-13 19:47:40 +00:00
parent 38d2284eeb
commit a14c1d09f6
2 changed files with 21 additions and 20 deletions

View File

@ -627,24 +627,19 @@ ProgramStateRef MallocChecker::FreeMemAux(CheckerContext &C,
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);
/// 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);
State = State->remove<FreeReturnValue>(Sym);
return std::pair<bool, ProgramStateRef>(FreeFailed.isConstrainedTrue(),
State);
RetStatusSymbol = *Ret;
return FreeFailed.isConstrainedTrue();
}
return std::pair<bool, ProgramStateRef>(false, State);
return false;
}
ProgramStateRef MallocChecker::FreeMemAux(CheckerContext &C,
@ -716,15 +711,12 @@ ProgramStateRef MallocChecker::FreeMemAux(CheckerContext &C,
SymbolRef Sym = SR->getSymbol();
const RefState *RS = State->get<RegionState>(Sym);
bool FailedToFree = false;
const SymbolRef *RetStatusSymbolPtr = 0;
llvm::tie(FailedToFree, State) =
checkAndCleanFreeFailedInfo(State, Sym, RetStatusSymbolPtr);
SymbolRef PreviousRetStatusSymbol = 0;
// Check double free.
if (RS &&
(RS->isReleased() || RS->isRelinquished()) &&
!FailedToFree) {
!didPreviousFreeFail(State, Sym, PreviousRetStatusSymbol)) {
if (ExplodedNode *N = C.generateSink()) {
if (!BT_DoubleFree)
@ -735,8 +727,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);
if (PreviousRetStatusSymbol)
R->markInteresting(PreviousRetStatusSymbol);
R->addVisitor(new MallocBugVisitor(Sym));
C.emitReport(R);
}
@ -745,6 +737,9 @@ ProgramStateRef MallocChecker::FreeMemAux(CheckerContext &C,
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) {

View File

@ -272,3 +272,9 @@ void test12365078_nested(unichar *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}}
}