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

[Jeff Law]

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