Index: clang/include/clang/StaticAnalyzer/Checkers/Checkers.td =================================================================== --- clang/include/clang/StaticAnalyzer/Checkers/Checkers.td +++ clang/include/clang/StaticAnalyzer/Checkers/Checkers.td @@ -489,7 +489,7 @@ def NewDeleteLeaksChecker : Checker<"NewDeleteLeaks">, HelpText<"Check for memory leaks. Traces memory managed by new/delete.">, - Dependencies<[NewDeleteChecker]>, + Dependencies<[DynamicMemoryModeling]>, Documentation; def PlacementNewChecker : Checker<"PlacementNew">, Index: clang/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h =================================================================== --- clang/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h +++ clang/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h @@ -175,8 +175,7 @@ /// @param Pred The transition will be generated from the specified Pred node /// to the newly generated node. /// @param Tag The tag to uniquely identify the creation site. - ExplodedNode *addTransition(ProgramStateRef State, - ExplodedNode *Pred, + ExplodedNode *addTransition(ProgramStateRef State, ExplodedNode *Pred, const ProgramPointTag *Tag = nullptr) { return addTransitionImpl(State, false, Pred, Tag); } @@ -189,6 +188,14 @@ return addTransitionImpl(State ? State : getState(), true, Pred, Tag); } + /// Add a sink node to the current path of execution, halting analysis. + void addSink(ProgramStateRef State = nullptr, + const ProgramPointTag *Tag = nullptr) { + // Say this 3 times fast. + State = State ? State : getState(); + addTransition(State, generateSink(State, getPredecessor())); + } + /// Generate a transition to a node that will be used to report /// an error. This node will be a sink. That is, it will stop exploration of /// the given path. Index: clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp =================================================================== --- clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp +++ clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp @@ -1741,6 +1741,15 @@ const MemRegion *R = ArgVal.getAsRegion(); const Expr *ParentExpr = Call.getOriginExpr(); + // NOTE: There is a philosophical question to be answered when we detect a + // bug, but the checker under whose name we would emit the error is disabled. + // Generally speaking, the MallocChecker family is an integral part of the + // Static Analyzer, and disabling any part of it should only be done under + // exceptional circumstances, such as frequent false positives. If this is the + // case, we can reasonably believe that there are serious faults in our + // understanding of the source code, and even if we don't emit an warning, we + // should terminate further analysis with a sink node. + // Nonlocs can't be freed, of course. // Non-region locations (labels and fixed addresses) also shouldn't be freed. if (!R) { @@ -2011,9 +2020,10 @@ SourceRange Range, const Expr *DeallocExpr, AllocationFamily Family) const { - if (!ChecksEnabled[CK_MallocChecker] && - !ChecksEnabled[CK_NewDeleteChecker]) + if (!ChecksEnabled[CK_MallocChecker] && !ChecksEnabled[CK_NewDeleteChecker]) { + C.addSink(); return; + } Optional CheckKind = getCheckIfTracked(Family); if (!CheckKind.hasValue()) @@ -2062,8 +2072,10 @@ CheckKind = CK_MallocChecker; else if (ChecksEnabled[CK_MismatchedDeallocatorChecker]) CheckKind = CK_MismatchedDeallocatorChecker; - else + else { + C.addSink(); return; + } if (ExplodedNode *N = C.generateErrorNode()) { if (!BT_FreeAlloca[*CheckKind]) @@ -2086,8 +2098,10 @@ SymbolRef Sym, bool OwnershipTransferred) const { - if (!ChecksEnabled[CK_MismatchedDeallocatorChecker]) + if (!ChecksEnabled[CK_MismatchedDeallocatorChecker]) { + C.addSink(); return; + } if (ExplodedNode *N = C.generateErrorNode()) { if (!BT_MismatchedDealloc) @@ -2140,9 +2154,10 @@ AllocationFamily Family, const Expr *AllocExpr) const { - if (!ChecksEnabled[CK_MallocChecker] && - !ChecksEnabled[CK_NewDeleteChecker]) + if (!ChecksEnabled[CK_MallocChecker] && !ChecksEnabled[CK_NewDeleteChecker]) { + C.addSink(); return; + } Optional CheckKind = getCheckIfTracked(Family); if (!CheckKind.hasValue()) @@ -2195,10 +2210,11 @@ void MallocChecker::ReportUseAfterFree(CheckerContext &C, SourceRange Range, SymbolRef Sym) const { - if (!ChecksEnabled[CK_MallocChecker] && - !ChecksEnabled[CK_NewDeleteChecker] && - !ChecksEnabled[CK_InnerPointerChecker]) + if (!ChecksEnabled[CK_MallocChecker] && !ChecksEnabled[CK_NewDeleteChecker] && + !ChecksEnabled[CK_InnerPointerChecker]) { + C.addSink(); return; + } Optional CheckKind = getCheckIfTracked(C, Sym); if (!CheckKind.hasValue()) @@ -2234,9 +2250,10 @@ bool Released, SymbolRef Sym, SymbolRef PrevSym) const { - if (!ChecksEnabled[CK_MallocChecker] && - !ChecksEnabled[CK_NewDeleteChecker]) + if (!ChecksEnabled[CK_MallocChecker] && !ChecksEnabled[CK_NewDeleteChecker]) { + C.addSink(); return; + } Optional CheckKind = getCheckIfTracked(C, Sym); if (!CheckKind.hasValue()) @@ -2263,8 +2280,10 @@ void MallocChecker::ReportDoubleDelete(CheckerContext &C, SymbolRef Sym) const { - if (!ChecksEnabled[CK_NewDeleteChecker]) + if (!ChecksEnabled[CK_NewDeleteChecker]) { + C.addSink(); return; + } Optional CheckKind = getCheckIfTracked(C, Sym); if (!CheckKind.hasValue()) @@ -2289,9 +2308,10 @@ SourceRange Range, SymbolRef Sym) const { - if (!ChecksEnabled[CK_MallocChecker] && - !ChecksEnabled[CK_NewDeleteChecker]) + if (!ChecksEnabled[CK_MallocChecker] && !ChecksEnabled[CK_NewDeleteChecker]) { + C.addSink(); return; + } Optional CheckKind = getCheckIfTracked(C, Sym); @@ -2320,8 +2340,10 @@ SourceRange Range, const Expr *FreeExpr, AllocationFamily Family) const { - if (!ChecksEnabled[CK_MallocChecker]) + if (!ChecksEnabled[CK_MallocChecker]) { + C.addSink(); return; + } Optional CheckKind = getCheckIfTracked(Family); if (!CheckKind.hasValue()) Index: clang/test/Analysis/Malloc+NewDelete_intersections.cpp =================================================================== --- clang/test/Analysis/Malloc+NewDelete_intersections.cpp +++ /dev/null @@ -1,15 +0,0 @@ -// RUN: %clang_analyze_cc1 -analyzer-checker=core,unix.Malloc,cplusplus.NewDelete -std=c++11 -verify %s -// RUN: %clang_analyze_cc1 -analyzer-checker=core,unix.Malloc,cplusplus.NewDelete,cplusplus.NewDeleteLeaks -std=c++11 -verify %s - -typedef __typeof(sizeof(int)) size_t; -void *malloc(size_t); -void free(void *); - -//------------------------------------------------------------------- -// Check that unix.Malloc + cplusplus.NewDelete does not enable -// warnings produced by unix.MismatchedDeallocator. -//------------------------------------------------------------------- -void testMismatchedDeallocator() { - int *p = (int *)malloc(sizeof(int)); - delete p; -} // expected-warning{{Potential leak of memory pointed to by 'p'}} Index: clang/test/Analysis/NewDelete-checker-test.cpp =================================================================== --- clang/test/Analysis/NewDelete-checker-test.cpp +++ clang/test/Analysis/NewDelete-checker-test.cpp @@ -1,42 +1,31 @@ -// RUN: %clang_analyze_cc1 -std=c++11 -fblocks -verify %s \ +// RUN: %clang_analyze_cc1 -std=c++11 -fblocks %s \ +// RUN: -verify=expected,newdelete \ // RUN: -analyzer-checker=core \ // RUN: -analyzer-checker=cplusplus.NewDelete // -// RUN: %clang_analyze_cc1 -DLEAKS -std=c++11 -fblocks -verify %s \ +// RUN: %clang_analyze_cc1 -DLEAKS -std=c++11 -fblocks %s \ +// RUN: -verify=expected,newdelete,leak \ // RUN: -analyzer-checker=core \ +// RUN: -analyzer-checker=cplusplus.NewDelete \ // RUN: -analyzer-checker=cplusplus.NewDeleteLeaks // -// RUN: %clang_analyze_cc1 -std=c++11 -fblocks -verify %s \ +// RUN: %clang_analyze_cc1 -std=c++11 -fblocks %s \ +// RUN: -verify=expected,newdelete \ // RUN: -analyzer-checker=core \ // RUN: -analyzer-checker=cplusplus.NewDelete \ // RUN: -analyzer-config c++-allocator-inlining=true // -// RUN: %clang_analyze_cc1 -DLEAKS -std=c++11 -fblocks -verify %s \ +// RUN: %clang_analyze_cc1 -std=c++11 -fblocks -verify %s \ +// RUN: -verify=expected,newdelete,leak \ // RUN: -analyzer-checker=core \ +// RUN: -analyzer-checker=cplusplus.NewDelete \ // RUN: -analyzer-checker=cplusplus.NewDeleteLeaks \ // RUN: -analyzer-config c++-allocator-inlining=true // -// RUN: %clang_analyze_cc1 -DTEST_INLINABLE_ALLOCATORS \ -// RUN: -std=c++11 -fblocks -verify %s \ -// RUN: -analyzer-checker=core \ -// RUN: -analyzer-checker=cplusplus.NewDelete -// -// RUN: %clang_analyze_cc1 -DLEAKS -DTEST_INLINABLE_ALLOCATORS \ -// RUN: -std=c++11 -fblocks -verify %s \ +// RUN: %clang_analyze_cc1 -std=c++11 -fblocks -verify %s \ +// RUN: -verify=expected,leak \ // RUN: -analyzer-checker=core \ // RUN: -analyzer-checker=cplusplus.NewDeleteLeaks -// -// RUN: %clang_analyze_cc1 -DTEST_INLINABLE_ALLOCATORS \ -// RUN: -std=c++11 -fblocks -verify %s \ -// RUN: -analyzer-checker=core \ -// RUN: -analyzer-checker=cplusplus.NewDelete \ -// RUN: -analyzer-config c++-allocator-inlining=true -// -// RUN: %clang_analyze_cc1 -DLEAKS -DTEST_INLINABLE_ALLOCATORS \ -// RUN: -std=c++11 -fblocks -verify %s \ -// RUN: -analyzer-checker=core \ -// RUN: -analyzer-checker=cplusplus.NewDeleteLeaks \ -// RUN: -analyzer-config c++-allocator-inlining=true #include "Inputs/system-header-simulator-cxx.h" @@ -52,50 +41,28 @@ //----- Standard non-placement operators void testGlobalOpNew() { void *p = operator new(0); -} -#ifdef LEAKS -// expected-warning@-2{{Potential leak of memory pointed to by 'p'}} -#endif +} // leak-warning{{Potential leak of memory pointed to by 'p'}} void testGlobalOpNewArray() { void *p = operator new[](0); -} -#ifdef LEAKS -// expected-warning@-2{{Potential leak of memory pointed to by 'p'}} -#endif +} // leak-warning{{Potential leak of memory pointed to by 'p'}} void testGlobalNewExpr() { int *p = new int; -} -#ifdef LEAKS -// expected-warning@-2{{Potential leak of memory pointed to by 'p'}} -#endif +} // leak-warning{{Potential leak of memory pointed to by 'p'}} void testGlobalNewExprArray() { int *p = new int[0]; -} -#ifdef LEAKS -// expected-warning@-2{{Potential leak of memory pointed to by 'p'}} -#endif +} // leak-warning{{Potential leak of memory pointed to by 'p'}} //----- Standard nothrow placement operators void testGlobalNoThrowPlacementOpNewBeforeOverload() { void *p = operator new(0, std::nothrow); -} -#ifdef LEAKS -#ifndef TEST_INLINABLE_ALLOCATORS -// expected-warning@-3{{Potential leak of memory pointed to by 'p'}} -#endif -#endif +} // leak-warning{{Potential leak of memory pointed to by 'p'}} void testGlobalNoThrowPlacementExprNewBeforeOverload() { int *p = new(std::nothrow) int; -} -#ifdef LEAKS -#ifndef TEST_INLINABLE_ALLOCATORS -// expected-warning@-3{{Potential leak of memory pointed to by 'p'}} -#endif -#endif +} // leak-warning{{Potential leak of memory pointed to by 'p'}} //----- Standard pointer placement operators void testGlobalPointerPlacementNew() { @@ -135,13 +102,13 @@ void testUseZeroAlloc1() { int *p = (int *)operator new(0); - *p = 1; // expected-warning {{Use of zero-allocated memory}} + *p = 1; // newdelete-warning {{Use of zero-allocated memory}} delete p; } int testUseZeroAlloc2() { int *p = (int *)operator new[](0); - return p[0]; // expected-warning {{Use of zero-allocated memory}} + return p[0]; // newdelete-warning {{Use of zero-allocated memory}} delete[] p; } @@ -149,7 +116,7 @@ void testUseZeroAlloc3() { int *p = new int[0]; - f(*p); // expected-warning {{Use of zero-allocated memory}} + f(*p); // newdelete-warning {{Use of zero-allocated memory}} delete[] p; } @@ -168,70 +135,68 @@ void testUseFirstArgAfterDelete() { int *p = new int; delete p; - f(p); // expected-warning{{Use of memory after it is freed}} + f(p); // newdelete-warning{{Use of memory after it is freed}} } void testUseMiddleArgAfterDelete(int *p) { delete p; - f(0, p); // expected-warning{{Use of memory after it is freed}} + f(0, p); // newdelete-warning{{Use of memory after it is freed}} } void testUseLastArgAfterDelete(int *p) { delete p; - f(0, 0, p); // expected-warning{{Use of memory after it is freed}} + f(0, 0, p); // newdelete-warning{{Use of memory after it is freed}} } void testUseSeveralArgsAfterDelete(int *p) { delete p; - f(p, p, p); // expected-warning{{Use of memory after it is freed}} + f(p, p, p); // newdelete-warning{{Use of memory after it is freed}} } void testUseRefArgAfterDelete(SomeClass &c) { delete &c; - g(c); // expected-warning{{Use of memory after it is freed}} + g(c); // newdelete-warning{{Use of memory after it is freed}} } void testVariadicArgAfterDelete() { SomeClass c; int *p = new int; delete p; - g(c, 0, p); // expected-warning{{Use of memory after it is freed}} + g(c, 0, p); // newdelete-warning{{Use of memory after it is freed}} } void testUseMethodArgAfterDelete(int *p) { SomeClass *c = new SomeClass; delete p; - c->f(p); // expected-warning{{Use of memory after it is freed}} + c->f(p); // newdelete-warning{{Use of memory after it is freed}} } void testUseThisAfterDelete() { SomeClass *c = new SomeClass; delete c; - c->f(0); // expected-warning{{Use of memory after it is freed}} + c->f(0); // newdelete-warning{{Use of memory after it is freed}} } void testDoubleDelete() { int *p = new int; delete p; - delete p; // expected-warning{{Attempt to free released memory}} + delete p; // newdelete-warning{{Attempt to free released memory}} } void testExprDeleteArg() { int i; - delete &i; // expected-warning{{Argument to 'delete' is the address of the local variable 'i', which is not memory allocated by 'new'}} + delete &i; // newdelete-warning{{Argument to 'delete' is the address of the local variable 'i', which is not memory allocated by 'new'}} } void testExprDeleteArrArg() { int i; - delete[] &i; // expected-warning{{Argument to 'delete[]' is the address of the local variable 'i', which is not memory allocated by 'new[]'}} + delete[] & i; // newdelete-warning{{Argument to 'delete[]' is the address of the local variable 'i', which is not memory allocated by 'new[]'}} } void testAllocDeallocNames() { int *p = new(std::nothrow) int[1]; delete[] (++p); -#ifndef TEST_INLINABLE_ALLOCATORS - // expected-warning@-2{{Argument to 'delete[]' is offset by 4 bytes from the start of memory allocated by 'new[]'}} -#endif + // newdelete-warning@-1{{Argument to 'delete[]' is offset by 4 bytes from the start of memory allocated by 'new[]'}} } //-------------------------------- @@ -408,7 +373,7 @@ void testDoubleDeleteClassInstance() { DerefClass *foo = new DerefClass(); delete foo; - delete foo; // expected-warning {{Attempt to delete released memory}} + delete foo; // newdelete-warning {{Attempt to delete released memory}} } class EmptyClass{ @@ -420,7 +385,7 @@ void testDoubleDeleteEmptyClass() { EmptyClass *foo = new EmptyClass(); delete foo; - delete foo; // expected-warning {{Attempt to delete released memory}} + delete foo; // newdelete-warning {{Attempt to delete released memory}} } struct Base { Index: clang/test/Analysis/NewDelete-intersections.mm =================================================================== --- clang/test/Analysis/NewDelete-intersections.mm +++ clang/test/Analysis/NewDelete-intersections.mm @@ -1,7 +1,20 @@ -// RUN: %clang_analyze_cc1 -analyzer-checker=core,cplusplus.NewDelete -std=c++11 -fblocks -verify %s -// RUN: %clang_analyze_cc1 -analyzer-checker=core,cplusplus.NewDelete,cplusplus.NewDeleteLeaks -std=c++11 -DLEAKS -fblocks -verify %s -// RUN: %clang_analyze_cc1 -analyzer-checker=core,cplusplus.NewDelete -std=c++11 -fblocks -DTEST_INLINABLE_ALLOCATORS -verify %s -// RUN: %clang_analyze_cc1 -analyzer-checker=core,cplusplus.NewDelete,cplusplus.NewDeleteLeaks -std=c++11 -DLEAKS -fblocks -DTEST_INLINABLE_ALLOCATORS -verify %s +// RUN: %clang_analyze_cc1 -std=c++11 -fblocks %s \ +// RUN: -verify=newdelete \ +// RUN: -analyzer-checker=core \ +// RUN: -analyzer-checker=cplusplus.NewDelete + +// RUN: %clang_analyze_cc1 -std=c++11 -DLEAKS -fblocks %s \ +// RUN: -verify=leak \ +// RUN: -analyzer-checker=core \ +// RUN: -analyzer-checker=cplusplus.NewDeleteLeaks + +// leak-no-diagnostics + +// RUN: %clang_analyze_cc1 -std=c++11 -DLEAKS -fblocks %s \ +// RUN: -verify=mismatch \ +// RUN: -analyzer-checker=core \ +// RUN: -analyzer-checker=unix.MismatchedDeallocator + #include "Inputs/system-header-simulator-cxx.h" #include "Inputs/system-header-simulator-objc.h" @@ -10,12 +23,6 @@ extern "C" void *alloca(size_t); extern "C" void free(void *); -//---------------------------------------------------------------------------- -// Check for intersections with unix.Malloc and unix.MallocWithAnnotations -// checkers bounded with cplusplus.NewDelete. -//---------------------------------------------------------------------------- - -//----- malloc()/free() are subjects of unix.Malloc and unix.MallocWithAnnotations void testMallocFreeNoWarn() { int i; free(&i); // no warn @@ -39,7 +46,8 @@ void testDeleteMalloced() { int *p1 = (int *)malloc(sizeof(int)); - delete p1; // no warn + delete p1; + // mismatch-warning@-1{{Memory allocated by malloc() should be deallocated by free(), not 'delete'}} int *p2 = (int *)__builtin_alloca(sizeof(int)); delete p2; // no warn @@ -54,35 +62,30 @@ void testFreeOpNew() { void *p = operator new(0); free(p); + // mismatch-warning@-1{{Memory allocated by operator new should be deallocated by 'delete', not free()}} } -#ifdef LEAKS -// expected-warning@-2 {{Potential leak of memory pointed to by 'p'}} -#endif void testFreeNewExpr() { int *p = new int; free(p); + // mismatch-warning@-1{{Memory allocated by 'new' should be deallocated by 'delete', not free()}} + free(p); } -#ifdef LEAKS -// expected-warning@-2 {{Potential leak of memory pointed to by 'p'}} -#endif void testObjcFreeNewed() { int *p = new int; NSData *nsdata = [NSData dataWithBytesNoCopy:p length:sizeof(int) freeWhenDone:1]; -#ifdef LEAKS - // expected-warning@-2 {{Potential leak of memory pointed to by 'p'}} -#endif + // mismatch-warning@-1{{+dataWithBytesNoCopy:length:freeWhenDone: cannot take ownership of memory allocated by 'new'}} } void testFreeAfterDelete() { int *p = new int; delete p; - free(p); // expected-warning{{Use of memory after it is freed}} + free(p); // newdelete-warning{{Use of memory after it is freed}} } void testStandardPlacementNewAfterDelete() { int *p = new int; delete p; - p = new(p) int; // expected-warning{{Use of memory after it is freed}} + p = new (p) int; // newdelete-warning{{Use of memory after it is freed}} } Index: clang/test/Analysis/new.cpp =================================================================== --- clang/test/Analysis/new.cpp +++ clang/test/Analysis/new.cpp @@ -115,11 +115,6 @@ delete c; } -//-------------------------------------------------------------------- -// Check for intersection with other checkers from MallocChecker.cpp -// bounded with unix.Malloc -//-------------------------------------------------------------------- - // new/delete oparators are subjects of cplusplus.NewDelete. void testNewDeleteNoWarn() { int i; @@ -135,11 +130,11 @@ int *p3 = new int; // no-warning } -// unix.Malloc does not know about operators new/delete. void testDeleteMallocked() { int *x = (int *)malloc(sizeof(int)); - delete x; // FIXME: Should detect pointer escape and keep silent after 'delete' is modeled properly. -} // expected-warning{{Potential leak of memory pointed to by 'x'}} + // unix.MismatchedDeallocator would catch this, but we're not testing it here. + delete x; +} void testDeleteOpAfterFree() { int *p = (int *)malloc(sizeof(int));