[cfe-commits] [PATCH] Set region size in GRRegionVals transfer function

[Zhongxing Xu]

On Thu, Nov 6, 2008 at 9:24 AM, Zhongxing Xu <xuzhongxing at gmail.com> wrote:

>
>
> On Thu, Nov 6, 2008 at 8:55 AM, Ted Kremenek <kremenek at apple.com> wrote:
>
>>
>> On Nov 5, 2008, at 3:33 AM, Zhongxing Xu wrote:
>>
>>  The first step of checking out of bound array references is to get the
>>> size of a memory region. In this patch I store the size of MemRegions with
>>> the GDM facility of GRState. I put this data into a new transfer function
>>> called GRRegionVals. GRRegionVals is intended to be used with the
>>> RegionStore when we want to check some properties enabled by RegionStore,
>>> for example, out of bound array references and memory leak. For memory leaks
>>> we need another piece of GDM to store the state of heap regions.
>>>
>>> I can think of no where to put this piece of data in current components.
>>> Store only manages the mapping from region to value. Other components should
>>> not manage this region-store specific data. So I made a new transfer
>>> function for it.
>>>
>>> Later in this new GRRegionVals we can add an EvalLocation() method to
>>> check for out-of-bound array references and other region-store enabled
>>> memory checks.
>>>
>>
>> Hi Zhongxing,
>>
>> I do not think we should introduce a new GRTransferFunc subclass to
>> implement this functionality.  Here's why:
>>
>> (a) This functionality can easily go into StoreManager, and conceptually
>> its the right place for it.  StoreManager handles the abstraction of memory,
>> and the extent is a natural property of a region.  It seem to me that adding
>> the interface would be an easy first start:
>>
>> class StoreManager {
>> ...
>>  virtual SVal getExtent(const GRState* state, const MemRegion* R) {
>>    return UnknownVal();
>>  }
>> };
>>
>> If the StoreManager cares about tracking region sizes, it should have all
>> the information it needs in the GRState* object or in the region itself.  If
>> not all of the interfaces are there to do this, this is the direction I
>> think we should move to.
>>
>> (b) The second reason I don't think we should have a GRRegionVals is
>> because it partitions functionality into a separate GRTransferFuncs object
>> that really can be easily recycled elsewhere through the StoreManager
>> interface.
>>
>> (c) The GRTransferFuncs interface was designed to implement plugin
>> transfer function logic for *operations*, e.g. function calls.  This is used
>> to implement specific analyses/checkers.  Region sizes seem like a
>> fundamental property of regions that all analyses might be interested in.
>>
>> (c) The third reason is that the entire GRTransferFuncs interface needs to
>> be overhauled.  What's there now is a mongrel (that I created) that serves
>> different masters.  Much of it needs to be potentially lifted to
>> GRExprEngine, and in its place have a more modular design that allows
>> transfer functions for different checkers/analyses to be composed (e.g., one
>> analysis that tracks reference counts, another that tracks the state of a
>> window object, another that tracks lock states, etc.).  It's not there yet,
>> but sticking more core functionality into the GRTransferFunc is interface
>> doesn't seem ideal in the long run.
>>
>> Thoughts?
>>
>> Ted
>
>
> Sounds reasonable! I remember that there was RegionExtent code. Maybe we
> should pick it up.
>
> Do you think using the GDM to associate regions with their extents is the
> right approach?
>

Adding an Extent field to MemRegion is simpler but not as versatile.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20081106/a3205c6b/attachment.html>