NoQ updated this revision to Diff 140168.
NoQ added a comment.
Fix a comment in tests.
https://reviews.llvm.org/D44854
Files:
lib/Analysis/CFG.cpp
lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
test/Analysis/cfg-rich-constructors.cpp
test/Analysis/cxx17-mandatory-elision.cpp
test/Analysis/temp-obj-dtors-cfg-output.cpp
Index: test/Analysis/temp-obj-dtors-cfg-output.cpp
===================================================================
--- test/Analysis/temp-obj-dtors-cfg-output.cpp
+++ 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: test/Analysis/cxx17-mandatory-elision.cpp
===================================================================
--- /dev/null
+++ 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: test/Analysis/cfg-rich-constructors.cpp
===================================================================
--- test/Analysis/cfg-rich-constructors.cpp
+++ 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
Index: lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
===================================================================
--- lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
+++ 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: lib/Analysis/CFG.cpp
===================================================================
--- lib/Analysis/CFG.cpp
+++ 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;
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
