From 747fefe4691d4b30b5f9859027f80f69cb9ed01d Mon Sep 17 00:00:00 2001 From: Artem Dergachev Date: Thu, 14 Jun 2018 01:59:35 +0000 Subject: [PATCH] [analyzer] Re-enable C++17-specific RVO construction contexts. Not contexts themselves, but rather support for them in the analyzer. Such construction contexts appear when C++17 mandatory copy elision occurs while returning an object from a function, and presence of a destructor causes a CXXBindTemporaryExpr to appear in the AST. Additionally, such construction contexts may be chained, because a return-value construction context doesn't really explain where the object is being returned into, but only points to the parent stack frame, where the object may be consumed by literally anything including another return statement. This behavior is now modeled correctly by the analyzer as long as the object is not returned beyond the boundaries of the analysis. Differential Revision: https://reviews.llvm.org/D47405 git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@334684 91177308-0d34-0410-b5e6-96231b3b80d8 --- lib/StaticAnalyzer/Core/ExprEngineCXX.cpp | 22 +++----- test/Analysis/cxx17-mandatory-elision.cpp | 66 ++++++++++++++++++++++- 2 files changed, 70 insertions(+), 18 deletions(-) diff --git a/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp b/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp index 1dd094b90f..45c8ed1772 100644 --- a/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp +++ b/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp @@ -180,7 +180,8 @@ std::pair ExprEngine::prepareForObjectConstruction( } break; } - case ConstructionContext::SimpleReturnedValueKind: { + case ConstructionContext::SimpleReturnedValueKind: + case ConstructionContext::CXX17ElidedCopyReturnedValueKind: { // The temporary is to be managed by the parent stack frame. // So build it in the parent stack frame if we're not in the // top frame of the analysis. @@ -193,16 +194,10 @@ std::pair ExprEngine::prepareForObjectConstruction( // call in the parent stack frame. This is equivalent to not being // able to find construction context at all. break; - } else if (!isa( - RTC->getConstructionContext())) { - // FIXME: The return value is constructed directly into a - // non-temporary due to C++17 mandatory copy elision. This is not - // implemented yet. - assert(getContext().getLangOpts().CPlusPlus17); - break; } - CC = RTC->getConstructionContext(); - LCtx = CallerLCtx; + return prepareForObjectConstruction( + 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 @@ -212,9 +207,7 @@ std::pair ExprEngine::prepareForObjectConstruction( SVal V = loc::MemRegionVal(MRMgr.getCXXTempObjectRegion(E, LCtx)); return std::make_pair(State, V); } - - // Continue as if we have a temporary with a different location context. - // FALLTHROUGH. + llvm_unreachable("Unhandled return value construction context!"); } case ConstructionContext::TemporaryObjectKind: { const auto *TCC = cast(CC); @@ -261,9 +254,6 @@ std::pair ExprEngine::prepareForObjectConstruction( CallOpts.IsTemporaryCtorOrDtor = true; return std::make_pair(State, V); } - case ConstructionContext::CXX17ElidedCopyReturnedValueKind: - // Not implemented yet. - break; } } // If we couldn't find an existing region to construct into, assume we're diff --git a/test/Analysis/cxx17-mandatory-elision.cpp b/test/Analysis/cxx17-mandatory-elision.cpp index be786be541..e826105279 100644 --- a/test/Analysis/cxx17-mandatory-elision.cpp +++ b/test/Analysis/cxx17-mandatory-elision.cpp @@ -150,9 +150,8 @@ void testMultipleReturns() { ClassWithoutDestructor c = make3(v); #if __cplusplus >= 201703L - // FIXME: Both should be TRUE. clang_analyzer_eval(v.len == 1); // expected-warning{{TRUE}} - clang_analyzer_eval(v.buf[0] == &c); // expected-warning{{FALSE}} + clang_analyzer_eval(v.buf[0] == &c); // expected-warning{{TRUE}} #else clang_analyzer_eval(v.len == 5); // expected-warning{{TRUE}} clang_analyzer_eval(v.buf[0] != v.buf[1]); // expected-warning{{TRUE}} @@ -183,6 +182,13 @@ void testVariable() { AddressVector v; { ClassWithDestructor c = ClassWithDestructor(v); + // Check if the last destructor is an automatic destructor. + // A temporary destructor would have fired by now. +#if __cplusplus >= 201703L + clang_analyzer_eval(v.len == 1); // expected-warning{{TRUE}} +#else + clang_analyzer_eval(v.len == 3); // expected-warning{{TRUE}} +#endif } #if __cplusplus >= 201703L // 0. Construct the variable. @@ -210,6 +216,13 @@ void testCtorInitializer() { AddressVector v; { TestCtorInitializer t(v); + // Check if the last destructor is an automatic destructor. + // A temporary destructor would have fired by now. +#if __cplusplus >= 201703L + clang_analyzer_eval(v.len == 1); // expected-warning{{TRUE}} +#else + clang_analyzer_eval(v.len == 3); // expected-warning{{TRUE}} +#endif } #if __cplusplus >= 201703L // 0. Construct the member variable. @@ -227,4 +240,53 @@ void testCtorInitializer() { #endif } + +ClassWithDestructor make1(AddressVector &v) { + return ClassWithDestructor(v); +} +ClassWithDestructor make2(AddressVector &v) { + return make1(v); +} +ClassWithDestructor make3(AddressVector &v) { + return make2(v); +} + +void testMultipleReturnsWithDestructors() { + AddressVector v; + { + ClassWithDestructor c = make3(v); + // Check if the last destructor is an automatic destructor. + // A temporary destructor would have fired by now. +#if __cplusplus >= 201703L + clang_analyzer_eval(v.len == 1); // expected-warning{{TRUE}} +#else + clang_analyzer_eval(v.len == 9); // expected-warning{{TRUE}} +#endif + } + +#if __cplusplus >= 201703L + // 0. Construct the variable. Yes, constructor in make1() constructs + // the variable 'c'. + // 1. Destroy the variable. + clang_analyzer_eval(v.len == 2); // expected-warning{{TRUE}} + clang_analyzer_eval(v.buf[0] == v.buf[1]); // expected-warning{{TRUE}} +#else + // 0. Construct the temporary in make1(). + // 1. Construct the temporary in make2(). + // 2. Destroy the temporary in make1(). + // 3. Construct the temporary in make3(). + // 4. Destroy the temporary in make2(). + // 5. Construct the temporary here. + // 6. Destroy the temporary in make3(). + // 7. Construct the variable. + // 8. Destroy the temporary here. + // 9. Destroy the variable. + clang_analyzer_eval(v.len == 10); // expected-warning{{TRUE}} + clang_analyzer_eval(v.buf[0] == v.buf[2]); // expected-warning{{TRUE}} + clang_analyzer_eval(v.buf[1] == v.buf[4]); // expected-warning{{TRUE}} + clang_analyzer_eval(v.buf[3] == v.buf[6]); // expected-warning{{TRUE}} + clang_analyzer_eval(v.buf[5] == v.buf[8]); // expected-warning{{TRUE}} + clang_analyzer_eval(v.buf[7] == v.buf[9]); // expected-warning{{TRUE}} +#endif +} } // namespace address_vector_tests -- 2.40.0