We've started seeing assertion failures after this commit. assert.h assertion failed at llvm/tools/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp:485 in static clang::ento::ProgramStateRef clang::ento::ExprEngine::elideDestructor(clang::ento::ProgramStateRef, const clang::CXXBindTemporaryExpr *, const clang::LocationContext *): !State->contains<ElidedDestructors>(I)
The stack trace is clang::ento::ExprEngine::elideDestructor clang::ento::ExprEngine::prepareForObjectConstruction clang::ento::ExprEngine::prepareForObjectConstruction clang::ento::ExprEngine::VisitCXXConstructExpr clang::ento::ExprEngine::Visit clang::ento::ExprEngine::ProcessStmt clang::ento::ExprEngine::processCFGElement clang::ento::CoreEngine::HandlePostStmt clang::ento::CoreEngine::dispatchWorkItem clang::ento::CoreEngine::ExecuteWorkList clang::ento::ExprEngine::ExecuteWorkList ::AnalysisConsumer::ActionExprEngine ::AnalysisConsumer::HandleCode ::AnalysisConsumer::HandleDeclsCallGraph ::AnalysisConsumer::runAnalysisOnTranslationUnit ::AnalysisConsumer::HandleTranslationUnit I haven't come up with a test case yet. On Thu, Jun 28, 2018 at 2:34 AM Artem Dergachev via cfe-commits < cfe-commits@lists.llvm.org> wrote: > Author: dergachev > Date: Wed Jun 27 17:30:18 2018 > New Revision: 335800 > > URL: http://llvm.org/viewvc/llvm-project?rev=335800&view=rev > Log: > [analyzer] Add support for pre-C++17 copy elision. > > r335795 adds copy elision information to CFG. This commit allows static > analyzer > to elide elidable copy constructors by constructing the objects that were > previously subject to elidable copy directly in the target region of the > copy. > > The chain of elided constructors may potentially be indefinitely long. This > only happens when the object is being returned from a function which in > turn is > returned from another function, etc. > > NRVO is not supported yet. > > Differential Revision: https://reviews.llvm.org/D47671 > > Modified: > cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h > cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp > cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp > cfe/trunk/test/Analysis/cxx17-mandatory-elision.cpp > cfe/trunk/test/Analysis/gtest.cpp > cfe/trunk/test/Analysis/inlining/temp-dtors-path-notes.cpp > cfe/trunk/test/Analysis/lifetime-extension.cpp > cfe/trunk/test/Analysis/temporaries.cpp > > Modified: > cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h?rev=335800&r1=335799&r2=335800&view=diff > > ============================================================================== > --- cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h > (original) > +++ cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h > Wed Jun 27 17:30:18 2018 > @@ -780,9 +780,30 @@ private: > llvm::PointerUnion<const Stmt *, const CXXCtorInitializer *> P, > const LocationContext *LC); > > + /// If the given expression corresponds to a temporary that was used for > + /// passing into an elidable copy/move constructor and that constructor > + /// was actually elided, track that we also need to elide the > destructor. > + static ProgramStateRef elideDestructor(ProgramStateRef State, > + const CXXBindTemporaryExpr *BTE, > + const LocationContext *LC); > + > + /// Stop tracking the destructor that corresponds to an elided > constructor. > + static ProgramStateRef > + cleanupElidedDestructor(ProgramStateRef State, > + const CXXBindTemporaryExpr *BTE, > + const LocationContext *LC); > + > + /// Returns true if the given expression corresponds to a temporary that > + /// was constructed for passing into an elidable copy/move constructor > + /// and that constructor was actually elided. > + static bool isDestructorElided(ProgramStateRef State, > + const CXXBindTemporaryExpr *BTE, > + const LocationContext *LC); > + > /// Check if all objects under construction have been fully constructed > /// for the given context range (including FromLC, not including ToLC). > - /// This is useful for assertions. > + /// This is useful for assertions. Also checks if elided destructors > + /// were cleaned up. > static bool areAllObjectsFullyConstructed(ProgramStateRef State, > const LocationContext *FromLC, > const LocationContext *ToLC); > > Modified: cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp?rev=335800&r1=335799&r2=335800&view=diff > > ============================================================================== > --- cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp (original) > +++ cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp Wed Jun 27 17:30:18 > 2018 > @@ -139,7 +139,8 @@ public: > // encountered. This list may be expanded when new actions are > implemented. > assert(getCXXCtorInitializer() || isa<DeclStmt>(getStmt()) || > isa<CXXNewExpr>(getStmt()) || > isa<CXXBindTemporaryExpr>(getStmt()) || > - isa<MaterializeTemporaryExpr>(getStmt())); > + isa<MaterializeTemporaryExpr>(getStmt()) || > + isa<CXXConstructExpr>(getStmt())); > } > > const Stmt *getStmt() const { > @@ -183,6 +184,14 @@ typedef llvm::ImmutableMap<ConstructedOb > REGISTER_TRAIT_WITH_PROGRAMSTATE(ObjectsUnderConstruction, > ObjectsUnderConstructionMap) > > +// Additionally, track a set of destructors that correspond to elided > +// constructors when copy elision occurs. > +typedef std::pair<const CXXBindTemporaryExpr *, const LocationContext *> > + ElidedDestructorItem; > +typedef llvm::ImmutableSet<ElidedDestructorItem> > + ElidedDestructorSet; > +REGISTER_TRAIT_WITH_PROGRAMSTATE(ElidedDestructors, > + ElidedDestructorSet); > > > > //===----------------------------------------------------------------------===// > // Engine construction and deletion. > @@ -358,14 +367,12 @@ ExprEngine::createTemporaryRegionIfNeede > // a new temporary region out of thin air and copy the contents of the > object > // (which are currently present in the Environment, because Init is an > rvalue) > // into that region. This is not correct, but it is better than nothing. > - bool FoundOriginalMaterializationRegion = false; > const TypedValueRegion *TR = nullptr; > if (const auto *MT = dyn_cast<MaterializeTemporaryExpr>(Result)) { > if (Optional<SVal> V = getObjectUnderConstruction(State, MT, LC)) { > - FoundOriginalMaterializationRegion = true; > - TR = cast<CXXTempObjectRegion>(V->getAsRegion()); > - assert(TR); > State = finishObjectConstruction(State, MT, LC); > + State = State->BindExpr(Result, LC, *V); > + return State; > } else { > StorageDuration SD = MT->getStorageDuration(); > // If this object is bound to a reference with static storage > duration, we > @@ -402,35 +409,33 @@ ExprEngine::createTemporaryRegionIfNeede > } > } > > - if (!FoundOriginalMaterializationRegion) { > - // What remains is to copy the value of the object to the new region. > - // FIXME: In other words, what we should always do is copy value of > the > - // Init expression (which corresponds to the bigger object) to the > whole > - // temporary region TR. However, this value is often no longer present > - // in the Environment. If it has disappeared, we instead invalidate > TR. > - // Still, what we can do is assign the value of expression Ex (which > - // corresponds to the sub-object) to the TR's sub-region Reg. At > least, > - // values inside Reg would be correct. > - SVal InitVal = State->getSVal(Init, LC); > - if (InitVal.isUnknown()) { > - InitVal = getSValBuilder().conjureSymbolVal(Result, LC, > Init->getType(), > - > currBldrCtx->blockCount()); > - State = State->bindLoc(BaseReg.castAs<Loc>(), InitVal, LC, false); > - > - // Then we'd need to take the value that certainly exists and bind > it > - // over. > - if (InitValWithAdjustments.isUnknown()) { > - // Try to recover some path sensitivity in case we couldn't > - // compute the value. > - InitValWithAdjustments = getSValBuilder().conjureSymbolVal( > - Result, LC, InitWithAdjustments->getType(), > - currBldrCtx->blockCount()); > - } > - State = > - State->bindLoc(Reg.castAs<Loc>(), InitValWithAdjustments, LC, > false); > - } else { > - State = State->bindLoc(BaseReg.castAs<Loc>(), InitVal, LC, false); > + // What remains is to copy the value of the object to the new region. > + // FIXME: In other words, what we should always do is copy value of the > + // Init expression (which corresponds to the bigger object) to the whole > + // temporary region TR. However, this value is often no longer present > + // in the Environment. If it has disappeared, we instead invalidate TR. > + // Still, what we can do is assign the value of expression Ex (which > + // corresponds to the sub-object) to the TR's sub-region Reg. At least, > + // values inside Reg would be correct. > + SVal InitVal = State->getSVal(Init, LC); > + if (InitVal.isUnknown()) { > + InitVal = getSValBuilder().conjureSymbolVal(Result, LC, > Init->getType(), > + > currBldrCtx->blockCount()); > + State = State->bindLoc(BaseReg.castAs<Loc>(), InitVal, LC, false); > + > + // Then we'd need to take the value that certainly exists and bind it > + // over. > + if (InitValWithAdjustments.isUnknown()) { > + // Try to recover some path sensitivity in case we couldn't > + // compute the value. > + InitValWithAdjustments = getSValBuilder().conjureSymbolVal( > + Result, LC, InitWithAdjustments->getType(), > + currBldrCtx->blockCount()); > } > + State = > + State->bindLoc(Reg.castAs<Loc>(), InitValWithAdjustments, LC, > false); > + } else { > + State = State->bindLoc(BaseReg.castAs<Loc>(), InitVal, LC, false); > } > > // The result expression would now point to the correct sub-region of > the > @@ -438,10 +443,8 @@ ExprEngine::createTemporaryRegionIfNeede > // correctly in case (Result == Init). > State = State->BindExpr(Result, LC, Reg); > > - if (!FoundOriginalMaterializationRegion) { > - // Notify checkers once for two bindLoc()s. > - State = processRegionChange(State, TR, LC); > - } > + // Notify checkers once for two bindLoc()s. > + State = processRegionChange(State, TR, LC); > > return State; > } > @@ -475,6 +478,30 @@ ProgramStateRef ExprEngine::finishObject > return State->remove<ObjectsUnderConstruction>(Key); > } > > +ProgramStateRef ExprEngine::elideDestructor(ProgramStateRef State, > + const CXXBindTemporaryExpr > *BTE, > + const LocationContext *LC) { > + ElidedDestructorItem I(BTE, LC); > + assert(!State->contains<ElidedDestructors>(I)); > + return State->add<ElidedDestructors>(I); > +} > + > +ProgramStateRef > +ExprEngine::cleanupElidedDestructor(ProgramStateRef State, > + const CXXBindTemporaryExpr *BTE, > + const LocationContext *LC) { > + ElidedDestructorItem I(BTE, LC); > + assert(State->contains<ElidedDestructors>(I)); > + return State->remove<ElidedDestructors>(I); > +} > + > +bool ExprEngine::isDestructorElided(ProgramStateRef State, > + const CXXBindTemporaryExpr *BTE, > + const LocationContext *LC) { > + ElidedDestructorItem I(BTE, LC); > + return State->contains<ElidedDestructors>(I); > +} > + > bool ExprEngine::areAllObjectsFullyConstructed(ProgramStateRef State, > const LocationContext > *FromLC, > const LocationContext > *ToLC) { > @@ -485,6 +512,10 @@ bool ExprEngine::areAllObjectsFullyConst > if (I.first.getLocationContext() == LC) > return false; > > + for (auto I: State->get<ElidedDestructors>()) > + if (I.second == LC) > + return false; > + > LC = LC->getParent(); > } > return true; > @@ -529,6 +560,14 @@ static void printObjectsUnderConstructio > Key.print(Out, nullptr, PP); > Out << " : " << Value << NL; > } > + > + for (auto I : State->get<ElidedDestructors>()) { > + if (I.second != LC) > + continue; > + Out << '(' << I.second << ',' << (const void *)I.first << ") "; > + I.first->printPretty(Out, nullptr, PP); > + Out << " : (constructor elided)" << NL; > + } > } > > void ExprEngine::printState(raw_ostream &Out, ProgramStateRef State, > @@ -1003,10 +1042,11 @@ void ExprEngine::ProcessMemberDtor(const > void ExprEngine::ProcessTemporaryDtor(const CFGTemporaryDtor D, > ExplodedNode *Pred, > ExplodedNodeSet &Dst) { > - ExplodedNodeSet CleanDtorState; > - StmtNodeBuilder StmtBldr(Pred, CleanDtorState, *currBldrCtx); > + const CXXBindTemporaryExpr *BTE = D.getBindTemporaryExpr(); > ProgramStateRef State = Pred->getState(); > + const LocationContext *LC = Pred->getLocationContext(); > const MemRegion *MR = nullptr; > + > if (Optional<SVal> V = > getObjectUnderConstruction(State, D.getBindTemporaryExpr(), > Pred->getLocationContext())) { > @@ -1017,6 +1057,21 @@ void ExprEngine::ProcessTemporaryDtor(co > Pred->getLocationContext()); > MR = V->getAsRegion(); > } > + > + // If copy elision has occured, and the constructor corresponding to the > + // destructor was elided, we need to skip the destructor as well. > + if (isDestructorElided(State, BTE, LC)) { > + State = cleanupElidedDestructor(State, BTE, LC); > + NodeBuilder Bldr(Pred, Dst, *currBldrCtx); > + PostImplicitCall PP(D.getDestructorDecl(getContext()), > + D.getBindTemporaryExpr()->getLocStart(), > + Pred->getLocationContext()); > + Bldr.generateNode(PP, State, Pred); > + return; > + } > + > + ExplodedNodeSet CleanDtorState; > + StmtNodeBuilder StmtBldr(Pred, CleanDtorState, *currBldrCtx); > StmtBldr.generateNode(D.getBindTemporaryExpr(), Pred, State); > > QualType T = D.getBindTemporaryExpr()->getSubExpr()->getType(); > @@ -2201,8 +2256,21 @@ void ExprEngine::processEndOfFunction(No > // it must be a separate problem. > assert(isa<CXXBindTemporaryExpr>(I.first.getStmt())); > State = State->remove<ObjectsUnderConstruction>(I.first); > + // Also cleanup the elided destructor info. > + ElidedDestructorItem Item( > + cast<CXXBindTemporaryExpr>(I.first.getStmt()), > + I.first.getLocationContext()); > + State = State->remove<ElidedDestructors>(Item); > } > > + // Also suppress the assertion for elided destructors when temporary > + // destructors are not provided at all by the CFG, because there's > no > + // good place to clean them up. > + if (!AMgr.getAnalyzerOptions().includeTemporaryDtorsInCFG()) > + for (auto I : State->get<ElidedDestructors>()) > + if (I.second == LC) > + State = State->remove<ElidedDestructors>(I); > + > LC = LC->getParent(); > } > if (State != Pred->getState()) { > > Modified: cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp?rev=335800&r1=335799&r2=335800&view=diff > > ============================================================================== > --- cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp (original) > +++ cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp Wed Jun 27 > 17:30:18 2018 > @@ -209,12 +209,37 @@ std::pair<ProgramStateRef, SVal> ExprEng > } > llvm_unreachable("Unhandled return value construction context!"); > } > - case ConstructionContext::ElidedTemporaryObjectKind: > + case ConstructionContext::ElidedTemporaryObjectKind: { > assert(AMgr.getAnalyzerOptions().shouldElideConstructors()); > - // FALL-THROUGH > + const auto *TCC = > cast<ElidedTemporaryObjectConstructionContext>(CC); > + const CXXBindTemporaryExpr *BTE = TCC->getCXXBindTemporaryExpr(); > + const MaterializeTemporaryExpr *MTE = > TCC->getMaterializedTemporaryExpr(); > + const CXXConstructExpr *CE = TCC->getConstructorAfterElision(); > + > + // Support pre-C++17 copy elision. We'll have the elidable copy > + // constructor in the AST and in the CFG, but we'll skip it > + // and construct directly into the final object. This call > + // also sets the CallOpts flags for us. > + SVal V; > + std::tie(State, V) = prepareForObjectConstruction( > + CE, State, LCtx, TCC->getConstructionContextAfterElision(), > CallOpts); > + > + // Remember that we've elided the constructor. > + State = addObjectUnderConstruction(State, CE, LCtx, V); > + > + // Remember that we've elided the destructor. > + if (BTE) > + State = elideDestructor(State, BTE, LCtx); > + > + // Instead of materialization, shamelessly return > + // the final object destination. > + if (MTE) > + State = addObjectUnderConstruction(State, MTE, LCtx, V); > + > + return std::make_pair(State, V); > + } > case ConstructionContext::SimpleTemporaryObjectKind: { > - // TODO: Copy elision implementation goes here. > - const auto *TCC = cast<TemporaryObjectConstructionContext>(CC); > + const auto *TCC = > cast<SimpleTemporaryObjectConstructionContext>(CC); > const CXXBindTemporaryExpr *BTE = TCC->getCXXBindTemporaryExpr(); > const MaterializeTemporaryExpr *MTE = > TCC->getMaterializedTemporaryExpr(); > SVal V = UnknownVal(); > @@ -266,6 +291,20 @@ void ExprEngine::VisitCXXConstructExpr(c > > SVal Target = UnknownVal(); > > + if (Optional<SVal> ElidedTarget = > + getObjectUnderConstruction(State, CE, LCtx)) { > + // We've previously modeled an elidable constructor by pretending > that it in > + // fact constructs into the correct target. This constructor can > therefore > + // be skipped. > + Target = *ElidedTarget; > + StmtNodeBuilder Bldr(Pred, destNodes, *currBldrCtx); > + State = finishObjectConstruction(State, CE, LCtx); > + if (auto L = Target.getAs<Loc>()) > + State = State->BindExpr(CE, LCtx, State->getSVal(*L, > CE->getType())); > + Bldr.generateNode(CE, Pred, State); > + return; > + } > + > // FIXME: Handle arrays, which run the same constructor for every > element. > // For now, we just run the first constructor (which should still > invalidate > // the entire array). > > Modified: cfe/trunk/test/Analysis/cxx17-mandatory-elision.cpp > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/cxx17-mandatory-elision.cpp?rev=335800&r1=335799&r2=335800&view=diff > > ============================================================================== > --- cfe/trunk/test/Analysis/cxx17-mandatory-elision.cpp (original) > +++ cfe/trunk/test/Analysis/cxx17-mandatory-elision.cpp Wed Jun 27 > 17:30:18 2018 > @@ -1,5 +1,17 @@ > // RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection > -std=c++11 -verify %s > // RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection > -std=c++17 -verify %s > +// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection > -std=c++11 -analyzer-config elide-constructors=false -DNO_ELIDE_FLAG > -verify %s > +// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection > -std=c++17 -analyzer-config elide-constructors=false -DNO_ELIDE_FLAG > -verify %s > + > +// Copy elision always occurs in C++17, otherwise it's under > +// an on-by-default flag. > +#if __cplusplus >= 201703L > + #define ELIDE 1 > +#else > + #ifndef NO_ELIDE_FLAG > + #define ELIDE 1 > + #endif > +#endif > > void clang_analyzer_eval(bool); > > @@ -39,9 +51,9 @@ public: > C() : t(T(4)) { > S s = {1, 2, 3}; > t.s = s; > - // FIXME: Should be TRUE in C++11 as well. > + // FIXME: Should be TRUE regardless of copy elision. > clang_analyzer_eval(t.w == 4); > -#if __cplusplus >= 201703L > +#ifdef ELIDE > // expected-warning@-2{{TRUE}} > #else > // expected-warning@-4{{UNKNOWN}} > @@ -149,7 +161,7 @@ void testMultipleReturns() { > AddressVector<ClassWithoutDestructor> v; > ClassWithoutDestructor c = make3(v); > > -#if __cplusplus >= 201703L > +#if ELIDE > clang_analyzer_eval(v.len == 1); // expected-warning{{TRUE}} > clang_analyzer_eval(v.buf[0] == &c); // expected-warning{{TRUE}} > #else > @@ -184,13 +196,13 @@ void testVariable() { > ClassWithDestructor c = ClassWithDestructor(v); > // Check if the last destructor is an automatic destructor. > // A temporary destructor would have fired by now. > -#if __cplusplus >= 201703L > +#if ELIDE > clang_analyzer_eval(v.len == 1); // expected-warning{{TRUE}} > #else > clang_analyzer_eval(v.len == 3); // expected-warning{{TRUE}} > #endif > } > -#if __cplusplus >= 201703L > +#if ELIDE > // 0. Construct the variable. > // 1. Destroy the variable. > clang_analyzer_eval(v.len == 2); // expected-warning{{TRUE}} > @@ -218,13 +230,13 @@ void testCtorInitializer() { > TestCtorInitializer t(v); > // Check if the last destructor is an automatic destructor. > // A temporary destructor would have fired by now. > -#if __cplusplus >= 201703L > +#if ELIDE > clang_analyzer_eval(v.len == 1); // expected-warning{{TRUE}} > #else > clang_analyzer_eval(v.len == 3); // expected-warning{{TRUE}} > #endif > } > -#if __cplusplus >= 201703L > +#if ELIDE > // 0. Construct the member variable. > // 1. Destroy the member variable. > clang_analyzer_eval(v.len == 2); // expected-warning{{TRUE}} > @@ -257,14 +269,14 @@ void testMultipleReturnsWithDestructors( > ClassWithDestructor c = make3(v); > // Check if the last destructor is an automatic destructor. > // A temporary destructor would have fired by now. > -#if __cplusplus >= 201703L > +#if ELIDE > clang_analyzer_eval(v.len == 1); // expected-warning{{TRUE}} > #else > clang_analyzer_eval(v.len == 9); // expected-warning{{TRUE}} > #endif > } > > -#if __cplusplus >= 201703L > +#if ELIDE > // 0. Construct the variable. Yes, constructor in make1() constructs > // the variable 'c'. > // 1. Destroy the variable. > > Modified: cfe/trunk/test/Analysis/gtest.cpp > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/gtest.cpp?rev=335800&r1=335799&r2=335800&view=diff > > ============================================================================== > --- cfe/trunk/test/Analysis/gtest.cpp (original) > +++ cfe/trunk/test/Analysis/gtest.cpp Wed Jun 27 17:30:18 2018 > @@ -154,13 +154,11 @@ void testConstrainState(int p) { > void testAssertSymbolicPtr(const bool *b) { > ASSERT_TRUE(*b); // no-crash > > - // FIXME: Our solver doesn't handle this well yet. > - clang_analyzer_eval(*b); // expected-warning{{UNKNOWN}} > + clang_analyzer_eval(*b); // expected-warning{{TRUE}} > } > > void testAssertSymbolicRef(const bool &b) { > ASSERT_TRUE(b); // no-crash > > - // FIXME: Our solver doesn't handle this well yet. > - clang_analyzer_eval(b); // expected-warning{{UNKNOWN}} > + clang_analyzer_eval(b); // expected-warning{{TRUE}} > } > > Modified: cfe/trunk/test/Analysis/inlining/temp-dtors-path-notes.cpp > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/inlining/temp-dtors-path-notes.cpp?rev=335800&r1=335799&r2=335800&view=diff > > ============================================================================== > --- cfe/trunk/test/Analysis/inlining/temp-dtors-path-notes.cpp (original) > +++ cfe/trunk/test/Analysis/inlining/temp-dtors-path-notes.cpp Wed Jun 27 > 17:30:18 2018 > @@ -30,19 +30,14 @@ public: > }; > > void test(int coin) { > - // We'd divide by zero in the temporary destructor that goes after the > - // elidable copy-constructor from C(0) to the lifetime-extended > temporary. > - // So in fact this example has nothing to do with lifetime extension. > - // Actually, it would probably be better to elide the constructor, and > - // put the note for the destructor call at the closing brace after nop. > + // We'd divide by zero in the automatic destructor for variable 'c'. > const C &c = coin ? C(1) : C(0); // expected-note {{Assuming 'coin' > is 0}} > // expected-note@-1{{'?' condition is > false}} > // expected-note@-2{{Passing the > value 0 via 1st parameter 'x'}} > // expected-note@-3{{Calling > constructor for 'C'}} > // expected-note@-4{{Returning from > constructor for 'C'}} > - // expected-note@-5{{Calling '~C'}} > c.nop(); > -} > +} // expected-note{{Calling '~C'}} > } // end namespace test_lifetime_extended_temporary > > namespace test_bug_after_dtor { > > Modified: cfe/trunk/test/Analysis/lifetime-extension.cpp > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/lifetime-extension.cpp?rev=335800&r1=335799&r2=335800&view=diff > > ============================================================================== > --- cfe/trunk/test/Analysis/lifetime-extension.cpp (original) > +++ cfe/trunk/test/Analysis/lifetime-extension.cpp Wed Jun 27 17:30:18 2018 > @@ -97,13 +97,10 @@ void f1() { > > void f2() { > C *after, *before; > - C c = C(1, &after, &before); > - clang_analyzer_eval(after == before); > -#ifdef TEMPORARIES > - // expected-warning@-2{{TRUE}} > -#else > - // expected-warning@-4{{UNKNOWN}} > -#endif > + { > + C c = C(1, &after, &before); > + } > + clang_analyzer_eval(after == before); // expected-warning{{TRUE}} > } > > void f3(bool coin) { > @@ -129,6 +126,7 @@ void f4(bool coin) { > // operator. Ideally also add support for the binary conditional > operator in > // C++. Because for now it calls the constructor for the condition > twice. > if (coin) { > + // FIXME: Should not warn. > clang_analyzer_eval(after == before); > #ifdef TEMPORARIES > // expected-warning@-2{{The left operand of '==' is a garbage value}} > @@ -136,13 +134,12 @@ void f4(bool coin) { > // expected-warning@-4{{UNKNOWN}} > #endif > } else { > + // FIXME: Should be TRUE. > clang_analyzer_eval(after == before); > #ifdef TEMPORARIES > - // Seems to work at the moment, but also seems accidental. > - // Feel free to break. > - // expected-warning@-4{{TRUE}} > + // expected-warning@-2{{FALSE}} > #else > - // expected-warning@-6{{UNKNOWN}} > + // expected-warning@-4{{UNKNOWN}} > #endif > } > } > @@ -234,24 +231,25 @@ void f2() { > } // end namespace maintain_original_object_address_on_move > > namespace maintain_address_of_copies { > +class C; > > -template <typename T> struct AddressVector { > - const T *buf[10]; > +struct AddressVector { > + C *buf[10]; > int len; > > AddressVector() : len(0) {} > > - void push(const T *t) { > - buf[len] = t; > + void push(C *c) { > + buf[len] = c; > ++len; > } > }; > > class C { > - AddressVector<C> &v; > + AddressVector &v; > > public: > - C(AddressVector<C> &v) : v(v) { v.push(this); } > + C(AddressVector &v) : v(v) { v.push(this); } > ~C() { v.push(this); } > > #ifdef MOVES > @@ -267,132 +265,70 @@ public: > #endif > } // no-warning > > - static C make(AddressVector<C> &v) { return C(v); } > + static C make(AddressVector &v) { return C(v); } > }; > > void f1() { > - AddressVector<C> v; > + AddressVector v; > { > C c = C(v); > } > - // 0. Create the original temporary and lifetime-extend it into > variable 'c' > - // construction argument. > - // 1. Construct variable 'c' (elidable copy/move). > - // 2. Destroy the temporary. > - // 3. Destroy variable 'c'. > - clang_analyzer_eval(v.len == 4); > - clang_analyzer_eval(v.buf[0] == v.buf[2]); > - clang_analyzer_eval(v.buf[1] == v.buf[3]); > -#ifdef TEMPORARIES > - // expected-warning@-4{{TRUE}} > - // expected-warning@-4{{TRUE}} > - // expected-warning@-4{{TRUE}} > -#else > - // expected-warning@-8{{UNKNOWN}} > - // expected-warning@-8{{UNKNOWN}} > - // expected-warning@-8{{UNKNOWN}} > -#endif > + // 0. Construct variable 'c' (copy/move elided). > + // 1. Destroy variable 'c'. > + clang_analyzer_eval(v.len == 2); // expected-warning{{TRUE}} > + clang_analyzer_eval(v.buf[0] == v.buf[1]); // expected-warning{{TRUE}} > } > > void f2() { > - AddressVector<C> v; > + AddressVector v; > { > const C &c = C::make(v); > } > - // 0. Construct the original temporary within make(), > - // 1. Construct the return value of make() (elidable copy/move) and > - // lifetime-extend it via reference 'c', > - // 2. Destroy the temporary within make(), > - // 3. Destroy the temporary lifetime-extended by 'c'. > - clang_analyzer_eval(v.len == 4); > - clang_analyzer_eval(v.buf[0] == v.buf[2]); > - clang_analyzer_eval(v.buf[1] == v.buf[3]); > + // 0. Construct the return value of make() (copy/move elided) and > + // lifetime-extend it directly via reference 'c', > + // 1. Destroy the temporary lifetime-extended by 'c'. > + clang_analyzer_eval(v.len == 2); > + clang_analyzer_eval(v.buf[0] == v.buf[1]); > #ifdef TEMPORARIES > - // expected-warning@-4{{TRUE}} > - // expected-warning@-4{{TRUE}} > - // expected-warning@-4{{TRUE}} > + // expected-warning@-3{{TRUE}} > + // expected-warning@-3{{TRUE}} > #else > - // expected-warning@-8{{UNKNOWN}} > - // expected-warning@-8{{UNKNOWN}} > - // expected-warning@-8{{UNKNOWN}} > + // expected-warning@-6{{UNKNOWN}} > + // expected-warning@-6{{UNKNOWN}} > #endif > } > > void f3() { > - AddressVector<C> v; > + AddressVector v; > { > C &&c = C::make(v); > } > - // 0. Construct the original temporary within make(), > - // 1. Construct the return value of make() (elidable copy/move) and > - // lifetime-extend it via reference 'c', > - // 2. Destroy the temporary within make(), > - // 3. Destroy the temporary lifetime-extended by 'c'. > - clang_analyzer_eval(v.len == 4); > - clang_analyzer_eval(v.buf[0] == v.buf[2]); > - clang_analyzer_eval(v.buf[1] == v.buf[3]); > + // 0. Construct the return value of make() (copy/move elided) and > + // lifetime-extend it directly via reference 'c', > + // 1. Destroy the temporary lifetime-extended by 'c'. > + clang_analyzer_eval(v.len == 2); > + clang_analyzer_eval(v.buf[0] == v.buf[1]); > #ifdef TEMPORARIES > - // expected-warning@-4{{TRUE}} > - // expected-warning@-4{{TRUE}} > - // expected-warning@-4{{TRUE}} > + // expected-warning@-3{{TRUE}} > + // expected-warning@-3{{TRUE}} > #else > - // expected-warning@-8{{UNKNOWN}} > - // expected-warning@-8{{UNKNOWN}} > - // expected-warning@-8{{UNKNOWN}} > + // expected-warning@-6{{UNKNOWN}} > + // expected-warning@-6{{UNKNOWN}} > #endif > } > > -C doubleMake(AddressVector<C> &v) { > +C doubleMake(AddressVector &v) { > return C::make(v); > } > > void f4() { > - AddressVector<C> v; > + AddressVector v; > { > C c = doubleMake(v); > } > - // 0. Construct the original temporary within make(), > - // 1. Construct the return value of make() (elidable copy/move) and > - // lifetime-extend it into the return value constructor argument > within > - // doubleMake(), > - // 2. Destroy the temporary within make(), > - // 3. Construct the return value of doubleMake() (elidable copy/move) > and > - // lifetime-extend it into the variable 'c' constructor argument, > - // 4. Destroy the return value of make(), > - // 5. Construct variable 'c' (elidable copy/move), > - // 6. Destroy the return value of doubleMake(), > - // 7. Destroy variable 'c'. > - clang_analyzer_eval(v.len == 8); > - clang_analyzer_eval(v.buf[0] == v.buf[2]); > - clang_analyzer_eval(v.buf[1] == v.buf[4]); > - clang_analyzer_eval(v.buf[3] == v.buf[6]); > - clang_analyzer_eval(v.buf[5] == v.buf[7]); > -#ifdef TEMPORARIES > - // expected-warning@-6{{TRUE}} > - // expected-warning@-6{{TRUE}} > - // expected-warning@-6{{TRUE}} > - // expected-warning@-6{{TRUE}} > - // expected-warning@-6{{TRUE}} > -#else > - // expected-warning@-12{{UNKNOWN}} > - // expected-warning@-12{{UNKNOWN}} > - // expected-warning@-12{{UNKNOWN}} > - // expected-warning@-12{{UNKNOWN}} > - // expected-warning@-12{{UNKNOWN}} > -#endif > -} > - > -class NoDtor { > - AddressVector<NoDtor> &v; > - > -public: > - NoDtor(AddressVector<NoDtor> &v) : v(v) { v.push(this); } > -}; > - > -void f5() { > - AddressVector<NoDtor> v; > - const NoDtor &N = NoDtor(v); > - clang_analyzer_eval(v.buf[0] == &N); // expected-warning{{TRUE}} > + // 0. Construct variable 'c' (all copies/moves elided), > + // 1. Destroy variable 'c'. > + clang_analyzer_eval(v.len == 2); // expected-warning{{TRUE}} > + clang_analyzer_eval(v.buf[0] == v.buf[1]); // expected-warning{{TRUE}} > } > - > } // end namespace maintain_address_of_copies > > Modified: cfe/trunk/test/Analysis/temporaries.cpp > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/temporaries.cpp?rev=335800&r1=335799&r2=335800&view=diff > > ============================================================================== > --- cfe/trunk/test/Analysis/temporaries.cpp (original) > +++ cfe/trunk/test/Analysis/temporaries.cpp Wed Jun 27 17:30:18 2018 > @@ -612,107 +612,44 @@ void test() { > clang_analyzer_eval(c3.getY() == 2); // expected-warning{{TRUE}} > > C c4 = returnTemporaryWithConstruction(); > - clang_analyzer_eval(c4.getX() == 1); > - clang_analyzer_eval(c4.getY() == 2); > -#ifdef TEMPORARY_DTORS > - // expected-warning@-3{{TRUE}} > - // expected-warning@-3{{TRUE}} > -#else > - // expected-warning@-6{{UNKNOWN}} > - // expected-warning@-6{{UNKNOWN}} > -#endif > + clang_analyzer_eval(c4.getX() == 1); // expected-warning{{TRUE}} > + clang_analyzer_eval(c4.getY() == 2); // expected-warning{{TRUE}} > > C c5 = returnTemporaryWithAnotherFunctionWithConstruction(); > - clang_analyzer_eval(c5.getX() == 1); > - clang_analyzer_eval(c5.getY() == 2); > -#ifdef TEMPORARY_DTORS > - // expected-warning@-3{{TRUE}} > - // expected-warning@-3{{TRUE}} > -#else > - // expected-warning@-6{{UNKNOWN}} > - // expected-warning@-6{{UNKNOWN}} > -#endif > + clang_analyzer_eval(c5.getX() == 1); // expected-warning{{TRUE}} > + clang_analyzer_eval(c5.getY() == 2); // expected-warning{{TRUE}} > > C c6 = returnTemporaryWithCopyConstructionWithConstruction(); > - clang_analyzer_eval(c5.getX() == 1); > - clang_analyzer_eval(c5.getY() == 2); > -#ifdef TEMPORARY_DTORS > - // expected-warning@-3{{TRUE}} > - // expected-warning@-3{{TRUE}} > -#else > - // expected-warning@-6{{UNKNOWN}} > - // expected-warning@-6{{UNKNOWN}} > -#endif > + clang_analyzer_eval(c5.getX() == 1); // expected-warning{{TRUE}} > + clang_analyzer_eval(c5.getY() == 2); // expected-warning{{TRUE}} > > #if __cplusplus >= 201103L > > C c7 = returnTemporaryWithBraces(); > - clang_analyzer_eval(c7.getX() == 1); > - clang_analyzer_eval(c7.getY() == 2); > -#ifdef TEMPORARY_DTORS > - // expected-warning@-3{{TRUE}} > - // expected-warning@-3{{TRUE}} > -#else > - // expected-warning@-6{{UNKNOWN}} > - // expected-warning@-6{{UNKNOWN}} > -#endif > + clang_analyzer_eval(c7.getX() == 1); // expected-warning{{TRUE}} > + clang_analyzer_eval(c7.getY() == 2); // expected-warning{{TRUE}} > > C c8 = returnTemporaryWithAnotherFunctionWithBraces(); > - clang_analyzer_eval(c8.getX() == 1); > - clang_analyzer_eval(c8.getY() == 2); > -#ifdef TEMPORARY_DTORS > - // expected-warning@-3{{TRUE}} > - // expected-warning@-3{{TRUE}} > -#else > - // expected-warning@-6{{UNKNOWN}} > - // expected-warning@-6{{UNKNOWN}} > -#endif > + clang_analyzer_eval(c8.getX() == 1); // expected-warning{{TRUE}} > + clang_analyzer_eval(c8.getY() == 2); // expected-warning{{TRUE}} > > C c9 = returnTemporaryWithCopyConstructionWithBraces(); > - clang_analyzer_eval(c9.getX() == 1); > - clang_analyzer_eval(c9.getY() == 2); > -#ifdef TEMPORARY_DTORS > - // expected-warning@-3{{TRUE}} > - // expected-warning@-3{{TRUE}} > -#else > - // expected-warning@-6{{UNKNOWN}} > - // expected-warning@-6{{UNKNOWN}} > -#endif > + clang_analyzer_eval(c9.getX() == 1); // expected-warning{{TRUE}} > + clang_analyzer_eval(c9.getY() == 2); // expected-warning{{TRUE}} > > #endif // C++11 > > D d1 = returnTemporaryWithVariableAndNonTrivialCopy(); > - clang_analyzer_eval(d1.getX() == 1); > - clang_analyzer_eval(d1.getY() == 2); > -#ifdef TEMPORARY_DTORS > - // expected-warning@-3{{TRUE}} > - // expected-warning@-3{{TRUE}} > -#else > - // expected-warning@-6{{UNKNOWN}} > - // expected-warning@-6{{UNKNOWN}} > -#endif > + clang_analyzer_eval(d1.getX() == 1); // expected-warning{{TRUE}} > + clang_analyzer_eval(d1.getY() == 2); // expected-warning{{TRUE}} > > D d2 = > returnTemporaryWithAnotherFunctionWithVariableAndNonTrivialCopy(); > - clang_analyzer_eval(d2.getX() == 1); > - clang_analyzer_eval(d2.getY() == 2); > -#ifdef TEMPORARY_DTORS > - // expected-warning@-3{{TRUE}} > - // expected-warning@-3{{TRUE}} > -#else > - // expected-warning@-6{{UNKNOWN}} > - // expected-warning@-6{{UNKNOWN}} > -#endif > + clang_analyzer_eval(d2.getX() == 1); // expected-warning{{TRUE}} > + clang_analyzer_eval(d2.getY() == 2); // expected-warning{{TRUE}} > > D d3 = > returnTemporaryWithCopyConstructionWithVariableAndNonTrivialCopy(); > - clang_analyzer_eval(d3.getX() == 1); > - clang_analyzer_eval(d3.getY() == 2); > -#ifdef TEMPORARY_DTORS > - // expected-warning@-3{{TRUE}} > - // expected-warning@-3{{TRUE}} > -#else > - // expected-warning@-6{{UNKNOWN}} > - // expected-warning@-6{{UNKNOWN}} > -#endif > + clang_analyzer_eval(d3.getX() == 1); // expected-warning{{TRUE}} > + clang_analyzer_eval(d3.getY() == 2); // expected-warning{{TRUE}} > } > } // namespace test_return_temporary > > @@ -767,19 +704,9 @@ void test(int coin) { > > C c2 = coin ? C(1) : C(2); > if (coin) { > - clang_analyzer_eval(c2.getX() == 1); > -#ifdef TEMPORARY_DTORS > - // expected-warning@-2{{TRUE}} > -#else > - // expected-warning@-4{{UNKNOWN}} > -#endif > + clang_analyzer_eval(c2.getX() == 1); // expected-warning{{TRUE}} > } else { > - clang_analyzer_eval(c2.getX() == 2); > -#ifdef TEMPORARY_DTORS > - // expected-warning@-2{{TRUE}} > -#else > - // expected-warning@-4{{UNKNOWN}} > -#endif > + clang_analyzer_eval(c2.getX() == 2); // expected-warning{{TRUE}} > } > } > > @@ -816,16 +743,9 @@ void test_simple_temporary_with_copy() { > { > C c = C(x, y); > } > - // Two constructors (temporary object expr and copy) and two > destructors. > - clang_analyzer_eval(x == 2); > - clang_analyzer_eval(y == 2); > -#ifdef TEMPORARY_DTORS > - // expected-warning@-3{{TRUE}} > - // expected-warning@-3{{TRUE}} > -#else > - // expected-warning@-6{{UNKNOWN}} > - // expected-warning@-6{{UNKNOWN}} > -#endif > + // Only one constructor directly into the variable, and one destructor. > + clang_analyzer_eval(x == 1); // expected-warning{{TRUE}} > + clang_analyzer_eval(y == 1); // expected-warning{{TRUE}} > } > > void test_ternary_temporary(int coin) { > @@ -833,10 +753,10 @@ void test_ternary_temporary(int coin) { > { > const C &c = coin ? C(x, y) : C(z, w); > } > - // This time each branch contains an additional elidable copy > constructor. > + // Only one constructor on every branch, and one automatic destructor. > if (coin) { > - clang_analyzer_eval(x == 2); > - clang_analyzer_eval(y == 2); > + clang_analyzer_eval(x == 1); > + clang_analyzer_eval(y == 1); > #ifdef TEMPORARY_DTORS > // expected-warning@-3{{TRUE}} > // expected-warning@-3{{TRUE}} > @@ -850,8 +770,8 @@ void test_ternary_temporary(int coin) { > } else { > clang_analyzer_eval(x == 0); // expected-warning{{TRUE}} > clang_analyzer_eval(y == 0); // expected-warning{{TRUE}} > - clang_analyzer_eval(z == 2); > - clang_analyzer_eval(w == 2); > + clang_analyzer_eval(z == 1); > + clang_analyzer_eval(w == 1); > #ifdef TEMPORARY_DTORS > // expected-warning@-3{{TRUE}} > // expected-warning@-3{{TRUE}} > @@ -867,33 +787,18 @@ void test_ternary_temporary_with_copy(in > { > C c = coin ? C(x, y) : C(z, w); > } > - // Temporary expression, elidable copy within branch, > - // constructor for variable - 3 total. > + // On each branch the variable is constructed directly. > if (coin) { > - clang_analyzer_eval(x == 3); > - clang_analyzer_eval(y == 3); > -#ifdef TEMPORARY_DTORS > - // expected-warning@-3{{TRUE}} > - // expected-warning@-3{{TRUE}} > -#else > - // expected-warning@-6{{UNKNOWN}} > - // expected-warning@-6{{UNKNOWN}} > -#endif > + clang_analyzer_eval(x == 1); // expected-warning{{TRUE}} > + clang_analyzer_eval(y == 1); // expected-warning{{TRUE}} > clang_analyzer_eval(z == 0); // expected-warning{{TRUE}} > clang_analyzer_eval(w == 0); // expected-warning{{TRUE}} > > } else { > clang_analyzer_eval(x == 0); // expected-warning{{TRUE}} > clang_analyzer_eval(y == 0); // expected-warning{{TRUE}} > - clang_analyzer_eval(z == 3); > - clang_analyzer_eval(w == 3); > -#ifdef TEMPORARY_DTORS > - // expected-warning@-3{{TRUE}} > - // expected-warning@-3{{TRUE}} > -#else > - // expected-warning@-6{{UNKNOWN}} > - // expected-warning@-6{{UNKNOWN}} > -#endif > + clang_analyzer_eval(z == 1); // expected-warning{{TRUE}} > + clang_analyzer_eval(w == 1); // expected-warning{{TRUE}} > } > } > } // namespace test_match_constructors_and_destructors > @@ -928,12 +833,13 @@ void testLifetimeExtendedCall() { > } > > void testCopiedCall() { > - C c = make(); > - // Should have divided by zero in the temporary destructor. > - clang_analyzer_warnIfReached(); > -#ifndef TEMPORARY_DTORS > - // expected-warning@-2{{REACHABLE}} > -#endif > + { > + C c = make(); > + // Should have elided the constructor/destructor for the temporary > + clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}} > + } > + // Should have divided by zero in the destructor. > + clang_analyzer_warnIfReached(); // no-warning > } > } // namespace destructors_for_return_values > > @@ -1018,20 +924,10 @@ void test() { > #endif > > S s = 20; > - clang_analyzer_eval(s.x == 20); > -#ifdef TEMPORARY_DTORS > - // expected-warning@-2{{TRUE}} > -#else > - // expected-warning@-4{{UNKNOWN}} > -#endif > + clang_analyzer_eval(s.x == 20); // expected-warning{{TRUE}} > > C c2 = s; > - clang_analyzer_eval(c2.getX() == 20); > -#ifdef TEMPORARY_DTORS > - // expected-warning@-2{{TRUE}} > -#else > - // expected-warning@-4{{UNKNOWN}} > -#endif > + clang_analyzer_eval(c2.getX() == 20); // expected-warning{{TRUE}} > } > } // end namespace implicit_constructor_conversion > > > > _______________________________________________ > cfe-commits mailing list > cfe-commits@lists.llvm.org > http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits >
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
