(S) RFR: 8139300: Internal Error (vm/utilities/debug.cpp:399), # Error: ShouldNotReachHere() (original) (raw)

Daniel D. Daugherty daniel.daugherty at oracle.com
Mon Nov 16 20:34:17 UTC 2015

On 11/11/15 11:33 PM, David Holmes wrote:

webrev: http://cr.openjdk.java.net/~dholmes/8139300/webrev/

src/share/vm/utilities/debug.cpp No comments.

test/runtime/ErrorHandling/SecondaryErrorTest.java No comments.

Thumbs up.

Since you've used the POSIX equivalent code:

 pthread_kill(pthread_self(), SIGFPE);

I'm presuming we have no plans to change this code back once MacOS X fixes the bug in raise().

Dan

Bug report is not public sorry. In the crash handler test logic in the VM we have: static void crashwithsigfpe() { // generate a native synchronous SIGFPE where possible; // if that did not cause a signal (e.g. on ppc), just // raise the signal. volatile int x = 0; volatile int y = 1/x; #ifndef WIN32 raise(SIGFPE); #endif } // end: crashwithsigfpe and it is used here: 402 case 15: crashwithsigfpe(); break; 403 404 default: tty->printcr("ERROR: %d: unexpected testnum value.", how); 405 } 406 ShouldNotReachHere(); We recently updated the compiler on OSX and started seeing occasional failures of the test exercising this code, the failure mode being that we hit the ShouldNotReachHere() at line #406. It seems the new compiler may not be generating code for the "y=1/x;"** so we don't get the SIGFPE and so proceed to raise it directly. That then hits an apparent bug on OSX where raise always sends the signal to the main thread not the current thread as required by POSIX in a multi-threaded app. Consequently raise() could return and we'd hit the ShouldNotReachHere(). Whether the test failed or appeared to pass depended on which thread got into the error handler first (though I'm still unclear on the finer details of that potential interaction - the other thread should have seen error reporting was already in progress by the recursively failing thread!) The solution I chose is to simply convert raise(sig) into its POSIX specified equivalent: pthreadkill(pthreeadself(), sig); I'm a little reluctant having pthread functions in a shared file, but AFAIK all our platforms supports pthreads. This also doesn't seem any worse than assuming raise() exists on all platforms. But I could add some ifdef POSIXCSOURCE if there is concern (and even have a #error for unknown platforms) ? ** I'm not surprised a compiler might elide the attempted division by zero. I don't think volatile on a local has any meaning and so could easily be optimized out completely. I overlooked that in the original commit of this logic, and it worked so ... Thanks, David

More information about the hotspot-runtime-dev mailing list