NoQ created this revision.
NoQ added reviewers: dcoughlin, xazax.hun, a.sidorin, george.karpenkov, szepet,
rnkovacs.
Herald added subscribers: cfe-commits, baloghadamsoftware.
As explained in http://lists.llvm.org/pipermail/cfe-dev/2018-May/058055.html,
`findDirectConstructorForCurrentCFGElement()` was not working correctly in the
current example and erroneously made us think that we've constructed into a
dummy temporary region rather than into the variable. Instead, it was proposed
to track it in the program state every time we are performing construction
correctly.
Additionally this information will be used to maintain liveness of the
variable's Store bindings, because previously an incorrect Environment binding
of the target region to the construct-expression was used for that purpose.
Such binding is incorrect because the constructor is a prvalue of an object
type and should therefore have a NonLoc representing its symbolic value.
Therefore the hack implemented by `isTemporaryPRValue()` can be safely removed.
`findDirectConstructorForCurrentCFGElement()` cannot be removed yet because it
is also used for constructor member initializers for the same purpose. It
doesn't seem to introduce bugs though.
Repository:
rC Clang
https://reviews.llvm.org/D47305
Files:
lib/StaticAnalyzer/Core/ExprEngineC.cpp
lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
test/Analysis/cxx17-mandatory-elision.cpp
Index: test/Analysis/cxx17-mandatory-elision.cpp
===================================================================
--- test/Analysis/cxx17-mandatory-elision.cpp
+++ test/Analysis/cxx17-mandatory-elision.cpp
@@ -3,6 +3,26 @@
void clang_analyzer_eval(bool);
+namespace variable_functional_cast_crash {
+
+struct A {
+ A(int) {}
+};
+
+void foo() {
+ A a = A(0);
+}
+
+struct B {
+ A a;
+ B(): a(A(0)) {}
+};
+
+} // namespace variable_functional_cast_crash
+
+
+namespace address_vector_tests {
+
template <typename T> struct AddressVector {
T *buf[10];
int len;
@@ -56,3 +76,5 @@
clang_analyzer_eval(v.buf[4] == &c); // expected-warning{{TRUE}}
#endif
}
+
+} // namespace address_vector_tests
Index: lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
===================================================================
--- lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
+++ lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
@@ -193,23 +193,6 @@
return RuntimeCallee->getCanonicalDecl() != StaticDecl->getCanonicalDecl();
}
-/// Returns true if the CXXConstructExpr \p E was intended to construct a
-/// prvalue for the region in \p V.
-///
-/// Note that we can't just test for rvalue vs. glvalue because
-/// CXXConstructExprs embedded in DeclStmts and initializers are considered
-/// rvalues by the AST, and the analyzer would like to treat them as lvalues.
-static bool isTemporaryPRValue(const CXXConstructExpr *E, SVal V) {
- if (E->isGLValue())
- return false;
-
- const MemRegion *MR = V.getAsRegion();
- if (!MR)
- return false;
-
- return isa<CXXTempObjectRegion>(MR);
-}
-
/// The call exit is simulated with a sequence of nodes, which occur between
/// CallExitBegin and CallExitEnd. The following operations occur between the
/// two program points:
@@ -269,11 +252,7 @@
loc::MemRegionVal This =
svalBuilder.getCXXThis(CCE->getConstructor()->getParent(), calleeCtx);
SVal ThisV = state->getSVal(This);
-
- // If the constructed object is a temporary prvalue, get its bindings.
- if (isTemporaryPRValue(CCE, ThisV))
- ThisV = state->getSVal(ThisV.castAs<Loc>());
-
+ ThisV = state->getSVal(ThisV.castAs<Loc>());
state = state->BindExpr(CCE, callerCtx, ThisV);
}
@@ -574,11 +553,7 @@
}
} else if (const CXXConstructorCall *C = dyn_cast<CXXConstructorCall>(&Call)){
SVal ThisV = C->getCXXThisVal();
-
- // If the constructed object is a temporary prvalue, get its bindings.
- if (isTemporaryPRValue(cast<CXXConstructExpr>(E), ThisV))
- ThisV = State->getSVal(ThisV.castAs<Loc>());
-
+ ThisV = State->getSVal(ThisV.castAs<Loc>());
return State->BindExpr(E, LCtx, ThisV);
}
Index: lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
===================================================================
--- lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
+++ lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
@@ -125,9 +125,11 @@
const auto *Var = cast<VarDecl>(DS->getSingleDecl());
SVal LValue = State->getLValue(Var, LCtx);
QualType Ty = Var->getType();
- return std::make_pair(
- State,
- makeZeroElementRegion(State, LValue, Ty, CallOpts.IsArrayCtorOrDtor));
+ LValue =
+ makeZeroElementRegion(State, LValue, Ty, CallOpts.IsArrayCtorOrDtor);
+ State =
+ addObjectUnderConstruction(State, DSCC->getDeclStmt(), LCtx, LValue);
+ return std::make_pair(State, LValue);
}
case ConstructionContext::SimpleConstructorInitializerKind: {
const auto *ICC = cast<ConstructorInitializerConstructionContext>(CC);
Index: lib/StaticAnalyzer/Core/ExprEngineC.cpp
===================================================================
--- lib/StaticAnalyzer/Core/ExprEngineC.cpp
+++ lib/StaticAnalyzer/Core/ExprEngineC.cpp
@@ -590,24 +590,12 @@
SVal InitVal = state->getSVal(InitEx, LC);
assert(DS->isSingleDecl());
- if (auto *CtorExpr = findDirectConstructorForCurrentCFGElement()) {
- assert(InitEx->IgnoreImplicit() == CtorExpr);
- (void)CtorExpr;
+ if (getObjectUnderConstruction(state, DS, LC)) {
+ state = finishObjectConstruction(state, DS, LC);
// We constructed the object directly in the variable.
// No need to bind anything.
B.generateNode(DS, UpdatedN, state);
} else {
- // We bound the temp obj region to the CXXConstructExpr. Now recover
- // the lazy compound value when the variable is not a reference.
- if (AMgr.getLangOpts().CPlusPlus && VD->getType()->isRecordType() &&
- !VD->getType()->isReferenceType()) {
- if (Optional<loc::MemRegionVal> M =
- InitVal.getAs<loc::MemRegionVal>()) {
- InitVal = state->getSVal(M->getRegion());
- assert(InitVal.getAs<nonloc::LazyCompoundVal>());
- }
- }
-
// Recover some path-sensitivity if a scalar value evaluated to
// UnknownVal.
if (InitVal.isUnknown()) {
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
