RFR(m): 8074552: SafeFetch32 and SafeFetchN do not work in error handling
David Holmes
david.holmes at oracle.com
Wed Mar 11 01:03:44 UTC 2015
On 11/03/2015 3:21 AM, Thomas Stüfe wrote:
> 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).
Okay - this all looks good to me. Running through internal testing now.
Need a second reviewer before I can submit this.
Thanks,
David
> 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/>>
>
>
> <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>>
>
>
> <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