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

[David Holmes]

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.

Thanks,
David H.
-------

> Kind regards, Thomas
>
>
>
> On Thu, Dec 10, 2015 at 11:26 AM, David Holmes <david.holmes at oracle.com
> <mailto: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
>
>
>