RFR: 8075533: Zero JVM segfaults for -version after JDK-8074552

[Thomas Stüfe]

Hi Volker, Serverin,

On Fri, Mar 20, 2015 at 4:17 PM, Volker Simonis <volker.simonis at gmail.com>
wrote:

> Hi Severin,
>
> I can't understand how SafeFetch32 and SafeFetchN are supposed to work
> at all in Zero. The whole point of having the SafeFetch routines is to
> have a possibility of peeking at potentially illegal memory addresses.
> The current implementation of SafeFetch on Zero can not provide this
> functionality so in my eyes it is useless.
>
>
I do not think there is a hard technical reason preventing us from doing
this on zero. See the short discussion at the end of:
http://mail.openjdk.java.net/pipermail/hotspot-runtime-dev/2015-March/014231.html

You need the stub routines and a bit of handling in the signal handlers.
>From what I understood it was a decision of the zero people not to support
it.

I think we should do the following:
>  - StubRoutines::SafeFetchN_stub() should just return 'null' on Zero
> (this way the unmodified CanUseSafeFetchXX() routines would work for
> Zero as well)
>  - remove the implementations of  SafeFetchN and SafeFetch32 and set
> StubRoutines::_safefetchN_entry and StubRoutines::_safefetch32_entry
> to NULL in stubGenerator_zero.cpp (this will automatically make
> StubRoutines::SafeFetchN_stub() return null on Zero)
>

I agree with all that.

Unless we question the decision to not support SafeFetch and add support to
it on zero, it is quite handy after all.


>  - the calls to test_safefetch32XX() should be guarded by
> CanUseSafeFetchXX() (this woul avoid the Zero ifdefs in
> stubRoutines.cpp)
>

Talk to Goetz :) I did this originally (guarding test_safeFetch() with
CanUseSafeFetch()), and his wish was to remove that condition; reason was
that we thought that all platforms support SafeFetch and hence the test was
unneccessary. We just did not think of zero.

In the light of zero not having (or wanting to have) SafeFetch support, we
should guard the test with CanUseSafeFetchxxx.

Regards, Thomas


>
> The test changes are good.
>
> Regards,
> Volker
>
>
> On Fri, Mar 20, 2015 at 12:57 PM, Severin Gehwolf <sgehwolf at redhat.com>
> wrote:
> > Hi,
> >
> > Could somebody please review and sponsor this patch?
> >
> > The commit of JDK-8074552 broke the Zero port since it performs tests on
> > JVM initialization if SafeFetch32 and SafeFetchN on invalid pointers
> > (relying on a continuation scheme for safe fetch errors) work. Yet
> > performing this test correctly requires support from the signal handler
> > and access routines to ucontext_t which are missing for Zero.
> >
> > The intent of JDK-8074552 was to improve error reporting on SEGV's.
> > Having this feature in Zero is less useful, since it's a C++ interpreter
> > only environment.
> >
> > This patch disables the test and makes CanUseSafeFetch* functions return
> > false unconditionally for Zero JVMs so as to not produce a recursive
> > SEGV's during error reporting.
> >
> > I've also updated test SafeFetchInErrorHandlingTest.java so it's skipped
> > for Zero.
> >
> > bug: https://bugs.openjdk.java.net/browse/JDK-8075533
> > webrev:
> > http://cr.openjdk.java.net/~sgehwolf/webrevs/JDK-8075533/webrev.01/
> >
> > Testing done: make bootcycle-images fails before this patch and passes
> > after for a Zero JVM on x86_64.
> >
> > Thanks,
> > Severin
> >
>