[cfe-commits] [PATCH] RegionStore

[Zhongxing Xu]

On Mon, Oct 20, 2008 at 12:51 PM, Zhongxing Xu <xuzhongxing at gmail.com>wrote:

>
>
> On Mon, Oct 20, 2008 at 12:48 PM, Zhongxing Xu <xuzhongxing at gmail.com>wrote:
>
>>
>>
>> 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.
>>
>
> It's actually a special "load" not going through the binding.
>

As a more direct example, consider code

int a[10];
a[1] = 3;

When we visit a[1], we evaluate a's rvalue. In VisitDeclRefExpr, we first
get a's lvalue loc::MemRegionVal(a's region), then do a load on it. We
expect to load loc::MemRegionVal(a's region) from itself. Then a
ImplicitCast will convert it to a pointer to a's first element.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20081020/1fa07739/attachment.html>