Index: include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h =================================================================== --- include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h +++ include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h @@ -420,7 +420,9 @@ void VisitCXXDestructor(QualType ObjectType, const MemRegion *Dest, const Stmt *S, bool IsBaseDtor, ExplodedNode *Pred, ExplodedNodeSet &Dst); - + void VisitCXXNewAllocatorCall(const CXXNewExpr *CNE, + ExplodedNode *Pred, + ExplodedNodeSet &Dst); void VisitCXXNewExpr(const CXXNewExpr *CNE, ExplodedNode *Pred, ExplodedNodeSet &Dst); Index: lib/StaticAnalyzer/Core/ExprEngine.cpp =================================================================== --- lib/StaticAnalyzer/Core/ExprEngine.cpp +++ lib/StaticAnalyzer/Core/ExprEngine.cpp @@ -553,12 +553,8 @@ void ExprEngine::ProcessNewAllocator(const CXXNewExpr *NE, ExplodedNode *Pred) { - //TODO: Implement VisitCXXNewAllocatorCall ExplodedNodeSet Dst; - NodeBuilder Bldr(Pred, Dst, *currBldrCtx); - const LocationContext *LCtx = Pred->getLocationContext(); - PostImplicitCall PP(NE->getOperatorNew(), NE->getLocStart(), LCtx); - Bldr.generateNode(PP, Pred->getState(), Pred); + VisitCXXNewAllocatorCall(NE, Pred, Dst); Engine.enqueue(Dst, currBldrCtx->getBlock(), currStmtIdx); } Index: lib/StaticAnalyzer/Core/ExprEngineCXX.cpp =================================================================== --- lib/StaticAnalyzer/Core/ExprEngineCXX.cpp +++ lib/StaticAnalyzer/Core/ExprEngineCXX.cpp @@ -18,6 +18,7 @@ #include "clang/StaticAnalyzer/Core/CheckerManager.h" #include "clang/StaticAnalyzer/Core/PathSensitive/AnalysisManager.h" #include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h" +#include "clang/AST/ParentMap.h" using namespace clang; using namespace ento; @@ -127,7 +128,7 @@ const CFGBlock *B = currBldrCtx->getBlock(); if (currStmtIdx + 1 < B->size()) { CFGElement Next = (*B)[currStmtIdx+1]; - + const Stmt *Parent = LCtx->getParentMap().getParent(CE); // Is this a constructor for a local variable? if (Optional StmtElem = Next.getAs()) { if (const DeclStmt *DS = dyn_cast(StmtElem->getStmt())) { @@ -140,10 +141,41 @@ } } } + else if (isa(Parent)) { + const CXXNewExpr *CNE = dyn_cast(Parent); + unsigned blockCount = currBldrCtx->blockCount(); + DefinedOrUnknownSVal symVal = UnknownVal(); + FunctionDecl *FD = CNE->getOperatorNew(); + bool IsStandardGlobalOpNewFunction = false; + if (FD && !isa(FD) && !FD->isVariadic()) { + if (FD->getNumParams() == 2) { + QualType T = FD->getParamDecl(1)->getType(); + if (const IdentifierInfo *II = T.getBaseTypeIdentifier()) + // NoThrow placement new behaves as a standard new. + IsStandardGlobalOpNewFunction = II->getName().equals("nothrow_t"); + } + else + // Placement forms are considered non-standard. + IsStandardGlobalOpNewFunction = (FD->getNumParams() == 1); + } + + if (IsStandardGlobalOpNewFunction) + symVal = svalBuilder.getConjuredHeapSymbolVal(CNE, LCtx, blockCount); + else + symVal = svalBuilder.conjureSymbolVal(0, CNE, LCtx, CNE->getType(), + blockCount); + SVal DestVal = UnknownVal(); + const MemRegion *Dest = symVal.getAsRegion(); + if (Dest) + DestVal = loc::MemRegionVal(Dest); + QualType Ty = CNE->getType(); + DestVal = makeZeroElementRegion(State, DestVal, Ty); + Target = DestVal.getAsRegion(); + } } // Is this a constructor for a member? - if (Optional InitElem = Next.getAs()) { + else if (Optional InitElem = Next.getAs()) { const CXXCtorInitializer *Init = InitElem->getInitializer(); assert(Init->isAnyMemberInitializer()); @@ -328,15 +360,39 @@ getCheckerManager().runCheckersForPostCall(Dst, DstInvalidated, *Call, *this); } +void ExprEngine::VisitCXXNewAllocatorCall(const CXXNewExpr *CNE, + ExplodedNode *Pred, + ExplodedNodeSet &Dst) { + ProgramStateRef State = Pred->getState(); + const LocationContext *LCtx = Pred->getLocationContext(); + CallEventManager &CEMgr = getStateManager().getCallEventManager(); + CallEventRef Call = + CEMgr.getCXXAllocatorCall(CNE, State, LCtx); + + PrettyStackTraceLoc CrashInfo(getContext().getSourceManager(), + Call->getSourceRange().getBegin(), + "Error evaluating destructor"); + + ExplodedNodeSet DstPreCall; + getCheckerManager().runCheckersForPreCall(DstPreCall, Pred, + *Call, *this); + + ExplodedNodeSet DstInvalidated; + StmtNodeBuilder Bldr(DstPreCall, DstInvalidated, *currBldrCtx); + for (ExplodedNodeSet::iterator I = DstPreCall.begin(), E = DstPreCall.end(); + I != E; ++I) + defaultEvalCall(Bldr, *I, *Call); + + getCheckerManager().runCheckersForPostCall(Dst, DstInvalidated, + *Call, *this); +} + void ExprEngine::VisitCXXNewExpr(const CXXNewExpr *CNE, ExplodedNode *Pred, ExplodedNodeSet &Dst) { - // FIXME: Much of this should eventually migrate to CXXAllocatorCall. - // Also, we need to decide how allocators actually work -- they're not - // really part of the CXXNewExpr because they happen BEFORE the - // CXXConstructExpr subexpression. See PR12014 for some discussion. - + unsigned blockCount = currBldrCtx->blockCount(); + ProgramStateRef State = Pred->getState(); const LocationContext *LCtx = Pred->getLocationContext(); DefinedOrUnknownSVal symVal = UnknownVal(); FunctionDecl *FD = CNE->getOperatorNew(); @@ -363,18 +419,6 @@ symVal = svalBuilder.conjureSymbolVal(0, CNE, LCtx, CNE->getType(), blockCount); - ProgramStateRef State = Pred->getState(); - CallEventManager &CEMgr = getStateManager().getCallEventManager(); - CallEventRef Call = - CEMgr.getCXXAllocatorCall(CNE, State, LCtx); - - // Invalidate placement args. - // FIXME: Once we figure out how we want allocators to work, - // we should be using the usual pre-/(default-)eval-/post-call checks here. - State = Call->invalidateRegions(blockCount); - if (!State) - return; - // If this allocation function is not declared as non-throwing, failures // /must/ be signalled by exceptions, and thus the return value will never be // NULL. -fno-exceptions does not influence this semantics. Index: lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp =================================================================== --- lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp +++ lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp @@ -614,15 +614,9 @@ if (Target && isa(Target)) return CIP_DisallowedOnce; - // FIXME: This is a hack. We don't use the correct region for a new - // expression, so if we inline the constructor its result will just be - // thrown away. This short-term hack is tracked in - // and the longer-term possible fix is discussed in PR12014. const CXXConstructExpr *CtorExpr = Ctor.getOriginExpr(); - if (const Stmt *Parent = CurLC->getParentMap().getParent(CtorExpr)) - if (isa(Parent)) - return CIP_DisallowedOnce; + // Inlining constructors requires including initializers in the CFG. const AnalysisDeclContext *ADC = CallerSFC->getAnalysisDeclContext(); assert(ADC->getCFGBuildOptions().AddInitializers && "No CFG initializers"); @@ -640,7 +634,7 @@ // FIXME: This is a hack. We don't handle temporary destructors // right now, so we shouldn't inline their constructors. if (CtorExpr->getConstructionKind() == CXXConstructExpr::CK_Complete) - if (!Target || !isa(Target)) + if (!Target) return CIP_DisallowedOnce; break; Index: test/Analysis/ctor.mm =================================================================== --- test/Analysis/ctor.mm +++ test/Analysis/ctor.mm @@ -563,19 +563,17 @@ } void testNew() { - // FIXME: Pending proper implementation of constructors for 'new'. raw_pair *pp = new raw_pair(); - clang_analyzer_eval(pp->p1 == 0); // expected-warning{{UNKNOWN}} - clang_analyzer_eval(pp->p2 == 0); // expected-warning{{UNKNOWN}} + clang_analyzer_eval(pp->p1 == 0); // expected-warning{{TRUE}} + clang_analyzer_eval(pp->p2 == 0); // expected-warning{{TRUE}} } void testArrayNew() { - // FIXME: Pending proper implementation of constructors for 'new[]'. raw_pair *p = new raw_pair[2](); - clang_analyzer_eval(p[0].p1 == 0); // expected-warning{{UNKNOWN}} - clang_analyzer_eval(p[0].p2 == 0); // expected-warning{{UNKNOWN}} - clang_analyzer_eval(p[1].p1 == 0); // expected-warning{{UNKNOWN}} - clang_analyzer_eval(p[1].p2 == 0); // expected-warning{{UNKNOWN}} + clang_analyzer_eval(p[0].p1 == 0); // expected-warning{{TRUE}} + clang_analyzer_eval(p[0].p2 == 0); // expected-warning{{TRUE}} + clang_analyzer_eval(p[1].p1 == 0); // expected-warning{{TRUE}} + clang_analyzer_eval(p[1].p2 == 0); // expected-warning{{TRUE}} } struct initializing_pair { @@ -670,7 +668,6 @@ void testDynamic() { List *list = new List{1, 2}; - // FIXME: When we handle constructors with 'new', this will be TRUE. - clang_analyzer_eval(list->usedInitializerList); // expected-warning{{UNKNOWN}} + clang_analyzer_eval(list->usedInitializerList); // expected-warning{{TRUE}} } } Index: test/Analysis/inline.cpp =================================================================== --- test/Analysis/inline.cpp +++ test/Analysis/inline.cpp @@ -307,17 +307,13 @@ int value; IntWrapper(int input) : value(input) { - // We don't want this constructor to be inlined unless we can actually - // use the proper region for operator new. - // See PR12014 and . - clang_analyzer_checkInlined(false); // no-warning + clang_analyzer_checkInlined(false); // expected-warning{{FALSE}} } }; void test() { IntWrapper *obj = new IntWrapper(42); - // should be TRUE - clang_analyzer_eval(obj->value == 42); // expected-warning{{UNKNOWN}} + clang_analyzer_eval(obj->value == 42); // expected-warning{{TRUE}} delete obj; } Index: test/Analysis/new.cpp =================================================================== --- test/Analysis/new.cpp +++ test/Analysis/new.cpp @@ -74,6 +74,18 @@ clang_analyzer_eval(*n == 0); // expected-warning{{TRUE}} } +class InlineConstructor { +public: + int* k; + InlineConstructor() {k=0;} + ~InlineConstructor() {*k=1;} // expected-warning{{Dereference of null pointer (loaded from field 'k')}} +}; + +void test_construct_call() { + InlineConstructor *m1 = new InlineConstructor(); + delete m1; +} + struct PtrWrapper { int *x; Index: test/Analysis/temporaries.cpp =================================================================== --- test/Analysis/temporaries.cpp +++ test/Analysis/temporaries.cpp @@ -105,7 +105,7 @@ // FIXME: should be TRUE, but we don't inline the constructors of // temporaries because we can't model their destructors yet. - clang_analyzer_eval(((HasCtorDtor){1, 42}).y == 42); // expected-warning{{UNKNOWN}} + clang_analyzer_eval(((HasCtorDtor){1, 42}).y == 42); // expected-warning{{TRUE}} #endif } }