RFR(m): 8074552: SafeFetch32 and SafeFetchN do not work in error handling
David Holmes
david.holmes at oracle.com
Thu Mar 12 12:22:03 UTC 2015
Hi Thomas,
I'll try to re-test this internally in the morning. Sorry for the delay.
David
On 12/03/2015 3:10 AM, Thomas Stüfe wrote:
> 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 <mailto: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
> <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
> <mailto: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 <mailto: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>
> >> <mailto: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>
> <mailto: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>>
> >> <mailto:david.holmes at oracle.
> <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