This revision was automatically updated to reflect the committed changes. Closed by commit rL328893: [CFG] [analyzer] Avoid modeling C++17 constructors that aren't fully supported. (authored by dergachev, committed by ). Herald added a subscriber: llvm-commits.
Changed prior to commit: https://reviews.llvm.org/D44854?vs=140168&id=140469#toc Repository: rL LLVM https://reviews.llvm.org/D44854 Files: 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/cxx17-mandatory-elision.cpp cfe/trunk/test/Analysis/temp-obj-dtors-cfg-output.cpp
Index: cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp =================================================================== --- cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp +++ cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp @@ -203,13 +203,24 @@ // 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; Index: cfe/trunk/lib/Analysis/CFG.cpp =================================================================== --- cfe/trunk/lib/Analysis/CFG.cpp +++ cfe/trunk/lib/Analysis/CFG.cpp @@ -1302,6 +1302,15 @@ } 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; Index: cfe/trunk/test/Analysis/cxx17-mandatory-elision.cpp =================================================================== --- cfe/trunk/test/Analysis/cxx17-mandatory-elision.cpp +++ cfe/trunk/test/Analysis/cxx17-mandatory-elision.cpp @@ -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 +} Index: cfe/trunk/test/Analysis/temp-obj-dtors-cfg-output.cpp =================================================================== --- cfe/trunk/test/Analysis/temp-obj-dtors-cfg-output.cpp +++ cfe/trunk/test/Analysis/temp-obj-dtors-cfg-output.cpp @@ -218,6 +218,9 @@ // 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)] Index: cfe/trunk/test/Analysis/cfg-rich-constructors.cpp =================================================================== --- cfe/trunk/test/Analysis/cfg-rich-constructors.cpp +++ cfe/trunk/test/Analysis/cfg-rich-constructors.cpp @@ -108,6 +108,9 @@ 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,15 +125,15 @@ // 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 *) // CXX11-NEXT: 3: [B3.2] (CXXConstructExpr, [B3.5], class C) // 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
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits