Re: RFR: 8253795: Implementation of JEP 391: macOS/AArch64 Port [v3]
On Tue, 26 Jan 2021 12:34:11 GMT, Alan Hayward wrote: >>> AIUI, the configure line needs passing a prebuilt JavaNativeFoundation >>> framework >>> ie: >>> `--with-extra-ldflags='-F >>> /Applications/Xcode.app/Contents/SharedFrameworks/ContentDeliveryServices.framework/Versions/A/itms/java/Frameworks/'` >>> >>> Otherwise there will be missing _JNFNative* functions. >>> >>> Is this the long term plan? Or will eventually the required code be moved >>> into JDK and/or the xcode one automatically get picked up by the configure >>> scripts? >> >> There is ongoing work by P. Race to eliminate dependence on JNF at all > >> > AIUI, the configure line needs passing a prebuilt JavaNativeFoundation >> > framework >> > ie: >> > `--with-extra-ldflags='-F >> > /Applications/Xcode.app/Contents/SharedFrameworks/ContentDeliveryServices.framework/Versions/A/itms/java/Frameworks/'` >> > Otherwise there will be missing _JNFNative* functions. >> > Is this the long term plan? Or will eventually the required code be moved >> > into JDK and/or the xcode one automatically get picked up by the configure >> > scripts? >> >> There is ongoing work by P. Race to eliminate dependence on JNF at all > > Ok, that's great. > In the meantime is it worth adding something to the MacOS section of > doc/building.* ? > (I pieced together the required line from multiple posts in a mailing list) Hi Anton, Just to add weight to comments already made by @coleenp and @stefank , I also find the W^X coding support to be too intrusive and polluting of the shared code. I would much rather see this support pushed down as far as possible, to minimise the impact and to use ifdef'd code for macos/Aarch64 (via MACOS_AARCH64_ONLY macro) rather than providing empty methods. Thanks, David - PR: https://git.openjdk.java.net/jdk/pull/2200
Integrated: 8252412: [macos11] system dynamic libraries removed from filesystem
On Sun, 17 Jan 2021 20:24:44 GMT, Martin Buchholz wrote: > 8252412: [macos11] system dynamic libraries removed from filesystem This pull request has now been integrated. Changeset: c836da38 Author:Martin Buchholz URL: https://git.openjdk.java.net/jdk/commit/c836da38 Stats: 19 lines in 1 file changed: 18 ins; 0 del; 1 mod 8252412: [macos11] system dynamic libraries removed from filesystem Co-authored-by: Dominik Röttsches Reviewed-by: jiangli, valeriep - PR: https://git.openjdk.java.net/jdk/pull/2119
Re: RFR: 8252412: [macos11] system dynamic libraries removed from filesystem [v2]
On Sat, 23 Jan 2021 02:03:01 GMT, Valerie Peng wrote: >> Martin Buchholz 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. > > Marked as reviewed by valeriep (Reviewer). @drott @valeriepeng @AlanBateman @jianglizhou Thanks all ! - PR: https://git.openjdk.java.net/jdk/pull/2119
Re: RFR: 8253795: Implementation of JEP 391: macOS/AArch64 Port [v3]
On Tue, 26 Jan 2021 09:23:18 GMT, Magnus Ihse Bursie wrote: >> Anton Kozlov has updated the pull request incrementally with two additional >> commits since the last revision: >> >> - Refactor CDS disabling >> - Redo builsys support for aarch64-darwin > > make/autoconf/build-aux/autoconf-config.guess line 1275: > >> 1273: UNAME_PROCESSOR="aarch64" >> 1274: fi >> 1275:fi ;; > > Almost, but not quite, correct. We cannot change the autoconf-config.guess > file due to license restrictions (the license allows redistribution, not > modifications). Instead we have the config.guess file which "wraps" > autoconf-config.guess and makes pre-/post-call modifications to work around > limitations in the autoconf original file. So you need to check there if you > are getting incorrect results back and adjust it in that case. See the > already existing clauses in that file. Hello I have updated PR and moved this logic to make/autoconf/build-aux/config.guess It's pretty similar to i386 -> x86_64 fix-up on macos_intel - PR: https://git.openjdk.java.net/jdk/pull/2200
Re: RFR: 8253795: Implementation of JEP 391: macOS/AArch64 Port [v6]
> 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 one additional commit since the last revision: Redo buildsys fix - Changes: - all: https://git.openjdk.java.net/jdk/pull/2200/files - new: https://git.openjdk.java.net/jdk/pull/2200/files/b2b396fc..f1ef6240 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=2200=05 - incr: https://webrevs.openjdk.java.net/?repo=jdk=2200=04-05 Stats: 18 lines in 2 files changed: 8 ins; 10 del; 0 mod Patch: https://git.openjdk.java.net/jdk/pull/2200.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/2200/head:pull/2200 PR: https://git.openjdk.java.net/jdk/pull/2200
Re: RFR: 8253795: Implementation of JEP 391: macOS/AArch64 Port [v5]
On Tue, 26 Jan 2021 19:33:28 GMT, Weijun Wang wrote: >> Anton Kozlov has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Revert harfbuzz changes, disable warnings for it > > src/java.security.jgss/share/native/libj2gss/gssapi.h line 48: > >> 46: // Condition was copied from >> 47: // >> Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX.sdk/usr/include/gssapi/gssapi.h >> 48: #if TARGET_OS_MAC && (defined(__ppc__) || defined(__ppc64__) || >> defined(__i386__) || defined(__x86_64__)) > > So for macOS/AArch64, this `if` is no longer true? That is according to Xcode sdk. - PR: https://git.openjdk.java.net/jdk/pull/2200
Re: RFR: 8259801: Enable XML Signature secure validation mode by default
On Fri, 22 Jan 2021 17:23:12 GMT, Sean Mullan wrote: > This change enables the XML Signature secure validation mode by default. This > will improve out of the box security by restricting signatures that contain > potentially unsafe content by default. > > Please also review the CSR: https://bugs.openjdk.java.net/browse/JDK-8260154 Marked as reviewed by rhalade (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/2197
Re: RFR: 8253795: Implementation of JEP 391: macOS/AArch64 Port [v2]
On Mon, 25 Jan 2021 17:43:35 GMT, Phil Race wrote:
>> Anton Kozlov has updated the pull request incrementally with two additional
>> commits since the last revision:
>>
>> - Address feedback for signature generators
>> - Enable -Wformat-nonliteral back
>
> src/java.desktop/share/native/libharfbuzz/hb-coretext.cc line 193:
>
>> 191:* crbug.com/549610 */
>> 192: // 0x0007 stands for "kCTVersionNumber10_10", see CoreText.h
>> 193: #if TARGET_OS_OSX && MAC_OS_X_VERSION_MIN_REQUIRED < __MAC_10_10
>
> I need a robust explanation of these changes.
> They are not mentioned, nor are they in upstream harfbuzz.
> Likely these should be pushed to upstream first if there is a reason for them.
I'm afraid it's one of those dirty changes that we did to make progress, but
forgot to revisit later. Without the guard you would get:
* For target support_native_java.desktop_libharfbuzz_hb-coretext.o:
if ( != nullptr && CTGetCoreTextVersion() < 0x0007) {
^
uint32_t CTGetCoreTextVersion( void ) CT_DEPRECATED("Use -[NSProcessInfo
operatingSystemVersion]", macos(10.5, 11.0), ios(3.2, 14.0), watchos(2.0, 7.0),
tvos(9.0, 14.0));
^
if ( != nullptr && CTGetCoreTextVersion() < 0x0007) {
^
uint32_t CTGetCoreTextVersion( void ) CT_DEPRECATED("Use -[NSProcessInfo
operatingSystemVersion]", macos(10.5, 11.0), ios(3.2, 14.0), watchos(2.0, 7.0),
tvos(9.0, 14.0));
^
2 errors generated.
For now, it's probably better to go with what Anton did in the latest commit
https://github.com/openjdk/jdk/pull/2200/commits/b2b396fca679fbc7ee78fb5bc80191bc79e1c490
Eventually upstream will do something about it I assume? How often does it get
synced with upstream? Maybe worth to file an issue.
-
PR: https://git.openjdk.java.net/jdk/pull/2200
Re: RFR: 8253795: Implementation of JEP 391: macOS/AArch64 Port [v5]
On Tue, 26 Jan 2021 18:42:02 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 one additional > commit since the last revision: > > Revert harfbuzz changes, disable warnings for it src/java.security.jgss/share/native/libj2gss/gssapi.h line 48: > 46: // Condition was copied from > 47: // > Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX.sdk/usr/include/gssapi/gssapi.h > 48: #if TARGET_OS_MAC && (defined(__ppc__) || defined(__ppc64__) || > defined(__i386__) || defined(__x86_64__)) So for macOS/AArch64, this `if` is no longer true? - PR: https://git.openjdk.java.net/jdk/pull/2200
Re: RFR: 8259801: Enable XML Signature secure validation mode by default
On Fri, 22 Jan 2021 17:23:12 GMT, Sean Mullan wrote: > This change enables the XML Signature secure validation mode by default. This > will improve out of the box security by restricting signatures that contain > potentially unsafe content by default. > > Please also review the CSR: https://bugs.openjdk.java.net/browse/JDK-8260154 Marked as reviewed by weijun (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/2197
Re: RFR: 8259662: Don't wrap SocketExceptions into SSLExceptions in SSLSocketImpl [v6]
> Redo for 8237578: JDK-8214339 (SSLSocketImpl wraps SocketException) appears > to not be fully fixed > > This also fixes JDK-8259516: Alerts sent by peer may not be received > correctly during TLS handshake Clive Verghese has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains four commits: - Update copyright year - Add error handling guidelines - Fix bugids and use server port passed as a parameter - 8259662: Don't wrap SocketExceptions into SSLExceptions in SSLSocketImpl - Changes: https://git.openjdk.java.net/jdk/pull/2057/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=2057=05 Stats: 501 lines in 7 files changed: 490 ins; 0 del; 11 mod Patch: https://git.openjdk.java.net/jdk/pull/2057.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/2057/head:pull/2057 PR: https://git.openjdk.java.net/jdk/pull/2057
Re: RFR: 8253795: Implementation of JEP 391: macOS/AArch64 Port [v2]
On Tue, 26 Jan 2021 16:35:54 GMT, Bernhard Urban-Forster wrote: >> @lewurm This and other harfbuzz changes came from MS, could you please >> comment here ? > > This looks like a merge fix mistake: > https://github.com/openjdk/jdk/commit/051357ef977ecab77fa9b2b1e61f94f288e716f9#diff-e3815f37244d076e27feef0c778fb78a4e5691b47db9c92abcb28bb2a9c2865e I've reverted this change and disabled some warnings for libharfbuzz instead. I should be blamed, yes. Now this is consistent with other changes covered by JDK-8260402 - PR: https://git.openjdk.java.net/jdk/pull/2200
Re: RFR: 8253795: Implementation of JEP 391: macOS/AArch64 Port [v3]
On 2021-01-26 04:44, Magnus Ihse Bursie wrote: On 2021-01-26 13:09, Vladimir Kempik wrote: On Tue, 26 Jan 2021 12:02:02 GMT, Alan Hayward wrote: AIUI, the configure line needs passing a prebuilt JavaNativeFoundation framework ie: `--with-extra-ldflags='-F /Applications/Xcode.app/Contents/SharedFrameworks/ContentDeliveryServices.framework/Versions/A/itms/java/Frameworks/'` Otherwise there will be missing _JNFNative* functions. Is this the long term plan? Or will eventually the required code be moved into JDK and/or the xcode one automatically get picked up by the configure scripts? There is ongoing work by P. Race to eliminate dependence on JNF at all How far has that work come? Otherwise the logic should be added to configure to look for this framework automatically, and provide a way to override it/set it if not found. I don't think it's OK to publish a new port that cannot build out-of-the-box without hacks like this. My understanding is that Apple chose to not provide JNF for aarch64, so if you want to build OpenJDK, you first need to build JNF yourself (it's available in github). Phil is working on removing this dependency completely, which will solve this issue [1]. In the meantime, I don't think we should rely on finding JNF in unsupported locations inside Xcode. Could we wait with integrating this port until it can be built without such hacks? If not, then adding something in the documentation on how to get a working JNF would at least be needed. /Erik [1] https://bugs.openjdk.java.net/browse/JDK-8257852
Re: RFR: 8253795: Implementation of JEP 391: macOS/AArch64 Port [v5]
> 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 one additional commit since the last revision: Revert harfbuzz changes, disable warnings for it - Changes: - all: https://git.openjdk.java.net/jdk/pull/2200/files - new: https://git.openjdk.java.net/jdk/pull/2200/files/fef36580..b2b396fc Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=2200=04 - incr: https://webrevs.openjdk.java.net/?repo=jdk=2200=03-04 Stats: 5 lines in 3 files changed: 1 ins; 2 del; 2 mod Patch: https://git.openjdk.java.net/jdk/pull/2200.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/2200/head:pull/2200 PR: https://git.openjdk.java.net/jdk/pull/2200
Re: RFR: 8217633: Configurable extensions with system properties [v2]
On Mon, 25 Jan 2021 22:27:25 GMT, Rajan Halade wrote: >> Xue-Lei Andrew Fan has updated the pull request incrementally with one >> additional commit since the last revision: >> >> Update copyright years to 2021 > > Marked as reviewed by rhalade (Reviewer). Hi Bernd, I agree with you that System property is not as useful to configure individual connections. It is mostly used for corner cases that have interoperability or compatibility issues. A general program should use APIs and the default system properties. > _Mailing list message from [Bernd Eckenfels](mailto:e...@zusammenkunft.net) > on [security-dev](mailto:security-dev@openjdk.java.net):_ > > Hello, > > I wanted to mention again, that all those System property configurations are > good, especially to resolve the update pains, but not really useful if you > want to make configurations on a per-connection base. If you have to support > multiple partners it can be a real pain to setup a common feature set or > multiple instances. For this a generic feature setter for the context would > be really useful. Most prominent recent example is the ca-extension, which > only really makes sense if you also did programmatically configure a small > list of trusted CAs. > Yes, ca-extension is an item I was thinking of to support in JDK. > I also think it would overall clean up the code and give a good place for > Javadoc all those options. > Not to mention the default could be tied to a few new context names. > Currently, the system properties are documented in the JSSE Reference Guides. But just as you know, it is as easy to follow. I agree with you that it would be nice to have better place to have them all together. Thank you for the review. Regards, Xuelei > Gruss > Bernd > -- > http://bernd.eckenfels.net - PR: https://git.openjdk.java.net/jdk/pull/1752
Re: RFR: 8259662: Don't wrap SocketExceptions into SSLExceptions in SSLSocketImpl [v5]
> Redo for 8237578: JDK-8214339 (SSLSocketImpl wraps SocketException) appears > to not be fully fixed > > This also fixes JDK-8259516: Alerts sent by peer may not be received > correctly during TLS handshake Clive Verghese has updated the pull request incrementally with one additional commit since the last revision: Update copyright year - Changes: - all: https://git.openjdk.java.net/jdk/pull/2057/files - new: https://git.openjdk.java.net/jdk/pull/2057/files/4f7f9e31..983842df Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=2057=04 - incr: https://webrevs.openjdk.java.net/?repo=jdk=2057=03-04 Stats: 3 lines in 3 files changed: 0 ins; 0 del; 3 mod Patch: https://git.openjdk.java.net/jdk/pull/2057.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/2057/head:pull/2057 PR: https://git.openjdk.java.net/jdk/pull/2057
Integrated: 8260286: Manual Test "ws/open/test/jdk/sun/security/tools/jarsigner/compatibility/Compatibility.java" fails
On Mon, 25 Jan 2021 17:08:45 GMT, Fernando Guallini wrote: > Fixing manual Test > "ws/open/test/jdk/sun/security/tools/jarsigner/compatibility/Compatibility.java". > It was not handling "weak algorithm" warning during jarsigner output > verification This pull request has now been integrated. Changeset: 9f0a0436 Author:Fernando Guallini Committer: Rajan Halade URL: https://git.openjdk.java.net/jdk/commit/9f0a0436 Stats: 21 lines in 2 files changed: 17 ins; 0 del; 4 mod 8260286: Manual Test "ws/open/test/jdk/sun/security/tools/jarsigner/compatibility/Compatibility.java" fails Reviewed-by: rhalade - PR: https://git.openjdk.java.net/jdk/pull/2224
Re: RFR: 8260286: Manual Test "ws/open/test/jdk/sun/security/tools/jarsigner/compatibility/Compatibility.java" fails [v2]
On Tue, 26 Jan 2021 09:06:57 GMT, Fernando Guallini wrote: >> Fixing manual Test >> "ws/open/test/jdk/sun/security/tools/jarsigner/compatibility/Compatibility.java". >> It was not handling "weak algorithm" warning during jarsigner output >> verification > > Fernando Guallini has updated the pull request incrementally with one > additional commit since the last revision: > > add bugid and missing space Marked as reviewed by rhalade (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/2224
Re: RFR: 8253795: Implementation of JEP 391: macOS/AArch64 Port [v2]
On Tue, 26 Jan 2021 16:07:19 GMT, Vladimir Kempik wrote: >> src/java.desktop/share/native/libharfbuzz/hb-common.h line 113: >> >>> 111: >>> 112: #define HB_TAG(c1,c2,c3,c4) >>> ((hb_tag_t)uint32_t)(c1)&0xFF)<<24)|(((uint32_t)(c2)&0xFF)<<16)|(((uint32_t)(c3)&0xFF)<<8)|((uint32_t)(c4)&0xFF))) >>> 113: #define HB_UNTAG(tag) (char)(((tag)>>24)&0xFF), >>> (char)(((tag)>>16)&0xFF), (char)(((tag)>>8)&0xFF), (char)((tag)&0xFF) >> >> I need a robust explanation of these changes. >> They are not mentioned, nor are they in upstream harfbuzz. >> Likely these should be pushed to upstream first if there is a reason for >> them. > > @lewurm This and other harfbuzz changes came from MS, could you please > comment here ? This looks like a merge fix mistake: https://github.com/openjdk/jdk/commit/051357ef977ecab77fa9b2b1e61f94f288e716f9#diff-e3815f37244d076e27feef0c778fb78a4e5691b47db9c92abcb28bb2a9c2865e - PR: https://git.openjdk.java.net/jdk/pull/2200
Re: RFR: 8253795: Implementation of JEP 391: macOS/AArch64 Port [v2]
On Mon, 25 Jan 2021 17:43:22 GMT, Phil Race wrote: >> Anton Kozlov has updated the pull request incrementally with two additional >> commits since the last revision: >> >> - Address feedback for signature generators >> - Enable -Wformat-nonliteral back > > src/java.desktop/share/native/libharfbuzz/hb-common.h line 113: > >> 111: >> 112: #define HB_TAG(c1,c2,c3,c4) >> ((hb_tag_t)uint32_t)(c1)&0xFF)<<24)|(((uint32_t)(c2)&0xFF)<<16)|(((uint32_t)(c3)&0xFF)<<8)|((uint32_t)(c4)&0xFF))) >> 113: #define HB_UNTAG(tag) (char)(((tag)>>24)&0xFF), >> (char)(((tag)>>16)&0xFF), (char)(((tag)>>8)&0xFF), (char)((tag)&0xFF) > > I need a robust explanation of these changes. > They are not mentioned, nor are they in upstream harfbuzz. > Likely these should be pushed to upstream first if there is a reason for them. @lewurm This and other harfbuzz changes came from MS, could you please comment here ? - PR: https://git.openjdk.java.net/jdk/pull/2200
Re: RFR: 8253795: Implementation of JEP 391: macOS/AArch64 Port [v4]
On Mon, 25 Jan 2021 10:00:20 GMT, Andrew Haley wrote: >> I like the suggestion. For the defense, new functions were made this way >> intentionally, to match existing `pass_int`, `pass_long`,.. I take your >> comment as a blessing to fix all of them. But I feel that refactoring of >> existing code should go in a separate commit. Especially after `pass_int` >> used `ldr/str` instead of `ldrw/strw`, which looks wrong. >> https://github.com/openjdk/jdk/pull/2200/files#diff-1ff58ce70aeea7e9842d34e8d8fd9c94dd91182999d455618b2a171efd8f742cL87-R122 > > This is new code, and it should not repeat the mistakes of the past. There's > no need to refactor the similar existing code as part of this patch. Makes sense, I've reverted changes in the old code but will propose them in the separate PR shortly. - PR: https://git.openjdk.java.net/jdk/pull/2200
Re: RFR: 8253795: Implementation of JEP 391: macOS/AArch64 Port [v4]
> 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 three additional commits since the last revision: - Little adjustement of SlowSignatureHandler - Partially bring previous commit - Revert "Address feedback for signature generators" This reverts commit 50b55f6684cd21f8b532fa979b7b6fbb4613266d. - Changes: - all: https://git.openjdk.java.net/jdk/pull/2200/files - new: https://git.openjdk.java.net/jdk/pull/2200/files/0c2cb0a3..fef36580 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=2200=03 - incr: https://webrevs.openjdk.java.net/?repo=jdk=2200=02-03 Stats: 98 lines in 1 file changed: 74 ins; 13 del; 11 mod Patch: https://git.openjdk.java.net/jdk/pull/2200.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/2200/head:pull/2200 PR: https://git.openjdk.java.net/jdk/pull/2200
Re: RFR: 8253795: Implementation of JEP 391: macOS/AArch64 Port [v2]
On Mon, 25 Jan 2021 19:12:17 GMT, Coleen Phillimore wrote:
>> I've tried to do something like this initially. The idea was to use Write in
>> VM state and Exec in Java and Native states. However, for example, JIT runs
>> in the Native state and needs Write access. So instead, W^X is managed on
>> entry and exit from VM code, in macros like JRT_ENTRY. Unfortunately, not
>> every JVM entry function is defined with an appropriate macro (at least for
>> now), so I had to add explicit W^X state management along with the explicit
>> java thread state, like here.
>
> Yes, that's why I thought it should be added to the classes
> ThreadInVMfromNative, etc like:
> class ThreadInVMfromNative : public ThreadStateTransition {
> ResetNoHandleMark __rnhm;
>
> We can look at it with cleaning up the thread transitions RFE or as a
> follow-on. If every line of ThreadInVMfromNative has to have one of these
> Thread::WXWriteVerifier __wx_write; people are going to miss them when
> adding the former.
Not every ThreadInVMfromNative needs this, for example JIT goes to Native state
without changing of W^X state. But from some experience of maintaining this
patch, I actually had a duty to add missing W^X transitions after assert
failures. A possible solution is actually to make W^X transition a part of
ThreadInVMfromNative (and similar), but controlled by an optional constructor
parameter with possible values "do exec->write", "verify write"...;. So in a
common case ThreadInVMfromNative would implicitly do the transition and still
would allow something uncommon like verification of the state for the JIT. I
have to think about this.
-
PR: https://git.openjdk.java.net/jdk/pull/2200
Re: RFR: 8253795: Implementation of JEP 391: macOS/AArch64 Port [v3]
On 2021-01-26 13:09, Vladimir Kempik wrote: On Tue, 26 Jan 2021 12:02:02 GMT, Alan Hayward wrote: AIUI, the configure line needs passing a prebuilt JavaNativeFoundation framework ie: `--with-extra-ldflags='-F /Applications/Xcode.app/Contents/SharedFrameworks/ContentDeliveryServices.framework/Versions/A/itms/java/Frameworks/'` Otherwise there will be missing _JNFNative* functions. Is this the long term plan? Or will eventually the required code be moved into JDK and/or the xcode one automatically get picked up by the configure scripts? There is ongoing work by P. Race to eliminate dependence on JNF at all How far has that work come? Otherwise the logic should be added to configure to look for this framework automatically, and provide a way to override it/set it if not found. I don't think it's OK to publish a new port that cannot build out-of-the-box without hacks like this. /Magnus - PR: https://git.openjdk.java.net/jdk/pull/2200
Re: RFR: 8253795: Implementation of JEP 391: macOS/AArch64 Port [v3]
On Tue, 26 Jan 2021 12:06:28 GMT, Vladimir Kempik wrote: > > AIUI, the configure line needs passing a prebuilt JavaNativeFoundation > > framework > > ie: > > `--with-extra-ldflags='-F > > /Applications/Xcode.app/Contents/SharedFrameworks/ContentDeliveryServices.framework/Versions/A/itms/java/Frameworks/'` > > Otherwise there will be missing _JNFNative* functions. > > Is this the long term plan? Or will eventually the required code be moved > > into JDK and/or the xcode one automatically get picked up by the configure > > scripts? > > There is ongoing work by P. Race to eliminate dependence on JNF at all Ok, that's great. In the meantime is it worth adding something to the MacOS section of doc/building.* ? (I pieced together the required line from multiple posts in a mailing list) - PR: https://git.openjdk.java.net/jdk/pull/2200
Re: RFR: 8253795: Implementation of JEP 391: macOS/AArch64 Port [v3]
On Tue, 26 Jan 2021 12:02:02 GMT, Alan Hayward wrote: > AIUI, the configure line needs passing a prebuilt JavaNativeFoundation > framework > ie: > `--with-extra-ldflags='-F > /Applications/Xcode.app/Contents/SharedFrameworks/ContentDeliveryServices.framework/Versions/A/itms/java/Frameworks/'` > > Otherwise there will be missing _JNFNative* functions. > > Is this the long term plan? Or will eventually the required code be moved > into JDK and/or the xcode one automatically get picked up by the configure > scripts? There is ongoing work by P. Race to eliminate dependence on JNF at all - PR: https://git.openjdk.java.net/jdk/pull/2200
Re: RFR: 8253795: Implementation of JEP 391: macOS/AArch64 Port [v3]
On Tue, 26 Jan 2021 09:23:23 GMT, Magnus Ihse Bursie wrote: >> Anton Kozlov has updated the pull request incrementally with two additional >> commits since the last revision: >> >> - Refactor CDS disabling >> - Redo builsys support for aarch64-darwin > > Changes requested by ihse (Reviewer). AIUI, the configure line needs passing a prebuilt JavaNativeFoundation framework ie: ` --with-extra-ldflags='-F /Applications/Xcode.app/Contents/SharedFrameworks/ContentDeliveryServices.framework/Versions/A/itms/java/Frameworks/' ` Otherwise there will be missing _JNFNative* functions. Is this the long term plan? Or will eventually the required code be moved into JDK and/or the xcode one automatically get picked up by the configure scripts? - PR: https://git.openjdk.java.net/jdk/pull/2200
Re: RFR: 8253795: Implementation of JEP 391: macOS/AArch64 Port [v2]
On Tue, 26 Jan 2021 11:31:18 GMT, Anton Kozlov wrote: >> This could be a follow-up RFE. > > I assume a WXVerifier class that tracks W^X mode in debug mode and does > nothing in release mode. I've considered to do this, it's relates to small > inefficiencies, while this patch brings zero overhead (in release) for a > platform that does not need W^X. > * We don't need thread instance in release to call > `os::current_thread_enable_wx`. Having WXVerifier a part of the Thread will > require calling `Thread::current()` first and we could only hope for compiler > to optimize this out, not sure if it will happen at all. In some contexts the > Thread instance is available, in some it's not. > * An instance of the empty class (as WXVerifier will be in the release) will > occupy non-zero space in the Thread instance. > > If such costs are negligible, I can do as suggested. I really just want the minimal number of lines of code and hooks in thread.hpp. You can still access it through the thread, just like these lines currently. Look at HandshakeState as an example. - PR: https://git.openjdk.java.net/jdk/pull/2200
Re: RFR: 8253795: Implementation of JEP 391: macOS/AArch64 Port [v2]
On Mon, 25 Jan 2021 14:40:42 GMT, Coleen Phillimore wrote: >> src/hotspot/share/runtime/thread.hpp line 915: >> >>> 913: verify_wx_state(WXExec); >>> 914: } >>> 915: }; >> >> Rather than add all this to thread.hpp, can you make a wxVerifier.hpp and >> just add the class instance as a field in thread.hpp? > > This could be a follow-up RFE. I assume a WXVerifier class that tracks W^X mode in debug mode and does nothing in release mode. I've considered to do this, it's relates to small inefficiencies, while this patch brings zero overhead (in release) for a platform that does not need W^X. * We don't need thread instance in release to call `os::current_thread_enable_wx`. Having WXVerifier a part of the Thread will require calling `Thread::current()` first and we could only hope for compiler to optimize this out, not sure if it will happen at all. In some contexts the Thread instance is available, in some it's not. * An instance of the empty class (as WXVerifier will be in the release) will occupy non-zero space in the Thread instance. If such costs are negligible, I can do as suggested. - PR: https://git.openjdk.java.net/jdk/pull/2200
Re: RFR: 8260286: Manual Test "ws/open/test/jdk/sun/security/tools/jarsigner/compatibility/Compatibility.java" fails [v2]
On Mon, 25 Jan 2021 22:57:04 GMT, Hai-May Chao wrote: >> Fernando Guallini has updated the pull request incrementally with one >> additional commit since the last revision: >> >> add bugid and missing space > > Change copyright year to 2021 Thanks for the review - PR: https://git.openjdk.java.net/jdk/pull/2224
Re: RFR: 8257733: Move module-specific data from make to respective module [v4]
On Sat, 23 Jan 2021 07:55:09 GMT, Alan Bateman wrote: > We should create a separate issue to rename them and get rid of the copying > in the make file. I opened https://bugs.openjdk.java.net/browse/JDK-8260406. - PR: https://git.openjdk.java.net/jdk/pull/1611
Re: RFR: 8253795: Implementation of JEP 391: macOS/AArch64 Port [v3]
On Mon, 25 Jan 2021 19:38:16 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: > > - Refactor CDS disabling > - Redo builsys support for aarch64-darwin Changes requested by ihse (Reviewer). make/autoconf/build-aux/autoconf-config.guess line 1275: > 1273: UNAME_PROCESSOR="aarch64" > 1274: fi > 1275: fi ;; Almost, but not quite, correct. We cannot change the autoconf-config.guess file due to license restrictions (the license allows redistribution, not modifications). Instead we have the config.guess file which "wraps" autoconf-config.guess and makes pre-/post-call modifications to work around limitations in the autoconf original file. So you need to check there if you are getting incorrect results back and adjust it in that case. See the already existing clauses in that file. - PR: https://git.openjdk.java.net/jdk/pull/2200
Re: RFR: 8260286: Manual Test "ws/open/test/jdk/sun/security/tools/jarsigner/compatibility/Compatibility.java" fails [v2]
> Fixing manual Test > "ws/open/test/jdk/sun/security/tools/jarsigner/compatibility/Compatibility.java". > It was not handling "weak algorithm" warning during jarsigner output > verification Fernando Guallini has updated the pull request incrementally with one additional commit since the last revision: add bugid and missing space - Changes: - all: https://git.openjdk.java.net/jdk/pull/2224/files - new: https://git.openjdk.java.net/jdk/pull/2224/files/e3b011d9..b1162373 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=2224=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk=2224=00-01 Stats: 3 lines in 2 files changed: 0 ins; 0 del; 3 mod Patch: https://git.openjdk.java.net/jdk/pull/2224.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/2224/head:pull/2224 PR: https://git.openjdk.java.net/jdk/pull/2224
Re: RFR: 8253795: Implementation of JEP 391: macOS/AArch64 Port [v3]
On Mon, 25 Jan 2021 19:38:16 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:
>
> - Refactor CDS disabling
> - Redo builsys support for aarch64-darwin
> * 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.
I wonder if this is the right choice. I think it would have been good if this
could have been completely hidden in the transition classes. However, that
doesn't seem to have been enough and now there are explicit
Thread::WXWriteFromExecSetter instances where it's not at all obvious why they
are needed. For example, why was it added here?:
OopStorageParIterPerf::~OopStorageParIterPerf() {
// missing transition to vm state
Thread::WXWriteFromExecSetter wx_write;
_storage.release(_entries, ARRAY_SIZE(_entries));
}
You need to dig down in the OopStorage implementation to find that it's because
it uses SafeFetch, which has the opposite transition:
inline int SafeFetch32(int* adr, int errValue) {
assert(StubRoutines::SafeFetch32_stub(), "stub not yet generated");
Thread::WXExecFromWriteSetter wx_exec;
return StubRoutines::SafeFetch32_stub()(adr, errValue);
}
I think this adds complexity to code that shouldn't have to deal with this. I'm
fine with having to force devs / code that writes to exec memory to have to
deal with a bit more complexity, but it seems wrong to let this leak out to
code that is staying far away from that.
For my understanding, was this choice made because the nmethods instances (and
maybe other types as well) are placed in the code cache memory segment, which
is executable? If the code cache only contained the JIT:ed code, and not object
instances, I think this could more easily have been contained a bit more.
If the proposed solution is going to stay, maybe this could be made a little
bit nicer by changing the SafeFetch implementation to use a new scoped object
that doesn't enforce the "from" state to be "write"? Instead it could check if
the state is "write" and only then flip the state back and forth. I think, this
would remove the change needed OopStorageParIterPerf, and probably other places
as well.
src/hotspot/os/linux/gc/z/zPhysicalMemoryBacking_linux.cpp line 38:
> 36: #include "runtime/os.hpp"
> 37: #include "runtime/stubRoutines.hpp"
> 38: #include "runtime/stubRoutines.inline.hpp"
Remove stubRutines.hpp line
src/hotspot/share/runtime/stubRoutines.inline.hpp line 29:
> 27:
> 28: #include
> 29: #include
Replace < > with " "
src/hotspot/os/aix/os_aix.cpp line 70:
> 68: #include "runtime/statSampler.hpp"
> 69: #include "runtime/stubRoutines.hpp"
> 70: #include "runtime/stubRoutines.inline.hpp"
Remove stubRutines.hpp line
-
Changes requested by stefank (Reviewer).
PR: https://git.openjdk.java.net/jdk/pull/2200
