Index: include/clang/StaticAnalyzer/Core/Checker.h =================================================================== --- include/clang/StaticAnalyzer/Core/Checker.h +++ include/clang/StaticAnalyzer/Core/Checker.h @@ -283,6 +283,22 @@ } }; +class NewAllocator { + template + static void _checkNewAllocator(void *checker, const CXXNewExpr *NE, + SVal Target, CheckerContext &C) { + ((const CHECKER *)checker)->checkNewAllocator(NE, Target, C); + } + +public: + template + static void _register(CHECKER *checker, CheckerManager &mgr) { + mgr._registerForNewAllocator( + CheckerManager::CheckNewAllocatorFunc(checker, + _checkNewAllocator)); + } +}; + class LiveSymbols { template static void _checkLiveSymbols(void *checker, ProgramStateRef state, Index: include/clang/StaticAnalyzer/Core/CheckerManager.h =================================================================== --- include/clang/StaticAnalyzer/Core/CheckerManager.h +++ include/clang/StaticAnalyzer/Core/CheckerManager.h @@ -303,6 +303,13 @@ ExplodedNodeSet &Dst, ExplodedNode *Pred, ExprEngine &Eng); + /// \brief Run checkers between C++ operator new and constructor calls. + void runCheckersForNewAllocator(const CXXNewExpr *NE, SVal Target, + ExplodedNodeSet &Dst, + ExplodedNode *Pred, + ExprEngine &Eng, + bool wasInlined = false); + /// \brief Run checkers for live symbols. /// /// Allows modifying SymbolReaper object. For example, checkers can explicitly @@ -437,6 +444,9 @@ typedef CheckerFn CheckBranchConditionFunc; + + typedef CheckerFn + CheckNewAllocatorFunc; typedef CheckerFn CheckDeadSymbolsFunc; @@ -494,6 +504,8 @@ void _registerForBranchCondition(CheckBranchConditionFunc checkfn); + void _registerForNewAllocator(CheckNewAllocatorFunc checkfn); + void _registerForLiveSymbols(CheckLiveSymbolsFunc checkfn); void _registerForDeadSymbols(CheckDeadSymbolsFunc checkfn); @@ -603,6 +615,8 @@ std::vector BranchConditionCheckers; + std::vector NewAllocatorCheckers; + std::vector LiveSymbolsCheckers; std::vector DeadSymbolsCheckers; Index: lib/StaticAnalyzer/Checkers/MallocChecker.cpp =================================================================== --- lib/StaticAnalyzer/Checkers/MallocChecker.cpp +++ lib/StaticAnalyzer/Checkers/MallocChecker.cpp @@ -162,6 +162,7 @@ check::PreCall, check::PostStmt, check::PostStmt, + check::NewAllocator, check::PreStmt, check::PostStmt, check::PostObjCMessage, @@ -207,6 +208,8 @@ void checkPreCall(const CallEvent &Call, CheckerContext &C) const; void checkPostStmt(const CallExpr *CE, CheckerContext &C) const; void checkPostStmt(const CXXNewExpr *NE, CheckerContext &C) const; + void checkNewAllocator(const CXXNewExpr *NE, SVal Target, + 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; @@ -281,10 +284,16 @@ bool isStandardNewDelete(const FunctionDecl *FD, ASTContext &C) const; ///@} + /// \brief Process C++ operator new()'s allocation, which is the part of C++ + /// new-expression that goes before the constructor. + void processNewAllocation(const CXXNewExpr *NE, CheckerContext &C, + SVal Target) const; + /// \brief Perform a zero-allocation check. ProgramStateRef ProcessZeroAllocation(CheckerContext &C, const Expr *E, const unsigned AllocationSizeArg, - ProgramStateRef State) const; + ProgramStateRef State, + Optional RetVal = None) const; ProgramStateRef MallocMemReturnsAttr(CheckerContext &C, const CallExpr *CE, @@ -300,7 +309,7 @@ AllocationFamily Family = AF_Malloc); static ProgramStateRef addExtentSize(CheckerContext &C, const CXXNewExpr *NE, - ProgramStateRef State); + ProgramStateRef State, SVal Target); // Check if this malloc() for special flags. At present that means M_ZERO or // __GFP_ZERO (in which case, treat it like calloc). @@ -311,7 +320,8 @@ /// Update the RefState to reflect the new memory allocation. static ProgramStateRef MallocUpdateRefState(CheckerContext &C, const Expr *E, ProgramStateRef State, - AllocationFamily Family = AF_Malloc); + AllocationFamily Family = AF_Malloc, + Optional RetVal = None); ProgramStateRef FreeMemAttr(CheckerContext &C, const CallExpr *CE, const OwnershipAttr* Att, @@ -949,13 +959,15 @@ } // Performs a 0-sized allocations check. -ProgramStateRef MallocChecker::ProcessZeroAllocation(CheckerContext &C, - const Expr *E, - const unsigned AllocationSizeArg, - ProgramStateRef State) const { +ProgramStateRef MallocChecker::ProcessZeroAllocation( + CheckerContext &C, const Expr *E, const unsigned AllocationSizeArg, + ProgramStateRef State, Optional retVal) const { if (!State) return nullptr; + if (!retVal) + retVal = C.getSVal(E); + const Expr *Arg = nullptr; if (const CallExpr *CE = dyn_cast(E)) { @@ -988,8 +1000,7 @@ State->assume(SvalBuilder.evalEQ(State, *DefArgVal, Zero)); if (TrueState && !FalseState) { - SVal retVal = State->getSVal(E, C.getLocationContext()); - SymbolRef Sym = retVal.getAsLocSymbol(); + SymbolRef Sym = retVal->getAsLocSymbol(); if (!Sym) return State; @@ -1050,9 +1061,9 @@ return false; } -void MallocChecker::checkPostStmt(const CXXNewExpr *NE, - CheckerContext &C) const { - +void MallocChecker::processNewAllocation(const CXXNewExpr *NE, + CheckerContext &C, + SVal Target) const { if (NE->getNumPlacementArgs()) for (CXXNewExpr::const_arg_iterator I = NE->placement_arg_begin(), E = NE->placement_arg_end(); I != E; ++I) @@ -1072,37 +1083,47 @@ // MallocUpdateRefState() instead of MallocMemAux() which breakes the // existing binding. State = MallocUpdateRefState(C, NE, State, NE->isArray() ? AF_CXXNewArray - : AF_CXXNew); - State = addExtentSize(C, NE, State); - State = ProcessZeroAllocation(C, NE, 0, State); + : AF_CXXNew, Target); + State = addExtentSize(C, NE, State, Target); + State = ProcessZeroAllocation(C, NE, 0, State, Target); C.addTransition(State); } +void MallocChecker::checkPostStmt(const CXXNewExpr *NE, + CheckerContext &C) const { + if (!C.getAnalysisManager().getAnalyzerOptions().mayInlineCXXAllocator()) + processNewAllocation(NE, C, C.getSVal(NE)); +} + +void MallocChecker::checkNewAllocator(const CXXNewExpr *NE, SVal Target, + CheckerContext &C) const { + processNewAllocation(NE, C, Target); +} + // Sets the extent value of the MemRegion allocated by // new expression NE to its size in Bytes. // ProgramStateRef MallocChecker::addExtentSize(CheckerContext &C, const CXXNewExpr *NE, - ProgramStateRef State) { + ProgramStateRef State, + SVal Target) { if (!State) return nullptr; SValBuilder &svalBuilder = C.getSValBuilder(); SVal ElementCount; - const LocationContext *LCtx = C.getLocationContext(); const SubRegion *Region; if (NE->isArray()) { const Expr *SizeExpr = NE->getArraySize(); ElementCount = State->getSVal(SizeExpr, C.getLocationContext()); // Store the extent size for the (symbolic)region // containing the elements. - Region = (State->getSVal(NE, LCtx)) - .getAsRegion() + Region = Target.getAsRegion() ->getAs() ->getSuperRegion() ->getAs(); } else { ElementCount = svalBuilder.makeIntVal(1, true); - Region = (State->getSVal(NE, LCtx)).getAsRegion()->getAs(); + Region = Target.getAsRegion()->getAs(); } assert(Region); @@ -1263,18 +1284,20 @@ ProgramStateRef MallocChecker::MallocUpdateRefState(CheckerContext &C, const Expr *E, ProgramStateRef State, - AllocationFamily Family) { + AllocationFamily Family, + Optional retVal) { if (!State) return nullptr; // Get the return value. - SVal retVal = State->getSVal(E, C.getLocationContext()); + if (!retVal) + retVal = State->getSVal(E, C.getLocationContext()); // We expect the malloc functions to return a pointer. - if (!retVal.getAs()) + if (!retVal->getAs()) return nullptr; - SymbolRef Sym = retVal.getAsLocSymbol(); + SymbolRef Sym = retVal->getAsLocSymbol(); assert(Sym); // Set the symbol's state to Allocated. Index: lib/StaticAnalyzer/Core/CheckerManager.cpp =================================================================== --- lib/StaticAnalyzer/Core/CheckerManager.cpp +++ lib/StaticAnalyzer/Core/CheckerManager.cpp @@ -469,6 +469,46 @@ expandGraphWithCheckers(C, Dst, Src); } +namespace { + struct CheckNewAllocatorContext { + typedef std::vector CheckersTy; + const CheckersTy &Checkers; + const CXXNewExpr *NE; + SVal Target; + bool WasInlined; + ExprEngine &Eng; + + CheckersTy::const_iterator checkers_begin() { return Checkers.begin(); } + CheckersTy::const_iterator checkers_end() { return Checkers.end(); } + + CheckNewAllocatorContext(const CheckersTy &Checkers, const CXXNewExpr *NE, + SVal Target, bool WasInlined, ExprEngine &Eng) + : Checkers(Checkers), NE(NE), Target(Target), WasInlined(WasInlined), + Eng(Eng) {} + + void runChecker(CheckerManager::CheckNewAllocatorFunc checkFn, + NodeBuilder &Bldr, ExplodedNode *Pred) { + // TODO: Does this deserve a custom program point? For now we're re-using + // PostImplicitCall because we're guaranteed to use the non-implicit + // PostStmt for the PostCall callback, because we have some sort of + // call site (CXXNewExpr) in this scenario. + ProgramPoint L = PostImplicitCall(NE->getOperatorNew(), NE->getLocStart(), + Pred->getLocationContext()); + CheckerContext C(Bldr, Eng, Pred, L); + checkFn(NE, Target, C); + } + }; +} + +void CheckerManager::runCheckersForNewAllocator( + const CXXNewExpr *NE, SVal Target, ExplodedNodeSet &Dst, ExplodedNode *Pred, + ExprEngine &Eng, bool WasInlined) { + ExplodedNodeSet Src; + Src.insert(Pred); + CheckNewAllocatorContext C(NewAllocatorCheckers, NE, Target, WasInlined, Eng); + expandGraphWithCheckers(C, Dst, Src); +} + /// \brief Run checkers for live symbols. void CheckerManager::runCheckersForLiveSymbols(ProgramStateRef state, SymbolReaper &SymReaper) { @@ -711,6 +751,10 @@ BranchConditionCheckers.push_back(checkfn); } +void CheckerManager::_registerForNewAllocator(CheckNewAllocatorFunc checkfn) { + NewAllocatorCheckers.push_back(checkfn); +} + void CheckerManager::_registerForLiveSymbols(CheckLiveSymbolsFunc checkfn) { LiveSymbolsCheckers.push_back(checkfn); } Index: lib/StaticAnalyzer/Core/ExprEngineCXX.cpp =================================================================== --- lib/StaticAnalyzer/Core/ExprEngineCXX.cpp +++ lib/StaticAnalyzer/Core/ExprEngineCXX.cpp @@ -469,8 +469,13 @@ CNE->getType())); } - getCheckerManager().runCheckersForPostCall(Dst, DstPostValue, - *Call, *this); + ExplodedNodeSet DstPostPostCallCallback; + getCheckerManager().runCheckersForPostCall(DstPostPostCallCallback, + DstPostValue, *Call, *this); + for (auto I : DstPostPostCallCallback) { + getCheckerManager().runCheckersForNewAllocator( + CNE, peekCXXNewAllocatorValue(I->getState()), Dst, I, *this); + } } bool ExprEngine::isStandardGlobalOperatorNewFunction(const CXXNewExpr *CNE) { Index: test/Analysis/NewDelete-custom.cpp =================================================================== --- test/Analysis/NewDelete-custom.cpp +++ test/Analysis/NewDelete-custom.cpp @@ -1,8 +1,10 @@ // RUN: %clang_analyze_cc1 -analyzer-checker=core,cplusplus.NewDelete,unix.Malloc -std=c++11 -fblocks -verify %s -// RUN: %clang_analyze_cc1 -analyzer-checker=core,cplusplus.NewDelete,cplusplus.NewDeleteLeaks,unix.Malloc -std=c++11 -DLEAKS -fblocks -verify %s +// RUN: %clang_analyze_cc1 -analyzer-checker=core,cplusplus.NewDelete,cplusplus.NewDeleteLeaks,unix.Malloc -std=c++11 -DLEAKS=1 -fblocks -verify %s +// RUN: %clang_analyze_cc1 -analyzer-checker=core,cplusplus.NewDelete,unix.Malloc -std=c++11 -analyzer-config c++-allocator-inlining=true -DALLOCATOR_INLINING=1 -fblocks -verify %s +// RUN: %clang_analyze_cc1 -analyzer-checker=core,cplusplus.NewDelete,cplusplus.NewDeleteLeaks,unix.Malloc -std=c++11 -analyzer-config c++-allocator-inlining=true -DLEAKS=1 -DALLOCATOR_INLINING=1 -fblocks -verify %s #include "Inputs/system-header-simulator-cxx.h" -#ifndef LEAKS +#if !LEAKS // expected-no-diagnostics #endif @@ -26,7 +28,7 @@ C *c3 = ::new C; } -#ifdef LEAKS +#if LEAKS && !ALLOCATOR_INLINING // expected-warning@-2{{Potential leak of memory pointed to by 'c3'}} #endif @@ -37,7 +39,7 @@ void testNewExprArray() { int *p = new int[0]; } -#ifdef LEAKS +#if LEAKS && !ALLOCATOR_INLINING // expected-warning@-2{{Potential leak of memory pointed to by 'p'}} #endif @@ -50,7 +52,7 @@ void testNewExpr() { int *p = new int; } -#ifdef LEAKS +#if LEAKS && !ALLOCATOR_INLINING // expected-warning@-2{{Potential leak of memory pointed to by 'p'}} #endif @@ -59,21 +61,21 @@ void testOpNewNoThrow() { void *p = operator new(0, std::nothrow); } -#ifdef LEAKS +#if LEAKS // expected-warning@-2{{Potential leak of memory pointed to by 'p'}} #endif void testNewExprNoThrow() { int *p = new(std::nothrow) int; } -#ifdef LEAKS +#if LEAKS // expected-warning@-2{{Potential leak of memory pointed to by 'p'}} #endif //----- Custom placement operators void testOpNewPlacement() { void *p = operator new(0, 0.1); // no warn -} +} void testNewExprPlacement() { int *p = new(0.1) int; // no warn Index: test/Analysis/new-ctor-malloc.cpp =================================================================== --- /dev/null +++ test/Analysis/new-ctor-malloc.cpp @@ -0,0 +1,18 @@ +// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection,unix.Malloc -analyzer-config c++-allocator-inlining=true -std=c++11 -verify %s + +void clang_analyzer_eval(bool); + +typedef __typeof__(sizeof(int)) size_t; + +void *malloc(size_t size); + +void *operator new(size_t size) throw() { + void *x = malloc(size); + if (!x) + return nullptr; + return x; +} + +void checkNewAndConstructorInlining() { + int *s = new int; +} // expected-warning {{Potential leak of memory pointed to by 's'}}