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

[Zhongxing Xu]

On Thu, Nov 13, 2008 at 11:45 PM, Ted Kremenek <kremenek at apple.com> 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
>> cfe-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20081114/06bdd70b/attachment.html>