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