From 22d5fd239a3ecd5736f04fd60a38d06af17cad38 Mon Sep 17 00:00:00 2001 From: Artem Dergachev Date: Thu, 28 Jun 2018 00:11:42 +0000 Subject: [PATCH] [analyzer] Re-enable lifetime extension for temporaries without destructors. When an object's class provides no destructor, it's less important to materialize that object properly because we don't have to model the destructor correctly, so previously we skipped the support for these syntax patterns. Additionally, fix support for construction contexts of "static temporaries" (temporaries that are lifetime-extended by static references) because it turned out that we only had tests for them without destructors, which caused us to regress when we re-introduced the construction context for such temporaries. Differential Revision: https://reviews.llvm.org/D47658 git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@335796 91177308-0d34-0410-b5e6-96231b3b80d8 --- lib/StaticAnalyzer/Core/ExprEngineCXX.cpp | 16 ++++---- test/Analysis/lifetime-extension.cpp | 39 +++++++++++++------- test/Analysis/temporaries-callback-order.cpp | 7 +--- 3 files changed, 34 insertions(+), 28 deletions(-) diff --git a/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp b/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp index 497e1fa556..2e8e14a7a0 100644 --- a/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp +++ b/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp @@ -218,12 +218,6 @@ std::pair ExprEngine::prepareForObjectConstruction( const CXXBindTemporaryExpr *BTE = TCC->getCXXBindTemporaryExpr(); const MaterializeTemporaryExpr *MTE = TCC->getMaterializedTemporaryExpr(); - if (!BTE) { - // FIXME: Lifetime extension for temporaries without destructors - // is not implemented yet. - MTE = nullptr; - } - if (MTE) { if (const ValueDecl *VD = MTE->getExtendingDecl()) { assert(MTE->getStorageDuration() != SD_FullExpression); @@ -238,16 +232,20 @@ std::pair ExprEngine::prepareForObjectConstruction( } } + SVal V = UnknownVal(); if (MTE && MTE->getStorageDuration() != SD_FullExpression) { // If the temporary is lifetime-extended, don't save the BTE, // because we don't need a temporary destructor, but an automatic // destructor. BTE = nullptr; + + if (MTE->getStorageDuration() == SD_Static || + MTE->getStorageDuration() == SD_Thread) + V = loc::MemRegionVal(MRMgr.getCXXStaticTempObjectRegion(E)); } - // FIXME: Support temporaries lifetime-extended via static references. - // They'd need a getCXXStaticTempObjectRegion(). - SVal V = loc::MemRegionVal(MRMgr.getCXXTempObjectRegion(E, LCtx)); + if (V.isUnknown()) + V = loc::MemRegionVal(MRMgr.getCXXTempObjectRegion(E, LCtx)); if (BTE) State = addObjectUnderConstruction(State, BTE, LCtx, V); diff --git a/test/Analysis/lifetime-extension.cpp b/test/Analysis/lifetime-extension.cpp index 1bad166d2d..ee55107838 100644 --- a/test/Analysis/lifetime-extension.cpp +++ b/test/Analysis/lifetime-extension.cpp @@ -234,25 +234,24 @@ void f2() { } // end namespace maintain_original_object_address_on_move namespace maintain_address_of_copies { -class C; -struct AddressVector { - C *buf[10]; +template struct AddressVector { + const T *buf[10]; int len; AddressVector() : len(0) {} - void push(C *c) { - buf[len] = c; + void push(const T *t) { + buf[len] = t; ++len; } }; class C { - AddressVector &v; + AddressVector &v; public: - C(AddressVector &v) : v(v) { v.push(this); } + C(AddressVector &v) : v(v) { v.push(this); } ~C() { v.push(this); } #ifdef MOVES @@ -268,11 +267,11 @@ public: #endif } // no-warning - static C make(AddressVector &v) { return C(v); } + static C make(AddressVector &v) { return C(v); } }; void f1() { - AddressVector v; + AddressVector v; { C c = C(v); } @@ -296,7 +295,7 @@ void f1() { } void f2() { - AddressVector v; + AddressVector v; { const C &c = C::make(v); } @@ -320,7 +319,7 @@ void f2() { } void f3() { - AddressVector v; + AddressVector v; { C &&c = C::make(v); } @@ -343,12 +342,12 @@ void f3() { #endif } -C doubleMake(AddressVector &v) { +C doubleMake(AddressVector &v) { return C::make(v); } void f4() { - AddressVector v; + AddressVector v; { C c = doubleMake(v); } @@ -382,4 +381,18 @@ void f4() { // expected-warning@-12{{UNKNOWN}} #endif } + +class NoDtor { + AddressVector &v; + +public: + NoDtor(AddressVector &v) : v(v) { v.push(this); } +}; + +void f5() { + AddressVector v; + const NoDtor &N = NoDtor(v); + clang_analyzer_eval(v.buf[0] == &N); // expected-warning{{TRUE}} +} + } // end namespace maintain_address_of_copies diff --git a/test/Analysis/temporaries-callback-order.cpp b/test/Analysis/temporaries-callback-order.cpp index df916cc4e7..120fabc104 100644 --- a/test/Analysis/temporaries-callback-order.cpp +++ b/test/Analysis/temporaries-callback-order.cpp @@ -8,11 +8,7 @@ struct Sub : Super { }; void testTemporaries() { - // This triggers RegionChanges twice: - // - Once for zero-initialization of the structure. - // - Once for creating a temporary region and copying the structure there. - // FIXME: This code shouldn't really produce the extra temporary, however - // that's how we behave for now. + // This triggers RegionChanges once for zero-initialization of the structure. Sub().m(); } @@ -29,7 +25,6 @@ void seeIfCheckBindWorks() { // testTemporaries(): // CHECK-NEXT: RegionChanges -// CHECK-NEXT: RegionChanges // Make sure there's no further output. // CHECK-NOT: Bind -- 2.40.0