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.

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

Reply via email to