On Fri, 12 Mar 2021 05:24:10 GMT, David Holmes <[email protected]> wrote:
>> Anton Kozlov has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> 8262903: [macos_aarch64] Thread::current() called on detached thread
>
> src/hotspot/share/runtime/safefetch.inline.hpp line 35:
>
>> 33: inline int SafeFetch32(int* adr, int errValue) {
>> 34: assert(StubRoutines::SafeFetch32_stub(), "stub not yet generated");
>> 35: MACOS_AARCH64_ONLY(ThreadWXEnable wx(WXExec, Thread::current()));
>
> I think you may have to use `Thread::current_or_null_safe()` here in case
> this gets called from a signal handling context - see vmError.cpp testing for
> TestSafeFetchInErrorHandler. Same possibly for SafeFetchN.
I'm not sure about expected behavior then. We may crash trying to execute the
generated code, since we may have no WXExec. If we switch to WXExec, we would
need to go back to a previous W^X state, but we don't know which one without
the thread.
BTW, the test passes, probably that's why it didn't get attention. All
non-trivial actions in the current implementation of
`pd_hotspot_signal_handler`
(hhttps://github.com/openjdk/jdk/pull/2200/files#diff-9dcc5bcf908e2f8eb00b2c2837d667687a7540936d8f538ee1ea14e31ad50b40R215-R324)
assume non-NULL thread. So AFAICS, we should always have a thread when the
SafeFetch is called.
Probably a fix to the https://bugs.openjdk.java.net/browse/JDK-8262903 could
just move ThreadWXEnable under the `if`. But now after
https://github.com/openjdk/jdk/pull/2200/commits/f6fb01b24f525e578692a1c6f2ff0a55b8233576is
ThreadWXEnable allows optional W^X state change, like `MutexLocker` allows
optional locking.
-------------
PR: https://git.openjdk.java.net/jdk/pull/2200
