RFR(s): 8076185: Provide SafeFetchX implementation for zero

[Thomas Stüfe]

Hi Severin, David,

In that case, lets push the change.

On Tue, Mar 31, 2015 at 10:52 AM, Severin Gehwolf <sgehwolf at redhat.com>
wrote:

> Hi Thomas,
>
> On Tue, 2015-03-31 at 10:38 +0200, Thomas Stüfe wrote:
> > Hi David.
> >
> >
> > I am fine with that, thanks!
> >
> >
> > But Severin should give his ok, too.
>
> You have my OK.
>
> > We also should accept that my SafeFetch implementation will be slower
> > than the standard one using inline assembly, because setjmp() does a
> > lot of stores. As far as I can see now, we do not use SafeFetch
> > extensivly anywhere, so this should be ok, but something to keep in
> > mind.
>
> This is expected. The only use of SafeFetch I'm aware is in
> src/share/vm/runtime/objectMonitor.cpp and the error handler. Speaking
> of which:
>
> $ java -XX:+UnlockDiagnosticVMOptions -Xmx100M -XX:ErrorHandlerTest=14
> -XX:+TestSafeFetchInErrorHandler -version
>
> Does not yet work as expected. But that's a separate issue.
>
>
That is ok.

For -XX:+TestSafeFetchInErrorHandler to work, we would need to handle
SafeFetch faults in the secondary signal handlers too (see
vmError_<os>.cpp). That could be easily added but we would leave POSIX
territory. Using longjmp() is not guaranteed to work from nested signal
handlers ("However, if longjmp() is invoked from a nested signal handler
(that is, from a function invoked as a result of a signal raised during the
handling of another signal), the behaviour is undefined.").

I think that use case is not that important for zero.



> > So far, I only tested this on Linux x64. Because of the build error
> > after 8074345, I was not able to test a product build of zero, only a
> > slowdebug. I ran a number of JTReg tests with it and got a number of
> > errors, but I got the same errors without my patch, so I assume the
> > errors stem from the slowdebug VM just being too slow.
>
> JDK-8076181 should have fixed this. FWIW, I've built this with your
> patch and JDK-8076181 using a product build and I've seen no issues.
>
>
nice, thanks!

..:Thomas


> Cheers,
> Severin
>
> > On Tue, Mar 31, 2015 at 3:45 AM, David Holmes
> > <david.holmes at oracle.com> wrote:
> >         Hi Thomas,
> >
> >         If you are happy with Severin's review I can add my Review to
> >         the shared part (Zero part looks okay too) and sponsor it.
> >
> >         David
> >
> >
> >         On 28/03/2015 4:00 AM, Thomas Stüfe wrote:
> >                 Hi all,
> >
> >                 Please review this change which provides a real
> >                 SafeFetch implementation
> >                 on zero.
> >
> >                 webrev:
> >
> http://cr.openjdk.java.net/~stuefe/webrevs/8076185/webrev.01/webrev/
> >                 bug:
> >                  https://bugs.openjdk.java.net/browse/JDK-8076185
> >
> >                 It works like this:
> >
> >                 Before a load is attempted from a potentially unsafe
> >                 address, we set up a
> >                 jump buffer with sigsetjmp(). In the signal handler,
> >                 for SIGSEGV and SIGBUS,
> >                 we test whether there is a jump buffer set and if yes,
> >                 take this as an
> >                 indication
> >                 that the crash was an attempted SafeFetch. In this
> >                 case we jump back via
> >                 longjmp().
> >
> >                 Coding is a bit more difficult because we need to be
> >                 threadsafe. We keep
> >                 the jump
> >                 buffer on the thread stack - this is ok, because that
> >                 stack never gets
> >                 unwinded
> >                   - either we crash, in which case signal handler
> >                 stack frames are built up
> >                 below
> >                 us, or we don't crash, in which case we never leave
> >                 the SafeFetch function.
> >                 In
> >                 both cases, we never loose the jump buffer.
> >
> >                 To communicate the jump buffer location to the signal
> >                 handler, TLS is used.
> >                 I
> >                 use POSIX tls, because that always works and is not
> >                 dependend on VM
> >                 infrastructure.
> >
> >                 ---
> >
> >                 I built and tested this on Linux x64 zero (Ubuntu
> >                 14.4). It works and the
> >                 initialization tests for SafeFetch in the stub routine
> >                 initialization now
> >                 work for
> >                 zero too.
> >
> >                 Note that I do not have a BSD system right now, so I
> >                 cannot check whether
> >                 this
> >                 change builds and works for BSD. But the change only
> >                 requires POSIX Apis, so
> >                 BSD should probably be ok.
> >
> >                 Someone from the zero team should definitly check and
> >                 test this for other
> >                 zero
> >                 platforms, but the chances are good that this just
> >                 works.
> >
> >                 Thanks and Kind Regards,
> >
> >                 Thomas Stuefe
> >
> >
> >
>
>
>
>