RFR(m): 8074552: SafeFetch32 and SafeFetchN do not work in error handling
Coleen Phillimore
coleen.phillimore at oracle.com
Wed Mar 18 19:58:22 UTC 2015
Hi, When I build a zero interpreter, I get a crash in SafeFetch32 from
the test. Is this patch missing support for Zero?
thanks,
coleen
On 3/12/15, 7:36 PM, David Holmes wrote:
> All looking good so pushing now.
>
> Thanks,
> David
>
> On 12/03/2015 10:22 PM, David Holmes wrote:
>> 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