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

Reply via email to