RFR(m): 8074552: SafeFetch32 and SafeFetchN do not work in error handling
Lindenmaier, Goetz
goetz.lindenmaier at sap.com
Wed Mar 11 15:17:27 UTC 2015
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