RFR(S): 8153892: Handle unsafe access error directly in signal handler instead of going through a stub

[David Holmes]

On 11/04/2016 7:15 PM, Thomas Stüfe wrote:
>
>
> On Mon, Apr 11, 2016 at 11:05 AM, Thomas Stüfe <thomas.stuefe at gmail.com
> <mailto:thomas.stuefe at gmail.com>> wrote:
>
>     Hi David,
>
>     On Mon, Apr 11, 2016 at 2:57 AM, David Holmes
>     <david.holmes at oracle.com <mailto:david.holmes at oracle.com>> wrote:
>
>         Hi Mikael,
>
>         I think we need to be able to answer the question as to why the
>         stubbed and stubless forms of this code exist to ensure that
>         converting all platforms to the same form is appropriate.
>
>         I'm still going through this but my initial reaction is to
>         wonder why we don't use the same form of handle_unsafe_access on
>         all platforms and always pass in npc? (That seems to be the only
>         difference in code that otherwise seems platform independent.)
>
>
>     On Solaris we get the npc for free in the signal ucontext. On x86 it
>     has to be calculated. But yes, this could be moved out of the handle
>     functions and just passed in.
>
>     I also saw that we apparently miss handling for ppc. No one seemed
>     to miss it until now, but it may make sense to add handling anyway.
>
>
> Oh, we do not miss it. Volker just showed me that  it is done directly
> in the signal handlers for AIX and Linux ppc.

So is there more scope to refactor this out of the platform specific 
code altogether?

David

>
>     Kind Regards, Thomas
>
>
>         Thanks,
>         David
>
>
>         On 9/04/2016 8:33 AM, Mikael Vidstedt wrote:
>
>
>             Please review:
>
>             Bug: https://bugs.openjdk.java.net/browse/JDK-8153892
>             Webrev:
>             http://cr.openjdk.java.net/~mikael/webrevs/8153892/webrev.01/hotspot/webrev/
>
>
>             * Note: this is patch 2 in a set of 3 all aiming to clean up
>             and unify
>             the unsafe memory getters/setters, along with the handling
>             of unsafe
>             access errors. The other two issues are:
>
>             https://bugs.openjdk.java.net/browse/JDK-8153890 - Handle
>             unsafe access
>             error as an asynchronous exception
>             https://bugs.openjdk.java.net/browse/JDK-8150921 - Update Unsafe
>             getters/setters to use double-register variants
>
>
>             * Summary (copied from the bug description)
>
>
>             In certain cases, such as accessing a region of a memory
>             mapped file
>             which has been truncated on unix-style operating systems, a
>             SIGBUS
>             signal will be raised and the VM will process it in the
>             signal handler.
>
>             How the signal is processed differs depending on the
>             operating system
>             and/or CPU architecture, with two major alternatives:
>
>             * "stubless"
>
>             Do the necessary thread state updates directly in the signal
>             handler,
>             and modify the context so that the signal handler returns to
>             the place
>             where the execution should continue
>
>             * Using a stub
>
>             Update the context so that when the signal handler returns
>             the thread
>             will continue execution in a generated stub, which in turn
>             will call
>             some native code in the VM to update the thread state and
>             figure out
>             where execution should continue. The stub will then jump to
>             that new place.
>
>
>             It should be noted that the work of updating the thread
>             state is very
>             small - it's setting a flag or two in the thread structure,
>             and figures
>             out where the next instruction starts. It should also be
>             noted that the
>             generated stubs today are broken, because they do not
>             preserve all the
>             live registers over the call into the VM. There are two ways
>             to address
>             this:
>
>             * Preserve all the necessary registers
>
>             This would mean implementing, in macro assembly, the
>             necessary logic for
>             preserving all the live registers, including, but not
>             limited to,
>             floating point registers, flag registers, etc. It quickly
>             becomes
>             obvious that this platform specific and error prone.
>
>             * Leverage the fact that the operating system already does
>             this as part
>             of the signal handling
>
>             Do the necessary work in the signal handler instead,
>             removing the need
>             for the stub alltogether
>
>             As mentioned, on some platforms the latter model is already
>             in use. It
>             is dramatically easier and all platforms should be updated
>             to do it the
>             same way.
>
>
>             * Testing
>
>             Just as mentioned in the RFR for JDK-8153890, a new test was
>             developed
>             to test this code path:
>
>             http://cr.openjdk.java.net/~mikael/webrevs/8150921/MappedTruncated.java
>
>             In fact, it was when running this test I found the register
>             preservation
>             issue. JPRT also passes. Much like JDK-8153890 I wanted to
>             get some
>             feedback on this before running additional tests.
>
>
>             Cheers,
>             Mikael
>
>
>