Re: RFR: 8253795: Implementation of JEP 391: macOS/AArch64 Port [v3]
On Mon, 25 Jan 2021 23:35:52 GMT, Phil Race wrote: >> That may actually be a valid concern. Both say macOS 10.12+ ... which might >> conflict with the 10.9 target. > > Maybe you should just file a bug after all for this to be dealt with > separately. > Certainly if it is NOT fixed now such a bug needs to be filed. I have created https://bugs.openjdk.java.net/browse/JDK-8260402 which is blocked by jep-391 currently - PR: https://git.openjdk.java.net/jdk/pull/2200
Re: RFR: 8217633: Configurable extensions with system properties [v2]
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. 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. Gruss Bernd -- http://bernd.eckenfels.net Von: security-dev im Auftrag von Xue-Lei Andrew Fan Gesendet: Monday, January 25, 2021 11:17:56 PM An: security-dev@openjdk.java.net Betreff: Re: RFR: 8217633: Configurable extensions with system properties [v2] > The TLS protocols are designed to tolerant unknown TLS extensions. However, > although it is not common, there are a few TLS implementations that cannot > handle unknown extensions properly. As results in unexpected interoperability > issue when new extensions are introduced in JDK. The interoperability impact > could be mitigated If applications can customize the extensions if needed. > > With this update, two system properties are added to configure the default > extensions in either client or server side of TLS connections. Please note > that the impact of blocking TLS extensions is complicated. For example, a > TLS connection may not be able to established if a mandatory extension is > blocked. Please don't use this feature unless you clearly understand the > impact. > > Bug: https://bugs.openjdk.java.net/browse/JDK-8217633 > CSR: https://bugs.openjdk.java.net/browse/JDK-8217993 Xue-Lei Andrew Fan has updated the pull request incrementally with one additional commit since the last revision: Update copyright years to 2021 - Changes: - all: https://git.openjdk.java.net/jdk/pull/1752/files - new: https://git.openjdk.java.net/jdk/pull/1752/files/ad5f3330..5bd6e865 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=1752=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk=1752=00-01 Stats: 4 lines in 2 files changed: 0 ins; 0 del; 4 mod Patch: https://git.openjdk.java.net/jdk/pull/1752.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/1752/head:pull/1752 PR: https://git.openjdk.java.net/jdk/pull/1752
Re: RFR: 8253795: Implementation of JEP 391: macOS/AArch64 Port [v3]
On Mon, 25 Jan 2021 23:34:04 GMT, Phil Race wrote: >> that sounds good to me, I am just afraid to break intel mac on older macos >> versions with this change. > > That may actually be a valid concern. Both say macOS 10.12+ ... which might > conflict with the 10.9 target. Maybe you should just file a bug after all for this to be dealt with separately. Certainly if it is NOT fixed now such a bug needs to be filed. - PR: https://git.openjdk.java.net/jdk/pull/2200
Re: RFR: 8253795: Implementation of JEP 391: macOS/AArch64 Port [v3]
On Mon, 25 Jan 2021 22:47:33 GMT, Vladimir Kempik wrote: >> 1) I meant change to NSWindowStyleMaskBorderless from NSBorderlessWindowMask >> 2) So maybe rather than the deprecation suppression you could change both >> constants to the new ones. >> Ordinarily I'd say let someone else do that but this looks like a simple >> obvious change and is dwarfed by all the other changes going on for Mac ARM >> ... > > that sounds good to me, I am just afraid to break intel mac on older macos > versions with this change. That may actually be a valid concern. Both say macOS 10.12+ ... which might conflict with the 10.9 target. - PR: https://git.openjdk.java.net/jdk/pull/2200
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 JDI changes also look good. - Marked as reviewed by cjplummer (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/2200
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 I looked through the SA changes. Overall looks good except for a couple of minor issues noted. For the most part it seems to have been boilerplate copy-n-paste from existing ports. If there is anything you want me to take a closer look at, let me know. src/jdk.hotspot.agent/macosx/native/libsaproc/MacosxDebuggerLocal.m line 702: > 700: primitiveArray = (*env)->GetLongArrayElements(env, registerArray, > NULL); > 701: > 702: #undef REG_INDEX I'm not so sure why the #undef and subsequent #define of REG_INDEX is needed since it seems to just get #define'd back to the same value. src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/debugger/bsd/aarch64/BsdAARCH64ThreadContext.java line 2: > 1: /* > 2: * Copyright (c) 2003, Oracle and/or its affiliates. All rights reserved. Update copyright. - Marked as reviewed by cjplummer (Reviewer). 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
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 Change copyright year to 2021 - 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
On Mon, 25 Jan 2021 22:45:31 GMT, Hai-May Chao wrote: >> Marked as reviewed by rhalade (Reviewer). > > Looks good. One comment: Add bug id to the changed test. Thanks. - 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 22:42:40 GMT, Phil Race wrote: >> Min_macos version is changed to 11.0 for macos_aarch64 >> >> https://github.com/openjdk/jdk/pull/2200/files/0c2cb0a372bf1a8607810d773b53d6959616a816#diff-7cd97cdbeb3053597e5d6659016cdf0f60b2c412bd39934a43817ee0b717b7a7R136 > > 1) I meant change to NSWindowStyleMaskBorderless from NSBorderlessWindowMask > 2) So maybe rather than the deprecation suppression you could change both > constants to the new ones. > Ordinarily I'd say let someone else do that but this looks like a simple > obvious change and is dwarfed by all the other changes going on for Mac ARM > ... that sounds good to me, I am just afraid to break intel mac on older macos versions with this change. - 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
On Mon, 25 Jan 2021 21:51:19 GMT, Rajan Halade 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 > > Marked as reviewed by rhalade (Reviewer). Looks good. - 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 22:25:48 GMT, Vladimir Kempik wrote: >> Are you doing something somewhere to change the target version of macOS or >> SDK ? I had no such problem. >> I think we currently target a macOS 10.9 and if you are changing that it >> would need discussion. >> If you are changing it only for Mac ARM that may make more sense .. >> >> And these appear to be just API churn by Apple >> NSAlphaFirstBitmapFormat is replaced by NSBitmapFormatAlphaFirst >> >> https://developer.apple.com/documentation/appkit/nsbitmapformat/nsbitmapformatalphafirst?language=objc >> >> NSBorderlessWindowMask is replaced by NSWindowStyleMask >> >> https://developer.apple.com/documentation/appkit/nswindowstylemask?language=occ > > Min_macos version is changed to 11.0 for macos_aarch64 > > https://github.com/openjdk/jdk/pull/2200/files/0c2cb0a372bf1a8607810d773b53d6959616a816#diff-7cd97cdbeb3053597e5d6659016cdf0f60b2c412bd39934a43817ee0b717b7a7R136 1) I meant change to NSWindowStyleMaskBorderless from NSBorderlessWindowMask 2) So maybe rather than the deprecation suppression you could change both constants to the new ones. Ordinarily I'd say let someone else do that but this looks like a simple obvious change and is dwarfed by all the other changes going on for Mac ARM ... - PR: https://git.openjdk.java.net/jdk/pull/2200
Re: RFR: 8217633: Configurable extensions with system properties [v2]
On Mon, 25 Jan 2021 22:17:56 GMT, Xue-Lei Andrew Fan wrote: >> The TLS protocols are designed to tolerant unknown TLS extensions. However, >> although it is not common, there are a few TLS implementations that cannot >> handle unknown extensions properly. As results in unexpected >> interoperability issue when new extensions are introduced in JDK. The >> interoperability impact could be mitigated If applications can customize the >> extensions if needed. >> >> With this update, two system properties are added to configure the default >> extensions in either client or server side of TLS connections. Please note >> that the impact of blocking TLS extensions is complicated. For example, a >> TLS connection may not be able to established if a mandatory extension is >> blocked. Please don't use this feature unless you clearly understand the >> impact. >> >> Bug: https://bugs.openjdk.java.net/browse/JDK-8217633 >> CSR: https://bugs.openjdk.java.net/browse/JDK-8217993 > > 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). - PR: https://git.openjdk.java.net/jdk/pull/1752
Re: RFR: 8253795: Implementation of JEP 391: macOS/AArch64 Port [v3]
On Mon, 25 Jan 2021 22:22:06 GMT, Phil Race wrote: >> It seems these workarounds are still needed: >> >> jdk/src/java.desktop/macosx/native/libsplashscreen/splashscreen_sys.m:300:39: >> error: 'NSAlphaFirstBitmapFormat' is deprecated: first deprecated in macOS >> 10.14 [-Werror,-Wdeprecated-declarations] >> bitmapFormat: NSAlphaFirstBitmapFormat | >> NSAlphaNonpremultipliedBitmapFormat >> ^~~~ >> NSBitmapFormatAlphaFirst >> >> jdk/src/java.desktop/macosx/native/libsplashscreen/splashscreen_sys.m:438:34: >> error: 'NSBorderlessWindowMask' is deprecated: first deprecated in macOS >> 10.12 [-Werror,-Wdeprecated-declarations] >> styleMask: NSBorderlessWindowMask >> ^~ >> NSWindowStyleMaskBorderless > > Are you doing something somewhere to change the target version of macOS or > SDK ? I had no such problem. > I think we currently target a macOS 10.9 and if you are changing that it > would need discussion. > If you are changing it only for Mac ARM that may make more sense .. > > And these appear to be just API churn by Apple > NSAlphaFirstBitmapFormat is replaced by NSBitmapFormatAlphaFirst > > https://developer.apple.com/documentation/appkit/nsbitmapformat/nsbitmapformatalphafirst?language=objc > > NSBorderlessWindowMask is replaced by NSWindowStyleMask > > https://developer.apple.com/documentation/appkit/nswindowstylemask?language=occ Min_macos version is changed to 11.0 for macos_aarch64 https://github.com/openjdk/jdk/pull/2200/files/0c2cb0a372bf1a8607810d773b53d6959616a816#diff-7cd97cdbeb3053597e5d6659016cdf0f60b2c412bd39934a43817ee0b717b7a7R136 - PR: https://git.openjdk.java.net/jdk/pull/2200
Re: RFR: 8253795: Implementation of JEP 391: macOS/AArch64 Port [v3]
On Mon, 25 Jan 2021 21:18:59 GMT, Vladimir Kempik wrote: >> Hello >> I believe it was a workaround for issues with xcode 12.2 in early beta days. >> Those issues were fixed later in upstream jdk, so most likely we need to >> remove these workarounds. > > It seems these workarounds are still needed: > > jdk/src/java.desktop/macosx/native/libsplashscreen/splashscreen_sys.m:300:39: > error: 'NSAlphaFirstBitmapFormat' is deprecated: first deprecated in macOS > 10.14 [-Werror,-Wdeprecated-declarations] > bitmapFormat: NSAlphaFirstBitmapFormat | > NSAlphaNonpremultipliedBitmapFormat > ^~~~ > NSBitmapFormatAlphaFirst > > jdk/src/java.desktop/macosx/native/libsplashscreen/splashscreen_sys.m:438:34: > error: 'NSBorderlessWindowMask' is deprecated: first deprecated in macOS > 10.12 [-Werror,-Wdeprecated-declarations] > styleMask: NSBorderlessWindowMask > ^~ > NSWindowStyleMaskBorderless Are you doing something somewhere to change the target version of macOS or SDK ? I had no such problem. I think we currently target a macOS 10.9 and if you are changing that it would need discussion. If you are changing it only for Mac ARM that may make more sense .. And these appear to be just API churn by Apple NSAlphaFirstBitmapFormat is replaced by NSBitmapFormatAlphaFirst https://developer.apple.com/documentation/appkit/nsbitmapformat/nsbitmapformatalphafirst?language=objc NSBorderlessWindowMask is replaced by NSWindowStyleMask https://developer.apple.com/documentation/appkit/nswindowstylemask?language=occ - PR: https://git.openjdk.java.net/jdk/pull/2200
Re: RFR: 8217633: Configurable extensions with system properties [v2]
On Mon, 25 Jan 2021 22:17:56 GMT, Xue-Lei Andrew Fan wrote: >> The TLS protocols are designed to tolerant unknown TLS extensions. However, >> although it is not common, there are a few TLS implementations that cannot >> handle unknown extensions properly. As results in unexpected >> interoperability issue when new extensions are introduced in JDK. The >> interoperability impact could be mitigated If applications can customize the >> extensions if needed. >> >> With this update, two system properties are added to configure the default >> extensions in either client or server side of TLS connections. Please note >> that the impact of blocking TLS extensions is complicated. For example, a >> TLS connection may not be able to established if a mandatory extension is >> blocked. Please don't use this feature unless you clearly understand the >> impact. >> >> Bug: https://bugs.openjdk.java.net/browse/JDK-8217633 >> CSR: https://bugs.openjdk.java.net/browse/JDK-8217993 > > Xue-Lei Andrew Fan has updated the pull request incrementally with one > additional commit since the last revision: > > Update copyright years to 2021 Looks good to me. - Marked as reviewed by jnimeh (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/1752
Re: RFR: 8217633: Configurable extensions with system properties [v2]
> The TLS protocols are designed to tolerant unknown TLS extensions. However, > although it is not common, there are a few TLS implementations that cannot > handle unknown extensions properly. As results in unexpected interoperability > issue when new extensions are introduced in JDK. The interoperability impact > could be mitigated If applications can customize the extensions if needed. > > With this update, two system properties are added to configure the default > extensions in either client or server side of TLS connections. Please note > that the impact of blocking TLS extensions is complicated. For example, a > TLS connection may not be able to established if a mandatory extension is > blocked. Please don't use this feature unless you clearly understand the > impact. > > Bug: https://bugs.openjdk.java.net/browse/JDK-8217633 > CSR: https://bugs.openjdk.java.net/browse/JDK-8217993 Xue-Lei Andrew Fan has updated the pull request incrementally with one additional commit since the last revision: Update copyright years to 2021 - Changes: - all: https://git.openjdk.java.net/jdk/pull/1752/files - new: https://git.openjdk.java.net/jdk/pull/1752/files/ad5f3330..5bd6e865 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=1752=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk=1752=00-01 Stats: 4 lines in 2 files changed: 0 ins; 0 del; 4 mod Patch: https://git.openjdk.java.net/jdk/pull/1752.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/1752/head:pull/1752 PR: https://git.openjdk.java.net/jdk/pull/1752
Re: RFR: 8217633: Configurable extensions with system properties [v2]
On Mon, 25 Jan 2021 21:32:18 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 > > test/jdk/sun/security/ssl/SSLSocketImpl/BlockedExtension.java line 70: > >> 68: >> 69: if (!shouldSuccess) { >> 70: throw new Exception( > > Suggestion: > > throw new RuntimeException( Good catches, the years should be 2021 now. Thanks! - PR: https://git.openjdk.java.net/jdk/pull/1752
Re: RFR: 8253795: Implementation of JEP 391: macOS/AArch64 Port [v3]
On Mon, 25 Jan 2021 19:42:41 GMT, Phil Race 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/modules/java.desktop/lib/Awt2dLibraries.gmk line 573: > >> 571: EXTRA_HEADER_DIRS := $(LIBFONTMANAGER_EXTRA_HEADER_DIRS), \ >> 572: WARNINGS_AS_ERRORS_xlc := false, \ >> 573: DISABLED_WARNINGS_clang := deprecated-declarations, \ > > I see this added here and in another location. What is deprecated ? You > really need to explain these sorts of things. > I've built JDK using Xcode 12.3 and not had any need for this. Hello I believe it was a workaround for issues with xcode 12.2 in early beta days. Those issues were fixed later in upstream jdk, so most likely we need to remove these workarounds. - PR: https://git.openjdk.java.net/jdk/pull/2200
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 make/modules/java.desktop/lib/Awt2dLibraries.gmk line 573: > 571: EXTRA_HEADER_DIRS := $(LIBFONTMANAGER_EXTRA_HEADER_DIRS), \ > 572: WARNINGS_AS_ERRORS_xlc := false, \ > 573: DISABLED_WARNINGS_clang := deprecated-declarations, \ I see this added here and in another location. What is deprecated ? You really need to explain these sorts of things. I've built JDK using Xcode 12.3 and not had any need for this. - PR: https://git.openjdk.java.net/jdk/pull/2200
Re: RFR: 8253795: Implementation of JEP 391: macOS/AArch64 Port [v3]
> 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: - all: https://git.openjdk.java.net/jdk/pull/2200/files - new: https://git.openjdk.java.net/jdk/pull/2200/files/50b55f66..0c2cb0a3 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=2200=02 - incr: https://webrevs.openjdk.java.net/?repo=jdk=2200=01-02 Stats: 36 lines in 3 files changed: 12 ins; 15 del; 9 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 15:01:25 GMT, Anton Kozlov wrote: >> src/hotspot/share/jfr/instrumentation/jfrJvmtiAgent.cpp line 87: >> >>> 85: JavaThread* jt = JavaThread::thread_from_jni_environment(jni_env); >>> 86: DEBUG_ONLY(JfrJavaSupport::check_java_thread_in_native(jt));; >>> 87: Thread::WXWriteFromExecSetter wx_write; >> >> Is this on every transition to VM from Native? Would it be better to add to >> ThreadInVMfromNative like the ResetNoHandleMark is? > > 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. - PR: https://git.openjdk.java.net/jdk/pull/2200
Integrated: 8258833: Cancel multi-part cipher operations in SunPKCS11 after failures
On Mon, 28 Dec 2020 16:24:43 GMT, Martin Balao wrote: > When a multi-part cipher operation fails in SunPKCS11 (i.e. because of an > invalid block size), we now cancel the operation before returning the > underlying Session to the Session Manager. This allows to use the returned > Session for a different purpose. Otherwise, an CKR_OPERATION_ACTIVE error > would be raised from the PKCS#11 library. > > The jdk/sun/security/pkcs11/Cipher/CancelMultipart.java regression test is > introduced as part of this PR. > > No regressions found in jdk/sun/security/pkcs11. This pull request has now been integrated. Changeset: 47c7dc77 Author:Martin Balao URL: https://git.openjdk.java.net/jdk/commit/47c7dc77 Stats: 301 lines in 6 files changed: 286 ins; 8 del; 7 mod 8258833: Cancel multi-part cipher operations in SunPKCS11 after failures Reviewed-by: valeriep - PR: https://git.openjdk.java.net/jdk/pull/1901
Re: RFR: 8253795: Implementation of JEP 391: macOS/AArch64 Port [v2]
On Mon, 25 Jan 2021 09:52:00 GMT, Andrew Haley wrote: >> Hello >> Why is it not nice ? >> linux_aarch64 uses some linux specific tls function >> _ZN10JavaThread25aarch64_get_thread_helperEv from >> hotspot/os_cpu/linux_aarch64/threadLS_linux_aarch64.s >> which clobbers only r0 and r1 >> macos_aarch64 has no such tls code and uses generic C-call to >> Thread::current(); >> Hence we are saving possibly clobbered regs here. > > Surely if you did as Linux does you wouldn't need to clobber all those > registers. I see how this can be done, from looking at C example with `__thread`, which involves poorly documented relocations and private libc interface invocation. But now I also wonder how much benefit would we have from this optimization. `get_thread` is called from few places and all of them are guarded by `#ifdef ASSERT`. The release build is still fine after I removed MacroAssembler::get_thread definition (as a verification). Callers of get_thread: * StubAssembler::call_RT * Runtime1::generate_patching * StubGenerator::generate_call_stub * StubGenerator::generate_catch_exception All of them are heavy-weight functions, nonoptimal get_thread is unlikely to slow them down even in fastdebug. - PR: https://git.openjdk.java.net/jdk/pull/2200
Re: RFR: 8253795: Implementation of JEP 391: macOS/AArch64 Port [v2]
On Sun, 24 Jan 2021 15:32:59 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: > > - Address feedback for signature generators > - Enable -Wformat-nonliteral back Changes requested by prr (Reviewer). 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. 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. - PR: https://git.openjdk.java.net/jdk/pull/2200
Re: RFR: 8253795: Implementation of JEP 391: macOS/AArch64 Port [v2]
On Sun, 24 Jan 2021 15:50:01 GMT, Anton Kozlov wrote: >> src/hotspot/cpu/aarch64/interpreterRT_aarch64.cpp line 86: >> >>> 84: >>> 85: switch (_num_int_args) { >>> 86: case 0: >> >> I don't think you need such a large switch statement. I think this can be >> expressed as >> if (num_int_args <= 6) { >> ldr(as_Register(num_int_args + r1.encoding()), src); >> ... etc. > > 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. - PR: https://git.openjdk.java.net/jdk/pull/2200
RFR: 8260286: Manual Test "ws/open/test/jdk/sun/security/tools/jarsigner/compatibility/Compatibility.java" fails
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 - Commit messages: - rename method - handle warning for SHA-1 weak algorithm Changes: https://git.openjdk.java.net/jdk/pull/2224/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=2224=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8260286 Stats: 19 lines in 2 files changed: 17 ins; 0 del; 2 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 [v2]
On Mon, 25 Jan 2021 13:30:55 GMT, Vladimir Kempik wrote: >> make/modules/jdk.hotspot.agent/Lib.gmk line 34: >> >>> 32: >>> 33: else ifeq ($(call isTargetOs, macosx), true) >>> 34: SA_CFLAGS := -D_GNU_SOURCE -mno-omit-leaf-frame-pointer \ >> >> Is this really proper for macos-x64? I thought we used -Damd64 as a marker >> for all macos-x64 C/C++ files. (Most files get their flags from the common >> setup in configure, but SA is a different beast due to historical reasons). > > @luhenry , could you please check this comment, I think SA-agent was MS's job > here. The target is identified by the header file now: https://github.com/openjdk/jdk/pull/2200/files#diff-51442e74eeef2163f0f0643df0ae995083f666367e25fba2b527a9a5bc8743a6R35-R41 Do you think there is any problem with this approach? - 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:36:35 GMT, Coleen Phillimore 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/hotspot/share/jfr/instrumentation/jfrJvmtiAgent.cpp line 87: > >> 85: JavaThread* jt = JavaThread::thread_from_jni_environment(jni_env); >> 86: DEBUG_ONLY(JfrJavaSupport::check_java_thread_in_native(jt));; >> 87: Thread::WXWriteFromExecSetter wx_write; > > Is this on every transition to VM from Native? Would it be better to add to > ThreadInVMfromNative like the ResetNoHandleMark is? 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. - 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:09 GMT, Coleen Phillimore 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/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. - PR: https://git.openjdk.java.net/jdk/pull/2200
Re: RFR: 8253795: Implementation of JEP 391: macOS/AArch64 Port [v2]
On Sun, 24 Jan 2021 15:32:59 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: > > - Address feedback for signature generators > - Enable -Wformat-nonliteral back 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? - PR: https://git.openjdk.java.net/jdk/pull/2200
Re: RFR: 8253795: Implementation of JEP 391: macOS/AArch64 Port [v2]
On Sun, 24 Jan 2021 15:32:59 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: > > - Address feedback for signature generators > - Enable -Wformat-nonliteral back src/hotspot/share/jfr/instrumentation/jfrJvmtiAgent.cpp line 87: > 85: JavaThread* jt = JavaThread::thread_from_jni_environment(jni_env); > 86: DEBUG_ONLY(JfrJavaSupport::check_java_thread_in_native(jt));; > 87: Thread::WXWriteFromExecSetter wx_write; Is this on every transition to VM from Native? Would it be better to add to ThreadInVMfromNative like the ResetNoHandleMark is? - 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 13:23:27 GMT, Magnus Ihse Bursie 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 > > Changes requested by ihse (Reviewer). In `make/autoconf/jvm-features.m4` I notice that you haven't enabled ZGC for macos/aarch64. Is that just an oversight, or is there a reason for that? - 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:03:40 GMT, Per Liden wrote: > In `make/autoconf/jvm-features.m4` I notice that you haven't enabled ZGC for > macos/aarch64. Is that just an oversight, or is there a reason for that? because it does not work processor_id has no "official docs"-friendly implementation, only a hacky one from ios world. Also ZGC needs additional W^X work. I believe zgc supports needs to be done in a separate commit. - 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 13:18:34 GMT, Magnus Ihse Bursie 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 > > make/common/NativeCompilation.gmk line 1180: > >> 1178: # silently fail otherwise. >> 1179: ifneq ($(CODESIGN), ) >> 1180: $(CODESIGN) -f -s "$(MACOSX_CODESIGN_IDENTITY)" >> --timestamp --options runtime \ > > What does -f do, and is it needed for macos-x64 as well? -f - replace signature if it's present, if not - just sign as usual macos-aarch64 binaries always have adhoc signature, so need to replace it. > make/modules/jdk.hotspot.agent/Lib.gmk line 34: > >> 32: >> 33: else ifeq ($(call isTargetOs, macosx), true) >> 34: SA_CFLAGS := -D_GNU_SOURCE -mno-omit-leaf-frame-pointer \ > > Is this really proper for macos-x64? I thought we used -Damd64 as a marker > for all macos-x64 C/C++ files. (Most files get their flags from the common > setup in configure, but SA is a different beast due to historical reasons). @luhenry , could you please check this comment, I think SA-agent was MS's job here. - PR: https://git.openjdk.java.net/jdk/pull/2200
Re: RFR: 8253795: Implementation of JEP 391: macOS/AArch64 Port [v2]
On Sun, 24 Jan 2021 15:32:59 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: > > - Address feedback for signature generators > - Enable -Wformat-nonliteral back Changes requested by ihse (Reviewer). make/autoconf/jvm-features.m4 line 269: > 267: if test "x$OPENJDK_TARGET_OS" != xaix && \ > 268: !( test "x$OPENJDK_TARGET_OS" = "xmacosx" && \ > 269: test "x$OPENJDK_TARGET_CPU" = "xaarch64" ) ; then This test is making my head spin. Can't you just invert it to this structure: if OS=aix || OS+CPU = mac+aarch64; then no else yes fi make/autoconf/platform.m4 line 75: > 73: ;; > 74: esac > 75: ;; This is solved in the wrong place. This code should just use the result from `config.guess/config.sub`. These files are imported from the autoconf project. Unfortunately, they have changed the license to one incompatible with OpenJDK, so we are stuck with an old version. Instead, we have created a "bugfix wrapper" that calls the original `autoconf-config.guess/sub` and fixes up the result, with stuff like this. make/autoconf/platform.m4 line 273: > 271: # Convert the autoconf OS/CPU value to our own data, into the > VAR_OS/CPU/LIBC variables. > 272: PLATFORM_EXTRACT_VARS_FROM_OS($build_os) > 273: PLATFORM_EXTRACT_VARS_FROM_CPU($build_cpu, $build_os) Fixing the comment above means this change, and the one below, can be reverted. make/common/NativeCompilation.gmk line 1180: > 1178: # silently fail otherwise. > 1179: ifneq ($(CODESIGN), ) > 1180: $(CODESIGN) -f -s "$(MACOSX_CODESIGN_IDENTITY)" > --timestamp --options runtime \ What does -f do, and is it needed for macos-x64 as well? make/modules/jdk.hotspot.agent/Lib.gmk line 34: > 32: > 33: else ifeq ($(call isTargetOs, macosx), true) > 34: SA_CFLAGS := -D_GNU_SOURCE -mno-omit-leaf-frame-pointer \ Is this really proper for macos-x64? I thought we used -Damd64 as a marker for all macos-x64 C/C++ files. (Most files get their flags from the common setup in configure, but SA is a different beast due to historical reasons). - PR: https://git.openjdk.java.net/jdk/pull/2200
Re: RFR: 8258915: Temporary buffer cleanup [v3]
On Sat, 23 Jan 2021 16:04:53 GMT, Weijun Wang wrote: >> src/java.base/share/classes/sun/security/pkcs/PKCS8Key.java line 221: >> >>> 219: if (encodedKey == null) { >>> 220: try { >>> 221: DerOutputStream tmp = new DerOutputStream(); >> >> What is the criteria of using the default constructor vs the one with a >> initial size? Here is using the default, are we sure about the key (line 224 >> below) will always fit? > > Here the key is the last thing to be written into the DerOutputStream, so > there will be no more reallocation after and its content will not be leaked. I see. Interesting... - PR: https://git.openjdk.java.net/jdk/pull/2070
Re: RFR: 8258915: Temporary buffer cleanup [v4]
On Sat, 23 Jan 2021 16:32:16 GMT, Weijun Wang wrote: >> I'll take a look. The test does not show it. Maybe because of the reversing? > > I found out the reason. This method is called during key pair generation but > my test only deals with manually crafted keys (so that I know what special > bytes to search for). I think I'll need to find out a different test method. > This might reveal other leaks in key generation. Yes, your approach is based on the test and the code path it exercised. I manually code inspected the files that you touched. Even with both approaches, it's still not gonna be 100%. - PR: https://git.openjdk.java.net/jdk/pull/2070
Re: RFR: 8258915: Temporary buffer cleanup [v4]
On Sat, 23 Jan 2021 16:11:02 GMT, Weijun Wang wrote: >> src/java.base/share/classes/com/sun/crypto/provider/DHPrivateKey.java line >> 116: >> >>> 114: encode(); >>> 115: } catch (IOException e) { >>> 116: throw new ProviderException("Cannot produce ASN.1 >>> encoding", e); >> >> Supposedly the IOException should never happen? Otherwise the >> Arrays.fill(...) call may not happen. Some throws AssertionError wrapping >> the IOException, just checking to see this is also the case. > > No, it should never happen. `DerValue::toByteArray` claims it might throw an > IOE because `DerOutputStream::putLength` claims so, but you can see the > latter never throws one. I thought about removing those `throws IOE` but > there are too many. We can do it in another code change. Ok. - PR: https://git.openjdk.java.net/jdk/pull/2070
Re: RFR: 8253795: Implementation of JEP 391: macOS/AArch64 Port [v2]
On Sun, 24 Jan 2021 16:10:44 GMT, Anton Kozlov wrote: >> src/hotspot/cpu/aarch64/interpreterRT_aarch64.cpp line 394: >> >>> 392: >>> 393: class SlowSignatureHandler >>> 394: : public NativeSignatureIterator { >> >> SlowSignatureHandler is turning into a maintenance nightmare. This isn't the >> fault of this commit so much as some nasty cut-and-paste programming in the >> code you're editing. It might well be better to rewrite this whole thing to >> use a table-driven approach, with a table that contains the increment and >> the size of each native type: then we'd have only a single routine which >> could pass data of any type, and each OS needs only supply a table of >> constants. > > Would you like me to do something about it now? The problem is that the > functions of SlowSignatureHandler are subtly different, so it will be > multiple tables, not sure how many. Such change is another candidate for a > separate code enhancement, which I would like not to mix into the JEP > implementation (it's already not a small change :)). But if you think it > would be a good thing, please let me know. In another case, I'll add this to > a queue of follow-up enhancements. I'm not sure it can wait. This change turns already-messy code into something significantly messier, to the extent that it's not really good enough to go into mainline. - PR: https://git.openjdk.java.net/jdk/pull/2200
Re: RFR: 8253795: Implementation of JEP 391: macOS/AArch64 Port [v2]
On Sun, 24 Jan 2021 16:29:31 GMT, Vladimir Kempik wrote: >> src/hotspot/cpu/aarch64/macroAssembler_aarch64.cpp line 5272: >> >>> 5270: void MacroAssembler::get_thread(Register dst) { >>> 5271: RegSet saved_regs = RegSet::range(r0, r1) + >>> BSD_ONLY(RegSet::range(r2, r17)) + lr - dst; >>> 5272: push(saved_regs, sp); >> >> This isn't very nice. > > Hello > Why is it not nice ? > linux_aarch64 uses some linux specific tls function > _ZN10JavaThread25aarch64_get_thread_helperEv from > hotspot/os_cpu/linux_aarch64/threadLS_linux_aarch64.s > which clobbers only r0 and r1 > macos_aarch64 has no such tls code and uses generic C-call to > Thread::current(); > Hence we are saving possibly clobbered regs here. Surely if you did as Linux does you wouldn't need to clobber all those registers. - PR: https://git.openjdk.java.net/jdk/pull/2200