Re: RFR: 8275063: Implementation of Foreign Function & Memory API (Second incubator) [v5]
On Fri, 15 Oct 2021 16:54:58 GMT, Maurizio Cimadamore wrote: >> 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 incrementally with one > additional commit since the last revision: > > * Fix javadoc issue in VaList > * Fix bug in concurrent logic for shared scope acquire src/jdk.incubator.foreign/share/classes/jdk/internal/foreign/SharedScope.java line 138: > 136: ResourceCleanup prev = (ResourceCleanup) > FST.getAcquire(this); > 137: cleanup.next = prev; > 138: ResourceCleanup newSegment = (ResourceCleanup) > FST.compareAndExchangeRelease(this, prev, cleanup); In this place we can actually overwrite CLOSED_LIST (if getAcquired has seen such value). While this particular add will fail later on (since we check witness), another concurrent add might then pass, since it sees a value other than CLOSED_LIST (which is supposed to be a terminal state). Also, after consulting with @DougLea and @shipilev - it seems like using volatile semantics is the way to go here. This should fix a spurious (and very rare) failure on TestResourceScope on arm64. - PR: https://git.openjdk.java.net/jdk/pull/5907
Re: RFR: 8275063: Implementation of Foreign Function & Memory API (Second incubator) [v5]
> 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 incrementally with one additional commit since the last revision: * Fix javadoc issue in VaList * Fix bug in concurrent logic for shared scope acquire - Changes: - all: https://git.openjdk.java.net/jdk/pull/5907/files - new: https://git.openjdk.java.net/jdk/pull/5907/files/27e71ee7..5ed94c30 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=5907=04 - incr: https://webrevs.openjdk.java.net/?repo=jdk=5907=03-04 Stats: 10 lines in 2 files changed: 2 ins; 3 del; 5 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: 8253757: Add LLVM-based backend for hsdis
On Fri, 15 Oct 2021 08:20:05 GMT, Ludovic Henry wrote: > Since the maintenance is fairly low (small codebase, small and knowledgable > user base), I would be biased towards including it with appropriate warnings. I don't think we should commit code that we know is broken. I don't believe that this view might be controversial. Maybe someone should try to reproduce the failure I've seen, and then we should look at fixing it. Maybe it's a local problem. Also, this patch breaks all current hsdis builds that follow the installation instructions. Either we get revised instructions or the build should be fixed. - PR: https://git.openjdk.java.net/jdk/pull/5920
Re: RFR: 8275252: Migrate cacerts from JKS to password-less PKCS12
On Fri, 15 Oct 2021 14:12:55 GMT, Magnus Ihse Bursie wrote: >> make/jdk/src/classes/build/tools/generatecacerts/GenerateCacerts.java line >> 74: >> >>> 72: cert = (X509Certificate) cf.generateCertificate(fis); >>> 73: } >>> 74: ks.setCertificateEntry(alias, cert); >> >> In the previous code, we always used a fixed date (cert's notBefore) for the >> creation date. Now, it seems it will be always different and based on when >> it was created. I'm not really sure if this is an issue in practice, but I >> think it is worth thinking about a bit more - do you have any thoughts on >> this? > > If that means the build will become non-reproducible, then *I* certainly have > thoughts about it! ;-) The certificate stored in a PKCS12 file has no date associated. Whenever you load a keystore, the creation time is set to the load time. In fact, the `VerifyCACerts.java` maintains a SHA-256 hash of the keystore and it will not change unless the certs themselves are changed. Here is the actual bytes for one certificate entry inside: :1AD48 [] SEQUENCE 0005:0659 [0] SEQUENCE 0009:000D [00] OID 1.2.840.113549.1.12.10.1.3 (CertBag) 0016:05DB [01] cont [0] 001A:05D7 [010] SEQUENCE 001E:000C [0100] OID 1.2.840.113549.1.9.22.1 (CertTypeX509) 002A:05C7 [0101] cont [0] 002E:05C3 [01010] OCTET STRING (1729119956) : 30 82 05 BB 30 82 03 A3 A0 03 02 01 02 02 08 57 0...0..W 0010: 0A 11 97 42 C4 E3 CC 30 0D 06 09 2A 86 48 86 F7 ...B...0...*.H.. 0020: 0D 01 01 0B 05 00 30 6B 31 0B 30 09 06 03 55 04 ..0k1.0...U. (1471 bytes) 05F1:006D [02] SET 05F3:0053 [020] SEQUENCE 05F5:000B [0200] OID 1.2.840.113549.1.9.20 (FriendlyName) 0600:0046 [0201] SET 0602:0044 [02010] STRING "actalisauthenticationrootca [jdk]" 0646:0018 [021] SEQUENCE 0648:000E [0210] OID 2.16.840.1.113894.746875.1.1 (ORACLE_TrustedKeyUsage) 0656:0008 [0211] SET 0658:0006 [02110] OID 2.5.29.37.0 (anyExtendedKeyUsage) - PR: https://git.openjdk.java.net/jdk/pull/5948
Re: RFR: 8253757: Add LLVM-based backend for hsdis
Hi all, I am on vacation right now, but we can take a look next week and see how we can support the LLVM backend for hsdis. It is much required by Windows-AArch64 and we would hate for all the good work to not make it upstream. Monica Sent from my Arm powered smart device. On Thu, Oct 14, 2021, 9:07 AM Andrew Haley wrote: > On Wed, 13 Oct 2021 07:26:21 GMT, Ludovic Henry > wrote: > > >> This patch expands the newly added system for hsdis backends to include > LLVM. > >> > >> The actual code in hsdis-llvm.cpp is based heavily on the work by > @luhenry, as published in the never integrated PR > https://github.com/openjdk/jdk/pull/392. (I have basically just ripped > out the binutils-based part of it.) > >> > >> Unfortunately I have not been able to make this work properly on > Windows. With some additional flags I made it compile without complaints, > but it caused hotspot to segfault in `LoadLibrary` (!) in `os::dll_load` > when I tried to load the library. This is somewhat ironic, since the > initial implementation was created by Ludovic for the very purpose of using > it on Windows. > >> > >> The lack of Windows support in this patch does not mean it is > impossible to get it to work, just that I need to co-operate with someone > who has more experience of compiling LLVM on Windows, and/or are more eager > to get this combination to work. > > > > Very happy to see it landing :) Thank you! > > > > I don't have access to a windows machine, and even less a > Windows-AArch64 machine. @lewurm would you be able to take a look? > > > As for LLVM not giving you a good user experience; I can't really tell > what's wrong. Apparently @luhenry (and @JornVernee I believe) has used > this. I'm not really the target audience myself; I'm only trying to make it > possible to use. If it is so severly limited as you say maybe this isn't > worth pursuing. Some feedback from those who have tested it would be > appeciated here, to help med understand if this patch should be dropped. > > I don't think it should be dropped, but I imagine that the bugs can be > fixed. If LLVM's disassembler always dies as soon as it sees something it > can't recognize, I'm astonished. Maybe the LLVM I'm using is bad. > > - > > PR: https://git.openjdk.java.net/jdk/pull/5920 >
Re: RFR: 8275252: Migrate cacerts from JKS to password-less PKCS12
On Fri, 15 Oct 2021 14:02:15 GMT, Sean Mullan wrote: >> The cacerts file is now a password-less PKCS12 file. This make sure old code >> that uses a JKS KeyStore object can continuously load it using a null >> password (in fact, any password) and see all certificates inside. > > make/jdk/src/classes/build/tools/generatecacerts/GenerateCacerts.java line 74: > >> 72: cert = (X509Certificate) cf.generateCertificate(fis); >> 73: } >> 74: ks.setCertificateEntry(alias, cert); > > In the previous code, we always used a fixed date (cert's notBefore) for the > creation date. Now, it seems it will be always different and based on when it > was created. I'm not really sure if this is an issue in practice, but I think > it is worth thinking about a bit more - do you have any thoughts on this? If that means the build will become non-reproducible, then *I* certainly have thoughts about it! ;-) - PR: https://git.openjdk.java.net/jdk/pull/5948
Re: RFR: 8275252: Migrate cacerts from JKS to password-less PKCS12
On Thu, 14 Oct 2021 13:36:19 GMT, Weijun Wang wrote: > The cacerts file is now a password-less PKCS12 file. This make sure old code > that uses a JKS KeyStore object can continuously load it using a null > password (in fact, any password) and see all certificates inside. make/jdk/src/classes/build/tools/generatecacerts/GenerateCacerts.java line 74: > 72: cert = (X509Certificate) cf.generateCertificate(fis); > 73: } > 74: ks.setCertificateEntry(alias, cert); In the previous code, we always used a fixed date (cert's notBefore) for the creation date. Now, it seems it will be always different and based on when it was created. I'm not really sure if this is an issue in practice, but I think it is worth thinking about a bit more - do you have any thoughts on this? - PR: https://git.openjdk.java.net/jdk/pull/5948
Re: RFR: 8253757: Add LLVM-based backend for hsdis
On Wed, 13 Oct 2021 00:00:22 GMT, Magnus Ihse Bursie wrote: > This patch expands the newly added system for hsdis backends to include LLVM. > > The actual code in hsdis-llvm.cpp is based heavily on the work by @luhenry, > as published in the never integrated PR > https://github.com/openjdk/jdk/pull/392. (I have basically just ripped out > the binutils-based part of it.) > > Unfortunately I have not been able to make this work properly on Windows. > With some additional flags I made it compile without complaints, but it > caused hotspot to segfault in `LoadLibrary` (!) in `os::dll_load` when I > tried to load the library. This is somewhat ironic, since the initial > implementation was created by Ludovic for the very purpose of using it on > Windows. > > The lack of Windows support in this patch does not mean it is impossible to > get it to work, just that I need to co-operate with someone who has more > experience of compiling LLVM on Windows, and/or are more eager to get this > combination to work. The value add of this LLVM-based hsdis is two-fold: - It supports platforms that aren't supported by binutils (Windows-AArch64 for example) - The license being more permissive would allow to build it as part of the OpenJDK build more easily (and even maybe ship it?) LLVM has a strong track record of supporting new platforms (Windows-AArch64 and macOS-AArch64 for example, mostly because of investment from Microsoft and Apple respectively), and `hsdis` is a necessary tool for porting the OpenJDK to any new platform. Since the maintenance is fairly low (small codebase, small and knowledgable user base), I would be biased towards including it with appropriate warnings. - PR: https://git.openjdk.java.net/jdk/pull/5920
