I'm ok with it if Anna approves.
On Mon, Aug 21, 2017 at 9:06 AM, Artem Dergachev <noqnoq...@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@lists.llvm.org >> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits > > _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
