One last question for you before jumping into patch review... Right now, the
new diagnostic looks like this:
Argument to operator delete[] was allocated by malloc(), not operator new[]
I wonder if this is more helpful, or some variation of this:
Memory allocated by malloc() should be deallocated by free(), not operator
delete[].
I'm really not sure whether it's better to mention the deallocator's expected
allocator, or the allocated region's expected deallocator. Which mistake do you
think people are more likely to make?
(Also, I came up with that phrasing in ten seconds; it could totally change.)
The second one of course. I definitely prefer to print the allocated
region's expected deallocator.
The single downside I see is that this breaks the uniformity of reports
and complicate ReportBadFree(). (other reports about, for example,
freeing addresses of labels, could not be printed in the new fashion)
- << " from the start of memory allocated by malloc()";
+ << " from the start of memory allocated by ";
+ if (AllocExpr)
+ if(!printAllocDeallocName(os, C, AllocExpr))
+ os << "an allocator";
+ else
+ printExpectedAllocName(os, C, DeallocExpr);
This is also not quite as nice as it could be (same reason). (Also, the "else" case
doesn't match the right "if"!)
Oops!
+void testDeleteOp1() {
+ int *p = (int *)malloc(sizeof(int));
+ operator delete(p); // FIXME: should complain "Argument to operator delete() was
allocated by malloc(), not operator new"
+}
Hm. Any idea why this is not working? Is it not showing up as a standard
operator delete?
Nasty error.
There appears to be no RefState attached to a symbolic region for 'p'
when operator delete(p) is processed by FreeMemAux(). Everything works
fine if the call to operator delete is replaced by a delete expression
or free().
Debugging of the following example
void test() {
int *p = (int *)malloc(sizeof(int));
operator delete(p); // case 1
free(p); // case2
}
showed that in both cases all the addresses of SVals and regions remain
the same from one call to FreeMemAux() to another but in the first case
"const RefState *RsBase = State->get<RegionState>(SymBase);" returns 0.
The same thing happens for Objective-C methods.
The problem seem to lie somewhere in the guts of the analyzer.
The new patch attached.
--
Anton
Index: lib/StaticAnalyzer/Checkers/MallocChecker.cpp
===================================================================
--- lib/StaticAnalyzer/Checkers/MallocChecker.cpp (revision 176092)
+++ lib/StaticAnalyzer/Checkers/MallocChecker.cpp (working copy)
@@ -35,6 +35,14 @@
namespace {
+// Used to check correspondence between allocators and deallocators.
+enum AllocationFamilies {
+ AF_None,
+ AF_Malloc,
+ AF_CXXNew,
+ AF_CXXNewArray
+};
+
class RefState {
enum Kind { // Reference to allocated memory.
Allocated,
@@ -42,33 +50,42 @@
Released,
// The responsibility for freeing resources has transfered from
// this reference. A relinquished symbol should not be freed.
- Relinquished } K;
+ Relinquished };
+
const Stmt *S;
+ unsigned K : 2; // Kind enum, but stored as a bitfield.
+ unsigned Family : 30; // Rest of 32-bit word, currently just an allocation
+ // family.
+ RefState(Kind k, const Stmt *s, unsigned family)
+ : K(k), S(s), Family(family) {}
public:
- RefState(Kind k, const Stmt *s) : K(k), S(s) {}
-
bool isAllocated() const { return K == Allocated; }
bool isReleased() const { return K == Released; }
bool isRelinquished() const { return K == Relinquished; }
-
+ AllocationFamilies getAllocationFamily() const {
+ return (AllocationFamilies)Family;
+ }
const Stmt *getStmt() const { return S; }
bool operator==(const RefState &X) const {
- return K == X.K && S == X.S;
+ return K == X.K && S == X.S && Family == X.Family;
}
- static RefState getAllocated(const Stmt *s) {
- return RefState(Allocated, s);
+ static RefState getAllocated(unsigned family, const Stmt *s) {
+ return RefState(Allocated, s, family);
}
- static RefState getReleased(const Stmt *s) { return RefState(Released, s); }
+ static RefState getReleased(const Stmt *s) {
+ return RefState(Released, s, AF_None);
+ }
static RefState getRelinquished(const Stmt *s) {
- return RefState(Relinquished, s);
+ return RefState(Relinquished, s, AF_None);
}
void Profile(llvm::FoldingSetNodeID &ID) const {
ID.AddInteger(K);
ID.AddPointer(S);
+ ID.AddInteger(Family);
}
void dump(raw_ostream &OS) const {
@@ -120,6 +137,8 @@
check::PreStmt<ReturnStmt>,
check::PreStmt<CallExpr>,
check::PostStmt<CallExpr>,
+ check::PostStmt<CXXNewExpr>,
+ check::PreStmt<CXXDeleteExpr>,
check::PostStmt<BlockExpr>,
check::PostObjCMessage,
check::Location,
@@ -148,6 +167,8 @@
void checkPreStmt(const CallExpr *S, CheckerContext &C) const;
void checkPostStmt(const CallExpr *CE, CheckerContext &C) const;
+ void checkPostStmt(const CXXNewExpr *NE, CheckerContext &C) const;
+ void checkPreStmt(const CXXDeleteExpr *DE, 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;
@@ -168,31 +189,50 @@
private:
void initIdentifierInfo(ASTContext &C) const;
- /// Check if this is one of the functions which can allocate/reallocate memory
+ /// \brief Determine family of a deallocation expression.
+ AllocationFamilies getAllocationFamily(CheckerContext &C,
+ const Expr *E) const;
+
+ /// \brief Print names of allocators and deallocators.
+ ///
+ /// \returns true on success.
+ bool printAllocDeallocName(raw_ostream &os, CheckerContext &C,
+ const Expr *E) const;
+
+ /// \brief Print expected name of an allocator based on the deallocator's
+ /// family.
+ void printExpectedAllocName(raw_ostream &os, CheckerContext &C,
+ const Expr *DeallocExpr) const;
+
+ ///@{
+ /// Check if this is one of the functions which can allocate/reallocate memory
/// pointed to by one of its arguments.
bool isMemFunction(const FunctionDecl *FD, ASTContext &C) const;
bool isFreeFunction(const FunctionDecl *FD, ASTContext &C) const;
bool isAllocationFunction(const FunctionDecl *FD, ASTContext &C) const;
-
+ bool isStandardNewDelete(const FunctionDecl *FD, CheckerContext &C) const;
+ ///@}
static ProgramStateRef MallocMemReturnsAttr(CheckerContext &C,
const CallExpr *CE,
const OwnershipAttr* Att);
static ProgramStateRef MallocMemAux(CheckerContext &C, const CallExpr *CE,
const Expr *SizeEx, SVal Init,
- ProgramStateRef state) {
+ ProgramStateRef State,
+ AllocationFamilies Family = AF_Malloc) {
return MallocMemAux(C, CE,
- state->getSVal(SizeEx, C.getLocationContext()),
- Init, state);
+ State->getSVal(SizeEx, C.getLocationContext()),
+ Init, State, Family);
}
static ProgramStateRef MallocMemAux(CheckerContext &C, const CallExpr *CE,
SVal SizeEx, SVal Init,
- ProgramStateRef state);
+ ProgramStateRef State,
+ AllocationFamilies Family = AF_Malloc);
/// Update the RefState to reflect the new memory allocation.
- static ProgramStateRef MallocUpdateRefState(CheckerContext &C,
- const CallExpr *CE,
- ProgramStateRef state);
+ static ProgramStateRef
+ MallocUpdateRefState(CheckerContext &C, const Expr *E, ProgramStateRef State,
+ AllocationFamilies Family = AF_Malloc);
ProgramStateRef FreeMemAttr(CheckerContext &C, const CallExpr *CE,
const OwnershipAttr* Att) const;
@@ -225,8 +265,11 @@
static bool SummarizeValue(raw_ostream &os, SVal V);
static bool SummarizeRegion(raw_ostream &os, const MemRegion *MR);
- void ReportBadFree(CheckerContext &C, SVal ArgVal, SourceRange range) const;
- void ReportOffsetFree(CheckerContext &C, SVal ArgVal, SourceRange Range)const;
+ void ReportBadFree(CheckerContext &C, SVal ArgVal, SourceRange range,
+ const Expr *DeallocExpr, const Expr *AllocExpr = 0) const;
+ void ReportOffsetFree(CheckerContext &C, SVal ArgVal, SourceRange Range,
+ const Expr *DeallocExpr,
+ const Expr *AllocExpr = 0) const;
/// Find the location of the allocation for Sym on the path leading to the
/// exploded node N.
@@ -439,6 +482,39 @@
return false;
}
+bool MallocChecker::isStandardNewDelete(const FunctionDecl *FD,
+ CheckerContext &C) const {
+ if (!FD)
+ return false;
+
+ if (FD->getDeclName().getNameKind() != DeclarationName::CXXOperatorName)
+ return false;
+
+ OverloadedOperatorKind Kind = FD->getOverloadedOperator();
+ if (Kind != OO_New && Kind != OO_Array_New &&
+ Kind != OO_Delete && Kind != OO_Array_Delete)
+ return false;
+
+ // Skip custom new operators.
+ if (!FD->isImplicit() &&
+ !C.getSourceManager().isInSystemHeader(FD->getLocStart()))
+ return false;
+
+ // Return true if tested operator is a standard placement nothrow operator.
+ if (FD->getNumParams() == 2) {
+ QualType T = FD->getParamDecl(1)->getType();
+ if (const IdentifierInfo *II = T.getBaseTypeIdentifier())
+ return II->getName().equals("nothrow_t");
+ }
+
+ // Skip placement operators.
+ if (FD->getNumParams() != 1 || FD->isVariadic())
+ return false;
+
+ // One of the standard new/new[]/delete/delete[] non-placement operators.
+ return true;
+}
+
void MallocChecker::checkPostStmt(const CallExpr *CE, CheckerContext &C) const {
if (C.wasInlined)
return;
@@ -470,6 +546,22 @@
State = MallocUpdateRefState(C, CE, State);
} else if (FunI == II_strndup) {
State = MallocUpdateRefState(C, CE, State);
+ } else if (isStandardNewDelete(FD, C)) {
+ // Process direct calls to operator new/new[]/delete/delete[] functions
+ // as distinct from new/new[]/delete/delete[] expressions that are
+ // processed by the checkPostStmt callbacks for CXXNewExpr and
+ // CXXDeleteExpr.
+ OverloadedOperatorKind K = FD->getDeclName().getCXXOverloadedOperator();
+ if (K == OO_New)
+ State = MallocMemAux(C, CE, CE->getArg(0), UndefinedVal(), State,
+ AF_CXXNew);
+ else if (K == OO_Array_New)
+ State = MallocMemAux(C, CE, CE->getArg(0), UndefinedVal(), State,
+ AF_CXXNewArray);
+ else if (K == OO_Delete || K == OO_Array_Delete)
+ State = FreeMemAux(C, CE, State, 0, false, ReleasedAllocatedMemory);
+ else
+ llvm_unreachable("not a new/delete operator");
}
}
@@ -495,6 +587,33 @@
C.addTransition(State);
}
+void MallocChecker::checkPostStmt(const CXXNewExpr *NE,
+ CheckerContext &C) const {
+ if (!isStandardNewDelete(NE->getOperatorNew(), C))
+ return;
+
+ ProgramStateRef State = C.getState();
+ // The return value from operator new is already bound and we don't want to
+ // break this binding so we call MallocUpdateRefState instead of MallocMemAux.
+ State = MallocUpdateRefState(C, NE, State, NE->isArray() ? AF_CXXNewArray
+ : AF_CXXNew);
+ C.addTransition(State);
+}
+
+void MallocChecker::checkPreStmt(const CXXDeleteExpr *DE,
+ CheckerContext &C) const {
+
+ if (!isStandardNewDelete(DE->getOperatorDelete(), C))
+ return;
+
+ ProgramStateRef State = C.getState();
+ bool ReleasedAllocated;
+ State = FreeMemAux(C, DE->getArgument(), DE, State,
+ /*Hold*/false, ReleasedAllocated);
+
+ C.addTransition(State);
+}
+
static bool isFreeWhenDoneSetToZero(const ObjCMethodCall &Call) {
Selector S = Call.getSelector();
for (unsigned i = 1; i < S.getNumArgs(); ++i)
@@ -514,7 +633,6 @@
// 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.
bool ReleasedAllocatedMemory = false;
Selector S = Call.getSelector();
if ((S.getNameForSlot(0) == "dataWithBytesNoCopy" ||
@@ -547,7 +665,8 @@
ProgramStateRef MallocChecker::MallocMemAux(CheckerContext &C,
const CallExpr *CE,
SVal Size, SVal Init,
- ProgramStateRef state) {
+ ProgramStateRef State,
+ AllocationFamilies Family) {
// Bind the return value to the symbolic value from the heap region.
// TODO: We could rewrite post visit to eval call; 'malloc' does not have
@@ -557,14 +676,14 @@
const LocationContext *LCtx = C.getPredecessor()->getLocationContext();
DefinedSVal RetVal = svalBuilder.getConjuredHeapSymbolVal(CE, LCtx, Count)
.castAs<DefinedSVal>();
- state = state->BindExpr(CE, C.getLocationContext(), RetVal);
+ State = State->BindExpr(CE, C.getLocationContext(), RetVal);
// We expect the malloc functions to return a pointer.
if (!RetVal.getAs<Loc>())
return 0;
// Fill the region with the initialization value.
- state = state->bindDefault(RetVal, Init);
+ State = State->bindDefault(RetVal, Init);
// Set the region's extent equal to the Size parameter.
const SymbolicRegion *R =
@@ -576,20 +695,21 @@
SValBuilder &svalBuilder = C.getSValBuilder();
DefinedOrUnknownSVal Extent = R->getExtent(svalBuilder);
DefinedOrUnknownSVal extentMatchesSize =
- svalBuilder.evalEQ(state, Extent, *DefinedSize);
+ svalBuilder.evalEQ(State, Extent, *DefinedSize);
- state = state->assume(extentMatchesSize, true);
- assert(state);
+ State = State->assume(extentMatchesSize, true);
+ assert(State);
}
- return MallocUpdateRefState(C, CE, state);
+ return MallocUpdateRefState(C, CE, State, Family);
}
ProgramStateRef MallocChecker::MallocUpdateRefState(CheckerContext &C,
- const CallExpr *CE,
- ProgramStateRef state) {
+ const Expr *E,
+ ProgramStateRef State,
+ AllocationFamilies Family) {
// Get the return value.
- SVal retVal = state->getSVal(CE, C.getLocationContext());
+ SVal retVal = State->getSVal(E, C.getLocationContext());
// We expect the malloc functions to return a pointer.
if (!retVal.getAs<Loc>())
@@ -599,8 +719,7 @@
assert(Sym);
// Set the symbol's state to Allocated.
- return state->set<RegionState>(Sym, RefState::getAllocated(CE));
-
+ return State->set<RegionState>(Sym, RefState::getAllocated(Family, E));
}
ProgramStateRef MallocChecker::FreeMemAttr(CheckerContext &C,
@@ -652,6 +771,86 @@
return false;
}
+AllocationFamilies MallocChecker::getAllocationFamily(CheckerContext &C,
+ const Expr *E) const {
+ if (!E)
+ return AF_None;
+
+ if (const CallExpr *CE = dyn_cast<CallExpr>(E)) {
+ const FunctionDecl *FD = C.getCalleeDecl(CE);
+ ASTContext &Ctx = C.getASTContext();
+
+ if (isFreeFunction(FD, Ctx))
+ return AF_Malloc;
+
+ if (isStandardNewDelete(FD, C)) {
+ OverloadedOperatorKind Kind = FD->getOverloadedOperator();
+ if (Kind == OO_Delete)
+ return AF_CXXNew;
+ else if (Kind == OO_Array_Delete)
+ return AF_CXXNewArray;
+ }
+
+ return AF_None;
+ }
+
+ if (const CXXDeleteExpr *DE = dyn_cast<CXXDeleteExpr>(E))
+ return DE->isArrayForm() ? AF_CXXNewArray : AF_CXXNew;
+
+ if (isa<ObjCMessageExpr>(E))
+ return AF_Malloc;
+
+ return AF_None;
+}
+
+bool MallocChecker::printAllocDeallocName(raw_ostream &os, CheckerContext &C,
+ const Expr *E) const {
+ if (const CallExpr *CE = dyn_cast<CallExpr>(E)) {
+ // FIXME: This doesn't handle indirect calls.
+ const FunctionDecl *FD = CE->getDirectCallee();
+ if (!FD)
+ return false;
+
+ os << *FD;
+ if (!FD->isOverloadedOperator())
+ os << "()";
+ return true;
+ }
+
+ if (const ObjCMessageExpr *Msg = dyn_cast<ObjCMessageExpr>(E)) {
+ if (Msg->isInstanceMessage())
+ os << "-";
+ else
+ os << "+";
+ os << Msg->getSelector().getAsString();
+ return true;
+ }
+
+ if (const CXXNewExpr *NE = dyn_cast<CXXNewExpr>(E)) {
+ os << *NE->getOperatorNew();
+ return true;
+ }
+
+ if (const CXXDeleteExpr *DE = dyn_cast<CXXDeleteExpr>(E)) {
+ os << *DE->getOperatorDelete();
+ return true;
+ }
+
+ return false;
+}
+
+void MallocChecker::printExpectedAllocName(raw_ostream &os, CheckerContext &C,
+ const Expr *E) const {
+ AllocationFamilies Family = getAllocationFamily(C, E);
+
+ switch(Family) {
+ case AF_Malloc: os << "malloc()"; return;
+ case AF_CXXNew: os << "operator new"; return;
+ case AF_CXXNewArray: os << "operator new[]"; return;
+ case AF_None: llvm_unreachable("not a deallocation expression");
+ }
+}
+
ProgramStateRef MallocChecker::FreeMemAux(CheckerContext &C,
const Expr *ArgExpr,
const Expr *ParentExpr,
@@ -685,7 +884,7 @@
// Nonlocs can't be freed, of course.
// Non-region locations (labels and fixed addresses) also shouldn't be freed.
if (!R) {
- ReportBadFree(C, ArgVal, ArgExpr->getSourceRange());
+ ReportBadFree(C, ArgVal, ArgExpr->getSourceRange(), ParentExpr);
return 0;
}
@@ -693,13 +892,14 @@
// Blocks might show up as heap data, but should not be free()d
if (isa<BlockDataRegion>(R)) {
- ReportBadFree(C, ArgVal, ArgExpr->getSourceRange());
+ ReportBadFree(C, ArgVal, ArgExpr->getSourceRange(), ParentExpr);
return 0;
}
const MemSpaceRegion *MS = R->getMemorySpace();
- // Parameters, locals, statics, and globals shouldn't be freed.
+ // Parameters, locals, statics, globals, and memory returned by alloca()
+ // shouldn't be freed.
if (!(isa<UnknownSpaceRegion>(MS) || isa<HeapSpaceRegion>(MS))) {
// FIXME: at the time this code was written, malloc() regions were
// represented by conjured symbols, which are all in UnknownSpaceRegion.
@@ -709,7 +909,7 @@
// function, so UnknownSpaceRegion is always a possibility.
// False negatives are better than false positives.
- ReportBadFree(C, ArgVal, ArgExpr->getSourceRange());
+ ReportBadFree(C, ArgVal, ArgExpr->getSourceRange(), ParentExpr);
return 0;
}
@@ -746,6 +946,15 @@
return 0;
}
+ // Check if an expected deallocation function matches the real one.
+ if (RsBase &&
+ RsBase->getAllocationFamily() != AF_None &&
+ RsBase->getAllocationFamily() != getAllocationFamily(C, ParentExpr) ) {
+ const Expr *AllocExpr = cast<Expr>(RsBase->getStmt());
+ ReportBadFree(C, ArgVal, ArgExpr->getSourceRange(), ParentExpr, AllocExpr);
+ return 0;
+ }
+
// Check if the memory location being freed is the actual location
// allocated, or an offset.
RegionOffset Offset = R->getAsOffset();
@@ -753,7 +962,9 @@
Offset.isValid() &&
!Offset.hasSymbolicOffset() &&
Offset.getOffset() != 0) {
- ReportOffsetFree(C, ArgVal, ArgExpr->getSourceRange());
+ const Expr *AllocExpr = cast<Expr>(RsBase->getStmt());
+ ReportOffsetFree(C, ArgVal, ArgExpr->getSourceRange(), ParentExpr,
+ AllocExpr);
return 0;
}
@@ -868,38 +1079,54 @@
}
}
-void MallocChecker::ReportBadFree(CheckerContext &C, SVal ArgVal,
- SourceRange range) const {
+void MallocChecker::ReportBadFree(CheckerContext &C, SVal ArgVal,
+ SourceRange range, const Expr *DeallocExpr,
+ const Expr *AllocExpr) const {
if (ExplodedNode *N = C.generateSink()) {
if (!BT_BadFree)
BT_BadFree.reset(new BugType("Bad free", "Memory Error"));
SmallString<100> buf;
llvm::raw_svector_ostream os(buf);
-
+
const MemRegion *MR = ArgVal.getAsRegion();
- if (MR) {
- while (const ElementRegion *ER = dyn_cast<ElementRegion>(MR))
- MR = ER->getSuperRegion();
-
- // Special case for alloca()
- if (isa<AllocaRegion>(MR))
- os << "Argument to free() was allocated by alloca(), not malloc()";
- else {
- os << "Argument to free() is ";
- if (SummarizeRegion(os, MR))
- os << ", which is not memory allocated by malloc()";
+
+ os << "Argument to ";
+ if (!printAllocDeallocName(os, C, DeallocExpr))
+ os << "deallocator";
+ if (AllocExpr) {
+ SmallString<100> TempBuf;
+ llvm::raw_svector_ostream TempOs(TempBuf);
+
+ if (printAllocDeallocName(TempOs, C, AllocExpr))
+ os << " was allocated by " << TempOs.str() << ", not ";
+ else
+ os << " was not allocated by ";
+ } else {
+ if (MR) {
+ while (const ElementRegion *ER = dyn_cast<ElementRegion>(MR))
+ MR = ER->getSuperRegion();
+
+ // Special case for alloca().
+ if (isa<AllocaRegion>(MR))
+ os << " was allocated by alloca(), not ";
+ else {
+ os << " is ";
+ if (SummarizeRegion(os, MR))
+ os << ", which is not memory allocated by ";
+ else
+ os << "not memory allocated by ";
+ }
+ } else {
+ os << " is ";
+ if (SummarizeValue(os, ArgVal))
+ os << ", which is not memory allocated by ";
else
- os << "not memory allocated by malloc()";
+ os << "not memory allocated by ";
}
- } else {
- os << "Argument to free() is ";
- if (SummarizeValue(os, ArgVal))
- os << ", which is not memory allocated by malloc()";
- else
- os << "not memory allocated by malloc()";
}
-
+ printExpectedAllocName(os, C, DeallocExpr);
+
BugReport *R = new BugReport(*BT_BadFree, os.str(), N);
R->markInteresting(MR);
R->addRange(range);
@@ -908,7 +1135,8 @@
}
void MallocChecker::ReportOffsetFree(CheckerContext &C, SVal ArgVal,
- SourceRange Range) const {
+ SourceRange Range, const Expr *DeallocExpr,
+ const Expr *AllocExpr) const {
ExplodedNode *N = C.generateSink();
if (N == NULL)
return;
@@ -930,12 +1158,25 @@
int offsetBytes = Offset.getOffset() / C.getASTContext().getCharWidth();
- os << "Argument to free() is offset by "
+ os << "Argument to ";
+ if (!printAllocDeallocName(os, C, DeallocExpr))
+ os << "deallocator";
+ os << " is offset by "
<< offsetBytes
<< " "
<< ((abs(offsetBytes) > 1) ? "bytes" : "byte")
- << " from the start of memory allocated by malloc()";
+ << " from the start of ";
+ if (AllocExpr) {
+ SmallString<100> TempBuf;
+ llvm::raw_svector_ostream TempOs(TempBuf);
+ if (printAllocDeallocName(TempOs, C, AllocExpr))
+ os << "memory allocated by " << TempOs.str();
+ else
+ os << "allocated memory";
+ } else
+ printExpectedAllocName(os, C, DeallocExpr);
+
BugReport *R = new BugReport(*BT_OffsetFree, os.str(), N);
R->markInteresting(MR->getBaseRegion());
R->addRange(Range);
@@ -1343,7 +1584,7 @@
if (RS->isReleased()) {
if (I.getData().Kind == RPToBeFreedAfterFailure)
state = state->set<RegionState>(ReallocSym,
- RefState::getAllocated(RS->getStmt()));
+ RefState::getAllocated(RS->getAllocationFamily(), RS->getStmt()));
else if (I.getData().Kind == RPDoNotTrackAfterFailure)
state = state->remove<RegionState>(ReallocSym);
else
Index: test/Analysis/alloc-match-dealloc.cpp
===================================================================
--- test/Analysis/alloc-match-dealloc.cpp (revision 0)
+++ test/Analysis/alloc-match-dealloc.cpp (working copy)
@@ -0,0 +1,119 @@
+// RUN: %clang_cc1 -analyze -analyzer-checker=core,alpha.unix.MallocWithAnnotations -verify %s
+
+#include "Inputs/system-header-simulator-cxx.h"
+
+typedef __typeof__(sizeof(int)) size_t;
+void *malloc(size_t);
+void *realloc(void *ptr, size_t size);
+void *calloc(size_t nmemb, size_t size);
+char *strdup(const char *s);
+void __attribute((ownership_returns(malloc))) *my_malloc(size_t);
+
+void free(void *);
+void __attribute((ownership_takes(malloc, 1))) my_free(void *);
+
+//--------------------------------------------------------------
+// Test if an allocation function matches deallocation function
+//--------------------------------------------------------------
+
+//--------------- test delete expression
+void testDeleteExpr1() {
+ int *p = (int *)malloc(sizeof(int));
+ delete p; // expected-warning{{Argument to operator delete was allocated by malloc(), not operator new}}
+}
+
+void testDeleteExpr2() {
+ int *p = (int *)malloc(8);
+ int *q = (int *)realloc(p, 16);
+ delete q; // expected-warning{{Argument to operator delete was allocated by realloc(), not operator new}}
+}
+
+void testDeleteExpr3() {
+ int *p = (int *)calloc(1, sizeof(int));
+ delete p; // expected-warning{{Argument to operator delete was allocated by calloc(), not operator new}}
+}
+
+void testDeleteExpr4(const char *s) {
+ char *p = strdup(s);
+ delete p; // expected-warning{{Argument to operator delete was allocated by strdup(), not operator new}}
+}
+
+void testDeleteExpr5() {
+ int *p = (int *)my_malloc(sizeof(int));
+ delete p; // expected-warning{{Argument to operator delete was allocated by my_malloc(), not operator new}}
+}
+
+void testDeleteExpr6() {
+ int *p = (int *)__builtin_alloca(sizeof(int));
+ delete p; // expected-warning{{Argument to operator delete was allocated by alloca(), not operator new}}
+}
+
+void testDeleteExpr7() {
+ int *p = new int[1];
+ delete p; // expected-warning{{Argument to operator delete was allocated by operator new[], not operator new}}
+}
+
+void testDeleteExpr8() {
+ int *p = (int *)operator new[](0);
+ delete p; // expected-warning{{Argument to operator delete was allocated by operator new[], not operator new}}
+}
+
+//--------------- test operator delete
+void testDeleteOp1() {
+ int *p = (int *)malloc(sizeof(int));
+ operator delete(p); // FIXME: should complain "Argument to operator delete() was allocated by malloc(), not operator new"
+}
+
+//--------------- test delete[] expression
+void testDeleteArrayExpr1() {
+ int *p = (int *)malloc(sizeof(int));
+ delete[] p; // expected-warning{{Argument to operator delete[] was allocated by malloc(), not operator new[]}}
+}
+
+void testDeleteArrayExpr2() {
+ int *p = new int;
+ delete[] p; // expected-warning{{Argument to operator delete[] was allocated by operator new, not operator new[]}}
+}
+
+void testDeleteArrayExpr3() {
+ int *p = (int *)operator new(0);
+ delete[] p; // expected-warning{{Argument to operator delete[] was allocated by operator new, not operator new[]}}
+}
+
+//--------------- test operator delete[]
+void testDeleteArrayOp1() {
+ int *p = (int *)malloc(sizeof(int));
+ operator delete[](p); // FIXME: should complain "Argument to operator delete[]() was allocated by malloc(), not operator new"
+}
+
+//--------------- test free()
+void testFree1() {
+ int *p = new int;
+ free(p); // expected-warning{{Argument to free() was allocated by operator new, not malloc()}}
+}
+
+void testFree2() {
+ int *p = (int *)operator new(0);
+ free(p); // expected-warning{{Argument to free() was allocated by operator new, not malloc()}}
+}
+
+void testFree3() {
+ int *p = new int[1];
+ free(p); // expected-warning{{Argument to free() was allocated by operator new[], not malloc()}}
+}
+
+//--------------- test realloc()
+void testRealloc1() {
+ int *p = new int;
+ realloc(p, sizeof(long)); // expected-warning{{Argument to realloc() was allocated by operator new, not malloc()}}
+}
+
+void testRealloc2() {
+ int *p = (int *)operator new(0);
+ realloc(p, sizeof(long)); // expected-warning{{Argument to realloc() was allocated by operator new, not malloc()}}
+}
+
+void testRealloc3() {
+ int *p = new int[1];
+ realloc(p, sizeof(long)); // expected-warning{{Argument to realloc() was allocated by operator new[], not malloc()}}
+}
Index: test/Analysis/inline.cpp
===================================================================
--- test/Analysis/inline.cpp (revision 175982)
+++ test/Analysis/inline.cpp (working copy)
@@ -279,6 +279,7 @@
IntWrapper *obj = new IntWrapper(42);
// should be TRUE
clang_analyzer_eval(obj->value == 42); // expected-warning{{UNKNOWN}}
+ delete obj;
}
void testPlacement() {
Index: test/Analysis/Inputs/system-header-simulator-cxx.h
===================================================================
--- test/Analysis/Inputs/system-header-simulator-cxx.h (revision 175982)
+++ test/Analysis/Inputs/system-header-simulator-cxx.h (working copy)
@@ -59,4 +59,28 @@
return 0;
}
};
+
+ class bad_alloc : public exception {
+ public:
+ bad_alloc() throw();
+ bad_alloc(const bad_alloc&) throw();
+ bad_alloc& operator=(const bad_alloc&) throw();
+ virtual const char* what() const throw() {
+ return 0;
+ }
+ };
+
+ struct nothrow_t {};
+
+ extern const nothrow_t nothrow;
}
+
+void* operator new(std::size_t, const std::nothrow_t&) throw();
+void* operator new[](std::size_t, const std::nothrow_t&) throw();
+void operator delete(void*, const std::nothrow_t&) throw();
+void operator delete[](void*, const std::nothrow_t&) throw();
+
+void* operator new (std::size_t size, void* ptr) throw() { return ptr; };
+void* operator new[] (std::size_t size, void* ptr) throw() { return ptr; };
+void operator delete (void* ptr, void*) throw() {};
+void operator delete[] (void* ptr, void*) throw() {};
Index: test/Analysis/new.cpp
===================================================================
--- test/Analysis/new.cpp (revision 175982)
+++ test/Analysis/new.cpp (working copy)
@@ -1,4 +1,5 @@
// RUN: %clang_cc1 -analyze -analyzer-checker=core,unix.Malloc,debug.ExprInspection -analyzer-store region -std=c++11 -verify %s
+#include "Inputs/system-header-simulator-cxx.h"
void clang_analyzer_eval(bool);
@@ -19,13 +20,6 @@
clang_analyzer_eval(someGlobal == 0); // expected-warning{{TRUE}}
}
-
-// This is the standard placement new.
-inline void* operator new(size_t, void* __p) throw()
-{
- return __p;
-}
-
void *testPlacementNew() {
int *x = (int *)malloc(sizeof(int));
*x = 1;
@@ -73,7 +67,6 @@
clang_analyzer_eval(*n == 0); // expected-warning{{TRUE}}
}
-
struct PtrWrapper {
int *x;
@@ -85,7 +78,84 @@
return new PtrWrapper(static_cast<int *>(malloc(4)));
}
+//--------------------------------
+// unix.Malloc checks
+//--------------------------------
+void testGlobalExprNewBeforeOverload1() {
+ int *p = new int;
+} // expected-warning{{Memory is never released; potential leak}}
+
+void testGlobalExprNewBeforeOverload2() {
+ int *p = ::new int;
+} // expected-warning{{Memory is never released; potential leak}}
+
+void testGlobalOpNewBeforeOverload() {
+ void *p = operator new(0);
+} // expected-warning{{Memory is never released; potential leak}}
+
+void *operator new(size_t);
+void *operator new(size_t, double d);
+void *operator new[](size_t);
+void *operator new[](size_t, double d);
+
+void testExprPlacementNew() {
+ int i;
+ int *p1 = new(&i) int; // no warn - standard placement new
+
+ int *p2 = new(1.0) int; // no warn - overloaded placement new
+
+ int *p3 = new (std::nothrow) int;
+} // expected-warning{{Memory is never released; potential leak}}
+
+void testExprPlacementNewArray() {
+ int i;
+ int *p1 = new(&i) int[1]; // no warn - standard placement new[]
+
+ int *p2 = new(1.0) int[1]; // no warn - overloaded placement new[]
+
+ int *p3 = new (std::nothrow) int[1];
+} // expected-warning{{Memory is never released; potential leak}}
+
+void testCustomOpNew() {
+ void *p = operator new(0); // no warn - call to a custom new
+}
+
+void testGlobalExprNew() {
+ void *p = ::new int; // no warn - call to a custom new
+}
+
+void testCustomExprNew() {
+ int *p = new int; // no warn - call to a custom new
+}
+
+void testGlobalExprNewArray() {
+ void *p = ::new int[1]; // no warn - call to a custom new
+}
+
+void testOverloadedExprNewArray() {
+ int *p = new int[1]; // no warn - call to a custom new
+}
+
+// test if unix.Malloc processes operator delete
+void testExprDeleteArg() {
+ int i;
+ delete &i; // expected-warning{{Argument to operator delete is the address of the local variable 'i', which is not memory allocated by operator new}}
+}
+
+// test if unix.Malloc handles operator delete[]
+void testExprDeleteArrArg() {
+ int i;
+ delete[] &i; // expected-warning{{Argument to operator delete[] is the address of the local variable 'i', which is not memory allocated by operator new[]}}
+}
+
+// test for proper allocator/deallocator names in the warning
+void testAllocDeallocNames() {
+ int *p = new(std::nothrow) int[1];
+ p += 1;
+ delete[] (p); // expected-warning{{Argument to operator delete[] is offset by 4 bytes from the start of memory allocated by operator new[]}}
+}
+
//--------------------------------
// Incorrectly-modelled behavior
//--------------------------------
@@ -95,8 +165,10 @@
// Should warn that *n is uninitialized.
if (*n) { // no-warning
+ delete n;
return 0;
}
+ delete n;
return 1;
}
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits