<br><br><div class="gmail_quote">On Thu, Nov 6, 2008 at 9:24 AM, Zhongxing Xu <span dir="ltr"><<a href="mailto:xuzhongxing@gmail.com" target="_blank">xuzhongxing@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;">

<br><br><div class="gmail_quote"><div><div></div><div>On Thu, Nov 6, 2008 at 8:55 AM, Ted Kremenek <span dir="ltr"><<a href="mailto:kremenek@apple.com" target="_blank">kremenek@apple.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;">
<div><div></div><div><br>
On Nov 5, 2008, at 3:33 AM, Zhongxing Xu wrote:<br>
<br>
<blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;">
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.<br>



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



<br>
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.<br>
</blockquote>
<br></div></div>
Hi Zhongxing,<br>
<br>
I do not think we should introduce a new GRTransferFunc subclass to implement this functionality.  Here's why:<br>
<br>
(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:<br>



<br>
class StoreManager {<br>
...<br>
  virtual SVal getExtent(const GRState* state, const MemRegion* R) {<br>
    return UnknownVal();<br>
  }<br>
};<br>
<br>
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.<br>



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



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



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



<br>
Thoughts?<br><font color="#888888">
<br>
Ted</font></blockquote></div></div><div><br>Sounds reasonable! I remember that there was RegionExtent code. Maybe we should pick it up. <br><br>Do you think using the GDM to associate regions with their extents is the right approach?<br>


</div></div></blockquote><div><br>Adding an Extent field to MemRegion is simpler but not as versatile.<br></div></div><br>