RFR(S): 8153892: Handle unsafe access error directly in signal handler instead of going through a stub
Mikael Vidstedt
mikael.vidstedt at oracle.com
Mon Apr 11 18:52:39 UTC 2016
David/Thomas,
Thanks for the early feedback. Some comments below.
On 4/11/2016 2:15 AM, 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.)
>
Yes, in effect what the handler is supposed to do is call
t->set_pending_unsafe_access_error() and update the context so that when
the thread eventually starts executing again it will start on the next
instruction, following the one that faulted. Given how trivial the
handler is I can see no reason to make it go through a stub, and several
reasons for handling it directly in the signal handler instead. Maybe
I'm missing something, let me know if you think of anything!
>
> 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.
>
Since the function is called from multiple different places it seems
appropriate to have a dedicated function for it, even though it's "just"
doing those two things. It also means it can be shared across the
different operating systems, within the same CPU architecture. As noted,
SPARC is indeed the odd man out which needs to take the additional "npc"
argument, but I really don't think that's a big issue in the grand
scheme of things.
>
> 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.
Exactly. AIX/ppc, linux/ppc and linux/aarch64 all handle it directly in
the signal handler.
Cheers,
Mikael
> 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/
> <http://cr.openjdk.java.net/%7Emikael/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
> <http://cr.openjdk.java.net/%7Emikael/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
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/ppc-aix-port-dev/attachments/20160411/f5c3c4a1/attachment-0001.html>
More information about the ppc-aix-port-dev
mailing list