RFR(xxs): 8225178: [Solaris] os::signal() should call sigaction() with SA_SIGINFO

[Thomas Stüfe]

Thanks for the quick review and the historical notes!

..Thomas

On Mon, Jun 3, 2019 at 10:05 PM Daniel D. Daugherty <
daniel.daugherty at oracle.com> wrote:

> On 6/3/19 9:56 AM, Thomas Stüfe wrote:
> > Hi all,
> >
> > may I have reviews please for this small patch:
> >
> > Bug: https://bugs.openjdk.java.net/browse/JDK-8225178
> > webrev:
> >
> http://cr.openjdk.java.net/~stuefe/webrevs/8225178-solaris-sa_siginfo-missing/webrev.00/webrev/
>
> src/hotspot/os/solaris/os_solaris.cpp
>      No comments.
>
> Thumbs up!
>
> Dan
>
> P.S.
> Just for historical note, I tracked the os::signal() code back into the
> TeamWare history and found that the missing SA_SIGINFO has been that way
> since that piece of code was introduced in 1998:
>
> $ sp -r1.82.1.3 src/os/solaris/vm/os_solaris.cpp
> src/os/solaris/vm/SCCS/s.os_solaris.cpp:
>
> D 1.82.1.3 98/10/20 16:47:14 jrose 277 276      00012/00003/02137
> MRs:
> COMMENTS:
> reconcile (and use sigaction instead of signal, to avoid SA_RESETHAND
> behavior)
>
> $ sccs sccsdiff -r1.82.1.{2,3} src/os/solaris/vm/os_solaris.cpp
>
> ------- os_solaris.cpp -------
> 1045,1046c1045
> <   /* We need to reinstate the signal handler each time... */
> <   os::signal(sig, UserHandler);
> ---
>  >   // We do not need to reinstate the signal handler each time...
> 1054c1053,1062
> <   return (void*)::signal(signal_number, (void (*)(int))handler);
> ---
>  >   struct sigaction sigAct, oldSigAct;
>  >   sigfillset(&(sigAct.sa_mask));
>  >   sigAct.sa_flags = SA_RESTART & ~SA_RESETHAND;
>  >   sigAct.sa_handler = (void (*)(int))handler;
>  >
>  >   if (sigaction(signal_number, &sigAct, &oldSigAct)) {
>  >     fatal("sigaction: cannot set signal %d (errno=%d)",
> signal_number, errno);
>  >   }
>  >
>  >   return (void*)oldSigAct.sa_handler;
> 2139a2148
>  > // 1.242 98/10/16 16:41:24 os_win32.cpp
>
>
>
> >
> > os::signal(), on Solaris, does not specify SA_SIGINFO. This leads to
> > signal handlers installed with this function - among others, the
> > secondary signal handler installed during error reporting - to be
> > called without siginfo_t pointer.
> >
> > All other platforms specify SA_SIGINFO in os::signal(), and so does
> > the Solaris-specific code which installs the "big" primary signal
> > handler. At SAP we run with this fix (specifying SA_SIGINFO) since years.
> >
> > Thanks, Thomas
>
>