llvm dev - Aug 2018 - ScalarEvolution in a ModulePass

Hey LLVMDev,

I'm working on a ModulePass that uses ScalarEvolution along with several
other analyses.  After some debugging, it looks to me like
ScalarEvolutionWrapperPass does not handle memory correctly for this case.
Here's my current understanding of the problem.

ScalarEvolutionWrapperPass maintains a unique_ptr to a ScalarEvolution.
Calling getSE() dereferences this pointer.  Meanwhile runOnFunction()
resets the pointer to point to a new ScalarEvolution.  As a result,
runOnFunction destructs and frees any ScalarEvolution the unique_ptr
pointed to before.

The ModulePass I'm working on uses ScalarEvolution and several other
analysis FunctionPasses, including DominatorTree, LoopInfo,
OptimizationRemarkEmitter, and a custom analysis pass I'm working on, which
resembles LoopInfo.  To run ScalarEvolution and these other analysis
FunctionPasses in a ModulePass, the ModulePass creates lambdas to get the
analysis for a particular function, e.g.,

auto GetSE = [this](Function &F) -> ScalarEvolution & {
  return this->getAnalysis<ScalarEvolutionWrapperPass>(F).getSE();
};


Later, when the ModulePass examines a particular Function, it calls the
appropriate lambda to get the analysis for that Function.

The problem seems to arise when lambdas for other analysis FunctionPasses
run after calling the GetSE lambda, e.g., when evaluating a statement like
this:

for (Function &F : M)
  Changed |= MyPassImpl(F, GetSE(F), GetDT(F), GetLI(F), GetORE(F),
...).run();


It appears that these other analysis FunctionPasses can cause
ScalarEvolutionWrapperPass::runOnFunction() to rerun after GetSE() returns
a pointer to a ScalarEvolution, which changes the underlying
ScalarEvolution object that the wrapper pass points to.  As a result, the
pointer originally obtained from GetSE() points to invalid memory, and
chaos ensues.  Running valgrind on the "opt -mypass" indicates that
the
ScalarEvolution object used by MyPassImpl points to freed memory.

I've been able to work around this problem, but I figured I should raise
the issue and see if you might have more insights into this problem.  For
example, I'm not totally sure why the other analysis FunctionPasses cause
ScalarEvolutionWrapperPass::runOnFunction() to rerun.  I'm also concerned
about other analysis FunctionPasses, that might suffer from the same
problem.  MemorySSAWrapperPass, for example, uses a unique_ptr similarly to
ScalarEvolution.

Cheers,
TB
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20180811/8d58a7bb/attachment.html>

Hi, TB,

Just to be clear, are you using the new pass manager or the old one? You
can't do this from the old pass manager - this was one of the
motivations to create the new pass manager.

 -Hal


On 08/11/2018 09:16 AM, TB Schardl via llvm-dev wrote:
> Hey LLVMDev,
>
> I'm working on a ModulePass that uses ScalarEvolution along with
> several other analyses.  After some debugging, it looks to me like
> ScalarEvolutionWrapperPass does not handle memory correctly for this
> case.  Here's my current understanding of the problem.
>
> ScalarEvolutionWrapperPass maintains a unique_ptr to a
> ScalarEvolution.  Calling getSE() dereferences this pointer. 
> Meanwhile runOnFunction() resets the pointer to point to a new
> ScalarEvolution.  As a result, runOnFunction destructs and frees any
> ScalarEvolution the unique_ptr pointed to before.
>
> The ModulePass I'm working on uses ScalarEvolution and several other
> analysis FunctionPasses, including DominatorTree, LoopInfo,
> OptimizationRemarkEmitter, and a custom analysis pass I'm working on,
> which resembles LoopInfo.  To run ScalarEvolution and these other
> analysis FunctionPasses in a ModulePass, the ModulePass creates
> lambdas to get the analysis for a particular function, e.g.,
>
>     auto GetSE = [this](Function &F) -> ScalarEvolution & {
>       return
this->getAnalysis<ScalarEvolutionWrapperPass>(F).getSE();
>     };
>
>
> Later, when the ModulePass examines a particular Function, it calls
> the appropriate lambda to get the analysis for that Function.
>
> The problem seems to arise when lambdas for other analysis
> FunctionPasses run after calling the GetSE lambda, e.g., when
> evaluating a statement like this:
>
>     for (Function &F : M)
>       Changed |= MyPassImpl(F, GetSE(F), GetDT(F), GetLI(F),
>     GetORE(F), ...).run();
>
>
> It appears that these other analysis FunctionPasses can cause
> ScalarEvolutionWrapperPass::runOnFunction() to rerun after GetSE()
> returns a pointer to a ScalarEvolution, which changes the underlying
> ScalarEvolution object that the wrapper pass points to.  As a result,
> the pointer originally obtained from GetSE() points to invalid memory,
> and chaos ensues.  Running valgrind on the "opt -mypass"
indicates
> that the ScalarEvolution object used by MyPassImpl points to freed memory.
>
> I've been able to work around this problem, but I figured I should
> raise the issue and see if you might have more insights into this
> problem.  For example, I'm not totally sure why the other analysis
> FunctionPasses cause ScalarEvolutionWrapperPass::runOnFunction() to
> rerun.  I'm also concerned about other analysis FunctionPasses, that
> might suffer from the same problem.  MemorySSAWrapperPass, for
> example, uses a unique_ptr similarly to ScalarEvolution.
>
> Cheers,
> TB
>
>
> _______________________________________________
> LLVM Developers mailing list
> llvm-dev at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
-- 
Hal Finkel
Lead, Compiler Technology and Programming Languages
Leadership Computing Facility
Argonne National Laboratory

-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20180811/6fb39093/attachment.html>

Hi,

you are using the Legacy PassManager (which is the default). It
generally does not allow passes to use analysis for sub entities (e.g.
a FunctionPass cannot use BasicBlockPass analysis). An exception was
hardcoded for ModulePass to use FunctionPass analsyis, called
OnTheFlyManagers, but it will hold only one sub-pass at a time (per
type) and free the previous pass once you request a pass for a
different function. This is why you get the memory error if you
continue to use the previous analysis.
I once tried to get around this by invoking the analysis pass without
the pass manager. Unfortunately this is complex because the Pass class
needs an AnalysisResolver object that belongs to the PassManager,

However, there is a new pass manager that works differently. It just
stores all analysis for each entity until a pass invalidates it. You
might want to try that one. I suggest to look into the class
ScalarEvolutionAnalysis (the equivalent of ScalarEvolutionWrapperPass
for the new pass manager) and see how it is used.

Michael



Am Sa., 11. Aug. 2018 um 09:16 Uhr schrieb TB Schardl via llvm-dev
<llvm-dev at lists.llvm.org>:
>
> Hey LLVMDev,
>
> I'm working on a ModulePass that uses ScalarEvolution along with
several other analyses.  After some debugging, it looks to me like
ScalarEvolutionWrapperPass does not handle memory correctly for this case. 
Here's my current understanding of the problem.
>
> ScalarEvolutionWrapperPass maintains a unique_ptr to a ScalarEvolution. 
Calling getSE() dereferences this pointer.  Meanwhile runOnFunction() resets the
pointer to point to a new ScalarEvolution.  As a result, runOnFunction destructs
and frees any ScalarEvolution the unique_ptr pointed to before.
>
> The ModulePass I'm working on uses ScalarEvolution and several other
analysis FunctionPasses, including DominatorTree, LoopInfo,
OptimizationRemarkEmitter, and a custom analysis pass I'm working on, which
resembles LoopInfo.  To run ScalarEvolution and these other analysis
FunctionPasses in a ModulePass, the ModulePass creates lambdas to get the
analysis for a particular function, e.g.,
>
> auto GetSE = [this](Function &F) -> ScalarEvolution & {
>   return this->getAnalysis<ScalarEvolutionWrapperPass>(F).getSE();
> };
>
>
> Later, when the ModulePass examines a particular Function, it calls the
appropriate lambda to get the analysis for that Function.
>
> The problem seems to arise when lambdas for other analysis FunctionPasses
run after calling the GetSE lambda, e.g., when evaluating a statement like this:
>
> for (Function &F : M)
>   Changed |= MyPassImpl(F, GetSE(F), GetDT(F), GetLI(F), GetORE(F),
...).run();
>
>
> It appears that these other analysis FunctionPasses can cause
ScalarEvolutionWrapperPass::runOnFunction() to rerun after GetSE() returns a
pointer to a ScalarEvolution, which changes the underlying ScalarEvolution
object that the wrapper pass points to.  As a result, the pointer originally
obtained from GetSE() points to invalid memory, and chaos ensues.  Running
valgrind on the "opt -mypass" indicates that the ScalarEvolution
object used by MyPassImpl points to freed memory.
>
> I've been able to work around this problem, but I figured I should
raise the issue and see if you might have more insights into this problem.  For
example, I'm not totally sure why the other analysis FunctionPasses cause
ScalarEvolutionWrapperPass::runOnFunction() to rerun.  I'm also concerned
about other analysis FunctionPasses, that might suffer from the same problem. 
MemorySSAWrapperPass, for example, uses a unique_ptr similarly to
ScalarEvolution.
>
> Cheers,
> TB
> _______________________________________________
> LLVM Developers mailing list
> llvm-dev at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev

Hey Hal and Michael,

Thanks for the replies.

You're right, I was using the old pass manager.  I already wrote a separate
interface for my pass to the new pass manager, and that seems to work great.

I don't have as much experience using the new pass manager, and I
haven't
tried running it by default.  (For reference, I'm doing most of my
development based on LLVM 6.0 release at the moment.)  I noticed some talk
from just under a year ago about making the new pass manager the default.
What's the status of that work?  Do the sanitizers work with the new pass
manager?

Cheers,
TB

On Sat, Aug 11, 2018 at 1:42 PM Michael Kruse <llvmdev at meinersbur.de>
wrote:
> Hi,
>
> you are using the Legacy PassManager (which is the default). It
> generally does not allow passes to use analysis for sub entities (e.g.
> a FunctionPass cannot use BasicBlockPass analysis). An exception was
> hardcoded for ModulePass to use FunctionPass analsyis, called
> OnTheFlyManagers, but it will hold only one sub-pass at a time (per
> type) and free the previous pass once you request a pass for a
> different function. This is why you get the memory error if you
> continue to use the previous analysis.
> I once tried to get around this by invoking the analysis pass without
> the pass manager. Unfortunately this is complex because the Pass class
> needs an AnalysisResolver object that belongs to the PassManager,
>
> However, there is a new pass manager that works differently. It just
> stores all analysis for each entity until a pass invalidates it. You
> might want to try that one. I suggest to look into the class
> ScalarEvolutionAnalysis (the equivalent of ScalarEvolutionWrapperPass
> for the new pass manager) and see how it is used.
>
> Michael
>
>
>
> Am Sa., 11. Aug. 2018 um 09:16 Uhr schrieb TB Schardl via llvm-dev
> <llvm-dev at lists.llvm.org>:
> >
> > Hey LLVMDev,
> >
> > I'm working on a ModulePass that uses ScalarEvolution along with
several
> other analyses.  After some debugging, it looks to me like
> ScalarEvolutionWrapperPass does not handle memory correctly for this case.
> Here's my current understanding of the problem.
> >
> > ScalarEvolutionWrapperPass maintains a unique_ptr to a
ScalarEvolution.
> Calling getSE() dereferences this pointer.  Meanwhile runOnFunction()
> resets the pointer to point to a new ScalarEvolution.  As a result,
> runOnFunction destructs and frees any ScalarEvolution the unique_ptr
> pointed to before.
> >
> > The ModulePass I'm working on uses ScalarEvolution and several
other
> analysis FunctionPasses, including DominatorTree, LoopInfo,
> OptimizationRemarkEmitter, and a custom analysis pass I'm working on,
which
> resembles LoopInfo.  To run ScalarEvolution and these other analysis
> FunctionPasses in a ModulePass, the ModulePass creates lambdas to get the
> analysis for a particular function, e.g.,
> >
> > auto GetSE = [this](Function &F) -> ScalarEvolution & {
> >   return
this->getAnalysis<ScalarEvolutionWrapperPass>(F).getSE();
> > };
> >
> >
> > Later, when the ModulePass examines a particular Function, it calls
the
> appropriate lambda to get the analysis for that Function.
> >
> > The problem seems to arise when lambdas for other analysis
> FunctionPasses run after calling the GetSE lambda, e.g., when evaluating a
> statement like this:
> >
> > for (Function &F : M)
> >   Changed |= MyPassImpl(F, GetSE(F), GetDT(F), GetLI(F), GetORE(F),
> ...).run();
> >
> >
> > It appears that these other analysis FunctionPasses can cause
> ScalarEvolutionWrapperPass::runOnFunction() to rerun after GetSE() returns
> a pointer to a ScalarEvolution, which changes the underlying
> ScalarEvolution object that the wrapper pass points to.  As a result, the
> pointer originally obtained from GetSE() points to invalid memory, and
> chaos ensues.  Running valgrind on the "opt -mypass" indicates
that the
> ScalarEvolution object used by MyPassImpl points to freed memory.
> >
> > I've been able to work around this problem, but I figured I should
raise
> the issue and see if you might have more insights into this problem.  For
> example, I'm not totally sure why the other analysis FunctionPasses
cause
> ScalarEvolutionWrapperPass::runOnFunction() to rerun.  I'm also
concerned
> about other analysis FunctionPasses, that might suffer from the same
> problem.  MemorySSAWrapperPass, for example, uses a unique_ptr similarly to
> ScalarEvolution.
> >
> > Cheers,
> > TB
> > _______________________________________________
> > LLVM Developers mailing list
> > llvm-dev at lists.llvm.org
> > http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20180811/570a2ed8/attachment.html>