On Tue, 2 Feb 2021 23:03:45 GMT, Daniel D. Daugherty <dcu...@openjdk.org> wrote:
>> Anton Kozlov has updated the pull request incrementally with one additional >> commit since the last revision: >> >> support macos_aarch64 in hsdis > > src/hotspot/share/runtime/thread.cpp line 3991: > >> 3989: JavaThread* thread = JavaThread::current(); >> 3990: ThreadToNativeFromVM ttn(thread); >> 3991: Thread::WXExecFromWriteSetter wx_exec; > > I saw somewhere in this review a comment about why this new > WXExecFromWriteSetter helper isn't folded into ThreadToNativeFromVM > and I understand that not every current ThreadToNativeFromVM needs > the new helper. If the vast majority of the ThreadToNativeFromVM > locations need WXExecFromWriteSetter, then perhaps those locations > that currently have a ThreadToNativeFromVM followed by the new > WXExecFromWriteSetter should use a new helper: > > ThreadToNativeWithWXExecFromVM > > so we'll see changes from: > > ThreadToNativeFromVM -> ThreadToNativeWithWXExecFromVM > > where we need them and hopefully a short comment can be added > at the same time to explain the need for WXExec. This will allow us > to easily distinguish ThreadToNativeFromVM locations that DO NOT > need WXExec from those that DO need it. With a small overhead for tracking the current W^X state, I avoided WXExecFromWriteSetter near ThreadToNativeFromVM at all. New ThreadWXEnable(WXExec) is used only to call a generated function. More common ThreadWXEnable(WXWrite) is tied to JVM entry functions. I added comments for functions that are not clear to be entries, although they are. Thank you for the suggestion! ------------- PR: https://git.openjdk.java.net/jdk/pull/2200
