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

[Thomas Stüfe]

pps

Instead of loading os::exception_name() with the task of both printing the
name and also checking whether a given signal number/exception number is
valid, I would prefer os::exception_name() to just always return a string
and for the checking an explicit API: we already have

bool os::Posix::is_valid_signal(int sig)

and it would be trivial to add this for windows and add an
os::is_valid_exception_number() or similar.

Kind Regards, Thomas


On Thu, Dec 10, 2015 at 11:35 AM, Thomas Stüfe <thomas.stuefe at gmail.com>
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)));
>
> e.g. os_solaris.cpp
>
> st->print("%s: ", os::exception_name(sig, buf, buflen));
>
> 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.
>
> Kind regards, Thomas
>
>
>
> On Thu, Dec 10, 2015 at 11:26 AM, David Holmes <david.holmes at oracle.com>
> wrote:
>
>> Hi David,
>>
>> On 10/12/2015 6:21 PM, David Lindholm wrote:
>>
>>> Hello Runtime,
>>>
>>> Please review this patch that fixes the posix version of
>>> os::exception_name(). The problem is that this function in a recent
>>> change started returning a string saying "invalid (<num>)" instead of
>>> NULL for invalid exception numbers, breaking the API. Accoring to os.hpp
>>> the API is still:
>>>
>>>    // returns a string to describe the exception/signal;
>>>    // returns NULL if exception_code is not an OS exception/signal.
>>>
>>> .. and this is also what windows does. This change in return value for
>>> the posix version makes the NULL-checks for calls to this function in
>>> vmError.cpp invalid, leading to that we don't get filename and
>>> linenumber printed in the hs_err file for asserts (for example).
>>>
>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8145073
>>> Webrev: http://cr.openjdk.java.net/~david/JDK-8145073/webrev.00/
>>>
>>
>> Sorry about that. I worked with Thomas on his change in this area. I
>> hadn't realized something was explicitly treating a NULL return to mean
>> "not really a signal/exception" :(
>>
>> Your fix to restore this behaviour looks good.
>>
>> Thanks,
>> David H.
>>
>>
>> Testing: JPRT.
>>>
>>>
>>> Thanks,
>>> David
>>>
>>>
>>>
>