RFR (s) 8251383: Disable Event::log from linux_mprotect when processing the assertion poison page

[David Holmes]

Thanks for the review!

David

On 11/08/2020 7:52 pm, Thomas Stüfe wrote:
> Looks good now. Thanks for fixing.
> 
> ...Thomas
> 
> On Tue, Aug 11, 2020 at 11:41 AM David Holmes <david.holmes at oracle.com 
> <mailto:david.holmes at oracle.com>> wrote:
> 
>     Hi Thomas,
> 
>     On 11/08/2020 7:28 pm, Thomas Stüfe wrote:
>      > Hi David,
>      >
>      > ugh. Sorry for the complexity. Our compiler guys really liked this
>      > feature, but I still have the nagging feeling this was not my best
>      > development ever (or, more precisely, that the added complexity
>     may not
>      > be worth the information).
> 
>     I had a painful time figuring out what was going wrong as I was looking
>     for a problem with my assert condition, not something in the assertion
>     code itself. That said I didn't pay enough attention to the gdb stack
>     either so mea culpa. :(
> 
>      > As for the patch, you should use CAN_SHOW_REGISTERS_ON_ASSERT
>     instead of
>      > ASSERT/DEBUG since this may break zero. See debug.hpp.
> 
>     Ah right. That is a bit uglier. webrev updated in place.
> 
>     Thanks,
>     David
>     -----
> 
>      > Cheers, Thomas
>      >
>      >
>      > On Tue, Aug 11, 2020 at 11:17 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:
>      >
>      >     Bug: https://bugs.openjdk.java.net/browse/JDK-8251383
>      >     webrev: http://cr.openjdk.java.net/~dholmes/8251383/webrev/
>      >
>      >     When the assertion poison page is enabled (Linux only and on by
>      >     default)
>      >     the signal handler will call os::protect_memory to change the
>     page
>      >     protection bits. That will call linux_mprotect which will call
>      >
>      >     Events::log(NULL, "Protecting memory [" INTPTR_FORMAT ","
>     INTPTR_FORMAT
>      >     "] with protection modes %x", p2i(bottom), p2i(bottom+size),
>     prot);
>      >
>      >     Event logging in turn can use Mutexes and other VM facilities
>     - all of
>      >     which are now being executed in a signal handling context
>     (which is
>      >     inherently unsafe). It also means that there cannot be any other
>      >     failing
>      >     assertions on that path as you will re-trigger the poison page
>      >     pagefault
>      >     and abort with no hs_err file. Hence, as happened to me, a
>     failing
>      >     assertion in the mutex code triggers this problem.
>      >
>      >     The issue can be worked-around by setting
>     -XX:-ShowRegistersOnAssert
>      >     (once you realise what is happening).
>      >
>      >     The simple fix is to skip the logging if the faulting address
>     is the
>      >     poison page address.
>      >
>      >     This only affects debug builds of course.
>      >
>      >     Testing:
>      >         - runtime/ErrorHandling
>      >         - tier 1-3
>      >
>      >     Thanks,
>      >     David
>      >
>