On Tue, 9 Feb 2021 09:23:50 GMT, Stefan Karlsson <[email protected]> wrote:
>> Anton Kozlov has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Update signal handler part for debugger
>
> src/hotspot/share/runtime/thread.cpp line 2515:
>
>> 2513: void JavaThread::check_special_condition_for_native_trans(JavaThread
>> *thread) {
>> 2514: // Enable WXWrite: called directly from interpreter native wrapper.
>> 2515: MACOS_AARCH64_ONLY(ThreadWXEnable wx(WXWrite, thread));
>
> FWIW, I personally think that adding these MACOS_AARCH64_ONLY usages at the
> call sites increase the line-noise in the affected functions. I think I would
> have preferred a version:
> ThreadWXEnable(WXMode new_mode, Thread* thread = NULL) {
> MACOS_AARCH64_ONLY(initialize(new_mode, thread);) {}
> void initialize(...); // Implementation in thread_bsd_aarch64.cpp (alt.
> inline.hpp)
> With that said, I'm fine with taking this discussion as a follow-up.
The former version used no such macros. I like that now it's clear the W^X
management is relevant to macos/aarch64 only. I see the point to move the
pre-processor condition into the class implementation. But I think it will
bring a bit of inconsistency, as the rest of W^X implementation is explicitly
guarded by preprocessor conditionals. I've also tried to push macro
conditionals as far as possible down to implementation, providing a kind of
generalized W^X interface. That required a few artificial decisions, e.g. how
would we call the mode we execute on the rest of platforms with write and
execute allowed, WXWriteExec?.. I abandoned that attempt.
-------------
PR: https://git.openjdk.java.net/jdk/pull/2200
