<br><br><div class="gmail_quote">On Sat, Nov 15, 2008 at 1:49 AM, Ted Kremenek <span dir="ltr"><<a href="mailto:kremenek@apple.com">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 class="Ih2E3d"><br>
On Nov 14, 2008, at 3:46 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;">
Hi Ted,<br>
<br>
I rewrite the code with an AnonTypedRegion created to attach type information to. Please review it.<br>
<br>
</blockquote>
<br></div>
This looks pretty good so far!  First I'll mentioned a couple thoughts, then a few specific comments on the code.<br>
<br>
Thoughts:<br>
<br>
(1) We might want GRExprEngine::VisitCast to do the delegation of pointer -> pointer casts to StoreManager.  I'm not certain if this is the right approach, but it would give more flexibility to the particular implementation of StoreManager of how to handle such things.  Basically:<div class="Ih2E3d">
<br>
<br>
if (isa<loc::MemRegionVal>(V)) {<br></div>
  assert (Loc::isLocType(CastE->getType());<br>
  assert (Loc::isLocType(Ex->getType());<br>
  // Delegate to store manager.<br>
  SVal VNew = StateManager.getStoreManager().CastRegion(CastE->getType()->getCanonicalType(), ..., V);<br>
  // update the state, etc.<br>
}<br>
<br>
Here "CastRegion" would do the actual region conversion, creating AnonTypedRegionm etc.  The advantage of this is that it allows different StoreManagers to handle arbitrary region to region casts differently and removes these details from GRExprEngine.<br>

<br>
(2) The conversion from AllocaRegion to an AnonTypedRegion probably applies to all regions that are not typed, not just AllocaRegion.<br>
<br>
(3) The interesting issues really come into play in how we query things in the store.  For example, how do we handle games like:<br>
<br>
  void* p = alloca(...);<br>
  char* q = (char*) p;<br>
  p = (void*) q;<br>
<br>
On the last line we should probably get back the original region returned from calling "alloca".  This is important if you consider a slightly different example:<br>
<br>
  void* p = alloca(...);<br>
  char *q = (char*) p;<br>
  void* r = (void*) q;<br>
  assert (r == p && "r and q should be the same!");<br>
<br>
Basically, my original concept of AnonTypedRegion was to layer some context on to an existing region, and then strip it away when it no longer applies.  This is really the design point we need to iterte on.  Is this the right approach?  etc.<br>

<br>
Also consider:<br>
<br>
  void* p = alloca(...);<br>
  ...<br>
  char *q = (char*) p;<br>
  *q = 'c';<br>
  ...<br>
  double *d = (double*) p;<br>
  *d = 1.0;<br>
  ...<br>
  char ch = *q;  // we should be able to flag this as an error, since that chunk of memory now binds to 'd'<br>
<br>
Another horrible case:<br>
<br>
  void* p = alloca(...);<br>
    ...<br>
  char *q = (char*) p;<br>
  *q = 'c';<br>
  ...<br>
  double *d = ((double*) p) + 1;<br>
  *d = 1.0;<br>
  ...<br>
  char ch = *q;  // whether or not this is an error is subjective.  It also gets down the point of what is the "extent" of AnonTypedRegion.<br>
<br>
I don't think we need to solve these problems all at once, but I think it's important to keep these design points in mind.  I'd like to converge to a clean and flexible design that allows us to model such things if we should so choose (as opposed to not allowing us to model such things at all).<br>

</blockquote><div><br>These quite complex cases are introduced by the C language property
that allows us to have different views on the same chunk of raw memory.
We intend to tackle these cases by adding another layer on top of a raw
memory region, i.e. the AnonTypedRegion. But it is not enough to know
what the view of a chunk of raw memory is in the current state. To flag
the above error, we also need to know besides the current view of the
memory, what other views of the memory exist? But from an
AnonTypedRegion we can only get its super region, but no other
AnonTypedRegion derived from that super region.<br>

<br>To get this knowledge, I propose to add a RegionView and a
RegionViewMap information. A RegionView is currently designed to be a
ImmutableList of MemRegions, which collects the current view on a
memory region. The RegionViewMap maps a region to its view. In code:<br>

<br>typedef llvm::ImmutableList<MemRegion*> RegionViewTy;<br>typedef llvm::ImmutableMap<MemRegion*, RegionViewTy> RegionViewMapTy;<br><br>Later
if necessary we can extend the RegionView into a real class to hold
other information. With RegionViewMap, we can get all of the views of a
memory region.<br><br>One side problem is that I cannot make the MemRegion* in the ImmutableList const. If I did that, I would get compile error, related to FoldingSet Profile() overloading. <br></div></div><br>