[cfe-commits] r74525 - in /cfe/trunk/lib/Analysis: Environment.cpp LiveVariables.cpp

[Zhongxing Xu]

On Wed, Jul 1, 2009 at 7:59 AM, Ted Kremenek<kremenek at apple.com> wrote:
> Hi Zhongxing,
>
> I thought about this a little more.  I'm trying to understand why this
> should be in Environment.cpp.  Doesn't RegionStoreManager scan the super
> regions of all regions::RemoveDeadBindings in DRoots?  I'm not certain, but
> I'm wondering if the issue (and the right fix) is there instead of in
> Environment::RemoveDeadBindings?

Initially I thought as you did. I added the code in RegionStore.cpp.
But that didn't work.
Look at the CFG:

b1 int x;
b2 (char*) &x
b3 int y = b2

the element region &x[0] is the binding of the block level expression
'(char*) &x'. Block level expression liveness is handled in
Environment.

>
> On Jun 30, 2009, at 12:31 PM, Ted Kremenek <kremenek at apple.com> wrote:
>
>>>     if (isa<loc::MemRegionVal>(X))
>>> -        DRoots.push_back(cast<loc::MemRegionVal>(X).getRegion());
>>> +      if (isa<loc::MemRegionVal>(X)) {
>>> +        const MemRegion* R = cast<loc::MemRegionVal>(X).getRegion();
>>> +        DRoots.push_back(R);
>>> +        // Mark the super region of the RX as live.
>>> +        // e.g.: int x; char *y = (char*) &x; if (*y) ...
>>> +        // 'y' => element region. 'x' is its super region.
>>> +        // We only add one level super region for now.
>>
>> Is there a reason we only add one level of super region?  If we need
>> to add the entire region hierarchy, I'd rather do it now before we
>> forget about it.   A recursive function should do the trick.  At the
>> very least there should be a FIXME comment if this is something that
>> needs to be fixed.
>>
>>> +        if (const SubRegion *SR = dyn_cast<SubRegion>(R)) {
>>> +          DRoots.push_back(SR->getSuperRegion());
>>> +        }
>>> +      }
>