[llvm] r250692 - [GlobalsAA] Fix a really horrible iterator invalidation bug

Yaron Keren via llvm-commits llvm-commits at lists.llvm.org
Mon Oct 19 02:40:25 PDT 2015


I had not seen any assert failing (build release with assertions) in my use
case.


2015-10-19 12:34 GMT+03:00 Sanjoy Das via llvm-commits <
llvm-commits at lists.llvm.org>:

> Did this bug trip manage to trip one of the EpochTracker related
> asserts?  EpochTracker is supposed to eagerly catch issues like these
> without actually needing the DenseMap resize.
>
> -- Sanjoy
>
> On Mon, Oct 19, 2015 at 1:55 AM, James Molloy via llvm-commits
> <llvm-commits at lists.llvm.org> wrote:
> > Author: jamesm
> > Date: Mon Oct 19 03:54:59 2015
> > New Revision: 250692
> >
> > URL: http://llvm.org/viewvc/llvm-project?rev=250692&view=rev
> > Log:
> > [GlobalsAA] Fix a really horrible iterator invalidation bug
> >
> > We were keeping a reference to an object in a DenseMap then mutating it.
> At the end of the function we were attempting to clone that reference into
> other keys in the DenseMap, but DenseMap may well decide to resize its
> hashtable which would invalidate the reference!
> >
> > It took an extremely complex testcase to catch this - many thanks to
> Zhendong Su for catching it in PR25225.
> >
> > This fixes PR25225.
> >
> > Modified:
> >     llvm/trunk/lib/Analysis/GlobalsModRef.cpp
> >
> > Modified: llvm/trunk/lib/Analysis/GlobalsModRef.cpp
> > URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Analysis/GlobalsModRef.cpp?rev=250692&r1=250691&r2=250692&view=diff
> >
> ==============================================================================
> > --- llvm/trunk/lib/Analysis/GlobalsModRef.cpp (original)
> > +++ llvm/trunk/lib/Analysis/GlobalsModRef.cpp Mon Oct 19 03:54:59 2015
> > @@ -587,8 +587,11 @@ void GlobalsAAResult::AnalyzeCallGraph(C
> >
> >      // Finally, now that we know the full effect on this SCC, clone the
> >      // information to each function in the SCC.
> > +    // FI is a reference into FunctionInfos, so copy it now so that it
> doesn't
> > +    // get invalidated if DenseMap decides to re-hash.
> > +    FunctionInfo CachedFI = FI;
> >      for (unsigned i = 1, e = SCC.size(); i != e; ++i)
> > -      FunctionInfos[SCC[i]->getFunction()] = FI;
> > +      FunctionInfos[SCC[i]->getFunction()] = CachedFI;
> >    }
> >  }
> >
> >
> >
> > _______________________________________________
> > llvm-commits mailing list
> > llvm-commits at lists.llvm.org
> > http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>
>
>
> --
> Sanjoy Das
> http://playingwithpointers.com
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20151019/d8c5407a/attachment.html>


More information about the llvm-commits mailing list