On 26.02.2013 6:31, Jordan Rose wrote:
Okay, starting over. Anna went and explained to me that we really do want to be planning
for the future case, where we allow people to specify their own custom allocation
families, and that we should start doing that now. (Sorry for sending you in the wrong
direction.) The design we hashed out is similar to what you came up with, but with a
change in focus towards what we're calling "families", which may have multiple
allocators and deallocators.
class RefState {
const Stmt *S;
unsigned State : 2; // Kind enum, but stored as a bitfield
unsigned Family : 30; // Rest of 32-bit word, currently just an ID
// ...
};
enum AllocationFamilies {
AF_None,
AF_Malloc,
AF_CXXNew,
AF_CXXNewArray,
AF_FirstUserFamily
};
That's all for now, but if/when we support user annotations for
allocations/deallocations, we can add a table to MallocChecker (not globally):
mutable llvm::SmallDenseMap<IdentifierInfo *, > FamilyIDTable;
mutable unsigned NextUserFamily = AF_FirstUserFamily;
and prepopulate it with any families we want, such as, say, "malloc". (See the old
malloc-annotations test case.) When we come across a family we haven't seen before, we add it to
the table and increment the "NextUserFamily" counter. (We don't need to design most of
this now.)
This does have the downside that you pointed out: in the error message at the end, we
can't say "memory was allocated by malloc()" if malloc is called indirectly.
Anna convinced me that this is acceptable, especially if we do rephrase the diagnostic
when we don't have a good message to print, because we'll still have the path note that
shows where the allocation happened. We can still rederive the allocator name from the
source statement.
We'll also have trouble printing out an archetypical allocator or deallocator in the
"user-defined family" case, but we can design that later. Anna and I tossed
around a few ideas but didn't settle on anything.
On the other hand, this keeps RefState a little more compact, and avoids
string-based lookups for function names when many functions don't have names.
Other issues: I now understand why you decided to check new/delete within
classes; excluding all cases with placement args is probably a good balance.
(LLVM uses placement-arg-based allocators for POD types on a BumpPtrAllocator,
for example.) Sorry for not getting it sooner.
Stepping back, there are many problems with this, the main one being that just
keeping track of the function name isn't really good enough.
MacOSKeychainAPIChecker can get away with it because its functions are
Could you, please, send the remainder of the sentence? Tried to guess, but
failed.
Since I have been convinced otherwise, this is less relevant, but I think I was
saying that MacOSKeychainAPIChecker can get away with this because its
functions are all simple C functions in the global namespace, whereas a general
alloc/dealloc checker needs to handle functions in namespaces, C++ instance
methods, ObjC methods...all sorts of things where a simple name can't
disambiguate.
Returning DK_free for Objective-C messages just mean that they belong to the
'malloc/free' family. Consider the following example:
void test() {
int *p = (int *)malloc(sizeof(int));
[NSData dataWithBytesNoCopy:bytes length:sizeof(int)];
}
Ha, I forgot about free-when-done; carry on. Not entirely happy with assuming
all of ObjC belongs to malloc, but we can revisit this when we have other
interesting families.
I'm deliberately not going to comment on your latest patch because I'd rather
have your feedback on this design. What do you think?
Jordan
Families are just what I thought about, but postponed the refactoring.
I am slightly suspicious of bit-fields as too much is
implementation-defined (again, when I used the enum type 'Kind' instead
of the 'unsigned' in 'unsigned State : 2' the State failed to hold enum
values for some reason. Haven't dig this, maybe the case is related to
"A bit-field shall have integral or enumeration type (3.9.1). It is
implementation-defined whether a plain (neither explicitly signed nor
unsigned) char, short, int, long, or long long bit-field is signed or
unsigned."), but this approach is much more convenient and readable.
The current solution handles the majority of cases. It is also a good
step to approach with a dynamic table that will eliminate the downsides
of a current approach and enable support of user-defined families.
I am for getting this in and moving towards the approach with dynamic
tables as a next step.
Fresh patch is 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,52 @@
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 deallocators
+ /// 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);
+ 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 +267,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 +484,40 @@
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->getDeclName().getCXXOverloadedOperator();
+ 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().getUnqualifiedType();
+ 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 +549,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 +590,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 = false;
+ 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 +636,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 +668,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
@@ -582,14 +704,15 @@
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 +722,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 +774,87 @@
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->getDeclName().getCXXOverloadedOperator();
+ 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("unknown allocation family");
+ }
+}
+
ProgramStateRef MallocChecker::FreeMemAux(CheckerContext &C,
const Expr *ArgExpr,
const Expr *ParentExpr,
@@ -685,7 +888,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 +896,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 +913,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 +950,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 +966,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 +1083,51 @@
}
}
-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 << "a deallocator";
+ if (AllocExpr) {
+ os << " was allocated by ";
+ if(!printAllocDeallocName(os, C, AllocExpr))
+ os << "an unknown allocator";
+ os << ", not ";
+ } 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 +1136,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,11 +1159,19 @@
int offsetBytes = Offset.getOffset() / C.getASTContext().getCharWidth();
- os << "Argument to free() is offset by "
+ os << "Argument to ";
+ if (!printAllocDeallocName(os, C, DeallocExpr))
+ os << "a deallocator";
+ os << " is offset by "
<< offsetBytes
<< " "
<< ((abs(offsetBytes) > 1) ? "bytes" : "byte")
- << " 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);
BugReport *R = new BugReport(*BT_OffsetFree, os.str(), N);
R->markInteresting(MR->getBaseRegion());
@@ -1343,7 +1580,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
