NoQ created this revision.
NoQ added reviewers: dcoughlin, xazax.hun, a.sidorin, george.karpenkov, szepet.
Herald added subscribers: cfe-commits, rnkovacs.
Tried to actually run the analyzer with temporary destructor support on some
real code, found two crashes for now - https://reviews.llvm.org/D43144 and this
one. In this example, a temporary of type `C[2]` is constructed in the context
of `InitListExpr` within `CXXBindTemporaryExpr`:
`-ExprWithCleanups 0x7f82b9020c28 <line:872:3, col:36>
'std::initializer_list<C>':'std::initializer_list<temporary_list_crash::C>'
`-CXXFunctionalCastExpr 0x7f82b9020c00 <col:3, col:36>
'std::initializer_list<C>':'std::initializer_list<temporary_list_crash::C>'
functional cast to std::initializer_list<C> <NoOp>
`-CXXStdInitializerListExpr 0x7f82ba0258f8 <col:27, col:36>
'std::initializer_list<C>':'std::initializer_list<temporary_list_crash::C>'
`-MaterializeTemporaryExpr 0x7f82ba0258e0 <col:27, col:36> 'const
temporary_list_crash::C [2]' xvalue
`-CXXBindTemporaryExpr 0x7f82ba0258c0 <col:27, col:36> 'const
temporary_list_crash::C [2]' (CXXTemporary 0x7f82ba0258b8) *** <=== THIS ONE
***
`-InitListExpr 0x7f82ba025748 <col:27, col:36> 'const
temporary_list_crash::C [2]'
|-CXXConstructExpr 0x7f82ba025818 <col:28, col:30> 'const
temporary_list_crash::C' 'void (const temporary_list_crash::C &) noexcept'
elidable
| `-MaterializeTemporaryExpr 0x7f82ba0257b0 <col:28, col:30>
'const temporary_list_crash::C' lvalue
| `-ImplicitCastExpr 0x7f82ba025798 <col:28, col:30> 'const
temporary_list_crash::C' <NoOp>
| `-CXXBindTemporaryExpr 0x7f82ba024ae8 <col:28, col:30>
'temporary_list_crash::C' (CXXTemporary 0x7f82ba024ae0)
| `-CXXTemporaryObjectExpr 0x7f82ba024aa8 <col:28, col:30>
'temporary_list_crash::C' 'void ()'
`-CXXConstructExpr 0x7f82ba025880 <col:33, col:35> 'const
temporary_list_crash::C' 'void (const temporary_list_crash::C &) noexcept'
elidable
`-MaterializeTemporaryExpr 0x7f82ba025868 <col:33, col:35>
'const temporary_list_crash::C' lvalue
`-ImplicitCastExpr 0x7f82ba025850 <col:33, col:35> 'const
temporary_list_crash::C' <NoOp>
`-CXXBindTemporaryExpr 0x7f82ba024b58 <col:33, col:35>
'temporary_list_crash::C' (CXXTemporary 0x7f82ba024b50)
`-CXXTemporaryObjectExpr 0x7f82ba024b18 <col:33, col:35>
'temporary_list_crash::C' 'void ()'
We'd have to actually support this case eventually, but for now do the trick
that we've always did: unwrap the array type and pretend we're destroying a
single object. Once we have the construction context provided for this
constructor, we'd have the region here, and in this case we'd also need to
construct `ElementRegion` as part of this trick. And even then, calling a
destructor of a single array element does not invalidate the whole array for
us, because destructors are `const` (unless there are mutable members). So we'd
have to do this manually later as well.
Repository:
rC Clang
https://reviews.llvm.org/D43149
Files:
lib/StaticAnalyzer/Core/ExprEngine.cpp
test/Analysis/temporaries.cpp
Index: test/Analysis/temporaries.cpp
===================================================================
--- test/Analysis/temporaries.cpp
+++ test/Analysis/temporaries.cpp
@@ -6,6 +6,8 @@
extern bool clang_analyzer_warnIfReached();
void clang_analyzer_checkInlined(bool);
+#include "Inputs/system-header-simulator-cxx.h";
+
struct Trivial {
Trivial(int x) : value(x) {}
int value;
@@ -857,3 +859,17 @@
}
}
} // namespace test_match_constructors_and_destructors
+
+#if __cplusplus >= 201103L
+namespace temporary_list_crash {
+class C {
+public:
+ C() {}
+ ~C() {}
+};
+
+void test() {
+ std::initializer_list<C>{C(), C()}; // no-crash
+}
+} // namespace temporary_list_crash
+#endif // C++11
Index: lib/StaticAnalyzer/Core/ExprEngine.cpp
===================================================================
--- lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -957,18 +957,37 @@
}
StmtBldr.generateNode(D.getBindTemporaryExpr(), Pred, State);
- QualType varType = D.getBindTemporaryExpr()->getSubExpr()->getType();
+ QualType T = D.getBindTemporaryExpr()->getSubExpr()->getType();
// FIXME: Currently CleanDtorState can be empty here due to temporaries being
// bound to default parameters.
assert(CleanDtorState.size() <= 1);
ExplodedNode *CleanPred =
CleanDtorState.empty() ? Pred : *CleanDtorState.begin();
EvalCallOptions CallOpts;
CallOpts.IsTemporaryCtorOrDtor = true;
- if (!MR)
+ if (!MR) {
CallOpts.IsCtorOrDtorWithImproperlyModeledTargetRegion = true;
- VisitCXXDestructor(varType, MR, D.getBindTemporaryExpr(),
+
+ // If we have no MR, we still need to unwrap the array to avoid destroying
+ // the whole array at once. Regardless, we'd eventually need to model array
+ // destructors properly, element-by-element.
+ while (const ArrayType *AT = getContext().getAsArrayType(T)) {
+ T = AT->getElementType();
+ CallOpts.IsArrayCtorOrDtor = true;
+ }
+ } else {
+ // We'd eventually need to makeZeroElementRegion() trick here,
+ // but for now we don't have the respective construction contexts,
+ // so MR would always be null in this case. Do nothing for now.
+ // Note that destructors are const, and therefore they don't invalidate
+ // their storage when no mutable members are present. It means that
+ // we cannot model array destructors by simply calling a destructor
+ // for a single element and expect it to invalidate the whole array for us.
+ // We'd have to invalidate it manually somehow. Escaping should work
+ // just fine though.
+ }
+ VisitCXXDestructor(T, MR, D.getBindTemporaryExpr(),
/*IsBase=*/false, CleanPred, Dst, CallOpts);
}
Index: test/Analysis/temporaries.cpp
===================================================================
--- test/Analysis/temporaries.cpp
+++ test/Analysis/temporaries.cpp
@@ -6,6 +6,8 @@
extern bool clang_analyzer_warnIfReached();
void clang_analyzer_checkInlined(bool);
+#include "Inputs/system-header-simulator-cxx.h";
+
struct Trivial {
Trivial(int x) : value(x) {}
int value;
@@ -857,3 +859,17 @@
}
}
} // namespace test_match_constructors_and_destructors
+
+#if __cplusplus >= 201103L
+namespace temporary_list_crash {
+class C {
+public:
+ C() {}
+ ~C() {}
+};
+
+void test() {
+ std::initializer_list<C>{C(), C()}; // no-crash
+}
+} // namespace temporary_list_crash
+#endif // C++11
Index: lib/StaticAnalyzer/Core/ExprEngine.cpp
===================================================================
--- lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -957,18 +957,37 @@
}
StmtBldr.generateNode(D.getBindTemporaryExpr(), Pred, State);
- QualType varType = D.getBindTemporaryExpr()->getSubExpr()->getType();
+ QualType T = D.getBindTemporaryExpr()->getSubExpr()->getType();
// FIXME: Currently CleanDtorState can be empty here due to temporaries being
// bound to default parameters.
assert(CleanDtorState.size() <= 1);
ExplodedNode *CleanPred =
CleanDtorState.empty() ? Pred : *CleanDtorState.begin();
EvalCallOptions CallOpts;
CallOpts.IsTemporaryCtorOrDtor = true;
- if (!MR)
+ if (!MR) {
CallOpts.IsCtorOrDtorWithImproperlyModeledTargetRegion = true;
- VisitCXXDestructor(varType, MR, D.getBindTemporaryExpr(),
+
+ // If we have no MR, we still need to unwrap the array to avoid destroying
+ // the whole array at once. Regardless, we'd eventually need to model array
+ // destructors properly, element-by-element.
+ while (const ArrayType *AT = getContext().getAsArrayType(T)) {
+ T = AT->getElementType();
+ CallOpts.IsArrayCtorOrDtor = true;
+ }
+ } else {
+ // We'd eventually need to makeZeroElementRegion() trick here,
+ // but for now we don't have the respective construction contexts,
+ // so MR would always be null in this case. Do nothing for now.
+ // Note that destructors are const, and therefore they don't invalidate
+ // their storage when no mutable members are present. It means that
+ // we cannot model array destructors by simply calling a destructor
+ // for a single element and expect it to invalidate the whole array for us.
+ // We'd have to invalidate it manually somehow. Escaping should work
+ // just fine though.
+ }
+ VisitCXXDestructor(T, MR, D.getBindTemporaryExpr(),
/*IsBase=*/false, CleanPred, Dst, CallOpts);
}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
