[llvm-commits] [PATCH] Avoid use after free in ScalarEvolution

[Dan Gohman]

On Jun 29, 2009, at 11:22 AM, Török Edwin wrote:


> On 2009-06-29 21:15, Dan Gohman wrote:
>
>>> +  if (!wasRun)
>>>
>>> +    return;
>>>
>>> +  for (unsigned Index = 0; Index < getNumContainedManagers(); +
>>>
>> +Index) {
>>
>>> +    FPPassManager *FPPM = getContainedManager(Index);
>>>
>>> +    for (unsigned Index = 0; Index < FPPM->getNumContainedPasses
>>>
>> (); ++Index) {
>>
>>> +      FPPM->getContainedPass(Index)->releaseMemory();
>>>
>>> +    }
>>>
>>> +  }
>>>
>>> +}
>>>
>>
>>
>> Should this set wasRun to true after freeing all the memory?
>>
>>
>>
>
> wasRun is already true, otherwise we would have returned early.
> wasRun only guards against calling releaseMemory() before the first
> run() call.

Oops. I meant to ask if wasRun should be set to false afterwards,
to indicate that all of the state has been reset. Even if not
necessary for correctness, this may be a nice invariant.

>
>
>> +  // Finalize on-the-fly passes
>>
>> +  for (std::map<Pass *, FunctionPassManagerImpl *>::iterator
>>
>> +       I = OnTheFlyManagers.begin(), E = OnTheFlyManagers.end();
>>
>> +       I != E; ++I) {
>>
>> +    FunctionPassManagerImpl *FPP = I->second;
>>
>> +    // We don't know when is the last time an on-the-fly pass is  
>> run,
>>
>> +    // so we need to releaseMemory / finalize here
>>
>> +    FPP->releaseMemoryOnTheFly();
>>
>> +    Changed |= FPP->doFinalization(M);
>>
>> +  }
>>
>>
>>
>> Is it correct to call releaseMemoryOnTheFly before calling
>>
>> doInitialization? It seems like it should be the other way
>>
>> around.
>>
>>
>>
>
> No, you can't call it before doInitialization.
> releaseMemoryOnTheFly is called before doFinalization, that looks  
> right
> to me.
> doInitialization is called above, always before releaseMemory.
> There is one situation where doInitialization is called, but run()
> isn't, then releaseMemoryOnTheFly then doFinalization.
> But that should be OK too, since then wasRun will be false, and
> releaseMemory won't be called.

Oops again. Here, I meant to ask if releaseMemoryOnTheFly should
be called after doFinialization. It seems that a pass' doFinalization
may want to have access to the pass' state before it gets freed.

Dan