Re: RFR: 8277012: Use blessed modifier order in src/utils
On Thu, 11 Nov 2021 14:32:18 GMT, Magnus Ihse Bursie wrote: > I ran bin/blessed-modifier-order.sh on source code in src/utils. This scripts > verifies that modifiers are in the "blessed" order, and fixes it otherwise. I > have manually checked the changes made by the script to make sure they are > sound. > > There are no clear ownership of this code, but I believe it's kind of > hotspot-related. Looks fine. Thanks, David - Marked as reviewed by dholmes (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/6354
Re: RFR: 8276905: Function frag_col has a deployment target which is incompatible with this OS [v2]
On Fri, 12 Nov 2021 03:56:57 GMT, Phil Race wrote: >> Jayathirth D V has updated the pull request incrementally with one >> additional commit since the last revision: >> >> Use Macro for version > > make/modules/java.desktop/lib/Awt2dLibraries.gmk line 850: > >> 848: COMMAND := $(METAL) -c -std=osx-metal2.0 \ >> 849: -mmacosx-version-min=$(MACOSX_VERSION_MIN) \ >> 850: -o $(SHADERS_AIR) $(SHADERS_SRC), \ > > No .. we decided metal requires macos 10.14 as a minimum. In fact it won't > run (by policy) on earlier releases so settting it to what the broader JDK > defines as a minimum is not appropriate right now. > And I've no idea what restrictions we'd be placing on metal saying it can > only use 10.12 features. > Nor do I know what the xcode 12.4 used to build JDK 17 defaults to and how > much a change to either 10.12 or 10.14 might require in re-testing. > > So - > we should use 10.14 > what's the actual impact of that on a current build using xcode 12.4 - does > it change anything we use ? bear in mind "impact" might mean metal can't use some new faster code, not just runtime errors - PR: https://git.openjdk.java.net/jdk/pull/6346
Re: RFR: 8276905: Function frag_col has a deployment target which is incompatible with this OS [v2]
On Thu, 11 Nov 2021 15:32:01 GMT, Jayathirth D V wrote: >> When JDK is built on BigSur with Xcode12.5 and used to run jtreg tests in >> macOS10.15 with Xcode <=12.4 we are getting compilation error. We dont set >> any deployment target when when we use xcrun to create .air file and this >> issue looks similar to https://developer.apple.com/forums/thread/105719. >> Also https://developer.apple.com/forums/thread/105719 states that even if >> they set app deployment target they need to explicitly set deployment in >> "xcrun metal". >> >> Made same change to use -mmacosx-version-min=10.12.00 in our make file. >> Whether we should keep 10.12.00 or 10.13/14 is open to discussion for metal >> pipeline. >> >> SwingSet2, J2DDemo & JCK/jtreg runs in CI are green. Also Vitaly(Submitter) >> has confirmed that patch resolves the issue. > > Jayathirth D V has updated the pull request incrementally with one additional > commit since the last revision: > > Use Macro for version make/modules/java.desktop/lib/Awt2dLibraries.gmk line 850: > 848: COMMAND := $(METAL) -c -std=osx-metal2.0 \ > 849: -mmacosx-version-min=$(MACOSX_VERSION_MIN) \ > 850: -o $(SHADERS_AIR) $(SHADERS_SRC), \ No .. we decided metal requires macos 10.14 as a minimum. In fact it won't run (by policy) on earlier releases so settting it to what the broader JDK defines as a minimum is not appropriate right now. And I've no idea what restrictions we'd be placing on metal saying it can only use 10.12 features. Nor do I know what the xcode 12.4 used to build JDK 17 defaults to and how much a change to either 10.12 or 10.14 might require in re-testing. So - we should use 10.14 what's the actual impact of that on a current build using xcode 12.4 - does it change anything we use ? - PR: https://git.openjdk.java.net/jdk/pull/6346
Re: RFR: 8275745: Reproducible copyright headers [v3]
On Thu, 11 Nov 2021 15:37:09 GMT, Emmanuel Bourg wrote: >> The copyright headers are generated at build time, and the year inserted in >> the template depends on the current date. This means the headers are not >> reproducible if the project is built a year later. The year in the headers >> could be derived from the SOURCE_DATE_EPOCH environment variable to make the >> build reproducible (this variable is already used in other parts of the >> build). > > Emmanuel Bourg has updated the pull request with a new target base due to a > merge or a rebase. The pull request now contains two commits: > > - Make gensrc code use $COPYRIGHT_YEAR > - Set COPYRIGHT_YEAR from SOURCE_DATE_EPOCH if present Thank you for the advice, I'm not familiar with the Skara workflow yet. - PR: https://git.openjdk.java.net/jdk/pull/1498
Re: RFR: 8276743: Make openjdk build Zip Archive generation "reproducible" [v4]
On Thu, 11 Nov 2021 20:18:40 GMT, Magnus Ihse Bursie wrote: > In the future, please refrain from force pushing to a PR. It makes history > hard to follow for reviewers, and is generally strongly discouraged. OpenJDK > uses the Skara tools which will automatically squash and rebase the commits > in the PR. @magicus I needed to cause a re-submit tests due to a macos timeout, and there seems no github Action or PR command to cause that, so I just force pushed, couldn't see any other way I presume there is? @magicus is pushing an empty commit or dummy change preferable? - PR: https://git.openjdk.java.net/jdk/pull/6311
Re: RFR: 8275745: Reproducible copyright headers [v3]
On Thu, 11 Nov 2021 15:37:09 GMT, Emmanuel Bourg wrote: >> The copyright headers are generated at build time, and the year inserted in >> the template depends on the current date. This means the headers are not >> reproducible if the project is built a year later. The year in the headers >> could be derived from the SOURCE_DATE_EPOCH environment variable to make the >> build reproducible (this variable is already used in other parts of the >> build). > > Emmanuel Bourg has updated the pull request with a new target base due to a > merge or a rebase. The pull request now contains two commits: > > - Make gensrc code use $COPYRIGHT_YEAR > - Set COPYRIGHT_YEAR from SOURCE_DATE_EPOCH if present In the future, please refrain from force pushing to a PR. It makes history hard to follow for reviewers, and is generally strongly discouraged. OpenJDK uses the Skara tools which will automatically squash and rebase the commits in the PR. - PR: https://git.openjdk.java.net/jdk/pull/1498
Re: RFR: 8276743: Make openjdk build Zip Archive generation "reproducible" [v4]
On Thu, 11 Nov 2021 19:48:04 GMT, Andrew Leonard wrote: >> This PR adds a new openjdk build tool GenerateZip, which generates a final >> "zip" file from an input folder, and creates it in a deterministic way, >> ensuring ordering and timestamps are set as specified. >> >> Using this tool in ZipArchive.gmk will ensure src.zip is then created >> deterministically. >> >> Signed-off-by: Andrew Leonard > > Andrew Leonard has refreshed the contents of this pull request, and previous > commits have been removed. The incremental views will show differences > compared to the previous content of the PR. In the future, please refrain from force pushing to a PR. It makes history hard to follow for reviewers, and is generally strongly discouraged. OpenJDK uses the Skara tools which will automatically squash and rebase the commits in the PR. - PR: https://git.openjdk.java.net/jdk/pull/6311
Re: RFR: 8276743: Make openjdk build Zip Archive generation "reproducible" [v4]
> This PR adds a new openjdk build tool GenerateZip, which generates a final > "zip" file from an input folder, and creates it in a deterministic way, > ensuring ordering and timestamps are set as specified. > > Using this tool in ZipArchive.gmk will ensure src.zip is then created > deterministically. > > Signed-off-by: Andrew Leonard Andrew Leonard has refreshed the contents of this pull request, and previous commits have been removed. The incremental views will show differences compared to the previous content of the PR. The pull request contains one new commit since the last revision: 8276743: Make openjdk build Zip Archive generation "reproducible" Signed-off-by: Andrew Leonard - Changes: - all: https://git.openjdk.java.net/jdk/pull/6311/files - new: https://git.openjdk.java.net/jdk/pull/6311/files/44036af7..8d148065 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=6311&range=03 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=6311&range=02-03 Stats: 0 lines in 0 files changed: 0 ins; 0 del; 0 mod Patch: https://git.openjdk.java.net/jdk/pull/6311.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/6311/head:pull/6311 PR: https://git.openjdk.java.net/jdk/pull/6311
Re: RFR: 8264130: PAC-RET protection for Linux/AArch64 [v2]
On Thu, 11 Nov 2021 18:07:37 GMT, Andrew Haley 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`. - PR: https://git.openjdk.java.net/jdk/pull/6334
Re: RFR: 8264130: PAC-RET protection for Linux/AArch64 [v2]
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]
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: 8276927: [PPC64] Port shenandoahgc to linux on ppc64le
On Wed, 10 Nov 2021 09:00:04 GMT, Niklas Radomski wrote:
> Port the Shenandoah garbage collector
> (JDK-8241457)[https://bugs.openjdk.java.net/browse/JDK-8241457] to linux on
> ppc64le.
src/hotspot/cpu/ppc/gc/shenandoah/shenandoahBarrierSetAssembler_ppc.cpp line
536:
> 534: if (!preserve_gp_registers) { __ clobber_volatile_gprs(dst); }
> 535: if (!needs_frame) { __ clobber_carg_stack_slots(tmp1); }
> 536: #endif
This clobber code was certainly good during development and early testing. But
is it worth keeping it? Other GCs and other places don't have it any more. So,
I'd slightly prefer removal. Feel free to do so if you agree.
-
PR: https://git.openjdk.java.net/jdk/pull/6325
Re: RFR: 8264130: PAC-RET protection for Linux/AArch64 [v2]
On Thu, 11 Nov 2021 15:30:29 GMT, Alan Hayward 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. That ensures that executable regions are executable but not writable (RX) from a Java thread when running JITted Java code and are writable but not executable (RW) when it calls into JVM code. > An additional concern I have is that if the globals data was attacked then > the UseROPProtection flag could be flipped, and all code after that point > would be generated without ROP protection. Marking all the globals data as RO > would fix that. Alternatively remove UseROPProtection and then in the > macroassembler always generate PAC code, using just the subset of > instructions that are NOPs on non-PAC hardware. Or alternatively only > generate PAC code based on a #define set at build time. Each option has its > own downsides. Globals data can legitimately be written during JVM startup (perhaps in some cases also during execution?). So, they cannot simply be marked as RO. I am not sure this concern is really warranted. If an attacker is already able to overwrite UseROPProtection then a concern over the resulting omission of JITted ROP protection seems like attending to the loud banging of the stable door while Shergar has already been diced into stew meat. - PR: https://git.openjdk.java.net/jdk/pull/6334
Re: RFR: 8276905: Function frag_col has a deployment target which is incompatible with this OS [v2]
On Thu, 11 Nov 2021 15:32:01 GMT, Jayathirth D V wrote: >> When JDK is built on BigSur with Xcode12.5 and used to run jtreg tests in >> macOS10.15 with Xcode <=12.4 we are getting compilation error. We dont set >> any deployment target when when we use xcrun to create .air file and this >> issue looks similar to https://developer.apple.com/forums/thread/105719. >> Also https://developer.apple.com/forums/thread/105719 states that even if >> they set app deployment target they need to explicitly set deployment in >> "xcrun metal". >> >> Made same change to use -mmacosx-version-min=10.12.00 in our make file. >> Whether we should keep 10.12.00 or 10.13/14 is open to discussion for metal >> pipeline. >> >> SwingSet2, J2DDemo & JCK/jtreg runs in CI are green. Also Vitaly(Submitter) >> has confirmed that patch resolves the issue. > > Jayathirth D V has updated the pull request incrementally with one additional > commit since the last revision: > > Use Macro for version Marked as reviewed by ihse (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/6346
Re: RFR: 8276743: Make openjdk build Zip Archive generation "reproducible"
On Wed, 10 Nov 2021 14:39:09 GMT, Erik Joelsson wrote: >> @erikj79 The flag --enable-reproducible-builds sets >> ENABLE_REPRODUCIBLE_BUILD in spec.gmk. This is set by our JIB profiles. I >> propose that we also turn it on for GHA builds. >> >> I think that the post-processing of the zip file can be dependent on this >> variable and that it serves no purpose to introduce a separate variable >> ENABLE_REPRODUCIBLE_ZIP that is set to the same value as >> ENABLE_REPRODUCIBLE_BUILD. Do you agree? > >> I think that the post-processing of the zip file can be dependent on this >> variable and that it serves no purpose to introduce a separate variable >> ENABLE_REPRODUCIBLE_ZIP that is set to the same value as >> ENABLE_REPRODUCIBLE_BUILD. Do you agree? > > Sure, that works for me. @erikj79 @magicus I have just pushed a new commit with the suggested changes, and it works well, thank you for the help I've also done a basic average benchmarking, on my rather slow Ubuntu VM: - Existing src.zip processing : 18 seconds - Additional MakeZipReproducible : 6 seconds - PR: https://git.openjdk.java.net/jdk/pull/6311
Re: RFR: 8275745: Reproducible copyright headers [v3]
> The copyright headers are generated at build time, and the year inserted in > the template depends on the current date. This means the headers are not > reproducible if the project is built a year later. The year in the headers > could be derived from the SOURCE_DATE_EPOCH environment variable to make the > build reproducible (this variable is already used in other parts of the > build). Emmanuel Bourg 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 two additional commits since the last revision: - Make gensrc code use $COPYRIGHT_YEAR - Set COPYRIGHT_YEAR from SOURCE_DATE_EPOCH if present - Changes: - all: https://git.openjdk.java.net/jdk/pull/1498/files - new: https://git.openjdk.java.net/jdk/pull/1498/files/9c4efbda..618d28a4 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=1498&range=02 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=1498&range=01-02 Stats: 118724 lines in 2698 files changed: 90310 ins; 15575 del; 12839 mod Patch: https://git.openjdk.java.net/jdk/pull/1498.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/1498/head:pull/1498 PR: https://git.openjdk.java.net/jdk/pull/1498
Re: RFR: 8264130: PAC-RET protection for Linux/AArch64 [v2]
On Thu, 11 Nov 2021 14:52:54 GMT, Andrew Dinn wrote: > The runtime generated runtime stubs and Java method code into which this > patch may insert the required PAC instructions are written into a code cache > in a section which is mapped RW(X) all the time. It would be hard to map even > a subset of this code cache RO because generated code includes call and data > sites that need to be patched during execution. Am I right is saying that for Macos, all generated code is remapped RO before execution? An additional concern I have is that if the globals data was attacked then the UseROPProtection flag could be flipped, and all code after that point would be generated without ROP protection. Marking all the globals data as RO would fix that. Alternatively remove UseROPProtection and then in the macroassembler always generate PAC code, using just the subset of instructions that are NOPs on non-PAC hardware. Or alternatively only generate PAC code based on a #define set at build time. Each option has its own downsides. - PR: https://git.openjdk.java.net/jdk/pull/6334
Re: RFR: 8276905: Function frag_col has a deployment target which is incompatible with this OS
On Thu, 11 Nov 2021 13:53:05 GMT, Magnus Ihse Bursie wrote: > Also, if you did not create this patch yourself, please make sure to use > `/contributor` to give proper credits. Or maybe you mean that Vitaly > submitted the bug report, not the patch? By Submitter i meant submitter of bug in JBS. I was not able to verify the patch in our CI, so i shared the patch with Vitaly so that he can verify the reproducibility of the issue. - PR: https://git.openjdk.java.net/jdk/pull/6346
Re: RFR: 8276905: Function frag_col has a deployment target which is incompatible with this OS [v2]
> When JDK is built on BigSur with Xcode12.5 and used to run jtreg tests in > macOS10.15 with Xcode <=12.4 we are getting compilation error. We dont set > any deployment target when when we use xcrun to create .air file and this > issue looks similar to https://developer.apple.com/forums/thread/105719. Also > https://developer.apple.com/forums/thread/105719 states that even if they set > app deployment target they need to explicitly set deployment in "xcrun metal". > > Made same change to use -mmacosx-version-min=10.12.00 in our make file. > Whether we should keep 10.12.00 or 10.13/14 is open to discussion for metal > pipeline. > > SwingSet2, J2DDemo & JCK/jtreg runs in CI are green. Also Vitaly(Submitter) > has confirmed that patch resolves the issue. Jayathirth D V has updated the pull request incrementally with one additional commit since the last revision: Use Macro for version - Changes: - all: https://git.openjdk.java.net/jdk/pull/6346/files - new: https://git.openjdk.java.net/jdk/pull/6346/files/a9f521a5..7f999650 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=6346&range=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=6346&range=00-01 Stats: 3 lines in 1 file changed: 2 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/6346.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/6346/head:pull/6346 PR: https://git.openjdk.java.net/jdk/pull/6346
Re: RFR: 8276905: Function frag_col has a deployment target which is incompatible with this OS [v2]
On Thu, 11 Nov 2021 13:52:13 GMT, Magnus Ihse Bursie wrote: > We should not hard-code version numbers like that. > > Fortunately for you, there already exists a variable $(MACOSX_VERSION_MIN) > that you can use. Thanks for the review i have updated code to use the Macro. - PR: https://git.openjdk.java.net/jdk/pull/6346
Re: RFR: 8276743: Make openjdk build Zip Archive generation "reproducible" [v3]
> This PR adds a new openjdk build tool GenerateZip, which generates a final > "zip" file from an input folder, and creates it in a deterministic way, > ensuring ordering and timestamps are set as specified. > > Using this tool in ZipArchive.gmk will ensure src.zip is then created > deterministically. > > Signed-off-by: Andrew Leonard Andrew Leonard has refreshed the contents of this pull request, and previous commits have been removed. The incremental views will show differences compared to the previous content of the PR. The pull request contains one new commit since the last revision: 8276743: Make openjdk build Zip Archive generation "reproducible" Signed-off-by: Andrew Leonard - Changes: - all: https://git.openjdk.java.net/jdk/pull/6311/files - new: https://git.openjdk.java.net/jdk/pull/6311/files/f8a816af..44036af7 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=6311&range=02 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=6311&range=01-02 Stats: 3 lines in 1 file changed: 0 ins; 0 del; 3 mod Patch: https://git.openjdk.java.net/jdk/pull/6311.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/6311/head:pull/6311 PR: https://git.openjdk.java.net/jdk/pull/6311
Re: RFR: 8276743: Make openjdk build Zip Archive generation "reproducible" [v2]
On Wed, 10 Nov 2021 11:22:39 GMT, Andrew Leonard wrote: >> Actually, you don't even need to save the ZipEntry:s in memory, you can just >> extract filenames from them on the first pass, sort them, then lookup the >> entries in ZipFile again on the second lap. :) I don't think that's >> necessary though. > > @erikj79 thanks I didn't realize you can do that: "you can use the ZipFile > class to access ZipEntry's in arbitrary order" See new commit - PR: https://git.openjdk.java.net/jdk/pull/6311
Re: RFR: 8276743: Make openjdk build Zip Archive generation "reproducible" [v2]
On Tue, 9 Nov 2021 14:00:18 GMT, Erik Joelsson wrote: >> Andrew Leonard has updated the pull request incrementally with one >> additional commit since the last revision: >> >> 8276743: Make openjdk build Zip Archive generation "reproducible" >> >> Signed-off-by: Andrew Leonard > > make/common/ZipArchive.gmk line 29: > >> 27: _ZIP_ARCHIVE_GMK := 1 >> 28: >> 29: include ../ToolsJdk.gmk > > Should probably add a comment about inclusion of this file now requires an > explicit dependency on build-tools in Main.gmk. done - PR: https://git.openjdk.java.net/jdk/pull/6311
Re: RFR: 8276743: Make openjdk build Zip Archive generation "reproducible" [v2]
> This PR adds a new openjdk build tool GenerateZip, which generates a final > "zip" file from an input folder, and creates it in a deterministic way, > ensuring ordering and timestamps are set as specified. > > Using this tool in ZipArchive.gmk will ensure src.zip is then created > deterministically. > > Signed-off-by: Andrew Leonard Andrew Leonard has updated the pull request incrementally with one additional commit since the last revision: 8276743: Make openjdk build Zip Archive generation "reproducible" Signed-off-by: Andrew Leonard - Changes: - all: https://git.openjdk.java.net/jdk/pull/6311/files - new: https://git.openjdk.java.net/jdk/pull/6311/files/dcf48d65..f8a816af Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=6311&range=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=6311&range=00-01 Stats: 544 lines in 5 files changed: 241 ins; 287 del; 16 mod Patch: https://git.openjdk.java.net/jdk/pull/6311.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/6311/head:pull/6311 PR: https://git.openjdk.java.net/jdk/pull/6311
Re: RFR: 8275745: Reproducible copyright headers
On Sat, 28 Nov 2020 23:14:35 GMT, Emmanuel Bourg wrote: > The copyright headers are generated at build time, and the year inserted in > the template depends on the current date. This means the headers are not > reproducible if the project is built a year later. The year in the headers > could be derived from the SOURCE_DATE_EPOCH environment variable to make the > build reproducible (this variable is already used in other parts of the > build). I can rebase the branch with your changes only, my commit just adds noise. - PR: https://git.openjdk.java.net/jdk/pull/1498
Re: RFR: 8275745: Reproducible copyright headers [v2]
> The copyright headers are generated at build time, and the year inserted in > the template depends on the current date. This means the headers are not > reproducible if the project is built a year later. The year in the headers > could be derived from the SOURCE_DATE_EPOCH environment variable to make the > build reproducible (this variable is already used in other parts of the > build). Emmanuel Bourg has updated the pull request incrementally with two additional commits since the last revision: - Make gensrc code use $COPYRIGHT_YEAR - Set COPYRIGHT_YEAR from SOURCE_DATE_EPOCH if present - Changes: - all: https://git.openjdk.java.net/jdk/pull/1498/files - new: https://git.openjdk.java.net/jdk/pull/1498/files/99161710..9c4efbda Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=1498&range=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=1498&range=00-01 Stats: 59 lines in 8 files changed: 28 ins; 22 del; 9 mod Patch: https://git.openjdk.java.net/jdk/pull/1498.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/1498/head:pull/1498 PR: https://git.openjdk.java.net/jdk/pull/1498
Re: RFR: 8275745: Reproducible copyright headers [v2]
On Thu, 11 Nov 2021 15:12:21 GMT, Emmanuel Bourg wrote: >> The copyright headers are generated at build time, and the year inserted in >> the template depends on the current date. This means the headers are not >> reproducible if the project is built a year later. The year in the headers >> could be derived from the SOURCE_DATE_EPOCH environment variable to make the >> build reproducible (this variable is already used in other parts of the >> build). > > Emmanuel Bourg has updated the pull request incrementally with two additional > commits since the last revision: > > - Make gensrc code use $COPYRIGHT_YEAR > - Set COPYRIGHT_YEAR from SOURCE_DATE_EPOCH if present LGTM but then again I wrote most of it. :) Please await further reviewers before integrating. - Marked as reviewed by ihse (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/1498
Re: RFR: 8275745: Reproducible copyright headers
On Thu, 11 Nov 2021 13:45:21 GMT, Magnus Ihse Bursie wrote: >> Yes that's fine. I guess this involves setting the `COPYRIGHT_YEAR` variable >> in `make/autoconf/jdk-options.m4` to a value derived from >> `SOURCE_DATE_EPOCH` (with `SOURCE_DATE_EPOCH` having the priority over >> `--with-copyright-year` or the opposite?), and then pick the value of >> `COPYRIGHT_YEAR` in `CopyrightHeaders.java` and `EquivMapsGenerator.java`, >> right? > > @ebourg I have now modified this patch so it uses COPYRIGHT_YEAR, and sets > COPYRIGHT_YEAR based on SOURCE_DATE_EPOCH, if it exists. The patch is in a > branch in my personal fork, > https://github.com/magicus/jdk/tree/reproducible-copyright-year. > > If you think it looks good, we have two possible ways forward. Either you > close this PR, and I open a new targeting the same JBS issue, and credit you > as co-author. Or you integrate my branch into this PR, and credit me as > co-author. Any of them is OK for me, but I think the former is simpler. @magicus Thank you! I've integrated your branch - PR: https://git.openjdk.java.net/jdk/pull/1498
Re: RFR: 8264130: PAC-RET protection for Linux/AArch64 [v2]
On Thu, 11 Nov 2021 14:20:33 GMT, Florian Weimer wrote: > Is the code still mapped read-write all the time? That depends on what code you mean. The JVM code compiled from C++ sources is mapped RO(X) in the text section like any compiled C/C++ code. Protection of that code is covered by the changes to the build system. The runtime generated runtime stubs and Java method code into which this patch may insert the required PAC instructions are written into a code cache in a section which is mapped RW(X) all the time. It would be hard to map even a subset of this code cache RO because generated code includes call and data sites that need to be patched during execution. - PR: https://git.openjdk.java.net/jdk/pull/6334
Re: RFR: 8276927: [PPC64] Port shenandoahgc to linux on ppc64le
On Thu, 11 Nov 2021 14:30:05 GMT, Martin Doerr wrote:
>> src/hotspot/cpu/ppc/gc/shenandoah/c1/shenandoahBarrierSetC1_ppc.cpp line 83:
>>
>>> 81: LIRGenerator* gen = access.gen();
>>> 82:
>>> 83: if (ShenandoahCASBarrier) {
>>
>> I am not sure, but I almost think we should not even end up in the method
>> with -ShenandoahCASBarrier. If anything, -ShenandoahCASBarrier should result
>> in only calling super to emit regular CAS without any barriers.
>
> We hit this case when running `jdk/bin/java -XX:+UseShenandoahGC
> -XX:ShenandoahGCMode=passive -version`. x86 and aarch64 check for
> ShenandoahCASBarrier, too. So, looks like these checks are needed and correct.
Ok then.
-
PR: https://git.openjdk.java.net/jdk/pull/6325
Re: RFR: 8264130: PAC-RET protection for Linux/AArch64 [v2]
On Thu, 11 Nov 2021 14:53:54 GMT, Florian Weimer wrote: >> I don't agree that this is incorrect, at least not for the stated reason. >> The flag switches on a protection mechanism that guards against ROP attacks. >> To my reading that does not imply it guards against all such attacks, merely >> that this is the nature of the protection it offers. >> >> The description might still be considered incorrect for an unrelated reason. >> Its use of the adjectival phrase ROP based constitutes a transferred >> epithet, conflating the symptom with the medicine. In other words, the >> protection offered is not ROP based i.e. does not rely on an ROP technique. >> What it does is protect against ROP attacks. So, I'd suggest rewording to >> >> "Enable protection of branches against ROP attacks". >> >> Florian, if you want to argue for rewording that to "Enable protection of >> branches against some categories of ROP attacks" or some other equivalently >> qualified variant please feel free to make a case. However, I don't think >> see any need to add that rider, nor any precedent in any of the other short >> descriptions provided in globals.hpp. > > 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. - PR: https://git.openjdk.java.net/jdk/pull/6334
Re: RFR: 8264130: PAC-RET protection for Linux/AArch64 [v2]
On Thu, 11 Nov 2021 14:43:59 GMT, Andrew Dinn wrote: >> src/hotspot/cpu/aarch64/globals_aarch64.hpp line 115: >> >>> 113: range(-1, 4096) >>> \ >>> 114: product(bool, UseROPProtection, false, >>> \ >>> 115: "Use ROP based branch protection") >>> \ >> >> The description is not correct. It's protection against certain ROP-based >> attack techniques. > > I don't agree that this is incorrect, at least not for the stated reason. The > flag switches on a protection mechanism that guards against ROP attacks. To > my reading that does not imply it guards against all such attacks, merely > that this is the nature of the protection it offers. > > The description might still be considered incorrect for an unrelated reason. > Its use of the adjectival phrase ROP based constitutes a transferred epithet, > conflating the symptom with the medicine. In other words, the protection > offered is not ROP based i.e. does not rely on an ROP technique. What it does > is protect against ROP attacks. So, I'd suggest rewording to > > "Enable protection of branches against ROP attacks". > > Florian, if you want to argue for rewording that to "Enable protection of > branches against some categories of ROP attacks" or some other equivalently > qualified variant please feel free to make a case. However, I don't think see > any need to add that rider, nor any precedent in any of the other short > descriptions provided in globals.hpp. I did mean the description, not the flag name. - PR: https://git.openjdk.java.net/jdk/pull/6334
Re: RFR: 8264130: PAC-RET protection for Linux/AArch64 [v2]
On Thu, 11 Nov 2021 14:20:20 GMT, Florian Weimer wrote: >> Alan Hayward has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Simplify branch protection configure check > > src/hotspot/cpu/aarch64/globals_aarch64.hpp line 115: > >> 113: range(-1, 4096) >> \ >> 114: product(bool, UseROPProtection, false, >> \ >> 115: "Use ROP based branch protection") >> \ > > The description is not correct. It's protection against certain ROP-based > attack techniques. I don't agree that this is incorrect, at least not for the stated reason. The flag switches on a protection mechanism that guards against ROP attacks. To my reading that does not imply it guards against all such attacks, merely that this is the nature of the protection it offers. The description might still be considered incorrect for an unrelated reason. Its use of the adjectival phrase ROP based constitutes a transferred epithet, conflating the symptom with the medicine. In other words, the protection offered is not ROP based i.e. does not rely on an ROP technique. What it does is protect against ROP attacks. So, I'd suggest rewording to "Enable protection of branches against ROP attacks". Florian, if you want to argue for rewording that to "Enable protection of branches against some categories of ROP attacks" or some other equivalently qualified variant please feel free to make a case. However, I don't think see any need to add that rider, nor any precedent in any of the other short descriptions provided in globals.hpp. - PR: https://git.openjdk.java.net/jdk/pull/6334
RFR: 8277012: Use blessed modifier order in src/utils
I ran bin/blessed-modifier-order.sh on source code in src/utils. This scripts verifies that modifiers are in the "blessed" order, and fixes it otherwise. I have manually checked the changes made by the script to make sure they are sound. There are no clear ownership of this code, but I believe it's kind of hotspot-related. - Commit messages: - 8277012: Use blessed modifier order in src/utils Changes: https://git.openjdk.java.net/jdk/pull/6354/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=6354&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8277012 Stats: 25 lines in 10 files changed: 0 ins; 0 del; 25 mod Patch: https://git.openjdk.java.net/jdk/pull/6354.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/6354/head:pull/6354 PR: https://git.openjdk.java.net/jdk/pull/6354
Re: RFR: 8276927: [PPC64] Port shenandoahgc to linux on ppc64le
On Thu, 11 Nov 2021 11:32:49 GMT, Roman Kennke wrote:
>> Port the Shenandoah garbage collector
>> (JDK-8241457)[https://bugs.openjdk.java.net/browse/JDK-8241457] to linux on
>> ppc64le.
>
> src/hotspot/cpu/ppc/gc/shenandoah/c1/shenandoahBarrierSetC1_ppc.cpp line 83:
>
>> 81: LIRGenerator* gen = access.gen();
>> 82:
>> 83: if (ShenandoahCASBarrier) {
>
> I am not sure, but I almost think we should not even end up in the method
> with -ShenandoahCASBarrier. If anything, -ShenandoahCASBarrier should result
> in only calling super to emit regular CAS without any barriers.
We hit this case when running `jdk/bin/java -XX:+UseShenandoahGC
-XX:ShenandoahGCMode=passive -version`. x86 and aarch64 check for
ShenandoahCASBarrier, too. So, looks like these checks are needed and correct.
-
PR: https://git.openjdk.java.net/jdk/pull/6325
Re: RFR: 8276927: [PPC64] Port shenandoahgc to linux on ppc64le
On Wed, 10 Nov 2021 09:00:04 GMT, Niklas Radomski wrote:
> Port the Shenandoah garbage collector
> (JDK-8241457)[https://bugs.openjdk.java.net/browse/JDK-8241457] to linux on
> ppc64le.
Nice work! Looks correct.
For others: Note that this change already contains feedback from my offline
review.
src/hotspot/cpu/ppc/gc/shenandoah/shenandoahBarrierSetAssembler_ppc.cpp line 74:
> 72: // IU barriers are also employed to avoid resurrection of weak
> references,
> 73: // even if Shenandoah does not operate in incremental update mode.
> 74: if (ShenandoahIUBarrier || ShenandoahSATBBarrier) {
Sharing the code for IU and SATB sounds like a good idea, but one needs to be
careful. `ShenandoahBarrierSetC1::iu_barrier` only works with
`ShenandoahIUBarrier`, so this trick can't be used in C1.
It's a bit confusing, but I'm ok with this version. At least, I don't have any
better suggestion at the moment.
-
Marked as reviewed by mdoerr (Reviewer).
PR: https://git.openjdk.java.net/jdk/pull/6325
Re: RFR: 8264130: PAC-RET protection for Linux/AArch64 [v2]
On Thu, 11 Nov 2021 08:48:07 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: > > Simplify branch protection configure check Is the code still mapped read-write all the time? src/hotspot/cpu/aarch64/globals_aarch64.hpp line 115: > 113: range(-1, 4096) \ > 114: product(bool, UseROPProtection, false,\ > 115: "Use ROP based branch protection")\ The description is not correct. It's protection against certain ROP-based attack techniques. - PR: https://git.openjdk.java.net/jdk/pull/6334
Re: RFR: 8276905: Function frag_col has a deployment target which is incompatible with this OS
On Thu, 11 Nov 2021 05:52:18 GMT, Jayathirth D V wrote: > When JDK is built on BigSur with Xcode12.5 and used to run jtreg tests in > macOS10.15 with Xcode <=12.4 we are getting compilation error. We dont set > any deployment target when when we use xcrun to create .air file and this > issue looks similar to https://developer.apple.com/forums/thread/105719. Also > https://developer.apple.com/forums/thread/105719 states that even if they set > app deployment target they need to explicitly set deployment in "xcrun metal". > > Made same change to use -mmacosx-version-min=10.12.00 in our make file. > Whether we should keep 10.12.00 or 10.13/14 is open to discussion for metal > pipeline. > > SwingSet2, J2DDemo & JCK/jtreg runs in CI are green. Also Vitaly(Submitter) > has confirmed that patch resolves the issue. We should not hard-code version numbers like that. Fortunately for you, there already exists a variable $(MACOSX_VERSION_MIN) that you can use. Also, if you did not create this patch yourself, please make sure to use `/contributor` to give proper credits. Or maybe you mean that Vitaly submitted the bug report, not the patch? - Changes requested by ihse (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/6346
Re: RFR: 8275745: Reproducible copyright headers
On Sun, 24 Oct 2021 12:11:57 GMT, Emmanuel Bourg wrote: >> The copyright headers are generated at build time, and the year inserted in >> the template depends on the current date. This means the headers are not >> reproducible if the project is built a year later. The year in the headers >> could be derived from the SOURCE_DATE_EPOCH environment variable to make the >> build reproducible (this variable is already used in other parts of the >> build). > > Yes that's fine. I guess this involves setting the `COPYRIGHT_YEAR` variable > in `make/autoconf/jdk-options.m4` to a value derived from `SOURCE_DATE_EPOCH` > (with `SOURCE_DATE_EPOCH` having the priority over `--with-copyright-year` or > the opposite?), and then pick the value of `COPYRIGHT_YEAR` in > `CopyrightHeaders.java` and `EquivMapsGenerator.java`, right? @ebourg I have now modified this patch so it uses COPYRIGHT_YEAR, and sets COPYRIGHT_YEAR based on SOURCE_DATE_EPOCH, if it exists. The patch is in a branch in my personal fork, https://github.com/magicus/jdk/tree/reproducible-copyright-year. If you think it looks good, we have two possible ways forward. Either you close this PR, and I open a new targeting the same JBS issue, and credit you as co-author. Or you integrate my branch into this PR, and credit me as co-author. Any of them is OK for me, but I think the former is simpler. - PR: https://git.openjdk.java.net/jdk/pull/1498
Re: RFR: 8275063: Implementation of Foreign Function & Memory API (Second incubator) [v23]
> This PR contains the API and implementation changes for JEP-419 [1]. A more > detailed description of such changes, to avoid repetitions during the review > process, is included as a separate comment. > > [1] - https://openjdk.java.net/jeps/419 Maurizio Cimadamore has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 32 commits: - Merge branch 'master' into JEP-419 - Revert removal of upcall MH customization (This change caused spurious VM crashes, so reverting to baseline) - Further tweak upcall safety considerations - Clarify safety considerations for upcalls - Rename MemorySegment::ofAddressNative to MemorySegment::ofAddress (which is consistent with other restricted factories in VaList and NativeSymbol) - Streamline javadoc for package-info - * Add two new CLinker static methods to compute upcall/downcall method types * Clarify section on CLinker downcall type * Add section on CLinker safety guarantees - Fix TestUpcall * reverse() has a bug, as it doesn't tweak parameter types * reverse() is applied to the wrong MH - Make ArenaAllocator impl more flexible in the face of OOME An ArenaAllocator should remain open for business, even if OOME is thrown in case other allocations can fit the arena size. - Simplify ArenaAllocator impl. The arena should respect its boundaries and never allocate more memory than its size specifies. - ... and 22 more: https://git.openjdk.java.net/jdk/compare/aea09677...8c3860f8 - Changes: https://git.openjdk.java.net/jdk/pull/5907/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=5907&range=22 Stats: 14686 lines in 193 files changed: 6956 ins; 5120 del; 2610 mod Patch: https://git.openjdk.java.net/jdk/pull/5907.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/5907/head:pull/5907 PR: https://git.openjdk.java.net/jdk/pull/5907
Re: RFR: 8276927: [PPC64] Port shenandoahgc to linux on ppc64le
On Wed, 10 Nov 2021 09:00:04 GMT, Niklas Radomski wrote: > Port the Shenandoah garbage collector > (JDK-8241457)[https://bugs.openjdk.java.net/browse/JDK-8241457] to linux on > ppc64le. Build changes look good. Actual code changes needs to be reviewed by someone more knowledgable about this area. - Marked as reviewed by ihse (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/6325
Re: RFR: 8276927: [PPC64] Port shenandoahgc to linux on ppc64le
On Wed, 10 Nov 2021 09:00:04 GMT, Niklas Radomski wrote:
> Port the Shenandoah garbage collector
> (JDK-8241457)[https://bugs.openjdk.java.net/browse/JDK-8241457] to linux on
> ppc64le.
Hi Niklas,
thanks for this awesome work!
I can't really comment on the actual PPC code, so this needs to be reviewed by
somebody else. Structurally the change looks correct. I have one comment about
the C1 CAS barrier code, but it's minor.
Thanks & cheers,
Roman
src/hotspot/cpu/ppc/gc/shenandoah/c1/shenandoahBarrierSetC1_ppc.cpp line 83:
> 81: LIRGenerator* gen = access.gen();
> 82:
> 83: if (ShenandoahCASBarrier) {
I am not sure, but I almost think we should not even end up in the method with
-ShenandoahCASBarrier. If anything, -ShenandoahCASBarrier should result in only
calling super to emit regular CAS without any barriers.
-
Marked as reviewed by rkennke (Reviewer).
PR: https://git.openjdk.java.net/jdk/pull/6325
Re: RFR: 8264130: PAC-RET protection for Linux/AArch64 [v2]
On Thu, 11 Nov 2021 08:48:07 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: > > Simplify branch protection configure check Build changes look much better now, thanks! Build part approved; the actual code changes needs approval from others. - Marked as reviewed by ihse (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/6334
Re: RFR: 8264130: PAC-RET protection for Linux/AArch64 [v2]
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]
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 [v2]
On Thu, 11 Nov 2021 11:34:09 GMT, Andrew Dinn wrote: >> As far as the AArch64 docs are concerned the relevant details are provided >> in ARM-ARM D >> >> - The PAC functionality is described in ARM-ARM Section D5.1.5 >> - Overview of the PAC instructions is provided in section C3.1.9 >> - Detailed PAC instruction descriptions are provided in C6.2.195 - C6.2.199 >> >> n.b. I am specifically referring to my (possibly out of date) copy ARM-DDI >> 0487D.a (ID103018) which is the Initial v8.4 EAC release from 2018. >> >> That said, I agree that a description of how these functions use the >> underlying PAC support and what, effectively, they achieve via that usage >> would be necessary. A reference to the relevant sections of the ARM doc in >> the code would be helpful. > > 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. - PR: https://git.openjdk.java.net/jdk/pull/6334
Re: RFR: 8264130: PAC-RET protection for Linux/AArch64 [v2]
On Thu, 11 Nov 2021 11:19:03 GMT, Andrew Dinn wrote:
>> 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.
>
> As far as the AArch64 docs are concerned the relevant details are provided in
> ARM-ARM D
>
> - The PAC functionality is described in ARM-ARM Section D5.1.5
> - Overview of the PAC instructions is provided in section C3.1.9
> - Detailed PAC instruction descriptions are provided in C6.2.195 - C6.2.199
>
> n.b. I am specifically referring to my (possibly out of date) copy ARM-DDI
> 0487D.a (ID103018) which is the Initial v8.4 EAC release from 2018.
>
> That said, I agree that a description of how these functions use the
> underlying PAC support and what, effectively, they achieve via that usage
> would be necessary. A reference to the relevant sections of the ARM doc in
> the code would be helpful.
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
-
PR: https://git.openjdk.java.net/jdk/pull/6334
Re: RFR: 8264130: PAC-RET protection for Linux/AArch64 [v2]
On Wed, 10 Nov 2021 13:22:37 GMT, Andrew Haley wrote:
>> Alan Hayward has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Simplify branch protection configure check
>
> 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.
As far as the AArch64 docs are concerned the relevant details are provided in
ARM-ARM D
- The PAC functionality is described in ARM-ARM Section D5.1.5
- Overview of the PAC instructions is provided in section C3.1.9
- Detailed PAC instruction descriptions are provided in C6.2.195 - C6.2.199
n.b. I am specifically referring to my (possibly out of date) copy ARM-DDI
0487D.a (ID103018) which is the Initial v8.4 EAC release from 2018.
That said, I agree that a description of how these functions use the underlying
PAC support and what, effectively, they achieve via that usage would be
necessary. A reference to the relevant sections of the ARM doc in the code
would be helpful.
-
PR: https://git.openjdk.java.net/jdk/pull/6334
RFR: 8276927: [PPC64] Port shenandoahgc to linux on ppc64le
Port the Shenandoah garbage collector (JDK-8241457)[https://bugs.openjdk.java.net/browse/JDK-8241457] to linux on ppc64le. - Commit messages: - Port shenandoahgc to linux on ppc64le Changes: https://git.openjdk.java.net/jdk/pull/6325/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=6325&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8276927 Stats: 1526 lines in 8 files changed: 1524 ins; 0 del; 2 mod Patch: https://git.openjdk.java.net/jdk/pull/6325.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/6325/head:pull/6325 PR: https://git.openjdk.java.net/jdk/pull/6325
Re: RFR: 8264130: PAC-RET protection for Linux/AArch64 [v2]
> 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: Simplify branch protection configure check - Changes: - all: https://git.openjdk.java.net/jdk/pull/6334/files - new: https://git.openjdk.java.net/jdk/pull/6334/files/e0e3f666..29471d30 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=6334&range=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=6334&range=00-01 Stats: 12 lines in 1 file changed: 0 ins; 6 del; 6 mod Patch: https://git.openjdk.java.net/jdk/pull/6334.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/6334/head:pull/6334 PR: https://git.openjdk.java.net/jdk/pull/6334
