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

[Ted Kremenek]

Hi Zhongxing,

This is a little heavy handed, but I'm actually going to revert this  
patch along with:

http://lists.cs.uiuc.edu/pipermail/cfe-commits/Week-of-Mon-20081110/009157.html

There is a reason why MemRegion* have a const qualifier; they never  
can be changed in any way.  The const_cast itself is an indicator to  
me that this is the wrong design.  It violates the functional  
programming design of how we track state, and modifying the MemRegion  
object in this way can introduce subtle bugs.

We'll need to iterate on this one.  We may end up applying your patch  
back, so please don't take my reverting it personally.

Consider:

char* p = alloca(BLOCK);
new (p) Object1();
...
new (p) Object2();

Untyped memory can be recycled.  While this won't  occur that often, I  
think with the right design we can handle such things naturally.

Ted

On Nov 12, 2008, at 11:58 PM, Zhongxing Xu wrote:

> Author: zhongxingxu
> Date: Thu Nov 13 01:58:20 2008
> New Revision: 59232
>
> URL: http://llvm.org/viewvc/llvm-project?rev=59232&view=rev
> Log:
> Lift the pointer to alloca'ed region to the pointer to its first  
> element.
> This is required by some operations, e.g., *p = 1; p[0] = 1;.
> Also set the AllocaRegion's type during the cast.
>
> Modified:
>    cfe/trunk/lib/Analysis/GRExprEngine.cpp
>
> Modified: cfe/trunk/lib/Analysis/GRExprEngine.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/GRExprEngine.cpp?rev=59232&r1=59231&r2=59232&view=diff
>
> = 
> = 
> = 
> = 
> = 
> = 
> = 
> = 
> ======================================================================
> --- cfe/trunk/lib/Analysis/GRExprEngine.cpp (original)
> +++ cfe/trunk/lib/Analysis/GRExprEngine.cpp Thu Nov 13 01:58:20 2008
> @@ -1277,7 +1277,7 @@
>             // FIXME: Refactor into StoreManager itself?
>             MemRegionManager& RM =  
> getStateManager().getRegionManager();
>             const MemRegion* R =
> -              RM.getAllocaRegion(CE, Builder- 
> >getCurrentBlockCount());
> +              RM.getAllocaRegion(CE, Builder- 
> >getCurrentBlockCount());
>             MakeNode(Dst, CE, *DI, BindExpr(St, CE,  
> loc::MemRegionVal(R)));
>             continue;
>           }
> @@ -1681,6 +1681,26 @@
>       continue;
>     }
>
> +    // Cast alloca'ed pointer to typed pointer.
> +    if (isa<loc::MemRegionVal>(V)) {
> +      if (const AllocaRegion* AR =
> +           
> dyn_cast<AllocaRegion>(cast<loc::MemRegionVal>(V).getRegion())) {
> +
> +        // Set the AllocaRegion's type.
> +        const_cast<AllocaRegion*>(AR)->setType(T);
> +
> +        // Set the CastExpr's value to a pointer to the first  
> element.
> +        MemRegionManager& RM = getStateManager().getRegionManager();
> +
> +        llvm::APSInt Zero(llvm::APInt::getNullValue(32), false);
> +        SVal  
> ZeroIdx(nonloc::ConcreteInt(getBasicVals().getValue(Zero)));
> +        const ElementRegion* ER = RM.getElementRegion(ZeroIdx, AR);
> +
> +        MakeNode(Dst, CastE, N, BindExpr(St, CastE,  
> loc::MemRegionVal(ER)));
> +        continue;
> +      }
> +    }
> +
>     // All other cases.
>     MakeNode(Dst, CastE, N, BindExpr(St, CastE, EvalCast(V, CastE- 
> >getType())));
>   }
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits