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

[Ted Kremenek]

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.  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.