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

[Zhongxing Xu]

On Fri, Nov 7, 2008 at 11:46 AM, Ted Kremenek <kremenek at apple.com> wrote:

>
> On Nov 6, 2008, at 7:06 PM, Zhongxing Xu wrote:
>
>  The nice thing about this approach is that it piggybacks on NonLoc.  This
>> means it can represent extents whose size is an integer constant, a symbol,
>> an expression (e.g., $1 + 2), unknown extents, etc.
>>
>> I like this approach. It is similar to my original thought: an Extent is
>> just a NonLoc. BTW, what is ElementSizeBits for? Does it describe the size
>> of element of array or the number of elements in an array?
>>
>
> It is the size, in bits, of one element.  This would be used by
> getElementsSize() (which maybe should be named getSizeInElements(); same for
> getSizeInBytes(), getSizeInBits()).
>
>  The extent for a variable is obvious while the extent for a malloced
>> region is not. We should make it clear whether we want all the extent data
>> be maintained by the GDM. If so, whether we do the lazy mapping or eager
>> mapping.
>>
>
> I think we can take a hybrid approach; ultimately it is up to an
> implementation of StoreManager, since it knows what extents it wants to
> manage/reason about.  Whether or not we even use the GDM is up to a
> particular implementation of StoreManager.
>
> A VarRegion has an obvious extent that a StoreManager can return without
> have to store anything explicitly in a map.  A malloced region would need to
> have the StoreManager be informed of the extent, either through setExtent,
> or when the region is created.
>
>  Calling setExtent() after we create a new region everytime is the eager
>> approach. Setting the extent after the first time we query for it is the
>> lazy approach.
>> But there is a problem with the lazy approach. Note that for a getExtent()
>> we not only return the extent but also need to return a new GRState, because
>> we might add a new GDM mapping for the extent.
>>
>
> My thought was that the lazy approach would only apply when the extent is
> obvious.  That is, there is enough information, embedded in the region
> itself, to not have to explicitly record it in a side map.  We only need a
> side map for regions whose extents are not so obvious.
>
> By lazy, we're just talking about whether or not the extent is in a side
> map, but that really is an implementation detail of how a StoreManager
> object retrieves the extent for a region.  Consider a few scenarios:
>
> (a) We want the extent for a VarRegion.  No side information needed.  Just
> use (within StoreManager::getExtent) the ASTContext object to determine the
> size of the variable for the VarRegion.  The same goes for most of the Typed
> Regions.
>
> (b) We create a new region to represent some piece of dynamically allocated
> memory.  We can:
>
> (1) use setExtent(), after we create the region, to explicitly register the
> extent of the region in GRState.  This seems fine, as this would just get
> bundled up for the transfer function logic for malloc(), alloca(), etc.
>
> (2) The literal act of creating the region also returns a new GRState
> object with the extent in the GRState (if it is needed).
>
> I personally like (b1), since (b2) requires a client to reason about both
> using  new GRState object and the extent.  Using GRStateRef simplifies
> things, but I can see this being the source of subtle bugs.
>
> The nice thing about (b1) is that it transfers some of the responsibility
> of reasoning about extents to clients who know about functions that create
> blocks of memory with extents.  There are a ton of different allocator
> functions, etc., that can conceptually create regions with extents.  No
> StoreManager should know about these functions.  So, for cases where an
> analysis/checker has specific knowledge about the extent of a region, it can
> inform StoreManager about it using setExtent().
>
> I actually don't think a hybrid approach is not all that difficult to
> implement.


 Do you mean " ... don't think ... is all that difficult ...", since two
negation imply no negation.


> A switch statement on the kind of a region should be enough to determine
> when we can determine the extent of a region simply from the MemRegion
> object itself or we need to look in a side map.
>

I like (b1), too. Then we have a clear direction to go now. Thanks!
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20081107/5fd55e8c/attachment.html>