alexshap created this revision.
Herald added a subscriber: xazax.hun.
This diff attempts to fixe analyzer's crash (triggered assert) on the newly
added test case.
The assert being discussed is assert(!B.lookup(R, BindingKey::Direct)) in
lib/StaticAnalyzer/Core/RegionStore.cpp,
however the root cause appears to be different.
For classes with empty bases the offsets might be tricky.
For example, let's assume we have
struct S: NonEmptyBase, EmptyBase {
...
};
In this case Clang applies empty base class optimization and the offset of
EmptyBase will be 0
(it can be verified via clang -cc1 -x c++ -v -fdump-record-layouts main.cpp
-emit-llvm -o /dev/null).
When the analyzer tries to do zero initialization of EmptyBase it will hit the
assert because that region has already been
"written" by the constructor of NonEmptyBase.
Test plan:
make check-all
Repository:
rL LLVM
https://reviews.llvm.org/D36851
Files:
lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
test/Analysis/ctor.mm
Index: test/Analysis/ctor.mm
===================================================================
--- test/Analysis/ctor.mm
+++ test/Analysis/ctor.mm
@@ -704,3 +704,20 @@
};
}
}
+
+namespace NoCrashOnEmptyBaseOptimization {
+ struct NonEmptyBase {
+ int X;
+ explicit NonEmptyBase(int X) : X(X) {}
+ };
+
+ struct EmptyBase {};
+
+ struct S : NonEmptyBase, EmptyBase {
+ S() : NonEmptyBase(0), EmptyBase() {}
+ };
+
+ void testSCtorNoCrash() {
+ S s;
+ }
+}
Index: lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
===================================================================
--- lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
+++ lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
@@ -297,7 +297,17 @@
ExplodedNodeSet PreInitialized;
{
StmtNodeBuilder Bldr(DstPreVisit, PreInitialized, *currBldrCtx);
- if (CE->requiresZeroInitialization()) {
+ // FIXME: The offsets of empty bases can be tricky because of
+ // of the so called "empty base class optimization".
+ // If a base class has been optimized out
+ // we should not try to create a binding, otherwise we should.
+ // Unfortunately, at the moment ASTRecordLayout doesn't expose
+ // the actual sizes of the empty bases
+ // and trying to infer this from offsets/alignments
+ // seems to be error-prone and tricky because of the trailing padding.
+ // As a temporary mitigation we don't create a binding for empty bases.
+ if (CE->requiresZeroInitialization() &&
+ !CE->getConstructor()->getParent()->isEmpty()) {
// Type of the zero doesn't matter.
SVal ZeroVal = svalBuilder.makeZeroVal(getContext().CharTy);
Index: test/Analysis/ctor.mm
===================================================================
--- test/Analysis/ctor.mm
+++ test/Analysis/ctor.mm
@@ -704,3 +704,20 @@
};
}
}
+
+namespace NoCrashOnEmptyBaseOptimization {
+ struct NonEmptyBase {
+ int X;
+ explicit NonEmptyBase(int X) : X(X) {}
+ };
+
+ struct EmptyBase {};
+
+ struct S : NonEmptyBase, EmptyBase {
+ S() : NonEmptyBase(0), EmptyBase() {}
+ };
+
+ void testSCtorNoCrash() {
+ S s;
+ }
+}
Index: lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
===================================================================
--- lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
+++ lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
@@ -297,7 +297,17 @@
ExplodedNodeSet PreInitialized;
{
StmtNodeBuilder Bldr(DstPreVisit, PreInitialized, *currBldrCtx);
- if (CE->requiresZeroInitialization()) {
+ // FIXME: The offsets of empty bases can be tricky because of
+ // of the so called "empty base class optimization".
+ // If a base class has been optimized out
+ // we should not try to create a binding, otherwise we should.
+ // Unfortunately, at the moment ASTRecordLayout doesn't expose
+ // the actual sizes of the empty bases
+ // and trying to infer this from offsets/alignments
+ // seems to be error-prone and tricky because of the trailing padding.
+ // As a temporary mitigation we don't create a binding for empty bases.
+ if (CE->requiresZeroInitialization() &&
+ !CE->getConstructor()->getParent()->isEmpty()) {
// Type of the zero doesn't matter.
SVal ZeroVal = svalBuilder.makeZeroVal(getContext().CharTy);
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
