This revision was automatically updated to reflect the committed changes.
Closed by commit rC327345: [analyzer] Destroy and lifetime-extend inlined
function return values properly. (authored by dergachev, committed by ).
Repository:
rC Clang
https://reviews.llvm.org/D44124
Files:
lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
test/Analysis/lifetime-extension.cpp
Index: lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
===================================================================
--- lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
+++ lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
@@ -197,16 +197,19 @@
return MRMgr.getCXXTempObjectRegion(CE, LCtx);
}
case ConstructionContext::ReturnedValueKind: {
- // 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.
+ // The temporary is to be managed by the parent stack frame.
+ // So build it in the parent stack frame if we're not in the
+ // top frame of the analysis.
+ // 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?
+ const LocationContext *TempLCtx = LCtx;
+ if (const LocationContext *CallerLCtx =
+ LCtx->getCurrentStackFrame()->getParent()) {
+ TempLCtx = CallerLCtx;
+ }
CallOpts.IsTemporaryCtorOrDtor = true;
- return MRMgr.getCXXTempObjectRegion(CE, LCtx);
+ return MRMgr.getCXXTempObjectRegion(CE, TempLCtx);
}
}
}
@@ -262,32 +265,52 @@
assert(C || getCurrentCFGElement().getAs<CFGStmt>());
const ConstructionContext *CC = C ? C->getConstructionContext() : nullptr;
+ bool IsReturnedIntoParentStackFrame = false;
const CXXBindTemporaryExpr *BTE = nullptr;
const MaterializeTemporaryExpr *MTE = nullptr;
switch (CE->getConstructionKind()) {
case CXXConstructExpr::CK_Complete: {
Target = getRegionForConstructedObject(CE, Pred, CC, CallOpts);
- // In case of temporary object construction, extract data necessary for
- // destruction and lifetime extension.
- if (const auto *TCC =
- dyn_cast_or_null<TemporaryObjectConstructionContext>(CC)) {
- assert(CallOpts.IsTemporaryCtorOrDtor);
- assert(!CallOpts.IsCtorOrDtorWithImproperlyModeledTargetRegion);
- if (AMgr.getAnalyzerOptions().includeTemporaryDtorsInCFG()) {
- BTE = TCC->getCXXBindTemporaryExpr();
- MTE = TCC->getMaterializedTemporaryExpr();
- if (!BTE) {
- // FIXME: lifetime extension for temporaries without destructors
- // is not implemented yet.
- MTE = nullptr;
+ 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 (const auto *RCC = dyn_cast<ReturnedValueConstructionContext>(CC)) {
+ const StackFrameContext *SFC = LCtx->getCurrentStackFrame();
+ assert(SFC);
+ if (SFC->getParent()) {
+ IsReturnedIntoParentStackFrame = true;
+ const CFGElement &CallElem =
+ (*SFC->getCallSiteBlock())[SFC->getIndex()];
+ if (auto RTCElem = CallElem.getAs<CFGCXXRecordTypedCall>()) {
+ TCC = cast<TemporaryObjectConstructionContext>(
+ RTCElem->getConstructionContext());
+ }
}
- if (MTE && MTE->getStorageDuration() != SD_FullExpression) {
- // If the temporary is lifetime-extended, don't save the BTE,
- // because we don't need a temporary destructor, but an automatic
- // destructor.
- BTE = nullptr;
+ }
+
+ if (TCC) {
+ assert(CallOpts.IsTemporaryCtorOrDtor);
+ assert(!CallOpts.IsCtorOrDtorWithImproperlyModeledTargetRegion);
+ if (AMgr.getAnalyzerOptions().includeTemporaryDtorsInCFG()) {
+ BTE = TCC->getCXXBindTemporaryExpr();
+ MTE = TCC->getMaterializedTemporaryExpr();
+ if (!BTE) {
+ // FIXME: lifetime extension for temporaries without destructors
+ // is not implemented yet.
+ MTE = nullptr;
+ }
+ if (MTE && MTE->getStorageDuration() != SD_FullExpression) {
+ // If the temporary is lifetime-extended, don't save the BTE,
+ // because we don't need a temporary destructor, but an automatic
+ // destructor.
+ BTE = nullptr;
+ }
}
}
}
@@ -385,13 +408,19 @@
State = State->bindDefault(loc::MemRegionVal(Target), ZeroVal, LCtx);
}
+ // Set up destruction and lifetime extension information.
+ const LocationContext *TempLCtx =
+ IsReturnedIntoParentStackFrame
+ ? LCtx->getCurrentStackFrame()->getParent()
+ : LCtx;
+
if (BTE) {
- State = addInitializedTemporary(State, BTE, LCtx,
+ State = addInitializedTemporary(State, BTE, TempLCtx,
cast<CXXTempObjectRegion>(Target));
}
if (MTE) {
- State = addTemporaryMaterialization(State, MTE, LCtx,
+ State = addTemporaryMaterialization(State, MTE, TempLCtx,
cast<CXXTempObjectRegion>(Target));
}
Index: test/Analysis/lifetime-extension.cpp
===================================================================
--- test/Analysis/lifetime-extension.cpp
+++ test/Analysis/lifetime-extension.cpp
@@ -1,7 +1,10 @@
// 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++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
void clang_analyzer_eval(bool);
+void clang_analyzer_checkInlined(bool);
namespace pr17001_call_wrong_destructor {
bool x;
@@ -63,6 +66,8 @@
~C() { if (after) *after = this; }
operator bool() const { return x; }
+
+ static C make(C **after, C **before) { return C(false, after, before); }
};
void f1() {
@@ -180,3 +185,154 @@
1 / x; // no-warning
}
} // end namespace maintain_original_object_address_on_move
+
+namespace maintain_address_of_copies {
+class C;
+
+struct AddressVector {
+ C *buf[10];
+ int len;
+
+ AddressVector() : len(0) {}
+
+ void push(C *c) {
+ buf[len] = c;
+ ++len;
+ }
+};
+
+class C {
+ AddressVector &v;
+
+public:
+ C(AddressVector &v) : v(v) { v.push(this); }
+ ~C() { v.push(this); }
+
+#ifdef MOVES
+ C(C &&c) : v(c.v) { v.push(this); }
+#endif
+
+ // Note how return-statements prefer move-constructors when available.
+ C(const C &c) : v(c.v) {
+#ifdef MOVES
+ clang_analyzer_checkInlined(false); // no-warning
+#else
+ v.push(this);
+#endif
+ } // no-warning
+
+ static C make(AddressVector &v) { return C(v); }
+};
+
+void f1() {
+ AddressVector v;
+ {
+ C c = C(v);
+ }
+ // 0. Create the original temporary and lifetime-extend it into variable 'c'
+ // construction argument.
+ // 1. Construct variable 'c' (elidable copy/move).
+ // 2. Destroy the temporary.
+ // 3. Destroy variable 'c'.
+ clang_analyzer_eval(v.len == 4);
+ clang_analyzer_eval(v.buf[0] == v.buf[2]);
+ clang_analyzer_eval(v.buf[1] == v.buf[3]);
+#ifdef TEMPORARIES
+ // expected-warning@-4{{TRUE}}
+ // expected-warning@-4{{TRUE}}
+ // expected-warning@-4{{TRUE}}
+#else
+ // expected-warning@-8{{UNKNOWN}}
+ // expected-warning@-8{{UNKNOWN}}
+ // expected-warning@-8{{UNKNOWN}}
+#endif
+}
+
+void f2() {
+ AddressVector v;
+ {
+ const C &c = C::make(v);
+ }
+ // 0. Construct the original temporary within make(),
+ // 1. Construct the return value of make() (elidable copy/move) and
+ // lifetime-extend it via reference 'c',
+ // 2. Destroy the temporary within make(),
+ // 3. Destroy the temporary lifetime-extended by 'c'.
+ clang_analyzer_eval(v.len == 4);
+ clang_analyzer_eval(v.buf[0] == v.buf[2]);
+ clang_analyzer_eval(v.buf[1] == v.buf[3]);
+#ifdef TEMPORARIES
+ // expected-warning@-4{{TRUE}}
+ // expected-warning@-4{{TRUE}}
+ // expected-warning@-4{{TRUE}}
+#else
+ // expected-warning@-8{{UNKNOWN}}
+ // expected-warning@-8{{UNKNOWN}}
+ // expected-warning@-8{{UNKNOWN}}
+#endif
+}
+
+void f3() {
+ AddressVector v;
+ {
+ C &&c = C::make(v);
+ }
+ // 0. Construct the original temporary within make(),
+ // 1. Construct the return value of make() (elidable copy/move) and
+ // lifetime-extend it via reference 'c',
+ // 2. Destroy the temporary within make(),
+ // 3. Destroy the temporary lifetime-extended by 'c'.
+ clang_analyzer_eval(v.len == 4);
+ clang_analyzer_eval(v.buf[0] == v.buf[2]);
+ clang_analyzer_eval(v.buf[1] == v.buf[3]);
+#ifdef TEMPORARIES
+ // expected-warning@-4{{TRUE}}
+ // expected-warning@-4{{TRUE}}
+ // expected-warning@-4{{TRUE}}
+#else
+ // expected-warning@-8{{UNKNOWN}}
+ // expected-warning@-8{{UNKNOWN}}
+ // expected-warning@-8{{UNKNOWN}}
+#endif
+}
+
+C doubleMake(AddressVector &v) {
+ return C::make(v);
+}
+
+void f4() {
+ AddressVector v;
+ {
+ C c = doubleMake(v);
+ }
+ // 0. Construct the original temporary within make(),
+ // 1. Construct the return value of make() (elidable copy/move) and
+ // lifetime-extend it into the return value constructor argument within
+ // doubleMake(),
+ // 2. Destroy the temporary within make(),
+ // 3. Construct the return value of doubleMake() (elidable copy/move) and
+ // lifetime-extend it into the variable 'c' constructor argument,
+ // 4. Destroy the return value of make(),
+ // 5. Construct variable 'c' (elidable copy/move),
+ // 6. Destroy the return value of doubleMake(),
+ // 7. Destroy variable 'c'.
+ clang_analyzer_eval(v.len == 8);
+ clang_analyzer_eval(v.buf[0] == v.buf[2]);
+ clang_analyzer_eval(v.buf[1] == v.buf[4]);
+ clang_analyzer_eval(v.buf[3] == v.buf[6]);
+ clang_analyzer_eval(v.buf[5] == v.buf[7]);
+#ifdef TEMPORARIES
+ // expected-warning@-6{{TRUE}}
+ // expected-warning@-6{{TRUE}}
+ // expected-warning@-6{{TRUE}}
+ // expected-warning@-6{{TRUE}}
+ // expected-warning@-6{{TRUE}}
+#else
+ // expected-warning@-12{{UNKNOWN}}
+ // expected-warning@-12{{UNKNOWN}}
+ // expected-warning@-12{{UNKNOWN}}
+ // expected-warning@-12{{UNKNOWN}}
+ // expected-warning@-12{{UNKNOWN}}
+#endif
+}
+} // end namespace maintain_address_of_copies
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
