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

[David Holmes]

On 31/03/2015 7:08 PM, Thomas Stüfe wrote:
> Hi Severin, David,
>
> In that case, lets push the change.

It is on its way. :)

BTW Thomas - you should request jdk9 Author status.

Cheers,
David

> On Tue, Mar 31, 2015 at 10:52 AM, Severin Gehwolf <sgehwolf at redhat.com
> <mailto: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 <mailto: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
>      >
>      >
>      >
>
>
>
>