Author: dergachev Date: Fri Mar 30 12:21:18 2018 New Revision: 328893 URL: http://llvm.org/viewvc/llvm-project?rev=328893&view=rev Log: [CFG] [analyzer] Avoid modeling C++17 constructors that aren't fully supported.
Not enough work has been done so far to ensure correctness of construction contexts in the CFG when C++17 copy elision is in effect, so for now we should drop construction contexts in the CFG and in the analyzer when they seem different from what we support anyway. This includes initializations with conditional operators and return values across multiple stack frames. Differential Revision: https://reviews.llvm.org/D44854 Added: cfe/trunk/test/Analysis/cxx17-mandatory-elision.cpp Modified: cfe/trunk/lib/Analysis/CFG.cpp cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp cfe/trunk/test/Analysis/cfg-rich-constructors.cpp cfe/trunk/test/Analysis/temp-obj-dtors-cfg-output.cpp Modified: cfe/trunk/lib/Analysis/CFG.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/CFG.cpp?rev=328893&r1=328892&r2=328893&view=diff ============================================================================== --- cfe/trunk/lib/Analysis/CFG.cpp (original) +++ cfe/trunk/lib/Analysis/CFG.cpp Fri Mar 30 12:21:18 2018 @@ -1302,6 +1302,15 @@ void CFGBuilder::findConstructionContext } case Stmt::ConditionalOperatorClass: { auto *CO = cast<ConditionalOperator>(Child); + if (!dyn_cast_or_null<MaterializeTemporaryExpr>(Layer->getTriggerStmt())) { + // If the object returned by the conditional operator is not going to be a + // temporary object that needs to be immediately materialized, then + // it must be C++17 with its mandatory copy elision. Do not yet promise + // to support this case. + assert(!CO->getType()->getAsCXXRecordDecl() || CO->isGLValue() || + Context->getLangOpts().CPlusPlus17); + break; + } findConstructionContexts(Layer, CO->getLHS()); findConstructionContexts(Layer, CO->getRHS()); break; Modified: cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp?rev=328893&r1=328892&r2=328893&view=diff ============================================================================== --- cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp (original) +++ cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp Fri Mar 30 12:21:18 2018 @@ -203,13 +203,24 @@ ExprEngine::getRegionForConstructedObjec // 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? - // TODO: We assume that the call site has a temporary object construction - // context. This is no longer true in C++17 or when copy elision is - // performed. We may need to unwrap multiple stack frames here and we - // won't necessarily end up with a temporary at the end. const LocationContext *TempLCtx = LCtx; - if (const LocationContext *CallerLCtx = - LCtx->getCurrentStackFrame()->getParent()) { + const StackFrameContext *SFC = LCtx->getCurrentStackFrame(); + if (const LocationContext *CallerLCtx = SFC->getParent()) { + auto RTC = (*SFC->getCallSiteBlock())[SFC->getIndex()] + .getAs<CFGCXXRecordTypedCall>(); + if (!RTC) { + // We were unable to find the correct construction context for the + // call in the parent stack frame. This is equivalent to not being + // able to find construction context at all. + CallOpts.IsCtorOrDtorWithImproperlyModeledTargetRegion = true; + } else if (!isa<TemporaryObjectConstructionContext>( + RTC->getConstructionContext())) { + // FXIME: 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); + CallOpts.IsCtorOrDtorWithImproperlyModeledTargetRegion = true; + } TempLCtx = CallerLCtx; } CallOpts.IsTemporaryCtorOrDtor = true; Modified: cfe/trunk/test/Analysis/cfg-rich-constructors.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/cfg-rich-constructors.cpp?rev=328893&r1=328892&r2=328893&view=diff ============================================================================== --- cfe/trunk/test/Analysis/cfg-rich-constructors.cpp (original) +++ cfe/trunk/test/Analysis/cfg-rich-constructors.cpp Fri Mar 30 12:21:18 2018 @@ -108,6 +108,9 @@ void simpleVariableInitializedByValue() C c = C::get(); } +// FIXME: Find construction contexts for both branches in C++17. +// Note that once it gets detected, the test for the get() branch would not +// fail, because FileCheck allows partial matches. // CHECK: void simpleVariableWithTernaryOperator(bool coin) // CHECK: [B1] // CXX11-NEXT: 1: [B4.2] ? [B2.5] : [B3.6] @@ -122,7 +125,7 @@ void simpleVariableInitializedByValue() // CXX11-NEXT: 3: [B2.2]() (CXXRecordTypedCall, [B2.4]) // CXX11-NEXT: 4: [B2.3] // CXX11-NEXT: 5: [B2.4] (CXXConstructExpr, [B1.2], class C) -// CXX17-NEXT: 3: [B2.2]() (CXXRecordTypedCall, [B1.2]) +// CXX17-NEXT: 3: [B2.2]() // CHECK: [B3] // CHECK-NEXT: 1: 0 // CHECK-NEXT: 2: [B3.1] (ImplicitCastExpr, NullToPointer, class C *) @@ -130,7 +133,7 @@ void simpleVariableInitializedByValue() // CXX11-NEXT: 4: C([B3.3]) (CXXFunctionalCastExpr, ConstructorConversion, class C) // CXX11-NEXT: 5: [B3.4] // CXX11-NEXT: 6: [B3.5] (CXXConstructExpr, [B1.2], class C) -// CXX17-NEXT: 3: [B3.2] (CXXConstructExpr, [B1.2], class C) +// CXX17-NEXT: 3: [B3.2] (CXXConstructExpr, class C) // CXX17-NEXT: 4: C([B3.3]) (CXXFunctionalCastExpr, ConstructorConversion, class C) // CHECK: [B4] // CHECK-NEXT: 1: coin Added: cfe/trunk/test/Analysis/cxx17-mandatory-elision.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/cxx17-mandatory-elision.cpp?rev=328893&view=auto ============================================================================== --- cfe/trunk/test/Analysis/cxx17-mandatory-elision.cpp (added) +++ cfe/trunk/test/Analysis/cxx17-mandatory-elision.cpp Fri Mar 30 12:21:18 2018 @@ -0,0 +1,58 @@ +// 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 + +void clang_analyzer_eval(bool); + +template <typename T> struct AddressVector { + T *buf[10]; + int len; + + AddressVector() : len(0) {} + + void push(T *t) { + buf[len] = t; + ++len; + } +}; + +class ClassWithoutDestructor { + AddressVector<ClassWithoutDestructor> &v; + +public: + ClassWithoutDestructor(AddressVector<ClassWithoutDestructor> &v) : v(v) { + v.push(this); + } + + ClassWithoutDestructor(ClassWithoutDestructor &&c) : v(c.v) { v.push(this); } + ClassWithoutDestructor(const ClassWithoutDestructor &c) : v(c.v) { + v.push(this); + } +}; + +ClassWithoutDestructor make1(AddressVector<ClassWithoutDestructor> &v) { + return ClassWithoutDestructor(v); +} +ClassWithoutDestructor make2(AddressVector<ClassWithoutDestructor> &v) { + return make1(v); +} +ClassWithoutDestructor make3(AddressVector<ClassWithoutDestructor> &v) { + return make2(v); +} + +void testMultipleReturns() { + AddressVector<ClassWithoutDestructor> v; + 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}} +#else + clang_analyzer_eval(v.len == 5); // expected-warning{{TRUE}} + clang_analyzer_eval(v.buf[0] != v.buf[1]); // expected-warning{{TRUE}} + clang_analyzer_eval(v.buf[1] != v.buf[2]); // expected-warning{{TRUE}} + clang_analyzer_eval(v.buf[2] != v.buf[3]); // expected-warning{{TRUE}} + clang_analyzer_eval(v.buf[3] != v.buf[4]); // expected-warning{{TRUE}} + clang_analyzer_eval(v.buf[4] == &c); // expected-warning{{TRUE}} +#endif +} Modified: cfe/trunk/test/Analysis/temp-obj-dtors-cfg-output.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/temp-obj-dtors-cfg-output.cpp?rev=328893&r1=328892&r2=328893&view=diff ============================================================================== --- cfe/trunk/test/Analysis/temp-obj-dtors-cfg-output.cpp (original) +++ cfe/trunk/test/Analysis/temp-obj-dtors-cfg-output.cpp Fri Mar 30 12:21:18 2018 @@ -218,6 +218,9 @@ C &&foo2(); // Don't try to figure out how to perform construction of the record here. const C &bar1() { return foo1(); } // no-crash C &&bar2() { return foo2(); } // no-crash +const C &bar3(bool coin) { + return coin ? foo1() : foo1(); // no-crash +} } // end namespace pass_references_through // CHECK: [B1 (ENTRY)] _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits