RFR: 8145073: Filename and linenumber are not printed for assers any more.

Thomas Stüfe thomas.stuefe at gmail.com
Thu Dec 10 13:15:47 UTC 2015


Hi David, David,

@David:

I would also like to restore the old behavior for now, and thereby fixing
> this regression. We can change these APIs later.
>
>
I think this is reasonable. Sorry again for the trouble.


>
> Thanks
> David L
>
>
@David:


>
> On 2015-12-10 13:01, David Holmes wrote:
>
>> Hi Thomas,
>>
>> On 10/12/2015 8:35 PM, Thomas Stüfe wrote:
>>
>>> Hi David, David,
>>>
>>> sorry about that and thank you for fixing!
>>>
>>> I changed that behaviour (inconsequentially, because I left out windows
>>> and left the comment in) because the return code for
>>> os::exception_name() is used in a number of places as input for
>>> printing, without doing a null check first:
>>>
>>> e.g os_solaris_x86.cpp:
>>>
>>> 364        warning("Ignoring %s - see 4229104 or 6499219",
>>> 365                os::exception_name(sig, buf, sizeof(buf)));
>>>
>>
>> But here sig is known to be SIGPIPE or SIGXFSZ so exception_name can
>> never return NULL.
>>
>> e.g. os_solaris.cpp
>>>
>>> st->print("%s: ", os::exception_name(sig, buf, buflen));
>>>
>>
>> This is in print_signal_handler so sig will always be a signal value - so
>> again NULL is not possible.
>>
>> So, I think it is besser to never return NULL at all. But if we go back
>>> to the old behaviour, we should fix those callsites and add NULL checks
>>> to them.
>>>
>>
>> I don't like the way NULL is used to indicate "hey you didn't really pass
>> me a signal number!". The _id in VMError is defined:
>>
>>   static int         _id;               // Solaris/Linux signals: 0 -
>> SIGRTMAX
>>                                         // Windows exceptions: 0xCxxxxxxx
>> system errors
>>                                         // 0x8xxxxxxx system warnings
>>
>> nothing there about assertions! But in fact it can have a range of
>> non-signal related values and primarily of interest in this case
>> INTERNAL_ERROR (0xe0000000). So while I don't like the way this is managed,
>> restoring the NULL behaviour is a lot simpler than trying to rework
>> everything.
>>
>
Let me just add as a separate argument against returning NULL that the
construct

const char* foo(char* buf, size_t len) {
   ...
  return buf;
}

traditionally is used to be able to use this as arguments for printf-like
functions, and returning NULL defies that purpose.

Thanks,

Thomas


More information about the hotspot-runtime-dev mailing list