RFR: 8265292: [macos_aarch64] java/foreign/TestDowncall.java crashes with SIGBUS

[Andrew Haley]

On Fri, 7 May 2021 15:38:24 GMT, Anton Kozlov <akozlov at openjdk.org> wrote:

> Please review a fix for the intermittent crash. It is caused by a mistake in the ProgrammableInvoker::invoke_native, the wrong order of W^X and JavaThread state transition. We need WXExec since we are about to call a generated stub. But we need to switch to WXExec only after JavaThread state change. The thread state change may trigger a safepoint, that would need to do bookkeeping in the codecache (MarkActivationClosure::do_code_blob from the bug). So the fix is to change JavaThread state first, then change WX.
> 
> The fix was verified with the help of https://bugs.openjdk.java.net/browse/JDK-8266742. The new check catches all test failures reported by 8265292, 8265183, 8265182. I've verified tests pass after the fix with that new check enabled.

src/hotspot/share/prims/universalNativeInvoker.cpp line 35:

> 33: void ProgrammableInvoker::invoke_native(Stub stub, address buff, JavaThread* thread) {
> 34:   ThreadToNativeFromVM ttnfvm(thread);
> 35:   MACOS_AARCH64_ONLY(ThreadWXEnable wx(WXExec, thread));

I think we need a comment here, not just in the bug report. if this confused you, it'll surely be enough to confuse a maintainer. Some think like this, perhaps?

`"We need WXExec because we are about to call a generated stub. But we need to switch to WXExec only after JavaThread state change. The thread state change may trigger a safepoint, that would need to do bookkeeping in the codecache. See JDK-8265292."`

I'm also wondering if it would be better to enable writes in the methods that actually write to the nmethod, WDYT?

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

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