RFR(m): 8074552: SafeFetch32 and SafeFetchN do not work in error handling
Thomas Stüfe
thomas.stuefe at gmail.com
Thu Mar 19 09:35:13 UTC 2015
Hi Coleen,
this may be the case. I do not know the zero port very good.
SefeFetch handling needs support from both signal handler and access
routines to ucontext_t (ucontext_set_pc(), ucontext_get_pc()). Both is
missing.
In theory it could be added, because doing so requires no assembly of any
sorts. But it looks like the zero port avoids any technique which requires
fiddling around with the ucontext_t in the signal handler to jump to a
continuation point. This looks intentional and I do not want to introduce
any code which breaks the intention of zero port.
Of course, the other way would be just to disable the test for zero.
What would be the right way?
Kind regards, Thomas
On Wed, Mar 18, 2015 at 8:58 PM, Coleen Phillimore <
coleen.phillimore at oracle.com> wrote:
>
> 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