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

[Daniel D. Daugherty]

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