RFR: 8285792: Posix signal handler modification checking issues.

David Holmes dholmes at openjdk.org
Mon Jul 25 22:04:07 UTC 2022


On Mon, 25 Jul 2022 17:21:27 GMT, Harold Seigel <hseigel at openjdk.org> wrote:

> Please review this fix for JDK-8285792.  The fix removes print statements from check_signal_handler() so that it doesn't print all the handlers every time it finds one that is modified.  Instead, it returns true if the handler is modified, false otherwise.  Its caller, user_handler(), then prints all the handlers just once even if multiple signal handlers were modified.
> 
> The fix also adds a check for VMError::crash_handler_adress() to check_signal_handler() to prevent it from being treated as a signal handler modification.
> 
> The fix was tested with Mach5 tiers 1-2 on Linux and Mac OS and Mach 5 tiers 3-5 on Linux x64.  The regression test is excluded on Windows.
> 
> Thanks, Harold

Hi Harold,

Overall seems fine. A couple of nits.

We could simplify the test by just having (as is common) on Java file which launches itself in a new VM when invoked with no-args, and does the real test when passed a (dummy) arg. Then we will have one less file and clearer names.

Thanks.

src/hotspot/os/posix/signals_posix.cpp line 843:

> 841:   // Compare both sigaction structures (intelligently; only the members we care about).
> 842:   // Ignore if the handler is our own crash handler.
> 843:   if (!are_handlers_equal(&act, expected_act) &&

Pre-existing nit: really this should be called `are_actions_equal`. The poor naming is clearer when you now follow this with  `HANDLER_IS`.

test/hotspot/jtreg/runtime/posixSig/TestPsig.java line 28:

> 26:         System.loadLibrary("TestPsig");
> 27:     }
> 28:     public static native void doSomething(int val);

Suggestion:

`private static native void changeSigActionFor(int sig);`

-------------

Changes requested by dholmes (Reviewer).

PR: https://git.openjdk.org/jdk/pull/9631



More information about the build-dev mailing list