RFR(m): 8074552: SafeFetch32 and SafeFetchN do not work in error handling
Thomas Stüfe
thomas.stuefe at gmail.com
Wed Mar 11 17:10:29 UTC 2015
Hi Goetz,
thanks for reviewing!
Here the updated changes:
http://cr.openjdk.java.net/~stuefe/webrevs/8074552/webrev.03/webrev/
- I changed os_linux_sparc.cpp to reflect your comments: removed
set_cont_address() and replaced it with ucontext_set_pc(). However, I do
not have a linux sparc machine to build and test, so the change is made
blind. Hope it does not break.
- I removed the "CanUseSafeFetch.." calls from the initial test routine.
Originally I was afraid there were platforms where there are no
implementations for SafeFetch32 or SafeFetchN, but that does not seem to be
the case. Hopefully, I cannot speak for arm or whatever other closed
platforms are there.
Kind Regards, Thomas
On Wed, Mar 11, 2015 at 4:17 PM, Lindenmaier, Goetz <
goetz.lindenmaier at sap.com> wrote:
> Hi Thomas,
>
> I had a look at your change. I'm fine with it, except for two minor
> things:
>
> os_linux_sparc.cpp
> Why didn't you just replace set_cont_address by ucontext_set_pc on
> linux_sparc?
> As it is now, ucontext_set_pc is just a rename of an existing function.
>
> stubRoutines.cpp:
> Why do you test for availability of the stubs when testing them? I would
> consider
> it a bug if the stub is not generated at that point, and that would not be
> seen here
> as is now.
>
> Best regards,
> Goetz.
>
>
>
> -----Original Message-----
> From: hotspot-runtime-dev [mailto:
> hotspot-runtime-dev-bounces at openjdk.java.net] On Behalf Of Thomas Stüfe
> Sent: Dienstag, 10. März 2015 18:21
> To: David Holmes
> Cc: hotspot-runtime-dev at openjdk.java.net
> Subject: Re: RFR(m): 8074552: SafeFetch32 and SafeFetchN do not work in
> error handling
>
> Hi David,
>
> see here my new patch:
>
> http://cr.openjdk.java.net/~stuefe/webrevs/8074552/webrev.02/webrev/
>
> Turns out that SafeFetch32 does not work in 32bit windows during
> initialization, because there is no SEH exception guard installed at that
> time (during the call to CreateJavaVM). Neither is there one for 64bit
> Windows, but there, we register SEH handlers for dynamic code, that hides
> the error.
>
> This is an issue I'd like to address with a separate patch (see
> https://bugs.openjdk.java.net/browse/JDK-8074860). For now, I deactivated
> the SafeFetch test during stub routine initialization for Windows 32bit.
>
> Other changes in the fix:
> - as you wished I changed the ULL literals to UCONST64
> - I activated the Jtreg test for windows too (SafeFetch works on windows
> during error handling, only not during initialization).
>
> Thomas
>
>
>
>
>
> On Mon, Mar 9, 2015 at 11:06 PM, David Holmes <david.holmes at oracle.com>
> wrote:
>
> > On 10/03/2015 2:30 AM, Thomas Stüfe wrote:
> >
> >> Hi David,
> >>
> >> I see now that only Windows 32bit is broken. I will investigate this.
> >>
> >
> > Yes, sorry, I hadn't noticed it was only 32-bit failing.
> >
> > Thanks,
> > David
> >
> > Thomas
> >>
> >> On Mon, Mar 9, 2015 at 4:12 PM, Thomas Stüfe <thomas.stuefe at gmail.com
> >> <mailto:thomas.stuefe at gmail.com>> wrote:
> >>
> >> Hi David,
> >>
> >> I am sorry, but I am unable to reproduce the error. Both slowdebug
> >> and fastdebug builds on Windows come up and SafeFetch works (also in
> >> error handling). Are you sure the crash is caused by my change? If
> >> yes, could you send me an hs-err file or at least the configure line
> >> you used for building?
> >>
> >> Kind Regards, Thomas
> >>
> >> On Mon, Mar 9, 2015 at 7:51 AM, David Holmes
> >> <david.holmes at oracle.com <mailto:david.holmes at oracle.com>> wrote:
> >>
> >> On 9/03/2015 4:21 PM, Thomas Stüfe wrote:
> >>
> >> Hi David,
> >>
> >> Ouch. The only thing I can think about are the added
> >> initialization
> >> tests in stubRoutines.cpp. I will check this.
> >>
> >>
> >> Ah! It hadn't registered with me that this test code was going
> >> to be executed. Seems the likely cause as a clean repo has no
> >> problems.
> >>
> >> David
> >>
> >> Kind Regards, Thomas
> >>
> >>
> >>
> >> On Mon, Mar 9, 2015 at 6:56 AM, David Holmes
> >> <david.holmes at oracle.com <mailto:david.holmes at oracle.com>
> >> <mailto:david.holmes at oracle.__com
> >> <mailto:david.holmes at oracle.com>>> wrote:
> >>
> >> Hi Thomas,
> >>
> >> FYI When I applied this to latest hs-rt forest and ran
> >> through JPRT
> >> the Windows fastdebug builds were DOA (instant segfault
> >> just running
> >> "java -version"). Can't see how your changes could do
> >> this so
> >> running a test on unmodified forest.
> >>
> >> David
> >>
> >>
> >> On 9/03/2015 9:40 AM, David Holmes wrote:
> >>
> >> Hi Thomas,
> >>
> >> Minor nit: in
> >>
> >> src/share/vm/runtime/____stubRoutines.cpp::test_____
> >> safefetchN
> >> can
> >> you use UCONST64 macro instead of explicit ULL -
> >> thanks.
> >>
> >> Otherwise this seems okay, but we will also need
> >> the aarch64
> >> component
> >> (and I'll look into our internal needs).
> >>
> >> Thanks,
> >> David
> >>
> >> On 7/03/2015 2:10 AM, Thomas Stüfe wrote:
> >>
> >> Hi all,
> >>
> >> could someone please review the following fix
> >> (I also will
> >> need a
> >> sponsor):
> >>
> >> http://cr.openjdk.java.net/~____stuefe/webrevs/8074552/
> >> webrev.____01/
> >> <http://cr.openjdk.java.net/~__stuefe/webrevs/8074552/
> >> webrev.__01/>
> >>
> >> <http://cr.openjdk.java.net/~__stuefe/webrevs/8074552/
> >> webrev.__01/
> >> <http://cr.openjdk.java.net/~stuefe/webrevs/8074552/webrev.
> >> 01/>>
> >>
> >> https://bugs.openjdk.java.net/____browse/JDK-8074552
> >> <https://bugs.openjdk.java.net/__browse/JDK-8074552>
> >>
> >>
> >> <https://bugs.openjdk.java.__net/browse/JDK-8074552
> >>
> >> <https://bugs.openjdk.java.net/browse/JDK-8074552>>
> >>
> >> The fix will make SafeFetch[32,N] work in error
> >> reporting.
> >>
> >> At SAP, we use SafeFetch a lot in error
> >> reporting to poke
> >> around in
> >> potentially invalid memory (e.g. writing hex
> >> dumps over
> >> areas which
> >> may be
> >> partly unmapped), and we feel that this could
> >> be useful for the
> >> OpenJDK too.
> >>
> >> Without this fix, SafeFetch will cause a crash,
> >> the current
> >> error
> >> reporting
> >> step will be interrupted and error reporting
> >> will continue
> >> with the next
> >> step; that is not optimal because the
> >> interrupted step may
> >> have shown
> >> valuable information.
> >>
> >> This fix handles SafeFetch faults during error
> >> reporting the
> >> same way as
> >> they are handled normally. The changes are:
> >>
> >> - handle safe fetch fault in the various
> >> (os_cpu dependend)
> >> secondary
> >> signal handlers
> >>
> >> - provide a function to check if it is safe to
> >> use SafeFetch:
> >> CanUseSafeFetch32(). SafeFetch needs stub
> >> routines and will
> >> crash when
> >> used
> >> before stub generation.
> >>
> >> - set_context_pc() is added which complements
> >> the existing
> >> get_context_pc()
> >> and all instances where the pc in ucontext_t
> >> was modified
> >> directly are
> >> changed to use set_context_pc()
> >>
> >> - in stubRoutines.cpp, a small test was added
> >> to the already
> >> existing
> >> stub
> >> routines tests which run at VM init
> >>
> >> - in vmError.cpp, a test was added to test
> >> SafeFetch during
> >> error
> >> reporting, similar to the tests introduced for
> >> 8065895
> >>
> >> - A JTreg test was added
> >>
> >> Thanks and Kind Regards,
> >>
> >> Thomas Stuefe
> >>
> >>
> >>
> >>
> >>
>
More information about the hotspot-runtime-dev
mailing list