r311182 - [analyzer] Fix modeling of constructors

[Anna Zaks via cfe-commits]

I approve.Thanks Hans!
Anna
> On Aug 21, 2017, at 1:05 PM, Hans Wennborg <hans at chromium.org> wrote:
> 
> I'm ok with it if Anna approves.
> 
> On Mon, Aug 21, 2017 at 9:06 AM, Artem Dergachev <noqnoqneo at gmail.com> wrote:
>> Hello,
>> 
>> Do we have time to merge this change into release 5.0.0? It's an assertion
>> failure fix, which shows up on C++ code involving double-inheritance with
>> empty base classes.
>> 
>> Artem.
>> 
>> 
>> On 8/18/17 9:20 PM, Alexander Shaposhnikov via cfe-commits wrote:
>>> 
>>> 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;
>>> +  }
>>> +}
>>> 
>>> 
>>> _______________________________________________
>>> cfe-commits mailing list
>>> cfe-commits at lists.llvm.org
>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>> 
>>