[OpenJDK 2D-Dev] RFR: 8253795: Implementation of JEP 391: macOS/AArch64 Port [v25]

[Anton Kozlov]

On Fri, 12 Mar 2021 05:24:10 GMT, David Holmes <dholmes at 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.

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