NoQ created this revision. NoQ added reviewers: dcoughlin, xazax.hun, a.sidorin, george.karpenkov, szepet. Herald added subscribers: cfe-commits, rnkovacs, baloghadamsoftware.
This hotfix is similar to https://reviews.llvm.org/D43689 (and needs a follow-up similar to https://reviews.llvm.org/D44238 and https://reviews.llvm.org/D44239). CFG again doesn't provide us with correct automatic destructors, this time it's in the following code: struct A { const C &c; }; void foo() { A a = { C(); } } In this code `a` is an aggregate, so it doesn't require construction or destruction. Instead, `C()` is lifetime-extended until the end of `a`'s scope. Additionally, we used to crash on my defensive "i know C++" assertion (no, i don't). Repository: rC Clang https://reviews.llvm.org/D46037 Files: include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h lib/StaticAnalyzer/Core/ExprEngineCXX.cpp lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp test/Analysis/lifetime-extension.cpp Index: test/Analysis/lifetime-extension.cpp =================================================================== --- test/Analysis/lifetime-extension.cpp +++ test/Analysis/lifetime-extension.cpp @@ -147,6 +147,30 @@ // FIXME: Should be TRUE. Should not warn about garbage value. clang_analyzer_eval(after == before); // expected-warning{{UNKNOWN}} } + +void f6() { + C *after, *before; + { + struct A { // A is an aggregate. + const C &c; + }; + A a{C(true, &after, &before)}; + } + // FIXME: Should be TRUE. Should not warn about garbage value. + clang_analyzer_eval(after == before); // expected-warning{{UNKNOWN}} +} + +void f7() { + C *after, *before; + { + struct A { // A is an aggregate. + const C &c; + }; + A a = {C(true, &after, &before)}; + } + // FIXME: Should be TRUE. Should not warn about garbage value. + clang_analyzer_eval(after == before); // expected-warning{{UNKNOWN}} +} } // end namespace maintain_original_object_address_on_lifetime_extension namespace maintain_original_object_address_on_move { Index: lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp =================================================================== --- lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp +++ lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp @@ -699,6 +699,11 @@ // within it to a reference, automatic destructors don't work properly. if (CallOpts.IsTemporaryLifetimeExtendedViaSubobject) return CIP_DisallowedOnce; + + // If the temporary is lifetime-extended by binding it to a reference-typ + // field within an aggregate, automatic destructors don't work properly. + if (CallOpts.IsTemporaryLifetimeExtendedViaAggregate) + return CIP_DisallowedOnce; } break; Index: lib/StaticAnalyzer/Core/ExprEngineCXX.cpp =================================================================== --- lib/StaticAnalyzer/Core/ExprEngineCXX.cpp +++ lib/StaticAnalyzer/Core/ExprEngineCXX.cpp @@ -180,14 +180,22 @@ } case ConstructionContext::TemporaryObjectKind: { const auto *TOCC = cast<TemporaryObjectConstructionContext>(CC); - // See if we're lifetime-extended via our field. If so, take a note. - // Because automatic destructors aren't quite working in this case. if (const auto *MTE = TOCC->getMaterializedTemporaryExpr()) { if (const ValueDecl *VD = MTE->getExtendingDecl()) { - assert(VD->getType()->isReferenceType()); - if (VD->getType()->getPointeeType().getCanonicalType() != - MTE->GetTemporaryExpr()->getType().getCanonicalType()) { - CallOpts.IsTemporaryLifetimeExtendedViaSubobject = true; + if (const auto *RD = VD->getType()->getAsCXXRecordDecl()) { + // We're lifetime-extended by a surrounding aggregate. + // Automatic destructors aren't quite working in this case. + assert(RD->isAggregate()); + assert(RD != CE->getConstructor()->getParent()); + CallOpts.IsTemporaryLifetimeExtendedViaAggregate = true; + } else { + assert(VD->getType()->isReferenceType()); + if (VD->getType()->getPointeeType().getCanonicalType() != + MTE->GetTemporaryExpr()->getType().getCanonicalType()) { + // We're lifetime-extended via our field. Automatic destructors + // aren't quite working in this case. + CallOpts.IsTemporaryLifetimeExtendedViaSubobject = true; + } } } } Index: include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h =================================================================== --- include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h +++ include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h @@ -110,6 +110,11 @@ /// 'const int &x = C().x;'. bool IsTemporaryLifetimeExtendedViaSubobject = false; + /// This call is a constructor for a temporary that is lifetime-extended + /// by binding it to a reference-type field within an aggregate, + /// for example 'A { const C &c; }; A a = { C() };' + bool IsTemporaryLifetimeExtendedViaAggregate = false; + EvalCallOptions() {} };
Index: test/Analysis/lifetime-extension.cpp =================================================================== --- test/Analysis/lifetime-extension.cpp +++ test/Analysis/lifetime-extension.cpp @@ -147,6 +147,30 @@ // FIXME: Should be TRUE. Should not warn about garbage value. clang_analyzer_eval(after == before); // expected-warning{{UNKNOWN}} } + +void f6() { + C *after, *before; + { + struct A { // A is an aggregate. + const C &c; + }; + A a{C(true, &after, &before)}; + } + // FIXME: Should be TRUE. Should not warn about garbage value. + clang_analyzer_eval(after == before); // expected-warning{{UNKNOWN}} +} + +void f7() { + C *after, *before; + { + struct A { // A is an aggregate. + const C &c; + }; + A a = {C(true, &after, &before)}; + } + // FIXME: Should be TRUE. Should not warn about garbage value. + clang_analyzer_eval(after == before); // expected-warning{{UNKNOWN}} +} } // end namespace maintain_original_object_address_on_lifetime_extension namespace maintain_original_object_address_on_move { Index: lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp =================================================================== --- lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp +++ lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp @@ -699,6 +699,11 @@ // within it to a reference, automatic destructors don't work properly. if (CallOpts.IsTemporaryLifetimeExtendedViaSubobject) return CIP_DisallowedOnce; + + // If the temporary is lifetime-extended by binding it to a reference-typ + // field within an aggregate, automatic destructors don't work properly. + if (CallOpts.IsTemporaryLifetimeExtendedViaAggregate) + return CIP_DisallowedOnce; } break; Index: lib/StaticAnalyzer/Core/ExprEngineCXX.cpp =================================================================== --- lib/StaticAnalyzer/Core/ExprEngineCXX.cpp +++ lib/StaticAnalyzer/Core/ExprEngineCXX.cpp @@ -180,14 +180,22 @@ } case ConstructionContext::TemporaryObjectKind: { const auto *TOCC = cast<TemporaryObjectConstructionContext>(CC); - // See if we're lifetime-extended via our field. If so, take a note. - // Because automatic destructors aren't quite working in this case. if (const auto *MTE = TOCC->getMaterializedTemporaryExpr()) { if (const ValueDecl *VD = MTE->getExtendingDecl()) { - assert(VD->getType()->isReferenceType()); - if (VD->getType()->getPointeeType().getCanonicalType() != - MTE->GetTemporaryExpr()->getType().getCanonicalType()) { - CallOpts.IsTemporaryLifetimeExtendedViaSubobject = true; + if (const auto *RD = VD->getType()->getAsCXXRecordDecl()) { + // We're lifetime-extended by a surrounding aggregate. + // Automatic destructors aren't quite working in this case. + assert(RD->isAggregate()); + assert(RD != CE->getConstructor()->getParent()); + CallOpts.IsTemporaryLifetimeExtendedViaAggregate = true; + } else { + assert(VD->getType()->isReferenceType()); + if (VD->getType()->getPointeeType().getCanonicalType() != + MTE->GetTemporaryExpr()->getType().getCanonicalType()) { + // We're lifetime-extended via our field. Automatic destructors + // aren't quite working in this case. + CallOpts.IsTemporaryLifetimeExtendedViaSubobject = true; + } } } } Index: include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h =================================================================== --- include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h +++ include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h @@ -110,6 +110,11 @@ /// 'const int &x = C().x;'. bool IsTemporaryLifetimeExtendedViaSubobject = false; + /// This call is a constructor for a temporary that is lifetime-extended + /// by binding it to a reference-type field within an aggregate, + /// for example 'A { const C &c; }; A a = { C() };' + bool IsTemporaryLifetimeExtendedViaAggregate = false; + EvalCallOptions() {} };
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits