r311182 - [analyzer] Fix modeling of constructors

[Alexander Shaposhnikov via cfe-commits]

Author: alexshap
Date: Fri Aug 18 11:20:43 2017
New Revision: 311182

URL: http://llvm.org/viewvc/llvm-project?rev=311182&view=rev
Log:
[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

Modified:
    cfe/trunk/lib/StaticAnalyzer/Core/RegionStore.cpp
    cfe/trunk/test/Analysis/ctor.mm

Modified: cfe/trunk/lib/StaticAnalyzer/Core/RegionStore.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/RegionStore.cpp?rev=311182&r1=311181&r2=311182&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Core/RegionStore.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/RegionStore.cpp Fri Aug 18 11:20:43 2017
@@ -409,6 +409,19 @@ public: // Part of public interface to c
 
   // 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));
 

Modified: cfe/trunk/test/Analysis/ctor.mm
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/ctor.mm?rev=311182&r1=311181&r2=311182&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/ctor.mm (original)
+++ cfe/trunk/test/Analysis/ctor.mm Fri Aug 18 11:20:43 2017
@@ -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;
+  }
+}