Skip to content

Commit

Permalink
Merging r311182:
Browse files Browse the repository at this point in the history
------------------------------------------------------------------------
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

------------------------------------------------------------------------

llvm-svn: 311378
  • Loading branch information
zmodem committed Aug 21, 2017
1 parent 69da959 commit f3b83c6
Show file tree
Hide file tree
Showing 2 changed files with 30 additions and 0 deletions.
13 changes: 13 additions & 0 deletions clang/lib/StaticAnalyzer/Core/RegionStore.cpp
Expand Up @@ -409,6 +409,19 @@ class RegionStoreManager : public StoreManager {

// 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));

Expand Down
17 changes: 17 additions & 0 deletions clang/test/Analysis/ctor.mm
Expand Up @@ -704,3 +704,20 @@ void g() {
};
}
}

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;
}
}

0 comments on commit f3b83c6

Please sign in to comment.