Index: cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp =================================================================== --- cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp +++ cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp @@ -113,7 +113,9 @@ std::pair ExprEngine::prepareForObjectConstruction( const Expr *E, ProgramStateRef State, const LocationContext *LCtx, const ConstructionContext *CC, EvalCallOptions &CallOpts) { - MemRegionManager &MRMgr = getSValBuilder().getRegionManager(); + SValBuilder &SVB = getSValBuilder(); + MemRegionManager &MRMgr = SVB.getRegionManager(); + ASTContext &ACtx = SVB.getContext(); // See if we're constructing an existing region by looking at the // current construction context. @@ -139,7 +141,7 @@ assert(Init->isAnyMemberInitializer()); const CXXMethodDecl *CurCtor = cast(LCtx->getDecl()); Loc ThisPtr = - getSValBuilder().getCXXThis(CurCtor, LCtx->getStackFrame()); + SVB.getCXXThis(CurCtor, LCtx->getStackFrame()); SVal ThisVal = State->getSVal(ThisPtr); const ValueDecl *Field; @@ -199,12 +201,25 @@ cast(SFC->getCallSite()), State, CallerLCtx, RTC->getConstructionContext(), CallOpts); } else { - // We are on the top frame of the analysis. - // TODO: What exactly happens when we are? Does the temporary object - // live long enough in the region store in this case? Would checkers - // think that this object immediately goes out of scope? - CallOpts.IsTemporaryCtorOrDtor = true; - SVal V = loc::MemRegionVal(MRMgr.getCXXTempObjectRegion(E, LCtx)); + // We are on the top frame of the analysis. We do not know where is the + // object returned to. Conjure a symbolic region for the return value. + // TODO: We probably need a new MemRegion kind to represent the storage + // of that SymbolicRegion, so that we cound produce a fancy symbol + // instead of an anonymous conjured symbol. + // TODO: Do we need to track the region to avoid having it dead + // too early? It does die too early, at least in C++17, but because + // putting anything into a SymbolicRegion causes an immediate escape, + // it doesn't cause any leak false positives. + const auto *RCC = cast(CC); + // Make sure that this doesn't coincide with any other symbol + // conjured for the returned expression. + static const int TopLevelSymRegionTag = 0; + const Expr *RetE = RCC->getReturnStmt()->getRetValue(); + assert(RetE && "Void returns should not have a construction context"); + QualType ReturnTy = RetE->getType(); + QualType RegionTy = ACtx.getPointerType(ReturnTy); + SVal V = SVB.conjureSymbolVal(&TopLevelSymRegionTag, RetE, SFC, + RegionTy, currBldrCtx->blockCount()); return std::make_pair(State, V); } llvm_unreachable("Unhandled return value construction context!"); Index: cfe/trunk/test/Analysis/temporaries.cpp =================================================================== --- cfe/trunk/test/Analysis/temporaries.cpp +++ cfe/trunk/test/Analysis/temporaries.cpp @@ -1,9 +1,25 @@ -// RUN: %clang_analyze_cc1 -Wno-non-pod-varargs -analyzer-checker=core,cplusplus,debug.ExprInspection -analyzer-config cfg-temporary-dtors=false -verify -w -std=c++03 -analyzer-config eagerly-assume=false %s -// RUN: %clang_analyze_cc1 -Wno-non-pod-varargs -analyzer-checker=core,cplusplus,debug.ExprInspection -analyzer-config cfg-temporary-dtors=false -verify -w -std=c++11 -analyzer-config eagerly-assume=false %s -// RUN: %clang_analyze_cc1 -Wno-non-pod-varargs -analyzer-checker=core,cplusplus,debug.ExprInspection -DTEMPORARY_DTORS -verify -w -analyzer-config cfg-temporary-dtors=true,c++-temp-dtor-inlining=true -analyzer-config eagerly-assume=false %s -std=c++11 -// RUN: %clang_analyze_cc1 -Wno-non-pod-varargs -analyzer-checker=core,cplusplus,debug.ExprInspection -DTEMPORARY_DTORS -w -analyzer-config cfg-temporary-dtors=true,c++-temp-dtor-inlining=true -analyzer-config eagerly-assume=false %s -std=c++17 +// RUN: %clang_analyze_cc1 -w -analyzer-checker=core,cplusplus\ +// RUN: -analyzer-checker debug.ExprInspection -Wno-non-pod-varargs\ +// RUN: -analyzer-config eagerly-assume=false -verify %s\ +// RUN: -std=c++03 -analyzer-config cfg-temporary-dtors=false + +// RUN: %clang_analyze_cc1 -w -analyzer-checker=core,cplusplus\ +// RUN: -analyzer-checker debug.ExprInspection -Wno-non-pod-varargs\ +// RUN: -analyzer-config eagerly-assume=false -verify %s\ +// RUN: -std=c++11 -analyzer-config cfg-temporary-dtors=false + +// RUN: %clang_analyze_cc1 -w -analyzer-checker=core,cplusplus\ +// RUN: -analyzer-checker debug.ExprInspection -Wno-non-pod-varargs\ +// RUN: -analyzer-config eagerly-assume=false -verify %s\ +// RUN: -std=c++11 -analyzer-config cfg-temporary-dtors=true\ +// RUN: -DTEMPORARY_DTORS + +// RUN: %clang_analyze_cc1 -w -analyzer-checker=core,cplusplus\ +// RUN: -analyzer-checker debug.ExprInspection -Wno-non-pod-varargs\ +// RUN: -analyzer-config eagerly-assume=false -verify %s\ +// RUN: -std=c++17 -analyzer-config cfg-temporary-dtors=true\ +// RUN: -DTEMPORARY_DTORS -// Note: The C++17 run-line doesn't -verify yet - it is a no-crash test. extern bool clang_analyzer_eval(bool); extern bool clang_analyzer_warnIfReached(); @@ -450,7 +466,16 @@ } #if __cplusplus >= 201103L - CtorWithNoReturnDtor returnNoReturnDtor() { + struct CtorWithNoReturnDtor2 { + CtorWithNoReturnDtor2() = default; + + CtorWithNoReturnDtor2(int x) { + clang_analyzer_checkInlined(true); // expected-warning{{TRUE}} + } + + ~CtorWithNoReturnDtor2() __attribute__((noreturn)); + }; + CtorWithNoReturnDtor2 returnNoReturnDtor() { return {1}; // no-crash } #endif @@ -805,7 +830,12 @@ // On each branch the variable is constructed directly. if (coin) { clang_analyzer_eval(x == 1); // expected-warning{{TRUE}} +#if __cplusplus < 201703L clang_analyzer_eval(y == 1); // expected-warning{{TRUE}} +#else + // FIXME: Destructor called twice in C++17? + clang_analyzer_eval(y == 2); // expected-warning{{TRUE}} +#endif clang_analyzer_eval(z == 0); // expected-warning{{TRUE}} clang_analyzer_eval(w == 0); // expected-warning{{TRUE}} @@ -813,7 +843,12 @@ clang_analyzer_eval(x == 0); // expected-warning{{TRUE}} clang_analyzer_eval(y == 0); // expected-warning{{TRUE}} clang_analyzer_eval(z == 1); // expected-warning{{TRUE}} +#if __cplusplus < 201703L clang_analyzer_eval(w == 1); // expected-warning{{TRUE}} +#else + // FIXME: Destructor called twice in C++17? + clang_analyzer_eval(w == 2); // expected-warning{{TRUE}} +#endif } } } // namespace test_match_constructors_and_destructors @@ -865,9 +900,12 @@ public: ~C() { glob = 1; + // FIXME: Why is destructor not inlined in C++17 clang_analyzer_checkInlined(true); #ifdef TEMPORARY_DTORS - // expected-warning@-2{{TRUE}} +#if __cplusplus < 201703L + // expected-warning@-3{{TRUE}} +#endif #endif } }; @@ -886,11 +924,16 @@ // temporaries returned from functions, so we took the wrong branch. coin && is(get()); // no-crash if (coin) { + // FIXME: Why is destructor not inlined in C++17 clang_analyzer_eval(glob); #ifdef TEMPORARY_DTORS - // expected-warning@-2{{TRUE}} +#if __cplusplus < 201703L + // expected-warning@-3{{TRUE}} #else - // expected-warning@-4{{UNKNOWN}} + // expected-warning@-5{{UNKNOWN}} +#endif +#else + // expected-warning@-8{{UNKNOWN}} #endif } else { // The destructor is not called on this branch. @@ -1012,11 +1055,16 @@ #endif bar2(S(2)); + // FIXME: Why are we losing information in C++17? clang_analyzer_eval(glob == 2); #ifdef TEMPORARY_DTORS - // expected-warning@-2{{TRUE}} +#if __cplusplus < 201703L + // expected-warning@-3{{TRUE}} #else - // expected-warning@-4{{UNKNOWN}} + // expected-warning@-5{{UNKNOWN}} +#endif +#else + // expected-warning@-8{{UNKNOWN}} #endif C *c = new D(); @@ -1172,3 +1220,29 @@ c.foo(); } } // namespace union_indirect_field_crash + +namespace return_from_top_frame { +struct S { + int *p; + S() { p = new int; } + S(S &&s) : p(s.p) { s.p = 0; } + ~S(); // Presumably releases 'p'. +}; + +S foo() { + S s; + return s; +} + +S bar1() { + return foo(); // no-warning +} + +S bar2() { + return S(); +} + +S bar3(int coin) { + return coin ? S() : foo(); // no-warning +} +} // namespace return_from_top_frame