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

[Török Edwin]

On 2009-06-29 23:46, Dan Gohman wrote:
> 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.
>   

Makes sense, I'll set it to false.

>   
>>     
>>> +  // 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.
>   

The documentation isn't very clear on that unfortunately.
What if doFinalization deletes a map that run() and releaseMemory() are
using, and releaseMemory only .clear()s it?


Best regards,
--Edwin