r328253 - [analyzer] Remove an assertion that doesn't hold in C++17.
Artem Dergachev via cfe-commits
cfe-commits at lists.llvm.org
Thu Mar 22 14:54:48 PDT 2018
Author: dergachev
Date: Thu Mar 22 14:54:48 2018
New Revision: 328253
URL: http://llvm.org/viewvc/llvm-project?rev=328253&view=rev
Log:
[analyzer] Remove an assertion that doesn't hold in C++17.
Function return values can be constructed directly in variables or passed
directly into return statements, without even an elidable copy in between.
This is how the C++17 mandatory copy elision AST behaves. The behavior we'll
have in such cases is the "old" behavior that we've had before we've
implemented destructor inlining and proper lifetime extension support.
Differential Revision: https://reviews.llvm.org/D44755
Modified:
cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp
cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
cfe/trunk/test/Analysis/lifetime-extension.cpp
cfe/trunk/test/Analysis/temporaries.cpp
Modified: cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp?rev=328253&r1=328252&r2=328253&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp Thu Mar 22 14:54:48 2018
@@ -455,29 +455,51 @@ ProgramStateRef ExprEngine::addAllNecess
const LocationContext *LC, const MemRegion *R) {
const CXXBindTemporaryExpr *BTE = nullptr;
const MaterializeTemporaryExpr *MTE = nullptr;
- const LocationContext *TempLC = LC;
if (CC) {
- // In case of temporary object construction, extract data necessary for
- // destruction and lifetime extension.
- const auto *TCC = dyn_cast<TemporaryObjectConstructionContext>(CC);
-
// If the temporary is being returned from the function, it will be
// destroyed or lifetime-extended in the caller stack frame.
if (isa<ReturnedValueConstructionContext>(CC)) {
const StackFrameContext *SFC = LC->getCurrentStackFrame();
assert(SFC);
- if (SFC->getParent()) {
- TempLC = SFC->getParent();
- const CFGElement &CallElem =
- (*SFC->getCallSiteBlock())[SFC->getIndex()];
- if (auto RTCElem = CallElem.getAs<CFGCXXRecordTypedCall>()) {
- TCC = cast<TemporaryObjectConstructionContext>(
- RTCElem->getConstructionContext());
- }
+ LC = SFC->getParent();
+ if (!LC) {
+ // We are on the top frame. We won't ever need any info
+ // for this temporary, so don't set anything.
+ return State;
+ }
+ const CFGElement &CallElem =
+ (*SFC->getCallSiteBlock())[SFC->getIndex()];
+ auto RTCElem = CallElem.getAs<CFGCXXRecordTypedCall>();
+ if (!RTCElem) {
+ // We have a parent stack frame, but no construction context for the
+ // return value. Give up until we provide the construction context
+ // at the call site.
+ return State;
}
+ // We use the ReturnedValueConstructionContext as an indication that we
+ // need to look for the actual construction context on the parent stack
+ // frame. This purpose has been fulfilled, so now we replace CC with the
+ // actual construction context.
+ CC = RTCElem->getConstructionContext();
+ if (!isa<TemporaryObjectConstructionContext>(CC)) {
+ // TODO: We are not returning an object into a temporary. There must
+ // be copy elision happening at the call site. We still need to
+ // explicitly support the situation when the return value is put
+ // into another return statement, i.e.
+ // ReturnedValueConstructionContexts are chained through multiple
+ // stack frames before finally settling in a temporary.
+ // We don't seem to need to explicitly support construction into
+ // a variable after a return.
+ return State;
+ }
+ // Proceed to deal with the temporary we've found on the parent
+ // stack frame.
}
- if (TCC) {
+
+ // In case of temporary object construction, extract data necessary for
+ // destruction and lifetime extension.
+ if (const auto *TCC = dyn_cast<TemporaryObjectConstructionContext>(CC)) {
if (AMgr.getAnalyzerOptions().includeTemporaryDtorsInCFG()) {
BTE = TCC->getCXXBindTemporaryExpr();
MTE = TCC->getMaterializedTemporaryExpr();
@@ -496,12 +518,12 @@ ProgramStateRef ExprEngine::addAllNecess
}
if (BTE) {
- State = addInitializedTemporary(State, BTE, TempLC,
+ State = addInitializedTemporary(State, BTE, LC,
cast<CXXTempObjectRegion>(R));
}
if (MTE) {
- State = addTemporaryMaterialization(State, MTE, TempLC,
+ State = addTemporaryMaterialization(State, MTE, LC,
cast<CXXTempObjectRegion>(R));
}
}
Modified: cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp?rev=328253&r1=328252&r2=328253&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp Thu Mar 22 14:54:48 2018
@@ -203,6 +203,10 @@ 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()) {
Modified: cfe/trunk/test/Analysis/lifetime-extension.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/lifetime-extension.cpp?rev=328253&r1=328252&r2=328253&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/lifetime-extension.cpp (original)
+++ cfe/trunk/test/Analysis/lifetime-extension.cpp Thu Mar 22 14:54:48 2018
@@ -1,7 +1,11 @@
// RUN: %clang_analyze_cc1 -Wno-unused -std=c++11 -analyzer-checker=core,debug.ExprInspection -analyzer-config cfg-temporary-dtors=false -verify %s
// RUN: %clang_analyze_cc1 -Wno-unused -std=c++11 -analyzer-checker=core,debug.ExprInspection -analyzer-config cfg-temporary-dtors=true,c++-temp-dtor-inlining=true -DTEMPORARIES -verify %s
+// RUN: %clang_analyze_cc1 -Wno-unused -std=c++17 -analyzer-checker=core,debug.ExprInspection -analyzer-config cfg-temporary-dtors=true,c++-temp-dtor-inlining=true -DTEMPORARIES %s
// RUN: %clang_analyze_cc1 -Wno-unused -std=c++11 -analyzer-checker=core,debug.ExprInspection -analyzer-config cfg-temporary-dtors=false -DMOVES -verify %s
// RUN: %clang_analyze_cc1 -Wno-unused -std=c++11 -analyzer-checker=core,debug.ExprInspection -analyzer-config cfg-temporary-dtors=true,c++-temp-dtor-inlining=true -DTEMPORARIES -DMOVES -verify %s
+// RUN: %clang_analyze_cc1 -Wno-unused -std=c++17 -analyzer-checker=core,debug.ExprInspection -analyzer-config cfg-temporary-dtors=true,c++-temp-dtor-inlining=true -DTEMPORARIES -DMOVES %s
+
+// Note: The C++17 run-lines don't -verify yet - it is a no-crash test.
void clang_analyzer_eval(bool);
void clang_analyzer_checkInlined(bool);
Modified: cfe/trunk/test/Analysis/temporaries.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/temporaries.cpp?rev=328253&r1=328252&r2=328253&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/temporaries.cpp (original)
+++ cfe/trunk/test/Analysis/temporaries.cpp Thu Mar 22 14:54:48 2018
@@ -1,6 +1,9 @@
// 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,c++-temp-dtor-inlining=true %s -std=c++11
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection -DTEMPORARY_DTORS -w -analyzer-config cfg-temporary-dtors=true,c++-temp-dtor-inlining=true %s -std=c++17
+
+// Note: The C++17 run-line doesn't -verify yet - it is a no-crash test.
extern bool clang_analyzer_eval(bool);
extern bool clang_analyzer_warnIfReached();
More information about the cfe-commits
mailing list