[cfe-commits] r59232 - /cfe/trunk/lib/Analysis/GRExprEngine.cpp

[Ted Kremenek]

First, this patch looks great!  Please apply.  Comments on your other  
points below.

On Nov 15, 2008, at 1:36 AM, Zhongxing Xu wrote:

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

That's correct.  The back-mapping from super regions to sub-regions is  
not something enforced by the region model.  It's up to a particular  
StoreManager to maintain this information if it so desires.

> 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:
>
> typedef llvm::ImmutableList<MemRegion*> RegionViewTy;
> typedef llvm::ImmutableMap<MemRegion*, RegionViewTy> RegionViewMapTy;
>
> 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.

These seem great to me, but they are a detail of how a particular  
StoreManager implements Stores.  These should probably be buried  
within StoreManager.  We can either use the GDM to host these or have  
RegionStore manage a foldingset for its own particular store objects  
(where each store object has a RegionView, a mapping from vardecls to  
regions, etc.).

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

We should enhance ImmutableList to not have this limitation.