[cfe-dev] [Patch] add KillStruct() to RegionStore

[Ted Kremenek]

On Jan 12, 2009, at 5:13 PM, Zhongxing Xu wrote:

>
> On Tue, Jan 13, 2009 at 7:23 AM, Ted Kremenek <kremenek at apple.com>  
> wrote:
> This looks great!  Two comments:
>
> +const GRState* RegionStoreManager::KillStruct(const GRState* St,
> +                                              const TypedRegion* R){
> +  GRStateRef state(St, StateMgr);
> +
> +  // Kill the struct region because it is assigned "unknown".
> +  St = state.add<RegionKills>(R);
> +
> +  // Set the default value of the struct region to "unknown".
> +  St = state.set<RegionDefaultValue>(R, UnknownVal());
> +
> +  // Remove the struct region from the RegionViews. It could only  
> be a "view" of
> +  // its super region.
> +  St = RemoveRegionView(St, R, R->getSuperRegion());
>
> Hmm.  I'm looking at this again, I guess removing the view of 'R'  
> from its super region doesn't make sense.  It's just the values  
> bound to 'R' that change, not the fact that 'R' is a view of its  
> super region.
>
> Yes, that was my initial thought. :-)

Indeed it was.  ;-)

>
>
>
> +
> +  Store store = St->getStore();
> +  RegionBindingsTy B = GetRegionBindings(store);
> +
> +  // Remove all bindings for the subregions of the struct.
> +  for (RegionBindingsTy::iterator I = B.begin(), E = B.end(); I !=  
> E; ++I) {
> +    const MemRegion* r = I.getKey();
> +    if (const SubRegion* sr = dyn_cast<SubRegion>(r))
> +      if (sr->isSubRegionOf(R))
> +        store = Remove(store, Loc::MakeVal(sr));
>
> I am wondering if we also need to remove bindings for views of  
> 'sr'.  That is, iterate over the views of 'sr' and remove any  
> bindings from state.  What do you think?  I guess it depends on how  
> we decide to bind values in weird cases where people abuse the type  
> system.
>
> I am afraid the cost would be too high, and the logic be too complex  
> for an initial implementation. After all we are doing an approximate  
> simulation, not an exhaustive one. I suggest we leave a FIXME here  
> and fix it when we see a real bug caused by this simplification.

Makes sense.  Please take out the RemoveRegionView call (but go ahead  
and add the method itself) and commit the change.
>
>
>
> +  }
> +
> +  return StateMgr.MakeStateWithStore(St, store);
> +}
> +

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20090112/d6d4e2ba/attachment.html>