r189590 - [analyzer] Fix handling of "empty" structs with base classes

[Richard Smith]

On Thu, Aug 29, 2013 at 9:06 AM, Pavel Labath <labath at google.com> wrote:

> Author: labath
> Date: Thu Aug 29 11:06:04 2013
> New Revision: 189590
>
> URL: http://llvm.org/viewvc/llvm-project?rev=189590&view=rev
> Log:
> [analyzer] Fix handling of "empty" structs with base classes
>
> Summary:
> RegionStoreManager had an optimization which replaces references to empty
> structs with UnknownVal. Unfortunately, this check didn't take into account
> possible field members in base classes.
>
> To address this, I changed this test to "is empty and has no base
> classes". I
> don't consider it worth the trouble to go through base classes and check
> if all
> of them are empty.
>

You could use CXXRecordDecl::isEmpty() (which also checks for virtual
functions/virtual bases, and handles the case where the only fields are
zero-length bitfields).


> Reviewers: jordan_rose
>
> CC: cfe-commits
>
> Differential Revision: http://llvm-reviews.chandlerc.com/D1547
>
> Modified:
>     cfe/trunk/lib/StaticAnalyzer/Core/RegionStore.cpp
>     cfe/trunk/test/Analysis/array-struct-region.cpp
>
> Modified: cfe/trunk/lib/StaticAnalyzer/Core/RegionStore.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/RegionStore.cpp?rev=189590&r1=189589&r2=189590&view=diff
>
> ==============================================================================
> --- cfe/trunk/lib/StaticAnalyzer/Core/RegionStore.cpp (original)
> +++ cfe/trunk/lib/StaticAnalyzer/Core/RegionStore.cpp Thu Aug 29 11:06:04
> 2013
> @@ -1832,10 +1832,18 @@ NonLoc RegionStoreManager::createLazyBin
>    return svalBuilder.makeLazyCompoundVal(StoreRef(B.asStore(), *this), R);
>  }
>
> +static bool isRecordEmpty(const RecordDecl *RD) {
> +  if (!RD->field_empty())
> +    return false;
> +  if (const CXXRecordDecl *CRD = dyn_cast<CXXRecordDecl>(RD))
> +    return CRD->getNumBases() == 0;
> +  return true;
> +}
> +
>  SVal RegionStoreManager::getBindingForStruct(RegionBindingsConstRef B,
>                                               const TypedValueRegion *R) {
>    const RecordDecl *RD =
> R->getValueType()->castAs<RecordType>()->getDecl();
> -  if (RD->field_empty())
> +  if (isRecordEmpty(RD))
>      return UnknownVal();
>
>    return createLazyBinding(B, R);
>
> Modified: cfe/trunk/test/Analysis/array-struct-region.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/array-struct-region.cpp?rev=189590&r1=189589&r2=189590&view=diff
>
> ==============================================================================
> --- cfe/trunk/test/Analysis/array-struct-region.cpp (original)
> +++ cfe/trunk/test/Analysis/array-struct-region.cpp Thu Aug 29 11:06:04
> 2013
> @@ -173,4 +173,27 @@ void testImmediateUseOp() {
>    clang_analyzer_eval(getConstrainedFieldRefOp(getS()) == 42); //
> expected-warning{{TRUE}}
>  }
>
> +namespace EmptyClass {
> +  struct Base {
> +    int& x;
> +
> +    Base(int& x) : x(x) {}
> +  };
> +
> +  struct Derived : public Base {
> +    Derived(int& x) : Base(x) {}
> +
> +    void operator=(int a) { x = a; }
> +  };
> +
> +  Derived ref(int& a) { return Derived(a); }
> +
> +  // There used to be a warning here, because analyzer treated Derived as
> empty.
> +  int test() {
> +    int a;
> +    ref(a) = 42;
> +    return a; // no warning
> +  }
> +}
> +
>  #endif
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130830/8093eb14/attachment.html>