[cfe-commits] [PATCH] RegionStore

[Zhongxing Xu]

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.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20081020/dca41fcb/attachment.html>