[cfe-commits] r59382 - in /cfe/trunk: include/clang/Analysis/PathSensitive/MemRegion.h include/clang/Analysis/PathSensitive/Store.h lib/Analysis/BasicStore.cpp lib/Analysis/GRExprEngine.cpp lib/Analysis/MemRegion.cpp lib/Analysis/RegionStore.cpp

[Ted Kremenek]

On Nov 15, 2008, at 8:07 PM, Zhongxing Xu wrote:

> +const GRState* RegionStoreManager::CastRegion(const GRState* St,
> +                                              SVal VoidPtr,
> +                                              QualType CastToTy,
> +                                              Stmt* CastE) {
> +  if (const AllocaRegion* AR =
> +       
> dyn_cast 
> <AllocaRegion>(cast<loc::MemRegionVal>(VoidPtr).getRegion())) {
> +
> +    // Create a new region to attach type information to it.
> +    const AnonTypedRegion* TR = MRMgr.getAnonTypedRegion(CastToTy,  
> AR);
> +
> +    // Get the pointer to the first element.
> +    nonloc::ConcreteInt  
> Idx(getBasicVals().getZeroWithPtrWidth(false));
> +    const ElementRegion* ER = MRMgr.getElementRegion(Idx, TR);
> +
> +    St = StateMgr.BindExpr(St, CastE, loc::MemRegionVal(ER));
> +

I'm thinking that we should not assume that CastRegion will be used  
only in EvalCast.  More specifically, StoreManager should never modify  
the environment (i.e., call BindExpr), since that violates the  
encapsulation between StoreManager and the rest of GRState.

A simple solution is return a std::pair of the new state and the new  
region, and have GRExprEngine do the actual call to BindExpr.