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

[Török Edwin]

On 2009-06-29 21:15, Dan Gohman wrote:
> +    return false;
>
> Please use LQ.empty() here instead of !LQ.size().
>   

Fixed.

>  > +void FunctionPassManagerImpl::releaseMemoryOnTheFly()
>  > +{
>
> LLVM style has the brace on the same line as the function name.
>   

Fixed.

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

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

Best regards,
--Edwin