Hi all,
the attached patch has three goals:
1) Bring handling of new/delete operators to the unix.Malloc checker.
This enables memory.LeakNeverReleased, memory.MultipleDelete,
memory.DeallocateNonPtr and memory.LeakPtrValChanged checkers from the
List of potential checkers
<http://clang-analyzer.llvm.org/potential_checkers.html> to check memory
allocated with new/delete. PR15237
<http://llvm.org/bugs/show_bug.cgi?id=15237> created to track progress
on the task.
2) Add memory.MismatchedFree and memory.MismatchedDelete checks. (
PR15238 <http://llvm.org/bugs/show_bug.cgi?id=15238> )
3) Display real allocator/deallocator names instead of hardcoded
'malloc()' and 'free()' if possible.
Vital for 1) and 2)
please review
--
Anton
Index: lib/StaticAnalyzer/Checkers/MallocChecker.cpp
===================================================================
--- lib/StaticAnalyzer/Checkers/MallocChecker.cpp (revision 174894)
+++ lib/StaticAnalyzer/Checkers/MallocChecker.cpp (working copy)
@@ -115,11 +115,33 @@
typedef std::pair<const ExplodedNode*, const MemRegion*> LeakInfo;
+enum AllocDeallocKind {
+ AD_malloc,
+ AD_new,
+ AD_newArr,
+ AD_free,
+ AD_delete,
+ AD_deleteArr,
+ AD_unknown
+};
+
+static const char *AllocDeallocNames[] = {
+ "malloc()",
+ "operator new",
+ "operator new[]",
+ "free()",
+ "operator delete",
+ "operator delete[]",
+ "unknown function"
+};
+
class MallocChecker : public Checker<check::DeadSymbols,
check::PointerEscape,
check::PreStmt<ReturnStmt>,
check::PreStmt<CallExpr>,
check::PostStmt<CallExpr>,
+ check::PostStmt<CXXNewExpr>,
+ check::PreStmt<CXXDeleteExpr>,
check::PostStmt<BlockExpr>,
check::PostObjCMessage,
check::Location,
@@ -148,6 +170,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,6 +192,26 @@
private:
void initIdentifierInfo(ASTContext &C) const;
+ /// Auxiliary functions that return kind and name of allocators/deallocators
+ /// and check if the type of an allocator matches the type of deallocator
+ AllocDeallocKind GetAllocDeallocKind(CheckerContext &C, const Expr *E) const;
+ AllocDeallocKind GetProperAllocKind(AllocDeallocKind DeallocKind) const;
+ AllocDeallocKind GetProperAllocKind(CheckerContext &C,
+ const Expr *DeallocExpr) const {
+ return GetProperAllocKind(GetAllocDeallocKind(C, DeallocExpr));
+ }
+ bool AllocMatchesDealloc(CheckerContext &C,
+ const Expr *AllocExpr,
+ const Expr *DeallocExpr) const;
+ std::string GetAllocDeallocName(AllocDeallocKind kind) const {
+ return AllocDeallocNames[kind];
+ }
+ std::string GetAllocDeallocName(CheckerContext &C, const Expr *E) const;
+ std::string GetProperAllocName(CheckerContext &C,
+ const Expr *DeallocExpr) const {
+ return GetAllocDeallocName(GetProperAllocKind(C, DeallocExpr));
+ }
+
/// 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;
@@ -191,7 +235,7 @@
/// Update the RefState to reflect the new memory allocation.
static ProgramStateRef MallocUpdateRefState(CheckerContext &C,
- const CallExpr *CE,
+ const Expr *E,
ProgramStateRef state);
ProgramStateRef FreeMemAttr(CheckerContext &C, const CallExpr *CE,
@@ -225,8 +269,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.
@@ -495,6 +542,31 @@
C.addTransition(State);
}
+void MallocChecker::checkPostStmt(const CXXNewExpr *NE,
+ CheckerContext &C) const {
+
+ // skip placement new operators as they may not allocate memory
+ if (NE->getNumPlacementArgs())
+ 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);
+
+ C.addTransition(State);
+}
+
+void MallocChecker::checkPreStmt(const CXXDeleteExpr *DE,
+ CheckerContext &C) const {
+ 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)
@@ -586,10 +658,10 @@
}
ProgramStateRef MallocChecker::MallocUpdateRefState(CheckerContext &C,
- const CallExpr *CE,
+ const Expr *E,
ProgramStateRef state) {
// 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 (!isa<Loc>(retVal))
@@ -599,7 +671,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(E));
}
@@ -652,6 +724,85 @@
return false;
}
+AllocDeallocKind MallocChecker::GetAllocDeallocKind(CheckerContext &C,
+ const Expr *E) const {
+ if (!E)
+ return AD_unknown;
+
+ if (isa<ObjCMessageExpr>(E))
+ return AD_malloc;
+
+ if (const CallExpr *CE = dyn_cast_or_null<CallExpr>(E)) {
+ const FunctionDecl *FD = CE->getDirectCallee();
+ ASTContext &Ctx = C.getASTContext();
+
+ if (isAllocationFunction(FD, Ctx))
+ return AD_malloc;
+
+ if (isFreeFunction(FD, Ctx))
+ return AD_free;
+
+ return AD_unknown;
+ }
+
+ if (const CXXNewExpr *NE = dyn_cast_or_null<CXXNewExpr>(E))
+ return NE->isArray() ? AD_newArr : AD_new;
+
+ if (const CXXDeleteExpr *DE = dyn_cast_or_null<CXXDeleteExpr>(E))
+ return DE->isArrayForm() ? AD_deleteArr : AD_delete;
+
+ return AD_unknown;
+}
+
+AllocDeallocKind
+MallocChecker::GetProperAllocKind(AllocDeallocKind DeallocKind) const {
+ switch(DeallocKind) {
+ case AD_malloc: // realloc and ObjC methods case
+ case AD_free: return AD_malloc;
+ case AD_delete: return AD_new;
+ case AD_deleteArr: return AD_newArr;
+ default: return AD_unknown;
+ }
+
+ return AD_unknown;
+}
+
+bool MallocChecker::AllocMatchesDealloc(CheckerContext &C,
+ const Expr *AllocExpr,
+ const Expr *DeallocExpr) const {
+
+ AllocDeallocKind AllocKind = GetAllocDeallocKind(C, AllocExpr);
+ AllocDeallocKind DeallocKind = GetAllocDeallocKind(C, DeallocExpr);
+ AllocDeallocKind ProperAllocKind = GetProperAllocKind(DeallocKind);
+
+ return ((AllocKind != AD_unknown) && (AllocKind == ProperAllocKind));
+}
+
+std::string
+MallocChecker::GetAllocDeallocName(CheckerContext &C, const Expr *E) const {
+
+ AllocDeallocKind kind = GetAllocDeallocKind(C, E);
+
+ // specify an allocator
+ if (kind == AD_malloc) {
+ // get the exact name of an allocation function
+ if (const CallExpr *CE = dyn_cast_or_null<CallExpr>(E)) {
+ if (const FunctionDecl *FD = CE->getDirectCallee()) {
+ if (FD->getKind() == Decl::Function) {
+ if (IdentifierInfo *II = FD->getIdentifier()) {
+ std::string name = II->getName().str() + "()";
+ return name;
+ }
+ }
+ }
+ } else if (isa<ObjCMessageExpr>(E)) {
+ return "ObjC method";
+ }
+ }
+
+ return GetAllocDeallocName(kind);
+}
+
ProgramStateRef MallocChecker::FreeMemAux(CheckerContext &C,
const Expr *ArgExpr,
const Expr *ParentExpr,
@@ -685,7 +836,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 +844,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 +861,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 +898,16 @@
return 0;
}
+ if (RsBase) {
+ // Check if an allocation function matches deallocation function
+ const Expr *AllocExpr = cast<Expr>(RsBase->getStmt());
+ if (!AllocMatchesDealloc(C, AllocExpr, ParentExpr)) {
+ 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 +915,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 +1032,52 @@
}
}
-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 /* = 0 */) 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()";
+
+ std::string ProperAllocName = GetProperAllocName(C, DeallocExpr);
+ std::string DeallocName = GetAllocDeallocName(C, DeallocExpr);
+
+ if (AllocExpr) {
+ assert(!AllocMatchesDealloc(C, AllocExpr, DeallocExpr) &&
+ "'allocExpr != 0' means 'alloc does not match dealloc'");
+ std::string AllocName = GetAllocDeallocName(C, AllocExpr);
+ os << "Argument to " << DeallocName << " was allocated by " << AllocName
+ << ", not " << ProperAllocName;
+ } else {
+ if (MR) {
+ while (const ElementRegion *ER = dyn_cast<ElementRegion>(MR))
+ MR = ER->getSuperRegion();
+
+ // Special case for alloca()
+ if (isa<AllocaRegion>(MR))
+ os << "Argument to " << DeallocName
+ << " was allocated by alloca(), not " << ProperAllocName;
+ else {
+ os << "Argument to " << DeallocName << " is ";
+ if (SummarizeRegion(os, MR))
+ os << ", which is not memory allocated by " << ProperAllocName;
+ else
+ os << "not memory allocated by " << ProperAllocName;
+ }
+ } else {
+ os << "Argument to " << DeallocName << " is ";
+ if (SummarizeValue(os, ArgVal))
+ os << ", which is not memory allocated by " << ProperAllocName;
else
- os << "not memory allocated by malloc()";
+ os << "not memory allocated by " << ProperAllocName;
}
- } 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()";
}
-
+
BugReport *R = new BugReport(*BT_BadFree, os.str(), N);
R->markInteresting(MR);
R->addRange(range);
@@ -908,7 +1086,8 @@
}
void MallocChecker::ReportOffsetFree(CheckerContext &C, SVal ArgVal,
- SourceRange Range) const {
+ SourceRange Range, const Expr *DeallocExpr,
+ const Expr *AllocExpr /* = 0 */) const {
ExplodedNode *N = C.generateSink();
if (N == NULL)
return;
@@ -930,11 +1109,15 @@
int offsetBytes = Offset.getOffset() / C.getASTContext().getCharWidth();
- os << "Argument to free() is offset by "
+ std::string AllocName = AllocExpr ? GetAllocDeallocName(C, AllocExpr)
+ : GetProperAllocName(C, DeallocExpr);
+ std::string DeallocName = GetAllocDeallocName(C, DeallocExpr);
+
+ os << "Argument to " << DeallocName << " is offset by "
<< offsetBytes
<< " "
<< ((abs(offsetBytes) > 1) ? "bytes" : "byte")
- << " from the start of memory allocated by malloc()";
+ << " from the start of memory allocated by " << AllocName;
BugReport *R = new BugReport(*BT_OffsetFree, os.str(), N);
R->markInteresting(MR->getBaseRegion());
Index: test/Analysis/inline.cpp
===================================================================
--- test/Analysis/inline.cpp (revision 174894)
+++ test/Analysis/inline.cpp (working copy)
@@ -255,6 +255,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 174894)
+++ test/Analysis/Inputs/system-header-simulator-cxx.h (working copy)
@@ -59,4 +59,24 @@
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 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 174894)
+++ 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,61 @@
return new PtrWrapper(static_cast<int *>(malloc(4)));
}
+//--------------------------------
+// unix.Malloc checks
+//--------------------------------
+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 testPlacementNewLeak() {
+ int i;
+ int *p0 = new(&i) int; // no warn - standard placement new
+
+ int *p1 = new(1.0) int; // no warn - overloaded placement new
+
+ int *p2 = new(&i) int[1]; // no warn - standard placement new[]
+
+ int *p3 = new(1.0) int[1]; // no warn - overloaded placement new[]
+}
+
+void testGlobalNewLeak() {
+ int *p = ::new int;
+} // expected-warning{{Memory is never released; potential leak}}
+
+void testOverloadedNewLeak() {
+ int *p = new int;
+} // expected-warning{{Memory is never released; potential leak}}
+
+void testGlobalNewArrLeak() {
+ int *p = ::new int[1];
+} // expected-warning{{Memory is never released; potential leak}}
+
+void testOverloadedNewArrLeak() {
+ int *p = new int[1];
+} // expected-warning{{Memory is never released; potential leak}}
+
+// test if unix.Malloc processes operator delete
+void testDeleteArg() {
+ 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 testDeleteArrArg() {
+ 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 int[2];
+ 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 +142,10 @@
// Should warn that *n is uninitialized.
if (*n) { // no-warning
+ delete n;
return 0;
}
+ delete n;
return 1;
}
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,87 @@
+// 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 operator delete
+void test1() {
+ int *p = (int *)malloc(sizeof(int));
+ delete p; // expected-warning{{Argument to operator delete was allocated by malloc(), not operator new}}
+}
+
+void test2() {
+ 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 test3() {
+ int *p = (int *)calloc(1, sizeof(int));
+ delete p; // expected-warning{{Argument to operator delete was allocated by calloc(), not operator new}}
+}
+
+void test4(const char *s) {
+ char *p = strdup(s);
+ delete p; // expected-warning{{Argument to operator delete was allocated by strdup(), not operator new}}
+}
+
+void test5() {
+ int *p = (int *)my_malloc(sizeof(int));
+ delete p; // expected-warning{{Argument to operator delete was allocated by my_malloc(), not operator new}}
+}
+
+void test6() {
+ int *p = (int *)__builtin_alloca(sizeof(int));
+ delete p; // expected-warning{{Argument to operator delete was allocated by alloca(), not operator new}}
+}
+
+void test7() {
+ int *p = new int[1];
+ delete p; // expected-warning{{Argument to operator delete was allocated by operator new[], not operator new}}
+}
+
+//--------------- test operator delete[]
+void test8() {
+ int *p = (int *)malloc(sizeof(int));
+ delete[] p; // expected-warning{{Argument to operator delete[] was allocated by malloc(), not operator new[]}}
+}
+
+void test9() {
+ int *p = new int;
+ delete[] p; // expected-warning{{Argument to operator delete[] was allocated by operator new, not operator new[]}}
+}
+
+//--------------- test free()
+void test10() {
+ int *p = new int;
+ free(p); // expected-warning{{Argument to free() was allocated by operator new, not malloc()}}
+}
+
+void test11() {
+ int *p = new int[1];
+ free(p); // expected-warning{{Argument to free() was allocated by operator new[], not malloc()}}
+}
+
+//--------------- test realloc()
+void test12() {
+ int *p = new int;
+ realloc(p, sizeof(long)); // expected-warning{{Argument to realloc() was allocated by operator new, not malloc()}}
+}
+
+void test13() {
+ int *p = new int[1];
+ realloc(p, sizeof(long)); // expected-warning{{Argument to realloc() was allocated by operator new[], not malloc()}}
+}
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
