Re: RFR: 8253795: Implementation of JEP 391: macOS/AArch64 Port [v10]

classic Classic list List threaded Threaded
2 messages Options
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8253795: Implementation of JEP 391: macOS/AArch64 Port [v10]

Vladimir Kempik-3
On Thu, 4 Feb 2021 22:49:23 GMT, Gerard Ziemski <[hidden email]> 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
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8253795: Implementation of JEP 391: macOS/AArch64 Port [v10]

Vladimir Kempik-3
On Thu, 4 Feb 2021 22:49:23 GMT, Gerard Ziemski <[hidden email]> 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