r328893 - [CFG] [analyzer] Avoid modeling C++17 constructors that aren't fully supported.

Artem Dergachev via cfe-commits cfe-commits at lists.llvm.org
Fri Mar 30 12:21:18 PDT 2018


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)]




More information about the cfe-commits mailing list