On 12/03/2021 5:12 pm, Anton Kozlov wrote:
On Fri, 12 Mar 2021 05:24:10 GMT, David Holmes <dhol...@openjdk.org> 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.
The NULL check is only part of it. In a signal handling context
Thread::current() is not safe to call, you have to use
Thread::current_or_null_safe().
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.
Okay but you still need to use Thread::current_or_null_safe().
Cheers,
David
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
