diff options
author | Hans Wennborg <hans@hanshq.net> | 2017-08-21 20:27:26 +0000 |
---|---|---|
committer | Hans Wennborg <hans@hanshq.net> | 2017-08-21 20:27:26 +0000 |
commit | 16b986343f64f416abbecfa0da51a481e86cf588 (patch) | |
tree | da95688f61ad0ad4cddb57a88b0ea930c378dc3f | |
parent | 606412eb2a44f0513512a1fbba7c5872c420bd43 (diff) |
Merging r311182:
------------------------------------------------------------------------
r311182 | alexshap | 2017-08-18 11:20:43 -0700 (Fri, 18 Aug 2017) | 22 lines
[analyzer] Fix modeling of constructors
This diff fixes 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 is 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 perform 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
Differential revision: https://reviews.llvm.org/D36851
------------------------------------------------------------------------
git-svn-id: https://llvm.org/svn/llvm-project/cfe/branches/release_50@311378 91177308-0d34-0410-b5e6-96231b3b80d8
-rw-r--r-- | lib/StaticAnalyzer/Core/RegionStore.cpp | 13 | ||||
-rw-r--r-- | test/Analysis/ctor.mm | 17 |
2 files changed, 30 insertions, 0 deletions
diff --git a/lib/StaticAnalyzer/Core/RegionStore.cpp b/lib/StaticAnalyzer/Core/RegionStore.cpp index 28f78fa3ff..11902f66df 100644 --- a/lib/StaticAnalyzer/Core/RegionStore.cpp +++ b/lib/StaticAnalyzer/Core/RegionStore.cpp @@ -409,6 +409,19 @@ public: // Part of public interface to class. // BindDefault is only used to initialize a region with a default value. StoreRef BindDefault(Store store, const MemRegion *R, SVal V) override { + // 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 them from offsets/alignments + // seems to be error-prone and non-trivial because of the trailing padding. + // As a temporary mitigation we don't create bindings for empty bases. + if (R->getKind() == MemRegion::CXXBaseObjectRegionKind && + cast<CXXBaseObjectRegion>(R)->getDecl()->isEmpty()) + return StoreRef(store, *this); + RegionBindingsRef B = getRegionBindings(store); assert(!B.lookup(R, BindingKey::Direct)); diff --git a/test/Analysis/ctor.mm b/test/Analysis/ctor.mm index 646229aac9..619e2cb0f0 100644 --- a/test/Analysis/ctor.mm +++ b/test/Analysis/ctor.mm @@ -704,3 +704,20 @@ namespace PR19579 { }; } } + +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; + } +} |