Re: RFR: 8253795: Implementation of JEP 391: macOS/AArch64 Port [v10]
On Thu, 4 Feb 2021 22:58:39 GMT, Gerard Ziemski wrote: >> Anton Kozlov has updated the pull request incrementally with six additional >> commits since the last revision: >> >> - Merge remote-tracking branch 'origin/jdk/jdk-macos' into jdk-macos >> - Add comments to WX transitions >> >>+ minor change of placements >> - Use macro conditionals instead of empty functions >> - Add W^X to tests >> - Do not require known W^X state >> - Revert w^x in gtests > > src/hotspot/os_cpu/bsd_aarch64/os_bsd_aarch64.cpp line 420: > >> 418: size_t os::Posix::_compiler_thread_min_stack_allowed = 72 * K; >> 419: size_t os::Posix::_java_thread_min_stack_allowed = 72 * K; >> 420: size_t os::Posix::_vm_internal_thread_min_stack_allowed = 72 * K; > > Those are slightly larger than their x86_64 counter parts. Are they > conservative/aggressive values? How did we arrive at those? These values were copied from linux_aarch64. The motivation is that clang on macos/aarch64 will likely to produce stack frames for C++ functions similar to frames generates by gcc on linux/aarch64. And sizes of java stack frames should not change. - PR: https://git.openjdk.java.net/jdk/pull/2200
Re: RFR: 8253795: Implementation of JEP 391: macOS/AArch64 Port [v10]
On Fri, 12 Feb 2021 15:21:06 GMT, Vladimir Kempik wrote: >> src/hotspot/os_cpu/bsd_aarch64/os_bsd_aarch64.cpp line 302: >> >>> 300: const uint64_t *detail_msg_ptr >>> 301: = (uint64_t*)(pc + NativeInstruction::instruction_size); >>> 302: const char *detail_msg = (const char *)*detail_msg_ptr; >> >> Where is `detail_msg` used? > > Came from linux_arm64. was used in os_linux_aarch64.cpp on line 246 in > report_and_die > But became unused on bsd_arm64. I agree this needs to be removed It seems we have merged master branch before JDK-8259539 was integrated. It brings back use of detail_msg. I reverted details_msg as well its following use. - PR: https://git.openjdk.java.net/jdk/pull/2200
Re: RFR: 8253795: Implementation of JEP 391: macOS/AArch64 Port [v10]
On Thu, 4 Feb 2021 22:34:16 GMT, Gerard Ziemski wrote: >> Anton Kozlov has updated the pull request incrementally with six additional >> commits since the last revision: >> >> - Merge remote-tracking branch 'origin/jdk/jdk-macos' into jdk-macos >> - Add comments to WX transitions >> >>+ minor change of placements >> - Use macro conditionals instead of empty functions >> - Add W^X to tests >> - Do not require known W^X state >> - Revert w^x in gtests > > src/hotspot/os_cpu/bsd_aarch64/os_bsd_aarch64.cpp line 237: > >> 235: os::Posix::ucontext_set_pc(uc, >> StubRoutines::continuation_for_safefetch_fault(pc)); >> 236: return true; >> 237: } > > Isn't this case already handled by `JVM_HANDLE_XXX_SIGNAL()` ? Why do we need > it here again? Good point, thanks. We are missing a few fixes in os_cpu/bsd_aarch64. This was moved out recently. I'm going to align bsd_aarch64 with the rest of platforms. - PR: https://git.openjdk.java.net/jdk/pull/2200
Re: RFR: 8253795: Implementation of JEP 391: macOS/AArch64 Port [v10]
On Thu, 4 Feb 2021 22:15:47 GMT, Gerard Ziemski wrote:
>> Anton Kozlov has updated the pull request incrementally with six additional
>> commits since the last revision:
>>
>> - Merge remote-tracking branch 'origin/jdk/jdk-macos' into jdk-macos
>> - Add comments to WX transitions
>>
>>+ minor change of placements
>> - Use macro conditionals instead of empty functions
>> - Add W^X to tests
>> - Do not require known W^X state
>> - Revert w^x in gtests
>
> src/hotspot/os_cpu/bsd_aarch64/os_bsd_aarch64.cpp line 322:
>
>> 320: #ifdef __APPLE__
>> 321: } else if (sig == SIGFPE && info->si_code == FPE_NOOP) {
>> 322: Unimplemented();
>
> Is there a follow up issue for this?
Thanks, this is a leftover from the development phase, it will be removed. In
macos/x86, this looks like a workaround. We've never met with this condition
and it looks recent darwin kernel should correctly report the cause in si_code:
https://github.com/apple/darwin-xnu/blob/33eb9835cd948dbbcdd8741aa52457cbe507c765/bsd/dev/arm/unix_signal.c#L436.
-
PR: https://git.openjdk.java.net/jdk/pull/2200
Re: RFR: 8253795: Implementation of JEP 391: macOS/AArch64 Port [v10]
On Fri, 12 Feb 2021 11:42:59 GMT, Vladimir Kempik wrote:
>> src/hotspot/os_cpu/bsd_aarch64/os_bsd_aarch64.cpp line 194:
>>
>>> 192: // may get turned off by -fomit-frame-pointer.
>>> 193: frame os::get_sender_for_C_frame(frame* fr) {
>>> 194: return frame(fr->link(), fr->link(), fr->sender_pc());
>>
>> Why is it
>>
>> return frame(fr->link(), fr->link(), fr->sender_pc());
>>
>> and not
>>
>> return frame(fr->sender_sp(), fr->link(), fr->sender_pc());
>>
>> like in the bsd-x86 counter part?
>
> bsd_aarcb64 was based on linux_aarch64, with addition of bsd-specific things
> from bsd_x86
> You think the bsd-x86 way is better here ?
There's no point copying x86. We don't have any way to know what the sender's
SP was in a C frame without using unwind info. I think this is just used when
trying to print the stack in a crash dump.
-
PR: https://git.openjdk.java.net/jdk/pull/2200
Re: RFR: 8253795: Implementation of JEP 391: macOS/AArch64 Port [v10]
On Thu, 4 Feb 2021 23:01:52 GMT, Gerard Ziemski wrote:
>> Anton Kozlov has updated the pull request incrementally with six additional
>> commits since the last revision:
>>
>> - Merge remote-tracking branch 'origin/jdk/jdk-macos' into jdk-macos
>> - Add comments to WX transitions
>>
>>+ minor change of placements
>> - Use macro conditionals instead of empty functions
>> - Add W^X to tests
>> - Do not require known W^X state
>> - Revert w^x in gtests
>
> src/hotspot/os_cpu/bsd_aarch64/os_bsd_aarch64.cpp line 652:
>
>> 650:
>> 651: void os::setup_fpu() {
>> 652: }
>
> Is there really nothing to do here, or does still need to be implemented? A
> clarification comment here would help/.
There is really nothing to do here.
> src/hotspot/os_cpu/bsd_aarch64/os_bsd_aarch64.cpp line 198:
>
>> 196:
>> 197: NOINLINE frame os::current_frame() {
>> 198: intptr_t *fp = *(intptr_t **)__builtin_frame_address(0);
>
> In the bsd_x86 counter part we initialize `fp` to `_get_previous_fp()` - do
> we need to implement it on aarch64 as well (and using address 0 is just a
> temp workaround) or is it doing the right thing here?
(0)``` looks right to me.
> src/hotspot/os_cpu/bsd_aarch64/os_bsd_aarch64.cpp line 291:
>
>> 289: bool is_unsafe_arraycopy = (thread->doing_unsafe_access() &&
>> UnsafeCopyMemory::contains_pc(pc));
>> 290: if ((nm != NULL && nm->has_unsafe_access()) ||
>> is_unsafe_arraycopy) {
>> 291: address next_pc = pc + NativeCall::instruction_size;
>
> Replace
>
> address next_pc = pc + NativeCall::instruction_size;
>
> with
>
> address next_pc = Assembler::locate_next_instruction(pc);
>
> there is at least one other place that needs it.
Why is this change needed? AFAICS ```locate_next_instruction()``` is an x86
thing for variable-length instructions, and no other port uses it.
-
PR: https://git.openjdk.java.net/jdk/pull/2200
Re: RFR: 8253795: Implementation of JEP 391: macOS/AArch64 Port [v10]
On Thu, 4 Feb 2021 22:49:23 GMT, Gerard Ziemski wrote:
>> Anton Kozlov has updated the pull request incrementally with six additional
>> commits since the last revision:
>>
>> - Merge remote-tracking branch 'origin/jdk/jdk-macos' into jdk-macos
>> - Add comments to WX transitions
>>
>>+ minor change of placements
>> - Use macro conditionals instead of empty functions
>> - Add W^X to tests
>> - Do not require known W^X state
>> - Revert w^x in gtests
>
> src/hotspot/os_cpu/bsd_aarch64/os_bsd_aarch64.cpp line 297:
>
>> 295: stub = SharedRuntime::handle_unsafe_access(thread, next_pc);
>> 296: }
>> 297: } else if (sig == SIGILL && nativeInstruction_at(pc)->is_stop()) {
>
> Can we add a comment here describing what this case means?
This was added as part of this commit ( to linux_aarch64) -
https://github.com/openjdk/jdk/commit/339d52600b285eb3bc57d9ff107567d4424efeb1
@gerard-ziemski do we really want to add anything new here ?
-
PR: https://git.openjdk.java.net/jdk/pull/2200
Re: RFR: 8253795: Implementation of JEP 391: macOS/AArch64 Port [v10]
On Thu, 4 Feb 2021 22:49:23 GMT, Gerard Ziemski wrote:
>> Anton Kozlov has updated the pull request incrementally with six additional
>> commits since the last revision:
>>
>> - Merge remote-tracking branch 'origin/jdk/jdk-macos' into jdk-macos
>> - Add comments to WX transitions
>>
>>+ minor change of placements
>> - Use macro conditionals instead of empty functions
>> - Add W^X to tests
>> - Do not require known W^X state
>> - Revert w^x in gtests
>
> src/hotspot/os_cpu/bsd_aarch64/os_bsd_aarch64.cpp line 297:
>
>> 295: stub = SharedRuntime::handle_unsafe_access(thread, next_pc);
>> 296: }
>> 297: } else if (sig == SIGILL && nativeInstruction_at(pc)->is_stop()) {
>
> Can we add a comment here describing what this case means?
this arm64 specific part came as is from linux_aarch64 and I can't add any
meaning comments here.
> src/hotspot/os_cpu/bsd_aarch64/os_bsd_aarch64.cpp line 302:
>
>> 300: const uint64_t *detail_msg_ptr
>> 301: = (uint64_t*)(pc + NativeInstruction::instruction_size);
>> 302: const char *detail_msg = (const char *)*detail_msg_ptr;
>
> Where is `detail_msg` used?
Came from linux_arm64. was used in os_linux_aarch64.cpp on line 246 in
report_and_die
But became unused on bsd_arm64. I agree this needs to be removed
> src/hotspot/os_cpu/bsd_aarch64/os_bsd_aarch64.cpp line 403:
>
>> 401: }
>> 402:
>> 403: return false; // Mute compiler
>
> Is this comment needed?
this part came as is from linux_aarch64 as well and was supposed to mean
something there.
-
PR: https://git.openjdk.java.net/jdk/pull/2200
Re: RFR: 8253795: Implementation of JEP 391: macOS/AArch64 Port [v10]
On Thu, 4 Feb 2021 21:59:02 GMT, Gerard Ziemski wrote:
>> Anton Kozlov has updated the pull request incrementally with six additional
>> commits since the last revision:
>>
>> - Merge remote-tracking branch 'origin/jdk/jdk-macos' into jdk-macos
>> - Add comments to WX transitions
>>
>>+ minor change of placements
>> - Use macro conditionals instead of empty functions
>> - Add W^X to tests
>> - Do not require known W^X state
>> - Revert w^x in gtests
>
> src/hotspot/os_cpu/bsd_aarch64/os_bsd_aarch64.cpp line 194:
>
>> 192: // may get turned off by -fomit-frame-pointer.
>> 193: frame os::get_sender_for_C_frame(frame* fr) {
>> 194: return frame(fr->link(), fr->link(), fr->sender_pc());
>
> Why is it
>
> return frame(fr->link(), fr->link(), fr->sender_pc());
>
> and not
>
> return frame(fr->sender_sp(), fr->link(), fr->sender_pc());
>
> like in the bsd-x86 counter part?
bsd_aarcb64 was based on linux_aarch64, with addition of bsd-specific things
from bsd_x86
You think the bsd-x86 is better here ?
-
PR: https://git.openjdk.java.net/jdk/pull/2200
Re: RFR: 8253795: Implementation of JEP 391: macOS/AArch64 Port [v10]
On Fri, 5 Feb 2021 12:26:27 GMT, Anton Kozlov wrote: >> Marked as reviewed by ihse (Reviewer). > >> I haven't got a MacOS AArch64 system right now. Is it possible to >> enable W^X in Linux in order to kick the tyres? > > I've just got rid of asserts that fired on Linux sometime :) As for W^X like > on macOS, I vaguely remember working with a Linux system with one-way > transition W->X, probably provided by SELinux. But I don't think it allowed > per-thread W^X state. > _Mailing list message from [daniel.daugherty at > oracle.com](mailto:daniel.daughe...@oracle.com) on > [security-dev](mailto:security-...@openjdk.java.net):_ > > On 2/5/21 4:51 AM, Magnus Ihse Bursie wrote: > > @magicus - I'm good with both of these answers. I personally like 'macosx'. > > Dan It's no longer `macosx`, it's just `macos` now - see https://en.wikipedia.org/wiki/MacOS - PR: https://git.openjdk.java.net/jdk/pull/2200
Re: RFR: 8253795: Implementation of JEP 391: macOS/AArch64 Port [v10]
On Fri, 5 Feb 2021 09:57:54 GMT, Magnus Ihse Bursie wrote: >> Anton Kozlov has updated the pull request incrementally with six additional >> commits since the last revision: >> >> - Merge remote-tracking branch 'origin/jdk/jdk-macos' into jdk-macos >> - Add comments to WX transitions >> >>+ minor change of placements >> - Use macro conditionals instead of empty functions >> - Add W^X to tests >> - Do not require known W^X state >> - Revert w^x in gtests > > Marked as reviewed by ihse (Reviewer). > I haven't got a MacOS AArch64 system right now. Is it possible to > enable W^X in Linux in order to kick the tyres? I've just got rid of asserts that fired on Linux sometime :) As for W^X like on macOS, I vaguely remember working with a Linux system with one-way transition W->X, probably provided by SELinux. But I don't think it allowed per-thread W^X state. - PR: https://git.openjdk.java.net/jdk/pull/2200
Re: RFR: 8253795: Implementation of JEP 391: macOS/AArch64 Port [v10]
On Fri, 5 Feb 2021 09:49:11 GMT, Andrew Haley wrote: >> I reviewed bsd_aarch64.cpp digging bit deeper and left some comments. > >> > Umm, so how does patching work? We don't even know if other threads are >> > executing the code we need to patch. >> >> I thought java can handle that scenario in usual (non W^X systems) just >> fine, so we just believe jvm did everything right and it's safe to rewrite >> some code at specific moment. > > Got it, OK. > This doesn't seem to be an issue anymore, After P.Race have finished with > JDK-8257852, Macarm port can be build without extra ld flags. @VladimirKempik I agree. That concludes the build issues with this PR. So from a build perspective, this is now good to go. - PR: https://git.openjdk.java.net/jdk/pull/2200
Re: RFR: 8253795: Implementation of JEP 391: macOS/AArch64 Port [v10]
On Wed, 3 Feb 2021 20:01:15 GMT, Anton Kozlov wrote: >> Please review the implementation of JEP 391: macOS/AArch64 Port. >> >> It's heavily based on existing ports to linux/aarch64, macos/x86_64, and >> windows/aarch64. >> >> Major changes are in: >> * src/hotspot/cpu/aarch64: support of the new calling convention (subtasks >> JDK-8253817, JDK-8253818) >> * src/hotspot/os_cpu/bsd_aarch64: copy of os_cpu/linux_aarch64 with >> necessary adjustments (JDK-8253819) >> * src/hotspot/share, test/hotspot/gtest: support of write-xor-execute (W^X), >> required on macOS/AArch64 platform. It's implemented with >> pthread_jit_write_protect_np provided by Apple. The W^X mode is local to a >> thread, so W^X mode change relates to the java thread state change (for java >> threads). In most cases, JVM executes in write-only mode, except when >> calling a generated stub like SafeFetch, which requires a temporary switch >> to execute-only mode. The same execute-only mode is enabled when a java >> thread executes in java or native states. This approach of managing W^X mode >> turned out to be simple and efficient enough. >> * src/jdk.hotspot.agent: serviceability agent implementation (JDK-8254941) > > Anton Kozlov has updated the pull request incrementally with six additional > commits since the last revision: > > - Merge remote-tracking branch 'origin/jdk/jdk-macos' into jdk-macos > - Add comments to WX transitions > >+ minor change of placements > - Use macro conditionals instead of empty functions > - Add W^X to tests > - Do not require known W^X state > - Revert w^x in gtests Marked as reviewed by ihse (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/2200
Re: RFR: 8253795: Implementation of JEP 391: macOS/AArch64 Port [v10]
On Thu, 4 Feb 2021 23:05:56 GMT, Gerard Ziemski wrote: >> Anton Kozlov has updated the pull request incrementally with six additional >> commits since the last revision: >> >> - Merge remote-tracking branch 'origin/jdk/jdk-macos' into jdk-macos >> - Add comments to WX transitions >> >>+ minor change of placements >> - Use macro conditionals instead of empty functions >> - Add W^X to tests >> - Do not require known W^X state >> - Revert w^x in gtests > > I reviewed bsd_aarch64.cpp digging bit deeper and left some comments. > > Umm, so how does patching work? We don't even know if other threads are > > executing the code we need to patch. > > I thought java can handle that scenario in usual (non W^X systems) just fine, > so we just believe jvm did everything right and it's safe to rewrite some > code at specific moment. Got it, OK. - PR: https://git.openjdk.java.net/jdk/pull/2200
Re: RFR: 8253795: Implementation of JEP 391: macOS/AArch64 Port [v10]
On Wed, 3 Feb 2021 20:01:15 GMT, Anton Kozlov wrote:
>> Please review the implementation of JEP 391: macOS/AArch64 Port.
>>
>> It's heavily based on existing ports to linux/aarch64, macos/x86_64, and
>> windows/aarch64.
>>
>> Major changes are in:
>> * src/hotspot/cpu/aarch64: support of the new calling convention (subtasks
>> JDK-8253817, JDK-8253818)
>> * src/hotspot/os_cpu/bsd_aarch64: copy of os_cpu/linux_aarch64 with
>> necessary adjustments (JDK-8253819)
>> * src/hotspot/share, test/hotspot/gtest: support of write-xor-execute (W^X),
>> required on macOS/AArch64 platform. It's implemented with
>> pthread_jit_write_protect_np provided by Apple. The W^X mode is local to a
>> thread, so W^X mode change relates to the java thread state change (for java
>> threads). In most cases, JVM executes in write-only mode, except when
>> calling a generated stub like SafeFetch, which requires a temporary switch
>> to execute-only mode. The same execute-only mode is enabled when a java
>> thread executes in java or native states. This approach of managing W^X mode
>> turned out to be simple and efficient enough.
>> * src/jdk.hotspot.agent: serviceability agent implementation (JDK-8254941)
>
> Anton Kozlov has updated the pull request incrementally with six additional
> commits since the last revision:
>
> - Merge remote-tracking branch 'origin/jdk/jdk-macos' into jdk-macos
> - Add comments to WX transitions
>
>+ minor change of placements
> - Use macro conditionals instead of empty functions
> - Add W^X to tests
> - Do not require known W^X state
> - Revert w^x in gtests
src/hotspot/os_cpu/bsd_aarch64/os_bsd_aarch64.cpp line 403:
> 401: }
> 402:
> 403: return false; // Mute compiler
Is this comment needed?
src/hotspot/os_cpu/bsd_aarch64/os_bsd_aarch64.cpp line 420:
> 418: size_t os::Posix::_compiler_thread_min_stack_allowed = 72 * K;
> 419: size_t os::Posix::_java_thread_min_stack_allowed = 72 * K;
> 420: size_t os::Posix::_vm_internal_thread_min_stack_allowed = 72 * K;
Those are slightly larger than their x86_64 counter parts. Are they
conservative/aggressive values? How did we arrive at those?
src/hotspot/os_cpu/bsd_aarch64/os_bsd_aarch64.cpp line 652:
> 650:
> 651: void os::setup_fpu() {
> 652: }
Is there really nothing to do here, or does still need to be implemented? A
clarification comment here would help/.
-
PR: https://git.openjdk.java.net/jdk/pull/2200
Re: RFR: 8253795: Implementation of JEP 391: macOS/AArch64 Port [v10]
On Wed, 3 Feb 2021 20:01:15 GMT, Anton Kozlov wrote: >> Please review the implementation of JEP 391: macOS/AArch64 Port. >> >> It's heavily based on existing ports to linux/aarch64, macos/x86_64, and >> windows/aarch64. >> >> Major changes are in: >> * src/hotspot/cpu/aarch64: support of the new calling convention (subtasks >> JDK-8253817, JDK-8253818) >> * src/hotspot/os_cpu/bsd_aarch64: copy of os_cpu/linux_aarch64 with >> necessary adjustments (JDK-8253819) >> * src/hotspot/share, test/hotspot/gtest: support of write-xor-execute (W^X), >> required on macOS/AArch64 platform. It's implemented with >> pthread_jit_write_protect_np provided by Apple. The W^X mode is local to a >> thread, so W^X mode change relates to the java thread state change (for java >> threads). In most cases, JVM executes in write-only mode, except when >> calling a generated stub like SafeFetch, which requires a temporary switch >> to execute-only mode. The same execute-only mode is enabled when a java >> thread executes in java or native states. This approach of managing W^X mode >> turned out to be simple and efficient enough. >> * src/jdk.hotspot.agent: serviceability agent implementation (JDK-8254941) > > Anton Kozlov has updated the pull request incrementally with six additional > commits since the last revision: > > - Merge remote-tracking branch 'origin/jdk/jdk-macos' into jdk-macos > - Add comments to WX transitions > >+ minor change of placements > - Use macro conditionals instead of empty functions > - Add W^X to tests > - Do not require known W^X state > - Revert w^x in gtests I reviewed bsd_aarch64.cpp digging bit deeper and left some comments. - Changes requested by gziemski (Committer). PR: https://git.openjdk.java.net/jdk/pull/2200
Re: RFR: 8253795: Implementation of JEP 391: macOS/AArch64 Port [v10]
On Wed, 3 Feb 2021 20:01:15 GMT, Anton Kozlov wrote:
>> Please review the implementation of JEP 391: macOS/AArch64 Port.
>>
>> It's heavily based on existing ports to linux/aarch64, macos/x86_64, and
>> windows/aarch64.
>>
>> Major changes are in:
>> * src/hotspot/cpu/aarch64: support of the new calling convention (subtasks
>> JDK-8253817, JDK-8253818)
>> * src/hotspot/os_cpu/bsd_aarch64: copy of os_cpu/linux_aarch64 with
>> necessary adjustments (JDK-8253819)
>> * src/hotspot/share, test/hotspot/gtest: support of write-xor-execute (W^X),
>> required on macOS/AArch64 platform. It's implemented with
>> pthread_jit_write_protect_np provided by Apple. The W^X mode is local to a
>> thread, so W^X mode change relates to the java thread state change (for java
>> threads). In most cases, JVM executes in write-only mode, except when
>> calling a generated stub like SafeFetch, which requires a temporary switch
>> to execute-only mode. The same execute-only mode is enabled when a java
>> thread executes in java or native states. This approach of managing W^X mode
>> turned out to be simple and efficient enough.
>> * src/jdk.hotspot.agent: serviceability agent implementation (JDK-8254941)
>
> Anton Kozlov has updated the pull request incrementally with six additional
> commits since the last revision:
>
> - Merge remote-tracking branch 'origin/jdk/jdk-macos' into jdk-macos
> - Add comments to WX transitions
>
>+ minor change of placements
> - Use macro conditionals instead of empty functions
> - Add W^X to tests
> - Do not require known W^X state
> - Revert w^x in gtests
src/hotspot/os_cpu/bsd_aarch64/os_bsd_aarch64.cpp line 297:
> 295: stub = SharedRuntime::handle_unsafe_access(thread, next_pc);
> 296: }
> 297: } else if (sig == SIGILL && nativeInstruction_at(pc)->is_stop()) {
Can we add a comment here describing what this case means?
-
PR: https://git.openjdk.java.net/jdk/pull/2200
Re: RFR: 8253795: Implementation of JEP 391: macOS/AArch64 Port [v10]
On Wed, 3 Feb 2021 20:01:15 GMT, Anton Kozlov wrote: >> Please review the implementation of JEP 391: macOS/AArch64 Port. >> >> It's heavily based on existing ports to linux/aarch64, macos/x86_64, and >> windows/aarch64. >> >> Major changes are in: >> * src/hotspot/cpu/aarch64: support of the new calling convention (subtasks >> JDK-8253817, JDK-8253818) >> * src/hotspot/os_cpu/bsd_aarch64: copy of os_cpu/linux_aarch64 with >> necessary adjustments (JDK-8253819) >> * src/hotspot/share, test/hotspot/gtest: support of write-xor-execute (W^X), >> required on macOS/AArch64 platform. It's implemented with >> pthread_jit_write_protect_np provided by Apple. The W^X mode is local to a >> thread, so W^X mode change relates to the java thread state change (for java >> threads). In most cases, JVM executes in write-only mode, except when >> calling a generated stub like SafeFetch, which requires a temporary switch >> to execute-only mode. The same execute-only mode is enabled when a java >> thread executes in java or native states. This approach of managing W^X mode >> turned out to be simple and efficient enough. >> * src/jdk.hotspot.agent: serviceability agent implementation (JDK-8254941) > > Anton Kozlov has updated the pull request incrementally with six additional > commits since the last revision: > > - Merge remote-tracking branch 'origin/jdk/jdk-macos' into jdk-macos > - Add comments to WX transitions > >+ minor change of placements > - Use macro conditionals instead of empty functions > - Add W^X to tests > - Do not require known W^X state > - Revert w^x in gtests src/hotspot/os_cpu/bsd_aarch64/os_bsd_aarch64.cpp line 302: > 300: const uint64_t *detail_msg_ptr > 301: = (uint64_t*)(pc + NativeInstruction::instruction_size); > 302: const char *detail_msg = (const char *)*detail_msg_ptr; Where is `detail_msg` used? - PR: https://git.openjdk.java.net/jdk/pull/2200
Re: RFR: 8253795: Implementation of JEP 391: macOS/AArch64 Port [v10]
On Wed, 3 Feb 2021 20:01:15 GMT, Anton Kozlov wrote: >> Please review the implementation of JEP 391: macOS/AArch64 Port. >> >> It's heavily based on existing ports to linux/aarch64, macos/x86_64, and >> windows/aarch64. >> >> Major changes are in: >> * src/hotspot/cpu/aarch64: support of the new calling convention (subtasks >> JDK-8253817, JDK-8253818) >> * src/hotspot/os_cpu/bsd_aarch64: copy of os_cpu/linux_aarch64 with >> necessary adjustments (JDK-8253819) >> * src/hotspot/share, test/hotspot/gtest: support of write-xor-execute (W^X), >> required on macOS/AArch64 platform. It's implemented with >> pthread_jit_write_protect_np provided by Apple. The W^X mode is local to a >> thread, so W^X mode change relates to the java thread state change (for java >> threads). In most cases, JVM executes in write-only mode, except when >> calling a generated stub like SafeFetch, which requires a temporary switch >> to execute-only mode. The same execute-only mode is enabled when a java >> thread executes in java or native states. This approach of managing W^X mode >> turned out to be simple and efficient enough. >> * src/jdk.hotspot.agent: serviceability agent implementation (JDK-8254941) > > Anton Kozlov has updated the pull request incrementally with six additional > commits since the last revision: > > - Merge remote-tracking branch 'origin/jdk/jdk-macos' into jdk-macos > - Add comments to WX transitions > >+ minor change of placements > - Use macro conditionals instead of empty functions > - Add W^X to tests > - Do not require known W^X state > - Revert w^x in gtests src/hotspot/os_cpu/bsd_aarch64/os_bsd_aarch64.cpp line 237: > 235: os::Posix::ucontext_set_pc(uc, > StubRoutines::continuation_for_safefetch_fault(pc)); > 236: return true; > 237: } Isn't this case already handled by `JVM_HANDLE_XXX_SIGNAL()` ? Why do we need it here again? - PR: https://git.openjdk.java.net/jdk/pull/2200
Re: RFR: 8253795: Implementation of JEP 391: macOS/AArch64 Port [v10]
On Wed, 3 Feb 2021 20:01:15 GMT, Anton Kozlov wrote:
>> Please review the implementation of JEP 391: macOS/AArch64 Port.
>>
>> It's heavily based on existing ports to linux/aarch64, macos/x86_64, and
>> windows/aarch64.
>>
>> Major changes are in:
>> * src/hotspot/cpu/aarch64: support of the new calling convention (subtasks
>> JDK-8253817, JDK-8253818)
>> * src/hotspot/os_cpu/bsd_aarch64: copy of os_cpu/linux_aarch64 with
>> necessary adjustments (JDK-8253819)
>> * src/hotspot/share, test/hotspot/gtest: support of write-xor-execute (W^X),
>> required on macOS/AArch64 platform. It's implemented with
>> pthread_jit_write_protect_np provided by Apple. The W^X mode is local to a
>> thread, so W^X mode change relates to the java thread state change (for java
>> threads). In most cases, JVM executes in write-only mode, except when
>> calling a generated stub like SafeFetch, which requires a temporary switch
>> to execute-only mode. The same execute-only mode is enabled when a java
>> thread executes in java or native states. This approach of managing W^X mode
>> turned out to be simple and efficient enough.
>> * src/jdk.hotspot.agent: serviceability agent implementation (JDK-8254941)
>
> Anton Kozlov has updated the pull request incrementally with six additional
> commits since the last revision:
>
> - Merge remote-tracking branch 'origin/jdk/jdk-macos' into jdk-macos
> - Add comments to WX transitions
>
>+ minor change of placements
> - Use macro conditionals instead of empty functions
> - Add W^X to tests
> - Do not require known W^X state
> - Revert w^x in gtests
src/hotspot/os_cpu/bsd_aarch64/os_bsd_aarch64.cpp line 198:
> 196:
> 197: NOINLINE frame os::current_frame() {
> 198: intptr_t *fp = *(intptr_t **)__builtin_frame_address(0);
In the bsd_x86 counter part we initialize `fp` to `_get_previous_fp()` - do we
need to implement it on aarch64 as well (and using address 0 is just a temp
workaround) or is it doing the right thing here?
-
PR: https://git.openjdk.java.net/jdk/pull/2200
Re: RFR: 8253795: Implementation of JEP 391: macOS/AArch64 Port [v10]
On Wed, 3 Feb 2021 20:01:15 GMT, Anton Kozlov wrote:
>> Please review the implementation of JEP 391: macOS/AArch64 Port.
>>
>> It's heavily based on existing ports to linux/aarch64, macos/x86_64, and
>> windows/aarch64.
>>
>> Major changes are in:
>> * src/hotspot/cpu/aarch64: support of the new calling convention (subtasks
>> JDK-8253817, JDK-8253818)
>> * src/hotspot/os_cpu/bsd_aarch64: copy of os_cpu/linux_aarch64 with
>> necessary adjustments (JDK-8253819)
>> * src/hotspot/share, test/hotspot/gtest: support of write-xor-execute (W^X),
>> required on macOS/AArch64 platform. It's implemented with
>> pthread_jit_write_protect_np provided by Apple. The W^X mode is local to a
>> thread, so W^X mode change relates to the java thread state change (for java
>> threads). In most cases, JVM executes in write-only mode, except when
>> calling a generated stub like SafeFetch, which requires a temporary switch
>> to execute-only mode. The same execute-only mode is enabled when a java
>> thread executes in java or native states. This approach of managing W^X mode
>> turned out to be simple and efficient enough.
>> * src/jdk.hotspot.agent: serviceability agent implementation (JDK-8254941)
>
> Anton Kozlov has updated the pull request incrementally with six additional
> commits since the last revision:
>
> - Merge remote-tracking branch 'origin/jdk/jdk-macos' into jdk-macos
> - Add comments to WX transitions
>
>+ minor change of placements
> - Use macro conditionals instead of empty functions
> - Add W^X to tests
> - Do not require known W^X state
> - Revert w^x in gtests
src/hotspot/os_cpu/bsd_aarch64/os_bsd_aarch64.cpp line 291:
> 289: bool is_unsafe_arraycopy = (thread->doing_unsafe_access() &&
> UnsafeCopyMemory::contains_pc(pc));
> 290: if ((nm != NULL && nm->has_unsafe_access()) ||
> is_unsafe_arraycopy) {
> 291: address next_pc = pc + NativeCall::instruction_size;
Replace
address next_pc = pc + NativeCall::instruction_size;
with
address next_pc = Assembler::locate_next_instruction(pc);
there is at least one other place that needs it.
src/hotspot/os_cpu/bsd_aarch64/os_bsd_aarch64.cpp line 322:
> 320: #ifdef __APPLE__
> 321: } else if (sig == SIGFPE && info->si_code == FPE_NOOP) {
> 322: Unimplemented();
Is there a follow up issue for this?
-
PR: https://git.openjdk.java.net/jdk/pull/2200
Re: RFR: 8253795: Implementation of JEP 391: macOS/AArch64 Port [v10]
On Wed, 3 Feb 2021 20:01:15 GMT, Anton Kozlov wrote:
>> Please review the implementation of JEP 391: macOS/AArch64 Port.
>>
>> It's heavily based on existing ports to linux/aarch64, macos/x86_64, and
>> windows/aarch64.
>>
>> Major changes are in:
>> * src/hotspot/cpu/aarch64: support of the new calling convention (subtasks
>> JDK-8253817, JDK-8253818)
>> * src/hotspot/os_cpu/bsd_aarch64: copy of os_cpu/linux_aarch64 with
>> necessary adjustments (JDK-8253819)
>> * src/hotspot/share, test/hotspot/gtest: support of write-xor-execute (W^X),
>> required on macOS/AArch64 platform. It's implemented with
>> pthread_jit_write_protect_np provided by Apple. The W^X mode is local to a
>> thread, so W^X mode change relates to the java thread state change (for java
>> threads). In most cases, JVM executes in write-only mode, except when
>> calling a generated stub like SafeFetch, which requires a temporary switch
>> to execute-only mode. The same execute-only mode is enabled when a java
>> thread executes in java or native states. This approach of managing W^X mode
>> turned out to be simple and efficient enough.
>> * src/jdk.hotspot.agent: serviceability agent implementation (JDK-8254941)
>
> Anton Kozlov has updated the pull request incrementally with six additional
> commits since the last revision:
>
> - Merge remote-tracking branch 'origin/jdk/jdk-macos' into jdk-macos
> - Add comments to WX transitions
>
>+ minor change of placements
> - Use macro conditionals instead of empty functions
> - Add W^X to tests
> - Do not require known W^X state
> - Revert w^x in gtests
src/hotspot/os_cpu/bsd_aarch64/os_bsd_aarch64.cpp line 194:
> 192: // may get turned off by -fomit-frame-pointer.
> 193: frame os::get_sender_for_C_frame(frame* fr) {
> 194: return frame(fr->link(), fr->link(), fr->sender_pc());
Why is it
return frame(fr->link(), fr->link(), fr->sender_pc());
and not
return frame(fr->sender_sp(), fr->link(), fr->sender_pc());
like in the bsd-x86 counter part?
-
PR: https://git.openjdk.java.net/jdk/pull/2200
Re: RFR: 8253795: Implementation of JEP 391: macOS/AArch64 Port [v10]
> Please review the implementation of JEP 391: macOS/AArch64 Port. > > It's heavily based on existing ports to linux/aarch64, macos/x86_64, and > windows/aarch64. > > Major changes are in: > * src/hotspot/cpu/aarch64: support of the new calling convention (subtasks > JDK-8253817, JDK-8253818) > * src/hotspot/os_cpu/bsd_aarch64: copy of os_cpu/linux_aarch64 with necessary > adjustments (JDK-8253819) > * src/hotspot/share, test/hotspot/gtest: support of write-xor-execute (W^X), > required on macOS/AArch64 platform. It's implemented with > pthread_jit_write_protect_np provided by Apple. The W^X mode is local to a > thread, so W^X mode change relates to the java thread state change (for java > threads). In most cases, JVM executes in write-only mode, except when calling > a generated stub like SafeFetch, which requires a temporary switch to > execute-only mode. The same execute-only mode is enabled when a java thread > executes in java or native states. This approach of managing W^X mode turned > out to be simple and efficient enough. > * src/jdk.hotspot.agent: serviceability agent implementation (JDK-8254941) Anton Kozlov has updated the pull request incrementally with six additional commits since the last revision: - Merge remote-tracking branch 'origin/jdk/jdk-macos' into jdk-macos - Add comments to WX transitions + minor change of placements - Use macro conditionals instead of empty functions - Add W^X to tests - Do not require known W^X state - Revert w^x in gtests - Changes: - all: https://git.openjdk.java.net/jdk/pull/2200/files - new: https://git.openjdk.java.net/jdk/pull/2200/files/3c705ae5..80827176 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=2200&range=09 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=2200&range=08-09 Stats: 444 lines in 64 files changed: 112 ins; 278 del; 54 mod Patch: https://git.openjdk.java.net/jdk/pull/2200.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/2200/head:pull/2200 PR: https://git.openjdk.java.net/jdk/pull/2200
