Skip to content

Commit 643102d

Browse files
committed
[analyzer] Re-enable constructors when lifetime extension through fields occurs.
Temporary object constructor inlining was disabled in r326240 for code like const int &x = A().x; because automatic destructor for the lifetime-extended object A() was not working correctly in CFG. CFG was fixed in r333941, so inlining can be re-enabled. CFG for lifetime extension through aggregates still needs to be fixed. Differential Revision: https://reviews.llvm.org/D44239 llvm-svn: 333946
1 parent 63db25b commit 643102d

File tree

4 files changed

+24
-31
lines changed

4 files changed

+24
-31
lines changed

clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -105,11 +105,6 @@ class ExprEngine : public SubEngine {
105105
/// This call is a constructor or a destructor of a temporary value.
106106
bool IsTemporaryCtorOrDtor = false;
107107

108-
/// This call is a constructor for a temporary that is lifetime-extended
109-
/// by binding a smaller object within it to a reference, for example
110-
/// 'const int &x = C().x;'.
111-
bool IsTemporaryLifetimeExtendedViaSubobject = false;
112-
113108
/// This call is a constructor for a temporary that is lifetime-extended
114109
/// by binding it to a reference-type field within an aggregate,
115110
/// for example 'A { const C &c; }; A a = { C() };'

clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp

Lines changed: 5 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -181,22 +181,13 @@ SVal ExprEngine::getLocationForConstructedObject(const CXXConstructExpr *CE,
181181
const auto *TOCC = cast<TemporaryObjectConstructionContext>(CC);
182182
if (const auto *MTE = TOCC->getMaterializedTemporaryExpr()) {
183183
if (const ValueDecl *VD = MTE->getExtendingDecl()) {
184-
// Pattern-match various forms of lifetime extension that aren't
185-
// currently supported by the CFG.
186-
// FIXME: Is there a better way to retrieve this information from
187-
// the MaterializeTemporaryExpr?
188184
assert(MTE->getStorageDuration() != SD_FullExpression);
189-
if (VD->getType()->isReferenceType()) {
190-
assert(VD->getType()->isReferenceType());
191-
if (VD->getType()->getPointeeType().getCanonicalType() !=
192-
MTE->GetTemporaryExpr()->getType().getCanonicalType()) {
193-
// We're lifetime-extended via our field. Automatic destructors
194-
// aren't quite working in this case.
195-
CallOpts.IsTemporaryLifetimeExtendedViaSubobject = true;
196-
}
197-
} else {
185+
if (!VD->getType()->isReferenceType()) {
198186
// We're lifetime-extended by a surrounding aggregate.
199-
// Automatic destructors aren't quite working in this case.
187+
// Automatic destructors aren't quite working in this case
188+
// on the CFG side. We should warn the caller about that.
189+
// FIXME: Is there a better way to retrieve this information from
190+
// the MaterializeTemporaryExpr?
200191
CallOpts.IsTemporaryLifetimeExtendedViaAggregate = true;
201192
}
202193
}

clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -696,12 +696,7 @@ ExprEngine::mayInlineCallKind(const CallEvent &Call, const ExplodedNode *Pred,
696696
if (CallOpts.IsCtorOrDtorWithImproperlyModeledTargetRegion)
697697
return CIP_DisallowedOnce;
698698

699-
// If the temporary is lifetime-extended by binding a smaller object
700-
// within it to a reference, automatic destructors don't work properly.
701-
if (CallOpts.IsTemporaryLifetimeExtendedViaSubobject)
702-
return CIP_DisallowedOnce;
703-
704-
// If the temporary is lifetime-extended by binding it to a reference-typ
699+
// If the temporary is lifetime-extended by binding it to a reference-type
705700
// field within an aggregate, automatic destructors don't work properly.
706701
if (CallOpts.IsTemporaryLifetimeExtendedViaAggregate)
707702
return CIP_DisallowedOnce;

clang/test/Analysis/lifetime-extension.cpp

Lines changed: 18 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -46,10 +46,18 @@ void f() {
4646
const int &y = A().j[1]; // no-crash
4747
const int &z = (A().j[1], A().j[0]); // no-crash
4848

49-
// FIXME: All of these should be TRUE, but constructors aren't inlined.
50-
clang_analyzer_eval(x == 1); // expected-warning{{UNKNOWN}}
51-
clang_analyzer_eval(y == 3); // expected-warning{{UNKNOWN}}
52-
clang_analyzer_eval(z == 2); // expected-warning{{UNKNOWN}}
49+
clang_analyzer_eval(x == 1);
50+
clang_analyzer_eval(y == 3);
51+
clang_analyzer_eval(z == 2);
52+
#ifdef TEMPORARIES
53+
// expected-warning@-4{{TRUE}}
54+
// expected-warning@-4{{TRUE}}
55+
// expected-warning@-4{{TRUE}}
56+
#else
57+
// expected-warning@-8{{UNKNOWN}}
58+
// expected-warning@-8{{UNKNOWN}}
59+
// expected-warning@-8{{UNKNOWN}}
60+
#endif
5361
}
5462
} // end namespace pr19539_crash_on_destroying_an_integer
5563

@@ -144,8 +152,12 @@ void f5() {
144152
{
145153
const bool &x = C(true, &after, &before).x; // no-crash
146154
}
147-
// FIXME: Should be TRUE. Should not warn about garbage value.
148-
clang_analyzer_eval(after == before); // expected-warning{{UNKNOWN}}
155+
clang_analyzer_eval(after == before);
156+
#ifdef TEMPORARIES
157+
// expected-warning@-2{{TRUE}}
158+
#else
159+
// expected-warning@-4{{UNKNOWN}}
160+
#endif
149161
}
150162

151163
struct A { // A is an aggregate.

0 commit comments

Comments
 (0)