Index: include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h =================================================================== --- include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h +++ include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h @@ -642,8 +642,8 @@ const CXXConstructExpr *findDirectConstructorForCurrentCFGElement(); /// For a CXXConstructExpr, walk forward in the current CFG block to find the - /// CFGElement for the DeclStmt or CXXInitCtorInitializer for which is - /// directly constructed by this constructor. Returns None if the current + /// CFGElement for the DeclStmt or CXXInitCtorInitializer or CXXNewExpr which + /// is directly constructed by this constructor. Returns None if the current /// constructor expression did not directly construct into an existing /// region. Optional findElementDirectlyInitializedByCurrentConstructor(); @@ -655,6 +655,19 @@ /// if not. const MemRegion *getRegionForConstructedObject(const CXXConstructExpr *CE, ExplodedNode *Pred); + + /// Store the region returned by operator new() so that the constructor + /// that follows it knew what location to initialize. The value should be + /// cleared once the respective CXXNewExpr CFGStmt element is processed. + ProgramStateRef pushCXXNewAllocatorValue(ProgramStateRef State, + SVal V); + + /// Retrieve the location returned by the current operator new(). + SVal peekCXXNewAllocatorValue(ProgramStateRef State); + + /// Clear the location returned by the respective operator new(). This needs + /// to be done as soon as CXXNewExpr CFG block is evaluated. + ProgramStateRef popCXXNewAllocatorValue(ProgramStateRef State); }; /// Traits for storing the call processing policy inside GDM. Index: lib/StaticAnalyzer/Core/ExprEngine.cpp =================================================================== --- lib/StaticAnalyzer/Core/ExprEngine.cpp +++ lib/StaticAnalyzer/Core/ExprEngine.cpp @@ -62,6 +62,11 @@ // functions do not interfere. REGISTER_TRAIT_WITH_PROGRAMSTATE(InitializedTemporariesSet, llvm::ImmutableSet) +// Keeps track of return values of various operator new() calls between +// evaluation of the inlined operator new(), through the constructor call, +// to the actual evaluation of the CXXNewExpr. +REGISTER_TRAIT_WITH_PROGRAMSTATE(CXXNewAllocatorValueStack, + llvm::ImmutableList); //===----------------------------------------------------------------------===// // Engine construction and deletion. @@ -308,6 +313,20 @@ return State; } +ProgramStateRef ExprEngine::pushCXXNewAllocatorValue(ProgramStateRef State, + SVal V) { + return State->add(V); +} + +SVal ExprEngine::peekCXXNewAllocatorValue(ProgramStateRef State) { + return State->get().getHead(); +} + +ProgramStateRef ExprEngine::popCXXNewAllocatorValue(ProgramStateRef State) { + return State->set( + State->get().getTail()); +} + //===----------------------------------------------------------------------===// // Top-level transfer function logic (Dispatcher). //===----------------------------------------------------------------------===// @@ -429,6 +448,13 @@ const StackFrameContext *SFC = LC ? LC->getCurrentStackFrame() : nullptr; SymbolReaper SymReaper(SFC, ReferenceStmt, SymMgr, getStoreManager()); + for (auto I: CleanedState->get()) { + if (SymbolRef Sym = I.getAsSymbol()) + SymReaper.markLive(Sym); + if (const MemRegion *MR = I.getAsRegion()) + SymReaper.markElementIndicesLive(MR); + } + getCheckerManager().runCheckersForLiveSymbols(CleanedState, SymReaper); // Create a state in which dead bindings are removed from the environment Index: lib/StaticAnalyzer/Core/ExprEngineCXX.cpp =================================================================== --- lib/StaticAnalyzer/Core/ExprEngineCXX.cpp +++ lib/StaticAnalyzer/Core/ExprEngineCXX.cpp @@ -107,6 +107,12 @@ const MemRegion * ExprEngine::getRegionForConstructedObject(const CXXConstructExpr *CE, ExplodedNode *Pred) { + // TODO: We can only construct a CXXConstructorCall with a region. + // It means that we cannot handle construction into null or garbage pointers. + // Such cosntructors need to be handled by checkers to ensure that a warning + // is displayed to the user and that analysis doesn't explore such paths + // any further. However, in order to act on such events, checker still need + // to receive them, so we need to be able to return non-regions from here. const LocationContext *LCtx = Pred->getLocationContext(); ProgramStateRef State = Pred->getState(); @@ -115,14 +121,22 @@ if (auto Elem = findElementDirectlyInitializedByCurrentConstructor()) { if (Optional StmtElem = Elem->getAs()) { - auto *DS = cast(StmtElem->getStmt()); - if (const auto *Var = dyn_cast(DS->getSingleDecl())) { - if (Var->getInit() && Var->getInit()->IgnoreImplicit() == CE) { - SVal LValue = State->getLValue(Var, LCtx); - QualType Ty = Var->getType(); - LValue = makeZeroElementRegion(State, LValue, Ty); - return LValue.getAsRegion(); + if (isa(StmtElem->getStmt())) { + if (AMgr.getAnalyzerOptions().mayInlineCXXAllocator()) + if (const MemRegion *MR = + peekCXXNewAllocatorValue(State).getAsRegion()) + return MR; + } else if (auto *DS = dyn_cast(StmtElem->getStmt())) { + if (const auto *Var = dyn_cast(DS->getSingleDecl())) { + if (Var->getInit() && Var->getInit()->IgnoreImplicit() == CE) { + SVal LValue = State->getLValue(Var, LCtx); + QualType Ty = Var->getType(); + LValue = makeZeroElementRegion(State, LValue, Ty); + return LValue.getAsRegion(); + } } + } else { + llvm_unreachable("Unexpected directly initialized element!"); } } else if (Optional InitElem = Elem->getAs()) { const CXXCtorInitializer *Init = InitElem->getInitializer(); @@ -166,6 +180,9 @@ if (isa(StmtElem->getStmt())) { return true; } + if (isa(StmtElem->getStmt())) { + return true; + } } if (Elem.getKind() == CFGElement::Initializer) { @@ -452,12 +469,22 @@ 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, + ExplodedNodeSet DstPostCall; + StmtNodeBuilder CallBldr(DstPreCall, DstPostCall, *currBldrCtx); + for (auto I: DstPreCall) + defaultEvalCall(CallBldr, I, *Call); + + // Store return value of operator new() for future use, until the actual + // CXXNewExpr gets processed. + ExplodedNodeSet DstPostValue; + StmtNodeBuilder ValueBldr(DstPostCall, DstPostValue, *currBldrCtx); + for (auto I: DstPostCall) { + ProgramStateRef State = I->getState(); + ValueBldr.generateNode( + CNE, I, pushCXXNewAllocatorValue(State, State->getSVal(CNE, LCtx))); + } + + getCheckerManager().runCheckersForPostCall(Dst, DstPostValue, *Call, *this); } @@ -471,7 +498,7 @@ unsigned blockCount = currBldrCtx->blockCount(); const LocationContext *LCtx = Pred->getLocationContext(); - DefinedOrUnknownSVal symVal = UnknownVal(); + SVal symVal = UnknownVal(); FunctionDecl *FD = CNE->getOperatorNew(); bool IsStandardGlobalOpNewFunction = false; @@ -487,16 +514,24 @@ IsStandardGlobalOpNewFunction = (FD->getNumParams() == 1); } + ProgramStateRef State = Pred->getState(); + + // Retrieve the stored operator new() return value. + if (AMgr.getAnalyzerOptions().mayInlineCXXAllocator()) + symVal = peekCXXNewAllocatorValue(State); + State = popCXXNewAllocatorValue(State); + // We assume all standard global 'operator new' functions allocate memory in // heap. We realize this is an approximation that might not correctly model // a custom global allocator. - if (IsStandardGlobalOpNewFunction) - symVal = svalBuilder.getConjuredHeapSymbolVal(CNE, LCtx, blockCount); - else - symVal = svalBuilder.conjureSymbolVal(nullptr, CNE, LCtx, CNE->getType(), - blockCount); + if (symVal.isUnknown()) { + if (IsStandardGlobalOpNewFunction) + symVal = svalBuilder.getConjuredHeapSymbolVal(CNE, LCtx, blockCount); + else + symVal = svalBuilder.conjureSymbolVal(nullptr, CNE, LCtx, CNE->getType(), + blockCount); + } - ProgramStateRef State = Pred->getState(); CallEventManager &CEMgr = getStateManager().getCallEventManager(); CallEventRef Call = CEMgr.getCXXAllocatorCall(CNE, State, LCtx); @@ -519,7 +554,8 @@ QualType Ty = FD->getType(); if (const FunctionProtoType *ProtoType = Ty->getAs()) if (!ProtoType->isNothrow(getContext())) - State = State->assume(symVal, true); + if (auto dSymVal = symVal.getAs()) + State = State->assume(*dSymVal, true); } StmtNodeBuilder Bldr(Pred, Dst, *currBldrCtx); Index: lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp =================================================================== --- lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp +++ lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp @@ -276,6 +276,13 @@ state = state->BindExpr(CCE, callerCtx, ThisV); } + + if (isa(CE)) { + // We are currently evaluating a CXXNewAllocator CFGElement. It takes a + // while to reach the actual CXXNewExpr element from here, so keep the + // region for later use. + state = pushCXXNewAllocatorValue(state, state->getSVal(CE, callerCtx)); + } } // Step 3: BindedRetNode -> CleanedNodes @@ -596,21 +603,30 @@ const CXXConstructorCall &Ctor = cast(Call); + const CXXConstructExpr *CtorExpr = Ctor.getOriginExpr(); + const Stmt *ParentExpr = CurLC->getParentMap().getParent(CtorExpr); + + if (ParentExpr && isa(ParentExpr) && + !Opts.mayInlineCXXAllocator()) + return CIP_DisallowedOnce; + // FIXME: We don't handle constructors or destructors for arrays properly. // Even once we do, we still need to be careful about implicitly-generated // initializers for array fields in default move/copy constructors. + // We still allow construction into ElementRegion targets when they don't + // represent array elements. const MemRegion *Target = Ctor.getCXXThisVal().getAsRegion(); - 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; + if (Target && isa(Target)) { + if (ParentExpr) + if (const CXXNewExpr *NewExpr = dyn_cast(ParentExpr)) + if (NewExpr->isArray()) + return CIP_DisallowedOnce; + + if (const TypedValueRegion *TR = dyn_cast( + cast(Target)->getSuperRegion())) + if (TR->getValueType()->isArrayType()) + return CIP_DisallowedOnce; + } // Inlining constructors requires including initializers in the CFG. const AnalysisDeclContext *ADC = CallerSFC->getAnalysisDeclContext(); @@ -629,7 +645,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 || isa(Target)) return CIP_DisallowedOnce; break; Index: test/Analysis/inline.cpp =================================================================== --- test/Analysis/inline.cpp +++ test/Analysis/inline.cpp @@ -315,17 +315,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(true); // expected-warning{{TRUE}} } }; 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-ctor-conservative.cpp =================================================================== --- /dev/null +++ test/Analysis/new-ctor-conservative.cpp @@ -0,0 +1,14 @@ +// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection -analyzer-config c++-allocator-inlining=true -std=c++11 -verify %s + +void clang_analyzer_eval(bool); + +struct S { + int x; + S() : x(1) {} + ~S() {} +}; + +void foo() { + S *s = new S; + clang_analyzer_eval(s->x == 1); // expected-warning{{TRUE}} +} Index: test/Analysis/new-ctor-inlined.cpp =================================================================== --- /dev/null +++ test/Analysis/new-ctor-inlined.cpp @@ -0,0 +1,27 @@ +// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection -analyzer-config c++-allocator-inlining=true -std=c++11 -verify %s + +void clang_analyzer_eval(bool); + +typedef __typeof__(sizeof(int)) size_t; + +void *conjure(); +void exit(int); + +void *operator new(size_t size) throw() { + void *x = conjure(); + if (x == 0) + exit(1); + return x; +} + +struct S { + int x; + S() : x(1) {} + ~S() {} +}; + +void foo() { + S *s = new S; + clang_analyzer_eval(s != 0); // expected-warning{{TRUE}} + clang_analyzer_eval(s->x == 1); // expected-warning{{TRUE}} +}