Re: [patch] avoid ICE due to NULL pointer dereference in ipa-comdats.c

[Jeff Law <law at redhat dot com>]

On 09/19/14 08:11, Sebastian Pop wrote:
> Jeff Law wrote:
> > On 09/17/14 13:59, Sebastian Pop wrote:
> > > Hi,
> > >
> > > I got an ICE while building libstdc++ of a cross compiler x86 to aarch64.  I
> > > have a testcase that ICEs on current GCC trunk.  I was trying to painfully
> > > reduce it with creduce, and it is still several thousand lines of c++. Frustrated
> > > that it does not reduce anymore, I decided to have a look with gdb at why the
> > > compiler was iceing: the code dereferences a NULL pointer that we get by looking
> > > up the value of a symbol in a map.  Around that place, there is another pattern
> > > that first makes sure that the pointer we get from the map is non NULL: I copied
> > > that code around and that seemed to have solved the ICE.
> > >
> > > Regtested on x86-64-linux, and also checked that my aarch64 cross compilers are
> > > now building correctly libstdc++.
> > >
> > > Ok to commit?
> > Well, the obvious question is whether or not we ought to be able to
> > lookup the symbol in that particular map.
> >
> > You don't mention the other similar pattern of code which checks for
> > a NULL return value from map.get, so I had to guess it was this one
> > just above the one you want to change:
>
> Right.
>
> >       /* Get current lattice value of SYMBOL.  */
> >        val = map.get (symbol);
> >        if (val)
> >          group = *val;
> >
> >        /* If it is bottom, there is nothing to do; do not clear AUX
> >           so we won't re-queue the symbol.  */
> >        if (group == error_mark_node)
> >          continue;
> >
> > I couldn't convince myself this code was correct if map.get returns
> > a NULL pointer either!  It certainly isn't obvious if just
> > continuing either loop is correct if the symbol isn't found.
> >
> > I really think Jan needs to chime in here.
>
> If there is need for a testcase, I will open a bug report and attach the
> preprocessed file that causes the ICE.  As I said it is still a pretty large
> file, and creduce was not able to reduce it further.
Certainly attach it to a BZ -- from Jan's message it sounds like a NULL 
in that spot shouldn't be happening.  So I suspect he'll want to take a 
deeper look.

jeff