r311182 - [analyzer] Fix modeling of constructors
Artem Dergachev via cfe-commits
cfe-commits at lists.llvm.org
Mon Aug 21 09:06:31 PDT 2017
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
More information about the cfe-commits
mailing list