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

Jan Hubicka hubicka@ucw.cz
Fri Sep 19 14:14:00 GMT 2014


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

I am bit in hurry, so just quickly (will give it second look at evening).  The
pass is a simple dataflow propagating preferred sections. NULL means that the
symbol in question was not visited yet.  Assuming that all symbols are reachable,
this should not happen by the end of dataflow process (well, thining about it,
perhaps used attribute can trigger it, but it ought to be handled earlier).

So I would like to know why the symbols survives remove_unreachable_functions...

Honza
> 
> Sebastian



More information about the Gcc-patches mailing list