NoQ updated this revision to Diff 134346.
NoQ added a comment.
Update the comment with some thoughts from
http://lists.llvm.org/pipermail/cfe-dev/2018-February/056898.html
https://reviews.llvm.org/D42876
Files:
include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
test/Analysis/temporaries.cpp
Index: test/Analysis/temporaries.cpp
===================================================================
--- test/Analysis/temporaries.cpp
+++ test/Analysis/temporaries.cpp
@@ -1,9 +1,10 @@
-// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection -verify -w -std=c++03 %s
-// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection -verify -w -std=c++11 %s
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection -analyzer-config cfg-temporary-dtors=false -verify -w -std=c++03 %s
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection -analyzer-config cfg-temporary-dtors=false -verify -w -std=c++11 %s
// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection -DTEMPORARY_DTORS -verify -w -analyzer-config cfg-temporary-dtors=true %s -std=c++11
extern bool clang_analyzer_eval(bool);
extern bool clang_analyzer_warnIfReached();
+void clang_analyzer_checkInlined(bool);
struct Trivial {
Trivial(int x) : value(x) {}
@@ -422,6 +423,10 @@
struct CtorWithNoReturnDtor {
CtorWithNoReturnDtor() = default;
+ CtorWithNoReturnDtor(int x) {
+ clang_analyzer_checkInlined(false); // no-warning
+ }
+
~CtorWithNoReturnDtor() __attribute__((noreturn));
};
@@ -439,6 +444,12 @@
clang_analyzer_warnIfReached(); // no-warning
}
+#if __cplusplus >= 201103L
+ CtorWithNoReturnDtor returnNoReturnDtor() {
+ return {1}; // no-crash
+ }
+#endif
+
#endif // TEMPORARY_DTORS
}
@@ -530,3 +541,152 @@
Sub(i).m();
}
}
+
+namespace test_return_temporary {
+class C {
+ int x, y;
+
+public:
+ C(int x, int y) : x(x), y(y) {}
+ int getX() const { return x; }
+ int getY() const { return y; }
+ ~C() {}
+};
+
+class D: public C {
+public:
+ D() : C(1, 2) {}
+ D(const D &d): C(d.getX(), d.getY()) {}
+};
+
+C returnTemporaryWithVariable() { C c(1, 2); return c; }
+C returnTemporaryWithAnotherFunctionWithVariable() {
+ return returnTemporaryWithVariable();
+}
+C returnTemporaryWithCopyConstructionWithVariable() {
+ return C(returnTemporaryWithVariable());
+}
+
+C returnTemporaryWithConstruction() { return C(1, 2); }
+C returnTemporaryWithAnotherFunctionWithConstruction() {
+ return returnTemporaryWithConstruction();
+}
+C returnTemporaryWithCopyConstructionWithConstruction() {
+ return C(returnTemporaryWithConstruction());
+}
+
+D returnTemporaryWithVariableAndNonTrivialCopy() { D d; return d; }
+D returnTemporaryWithAnotherFunctionWithVariableAndNonTrivialCopy() {
+ return returnTemporaryWithVariableAndNonTrivialCopy();
+}
+D returnTemporaryWithCopyConstructionWithVariableAndNonTrivialCopy() {
+ return D(returnTemporaryWithVariableAndNonTrivialCopy());
+}
+
+#if __cplusplus >= 201103L
+C returnTemporaryWithBraces() { return {1, 2}; }
+C returnTemporaryWithAnotherFunctionWithBraces() {
+ return returnTemporaryWithBraces();
+}
+C returnTemporaryWithCopyConstructionWithBraces() {
+ return C(returnTemporaryWithBraces());
+}
+#endif // C++11
+
+void test() {
+ C c1 = returnTemporaryWithVariable();
+ clang_analyzer_eval(c1.getX() == 1); // expected-warning{{TRUE}}
+ clang_analyzer_eval(c1.getY() == 2); // expected-warning{{TRUE}}
+
+ C c2 = returnTemporaryWithAnotherFunctionWithVariable();
+ clang_analyzer_eval(c2.getX() == 1); // expected-warning{{TRUE}}
+ clang_analyzer_eval(c2.getY() == 2); // expected-warning{{TRUE}}
+
+ C c3 = returnTemporaryWithCopyConstructionWithVariable();
+ clang_analyzer_eval(c3.getX() == 1); // expected-warning{{TRUE}}
+ clang_analyzer_eval(c3.getY() == 2); // expected-warning{{TRUE}}
+
+ C c4 = returnTemporaryWithConstruction();
+ // Should be TRUE under TEMPORARY_DTORS once this sort of construction
+ // in the inlined function is supported.
+ clang_analyzer_eval(c4.getX() == 1); // expected-warning{{UNKNOWN}}
+ clang_analyzer_eval(c4.getY() == 2); // expected-warning{{UNKNOWN}}
+
+ C c5 = returnTemporaryWithAnotherFunctionWithConstruction();
+ // Should be TRUE under TEMPORARY_DTORS once this sort of construction
+ // in the inlined function is supported.
+ clang_analyzer_eval(c5.getX() == 1); // expected-warning{{UNKNOWN}}
+ clang_analyzer_eval(c5.getY() == 2); // expected-warning{{UNKNOWN}}
+
+ C c6 = returnTemporaryWithCopyConstructionWithConstruction();
+ // Should be TRUE under TEMPORARY_DTORS once this sort of construction
+ // in the inlined function is supported.
+ clang_analyzer_eval(c5.getX() == 1); // expected-warning{{UNKNOWN}}
+ clang_analyzer_eval(c5.getY() == 2); // expected-warning{{UNKNOWN}}
+
+#if __cplusplus >= 201103L
+
+ C c7 = returnTemporaryWithBraces();
+ clang_analyzer_eval(c7.getX() == 1);
+ clang_analyzer_eval(c7.getY() == 2);
+#ifdef TEMPORARY_DTORS
+ // expected-warning@-3{{TRUE}}
+ // expected-warning@-3{{TRUE}}
+#else
+ // expected-warning@-6{{UNKNOWN}}
+ // expected-warning@-6{{UNKNOWN}}
+#endif
+
+ C c8 = returnTemporaryWithAnotherFunctionWithBraces();
+ clang_analyzer_eval(c8.getX() == 1);
+ clang_analyzer_eval(c8.getY() == 2);
+#ifdef TEMPORARY_DTORS
+ // expected-warning@-3{{TRUE}}
+ // expected-warning@-3{{TRUE}}
+#else
+ // expected-warning@-6{{UNKNOWN}}
+ // expected-warning@-6{{UNKNOWN}}
+#endif
+
+ C c9 = returnTemporaryWithCopyConstructionWithBraces();
+ clang_analyzer_eval(c9.getX() == 1);
+ clang_analyzer_eval(c9.getY() == 2);
+#ifdef TEMPORARY_DTORS
+ // expected-warning@-3{{TRUE}}
+ // expected-warning@-3{{TRUE}}
+#else
+ // expected-warning@-6{{UNKNOWN}}
+ // expected-warning@-6{{UNKNOWN}}
+#endif
+
+#endif // C++11
+
+ D d1 = returnTemporaryWithVariableAndNonTrivialCopy();
+ clang_analyzer_eval(d1.getX() == 1);
+ clang_analyzer_eval(d1.getY() == 2);
+#ifdef TEMPORARY_DTORS
+ // expected-warning@-3{{TRUE}}
+ // expected-warning@-3{{TRUE}}
+#else
+ // expected-warning@-6{{UNKNOWN}}
+ // expected-warning@-6{{UNKNOWN}}
+#endif
+
+ D d2 = returnTemporaryWithAnotherFunctionWithVariableAndNonTrivialCopy();
+ clang_analyzer_eval(d2.getX() == 1);
+ clang_analyzer_eval(d2.getY() == 2);
+#ifdef TEMPORARY_DTORS
+ // expected-warning@-3{{TRUE}}
+ // expected-warning@-3{{TRUE}}
+#else
+ // expected-warning@-6{{UNKNOWN}}
+ // expected-warning@-6{{UNKNOWN}}
+#endif
+
+ // Should be TRUE under TEMPORARY_DTORS once this sort of construction
+ // in the inlined function is supported.
+ D d3 = returnTemporaryWithCopyConstructionWithVariableAndNonTrivialCopy();
+ clang_analyzer_eval(d3.getX() == 1); // expected-warning{{UNKNOWN}}
+ clang_analyzer_eval(d3.getY() == 2); // expected-warning{{UNKNOWN}}
+}
+} // namespace test_return_temporary
Index: lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
===================================================================
--- lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
+++ lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
@@ -667,11 +667,19 @@
if (!Opts.mayInlineCXXMemberFunction(CIMK_Destructors))
return CIP_DisallowedAlways;
- // FIXME: This is a hack. We don't handle temporary destructors
- // right now, so we shouldn't inline their constructors.
- if (CtorExpr->getConstructionKind() == CXXConstructExpr::CK_Complete)
+ if (CtorExpr->getConstructionKind() == CXXConstructExpr::CK_Complete) {
+ // If we don't handle temporary destructors, we shouldn't inline
+ // their constructors.
+ if (CallOpts.IsConstructorIntoTemporary &&
+ !Opts.includeTemporaryDtorsInCFG())
+ return CIP_DisallowedOnce;
+
+ // If we did not construct the correct this-region, it would be pointless
+ // to inline the constructor. Instead we will simply invalidate
+ // the fake temporary target.
if (CallOpts.IsConstructorWithImproperlyModeledTargetRegion)
return CIP_DisallowedOnce;
+ }
break;
}
Index: lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
===================================================================
--- lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
+++ lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
@@ -113,6 +113,7 @@
EvalCallOptions &CallOpts) {
const LocationContext *LCtx = Pred->getLocationContext();
ProgramStateRef State = Pred->getState();
+ MemRegionManager &MRMgr = getSValBuilder().getRegionManager();
// See if we're constructing an existing region by looking at the
// current construction context.
@@ -141,6 +142,17 @@
LValue = makeZeroElementRegion(State, LValue, Ty,
CallOpts.IsArrayConstructorOrDestructor);
return LValue.getAsRegion();
+ } else if (auto *RS = dyn_cast<ReturnStmt>(TriggerStmt)) {
+ // TODO: We should construct into a CXXBindTemporaryExpr or a
+ // MaterializeTemporaryExpr around the call-expression on the previous
+ // stack frame. Currently we re-bind the temporary to the correct region
+ // later, but that's not semantically correct. This of course does not
+ // apply when we're in the top frame. But if we are in an inlined
+ // function, we should be able to take the call-site CFG element,
+ // and it should contain (but right now it wouldn't) some sort of
+ // construction context that'd give us the right temporary expression.
+ CallOpts.IsConstructorIntoTemporary = true;
+ return MRMgr.getCXXTempObjectRegion(CE, LCtx);
}
// TODO: Consider other directly initialized elements.
} else if (const CXXCtorInitializer *Init = CC->getTriggerInit()) {
@@ -173,7 +185,6 @@
// If we couldn't find an existing region to construct into, assume we're
// constructing a temporary. Notify the caller of our failure.
CallOpts.IsConstructorWithImproperlyModeledTargetRegion = true;
- MemRegionManager &MRMgr = getSValBuilder().getRegionManager();
return MRMgr.getCXXTempObjectRegion(CE, LCtx);
}
Index: include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
===================================================================
--- include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
+++ include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
@@ -577,6 +577,7 @@
struct EvalCallOptions {
bool IsConstructorWithImproperlyModeledTargetRegion = false;
bool IsArrayConstructorOrDestructor = false;
+ bool IsConstructorIntoTemporary = false;
EvalCallOptions() {}
};
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
