On Tue, 9 Feb 2021 09:23:50 GMT, Stefan Karlsson <stef...@openjdk.org> 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