r327345 - [analyzer] Destroy and lifetime-extend inlined function return values properly.
Mikael Holmén via cfe-commits
cfe-commits at lists.llvm.org
Tue Mar 13 23:34:45 PDT 2018
Hi Artem,
This commit gives the following warning:
../tools/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp:283:23: error:
unused variable 'RCC' [-Werror,-Wunused-variable]
if (const auto *RCC =
dyn_cast<ReturnedValueConstructionContext>(CC)) {
^
1 error generated.
Regards,
Mikael
On 03/13/2018 12:22 AM, Artem Dergachev via cfe-commits wrote:
> Author: dergachev
> Date: Mon Mar 12 16:22:35 2018
> New Revision: 327345
>
> URL: http://llvm.org/viewvc/llvm-project?rev=327345&view=rev
> Log:
> [analyzer] Destroy and lifetime-extend inlined function return values properly.
>
> This patch uses the newly added CFGCXXRecordTypedCall element at the call site
> of the caller to construct the return value within the callee directly into the
> caller's stack frame. This way it is also capable of populating the temporary
> destructor and lifetime extension maps for the temporary, which allows
> temporary destructors and lifetime extension to work correctly.
>
> This patch does not affect temporaries that were returned from conservatively
> evaluated functions.
>
> Differential Revision: https://reviews.llvm.org/D44124
>
> Modified:
> cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
> cfe/trunk/test/Analysis/lifetime-extension.cpp
>
> Modified: cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp?rev=327345&r1=327344&r2=327345&view=diff
> ==============================================================================
> --- cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp (original)
> +++ cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp Mon Mar 12 16:22:35 2018
> @@ -197,16 +197,19 @@ ExprEngine::getRegionForConstructedObjec
> 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,6 +265,7 @@ void ExprEngine::VisitCXXConstructExpr(c
> assert(C || getCurrentCFGElement().getAs<CFGStmt>());
> const ConstructionContext *CC = C ? C->getConstructionContext() : nullptr;
>
> + bool IsReturnedIntoParentStackFrame = false;
> const CXXBindTemporaryExpr *BTE = nullptr;
> const MaterializeTemporaryExpr *MTE = nullptr;
>
> @@ -269,25 +273,44 @@ void ExprEngine::VisitCXXConstructExpr(c
> 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 @@ void ExprEngine::VisitCXXConstructExpr(c
> 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));
> }
>
>
> Modified: cfe/trunk/test/Analysis/lifetime-extension.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/lifetime-extension.cpp?rev=327345&r1=327344&r2=327345&view=diff
> ==============================================================================
> --- cfe/trunk/test/Analysis/lifetime-extension.cpp (original)
> +++ cfe/trunk/test/Analysis/lifetime-extension.cpp Mon Mar 12 16:22:35 2018
> @@ -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 @@ public:
> ~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 @@ void f2() {
> 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 at -4{{TRUE}}
> + // expected-warning at -4{{TRUE}}
> + // expected-warning at -4{{TRUE}}
> +#else
> + // expected-warning at -8{{UNKNOWN}}
> + // expected-warning at -8{{UNKNOWN}}
> + // expected-warning at -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 at -4{{TRUE}}
> + // expected-warning at -4{{TRUE}}
> + // expected-warning at -4{{TRUE}}
> +#else
> + // expected-warning at -8{{UNKNOWN}}
> + // expected-warning at -8{{UNKNOWN}}
> + // expected-warning at -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 at -4{{TRUE}}
> + // expected-warning at -4{{TRUE}}
> + // expected-warning at -4{{TRUE}}
> +#else
> + // expected-warning at -8{{UNKNOWN}}
> + // expected-warning at -8{{UNKNOWN}}
> + // expected-warning at -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 at -6{{TRUE}}
> + // expected-warning at -6{{TRUE}}
> + // expected-warning at -6{{TRUE}}
> + // expected-warning at -6{{TRUE}}
> + // expected-warning at -6{{TRUE}}
> +#else
> + // expected-warning at -12{{UNKNOWN}}
> + // expected-warning at -12{{UNKNOWN}}
> + // expected-warning at -12{{UNKNOWN}}
> + // expected-warning at -12{{UNKNOWN}}
> + // expected-warning at -12{{UNKNOWN}}
> +#endif
> +}
> +} // end namespace maintain_address_of_copies
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
More information about the cfe-commits
mailing list