[cfe-commits] r162533 - in /cfe/trunk: lib/StaticAnalyzer/Checkers/MallocChecker.cpp test/Analysis/malloc.c
Anna Zaks
ganna at apple.com
Thu Aug 23 19:28:20 PDT 2012
Author: zaks
Date: Thu Aug 23 21:28:20 2012
New Revision: 162533
URL: http://llvm.org/viewvc/llvm-project?rev=162533&view=rev
Log:
[analyzer] Fix realloc related bug in the malloc checker.
When reallocation of a non-allocated (not owned) symbol fails do not
expect it to be freed.
Modified:
cfe/trunk/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
cfe/trunk/test/Analysis/malloc.c
Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/MallocChecker.cpp?rev=162533&r1=162532&r2=162533&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Checkers/MallocChecker.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/MallocChecker.cpp Thu Aug 23 21:28:20 2012
@@ -70,9 +70,16 @@
}
};
+/// \class ReallocPair
+/// \brief Stores information about the symbol being reallocated by a call to
+/// 'realloc' to allow modeling failed reallocation later in the path.
struct ReallocPair {
+ // \brief The symbol which realloc reallocated.
SymbolRef ReallocatedSym;
+ // \brief The flag is true if the symbol does not need to be freed after
+ // reallocation fails.
bool IsFreeOnFailure;
+
ReallocPair(SymbolRef S, bool F) : ReallocatedSym(S), IsFreeOnFailure(F) {}
void Profile(llvm::FoldingSetNodeID &ID) const {
ID.AddInteger(IsFreeOnFailure);
@@ -177,11 +184,13 @@
const OwnershipAttr* Att) const;
ProgramStateRef FreeMemAux(CheckerContext &C, const CallExpr *CE,
ProgramStateRef state, unsigned Num,
- bool Hold) const;
+ bool Hold,
+ bool &ReleasedAllocated) const;
ProgramStateRef FreeMemAux(CheckerContext &C, const Expr *Arg,
const Expr *ParentExpr,
ProgramStateRef state,
- bool Hold) const;
+ bool Hold,
+ bool &ReleasedAllocated) const;
ProgramStateRef ReallocMem(CheckerContext &C, const CallExpr *CE,
bool FreesMemOnFailure) const;
@@ -431,6 +440,7 @@
return;
ProgramStateRef State = C.getState();
+ bool ReleasedAllocatedMemory = false;
if (FD->getKind() == Decl::Function) {
initIdentifierInfo(C.getASTContext());
@@ -447,7 +457,7 @@
} else if (FunI == II_calloc) {
State = CallocMem(C, CE);
} else if (FunI == II_free) {
- State = FreeMemAux(C, CE, State, 0, false);
+ State = FreeMemAux(C, CE, State, 0, false, ReleasedAllocatedMemory);
} else if (FunI == II_strdup) {
State = MallocUpdateRefState(C, CE, State);
} else if (FunI == II_strndup) {
@@ -494,6 +504,7 @@
// Ex: [NSData dataWithBytesNoCopy:bytes length:10];
// Unless 'freeWhenDone' param set to 0.
// TODO: Check that the memory was allocated with malloc.
+ bool ReleasedAllocatedMemory = false;
Selector S = Call.getSelector();
if ((S.getNameForSlot(0) == "dataWithBytesNoCopy" ||
S.getNameForSlot(0) == "initWithBytesNoCopy" ||
@@ -501,7 +512,8 @@
!isFreeWhenDoneSetToZero(Call)){
unsigned int argIdx = 0;
C.addTransition(FreeMemAux(C, Call.getArgExpr(argIdx),
- Call.getOriginExpr(), C.getState(), true));
+ Call.getOriginExpr(), C.getState(), true,
+ ReleasedAllocatedMemory));
}
}
@@ -584,11 +596,13 @@
return 0;
ProgramStateRef State = C.getState();
+ bool ReleasedAllocated = false;
for (OwnershipAttr::args_iterator I = Att->args_begin(), E = Att->args_end();
I != E; ++I) {
ProgramStateRef StateI = FreeMemAux(C, CE, State, *I,
- Att->getOwnKind() == OwnershipAttr::Holds);
+ Att->getOwnKind() == OwnershipAttr::Holds,
+ ReleasedAllocated);
if (StateI)
State = StateI;
}
@@ -599,18 +613,20 @@
const CallExpr *CE,
ProgramStateRef state,
unsigned Num,
- bool Hold) const {
+ bool Hold,
+ bool &ReleasedAllocated) const {
if (CE->getNumArgs() < (Num + 1))
return 0;
- return FreeMemAux(C, CE->getArg(Num), CE, state, Hold);
+ return FreeMemAux(C, CE->getArg(Num), CE, state, Hold, ReleasedAllocated);
}
ProgramStateRef MallocChecker::FreeMemAux(CheckerContext &C,
const Expr *ArgExpr,
const Expr *ParentExpr,
ProgramStateRef state,
- bool Hold) const {
+ bool Hold,
+ bool &ReleasedAllocated) const {
SVal ArgVal = state->getSVal(ArgExpr, C.getLocationContext());
if (!isa<DefinedOrUnknownSVal>(ArgVal))
@@ -691,6 +707,8 @@
return 0;
}
+ ReleasedAllocated = (RS != 0);
+
// Normal free.
if (Hold)
return state->set<RegionState>(Sym, RefState::getRelinquished(ParentExpr));
@@ -886,9 +904,12 @@
if (!FromPtr || !ToPtr)
return 0;
+ bool ReleasedAllocated = false;
+
// If the size is 0, free the memory.
if (SizeIsZero)
- if (ProgramStateRef stateFree = FreeMemAux(C, CE, StateSizeIsZero,0,false)){
+ if (ProgramStateRef stateFree = FreeMemAux(C, CE, StateSizeIsZero, 0,
+ false, ReleasedAllocated)){
// The semantics of the return value are:
// If size was equal to 0, either NULL or a pointer suitable to be passed
// to free() is returned. We just free the input pointer and do not add
@@ -897,14 +918,19 @@
}
// Default behavior.
- if (ProgramStateRef stateFree = FreeMemAux(C, CE, state, 0, false)) {
- // FIXME: We should copy the content of the original buffer.
+ if (ProgramStateRef stateFree =
+ FreeMemAux(C, CE, state, 0, false, ReleasedAllocated)) {
+
ProgramStateRef stateRealloc = MallocMemAux(C, CE, CE->getArg(1),
UnknownVal(), stateFree);
if (!stateRealloc)
return 0;
+
+ // Record the info about the reallocated symbol so that we could properly
+ // process failed reallocation.
stateRealloc = stateRealloc->set<ReallocPairs>(ToPtr,
- ReallocPair(FromPtr, FreesOnFail));
+ ReallocPair(FromPtr, FreesOnFail || !ReleasedAllocated));
+ // The reallocated symbol should stay alive for as long as the new symbol.
C.getSymbolManager().addSymbolDependency(ToPtr, FromPtr);
return stateRealloc;
}
Modified: cfe/trunk/test/Analysis/malloc.c
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/malloc.c?rev=162533&r1=162532&r2=162533&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/malloc.c (original)
+++ cfe/trunk/test/Analysis/malloc.c Thu Aug 23 21:28:20 2012
@@ -1000,17 +1000,26 @@
}
struct HasPtr {
- int *p;
+ char *p;
};
-int* reallocButNoMalloc(struct HasPtr *a, int c, int size) {
+char* reallocButNoMalloc(struct HasPtr *a, int c, int size) {
int *s;
- a->p = (int *)realloc(a->p, size);
- if (a->p == 0)
- return 0; // expected-warning{{Memory is never released; potential leak}}
+ char *b = realloc(a->p, size);
+ char *m = realloc(a->p, size); // expected-warning {{Attempt to free released memory}}
return a->p;
}
+// We should not warn in this case since the caller will presumably free a->p in all cases.
+int reallocButNoMallocPR13674(struct HasPtr *a, int c, int size) {
+ int *s;
+ char *b = realloc(a->p, size);
+ if (b == 0)
+ return -1;
+ a->p = b;
+ return 0;
+}
+
// ----------------------------------------------------------------------------
// False negatives.
More information about the cfe-commits
mailing list