On Sun, 9 Oct 2022 01:40:29 GMT, Kim Barrett <kbarr...@openjdk.org> wrote:
>> src/hotspot/os/posix/signals_posix.cpp line 1727: >> >>> 1725: // Set up signal handler for suspend/resume >>> 1726: act.sa_flags = SA_RESTART|SA_SIGINFO; >>> 1727: act.sa_handler = CAST_FROM_FN_PTR(sa_handler_t, SR_handler); >> >> The underlying cast in the macro seems a little odd for this situation but >> if it appeases the compiler then that is fine. > > No, don't do this. The code is just wrong, and should be fixed. > > We have a struct sigaction. We've set the sa_flags member to a value that > includes SA_SIGINFO. That means the 3 argument sa_sigaction handler is > supposed to be invoked. > > We're storing the SR_handler (cast to a value matching the type of the > sa_handler member of the sigaction). This is "working" purely by accident. > Apparently on all POSIX platforms we support, the sa_handler and sa_sigaction > occupy the same memory (being in a union, which is permitted by the spec). > > The signature for SR_handler should be changed from > void SR_handler(int, siginfo_t*, ucontext_t*) > to > void SR_handler(int, siginfo_t*, void*) > and adjust the code to deal with the change to the type of the 3rd parameter. > > Then we can just assign the sa_sigaction member to SR_handler without any > casts. This is an excellent catch, @kimbarrett! I think I missed it the same way David did: I completely missed there is `sa_handler` and `sa_sigaction`! Re-done this in new commits. ------------- PR: https://git.openjdk.org/jdk/pull/10494
