RFR: 8292054: Test runtime/posixSig/TestPosixSig.java fails with 'Test failed, bad output.'

[Coleen Phillimore]

On Mon, 15 Aug 2022 18:29:55 GMT, Harold Seigel <hseigel at openjdk.org> wrote:

> Please review this fix for JDK-8292054.  The existing regression test for JDK-8285792, test/hotspot/jtreg/runtime/posixSig/TestPosixSig.java, intermittently fails because it depends on periodic calls to JVM function os::run_periodic_checks().  This fix replaces test TestPosixSig.java with a gtest that does its own explicit call to os::run_periodic_checks().
> 
> The fix was tested by running the new test 150+ times on Linux, Mac OS, and Windows.

This seems like an improvement overall, but not crazy about reassigning a global tty.

test/hotspot/gtest/runtime/test_signals.cpp line 52:

> 50:     sigemptyset(&act.sa_mask);
> 51:     act.sa_flags = 0;
> 52:     if (sigaction(SIGFPE, &act, &old_SIGFPE_act) == -1) {

Instead of perror() which I haven't seen in the gtests (maybe it's ok), we usually use ASSERT statements like:
ASSERT_NE(sigaction(SIGFPE, &act, &old_SIGFPE_act), -1) << sigaction(SIGFPE) failed;

test/hotspot/gtest/runtime/test_signals.cpp line 66:

> 64:     // Set tty to local stringStream to capture output from run_periodic_checks()
> 65:     // calls to print_signal_handlers().
> 66:     tty = &st;

Would it be possible to add an outputStream* st parameter to run_periodic_checks() instead of doing this? I believe that function only uses tty twice.

test/hotspot/gtest/runtime/test_signals.cpp line 76:

> 74:       return "restoring SIGFPE handler failed";
> 75:     }
> 76:     if (sigaction(SIGILL, &old_SIGILL_act, 0)) {

Not == -1 ?

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

Changes requested by coleenp (Reviewer).

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