[cfe-commits] [PATCH] RegionStore

[Zhongxing Xu]

On Mon, Oct 20, 2008 at 12:28 PM, Ted Kremenek <kremenek at apple.com> wrote:

>
> On Oct 19, 2008, at 6:24 AM, Zhongxing Xu wrote:
>
>  1. Initial support for local arrays in RegionStoreManager::AddDecl()
>> 2. Move special processing of loading array variable's lvalue from
>> GRExprEngine to Store. This simplifies code. And this is also what we have
>> to do when we processing code like:
>>
>
> Hi Zhongxing,
>
> The following down cast is not necessary, as R is a VarRegion:


Fixed.


>
>
> +++ lib/Analysis/BasicStore.cpp （工作副本）
> @@ -42,7 +42,6 @@
>
>   virtual MemRegionManager& getRegionManager() { return MRMgr; }
>
> -  // FIXME: Investigate what is using this. This method should be removed.
>   virtual Loc getLoc(const VarDecl* VD) {
>     return loc::MemRegionVal(MRMgr.getVarRegion(VD));
>   }
> @@ -148,7 +147,15 @@
>
>       if (!R)
>         return UnknownVal();
> -
> +
> +      // We let an array variable's rvalue be its lvalue, because we have
> to
> +      // process code like: int a[10]; int (*p)[10]; p = &a; (*p)[3] = 1;
> The
> +      // other benifit is that we can save the special handling of array
> +      // variables in VisitDeclRefExpr().
> +      if (const VarRegion* VR = dyn_cast<VarRegion>(R))
> +        if (VR->getDecl()->getType()->isArrayType())
> +          return LV;
> +
>
> I'm also not certain why this logic is needed (or if it is correct).  The
> expression '&a' is handled by VisitUnaryOperator, and for the case of '&' it
> calls VisitLValue on its subexpression.  I don't see where a call to GetSVal
> is involved.
>
> The cleanups to GRExprEngine are very nice.
>
> All in all the patch looks good, but I am a little unclear about why
> BasicStore::GetSVal should return the lvalue for an array in an rvalue
> context.  GRExprEngine handles the implicit conversions between lvalues and
> rvalues, not the store.
>
> When we visit (*p)[3] = 1, we evaluate (*p)'s rvalue. We will do a load on
p's rvalue, which is loc::MemRegionVal(region of a). We want (*p)'s rvalue
still be loc::MemRegionVal(region of a). So we should handle this in Store.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20081020/951e9786/attachment.html>