Re: gcc, arm, and thumbs mode

[Andrew Haley]

On 5/30/22 06:57, Thomas Stüfe wrote:

For my specific problem, I can and probably should use .function. But my
question was more general, should we leave the decision whether to use
thumb or arm up to the toolchain. For two reasons, one is executable size -
I assume using thumb has certain advantages, executables are smaller and
you can fit more into the instruction cache, and second, because errors
like mine are probably not that uncommon and it makes me nervous that we
only caught it because someone happened to build on his Raspi.


Indeed! I'm surprised the problem doesn't come up in other places too.

--
Andrew Haley  (he/him)
Java Platform Lead Engineer
Red Hat UK Ltd. <https://www.redhat.com>
https://keybase.io/andrewhaley
EAC8 43EB D3EF DB98 CC77 2FAD A5CD 6035 332F A671

Re: RFR: 8282322: AArch64: Provide a means to eliminate all STREX family of instructions [v3]

[Andrew Haley]

On Mon, 23 May 2022 16:12:34 GMT, Dmitry Chuyko  wrote:

>> On AArch64 it is sometimes convenient to have LSE atomics right from the 
>> start. Currently they are enabled after feature detection and RR reverse 
>> debugger works incorrectly.
>> 
>> New build configuration feature 'hardlse' is added. If it is enabled for 
>> aarch64 type of build, then statically compiled stubs replace the initial 
>> pessimistic implementation and dynamically generated replacements (when LSE 
>> support is detected). The feature works for builds of all debug levels.
>> 
>> New file atomic_linux_aarch64_lse.S is derived from atomic_linux_aarch64.S 
>> and inherits its copyright. This alternative static implementation 
>> corresponds to the dynamically generated code.
>> 
>> Note, this configuration part is necessary but not sufficient to fully avoid 
>> strex instructions for practical purposes. Other parts are:
>> 
>> * Run on the OS built without strex family instructions. E.g. Amazon Linux 
>> 2022.
>> * Compile with outline atomics enabled and the configuration flag enabled. 
>> E.g. configure with
>> --with-extra-cflags='-march=armv8.3-a+crc+crypto -moutline-atomics' 
>> --with-extra-cxxflags='-march=armv8.3-a+crc+crypto -moutline-atomics' 
>> --with-extra-ldflags='-Wl,--allow-multiple-definition' 
>> --with-jvm-features=hardlse
>> 
>> Testing: tier1, tier2 on linux-aarch64 release builds with feature off and 
>> feature on.
>
> Dmitry Chuyko has updated the pull request with a new target base due to a 
> merge or a rebase. The incremental webrev excludes the unrelated changes 
> brought in by the merge/rebase. The pull request contains five additional 
> commits since the last revision:
> 
>  - Merge branch 'openjdk:master' into JDK-8282322
>  - Use LSE in linux-aarch64 asm code if __ARM_FEATURE_ATOMICS is on
>  - Revert "hardlse feature"
>
>This reverts commit c5da85d3282bb995f69639f8f592cc94560916c5.
>  - Merge branch 'openjdk:master' into JDK-8282322
>  - hardlse feature

src/hotspot/os_cpu/linux_aarch64/atomic_linux_aarch64.S line 291:

> 289: #endif
> 290: 1:  mov x0, x3
> 291: ret

Oh gosh, I find all those `#if`s really hard to read.

-

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

Re: RFR: 8282322: AArch64: Provide a means to eliminate all STREX family of instructions

[Andrew Haley]

On Thu, 19 May 2022 14:36:28 GMT, Andrew Haley  wrote:

> > Some concerns:
> > ```
> > 1. It should work at least for Clang and GCC but still will require 
> > checking all os-cpu/compiler variants.

Non-Linux systems don't use this stuff at all, so don't worry about it.

-

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

Re: RFR: 8282322: AArch64: Provide a means to eliminate all STREX family of instructions

[Andrew Haley]

On Wed, 18 May 2022 19:05:03 GMT, Dmitry Chuyko  wrote:

> On AArch64 it is sometimes convenient to have LSE atomics right from the 
> start. Currently they are enabled after feature detection and RR reverse 
> debugger works incorrectly.
> 
> New build configuration feature 'hardlse' is added. If it is enabled for 
> aarch64 type of build, then statically compiled stubs replace the initial 
> pessimistic implementation and dynamically generated replacements (when LSE 
> support is detected). The feature works for builds of all debug levels.
> 
> New file atomic_linux_aarch64_lse.S is derived from atomic_linux_aarch64.S 
> and inherits its copyright. This alternative static implementation 
> corresponds to the dynamically generated code.
> 
> Note, this configuration part is necessary but not sufficient to fully avoid 
> strex instructions for practical purposes. Other parts are:
> 
> * Run on the OS built without strex family instructions. E.g. Amazon Linux 
> 2022.
> * Compile with outline atomics enabled and the configuration flag enabled. 
> E.g. configure with
> --with-extra-cflags='-march=armv8.3-a+crc+crypto -moutline-atomics' 
> --with-extra-cxxflags='-march=armv8.3-a+crc+crypto -moutline-atomics' 
> --with-extra-ldflags='-Wl,--allow-multiple-definition' 
> --with-jvm-features=hardlse
> 
> Testing: tier1, tier2 on linux-aarch64 release builds with feature off and 
> feature on.

src/hotspot/os_cpu/linux_aarch64/atomic_linux_aarch64_lse.S line 30:

> 28: aarch64_atomic_fetch_add_8_default_impl:
> 29: prfmpstl1strm, [x0]
> 30: 0:  ldaddal x1, x2, [x0]

Suggestion:

ldaddal x1, x2, [x0]

You don't need the `0:`label.

-

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

Re: RFR: 8282322: AArch64: Provide a means to eliminate all STREX family of instructions

[Andrew Haley]

On Wed, 18 May 2022 19:05:03 GMT, Dmitry Chuyko  wrote:

> On AArch64 it is sometimes convenient to have LSE atomics right from the 
> start. Currently they are enabled after feature detection and RR reverse 
> debugger works incorrectly.
> 
> New build configuration feature 'hardlse' is added. If it is enabled for 
> aarch64 type of build, then statically compiled stubs replace the initial 
> pessimistic implementation and dynamically generated replacements (when LSE 
> support is detected). The feature works for builds of all debug levels.
> 
> New file atomic_linux_aarch64_lse.S is derived from atomic_linux_aarch64.S 
> and inherits its copyright. This alternative static implementation 
> corresponds to the dynamically generated code.
> 
> Note, this configuration part is necessary but not sufficient to fully avoid 
> strex instructions for practical purposes. Other parts are:
> 
> * Run on the OS built without strex family instructions. E.g. Amazon Linux 
> 2022.
> * Compile with outline atomics enabled and the configuration flag enabled. 
> E.g. configure with
> --with-extra-cflags='-march=armv8.3-a+crc+crypto -moutline-atomics' 
> --with-extra-cxxflags='-march=armv8.3-a+crc+crypto -moutline-atomics' 
> --with-extra-ldflags='-Wl,--allow-multiple-definition' 
> --with-jvm-features=hardlse
> 
> Testing: tier1, tier2 on linux-aarch64 release builds with feature off and 
> feature on.

I literally just tried this:






.text

#ifdef __ARM_FEATURE_ATOMICS

.globl aarch64_atomic_fetch_add_8_default_impl
.align 5
aarch64_atomic_fetch_add_8_default_impl:
prfmpstl1strm, [x0]
0:  ldaddal x1, x2, [x0]
dmb ish
mov x0, x2



with the obvious `#else` and `#endif` around the non-LSE part

and


$ objdump -d 
/home/aph/theRealAph-jdk/build/linux-aarch64-server-slowdebug/hotspot/variant-server/libjvm/objs/atomic_linux_aarch64.o
 | head -40

-

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

Re: RFR: 8282322: AArch64: Provide a means to eliminate all STREX family of instructions

[Andrew Haley]

On Thu, 19 May 2022 10:05:41 GMT, Dmitry Chuyko  wrote:

> Some concerns:
> 
> 1. It should work at least for Clang and GCC but still will require 
> checking all os-cpu/compiler variants.

Really, no. Don't think that way. Just do Linux for now, and throw it over the 
wall for Windows people.

> 3. The way of configuration is different for different compilers, not a 
> problem though if we need an optimized build or to make rr work.

See above.

-

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

Re: RFR: 8282322: AArch64: Provide a means to eliminate all STREX family of instructions

[Andrew Haley]

On Thu, 19 May 2022 08:43:55 GMT, Nick Gasson  wrote:

> What's the advantage of defining the new hardlse VM feature over using the 
> existing `__ARM_FEATURE_ATOMICS` preprocessor symbol? Both GCC and Clang will 
> define that with an appropriate `-march` value, which you're passing to 
> configure anyway. You could then conditionally emit the LSE instructions in 
> atomic_linux_aarch64.S based on that.

That's not a bad idea either. In fact, maybe I prefer it to my own suggestion.

-

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

Re: RFR: 8282322: AArch64: Provide a means to eliminate all STREX family of instructions

[Andrew Haley]

On Wed, 18 May 2022 19:05:03 GMT, Dmitry Chuyko  wrote:

> On AArch64 it is sometimes convenient to have LSE atomics right from the 
> start. Currently they are enabled after feature detection and RR reverse 
> debugger works incorrectly.
> 
> New build configuration feature 'hardlse' is added. If it is enabled for 
> aarch64 type of build, then statically compiled stubs replace the initial 
> pessimistic implementation and dynamically generated replacements (when LSE 
> support is detected). The feature works for builds of all debug levels.
> 
> New file atomic_linux_aarch64_lse.S is derived from atomic_linux_aarch64.S 
> and inherits its copyright. This alternative static implementation 
> corresponds to the dynamically generated code.
> 
> Note, this configuration part is necessary but not sufficient to fully avoid 
> strex instructions for practical purposes. Other parts are:
> 
> * Run on the OS built without strex family instructions. E.g. Amazon Linux 
> 2022.
> * Compile with outline atomics enabled and the configuration flag enabled. 
> E.g. configure with
> --with-extra-cflags='-march=armv8.3-a+crc+crypto -moutline-atomics' 
> --with-extra-cxxflags='-march=armv8.3-a+crc+crypto -moutline-atomics' 
> --with-extra-ldflags='-Wl,--allow-multiple-definition' 
> --with-jvm-features=hardlse
> 
> Testing: tier1, tier2 on linux-aarch64 release builds with feature off and 
> feature on.

There is another way to achieve what you want, which is less disruptive and I 
believe more useful.

I just did this:

`cp src/hotspot/os_cpu/bsd_aarch64/atomic_bsd_aarch64.hpp 
src/hotspot/os_cpu/linux_aarch64/atomic_linux_aarch64.hpp`

and built OpenJDK with your configury, and it achieves exactly what you want: a 
HotSpot that uses LSE throughout. If you, therefore, add an option to build 
with host compiler's atomics, just like BSD does, you'll get what you need with 
almost no code changes.

-

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

Re: RFR: 8282322: AArch64: Provide a means to eliminate all STREX family of instructions

[Andrew Haley]

On Thu, 19 May 2022 07:59:56 GMT, Andrew Haley  wrote:

> This looks reasonable enough, but I take it that this would create an OpenJDK 
> build that would not run on AArch64 systems without LSE instructions.

Forget that, we already do so with extra-cflags.

-

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

Re: RFR: 8282322: AArch64: Provide a means to eliminate all STREX family of instructions

[Andrew Haley]

On Wed, 18 May 2022 19:05:03 GMT, Dmitry Chuyko  wrote:

> On AArch64 it is sometimes convenient to have LSE atomics right from the 
> start. Currently they are enabled after feature detection and RR reverse 
> debugger works incorrectly.
> 
> New build configuration feature 'hardlse' is added. If it is enabled for 
> aarch64 type of build, then statically compiled stubs replace the initial 
> pessimistic implementation and dynamically generated replacements (when LSE 
> support is detected). The feature works for builds of all debug levels.
> 
> New file atomic_linux_aarch64_lse.S is derived from atomic_linux_aarch64.S 
> and inherits its copyright. This alternative static implementation 
> corresponds to the dynamically generated code.
> 
> Note, this configuration part is necessary but not sufficient to fully avoid 
> strex instructions for practical purposes. Other parts are:
> 
> * Run on the OS built without strex family instructions. E.g. Amazon Linux 
> 2022.
> * Compile with outline atomics enabled and the configuration flag enabled. 
> E.g. configure with
> --with-extra-cflags='-march=armv8.3-a+crc+crypto -moutline-atomics' 
> --with-extra-cxxflags='-march=armv8.3-a+crc+crypto -moutline-atomics' 
> --with-extra-ldflags='-Wl,--allow-multiple-definition' 
> --with-jvm-features=hardlse
> 
> Testing: tier1, tier2 on linux-aarch64 release builds with feature off and 
> feature on.

This looks reasonable enough, but I take it that this would create an OpenJDK 
build that would not run on AArch64 systems without LSE instructions. Might it 
not be a better idea to build with outline-atomics, and use them in OpenJDK? 
That would also solve your problem, and we wouldn't have AArch64 Linux OpenJDK 
binaries which don't work on all systems.

Note that atomic_bsd_aarch64.hpp already does this.

I'm also concerned about code rot with options like this one. 
atomic_linux_aarch64_lse.S would not be tested in any standard configuration.

-

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

Re: RFR: 8282322: AArch64: Provide a means to eliminate all STREX family of instructions

[Andrew Haley]

On Wed, 18 May 2022 19:05:03 GMT, Dmitry Chuyko  wrote:

> On AArch64 it is sometimes convenient to have LSE atomics right from the 
> start. Currently they are enabled after feature detection and RR reverse 
> debugger works incorrectly.
> 
> New build configuration feature 'hardlse' is added. If it is enabled for 
> aarch64 type of build, then statically compiled stubs replace the initial 
> pessimistic implementation and dynamically generated replacements (when LSE 
> support is detected). The feature works for builds of all debug levels.
> 
> New file atomic_linux_aarch64_lse.S is derived from atomic_linux_aarch64.S 
> and inherits its copyright. This alternative static implementation 
> corresponds to the dynamically generated code.
> 
> Note, this configuration part is necessary but not sufficient to fully avoid 
> strex instructions for practical purposes. Other parts are:
> 
> * Run on the OS built without strex family instructions. E.g. Amazon Linux 
> 2022.
> * Compile with outline atomics enabled and the configuration flag enabled. 
> E.g. configure with
> --with-extra-cflags='-march=armv8.3-a+crc+crypto -moutline-atomics' 
> --with-extra-cxxflags='-march=armv8.3-a+crc+crypto -moutline-atomics' 
> --with-extra-ldflags='-Wl,--allow-multiple-definition' 
> --with-jvm-features=hardlse
> 
> Testing: tier1, tier2 on linux-aarch64 release builds with feature off and 
> feature on.

src/hotspot/os_cpu/linux_aarch64/atomic_linux_aarch64_lse.S line 57:

> 55: .globl aarch64_atomic_xchg_4_default_impl
> 56: .align 5
> 57: prfmpstl1strm, [x0]

Do we need these `prfm` with LSE instructions?

-

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

Re: RFR: 8284661: Reproducible assembly builds without relative linking

[Andrew Haley]

On Mon, 11 Apr 2022 17:27:35 GMT, Andrew Leonard  wrote:

> I excluded all kinds of debuginfo files because I didn't know if they could 
> be made free of absolute paths, and it was out of scope for what I was doing 
> at the time.

GCC, I believe, uses whatever pathname you give to the compiler in the 
debuginfo. If you give GCC relative pathnames, that's what it emits.

-

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

Re: RFR: 8282507: Add LICENSE file for hsdis

[Andrew Haley]

On Thu, 10 Mar 2022 05:35:39 GMT, Man Cao  wrote:

> Any updates on whether this is acceptable?

What is the point of this change? It'd require legal inspection, and as far as 
I can tell there is no point.

-

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

Re: RFR: 8277204: Implement PAC-RET branch protection on Linux/AArch64 [v25]

[Andrew Haley]

On 2/25/22 13:12, David Holmes wrote:

the Zero build


Zero on AArch64? I wonder if we need a better way of handling this than

AARCH64_ONLY(NOT_ZERO(

Maybe AARCH64_PORT_ONLY would be a Good Thing.

--
Andrew Haley  (he/him)
Java Platform Lead Engineer
Red Hat UK Ltd. <https://www.redhat.com>
https://keybase.io/andrewhaley
EAC8 43EB D3EF DB98 CC77 2FAD A5CD 6035 332F A671

Re: RFR: 8209784: Include hsdis in the JDK

[Andrew Haley]

On Tue, 22 Feb 2022 16:10:22 GMT, Magnus Ihse Bursie  wrote:

> This PR adds a new configure argument, `--enable-hsdis-bundling`. Setting 
> this, together with a valid backend using `--with-hsdis`, will bundle the 
> generated hsdis library with the JDK.

Maybe I missed it, but are there instructions somewhere about how to build 
this, and what should be downloaded?

-

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

Re: RFR: 8277204: Implement PAC-RET branch protection on Linux/AArch64 [v21]

[Andrew Haley]

On Fri, 11 Feb 2022 11:37:56 GMT, Alan Hayward  wrote:

>> PAC is an optional feature in AArch64 8.3 and is compulsory in v9. One
>> of its uses is to protect against ROP based attacks. This is done by
>> signing the Link Register whenever it is stored on the stack, and
>> authenticating the value when it is loaded back from the stack. If an
>> attacker were to try to change control flow by editing the stack then
>> the authentication check of the Link Register will fail, causing a
>> segfault when the function returns.
>> 
>> On a system with PAC enabled, it is expected that all applications will
>> be compiled with ROP protection. Fedora 33 and upwards already provide
>> this. By compiling for ARMv8.0, GCC and LLVM will only use the set of
>> PAC instructions that exist in the NOP space - on hardware without PAC,
>> these instructions act as NOPs, allowing backward compatibility for
>> negligible performance cost (2 NOPs per non-leaf function).
>> 
>> Hardware is currently limited to the Apple M1 MacBooks. All testing has
>> been done within a Fedora Docker image. A run of SpecJVM showed no
>> difference to that of noise - which was surprising.
>> 
>> The most important part of this patch is simply compiling using branch
>> protection provided by GCC/LLVM. This protects all C++ code from being
>> used in ROP attacks, removing all static ROP gadgets from use.
>> 
>> The remainder of the patch adds ROP protection to runtime generated
>> code, in both stubs and compiled Java code. Attacks here are much harder
>> as ROP gadgets must be found dynamically at runtime. If/when AOT
>> compilation is added to JDK, then all stubs and compiled Java will be
>> susceptible ROP gadgets being found by static analysis and therefore
>> potentially as vulnerable as C++ code.
>> 
>> There are a number of places where the VM changes control flow by
>> rewriting the stack or otherwise. I’ve done some analysis as to how
>> these could also be used for attacks (which I didn’t want to post here).
>> These areas can be protected ensuring the pointers to various stubs and
>> entry points are stored in memory as signed pointers. These changes are
>> simple to make (they can be reduced to a type change in common code and
>> a few addition sign/auth calls in the backend), but there a lot of them
>> and the total code change is fairly large. I’m happy to provide a few
>> work in progress patches.
>> 
>> In order to match the security benefits of the Apple Arm64e ABI across
>> the whole of JDK, then all the changes mentioned above would be
>> required.
>
> Alan Hayward has updated the pull request incrementally with two additional 
> commits since the last revision:
> 
>  - Add comments to enter calls
>  - Set PreserveFramePointer if use_rop_protection is set

This is looking pretty nice now. With the check for -XX:-UseFramePointer 
argument consistency we're done.

-

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

Re: RFR: 8277204: Implement PAC-RET branch protection on Linux/AArch64 [v21]

[Andrew Haley]

On Fri, 11 Feb 2022 11:37:56 GMT, Alan Hayward  wrote:

>> PAC is an optional feature in AArch64 8.3 and is compulsory in v9. One
>> of its uses is to protect against ROP based attacks. This is done by
>> signing the Link Register whenever it is stored on the stack, and
>> authenticating the value when it is loaded back from the stack. If an
>> attacker were to try to change control flow by editing the stack then
>> the authentication check of the Link Register will fail, causing a
>> segfault when the function returns.
>> 
>> On a system with PAC enabled, it is expected that all applications will
>> be compiled with ROP protection. Fedora 33 and upwards already provide
>> this. By compiling for ARMv8.0, GCC and LLVM will only use the set of
>> PAC instructions that exist in the NOP space - on hardware without PAC,
>> these instructions act as NOPs, allowing backward compatibility for
>> negligible performance cost (2 NOPs per non-leaf function).
>> 
>> Hardware is currently limited to the Apple M1 MacBooks. All testing has
>> been done within a Fedora Docker image. A run of SpecJVM showed no
>> difference to that of noise - which was surprising.
>> 
>> The most important part of this patch is simply compiling using branch
>> protection provided by GCC/LLVM. This protects all C++ code from being
>> used in ROP attacks, removing all static ROP gadgets from use.
>> 
>> The remainder of the patch adds ROP protection to runtime generated
>> code, in both stubs and compiled Java code. Attacks here are much harder
>> as ROP gadgets must be found dynamically at runtime. If/when AOT
>> compilation is added to JDK, then all stubs and compiled Java will be
>> susceptible ROP gadgets being found by static analysis and therefore
>> potentially as vulnerable as C++ code.
>> 
>> There are a number of places where the VM changes control flow by
>> rewriting the stack or otherwise. I’ve done some analysis as to how
>> these could also be used for attacks (which I didn’t want to post here).
>> These areas can be protected ensuring the pointers to various stubs and
>> entry points are stored in memory as signed pointers. These changes are
>> simple to make (they can be reduced to a type change in common code and
>> a few addition sign/auth calls in the backend), but there a lot of them
>> and the total code change is fairly large. I’m happy to provide a few
>> work in progress patches.
>> 
>> In order to match the security benefits of the Apple Arm64e ABI across
>> the whole of JDK, then all the changes mentioned above would be
>> required.
>
> Alan Hayward has updated the pull request incrementally with two additional 
> commits since the last revision:
> 
>  - Add comments to enter calls
>  - Set PreserveFramePointer if use_rop_protection is set

src/hotspot/cpu/aarch64/vm_version_aarch64.cpp line 439:

> 437:   if (_rop_protection == true) {
> 438: PreserveFramePointer = true;
> 439:   }

You need an error message for -PreserveFramePointer +UseROPProtection.

-

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

Re: RFR: 8277204: Implement PAC-RET branch protection on Linux/AArch64 [v18]

[Andrew Haley]

On Tue, 8 Feb 2022 09:40:49 GMT, Andrew Haley  wrote:

>> Doing this caused 7 failures across a full jtreg run, namely:
>> 
>> serviceability/sa/ClhsdbFindPC.java#xcomp-core
>> vmTestbase/jit/misctests/fpustack/GraphApplet.java
>> vmTestbase/nsk/jdi/MonitorWaitRequest/MonitorWaitRequest001/TestDescription.java
>> vmTestbase/nsk/jdi/MonitorWaitedRequest/MonitorWaitedRequest001/TestDescription.java
>> vmTestbase/nsk/jdwp/ThreadReference/ForceEarlyReturn/forceEarlyReturn002/forceEarlyReturn002.java
>> vmTestbase/nsk/jdwp/ThreadReference/OwnedMonitorsStackDepthInfo/ownedMonitorsStackDepthInfo002/ownedMonitorsStackDepthInfo002.java
>> vmTestbase/nsk/jvmti/RedefineClasses/StressRedefine/TestDescription.java
>> 
>> I'll investigate.
>
>> Doing this caused 7 failures across a full jtreg run, namely:
> 
> I'm glad we caught that.

Status? Is branch protection really incompatible with PreserveFramePointer?

-

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

Re: RFR: 8277204: Implement PAC-RET branch protection on Linux/AArch64 [v18]

[Andrew Haley]

On Tue, 8 Feb 2022 09:22:39 GMT, Alan Hayward  wrote:

> Doing this caused 7 failures across a full jtreg run, namely:

I'm glad we caught that.

-

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

Re: RFR: 8277204: Implement PAC-RET branch protection on Linux/AArch64 [v18]

[Andrew Haley]

On Mon, 7 Feb 2022 13:43:55 GMT, Alan Hayward  wrote:

>> Tell you what, first put a comment here that says when it should (and 
>> therefore, should not) be used. Once it's clear exactly what this is for, 
>> thinking of a name maight be easier.
>
> How about extending the existing enter() function: 
> 
> // Enter a new stack frame for the current method.
> // nested: Indicates a frame has already been entered (and not left) for 
> the current method. 
> void MacroAssembler::enter(bool nested=false) {
>if (nested) strip()
>protect()
>stp()
>mov()
> }
> 
> This would add an additional bool check for every call of enter() - that's at 
> code generation time, so probably not an issue.

So, `nested` is true iff we are, say, pushing an extra frame for a runtime call 
in the middle of generated code, but for some mysterious reason the logic is 
inline instead of being implemented in the obvious way as a stub.

Please do this as:

` MacroAssembler::enter(bool strip_return_address=false)`

and I'll be happy. Please make sure that all calls are commented, as in

`__ enter(/*strip_return_address*/true);`

and I'll be happy.

-

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

Re: RFR: 8277204: Implement PAC-RET branch protection on Linux/AArch64 [v18]

[Andrew Haley]

On Mon, 7 Feb 2022 12:01:18 GMT, Alan Hayward  wrote:

> PreserveFramePointer is doing some additional stuff. I'll give it a test to 
> make sure everything still works with PreserveFramePointer fully set. It 
> would make things easier just to force it set with rop protection on.

Using PreserveFramePointer greatly simplifies the testing matrix, and has 
little adverse performance impact beyond disallowing C2 from allocating FP as a 
scratch register. It also simplifies this patch, which would be a very Good 
Thing. Let's do it.

-

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

Re: RFR: 8277204: Implement PAC-RET branch protection on Linux/AArch64 [v18]

[Andrew Haley]

On Mon, 7 Feb 2022 11:42:43 GMT, Alan Hayward  wrote:

>> Because enter always enters a subframe. That's what it's for.
>
> enter_nested() ?
> enter_inner() ?

Tell you what, first put a comment here that says when it should (and 
therefore, should not) be used. Once it's clear exactly what this is for, 
thinking of a name maight be easier.

-

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

Re: RFR: 8277204: Implement PAC-RET branch protection on Linux/AArch64 [v18]

[Andrew Haley]

On Mon, 7 Feb 2022 11:41:57 GMT, Alan Hayward  wrote:

>> src/hotspot/cpu/aarch64/macroAssembler_aarch64.cpp line 5328:
>> 
>>> 5326: // Uses the FP from the start of the function as the modifier - which 
>>> is stored at the address of
>>> 5327: // the current FP.
>>> 5328: //
>> 
>> Is it? C2 uses FP as a scratch register. I guess we know that this is never 
>> used in C2-generated code? I'm tempted to put an assertion here, just in 
>> case. Or does it not matter?
>
> Allocating FP is disabled for rop protection:
> 
> aarch64.md has:
> // r29 is not allocatable when PreserveFramePointer or ROP protection is on
> if (PreserveFramePointer || VM_Version::use_rop_protection()) {
> 
> I think that covers it.
> What assertion would you want to check?

If `UseROPProtection` is on, is there any reason not to set 
`PreserveFramePointer`, and assert here that it is set? It is a crucial 
assumption, so let's assert it.

-

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

Re: RFR: 8277204: Implement PAC-RET branch protection on Linux/AArch64 [v18]

[Andrew Haley]

On Mon, 7 Feb 2022 11:28:30 GMT, Alan Hayward  wrote:

>>> Let's see both pc and signed pc here, if they are different.
>
> Are you sure? At the moment with PAC we get:
> 
> patch_pc at address 0xf58edf98 [0x0068ed17b5fc -> 
> 0x00abed17b7f8]
> 
> With both signed and unsigned you'd have:
> 
> patch_pc at address 0xf58edf98 [0x0068ed17b5fc 
> (0xed17b5fc) -> 0x00abed17b7f8 (0xed17b7f8)]
> 
> I prefer the first - it's shorter and you can infer the address from the 
> signed version. Happy to go with the longer version if you think the shorter 
> version is confusing.

You've been looking at PAC-signed addresses for a long time. 
Let's see "at address [prev true dest -> new true dest] [signed prev signed 
dest -> new signed dest]", but only show the signed dests if they're different. 
So it appears as 
`patch_pc at address 0xf58edf98 [0x0068ed17b5fc -> 
0x00abed17b7f8] [signed 0xed17b5fc ->  0xed17b7f8]` .

-

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

Re: RFR: 8277204: Implement PAC-RET branch protection on Linux/AArch64 [v18]

[Andrew Haley]

On Thu, 3 Feb 2022 16:51:48 GMT, Alan Hayward  wrote:

>> PAC is an optional feature in AArch64 8.3 and is compulsory in v9. One
>> of its uses is to protect against ROP based attacks. This is done by
>> signing the Link Register whenever it is stored on the stack, and
>> authenticating the value when it is loaded back from the stack. If an
>> attacker were to try to change control flow by editing the stack then
>> the authentication check of the Link Register will fail, causing a
>> segfault when the function returns.
>> 
>> On a system with PAC enabled, it is expected that all applications will
>> be compiled with ROP protection. Fedora 33 and upwards already provide
>> this. By compiling for ARMv8.0, GCC and LLVM will only use the set of
>> PAC instructions that exist in the NOP space - on hardware without PAC,
>> these instructions act as NOPs, allowing backward compatibility for
>> negligible performance cost (2 NOPs per non-leaf function).
>> 
>> Hardware is currently limited to the Apple M1 MacBooks. All testing has
>> been done within a Fedora Docker image. A run of SpecJVM showed no
>> difference to that of noise - which was surprising.
>> 
>> The most important part of this patch is simply compiling using branch
>> protection provided by GCC/LLVM. This protects all C++ code from being
>> used in ROP attacks, removing all static ROP gadgets from use.
>> 
>> The remainder of the patch adds ROP protection to runtime generated
>> code, in both stubs and compiled Java code. Attacks here are much harder
>> as ROP gadgets must be found dynamically at runtime. If/when AOT
>> compilation is added to JDK, then all stubs and compiled Java will be
>> susceptible ROP gadgets being found by static analysis and therefore
>> potentially as vulnerable as C++ code.
>> 
>> There are a number of places where the VM changes control flow by
>> rewriting the stack or otherwise. I’ve done some analysis as to how
>> these could also be used for attacks (which I didn’t want to post here).
>> These areas can be protected ensuring the pointers to various stubs and
>> entry points are stored in memory as signed pointers. These changes are
>> simple to make (they can be reduced to a type change in common code and
>> a few addition sign/auth calls in the backend), but there a lot of them
>> and the total code change is fairly large. I’m happy to provide a few
>> work in progress patches.
>> 
>> In order to match the security benefits of the Apple Arm64e ABI across
>> the whole of JDK, then all the changes mentioned above would be
>> required.
>
> Alan Hayward has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Documentation updates

src/hotspot/cpu/aarch64/macroAssembler_aarch64.cpp line 5328:

> 5326: // Uses the FP from the start of the function as the modifier - which 
> is stored at the address of
> 5327: // the current FP.
> 5328: //

Is it? C2 uses FP as a scratch register. I guess we know that this is never 
used in C2-generated code? I'm tempted to put an assertion here, just in case. 
Or does it not matter?

-

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

Re: RFR: 8277204: Implement PAC-RET branch protection on Linux/AArch64 [v18]

[Andrew Haley]

On Mon, 7 Feb 2022 10:58:59 GMT, Andrew Haley  wrote:

>> Alan Hayward has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   Documentation updates
>
> src/hotspot/cpu/aarch64/frame_aarch64.cpp line 275:
> 
>> 273:   if (TracePcPatching) {
>> 274: tty->print_cr("patch_pc at address " INTPTR_FORMAT " [" 
>> INTPTR_FORMAT " -> " INTPTR_FORMAT "]",
>> 275:   p2i(pc_addr), p2i(*pc_addr), p2i(signed_pc));
> 
> Let's see both pc and signed pc here.

> Let's see both pc and signed pc here, if they are different.

-

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

Re: RFR: 8277204: Implement PAC-RET branch protection on Linux/AArch64 [v18]

[Andrew Haley]

On Thu, 3 Feb 2022 16:51:48 GMT, Alan Hayward  wrote:

>> PAC is an optional feature in AArch64 8.3 and is compulsory in v9. One
>> of its uses is to protect against ROP based attacks. This is done by
>> signing the Link Register whenever it is stored on the stack, and
>> authenticating the value when it is loaded back from the stack. If an
>> attacker were to try to change control flow by editing the stack then
>> the authentication check of the Link Register will fail, causing a
>> segfault when the function returns.
>> 
>> On a system with PAC enabled, it is expected that all applications will
>> be compiled with ROP protection. Fedora 33 and upwards already provide
>> this. By compiling for ARMv8.0, GCC and LLVM will only use the set of
>> PAC instructions that exist in the NOP space - on hardware without PAC,
>> these instructions act as NOPs, allowing backward compatibility for
>> negligible performance cost (2 NOPs per non-leaf function).
>> 
>> Hardware is currently limited to the Apple M1 MacBooks. All testing has
>> been done within a Fedora Docker image. A run of SpecJVM showed no
>> difference to that of noise - which was surprising.
>> 
>> The most important part of this patch is simply compiling using branch
>> protection provided by GCC/LLVM. This protects all C++ code from being
>> used in ROP attacks, removing all static ROP gadgets from use.
>> 
>> The remainder of the patch adds ROP protection to runtime generated
>> code, in both stubs and compiled Java code. Attacks here are much harder
>> as ROP gadgets must be found dynamically at runtime. If/when AOT
>> compilation is added to JDK, then all stubs and compiled Java will be
>> susceptible ROP gadgets being found by static analysis and therefore
>> potentially as vulnerable as C++ code.
>> 
>> There are a number of places where the VM changes control flow by
>> rewriting the stack or otherwise. I’ve done some analysis as to how
>> these could also be used for attacks (which I didn’t want to post here).
>> These areas can be protected ensuring the pointers to various stubs and
>> entry points are stored in memory as signed pointers. These changes are
>> simple to make (they can be reduced to a type change in common code and
>> a few addition sign/auth calls in the backend), but there a lot of them
>> and the total code change is fairly large. I’m happy to provide a few
>> work in progress patches.
>> 
>> In order to match the security benefits of the Apple Arm64e ABI across
>> the whole of JDK, then all the changes mentioned above would be
>> required.
>
> Alan Hayward has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Documentation updates

src/hotspot/os_cpu/linux_aarch64/pauth_linux_aarch64.inline.hpp line 57:

> 55: register address r17 __asm("r17") = ret_addr;
> 56: register address r16 __asm("r16") = sp;
> 57: asm volatile (PACIA1716 : "+r"(r17) : "r"(r16));

I don't see the point of `volatile` here, any more than you'd use volatile on 
an addition. `volatile` is when you have a side effect you care about.

-

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

Re: RFR: 8277204: Implement PAC-RET branch protection on Linux/AArch64 [v18]

[Andrew Haley]

On Mon, 7 Feb 2022 10:55:52 GMT, Andrew Haley  wrote:

>> Alan Hayward has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   Documentation updates
>
> src/hotspot/cpu/aarch64/macroAssembler_aarch64.cpp line 5293:
> 
>> 5291: // Create an additional frame for a function.
>> 5292: void MacroAssembler::enter_subframe() {
>> 5293:   // Addresses can only be signed once, so strip it first. PAC safe 
>> because the value is not
> 
> This needs a more descriptive name. `enter_and_sign()` ? No, that's not right 
> either. How do we come up with a name that's more descriptive?

Because enter always enters a subframe. That's what it's for.

-

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

Re: RFR: 8277204: Implement PAC-RET branch protection on Linux/AArch64 [v18]

[Andrew Haley]

On Thu, 3 Feb 2022 16:51:48 GMT, Alan Hayward  wrote:

>> PAC is an optional feature in AArch64 8.3 and is compulsory in v9. One
>> of its uses is to protect against ROP based attacks. This is done by
>> signing the Link Register whenever it is stored on the stack, and
>> authenticating the value when it is loaded back from the stack. If an
>> attacker were to try to change control flow by editing the stack then
>> the authentication check of the Link Register will fail, causing a
>> segfault when the function returns.
>> 
>> On a system with PAC enabled, it is expected that all applications will
>> be compiled with ROP protection. Fedora 33 and upwards already provide
>> this. By compiling for ARMv8.0, GCC and LLVM will only use the set of
>> PAC instructions that exist in the NOP space - on hardware without PAC,
>> these instructions act as NOPs, allowing backward compatibility for
>> negligible performance cost (2 NOPs per non-leaf function).
>> 
>> Hardware is currently limited to the Apple M1 MacBooks. All testing has
>> been done within a Fedora Docker image. A run of SpecJVM showed no
>> difference to that of noise - which was surprising.
>> 
>> The most important part of this patch is simply compiling using branch
>> protection provided by GCC/LLVM. This protects all C++ code from being
>> used in ROP attacks, removing all static ROP gadgets from use.
>> 
>> The remainder of the patch adds ROP protection to runtime generated
>> code, in both stubs and compiled Java code. Attacks here are much harder
>> as ROP gadgets must be found dynamically at runtime. If/when AOT
>> compilation is added to JDK, then all stubs and compiled Java will be
>> susceptible ROP gadgets being found by static analysis and therefore
>> potentially as vulnerable as C++ code.
>> 
>> There are a number of places where the VM changes control flow by
>> rewriting the stack or otherwise. I’ve done some analysis as to how
>> these could also be used for attacks (which I didn’t want to post here).
>> These areas can be protected ensuring the pointers to various stubs and
>> entry points are stored in memory as signed pointers. These changes are
>> simple to make (they can be reduced to a type change in common code and
>> a few addition sign/auth calls in the backend), but there a lot of them
>> and the total code change is fairly large. I’m happy to provide a few
>> work in progress patches.
>> 
>> In order to match the security benefits of the Apple Arm64e ABI across
>> the whole of JDK, then all the changes mentioned above would be
>> required.
>
> Alan Hayward has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Documentation updates

src/hotspot/cpu/aarch64/assembler_aarch64.hpp line 1163:

> 1161: #undef INSN
> 1162: 
> 1163:   // PAC branch instructions (with register modifier)

This section title is wrong. According to DDI0487G, the correct section title 
is "Unconditional branch (register)". All of the instructions in each section 
of this file should be grouped in the same way that they are in the Arm ARM.

src/hotspot/cpu/aarch64/frame_aarch64.cpp line 275:

> 273:   if (TracePcPatching) {
> 274: tty->print_cr("patch_pc at address " INTPTR_FORMAT " [" 
> INTPTR_FORMAT " -> " INTPTR_FORMAT "]",
> 275:   p2i(pc_addr), p2i(*pc_addr), p2i(signed_pc));

Let's see both pc and signed pc here.

src/hotspot/cpu/aarch64/macroAssembler_aarch64.cpp line 5293:

> 5291: // Create an additional frame for a function.
> 5292: void MacroAssembler::enter_subframe() {
> 5293:   // Addresses can only be signed once, so strip it first. PAC safe 
> because the value is not

This needs a more descriptive name. `enter_and_sign()` ? No, that's not right 
either. How do we come up with a name that's more descriptive?

-

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

Re: RFR: 8277204: Implement PAC-RET branch protection on Linux/AArch64 [v18]

[Andrew Haley]

On Thu, 3 Feb 2022 16:51:48 GMT, Alan Hayward  wrote:

>> PAC is an optional feature in AArch64 8.3 and is compulsory in v9. One
>> of its uses is to protect against ROP based attacks. This is done by
>> signing the Link Register whenever it is stored on the stack, and
>> authenticating the value when it is loaded back from the stack. If an
>> attacker were to try to change control flow by editing the stack then
>> the authentication check of the Link Register will fail, causing a
>> segfault when the function returns.
>> 
>> On a system with PAC enabled, it is expected that all applications will
>> be compiled with ROP protection. Fedora 33 and upwards already provide
>> this. By compiling for ARMv8.0, GCC and LLVM will only use the set of
>> PAC instructions that exist in the NOP space - on hardware without PAC,
>> these instructions act as NOPs, allowing backward compatibility for
>> negligible performance cost (2 NOPs per non-leaf function).
>> 
>> Hardware is currently limited to the Apple M1 MacBooks. All testing has
>> been done within a Fedora Docker image. A run of SpecJVM showed no
>> difference to that of noise - which was surprising.
>> 
>> The most important part of this patch is simply compiling using branch
>> protection provided by GCC/LLVM. This protects all C++ code from being
>> used in ROP attacks, removing all static ROP gadgets from use.
>> 
>> The remainder of the patch adds ROP protection to runtime generated
>> code, in both stubs and compiled Java code. Attacks here are much harder
>> as ROP gadgets must be found dynamically at runtime. If/when AOT
>> compilation is added to JDK, then all stubs and compiled Java will be
>> susceptible ROP gadgets being found by static analysis and therefore
>> potentially as vulnerable as C++ code.
>> 
>> There are a number of places where the VM changes control flow by
>> rewriting the stack or otherwise. I’ve done some analysis as to how
>> these could also be used for attacks (which I didn’t want to post here).
>> These areas can be protected ensuring the pointers to various stubs and
>> entry points are stored in memory as signed pointers. These changes are
>> simple to make (they can be reduced to a type change in common code and
>> a few addition sign/auth calls in the backend), but there a lot of them
>> and the total code change is fairly large. I’m happy to provide a few
>> work in progress patches.
>> 
>> In order to match the security benefits of the Apple Arm64e ABI across
>> the whole of JDK, then all the changes mentioned above would be
>> required.
>
> Alan Hayward has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Documentation updates

doc/building.md line 141:

> 139: 
> 140: In order to use Branch Protection features in the VM, 
> `--enable-branch-protection`
> 141: must be provided. This requires compiler support (GCC 9.1.0+ or Clang 
> 10+). The

Suggestion:

must be used. This option requires C++ compiler support (GCC 9.1.0+ or Clang 
10+). The

doc/building.md line 143:

> 141: must be provided. This requires compiler support (GCC 9.1.0+ or Clang 
> 10+). The
> 142: resulting build can be run on both machines with and without support for 
> branch
> 143: protection in hardware. This is only supported for Linux targets.

Suggestion:

protection in hardware. Branch Protection is only supported for Linux targets.

-

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

Re: RFR: 8277204: Implementation of JEP 8264130: PAC-RET protection for Linux/AArch64 [v17]

[Andrew Haley]

On Thu, 3 Feb 2022 12:11:16 GMT, Alan Hayward  wrote:

> As mentioned on the CSR, the JEP is being dropped - unless anyone has any 
> objections. JDK-8277204 will become a normal RFE.

Good decision.

-

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

Re: RFR: 8277204: Implementation of JEP 8264130: PAC-RET protection for Linux/AArch64 [v14]

[Andrew Haley]

On Wed, 2 Feb 2022 09:34:20 GMT, Alan Hayward  wrote:

> And this change will keep ROP protection enabled if we fall into the "this VM 
> was built without ROP-protection support.". In that case we'll be protecting 
> generated code, but the VM itself won't be protected. This will run without 
> crashing.

That's perfect.

-

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

Re: RFR: 8277204: Implementation of JEP 8264130: PAC-RET protection for Linux/AArch64 [v14]

[Andrew Haley]

On Tue, 1 Feb 2022 12:42:26 GMT, David Holmes  wrote:

>> As per this conversation: 
>> https://github.com/openjdk/jdk/pull/6334#discussion_r791722292
>> 
>> The idea was, the user is explicitly asking for asking for pac-ret so we 
>> should honour that. Whereas standard would only enable what is supported for 
>> that system.
>
> But we can't honour that because it is not supported. Further, the suggestion 
> in the referenced discussion seemed to be based on the assumption that doing 
> so would be harmless because it is NOP based, but you have indicated that may 
> not be the case and so it may actually lead to a crash!

Given that the implementation has now changed so much that it's no longer NOP 
based, I'll go with @dholmes-ora .
One other thing, though: it might be better to say here "but this VM was built 
without ROP-protection support." That's more informative, IMO.

-

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

Re: RFR: 8277204: Implementation of JEP 8264130: PAC-RET protection for Linux/AArch64 [v14]

[Andrew Haley]

On Mon, 31 Jan 2022 22:25:38 GMT, David Holmes  wrote:

>> Alan Hayward has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   Fix popframe failures
>
> src/hotspot/cpu/aarch64/vm_version_aarch64.cpp line 417:
> 
>> 415: // Enable PAC if this code has been built with branch-protection 
>> and the CPU/OS supports it.
>> 416: #ifdef __ARM_FEATURE_PAC_DEFAULT
>> 417: if (_features & CPU_PACA) {
> 
> Style nit: no implicit booleans - expand as "if ( A & B != 0)"

Oh yuck, really? This is my punishment for not paying attention to the style 
guide dicussions.

-

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

Re: RFR: 8277204: Implementation of JEP 8264130: PAC-RET protection for Linux/AArch64 [v14]

[Andrew Haley]

On Mon, 24 Jan 2022 15:56:06 GMT, Alan Hayward  wrote:

>> PAC is an optional feature in AArch64 8.3 and is compulsory in v9. One
>> of its uses is to protect against ROP based attacks. This is done by
>> signing the Link Register whenever it is stored on the stack, and
>> authenticating the value when it is loaded back from the stack. If an
>> attacker were to try to change control flow by editing the stack then
>> the authentication check of the Link Register will fail, causing a
>> segfault when the function returns.
>> 
>> On a system with PAC enabled, it is expected that all applications will
>> be compiled with ROP protection. Fedora 33 and upwards already provide
>> this. By compiling for ARMv8.0, GCC and LLVM will only use the set of
>> PAC instructions that exist in the NOP space - on hardware without PAC,
>> these instructions act as NOPs, allowing backward compatibility for
>> negligible performance cost (2 NOPs per non-leaf function).
>> 
>> Hardware is currently limited to the Apple M1 MacBooks. All testing has
>> been done within a Fedora Docker image. A run of SpecJVM showed no
>> difference to that of noise - which was surprising.
>> 
>> The most important part of this patch is simply compiling using branch
>> protection provided by GCC/LLVM. This protects all C++ code from being
>> used in ROP attacks, removing all static ROP gadgets from use.
>> 
>> The remainder of the patch adds ROP protection to runtime generated
>> code, in both stubs and compiled Java code. Attacks here are much harder
>> as ROP gadgets must be found dynamically at runtime. If/when AOT
>> compilation is added to JDK, then all stubs and compiled Java will be
>> susceptible ROP gadgets being found by static analysis and therefore
>> potentially as vulnerable as C++ code.
>> 
>> There are a number of places where the VM changes control flow by
>> rewriting the stack or otherwise. I’ve done some analysis as to how
>> these could also be used for attacks (which I didn’t want to post here).
>> These areas can be protected ensuring the pointers to various stubs and
>> entry points are stored in memory as signed pointers. These changes are
>> simple to make (they can be reduced to a type change in common code and
>> a few addition sign/auth calls in the backend), but there a lot of them
>> and the total code change is fairly large. I’m happy to provide a few
>> work in progress patches.
>> 
>> In order to match the security benefits of the Apple Arm64e ABI across
>> the whole of JDK, then all the changes mentioned above would be
>> required.
>
> Alan Hayward has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Fix popframe failures

Marked as reviewed by aph (Reviewer).

-

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

Re: RFR: 8277204: Implementation of JEP 8264130: PAC-RET protection for Linux/AArch64 [v9]

[Andrew Haley]

On Tue, 25 Jan 2022 14:22:15 GMT, Alan Hayward  wrote:

>> Was this ever resolved?
>
> Sort of. That code has changed quite a bit - UseROPProtection now is a string 
> field not a bool.
> "none" or not set - pac disabled.
> "pac-ret" - pac always enabled
> "standard" - pac enabled if the cpu+os support it.
> 
> Also, the pac instructions used aren't all in the NOP space. So, it will 
> crash on a non-pac machine.  It might be possible to change it so it does 
> only use nop space instructions, but I don't think it'll be optimal (need to 
> double check).

OK, cool.

-

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

Re: RFR: 8277204: Implementation of JEP 8264130: PAC-RET protection for Linux/AArch64 [v14]

[Andrew Haley]

On Mon, 24 Jan 2022 15:56:06 GMT, Alan Hayward  wrote:

>> PAC is an optional feature in AArch64 8.3 and is compulsory in v9. One
>> of its uses is to protect against ROP based attacks. This is done by
>> signing the Link Register whenever it is stored on the stack, and
>> authenticating the value when it is loaded back from the stack. If an
>> attacker were to try to change control flow by editing the stack then
>> the authentication check of the Link Register will fail, causing a
>> segfault when the function returns.
>> 
>> On a system with PAC enabled, it is expected that all applications will
>> be compiled with ROP protection. Fedora 33 and upwards already provide
>> this. By compiling for ARMv8.0, GCC and LLVM will only use the set of
>> PAC instructions that exist in the NOP space - on hardware without PAC,
>> these instructions act as NOPs, allowing backward compatibility for
>> negligible performance cost (2 NOPs per non-leaf function).
>> 
>> Hardware is currently limited to the Apple M1 MacBooks. All testing has
>> been done within a Fedora Docker image. A run of SpecJVM showed no
>> difference to that of noise - which was surprising.
>> 
>> The most important part of this patch is simply compiling using branch
>> protection provided by GCC/LLVM. This protects all C++ code from being
>> used in ROP attacks, removing all static ROP gadgets from use.
>> 
>> The remainder of the patch adds ROP protection to runtime generated
>> code, in both stubs and compiled Java code. Attacks here are much harder
>> as ROP gadgets must be found dynamically at runtime. If/when AOT
>> compilation is added to JDK, then all stubs and compiled Java will be
>> susceptible ROP gadgets being found by static analysis and therefore
>> potentially as vulnerable as C++ code.
>> 
>> There are a number of places where the VM changes control flow by
>> rewriting the stack or otherwise. I’ve done some analysis as to how
>> these could also be used for attacks (which I didn’t want to post here).
>> These areas can be protected ensuring the pointers to various stubs and
>> entry points are stored in memory as signed pointers. These changes are
>> simple to make (they can be reduced to a type change in common code and
>> a few addition sign/auth calls in the backend), but there a lot of them
>> and the total code change is fairly large. I’m happy to provide a few
>> work in progress patches.
>> 
>> In order to match the security benefits of the Apple Arm64e ABI across
>> the whole of JDK, then all the changes mentioned above would be
>> required.
>
> Alan Hayward has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Fix popframe failures

I've reverted my approval pending my question which wasn't resolved.

-

Changes requested by aph (Reviewer).

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

Re: RFR: 8277204: Implementation of JEP 8264130: PAC-RET protection for Linux/AArch64 [v9]

[Andrew Haley]

On Sat, 11 Dec 2021 09:30:32 GMT, Andrew Haley  wrote:

>> Ok, I think that's fine. How about on a non pac system allowing it for 
>> development only ?
>
> Maybe. Mind you, a lot of the time I'm looking at the output from production 
> systems.
> From a rather philosophical point of view, I assume that if the user of a 
> computer asks for something that isn't going to break anything or confuse 
> anyone, we should honour their request.

Was this ever resolved?

-

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

Re: RFR: 8277204: Implementation of JEP 8264130: PAC-RET protection for Linux/AArch64 [v2]

[Andrew Haley]

On Thu, 11 Nov 2021 18:15:08 GMT, Florian Weimer  wrote:

> > > > Am I right is saying that for Macos, all generated code is remapped RO 
> > > > before execution?
> > > 
> > > 
> > > Ah, no, it seems the code cache is not RWX all the time as far as Java 
> > > threads are concerned. The Macos/AArch64 code is strategically calling 
> > > pthread_jit_write_protect_np at Java <-> JVM transition points.
> > 
> > 
> > And this requires magic kernel support. I did mention it to a kernel 
> > engineer who wasn't very impressed, but I think it's pretty cool.
> 
> It's possible to emulate this to some extent with memory protection keys on 
> POWER and (recent) x86. See `pkey_alloc`.

I don't think this does exactly what we need, because (at least according to 
the docs) it does it for the whole process, not just the jit threads. Unless 
I've read the docs wrongly.

-

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

Re: [jdk18] RFR: 8279702: [macosx] ignore xcodebuild warnings on M1

[Andrew Haley]

On Fri, 14 Jan 2022 09:18:43 GMT, Johannes Bechberger  
wrote:

>> make/autoconf/toolchain.m4 line 237:
>> 
>>> 235: if test -n "$XCODEBUILD"; then
>>> 236:   # On Mac OS X, default toolchain to clang after Xcode 5
>>> 237:   XCODE_VERSION_OUTPUT=`"$XCODEBUILD" -version 2>&1 | $HEAD -n 1`
>> 
>> What's the warning? If it is truly expected, shouldn't it go to /dev/null ?
>
> The warning is
> 
> objc[3881]: Class AMSupportURLConnectionDelegate is implemented in both 
> /usr/lib/libauthinstall.dylib (0x1e32b6b90) and 
> /Library/Apple/System/Library/PrivateFrameworks/MobileDevice.framework/Versions/A/MobileDevice
>  (0x1087e42c8). One of the two will be used. Which one is undefined.
> objc[3881]: Class AMSupportURLSession is implemented in both 
> /usr/lib/libauthinstall.dylib (0x1e32b6be0) and 
> /Library/Apple/System/Library/PrivateFrameworks/MobileDevice.framework/Versions/A/MobileDevice
>  (0x1087e4318). One of the two will be used. Which one is undefined.
> 
> as noted in the issues referenced by my bug report and we can ignore it (as 
> build scripts of other projects do).
> 
> The problem with fully discarding the output is that there are other errors 
> that might cause the exit code to be non-zero and in this case displaying the 
> warning should be helpful for debugging.
> Of course one could either just remove the expected warning or only display 
> the error output when the return code is non-zero, but these alternatives 
> seemed to cumbersome for me (and the first alternative to error-prone).

OK. I've seen this a couple of times, and I hate warnings that can't be fixed. 
But it is what it is, thanks.

-

PR: https://git.openjdk.java.net/jdk18/pull/95

Re: [jdk18] RFR: 8279702: [macosx] ignore xcodebuild warnings on M1

[Andrew Haley]

On Wed, 12 Jan 2022 09:42:07 GMT, Johannes Bechberger  
wrote:

> Simple change to `make/autoconf/toolchain.m4` to fix the bug as described in 
> its title and description.

make/autoconf/toolchain.m4 line 237:

> 235: if test -n "$XCODEBUILD"; then
> 236:   # On Mac OS X, default toolchain to clang after Xcode 5
> 237:   XCODE_VERSION_OUTPUT=`"$XCODEBUILD" -version 2>&1 | $HEAD -n 1`

What's the warning? If it is truly expected, shouldn't it go to /dev/null ?

-

PR: https://git.openjdk.java.net/jdk18/pull/95

Re: Need OpenJDK to be used on PowerPC for our products.

[Andrew Haley]

On 1/7/22 08:38, Dipendu Ghosh wrote:

“configure: error: Could not find X11 libraries. You might be able to fix this 
by running 'sudo apt-get install libx11-dev libxext-dev libxrender-dev 
libxrandr-dev libxtst-dev libxt-dev'. configure exiting with result code 1”

The fix provided after the error is not solving the problem since the packages 
are already installed but somehow they are not being included in the sysroot.


It is your job to copy target versions of the packages into the sysroot.
The easiest way to do this is to install the packages into the root
filesystem of your target and then copy that filesystem into your
sysroot.

--
Andrew Haley  (he/him)
Java Platform Lead Engineer
Red Hat UK Ltd. <https://www.redhat.com>
https://keybase.io/andrewhaley
EAC8 43EB D3EF DB98 CC77 2FAD A5CD 6035 332F A671

Re: RFR: 8279445: Update JMH devkit to 1.34

[Andrew Haley]

On Tue, 4 Jan 2022 12:15:10 GMT, Aleksey Shipilev  wrote:

> Brings lots of goodies, including automatic enablement of Compiler 
> Blackholes: 
> https://mail.openjdk.java.net/pipermail/jmh-dev/2021-December/003406.html 
> 
> Additional testing:
>  - [x] Devkit creation works
>  - [x] Sample benchmarks runs with new devkit

Marked as reviewed by aph (Reviewer).

-

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

Re: RFR: 8277204: Implementation of JEP 8264130: PAC-RET protection for Linux/AArch64 [v11]

[Andrew Haley]

On Tue, 14 Dec 2021 09:40:03 GMT, Alan Hayward  wrote:

>> PAC is an optional feature in AArch64 8.3 and is compulsory in v9. One
>> of its uses is to protect against ROP based attacks. This is done by
>> signing the Link Register whenever it is stored on the stack, and
>> authenticating the value when it is loaded back from the stack. If an
>> attacker were to try to change control flow by editing the stack then
>> the authentication check of the Link Register will fail, causing a
>> segfault when the function returns.
>> 
>> On a system with PAC enabled, it is expected that all applications will
>> be compiled with ROP protection. Fedora 33 and upwards already provide
>> this. By compiling for ARMv8.0, GCC and LLVM will only use the set of
>> PAC instructions that exist in the NOP space - on hardware without PAC,
>> these instructions act as NOPs, allowing backward compatibility for
>> negligible performance cost (2 NOPs per non-leaf function).
>> 
>> Hardware is currently limited to the Apple M1 MacBooks. All testing has
>> been done within a Fedora Docker image. A run of SpecJVM showed no
>> difference to that of noise - which was surprising.
>> 
>> The most important part of this patch is simply compiling using branch
>> protection provided by GCC/LLVM. This protects all C++ code from being
>> used in ROP attacks, removing all static ROP gadgets from use.
>> 
>> The remainder of the patch adds ROP protection to runtime generated
>> code, in both stubs and compiled Java code. Attacks here are much harder
>> as ROP gadgets must be found dynamically at runtime. If/when AOT
>> compilation is added to JDK, then all stubs and compiled Java will be
>> susceptible ROP gadgets being found by static analysis and therefore
>> potentially as vulnerable as C++ code.
>> 
>> There are a number of places where the VM changes control flow by
>> rewriting the stack or otherwise. I’ve done some analysis as to how
>> these could also be used for attacks (which I didn’t want to post here).
>> These areas can be protected ensuring the pointers to various stubs and
>> entry points are stored in memory as signed pointers. These changes are
>> simple to make (they can be reduced to a type change in common code and
>> a few addition sign/auth calls in the backend), but there a lot of them
>> and the total code change is fairly large. I’m happy to provide a few
>> work in progress patches.
>> 
>> In order to match the security benefits of the Apple Arm64e ABI across
>> the whole of JDK, then all the changes mentioned above would be
>> required.
>
> Alan Hayward has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Change UseROPProtection to UseBranchProtection
>   
>   Change-Id: I31c5e1bb5c285262f262459c13057a46221682f1
>   CustomizedGitHooks: yes

Looks fine. I've done some basic testing on Graviton 3, which all seems to work.

-

Marked as reviewed by aph (Reviewer).

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

Re: RFR: 8277204: Implementation of JEP 8264130: PAC-RET protection for Linux/AArch64 [v10]

[Andrew Haley]

On Mon, 13 Dec 2021 09:50:26 GMT, Alan Hayward  wrote:

> You can support one without the other. The architecture allows you to have 
> one without the other. The GCC flag is an enum of 
> "none|standard|pac-ret[+leaf]|bti", with some of them changing depending on 
> which cpu you specify to -mcpu (8.0,8.3,8.5 etc). Clang has the same flags. 

OK, so we have a precedent.

> If your system had both, the only scenario I could see for only wanting just 
> one would be for test/dev purposes. In a real production scenario you would 
> want everything the system supports or nothing.

Yes.

> An earlier version of my code had a 
> UseBranchProtection="pac|bti|pac+bti|all|none" style option

That sounds great.

It seems to me that following the GCC/Clang precedent is the wisest thing we 
could do. I can see no possible advantage in diverging: it only confuses 
programmers.

-

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

Re: RFR: 8277204: Implementation of JEP 8264130: PAC-RET protection for Linux/AArch64 [v10]

[Andrew Haley]

On Sat, 11 Dec 2021 15:39:24 GMT, Florian Weimer  wrote:

>> src/hotspot/cpu/aarch64/globals_aarch64.hpp line 122:
>> 
>>> 120:   "It cannot be used with OnSpinWaitInst=none.")   
>>>  \
>>> 121:   range(1, 99) 
>>>  \
>>> 122:   product(bool, UseROPProtection, false,   
>>>  \
>> 
>> Question: this is called "UseROPProtection", the configure option is called 
>> "enable-branch-protection", and GCC option is called "-mbranch-protection". 
>> This is confusing. I would have thought we would want the same name, and use 
>> it for all branch protection. So why is this not "UseBranchProtection"?
>
> `-mbranch-protection` switches on both PAC-RET and BTI. This PR only covers a 
> use of PAC that looks very ROP-focused to me.

True, because we don't (yet) support BTI. Is there any point having two 
separate flags for BTI and PAC-RET? If someone wants one, they'll very likely 
want the other, won't they?

-

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

Re: RFR: 8277204: Implementation of JEP 8264130: PAC-RET protection for Linux/AArch64 [v10]

[Andrew Haley]

On Fri, 10 Dec 2021 15:14:47 GMT, Alan Hayward  wrote:

>> PAC is an optional feature in AArch64 8.3 and is compulsory in v9. One
>> of its uses is to protect against ROP based attacks. This is done by
>> signing the Link Register whenever it is stored on the stack, and
>> authenticating the value when it is loaded back from the stack. If an
>> attacker were to try to change control flow by editing the stack then
>> the authentication check of the Link Register will fail, causing a
>> segfault when the function returns.
>> 
>> On a system with PAC enabled, it is expected that all applications will
>> be compiled with ROP protection. Fedora 33 and upwards already provide
>> this. By compiling for ARMv8.0, GCC and LLVM will only use the set of
>> PAC instructions that exist in the NOP space - on hardware without PAC,
>> these instructions act as NOPs, allowing backward compatibility for
>> negligible performance cost (2 NOPs per non-leaf function).
>> 
>> Hardware is currently limited to the Apple M1 MacBooks. All testing has
>> been done within a Fedora Docker image. A run of SpecJVM showed no
>> difference to that of noise - which was surprising.
>> 
>> The most important part of this patch is simply compiling using branch
>> protection provided by GCC/LLVM. This protects all C++ code from being
>> used in ROP attacks, removing all static ROP gadgets from use.
>> 
>> The remainder of the patch adds ROP protection to runtime generated
>> code, in both stubs and compiled Java code. Attacks here are much harder
>> as ROP gadgets must be found dynamically at runtime. If/when AOT
>> compilation is added to JDK, then all stubs and compiled Java will be
>> susceptible ROP gadgets being found by static analysis and therefore
>> potentially as vulnerable as C++ code.
>> 
>> There are a number of places where the VM changes control flow by
>> rewriting the stack or otherwise. I’ve done some analysis as to how
>> these could also be used for attacks (which I didn’t want to post here).
>> These areas can be protected ensuring the pointers to various stubs and
>> entry points are stored in memory as signed pointers. These changes are
>> simple to make (they can be reduced to a type change in common code and
>> a few addition sign/auth calls in the backend), but there a lot of them
>> and the total code change is fairly large. I’m happy to provide a few
>> work in progress patches.
>> 
>> In order to match the security benefits of the Apple Arm64e ABI across
>> the whole of JDK, then all the changes mentioned above would be
>> required.
>
> Alan Hayward has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Remove BSD/Apple specific code

src/hotspot/cpu/aarch64/globals_aarch64.hpp line 122:

> 120:   "It cannot be used with OnSpinWaitInst=none.")\
> 121:   range(1, 99)  \
> 122:   product(bool, UseROPProtection, false,\

Question: this is called "UseROPProtection", the configure option is called 
"enable-branch-protection", and GCC option is called "-mbranch-protection". 
This is confusing. I would have thought we would want the same name, and use it 
for all branch protection. So why is this not "UseBranchProtection"?

-

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

Re: RFR: 8277204: Implementation of JEP 8264130: PAC-RET protection for Linux/AArch64 [v9]

[Andrew Haley]

On Fri, 10 Dec 2021 15:16:19 GMT, Alan Hayward  wrote:

>> src/hotspot/cpu/aarch64/vm_version_aarch64.cpp line 419:
>> 
>>> 417: if (UseROPProtection) {
>>> 418:   warning("UseROPProtection specified, but not supported on this 
>>> CPU.");
>>> 419:   FLAG_SET_DEFAULT(UseROPProtection, false);
>> 
>> Suggestion:
>> 
>>   FLAG_SET_DEFAULT(UseROPProtection, true);
>> 
>> Given that the instructions used are in NOP space, this won't do any harm, 
>> and it will allow developers without PAC-enabled systems to see what code 
>> PAC would generate.
>
> Ok, I think that's fine. How about on a non pac system allowing it for 
> development only ?

Maybe. Mind you, a lot of the time I'm looking at the output from production 
systems.
>From a rather philosophical point of view, I assume that if the user of a 
>computer asks for something that isn't going to break anything or confuse 
>anyone, we should honour their request.

-

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

Re: RFR: 8277204: Implementation of JEP 8264130: PAC-RET protection for Linux/AArch64 [v9]

[Andrew Haley]

On Fri, 10 Dec 2021 12:39:50 GMT, Alan Hayward  wrote:

>> PAC is an optional feature in AArch64 8.3 and is compulsory in v9. One
>> of its uses is to protect against ROP based attacks. This is done by
>> signing the Link Register whenever it is stored on the stack, and
>> authenticating the value when it is loaded back from the stack. If an
>> attacker were to try to change control flow by editing the stack then
>> the authentication check of the Link Register will fail, causing a
>> segfault when the function returns.
>> 
>> On a system with PAC enabled, it is expected that all applications will
>> be compiled with ROP protection. Fedora 33 and upwards already provide
>> this. By compiling for ARMv8.0, GCC and LLVM will only use the set of
>> PAC instructions that exist in the NOP space - on hardware without PAC,
>> these instructions act as NOPs, allowing backward compatibility for
>> negligible performance cost (2 NOPs per non-leaf function).
>> 
>> Hardware is currently limited to the Apple M1 MacBooks. All testing has
>> been done within a Fedora Docker image. A run of SpecJVM showed no
>> difference to that of noise - which was surprising.
>> 
>> The most important part of this patch is simply compiling using branch
>> protection provided by GCC/LLVM. This protects all C++ code from being
>> used in ROP attacks, removing all static ROP gadgets from use.
>> 
>> The remainder of the patch adds ROP protection to runtime generated
>> code, in both stubs and compiled Java code. Attacks here are much harder
>> as ROP gadgets must be found dynamically at runtime. If/when AOT
>> compilation is added to JDK, then all stubs and compiled Java will be
>> susceptible ROP gadgets being found by static analysis and therefore
>> potentially as vulnerable as C++ code.
>> 
>> There are a number of places where the VM changes control flow by
>> rewriting the stack or otherwise. I’ve done some analysis as to how
>> these could also be used for attacks (which I didn’t want to post here).
>> These areas can be protected ensuring the pointers to various stubs and
>> entry points are stored in memory as signed pointers. These changes are
>> simple to make (they can be reduced to a type change in common code and
>> a few addition sign/auth calls in the backend), but there a lot of them
>> and the total code change is fairly large. I’m happy to provide a few
>> work in progress patches.
>> 
>> In order to match the security benefits of the Apple Arm64e ABI across
>> the whole of JDK, then all the changes mentioned above would be
>> required.
>
> Alan Hayward has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Default to building without branch-protection

src/hotspot/cpu/aarch64/vm_version_aarch64.cpp line 419:

> 417: if (UseROPProtection) {
> 418:   warning("UseROPProtection specified, but not supported on this 
> CPU.");
> 419:   FLAG_SET_DEFAULT(UseROPProtection, false);

Suggestion:

  FLAG_SET_DEFAULT(UseROPProtection, true);

Given that the instructions used are in NOP space, this won't do any harm, and 
it will allow developers without PAC-enabled systems to see what code PAC would 
generate.

-

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

Re: RFR: 8277204: Implementation of JEP 8264130: PAC-RET protection for Linux/AArch64 [v8]

[Andrew Haley]

On Thu, 2 Dec 2021 09:20:53 GMT, Alan Hayward  wrote:

>> PAC is an optional feature in AArch64 8.3 and is compulsory in v9. One
>> of its uses is to protect against ROP based attacks. This is done by
>> signing the Link Register whenever it is stored on the stack, and
>> authenticating the value when it is loaded back from the stack. If an
>> attacker were to try to change control flow by editing the stack then
>> the authentication check of the Link Register will fail, causing a
>> segfault when the function returns.
>> 
>> On a system with PAC enabled, it is expected that all applications will
>> be compiled with ROP protection. Fedora 33 and upwards already provide
>> this. By compiling for ARMv8.0, GCC and LLVM will only use the set of
>> PAC instructions that exist in the NOP space - on hardware without PAC,
>> these instructions act as NOPs, allowing backward compatibility for
>> negligible performance cost (2 NOPs per non-leaf function).
>> 
>> Hardware is currently limited to the Apple M1 MacBooks. All testing has
>> been done within a Fedora Docker image. A run of SpecJVM showed no
>> difference to that of noise - which was surprising.
>> 
>> The most important part of this patch is simply compiling using branch
>> protection provided by GCC/LLVM. This protects all C++ code from being
>> used in ROP attacks, removing all static ROP gadgets from use.
>> 
>> The remainder of the patch adds ROP protection to runtime generated
>> code, in both stubs and compiled Java code. Attacks here are much harder
>> as ROP gadgets must be found dynamically at runtime. If/when AOT
>> compilation is added to JDK, then all stubs and compiled Java will be
>> susceptible ROP gadgets being found by static analysis and therefore
>> potentially as vulnerable as C++ code.
>> 
>> There are a number of places where the VM changes control flow by
>> rewriting the stack or otherwise. I’ve done some analysis as to how
>> these could also be used for attacks (which I didn’t want to post here).
>> These areas can be protected ensuring the pointers to various stubs and
>> entry points are stored in memory as signed pointers. These changes are
>> simple to make (they can be reduced to a type change in common code and
>> a few addition sign/auth calls in the backend), but there a lot of them
>> and the total code change is fairly large. I’m happy to provide a few
>> work in progress patches.
>> 
>> In order to match the security benefits of the Apple Arm64e ABI across
>> the whole of JDK, then all the changes mentioned above would be
>> required.
>
> Alan Hayward has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Fix up UseROPProtection flag

make/autoconf/flags-cflags.m4 line 902:

> 900:   BRANCH_PROTECTION_CFLAGS=""
> 901:   UTIL_ARG_ENABLE(NAME: branch-protection, DEFAULT: auto,
> 902:   RESULT: USE_BRANCH_PROTECTION, AVAILABLE: 
> $BRANCH_PROTECTION_AVAILABLE,

What exactly is going on here? Is it that if the host compiler has "branch 
protection" supported, we will build with it on by default? if so, no, we don't 
want to do that. For now, branch protection should be an explicit opt in.

-

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

Re: RFR: 8264130: PAC-RET protection for Linux/AArch64 [v4]

[Andrew Haley]

On Mon, 15 Nov 2021 11:21:37 GMT, Alan Hayward  wrote:

>> src/hotspot/cpu/aarch64/c1_Runtime1_aarch64.cpp line 452:
>> 
>>> 450:   // patch the return address, this stub will directly return to the 
>>> exception handler
>>> 451:   __ str(r0, Address(rfp, 1*BytesPerWord));
>>> 452: 
>> 
>> Please explain the reason for this change, that leaves `lr` live across 
>> `restore_live_registers()`.
>
> In the original code:
> *save r0 to the lr location on the stack
> *restore_live_registers
> *Standard return: remove stack frame, load lr and fp off the stack, jump to 
> lr.
>  
> With PAC it would now be:
> *Sign r0 then save it to the lr location on the stack
> *restore_live_registers
> *Standard return: remove stack frame, load lr and fp off the stack, auth lr, 
> jump to lr.
> 
> After reading the code in restore_live_registers, it doesn't touch lr and so 
> seemed odd to have the save to the stack, only to restore it directly 
> afterwards.

That's an optimization, though. You shouldn't need to read the code in 
`restore_live_registers()` to see if it's safe to keep the return address in 
LR: at best it's pathological coupling, in the sense that the correctness of 
this code depends on the internal details of  `restore_live_registers()`. Let's 
keep LR live ranges as short as possible.

-

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

Re: RFR: 8264130: PAC-RET protection for Linux/AArch64 [v4]

[Andrew Haley]

On Mon, 15 Nov 2021 10:58:06 GMT, Alan Hayward  wrote:

>> src/hotspot/cpu/aarch64/pauth_aarch64.hpp line 132:
>> 
>>> 130: // Authenticate or strip a return value. Use for efficiency and only 
>>> when the safety of the data
>>> 131: // isn't an issue - for example when viewing the stack.
>>> 132: //
>> 
>> So, whether this function authenticates or strips the address depends only 
>> on debugging? The vague name makes the callers hard to read.
>
>>whether this function authenticates or strips the address depends only on 
>>debugging?
> 
> Yes. We only need to strip the value, because we're not jumping to the lr 
> value, only viewing it.
> 
> The interface is different to a strip (as we need to pass in the modifier). 
> 
> How about something like pauth_authenticate_fast() ? or 
> pauth_authenticate_unsafe() ?
> 
> Alternatively, this function is only called by the functions in Frame, so the 
> frequency of use is probably low enough (compared to the sign/auth every 
> function) that it's not going to cause any performance issues. So, could just 
> replace with calls to pauth_authenticate. I think that might be the best 
> option.

A simple rule here: function names go with what the release version does. So 
I'd go with the actual purpose, which is `pauth_strip_addr_for_debuginfo()`. 
That's right, isn't it? You only want this thing for stack traces, logs, etc.

-

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

Re: RFR: 8264130: PAC-RET protection for Linux/AArch64 [v4]

[Andrew Haley]

On Mon, 15 Nov 2021 09:07:11 GMT, Alan Hayward  wrote:

>> PAC is an optional feature in AArch64 8.3 and is compulsory in v9. One
>> of its uses is to protect against ROP based attacks. This is done by
>> signing the Link Register whenever it is stored on the stack, and
>> authenticating the value when it is loaded back from the stack. If an
>> attacker were to try to change control flow by editing the stack then
>> the authentication check of the Link Register will fail, causing a
>> segfault when the function returns.
>> 
>> On a system with PAC enabled, it is expected that all applications will
>> be compiled with ROP protection. Fedora 33 and upwards already provide
>> this. By compiling for ARMv8.0, GCC and LLVM will only use the set of
>> PAC instructions that exist in the NOP space - on hardware without PAC,
>> these instructions act as NOPs, allowing backward compatibility for
>> negligible performance cost (2 NOPs per non-leaf function).
>> 
>> Hardware is currently limited to the Apple M1 MacBooks. All testing has
>> been done within a Fedora Docker image. A run of SpecJVM showed no
>> difference to that of noise - which was surprising.
>> 
>> The most important part of this patch is simply compiling using branch
>> protection provided by GCC/LLVM. This protects all C++ code from being
>> used in ROP attacks, removing all static ROP gadgets from use.
>> 
>> The remainder of the patch adds ROP protection to runtime generated
>> code, in both stubs and compiled Java code. Attacks here are much harder
>> as ROP gadgets must be found dynamically at runtime. If/when AOT
>> compilation is added to JDK, then all stubs and compiled Java will be
>> susceptible ROP gadgets being found by static analysis and therefore
>> potentially as vulnerable as C++ code.
>> 
>> There are a number of places where the VM changes control flow by
>> rewriting the stack or otherwise. I’ve done some analysis as to how
>> these could also be used for attacks (which I didn’t want to post here).
>> These areas can be protected ensuring the pointers to various stubs and
>> entry points are stored in memory as signed pointers. These changes are
>> simple to make (they can be reduced to a type change in common code and
>> a few addition sign/auth calls in the backend), but there a lot of them
>> and the total code change is fairly large. I’m happy to provide a few
>> work in progress patches.
>> 
>> In order to match the security benefits of the Apple Arm64e ABI across
>> the whole of JDK, then all the changes mentioned above would be
>> required.
>
> Alan Hayward has updated the pull request with a new target base due to a 
> merge or a rebase. The pull request now contains eight commits:
> 
>  - Merge master
>  - Document pauth functions && remove OS split
>  - Update UseROPProtection description
>  - Simplify branch protection configure check
>  - 8264130: PAC-RET protection for Linux/AArch64
>
>PAC is an optional feature in AArch64 8.3 and is compulsory in v9. One
>of its uses is to protect against ROP based attacks. This is done by
>signing the Link Register whenever it is stored on the stack, and
>authenticating the value when it is loaded back from the stack. If an
>attacker were to try to change control flow by editing the stack then
>the authentication check of the Link Register will fail, causing a
>segfault when the function returns.
>
>On a system with PAC enabled, it is expected that all applications will
>be compiled with ROP protection. Fedora 33 and upwards already provide
>this. By compiling for ARMv8.0, GCC and LLVM will only use the set of
>PAC instructions that exist in the NOP space - on hardware without PAC,
>these instructions act as NOPs, allowing backward compatibility for
>negligible performance cost (2 NOPs per non-leaf function).
>
>Hardware is currently limited to the Apple M1 MacBooks. All testing has
>been done within a Fedora Docker image. A run of SpecJVM showed no
>difference to that of noise - which was surprising.
>
>The most important part of this patch is simply compiling using branch
>protection provided by GCC/LLVM. This protects all C++ code from being
>used in ROP attacks, removing all static ROP gadgets from use.
>
>The remainder of the patch adds ROP protection to runtime generated
>code, in both stubs and compiled Java code. Attacks here are much harder
>as ROP gadgets must be found dynamically at runtime. If/when AOT
>compilation is added to JDK, then all stubs and compiled Java will be
>susceptible ROP gadgets being found by static analysis and therefore
>potentially as vulnerable as C++ code.
>
>There are a number of places where the VM changes control flow by
>rewriting the stack or otherwise. I’ve done some analysis as to how
>these could also be used for attacks (which I didn’t want to post here).
>These areas can be protected ensuring the 

Re: RFR: 8264130: PAC-RET protection for Linux/AArch64 [v4]

[Andrew Haley]

On Mon, 15 Nov 2021 09:07:11 GMT, Alan Hayward  wrote:

>> PAC is an optional feature in AArch64 8.3 and is compulsory in v9. One
>> of its uses is to protect against ROP based attacks. This is done by
>> signing the Link Register whenever it is stored on the stack, and
>> authenticating the value when it is loaded back from the stack. If an
>> attacker were to try to change control flow by editing the stack then
>> the authentication check of the Link Register will fail, causing a
>> segfault when the function returns.
>> 
>> On a system with PAC enabled, it is expected that all applications will
>> be compiled with ROP protection. Fedora 33 and upwards already provide
>> this. By compiling for ARMv8.0, GCC and LLVM will only use the set of
>> PAC instructions that exist in the NOP space - on hardware without PAC,
>> these instructions act as NOPs, allowing backward compatibility for
>> negligible performance cost (2 NOPs per non-leaf function).
>> 
>> Hardware is currently limited to the Apple M1 MacBooks. All testing has
>> been done within a Fedora Docker image. A run of SpecJVM showed no
>> difference to that of noise - which was surprising.
>> 
>> The most important part of this patch is simply compiling using branch
>> protection provided by GCC/LLVM. This protects all C++ code from being
>> used in ROP attacks, removing all static ROP gadgets from use.
>> 
>> The remainder of the patch adds ROP protection to runtime generated
>> code, in both stubs and compiled Java code. Attacks here are much harder
>> as ROP gadgets must be found dynamically at runtime. If/when AOT
>> compilation is added to JDK, then all stubs and compiled Java will be
>> susceptible ROP gadgets being found by static analysis and therefore
>> potentially as vulnerable as C++ code.
>> 
>> There are a number of places where the VM changes control flow by
>> rewriting the stack or otherwise. I’ve done some analysis as to how
>> these could also be used for attacks (which I didn’t want to post here).
>> These areas can be protected ensuring the pointers to various stubs and
>> entry points are stored in memory as signed pointers. These changes are
>> simple to make (they can be reduced to a type change in common code and
>> a few addition sign/auth calls in the backend), but there a lot of them
>> and the total code change is fairly large. I’m happy to provide a few
>> work in progress patches.
>> 
>> In order to match the security benefits of the Apple Arm64e ABI across
>> the whole of JDK, then all the changes mentioned above would be
>> required.
>
> Alan Hayward has updated the pull request with a new target base due to a 
> merge or a rebase. The pull request now contains eight commits:
> 
>  - Merge master
>  - Document pauth functions && remove OS split
>  - Update UseROPProtection description
>  - Simplify branch protection configure check
>  - 8264130: PAC-RET protection for Linux/AArch64
>
>PAC is an optional feature in AArch64 8.3 and is compulsory in v9. One
>of its uses is to protect against ROP based attacks. This is done by
>signing the Link Register whenever it is stored on the stack, and
>authenticating the value when it is loaded back from the stack. If an
>attacker were to try to change control flow by editing the stack then
>the authentication check of the Link Register will fail, causing a
>segfault when the function returns.
>
>On a system with PAC enabled, it is expected that all applications will
>be compiled with ROP protection. Fedora 33 and upwards already provide
>this. By compiling for ARMv8.0, GCC and LLVM will only use the set of
>PAC instructions that exist in the NOP space - on hardware without PAC,
>these instructions act as NOPs, allowing backward compatibility for
>negligible performance cost (2 NOPs per non-leaf function).
>
>Hardware is currently limited to the Apple M1 MacBooks. All testing has
>been done within a Fedora Docker image. A run of SpecJVM showed no
>difference to that of noise - which was surprising.
>
>The most important part of this patch is simply compiling using branch
>protection provided by GCC/LLVM. This protects all C++ code from being
>used in ROP attacks, removing all static ROP gadgets from use.
>
>The remainder of the patch adds ROP protection to runtime generated
>code, in both stubs and compiled Java code. Attacks here are much harder
>as ROP gadgets must be found dynamically at runtime. If/when AOT
>compilation is added to JDK, then all stubs and compiled Java will be
>susceptible ROP gadgets being found by static analysis and therefore
>potentially as vulnerable as C++ code.
>
>There are a number of places where the VM changes control flow by
>rewriting the stack or otherwise. I’ve done some analysis as to how
>these could also be used for attacks (which I didn’t want to post here).
>These areas can be protected ensuring the 

Re: RFR: 8264130: PAC-RET protection for Linux/AArch64 [v4]

[Andrew Haley]

On Mon, 15 Nov 2021 09:07:11 GMT, Alan Hayward  wrote:

>> PAC is an optional feature in AArch64 8.3 and is compulsory in v9. One
>> of its uses is to protect against ROP based attacks. This is done by
>> signing the Link Register whenever it is stored on the stack, and
>> authenticating the value when it is loaded back from the stack. If an
>> attacker were to try to change control flow by editing the stack then
>> the authentication check of the Link Register will fail, causing a
>> segfault when the function returns.
>> 
>> On a system with PAC enabled, it is expected that all applications will
>> be compiled with ROP protection. Fedora 33 and upwards already provide
>> this. By compiling for ARMv8.0, GCC and LLVM will only use the set of
>> PAC instructions that exist in the NOP space - on hardware without PAC,
>> these instructions act as NOPs, allowing backward compatibility for
>> negligible performance cost (2 NOPs per non-leaf function).
>> 
>> Hardware is currently limited to the Apple M1 MacBooks. All testing has
>> been done within a Fedora Docker image. A run of SpecJVM showed no
>> difference to that of noise - which was surprising.
>> 
>> The most important part of this patch is simply compiling using branch
>> protection provided by GCC/LLVM. This protects all C++ code from being
>> used in ROP attacks, removing all static ROP gadgets from use.
>> 
>> The remainder of the patch adds ROP protection to runtime generated
>> code, in both stubs and compiled Java code. Attacks here are much harder
>> as ROP gadgets must be found dynamically at runtime. If/when AOT
>> compilation is added to JDK, then all stubs and compiled Java will be
>> susceptible ROP gadgets being found by static analysis and therefore
>> potentially as vulnerable as C++ code.
>> 
>> There are a number of places where the VM changes control flow by
>> rewriting the stack or otherwise. I’ve done some analysis as to how
>> these could also be used for attacks (which I didn’t want to post here).
>> These areas can be protected ensuring the pointers to various stubs and
>> entry points are stored in memory as signed pointers. These changes are
>> simple to make (they can be reduced to a type change in common code and
>> a few addition sign/auth calls in the backend), but there a lot of them
>> and the total code change is fairly large. I’m happy to provide a few
>> work in progress patches.
>> 
>> In order to match the security benefits of the Apple Arm64e ABI across
>> the whole of JDK, then all the changes mentioned above would be
>> required.
>
> Alan Hayward has updated the pull request with a new target base due to a 
> merge or a rebase. The pull request now contains eight commits:
> 
>  - Merge master
>  - Document pauth functions && remove OS split
>  - Update UseROPProtection description
>  - Simplify branch protection configure check
>  - 8264130: PAC-RET protection for Linux/AArch64
>
>PAC is an optional feature in AArch64 8.3 and is compulsory in v9. One
>of its uses is to protect against ROP based attacks. This is done by
>signing the Link Register whenever it is stored on the stack, and
>authenticating the value when it is loaded back from the stack. If an
>attacker were to try to change control flow by editing the stack then
>the authentication check of the Link Register will fail, causing a
>segfault when the function returns.
>
>On a system with PAC enabled, it is expected that all applications will
>be compiled with ROP protection. Fedora 33 and upwards already provide
>this. By compiling for ARMv8.0, GCC and LLVM will only use the set of
>PAC instructions that exist in the NOP space - on hardware without PAC,
>these instructions act as NOPs, allowing backward compatibility for
>negligible performance cost (2 NOPs per non-leaf function).
>
>Hardware is currently limited to the Apple M1 MacBooks. All testing has
>been done within a Fedora Docker image. A run of SpecJVM showed no
>difference to that of noise - which was surprising.
>
>The most important part of this patch is simply compiling using branch
>protection provided by GCC/LLVM. This protects all C++ code from being
>used in ROP attacks, removing all static ROP gadgets from use.
>
>The remainder of the patch adds ROP protection to runtime generated
>code, in both stubs and compiled Java code. Attacks here are much harder
>as ROP gadgets must be found dynamically at runtime. If/when AOT
>compilation is added to JDK, then all stubs and compiled Java will be
>susceptible ROP gadgets being found by static analysis and therefore
>potentially as vulnerable as C++ code.
>
>There are a number of places where the VM changes control flow by
>rewriting the stack or otherwise. I’ve done some analysis as to how
>these could also be used for attacks (which I didn’t want to post here).
>These areas can be protected ensuring the 

Re: RFR: 8264130: PAC-RET protection for Linux/AArch64 [v3]

[Andrew Haley]

On Fri, 12 Nov 2021 16:18:04 GMT, Alan Hayward  wrote:

>> PAC is an optional feature in AArch64 8.3 and is compulsory in v9. One
>> of its uses is to protect against ROP based attacks. This is done by
>> signing the Link Register whenever it is stored on the stack, and
>> authenticating the value when it is loaded back from the stack. If an
>> attacker were to try to change control flow by editing the stack then
>> the authentication check of the Link Register will fail, causing a
>> segfault when the function returns.
>> 
>> On a system with PAC enabled, it is expected that all applications will
>> be compiled with ROP protection. Fedora 33 and upwards already provide
>> this. By compiling for ARMv8.0, GCC and LLVM will only use the set of
>> PAC instructions that exist in the NOP space - on hardware without PAC,
>> these instructions act as NOPs, allowing backward compatibility for
>> negligible performance cost (2 NOPs per non-leaf function).
>> 
>> Hardware is currently limited to the Apple M1 MacBooks. All testing has
>> been done within a Fedora Docker image. A run of SpecJVM showed no
>> difference to that of noise - which was surprising.
>> 
>> The most important part of this patch is simply compiling using branch
>> protection provided by GCC/LLVM. This protects all C++ code from being
>> used in ROP attacks, removing all static ROP gadgets from use.
>> 
>> The remainder of the patch adds ROP protection to runtime generated
>> code, in both stubs and compiled Java code. Attacks here are much harder
>> as ROP gadgets must be found dynamically at runtime. If/when AOT
>> compilation is added to JDK, then all stubs and compiled Java will be
>> susceptible ROP gadgets being found by static analysis and therefore
>> potentially as vulnerable as C++ code.
>> 
>> There are a number of places where the VM changes control flow by
>> rewriting the stack or otherwise. I’ve done some analysis as to how
>> these could also be used for attacks (which I didn’t want to post here).
>> These areas can be protected ensuring the pointers to various stubs and
>> entry points are stored in memory as signed pointers. These changes are
>> simple to make (they can be reduced to a type change in common code and
>> a few addition sign/auth calls in the backend), but there a lot of them
>> and the total code change is fairly large. I’m happy to provide a few
>> work in progress patches.
>> 
>> In order to match the security benefits of the Apple Arm64e ABI across
>> the whole of JDK, then all the changes mentioned above would be
>> required.
>
> Alan Hayward has updated the pull request incrementally with two additional 
> commits since the last revision:
> 
>  - Document pauth functions && remove OS split
>  - Update UseROPProtection description

src/hotspot/cpu/aarch64/macroAssembler_aarch64.cpp line 5254:

> 5252: // Also use before signing to check that the pointer is valid and 
> hasn't already been signed.
> 5253: //
> 5254: void MacroAssembler::check_return_address(Register return_reg) {

This commentary is excellent. Thanks.

-

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

Re: RFR: 8264130: PAC-RET protection for Linux/AArch64 [v2]

[Andrew Haley]

On Thu, 11 Nov 2021 16:31:41 GMT, Andrew Dinn  wrote:

> > Am I right is saying that for Macos, all generated code is remapped RO 
> > before execution?
> 
> Ah, no, it seems the code cache is not RWX all the time as far as Java 
> threads are concerned. The Macos/AArch64 code is strategically calling 
> pthread_jit_write_protect_np at Java <-> JVM transition points.

And this requires magic kernel support. I did mention it to a kernel engineer 
who wasn't very impressed, but I think it's pretty cool.

-

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

Re: RFR: 8264130: PAC-RET protection for Linux/AArch64 [v2]

[Andrew Haley]

On Thu, 11 Nov 2021 14:59:32 GMT, Andrew Dinn  wrote:

>> I did mean the description, not the flag name.
>
> Yes, understood. I too was talking about the description even though I 
> introduced my comment by talking about what the flag does.

`"Protect branches against ROP attacks".`

-

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

Re: RFR: 8264130: PAC-RET protection for Linux/AArch64 [v2]

[Andrew Haley]

On Thu, 11 Nov 2021 11:52:46 GMT, Andrew Haley  wrote:

>> I'm thinking for references to the Arm Arm to use header titles instead of 
>> section numbers, as the titles should be more stable.
>> 
>> Also probably need some description around the code in the pauth_aarch64.hpp 
>> too. But I want to make sure I'm not duplicating comments - maybe the 
>> macroassembler comments should point to the pauth_aarch64 comments.
>> 
>> It didn't seen common in the code to describe instruction functionality, 
>> which is why I didn't add any. Agreed it needs something added though.
>
> Yeah. At the definitions of `authenticate_return_address()` et al you can say 
> what you expect in the normal case and what you expect when you've been 
> hacked, along with an overview. I realize that it was a bit tricky to make 
> this work with HotSpot because we're synthesizing return addresses just like 
> hackers do, so a comment where we're patching return addresses would be nice.
> 
> As long as the instructions are easily findable in the docs that's good.

Just to be clear: no, don't describe instructions. describe what the macros do, 
and when to use them. Imagine that you, the reader can't see the contents of 
the macro at all, just the name and the comments.

-

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

Re: RFR: 8264130: PAC-RET protection for Linux/AArch64 [v2]

[Andrew Haley]

On Thu, 11 Nov 2021 11:44:09 GMT, Alan Hayward  wrote:

>> Correction:
>> Using the most up to date ARM ARM G  [ARM DDI 0487G.a (ID011921)]
>> 
>> - The PAC functionality is described in ARM-ARM Section D5.1.5
>> - Overview of the PAC instructions is provided in section C3.1.10
>> - Detailed PAC instruction descriptions are provided in C6.2.208 - C6.2.212
>
> I'm thinking for references to the Arm Arm to use header titles instead of 
> section numbers, as the titles should be more stable.
> 
> Also probably need some description around the code in the pauth_aarch64.hpp 
> too. But I want to make sure I'm not duplicating comments - maybe the 
> macroassembler comments should point to the pauth_aarch64 comments.
> 
> It didn't seen common in the code to describe instruction functionality, 
> which is why I didn't add any. Agreed it needs something added though.

Yeah. At the definitions of `authenticate_return_address()` et al you can say 
what you expect in the normal case and what you expect when you've been hacked, 
along with an overview. I realize that it was a bit tricky to make this work 
with HotSpot because we're synthesizing return addresses just like hackers do, 
so a comment where we're patching return addresses would be nice.

As long as the instructions are easily findable in the docs that's good.

-

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

Re: RFR: 8264130: PAC-RET protection for Linux/AArch64

[Andrew Haley]

On Wed, 10 Nov 2021 12:32:53 GMT, Alan Hayward  wrote:

> PAC is an optional feature in AArch64 8.3 and is compulsory in v9. One
> of its uses is to protect against ROP based attacks. This is done by
> signing the Link Register whenever it is stored on the stack, and
> authenticating the value when it is loaded back from the stack. If an
> attacker were to try to change control flow by editing the stack then
> the authentication check of the Link Register will fail, causing a
> segfault when the function returns.
> 
> On a system with PAC enabled, it is expected that all applications will
> be compiled with ROP protection. Fedora 33 and upwards already provide
> this. By compiling for ARMv8.0, GCC and LLVM will only use the set of
> PAC instructions that exist in the NOP space - on hardware without PAC,
> these instructions act as NOPs, allowing backward compatibility for
> negligible performance cost (2 NOPs per non-leaf function).
> 
> Hardware is currently limited to the Apple M1 MacBooks. All testing has
> been done within a Fedora Docker image. A run of SpecJVM showed no
> difference to that of noise - which was surprising.
> 
> The most important part of this patch is simply compiling using branch
> protection provided by GCC/LLVM. This protects all C++ code from being
> used in ROP attacks, removing all static ROP gadgets from use.
> 
> The remainder of the patch adds ROP protection to runtime generated
> code, in both stubs and compiled Java code. Attacks here are much harder
> as ROP gadgets must be found dynamically at runtime. If/when AOT
> compilation is added to JDK, then all stubs and compiled Java will be
> susceptible ROP gadgets being found by static analysis and therefore
> potentially as vulnerable as C++ code.
> 
> There are a number of places where the VM changes control flow by
> rewriting the stack or otherwise. I’ve done some analysis as to how
> these could also be used for attacks (which I didn’t want to post here).
> These areas can be protected ensuring the pointers to various stubs and
> entry points are stored in memory as signed pointers. These changes are
> simple to make (they can be reduced to a type change in common code and
> a few addition sign/auth calls in the backend), but there a lot of them
> and the total code change is fairly large. I’m happy to provide a few
> work in progress patches.
> 
> In order to match the security benefits of the Apple Arm64e ABI across
> the whole of JDK, then all the changes mentioned above would be
> required.

src/hotspot/os_cpu/bsd_aarch64/pauth_bsd_aarch64.inline.hpp line 25:

> 23:  */
> 24: 
> 25: #ifndef OS_CPU_BSD_AARCH64_PAUTH_BSD_AARCH64_INLINE_HPP

Are these two files different enough to separate them for BSD and Linux?

-

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

Re: RFR: 8264130: PAC-RET protection for Linux/AArch64

[Andrew Haley]

On Wed, 10 Nov 2021 12:32:53 GMT, Alan Hayward  wrote:

> PAC is an optional feature in AArch64 8.3 and is compulsory in v9. One
> of its uses is to protect against ROP based attacks. This is done by
> signing the Link Register whenever it is stored on the stack, and
> authenticating the value when it is loaded back from the stack. If an
> attacker were to try to change control flow by editing the stack then
> the authentication check of the Link Register will fail, causing a
> segfault when the function returns.
> 
> On a system with PAC enabled, it is expected that all applications will
> be compiled with ROP protection. Fedora 33 and upwards already provide
> this. By compiling for ARMv8.0, GCC and LLVM will only use the set of
> PAC instructions that exist in the NOP space - on hardware without PAC,
> these instructions act as NOPs, allowing backward compatibility for
> negligible performance cost (2 NOPs per non-leaf function).
> 
> Hardware is currently limited to the Apple M1 MacBooks. All testing has
> been done within a Fedora Docker image. A run of SpecJVM showed no
> difference to that of noise - which was surprising.
> 
> The most important part of this patch is simply compiling using branch
> protection provided by GCC/LLVM. This protects all C++ code from being
> used in ROP attacks, removing all static ROP gadgets from use.
> 
> The remainder of the patch adds ROP protection to runtime generated
> code, in both stubs and compiled Java code. Attacks here are much harder
> as ROP gadgets must be found dynamically at runtime. If/when AOT
> compilation is added to JDK, then all stubs and compiled Java will be
> susceptible ROP gadgets being found by static analysis and therefore
> potentially as vulnerable as C++ code.
> 
> There are a number of places where the VM changes control flow by
> rewriting the stack or otherwise. I’ve done some analysis as to how
> these could also be used for attacks (which I didn’t want to post here).
> These areas can be protected ensuring the pointers to various stubs and
> entry points are stored in memory as signed pointers. These changes are
> simple to make (they can be reduced to a type change in common code and
> a few addition sign/auth calls in the backend), but there a lot of them
> and the total code change is fairly large. I’m happy to provide a few
> work in progress patches.
> 
> In order to match the security benefits of the Apple Arm64e ABI across
> the whole of JDK, then all the changes mentioned above would be
> required.

src/hotspot/cpu/aarch64/macroAssembler_aarch64.cpp line 5185:

> 5183: // ROP Protection
> 5184: 
> 5185: void MacroAssembler::protect_return_address() {

We need proper, full, detailed comments about what these functions do, with 
reference to primary AArch64 documentation.

-

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

Re: RFR: 8264130: PAC-RET protection for Linux/AArch64

[Andrew Haley]

On Wed, 10 Nov 2021 12:32:53 GMT, Alan Hayward  wrote:

> PAC is an optional feature in AArch64 8.3 and is compulsory in v9. One
> of its uses is to protect against ROP based attacks. This is done by
> signing the Link Register whenever it is stored on the stack, and
> authenticating the value when it is loaded back from the stack. If an
> attacker were to try to change control flow by editing the stack then
> the authentication check of the Link Register will fail, causing a
> segfault when the function returns.
> 
> On a system with PAC enabled, it is expected that all applications will
> be compiled with ROP protection. Fedora 33 and upwards already provide
> this. By compiling for ARMv8.0, GCC and LLVM will only use the set of
> PAC instructions that exist in the NOP space - on hardware without PAC,
> these instructions act as NOPs, allowing backward compatibility for
> negligible performance cost (2 NOPs per non-leaf function).
> 
> Hardware is currently limited to the Apple M1 MacBooks. All testing has
> been done within a Fedora Docker image. A run of SpecJVM showed no
> difference to that of noise - which was surprising.
> 
> The most important part of this patch is simply compiling using branch
> protection provided by GCC/LLVM. This protects all C++ code from being
> used in ROP attacks, removing all static ROP gadgets from use.
> 
> The remainder of the patch adds ROP protection to runtime generated
> code, in both stubs and compiled Java code. Attacks here are much harder
> as ROP gadgets must be found dynamically at runtime. If/when AOT
> compilation is added to JDK, then all stubs and compiled Java will be
> susceptible ROP gadgets being found by static analysis and therefore
> potentially as vulnerable as C++ code.
> 
> There are a number of places where the VM changes control flow by
> rewriting the stack or otherwise. I’ve done some analysis as to how
> these could also be used for attacks (which I didn’t want to post here).
> These areas can be protected ensuring the pointers to various stubs and
> entry points are stored in memory as signed pointers. These changes are
> simple to make (they can be reduced to a type change in common code and
> a few addition sign/auth calls in the backend), but there a lot of them
> and the total code change is fairly large. I’m happy to provide a few
> work in progress patches.
> 
> In order to match the security benefits of the Apple Arm64e ABI across
> the whole of JDK, then all the changes mentioned above would be
> required.

Gosh. This is going to take some time to review, and will need at least two 
reviewers.

-

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

Re: RFR: 8276550: Use SHA256 hash in build.tools.depend.Depend

[Andrew Haley]

On Wed, 3 Nov 2021 11:54:39 GMT, Aleksey Shipilev  wrote:

> [JDK-8182285](https://bugs.openjdk.java.net/browse/JDK-8182285) added the 
> incremental build capabilities for modules, by hashing the APIs of each 
> module.
> 
> The original change uses MD5, which is quite weak, and 
> [JDK-8214483](https://bugs.openjdk.java.net/browse/JDK-8214483) allows 
> `MessageDigest` to have no MD5 implementation. This is the cause of some 
> build failures when using a FIPS-compliant boot JDK that has no MD5 
> implementation. I suggest we switch to the latest available hash instead.
> 
> Additional testing:
>  - [x] Linux x86_64 fastdebug build completes
>  - [x] Linux x86_64 fastdebug build times do not regress

Marked as reviewed by aph (Reviewer).

-

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

Re: RFR: 8276057: Update JMH devkit to 1.33

[Andrew Haley]

On Wed, 27 Oct 2021 12:28:16 GMT, Aleksey Shipilev  wrote:

> Time to update the devkit to the latest JMH. 
> 
> Additional testing:
>  - [x] Devkit generation works
>  - [x] Sample benchmarks run

Marked as reviewed by aph (Reviewer).

-

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

Re: RFR: 8253757: Add LLVM-based backend for hsdis

[Andrew Haley]

On Mon, 18 Oct 2021 09:10:32 GMT, Andrew Haley  wrote:

> >Can you describe a reproducer?
> Sure. Create a .hotspot_compiler file containing print 
> java.lang.String::checkIndex then 

Sorry, thinko. You don't need the .hotspot_compiler file

-

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

Re: RFR: 8253757: Add LLVM-based backend for hsdis

[Andrew Haley]

On Wed, 13 Oct 2021 00:00:22 GMT, Magnus Ihse Bursie  wrote:

> This patch expands the newly added system for hsdis backends to include LLVM.
> 
> The actual code in hsdis-llvm.cpp is based heavily on the work by @luhenry, 
> as published in the never integrated PR 
> https://github.com/openjdk/jdk/pull/392. (I have basically just ripped out 
> the binutils-based part of it.)
> 
> Unfortunately I have not been able to make this work properly on Windows. 
> With some additional flags I made it compile without complaints, but it 
> caused hotspot to segfault in `LoadLibrary` (!) in `os::dll_load` when I 
> tried to load the library. This is somewhat ironic, since the initial 
> implementation was created by Ludovic for the very purpose of using it on 
> Windows.
> 
> The lack of Windows support in this patch does not mean it is impossible to 
> get it to work, just that I need to co-operate with someone who has more 
> experience of compiling LLVM on Windows, and/or are more eager to get this 
> combination to work.

On 10/16/21 09:13, Magnus Ihse Bursie wrote:
> @theRealAph We should not push broken code, and we should not break the 
> existing build of hsdis. I fully agree with this. I will not push this patch 
> until all reviewers are happy, so you don't need to worry about that. :)
> 
> My initial plan was to get the unix platforms working in this push, and 
> tackle Windows later on, but it seems now that it's better to keep this PR 
> around for a bit longer instead, and fold Windows support into it as well. 
> (Which means I'll wait for Monica to return and being able to test and help 
> out.)
> 
> I need to understand better why things are failing for you. Can you describe 
> a reproducer?

Sure. Create a .hotspot_compiler file containing

   print java.lang.String::checkIndex

then

   ./build/macosx-aarch64-server-fastdebug/jdk/bin/java -XX:+PrintAssembly 
-version

GNU disassembly of java.lang.String::isLatin1 prints in full, ending thusly

   [Exception Handler]
   0x00010a72c340:   mov x19, #0xdead// #57005
 ;   {no_reloc}
   0x00010a72c344:   mov x2, #0xa// #10
   0x00010a72c348:   mov x4, #0xdead // #57005
   0x00010a72c34c:   mov x5, #0xdead // #57005
   0x00010a72c350:   adrpx8, 0x00010a1c3000  ;   {runtime_call 
handle_exception_from_callee Runtime1 stub}
   0x00010a72c354:   add x8, x8, #0x1c0
   0x00010a72c358:   blr x8
   0x00010a72c35c:   dcps1   #0xdeae
   0x00010a72c360:   .inst   0x0995f68e ; undefined
   0x00010a72c364:   udf #1
[Deopt Handler Code]
   0x00010a72c368:   adr x30, 0x00010a72c368
   0x00010a72c36c:   adrpx8, 0x00010a26c000  ;   {runtime_call 
DeoptimizationBlob}
   0x00010a72c370:   add x8, x8, #0xbc0
   0x00010a72c374:   br  x8


LLVM disassembly dies at

[Exception Handler]
   0x00010672c340:   mov x19, #0xdead;   {no_reloc}
   0x00010672c344:   mov x2, #0xa
   0x00010672c348:   mov x4, #0xdead
   0x00010672c34c:   mov x5, #0xdead
   0x00010672c350:   adrpx8, #-5672960   ;   {runtime_call 
handle_exception_from_callee Runtime1 stub}
   0x00010672c354:   add x8, x8, #0x1c0
   0x00010672c358:   blr x8
   0x00010672c35c:   dcps1   #0xdeae
   0x00010672c360:   


This is llvm 12.0.1 from homebrew.

-

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

Re: RFR: 8253757: Add LLVM-based backend for hsdis

[Andrew Haley]

On Fri, 15 Oct 2021 08:20:05 GMT, Ludovic Henry  wrote:

> Since the maintenance is fairly low (small codebase, small and knowledgable 
> user base), I would be biased towards including it with appropriate warnings.

I don't think we should commit code that we know is broken. I don't believe 
that this view might be controversial.
Maybe someone should try to reproduce the failure I've seen, and then we should 
look at fixing it. Maybe it's a local problem.

Also, this patch breaks all current hsdis builds that follow the installation 
instructions. Either we get revised instructions or the build should be fixed.

-

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

Re: RFR: 8253757: Add LLVM-based backend for hsdis

[Andrew Haley]

On Wed, 13 Oct 2021 07:26:21 GMT, Ludovic Henry  wrote:

>> This patch expands the newly added system for hsdis backends to include LLVM.
>> 
>> The actual code in hsdis-llvm.cpp is based heavily on the work by @luhenry, 
>> as published in the never integrated PR 
>> https://github.com/openjdk/jdk/pull/392. (I have basically just ripped out 
>> the binutils-based part of it.)
>> 
>> Unfortunately I have not been able to make this work properly on Windows. 
>> With some additional flags I made it compile without complaints, but it 
>> caused hotspot to segfault in `LoadLibrary` (!) in `os::dll_load` when I 
>> tried to load the library. This is somewhat ironic, since the initial 
>> implementation was created by Ludovic for the very purpose of using it on 
>> Windows.
>> 
>> The lack of Windows support in this patch does not mean it is impossible to 
>> get it to work, just that I need to co-operate with someone who has more 
>> experience of compiling LLVM on Windows, and/or are more eager to get this 
>> combination to work.
>
> Very happy to see it landing :) Thank you!
> 
> I don't have access to a windows machine, and even less a Windows-AArch64 
> machine. @lewurm would you be able to take a look?

> As for LLVM not giving you a good user experience; I can't really tell what's 
> wrong. Apparently @luhenry (and @JornVernee I believe) has used this. I'm not 
> really the target audience myself; I'm only trying to make it possible to 
> use. If it is so severly limited as you say maybe this isn't worth pursuing. 
> Some feedback from those who have tested it would be appeciated here, to help 
> med understand if this patch should be dropped.

I don't think it should be dropped, but I imagine that the bugs can be fixed. 
If LLVM's disassembler always dies as soon as it sees something it can't 
recognize, I'm astonished. Maybe the LLVM I'm using is bad.

-

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

Re: RFR: 8253757: Add LLVM-based backend for hsdis

[Andrew Haley]

On Wed, 13 Oct 2021 00:00:22 GMT, Magnus Ihse Bursie  wrote:

> This patch expands the newly added system for hsdis backends to include LLVM.
> 
> The actual code in hsdis-llvm.cpp is based heavily on the work by @luhenry, 
> as published in the never integrated PR 
> https://github.com/openjdk/jdk/pull/392. (I have basically just ripped out 
> the binutils-based part of it.)
> 
> Unfortunately I have not been able to make this work properly on Windows. 
> With some additional flags I made it compile without complaints, but it 
> caused hotspot to segfault in `LoadLibrary` (!) in `os::dll_load` when I 
> tried to load the library. This is somewhat ironic, since the initial 
> implementation was created by Ludovic for the very purpose of using it on 
> Windows.
> 
> The lack of Windows support in this patch does not mean it is impossible to 
> get it to work, just that I need to co-operate with someone who has more 
> experience of compiling LLVM on Windows, and/or are more eager to get this 
> combination to work.

So I figured out how to build it with `make build-hsdis`. The instructions need 
to be fixed.

Two problems: firstly, the library seems to be built with the wrong name, so 
the runtime doesn't find it. I had to use
`ln -sf ~/lib/libhsdis.dylib ~/lib/hsdis-aarch64.dylib`
to get it to work.

More severely, the disassembly is truncated, so lots of stuff doesn't work. The 
binutils-based hsdis prints


 0x0001105665b4:   stp wzr, wzr, [x20, #60];*putfield 
preCounterBlock {reexecute=0 rethrow=0 return_oop=0}
; - 
com.sun.crypto.provider.GaloisCounterMode$GCMEngine::@80 (line 673)
; - 
com.sun.crypto.provider.GaloisCounterMode$GCMDecrypt::@8 (line 1346)
; - 
com.sun.crypto.provider.GaloisCounterMode::checkInit@60 (line 329)
 ;; B6: #   out( B242 B7 ) <- in( B192 B5 )  Freq: 0.99
  0x0001105665b8:   cbnzx21, 0x0001105665c8
 ;; null oop passed to encode_heap_oop_not_null2
  0x0001105665bc:   dcps1   #0xdeae
  0x0001105665c0:   .inst   0x082adca6 ; undefined
  0x0001105665c4:   udf #1
  0x0001105665c8:   lsr x10, x21, #3;*invokevirtual 
getLongUnaligned {reexecute=0 rethrow=0 return_oop=0}
; - 
java.lang.invoke.VarHandleByteArrayAsLongs$ArrayHandle::get@32 (line 115)
   ... lots more


but the llvm-based one stops here:


  0x00010ed79034:   stp wzr, wzr, [x20, #0x3c];*putfield 
preCounterBlock {reexecute=0 rethrow=0 return_oop=0}
; - 
com.sun.crypto.provider.GaloisCounterMode$GCMEngine::@80 (line 673)
; - 
com.sun.crypto.provider.GaloisCounterMode$GCMDecrypt::@8 (line 1346)
; - 
com.sun.crypto.provider.GaloisCounterMode::checkInit@60 (line 329)
 ;; B6: #   out( B242 B7 ) <- in( B192 B5 )  Freq: 0.99
  0x00010ed79038:   cbnzx21, #0x10
 ;; null oop passed to encode_heap_oop_not_null2
  0x00010ed7903c:   dcps1   #0xdeae
  0x00010ed79040:   

[/Disassembly]



I think it's giving up as soon as it sees something it doesn't recognize, so 
it's pretty much useless.

In addition, even when it does work the LLVM disassembly is pretty poor. For 
example, the unverified entry point looks like


  0x00010ed78f80:   ldr w8, [x1, #0x8]
  0x00010ed78f84:   cmp w9, w8
  0x00010ed78f88:   b.eq#0x10
  0x00010ed78f8c:   adrpx8, #-128524288 ;   {runtime_call 
ic_miss_stub}
  0x00010ed78f90:   add x8, x8, #0x1c0
  0x00010ed78f94:   br  x8


instead of


  0x000110566500:   ldr w8, [x1, #8]
  0x000110566504:   cmp w9, w8
  0x000110566508:   b.eq0x000110566518  // b.none
  0x00011056650c:   adrpx8, 0x000108ae6000  ;   {runtime_call 
ic_miss_stub}
  0x000110566510:   add x8, x8, #0x1c0
  0x000110566514:   br  x8


Sure, it's good to have a choice, but the LLVM-based hsdis doesn't look to me 
like a serious alternative for professional work.

-

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

Re: RFR: 8253757: Add LLVM-based backend for hsdis

[Andrew Haley]

On Wed, 13 Oct 2021 13:55:04 GMT, Andrew Haley  wrote:

>> This patch expands the newly added system for hsdis backends to include LLVM.
>> 
>> The actual code in hsdis-llvm.cpp is based heavily on the work by @luhenry, 
>> as published in the never integrated PR 
>> https://github.com/openjdk/jdk/pull/392. (I have basically just ripped out 
>> the binutils-based part of it.)
>> 
>> Unfortunately I have not been able to make this work properly on Windows. 
>> With some additional flags I made it compile without complaints, but it 
>> caused hotspot to segfault in `LoadLibrary` (!) in `os::dll_load` when I 
>> tried to load the library. This is somewhat ironic, since the initial 
>> implementation was created by Ludovic for the very purpose of using it on 
>> Windows.
>> 
>> The lack of Windows support in this patch does not mean it is impossible to 
>> get it to work, just that I need to co-operate with someone who has more 
>> experience of compiling LLVM on Windows, and/or are more eager to get this 
>> combination to work.
>
>> This patch expands the newly added system for hsdis backends to include LLVM.
>> 
>> The actual code in hsdis-llvm.cpp is based heavily on the work by @luhenry, 
>> as published in the never integrated PR #392. (I have basically just ripped 
>> out the binutils-based part of it.)
>> 
>> Unfortunately I have not been able to make this work properly on Windows. 
> 
> What is it for, then?
> 
> hsdis builds on AArch64-MacOS-LLVM with
> 
> 
>   cd src/utils/hsdis
>   mkdir build
>   cd build
>   git clone https://github.com/bminor/binutils-gdb
>   ln -s binutils-gdb binutils
>   cd ..
>   make

> @theRealAph As you might be aware, the licensing criteria for binutils makes 
> it impossible to distribute a binutils-based hsdis with the JDK. While IANAL, 
> my understanding is that the LLVM license is less problematic in that way.
> 
> Also, this is to allow a bit of freedom of choice. If you prefer the LLVM 
> backend (I've been told that it generates better disassembly in some case) 
> you should be able to select it.
> 
> And finally, I do think that the LLVM backend should be able to work on 
> Windows, too. It's just that this is tricky enough to motivate doing this in 
> a separate, later, step.

OK, but how do you build it? I have applied this patch, and the instructions in 
`hsdis/README` don't mention LLVM, just binutils. Shouldn't the instructions 
have been updated?

-

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

Re: RFR: 8253757: Add LLVM-based backend for hsdis

[Andrew Haley]

On Wed, 13 Oct 2021 00:00:22 GMT, Magnus Ihse Bursie  wrote:

> This patch expands the newly added system for hsdis backends to include LLVM.
> 
> The actual code in hsdis-llvm.cpp is based heavily on the work by @luhenry, 
> as published in the never integrated PR 
> https://github.com/openjdk/jdk/pull/392. (I have basically just ripped out 
> the binutils-based part of it.)
> 
> Unfortunately I have not been able to make this work properly on Windows. 
> With some additional flags I made it compile without complaints, but it 
> caused hotspot to segfault in `LoadLibrary` (!) in `os::dll_load` when I 
> tried to load the library. This is somewhat ironic, since the initial 
> implementation was created by Ludovic for the very purpose of using it on 
> Windows.
> 
> The lack of Windows support in this patch does not mean it is impossible to 
> get it to work, just that I need to co-operate with someone who has more 
> experience of compiling LLVM on Windows, and/or are more eager to get this 
> combination to work.

> This patch expands the newly added system for hsdis backends to include LLVM.
> 
> The actual code in hsdis-llvm.cpp is based heavily on the work by @luhenry, 
> as published in the never integrated PR #392. (I have basically just ripped 
> out the binutils-based part of it.)
> 
> Unfortunately I have not been able to make this work properly on Windows. 

What is it for, then?

hsdis builds on AArch64-MacOS-LLVM with


  cd src/utils/hsdis
  mkdir build
  cd build
  git clone https://github.com/bminor/binutils-gdb
  ln -s binutils-gdb binutils
  cd ..
  make

-

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

Re: RFR: 8264805: Remove the experimental Ahead-of-Time Compiler [v4]

[Andrew Haley]

On Thu, 8 Apr 2021 17:24:38 GMT, Vladimir Kozlov  wrote:

>> As part of [JEP 410](http://openjdk.java.net/jeps/410) remove code related 
>> to Ahead-of-Time Compiler from JDK: 
>> 
>> - `jdk.aot` module — the `jaotc` tool 
>> - `src/hotspot/share/aot` — loads AoT compiled code into VM for execution 
>> - related HotSpot code guarded by `#if INCLUDE_AOT` 
>> 
>> Additionally, remove tests as well as code in makefiles related to AoT 
>> compilation.
>> 
>> Tested hs-tier1-4
>
> Vladimir Kozlov has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   Remove exports from Graal module to jdk.aot

AArch64 looks fine.

-

Marked as reviewed by aph (Reviewer).

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

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

[Andrew Haley]

On Tue, 23 Mar 2021 16:20:47 GMT, Ludovic Henry  wrote:

>> Anton Kozlov has updated the pull request with a new target base due to a 
>> merge or a rebase. The pull request now contains 115 commits:
>> 
>>  - Merge branch 'master' into jdk-macos
>>  - JDK-8262491: bsd_aarch64 part
>>  - JDK-8263002: bsd_aarch64 part
>>  - Merge remote-tracking branch 'upstream/jdk/master' into jdk-macos
>>  - Wider #ifdef block
>>  - Fix most of issues in java/foreign/ tests
>>
>>Failures related to va_args are tracked in JDK-8263512.
>>  - Add Azul copyright
>>  - Update Oracle copyright years
>>  - Use Thread::current_or_null_safe in SafeFetch
>>  - 8262903: [macos_aarch64] Thread::current() called on detached thread
>>  - ... and 105 more: 
>> https://git.openjdk.java.net/jdk/compare/a9d2267f...5add9269
>
> Marked as reviewed by luhenry (Author).

> > > > [ Back-porting this patch to JDK 11] depends on the will of openjdk11 
> > > > maintainers to accept this (and few other, like jep-388, as we depend 
> > > > on it) contribution.
> > > 
> > > 
> > > To the extent that 11u has fixed policies :) we definitely have a policy 
> > > of accepting patches to keep 11u working on current hardware. So yes.
> > 
> > 
> > @lewurm That sounds like a green flag for you and jep-388 (with its 
> > R18_RESERVED functionality) ;)
> 
> Thanks, @theRealAph, and @VladimirKempik . We are on it!

It's going to be tricky to do in a really clean way, given some of the 
weirdnesses of the ABI. However, I think there's probably a need for it

-

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

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

[Andrew Haley]

On Tue, 23 Mar 2021 13:54:24 GMT, Andrew Haley  wrote:

>> Anton Kozlov has updated the pull request with a new target base due to a 
>> merge or a rebase. The pull request now contains 115 commits:
>> 
>>  - Merge branch 'master' into jdk-macos
>>  - JDK-8262491: bsd_aarch64 part
>>  - JDK-8263002: bsd_aarch64 part
>>  - Merge remote-tracking branch 'upstream/jdk/master' into jdk-macos
>>  - Wider #ifdef block
>>  - Fix most of issues in java/foreign/ tests
>>
>>Failures related to va_args are tracked in JDK-8263512.
>>  - Add Azul copyright
>>  - Update Oracle copyright years
>>  - Use Thread::current_or_null_safe in SafeFetch
>>  - 8262903: [macos_aarch64] Thread::current() called on detached thread
>>  - ... and 105 more: 
>> https://git.openjdk.java.net/jdk/compare/a9d2267f...5add9269
>
> Marked as reviewed by aph (Reviewer).

> [ Back-porting this patch to JDK 11] depends on the will of openjdk11 
> maintainers to accept this (and few other, like jep-388, as we depend on it) 
> contribution.

To the extent that 11u has fixed policies :)  we definitely have a policy of 
accepting patches to keep 11u working on current hardware. So yes.

-

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

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

[Andrew Haley]

On Mon, 22 Mar 2021 12:50:14 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 with a new target base due to a 
> merge or a rebase. The pull request now contains 115 commits:
> 
>  - Merge branch 'master' into jdk-macos
>  - JDK-8262491: bsd_aarch64 part
>  - JDK-8263002: bsd_aarch64 part
>  - Merge remote-tracking branch 'upstream/jdk/master' into jdk-macos
>  - Wider #ifdef block
>  - Fix most of issues in java/foreign/ tests
>
>Failures related to va_args are tracked in JDK-8263512.
>  - Add Azul copyright
>  - Update Oracle copyright years
>  - Use Thread::current_or_null_safe in SafeFetch
>  - 8262903: [macos_aarch64] Thread::current() called on detached thread
>  - ... and 105 more: 
> https://git.openjdk.java.net/jdk/compare/a9d2267f...5add9269

Marked as reviewed by aph (Reviewer).

-

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

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

[Andrew Haley]

On Fri, 12 Mar 2021 16:32:10 GMT, Andrew Haley  wrote:

>> Anton Kozlov has updated the pull request with a new target base due to a 
>> merge or a rebase. The pull request now contains 105 commits:
>> 
>>  - Merge commit 'refs/pull/11/head' of https://github.com/AntonKozlov/jdk 
>> into jdk-macos
>>  - workaround JDK-8262895 by disabling subtest
>>  - Fix typo
>>  - Rename threadWXSetters.hpp -> threadWXSetters.inline.hpp
>>  - JDK-8259937: bsd_aarch64 part
>>  - Merge remote-tracking branch 'upstream/jdk/master' into jdk-macos
>>  - Fix after JDK-8259539, partially revert preconditions
>>  - JDK-8260471: bsd_aarch64 part
>>  - JDK-8259539: bsd_aarch64 part
>>  - JDK-8257828: bsd_aarch64 part
>>  - ... and 95 more: 
>> https://git.openjdk.java.net/jdk/compare/a6e34b3d...a72f6834
>
>> @theRealAph, could you elaborate on what is need to be done for [#2200 
>> (review)](https://github.com/openjdk/jdk/pull/2200#pullrequestreview-600597066).
> 
> I think that what you've got now is fine.

> _Mailing list message from [Andrew Haley](mailto:a...@redhat.com) on 
> [build-dev](mailto:build-dev@openjdk.java.net):_
> 
> On 3/15/21 6:56 PM, Anton Kozlov wrote:
> 
> > On Wed, 10 Mar 2021 11:21:44 GMT, Andrew Haley  wrote:
> > > > We always check for `R18_RESERVED` with `#if(n)def`, is there any 
> > > > reason to define the value for the macro?
> > > 
> > > 
> > > Robustness, clarity, maintainability, convention. Why not?
> > 
> > 
> > I've tried to implement the suggestion, but it pulled more unnecessary 
> > changes. It makes the intended way to check the condition less clear 
> > (`#ifdef` and not `#if`).
> 
> No, no, no! I am not suggesting you change anything else, just that
> you do not define contentless macros. You might as well define it
> to be something, and true is a reasonable default, that's all. It's
> not terribly important, it's just good practice.

I'm quite prepared to drop this if it's holding up the port. It's a style 
thing, but it's not critical.

-

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

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

[Andrew Haley]

On Tue, 23 Mar 2021 09:53:54 GMT, Andrew Haley  wrote:

>>> @theRealAph, could you elaborate on what is need to be done for [#2200 
>>> (review)](https://github.com/openjdk/jdk/pull/2200#pullrequestreview-600597066).
>> 
>> I think that what you've got now is fine.
>
>> _Mailing list message from [Andrew Haley](mailto:a...@redhat.com) on 
>> [build-dev](mailto:build-dev@openjdk.java.net):_
>> 
>> On 3/15/21 6:56 PM, Anton Kozlov wrote:
>> 
>> > On Wed, 10 Mar 2021 11:21:44 GMT, Andrew Haley  wrote:
>> > > > We always check for `R18_RESERVED` with `#if(n)def`, is there any 
>> > > > reason to define the value for the macro?
>> > > 
>> > > 
>> > > Robustness, clarity, maintainability, convention. Why not?
>> > 
>> > 
>> > I've tried to implement the suggestion, but it pulled more unnecessary 
>> > changes. It makes the intended way to check the condition less clear 
>> > (`#ifdef` and not `#if`).
>> 
>> No, no, no! I am not suggesting you change anything else, just that
>> you do not define contentless macros. You might as well define it
>> to be something, and true is a reasonable default, that's all. It's
>> not terribly important, it's just good practice.
> 
> I'm quite prepared to drop this if it's holding up the port. It's a style 
> thing, but it's not critical.

So, where are we up to now? Are we done yet?

-

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

Re: CMake replacing Autotools?

[Andrew Haley]

On 3/19/21 10:14 AM, Thomas Stüfe wrote:
> On Fri, Mar 19, 2021 at 11:04 AM Andrew Haley  wrote:
> 
>> On 3/19/21 9:22 AM, Thomas Stüfe wrote:
>>>> 2. More choices to actually build the project: Use integrated build
>>>> tools of IDEs (Visual Studio, Xcode) or use Ninja, which is faster than
>>>> gmake
>>>>
>>> Is gmake really where we lose time? Did you analyze this or is this just
>> an
>>> assumption? I would have thought it's things like single threaded jmod,
>>> jlink, and subprocess spawning.
>>
>> I'm sure it is. The other slow thing is linking HotSpot.
>>
> 
> What is so slow with gmake? Rule processing?

Nothing, I suspect.

> It also depends on the platform, I guess. Eg on Cygwin, the fork emulation
> is extremely slow.

Weren't many Cygwin tools changed to use spawn() instead? It's been
a while.

-- 
Andrew Haley  (he/him)
Java Platform Lead Engineer
Red Hat UK Ltd. <https://www.redhat.com>
https://keybase.io/andrewhaley
EAC8 43EB D3EF DB98 CC77 2FAD A5CD 6035 332F A671

Re: CMake replacing Autotools?

[Andrew Haley]

On 3/19/21 9:22 AM, Thomas Stüfe wrote:
>> 2. More choices to actually build the project: Use integrated build
>> tools of IDEs (Visual Studio, Xcode) or use Ninja, which is faster than
>> gmake
>>
> Is gmake really where we lose time? Did you analyze this or is this just an
> assumption? I would have thought it's things like single threaded jmod,
> jlink, and subprocess spawning.

I'm sure it is. The other slow thing is linking HotSpot.

-- 
Andrew Haley  (he/him)
Java Platform Lead Engineer
Red Hat UK Ltd. <https://www.redhat.com>
https://keybase.io/andrewhaley
EAC8 43EB D3EF DB98 CC77 2FAD A5CD 6035 332F A671

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

[Andrew Haley]

On 3/15/21 6:56 PM, Anton Kozlov wrote:
> On Wed, 10 Mar 2021 11:21:44 GMT, Andrew Haley  wrote:
> 
>>> We always check for `R18_RESERVED` with `#if(n)def`, is there any reason to 
>>> define the value for the macro?
>>
>> Robustness, clarity, maintainability, convention. Why not?
> 
> I've tried to implement the suggestion, but it pulled more unnecessary 
> changes. It makes the intended way to check the condition less clear 
> (`#ifdef` and not `#if`).

No, no, no! I am not suggesting you change anything else, just that
you do not define contentless macros. You might as well define it
to be something, and true is a reasonable default, that's all. It's
not terribly important, it's just good practice.

-- 
Andrew Haley  (he/him)
Java Platform Lead Engineer
Red Hat UK Ltd. <https://www.redhat.com>
https://keybase.io/andrewhaley
EAC8 43EB D3EF DB98 CC77 2FAD A5CD 6035 332F A671

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

[Andrew Haley]

On Tue, 9 Mar 2021 18:01:11 GMT, Anton Kozlov  wrote:

>> src/hotspot/cpu/aarch64/globalDefinitions_aarch64.hpp line 62:
>> 
>>> 60: 
>>> 61: #if defined(__APPLE__) || defined(_WIN64)
>>> 62: #define R18_RESERVED
>> 
>> #define R18_RESERVED true```
>
> We always check for `R18_RESERVED` with `#if(n)def`, is there any reason to 
> define the value for the macro?

Robustness, clarity, maintainability, convention. Why not?

-

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

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

[Andrew Haley]

On Tue, 9 Mar 2021 16:12:36 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 with a new target base due to a 
> merge or a rebase. The pull request now contains 105 commits:
> 
>  - Merge commit 'refs/pull/11/head' of https://github.com/AntonKozlov/jdk 
> into jdk-macos
>  - workaround JDK-8262895 by disabling subtest
>  - Fix typo
>  - Rename threadWXSetters.hpp -> threadWXSetters.inline.hpp
>  - JDK-8259937: bsd_aarch64 part
>  - Merge remote-tracking branch 'upstream/jdk/master' into jdk-macos
>  - Fix after JDK-8259539, partially revert preconditions
>  - JDK-8260471: bsd_aarch64 part
>  - JDK-8259539: bsd_aarch64 part
>  - JDK-8257828: bsd_aarch64 part
>  - ... and 95 more: 
> https://git.openjdk.java.net/jdk/compare/a6e34b3d...a72f6834

> @theRealAph, could you elaborate on what is need to be done for [#2200 
> (review)](https://github.com/openjdk/jdk/pull/2200#pullrequestreview-600597066).

I think that what you've got now is fine.

-

Marked as reviewed by aph (Reviewer).

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

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

[Andrew Haley]

On Wed, 3 Mar 2021 17:39:28 GMT, Gerard Ziemski  wrote:

> A list of the bugs that our internal testing revealed so far:

Are any of these blockers for integration? Some of them are to do with things 
like features that aren't yet supported, and we can't fix what we can't see.

-

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

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

[Andrew Haley]

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 [v9]

[Andrew Haley]

On Fri, 5 Feb 2021 17:20:55 GMT, Anton Kozlov  wrote:

>> src/hotspot/cpu/aarch64/vm_version_aarch64.hpp line 93:
>> 
>>> 91: CPU_MARVELL   = 'V',
>>> 92: CPU_INTEL = 'i',
>>> 93: CPU_APPLE = 'a',
>> 
>> The `ARM Architecture Reference Manual ARMv8, for ARMv8-A architecture 
>> profile` has 8538 pages, can we be more specific and point to the particular 
>> section of the document where the CPU codes are defined?
>
> They are defined in 13.2.95. MIDR_EL1, Main ID Register. Apple's code is not 
> there, but "Arm can assign codes that are not published in this manual. All 
> values not assigned by Arm are reserved and must not be used.". I assume the 
> value was obtained by digging around 
> https://github.com/apple/darwin-xnu/blob/main/osfmk/arm/cpuid.h#L62

Anton, this paragraph looks like an excellent comment.

-

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

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

[Andrew Haley]

On Fri, 26 Feb 2021 19:17:12 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 two additional 
> commits since the last revision:
> 
>  - Merge remote-tracking branch 'origin/jdk/jdk-macos' into jdk-macos
>  - Minor fixes

Thanks. With this, I think we're done.

-

Changes requested by aph (Reviewer).

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

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

[Andrew Haley]

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 [v21]

[Andrew Haley]

On Fri, 26 Feb 2021 19:17:12 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 two additional 
> commits since the last revision:
> 
>  - Merge remote-tracking branch 'origin/jdk/jdk-macos' into jdk-macos
>  - Minor fixes

src/hotspot/cpu/aarch64/globalDefinitions_aarch64.hpp line 62:

> 60: 
> 61: #if defined(__APPLE__) || defined(_WIN64)
> 62: #define R18_RESERVED

#define R18_RESERVED true```

-

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

Re: RFR: 8236847: CDS archive with 4K alignment unusable on machines with 64k pages [v2]

[Andrew Haley]

On Sun, 28 Feb 2021 14:08:37 GMT, Andrew Haley  wrote:

>> Before this fix, the alignment is defaulting to that of the build host. We 
>> would like to provide a way to produce a JDK distribution, with a pre 
>> generated CDS archive, where the alignment has the highest known value of 
>> any target host for maximum compatibility. The currently known such values 
>> are 64K for Linux and 16K for Mac. If we aren't going to allow the user 
>> (builder of OpenJDK) the free choice of any alignment anyway, would it make 
>> sense to limit the choice between something more abstract like "host" and 
>> "compatible" instead of listing explicit numbers?
>> 
>> Regardless of how the option is constructed, it will need some explanation 
>> in doc/building.md.
>> 
>> Finally there is the question of if "host" or "compatible" should be the 
>> default. I see good arguments for both sides, as long as there is an option 
>> to switch between the too that isn't too cryptic to understand.
>
>> Before this fix, the alignment is defaulting to that of the build host. We 
>> would like to provide a way to produce a JDK distribution, with a pre 
>> generated CDS archive, where the alignment has the highest known value of 
>> any target host for maximum compatibility. The currently known such values 
>> are 64K for Linux and 16K for Mac. If we aren't going to allow the user 
>> (builder of OpenJDK) the free choice of any alignment anyway, would it make 
>> sense to limit the choice between something more abstract like "host" and 
>> "compatible" instead of listing explicit numbers?
> 
>  That's problematic because it assumes we know all of the possible 
> alignments.  At the present time we think that 64 is the largest we'll ever 
> encounter, but IMO that isn't a great way to think about things. It would be 
> very nice indeed if we didn't have to edit OpenJDK for the next page size. I 
> guess 4k, 16k, and 64k are all we'll ever see, but I wouldn't bet the farm on 
> it.
> 
>> Finally there is the question of if "host" or "compatible" should be the 
>> default. I see good arguments for both sides, as long as there is an option 
>> to switch between the too that isn't too cryptic to understand.
> 
> I would have thought that "host" made the most sense, but I don't really mind.

> _Mailing list message from [Andrew Haley](mailto:a...@redhat.com) on 
> [hotspot-runtime-dev](mailto:hotspot-runtime-...@openjdk.java.net):_
> 
> Hmm. I'm not convinced by making the numeric argument here a free field.
> I'd allow precisely two options, "4k" and "64k", and if any crazy person
> needs to add "1M" in the future, let them do so.

I now realize this was wrong, because at least 16k exists as well.

-

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

Re: RFR: 8236847: CDS archive with 4K alignment unusable on machines with 64k pages [v2]

[Andrew Haley]

On Fri, 26 Feb 2021 18:28:04 GMT, Erik Joelsson  wrote:

> Before this fix, the alignment is defaulting to that of the build host. We 
> would like to provide a way to produce a JDK distribution, with a pre 
> generated CDS archive, where the alignment has the highest known value of any 
> target host for maximum compatibility. The currently known such values are 
> 64K for Linux and 16K for Mac. If we aren't going to allow the user (builder 
> of OpenJDK) the free choice of any alignment anyway, would it make sense to 
> limit the choice between something more abstract like "host" and "compatible" 
> instead of listing explicit numbers?

 That's problematic because it assumes we know all of the possible alignments.  
At the present time we think that 64 is the largest we'll ever encounter, but 
IMO that isn't a great way to think about things. It would be very nice indeed 
if we didn't have to edit OpenJDK for the next page size. I guess 4k, 16k, and 
64k are all we'll ever see, but I wouldn't bet the farm on it.

> Finally there is the question of if "host" or "compatible" should be the 
> default. I see good arguments for both sides, as long as there is an option 
> to switch between the too that isn't too cryptic to understand.

I would have thought that "host" made the most sense, but I don't really mind.

-

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

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

[Andrew Haley]

On Mon, 15 Feb 2021 17:45:32 GMT, Anton Kozlov  wrote:

>> I'm not sure it can wait. This change turns already-messy code into 
>> something significantly messier, to the extent that it's not really good 
>> enough to go into mainline.
>
> The latest merge with JDK-8261071 should resolve the issue. Please take a 
> look.

Looks much better, thanks.

-

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

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

[Andrew Haley]

On Tue, 16 Feb 2021 06:24:05 GMT, Vladimir Kempik  wrote:

>> This is when passing a float, yes? In the case where we have more float 
>> arguments than n_float_register_parameters_c.
>> I don't understand why you think it's acceptable to bail in this case. Can 
>> you explain, please?
>
> it's for everything that uses less than 8 bytes on a stack( ints ( 4), 
> shorts(2), bytes(1), floats(4)).
> currently native wrapper generation does not support such cases at all, it 
> needs refactoring before this can be implemented.
> So when a method has more argument than can be placed in registers, we may 
> have issues. 
> 
> So we just bailing out to interpreter in case when a smaller (<=4 b) type is 
> going to be passed thru the stack.
> 
> There was attempt to implement handling such cases but currently it requires 
> some hacks (like using some vectors for non-specific task) - 
> https://github.com/openjdk/aarch64-port/pull/3

OK. I checked and the Panama preview doesn't support direct native calls for 
stack arguments, so we're good for now.

-

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

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

[Andrew Haley]

On Mon, 15 Feb 2021 18:00:50 GMT, Vladimir Kempik  wrote:

>> src/hotspot/cpu/aarch64/sharedRuntime_aarch64.cpp line 839:
>> 
>>> 837:   // The code unable to handle this, bailout.
>>> 838:   return -1;
>>> 839: #endif
>> 
>> This looks like a bug to me. The caller doesn't necessarily check the return 
>> value. See CallRuntimeNode::calling_convention.
>
> Hello, we have updated PR, now this bailout is used only by the code which 
> can handle it (native wrapper generator), for the rest it will cause 
> guarantee failed if this bailout is triggered

This is when passing a float, yes? In the case where we have more float 
arguments than n_float_register_parameters_c.
I don't understand why you think it's acceptable to bail in this case. Can you 
explain, please?

-

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

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

[Andrew Haley]

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 [v9]

[Andrew Haley]

On Thu, 4 Feb 2021 09:49:17 GMT, Vladimir Kempik  wrote:

> > You read my mind, Andrew. Unless, of course, it's optimized to leverage the 
> > fact that it's thread-specific..
> 
> it's thread-specific
> 
> https://developer.apple.com/documentation/apple_silicon/porting_just-in-time_compilers_to_apple_silicon
> 
> > Because pthread_jit_write_protect_np changes only the current thread’s 
> > permissions, avoid accessing the same memory region from multiple threads. 
> > Giving multiple threads access to the same memory region opens up a 
> > potential attack vector, in which one thread has write access and another 
> > has executable access to the same region.

Umm, so how does patching work? We don't even know if other threads are 
executing the code we need to patch.

-

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

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

[Andrew Haley]

On Tue, 2 Feb 2021 21:49:36 GMT, Daniel D. Daugherty  wrote:

>> Anton Kozlov has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   support macos_aarch64 in hsdis
>
> src/hotspot/cpu/aarch64/macroAssembler_aarch64.cpp line 323:
> 
>> 321:   str(zr, Address(rthread, JavaThread::last_Java_pc_offset()));
>> 322: 
>> 323:   str(zr, Address(rthread, JavaFrameAnchor::saved_fp_address_offset()));
> 
> I don't think this switch from `JavaThread::saved_fp_address_offset()`
> to `JavaFrameAnchor::saved_fp_address_offset()` is correct since
> `rthread` is still used and is a JavaThread*. The new code will give you:
> 
> `rthread` + offset of the `saved_fp_address` field in a JavaFrameAnchor
> 
> The old code gave you:
> 
> `rthread` + offset of the `saved_fp_address` field in the JavaFrameAnchor 
> field in the JavaThread
> 
> Those are not the same things.

I agree, I don't understand why this change was made.

> src/hotspot/os_cpu/bsd_aarch64/os_bsd_aarch64.hpp line 45:
> 
>> 43:   // Atomically copy 64 bits of data
>> 44:   static void atomic_copy64(const volatile void *src, volatile void 
>> *dst) {
>> 45: *(jlong *) dst = *(const jlong *) src;
> 
> Is this construct actually atomic on aarch64?

Yes.

> src/hotspot/os_cpu/windows_aarch64/os_windows_aarch64.hpp line 37:
> 
>> 35: 
>> 36: private:
>> 37: 
> 
> 'private' usually has one space in front of it.
> Also, why the blank line after it?

It reads better with the blank line, and it's not in violation of HotSpot 
conventions.

-

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

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

[Andrew Haley]

On Tue, 2 Feb 2021 18:03:50 GMT, Gerard Ziemski  wrote:

>> Anton Kozlov has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   support macos_aarch64 in hsdis
>
> src/hotspot/cpu/aarch64/macroAssembler_aarch64.cpp line 5271:
> 
>> 5269: //
>> 5270: void MacroAssembler::get_thread(Register dst) {
>> 5271:   RegSet saved_regs = RegSet::range(r0, r1) + 
>> BSD_ONLY(RegSet::range(r2, r17)) + lr - dst;
> 
> The comment needs to be updated, since on BSD we also seem to clobber r2,r17 ?

Surely this should be 

 saved_regs = RegSet::range(r0, r1) BSD_ONLY(+ RegSet::range(r2, r17)) + lr - 
dst;```

Shouldn't it?

-

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

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

[Andrew Haley]

On Sun, 31 Jan 2021 20:14:01 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 with a new target base due to a 
> merge or a rebase. The pull request now contains 62 commits:
> 
>  - Merge branch 'master' into jdk-macos
>  - Update copyright year for BsdAARCH64ThreadContext.java
>  - Fix inclusing of StubRoutines header
>  - Redo buildsys fix
>  - Revert harfbuzz changes, disable warnings for it
>  - Little adjustement of SlowSignatureHandler
>  - Partially bring previous commit
>  - Revert "Address feedback for signature generators"
>
>This reverts commit 50b55f6684cd21f8b532fa979b7b6fbb4613266d.
>  - Refactor CDS disabling
>  - Redo builsys support for aarch64-darwin
>  - ... and 52 more: 
> https://git.openjdk.java.net/jdk/compare/8a9004da...b421e0b4

src/hotspot/cpu/aarch64/interpreterRT_aarch64.cpp line 84:

> 82: // on stack. Natural alignment for types are still in place,
> 83: // for example double/long should be 8 bytes alligned
> 84: 

This comment is a bit confusing because it's no longer #ifdef APPLE. Better 
move it up to Line 41.

src/hotspot/cpu/aarch64/interpreterRT_aarch64.cpp line 352:

> 350: 
> 351: #ifdef __APPLE__
> 352:   virtual void pass_byte()

Please remove ```#ifdef __APPLE__``` around this region.

src/hotspot/cpu/aarch64/sharedRuntime_aarch64.cpp line 839:

> 837:   // The code unable to handle this, bailout.
> 838:   return -1;
> 839: #endif

This looks like a bug to me. The caller doesn't necessarily check the return 
value. See CallRuntimeNode::calling_convention.

-

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

Re: AArch64 OpenJDK bootstrap failure on head

[Andrew Haley]

On 2/1/21 5:14 PM, Aleksey Shipilev wrote:
> On 2/1/21 4:38 PM, Andrew Haley wrote:
>> but that doesn't work either. Any ideas? I'm really stuck.
> 
> Did you "make clean" after changing any of the configure files and/or 
> configure arguments? I.e. did 
> AWTIcon32_java_icon16_png actually regenerate?

Many times.

> Prepending the build with "LOG=debug" would tell what cmdlines are used at 
> every step of build process.

Sure, I can see what it's doing. I think this is actually a regression
caused by the Windows-AArch64 port. I'll do some bisecting.

-- 
Andrew Haley  (he/him)
Java Platform Lead Engineer
Red Hat UK Ltd. <https://www.redhat.com>
https://keybase.io/andrewhaley
EAC8 43EB D3EF DB98 CC77 2FAD A5CD 6035 332F A671