On Thu, Nov 13, 2008 at 11:45 PM, Ted Kremenek <[EMAIL PROTECTED]> wrote:
> 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. Please revert these two patches. Sorry that I forgot some fundamental points. They were quick fixes to some immediate crashes when I test the analyzer. We should get better designs. > > > 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 >> [email protected] >> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits >> > >
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
