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

[Dmitry Samersoff]

David,

Do we still need:

331   volatile int x = 0;
332   volatile int y = 1/x;

I guess current compiler tendency is to warn/optimize as much as
possible, so sooner or later we will have a warning/NOP here on all
platforms.

-Dmitry


On 2015-11-12 09:33, David Holmes wrote:
> webrev: http://cr.openjdk.java.net/~dholmes/8139300/webrev/
> 
> Bug report is not public sorry.
> 
> In the crash handler test logic in the VM we have:
> 
> static void crash_with_sigfpe() {
>    // 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: crash_with_sigfpe
> 
> and it is used here:
> 
>  402     case 15: crash_with_sigfpe(); break;
>  403
>  404     default: tty->print_cr("ERROR: %d: unexpected test_num 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: pthread_kill(pthreead_self(), 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 _POSIX_C_SOURCE 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


-- 
Dmitry Samersoff
Oracle Java development team, Saint Petersburg, Russia
* I would love to change the world, but they won't give me the sources.