RFR: 8262894: [macos_aarch64] SIGBUS in Assembler::ld_st2

[David Holmes]

On Mon, 29 Mar 2021 11:39:31 GMT, Anton Kozlov <akozlov at openjdk.org> wrote:

> Please review a fix for compiler/debug/VerifyAdapterSharing.java failure on macos/aarch64 platform. The root cause is in missing W^X switch in JNI DestroyJavaVM. 
> 
> I reviewed the rest of the JNI Invoke Interface functions. DetachCurrentThread needs a similar change, although nothing fails immediately. So DetachCurrentThread is changed as a precaution.

Hi Anton,

In so much as I understand the fact transitions are missing the introduction of those transitions seems fine - but can be simplified due to an existing code quirk (see comments below).

My main concern with this W^X stuff is that I don't see a clear way to know exactly where a transition needs to be placed. The missing cases here suggest it should be handled in the thread-state transition code, but you've previously written:

" when we execute JVM code (owned by libjvm.so, starting from JVM entry function), we switch to Write state. When we leave JVM to execute generated or JNI code, we switch to Executable state. I would like to highlight that JVM code does not mean the VM state of the java thread"

so I'm unclear exactly how we identify the points where these transitions must occur? What kind of "VM code" must be guarded this way? I don't see this documented in the code anywhere.

Thanks,
David

src/hotspot/share/prims/jni.cpp line 3728:

> 3726: 
> 3727:   // We are going to VM, change W^X state to the expected one.
> 3728:   MACOS_AARCH64_ONLY(WXMode oldmode = thread->enable_wx(WXWrite));

No need to save old state - see below.

src/hotspot/share/prims/jni.cpp line 3738:

> 3736:   } else {
> 3737:     ThreadStateTransition::transition(thread, _thread_in_vm, _thread_in_native);
> 3738:     MACOS_AARCH64_ONLY(thread->enable_wx(oldmode));

This is actually unnecessary as Threads::destroy_vm never returns anything but true (we should change it to a void method and clean this up). If it were to fail you would have to know at what point it failed to determine whether you can actually touch the current thread any more.

-------------

Marked as reviewed by dholmes (Reviewer).

PR: https://git.openjdk.java.net/jdk/pull/3241