Re: RFR: 8301793: AlgorithmId should not encode a missing parameters field as NULL unless hardcoded
On Fri, 3 Feb 2023 17:35:45 GMT, Weijun Wang wrote:
> Change blocklist to allowlist for encoding null parameters in `AlgorithmId`.
test/jdk/sun/security/x509/AlgorithmId/NullParams.java line 108:
> 106: } else {
> 107: if (data.available() != 0) {
> 108: System.out.println("Has expected NULL");
Should the message be "Has unexpected NULL"?
-
PR: https://git.openjdk.org/jdk/pull/12412
Re: RFR: 8301793: AlgorithmId should not encode a missing parameters field as NULL unless hardcoded
On Fri, 3 Feb 2023 17:35:45 GMT, Weijun Wang wrote: > Change blocklist to allowlist for encoding null parameters in `AlgorithmId`. Looks good other than the minor test comment but I think this is probably an Enhancement rather than a Bug. - Marked as reviewed by mullan (Reviewer). PR: https://git.openjdk.org/jdk/pull/12412
Re: RFR: 8301793: AlgorithmId should not encode a missing parameters field as NULL unless hardcoded
On Mon, 6 Mar 2023 16:04:13 GMT, Sean Mullan wrote:
> Looks good other than the minor test comment but I think this is probably an
> Enhancement rather than a Bug.
Thanks. Fixed the test and updated the issue as an enhancement.
> test/jdk/sun/security/x509/AlgorithmId/NullParams.java line 108:
>
>> 106: } else {
>> 107: if (data.available() != 0) {
>> 108: System.out.println("Has expected NULL");
>
> Should the message be "Has unexpected NULL"?
Yes.
-
PR: https://git.openjdk.org/jdk/pull/12412
Re: RFR: 8301793: AlgorithmId should not encode a missing parameters field as NULL unless hardcoded [v2]
> Change blocklist to allowlist for encoding null parameters in `AlgorithmId`. Weijun Wang has updated the pull request incrementally with one additional commit since the last revision: unexpected - Changes: - all: https://git.openjdk.org/jdk/pull/12412/files - new: https://git.openjdk.org/jdk/pull/12412/files/d2fd2048..f6f676c0 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=12412&range=01 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=12412&range=00-01 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.org/jdk/pull/12412.diff Fetch: git fetch https://git.openjdk.org/jdk pull/12412/head:pull/12412 PR: https://git.openjdk.org/jdk/pull/12412
Integrated: 8301793: AlgorithmId should not encode a missing parameters field as NULL unless hardcoded
On Fri, 3 Feb 2023 17:35:45 GMT, Weijun Wang wrote: > Change blocklist to allowlist for encoding null parameters in `AlgorithmId`. This pull request has now been integrated. Changeset: a97271e3 Author:Weijun Wang URL: https://git.openjdk.org/jdk/commit/a97271e3b5d5a08fc503a11cd3e253974fb77ce6 Stats: 193 lines in 2 files changed: 121 ins; 21 del; 51 mod 8301793: AlgorithmId should not encode a missing parameters field as NULL unless hardcoded Reviewed-by: mullan - PR: https://git.openjdk.org/jdk/pull/12412
Re: RFR: 8303410: Remove ContentSigner APIs and jarsigner -altsigner and -altsignerpath options
On Tue, 28 Feb 2023 19:09:00 GMT, Eirik Bjorsnos wrote: > The `-altsigner` and `-altsignerpath` options in JarSigner with the > underlying `ContentSigner` mechanism were deprected in Java 9, for removal in > Java 15. See [JDK-8076535](https://bugs.openjdk.org/browse/JDK-8076535), > [JDK-8242260](https://bugs.openjdk.org/browse/JDK-8242260). > > This PR suggests it's time to remove this code: > > - The package `com/sun/jarsigner` is removed. This contained the > `ContentSigner` and `ContentSignerParameters` along with a > `package-info.java` file. > - `JarSigner.java` is updated to remove processing of the `-altsigner` and > `-altsignerpath` options and the loading and processing of the custom > `ContentSigner`. > - Similarly `c.s.s.t.jarsigner.Main` is updated to remove processing and > mentioning of `-altsigner` and `-altsignerpath` options. > - Mentions of the options in Resource files in the same directory is removed > - The `jarsigner.1` man page is updated to remove the section on the > deprecated options > - The `Spec` and `Options` tests are update to remove usage of the > `-altsigner` and `-altsignerpath` options. The CSR seems to be approved, so I guess it should be OK to proceeed with reviewing the actual deletions now? - PR: https://git.openjdk.org/jdk/pull/12791
Re: RFR: 8303410: Remove ContentSigner APIs and jarsigner -altsigner and -altsignerpath options
On Tue, 28 Feb 2023 19:09:00 GMT, Eirik Bjorsnos wrote: > The `-altsigner` and `-altsignerpath` options in JarSigner with the > underlying `ContentSigner` mechanism were deprected in Java 9, for removal in > Java 15. See [JDK-8076535](https://bugs.openjdk.org/browse/JDK-8076535), > [JDK-8242260](https://bugs.openjdk.org/browse/JDK-8242260). > > This PR suggests it's time to remove this code: > > - The package `com/sun/jarsigner` is removed. This contained the > `ContentSigner` and `ContentSignerParameters` along with a > `package-info.java` file. > - `JarSigner.java` is updated to remove processing of the `-altsigner` and > `-altsignerpath` options and the loading and processing of the custom > `ContentSigner`. > - Similarly `c.s.s.t.jarsigner.Main` is updated to remove processing and > mentioning of `-altsigner` and `-altsignerpath` options. > - Mentions of the options in Resource files in the same directory is removed > - The `jarsigner.1` man page is updated to remove the section on the > deprecated options > - The `Spec` and `Options` tests are update to remove usage of the > `-altsigner` and `-altsignerpath` options. Yes. I'll review the change. BTW, you don't need to touch `jarsigner.1`. It is auto-generated and another team maintains the source. - PR: https://git.openjdk.org/jdk/pull/12791
Re: RFR: 8303410: Remove ContentSigner APIs and jarsigner -altsigner and -altsignerpath options [v2]
> The `-altsigner` and `-altsignerpath` options in JarSigner with the > underlying `ContentSigner` mechanism were deprected in Java 9, for removal in > Java 15. See [JDK-8076535](https://bugs.openjdk.org/browse/JDK-8076535), > [JDK-8242260](https://bugs.openjdk.org/browse/JDK-8242260). > > This PR suggests it's time to remove this code: > > - The package `com/sun/jarsigner` is removed. This contained the > `ContentSigner` and `ContentSignerParameters` along with a > `package-info.java` file. > - `JarSigner.java` is updated to remove processing of the `-altsigner` and > `-altsignerpath` options and the loading and processing of the custom > `ContentSigner`. > - Similarly `c.s.s.t.jarsigner.Main` is updated to remove processing and > mentioning of `-altsigner` and `-altsignerpath` options. > - Mentions of the options in Resource files in the same directory is removed > - The `jarsigner.1` man page is updated to remove the section on the > deprecated options > - The `Spec` and `Options` tests are update to remove usage of the > `-altsigner` and `-altsignerpath` options. Eirik Bjorsnos has updated the pull request incrementally with one additional commit since the last revision: Revert changes to generated file jarsigner.1 - Changes: - all: https://git.openjdk.org/jdk/pull/12791/files - new: https://git.openjdk.org/jdk/pull/12791/files/71c0e1b9..fdbcf021 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=12791&range=01 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=12791&range=00-01 Stats: 58 lines in 1 file changed: 57 ins; 0 del; 1 mod Patch: https://git.openjdk.org/jdk/pull/12791.diff Fetch: git fetch https://git.openjdk.org/jdk pull/12791/head:pull/12791 PR: https://git.openjdk.org/jdk/pull/12791
Re: RFR: 8303410: Remove ContentSigner APIs and jarsigner -altsigner and -altsignerpath options
On Mon, 6 Mar 2023 18:52:52 GMT, Weijun Wang wrote: > BTW, you don't need to touch `jarsigner.1`. It is auto-generated and another > team maintains the source. Thanks, I was not aware. I have reverted the changes to this file and trust it will be updated through some other channel. - PR: https://git.openjdk.org/jdk/pull/12791
Re: RFR: 8303480: Miscellaneous fixes to mostly invisible doc comments [v2]
> Please review this superficial documentation cleanup that was triggered by > unrelated analysis of doc comments in JDK API. > > The only effect that this multi-area PR has on the JDK API Documentation > (i.e. the observable effect on the generated HTML pages) can be summarized as > follows: > > > diff -ur build/macosx-aarch64/images/docs-before/api/serialized-form.html > build/macosx-aarch64/images/docs-after/api/serialized-form.html > --- build/macosx-aarch64/images/docs-before/api/serialized-form.html > 2023-03-02 11:47:44 > +++ build/macosx-aarch64/images/docs-after/api/serialized-form.html > 2023-03-02 11:48:45 > @@ -17084,7 +17084,7 @@ > throws href="java.base/java/io/IOException.html" title="class in > java.io">IOException, > ClassNotFoundException > readObject is called to restore the > state of the > - (@code BasicPermission} from a stream. > + BasicPermission from a stream. > > Parameters: > s - the ObjectInputStream from which data > is read > > Notes > - > > * I'm not an expert in any of the affected areas, except for jdk.javadoc, and > I was merely after misused tags. Because of that, I would appreciate reviews > from experts in other areas. > * I discovered many more issues than I included in this PR. The excluded > issues seem to occur in infrequently updated third-party code (e.g. > javax.xml), which I assume we shouldn't touch unless necessary. > * I will update copyright years after (and if) the fix had been approved, as > required. Pavel Rappo has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains two additional commits since the last revision: - Merge branch 'master' into 8303480 - Initial commit - Changes: - all: https://git.openjdk.org/jdk/pull/12826/files - new: https://git.openjdk.org/jdk/pull/12826/files/d2f4a553..87166408 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=12826&range=01 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=12826&range=00-01 Stats: 13433 lines in 415 files changed: 9003 ins; 2610 del; 1820 mod Patch: https://git.openjdk.org/jdk/pull/12826.diff Fetch: git fetch https://git.openjdk.org/jdk pull/12826/head:pull/12826 PR: https://git.openjdk.org/jdk/pull/12826
Re: RFR: 8303480: Miscellaneous fixes to mostly invisible doc comments [v2]
On Fri, 3 Mar 2023 11:31:04 GMT, Alexey Ivanov wrote: >> Yes, iff means if-and-only-if and is used for extra precision in formal >> logic, mathematics. As @pavelrappo points out it's a relatively common >> occurrence in the OpenJDK sources, though perhaps not in the public >> javadocs. Perhaps a bit pretentious, but mostly a terse way to say "return >> true if the BSM method type exactly matches X, otherwise false". >> >> The broken link stems from the fact that the method I was targeting (a way >> to use condy for lambda proxy singletons rather than a >> `MethodHandle.constant`) was never integrated. We'll look at either getting >> that done (@briangoetz suggested the time might be ready for it) or remove >> this currently pointless static bootstrap specialization test. > >> Yes, iff means if-and-only-if and is used for extra precision in formal >> logic, mathematics. > > I've never come across it before. With your explanations, it makes perfect > sense. I would recommend (separately) changing `iff` to the expanded form `if and only if` - PR: https://git.openjdk.org/jdk/pull/12826
Re: RFR: 8303480: Miscellaneous fixes to mostly invisible doc comments [v2]
The message from this sender included one or more files which could not be scanned for virus detection; do not open these files unless you are certain of the sender's intent. -- On Mon, 6 Mar 2023 20:22:48 GMT, Pavel Rappo wrote: >> Please review this superficial documentation cleanup that was triggered by >> unrelated analysis of doc comments in JDK API. >> >> The only effect that this multi-area PR has on the JDK API Documentation >> (i.e. the observable effect on the generated HTML pages) can be summarized >> as follows: >> >> >> diff -ur >> build/macosx-aarch64/images/docs-before/api/serialized-form.html >> build/macosx-aarch64/images/docs-after/api/serialized-form.html >> --- build/macosx-aarch64/images/docs-before/api/serialized-form.html >> 2023-03-02 11:47:44 >> +++ build/macosx-aarch64/images/docs-after/api/serialized-form.html >> 2023-03-02 11:48:45 >> @@ -17084,7 +17084,7 @@ >> throws > href="java.base/java/io/IOException.html" title="class in >> java.io">IOException, >> ClassNotFoundException >> readObject is called to restore the >> state of the >> - (@code BasicPermission} from a stream. >> + BasicPermission from a stream. >> >> Parameters: >> s - the ObjectInputStream from which data >> is read >> >> Notes >> - >> >> * I'm not an expert in any of the affected areas, except for jdk.javadoc, >> and I was merely after misused tags. Because of that, I would appreciate >> reviews from experts in other areas. >> * I discovered many more issues than I included in this PR. The excluded >> issues seem to occur in infrequently updated third-party code (e.g. >> javax.xml), which I assume we shouldn't touch unless necessary. >> * I will update copyright years after (and if) the fix had been approved, as >> required. > > Pavel Rappo has updated the pull request with a new target base due to a > merge or a rebase. The incremental webrev excludes the unrelated changes > brought in by the merge/rebase. The pull request contains two additional > commits since the last revision: > > - Merge branch 'master' into 8303480 > - Initial commit Marked as reviewed by jjg (Reviewer). - PR: https://git.openjdk.org/jdk/pull/12826
Re: RFR: 8303480: Miscellaneous fixes to mostly invisible doc comments [v2]
On Mon, 6 Mar 2023 20:22:48 GMT, Pavel Rappo wrote: >> Please review this superficial documentation cleanup that was triggered by >> unrelated analysis of doc comments in JDK API. >> >> The only effect that this multi-area PR has on the JDK API Documentation >> (i.e. the observable effect on the generated HTML pages) can be summarized >> as follows: >> >> >> diff -ur >> build/macosx-aarch64/images/docs-before/api/serialized-form.html >> build/macosx-aarch64/images/docs-after/api/serialized-form.html >> --- build/macosx-aarch64/images/docs-before/api/serialized-form.html >> 2023-03-02 11:47:44 >> +++ build/macosx-aarch64/images/docs-after/api/serialized-form.html >> 2023-03-02 11:48:45 >> @@ -17084,7 +17084,7 @@ >> throws > href="java.base/java/io/IOException.html" title="class in >> java.io">IOException, >> ClassNotFoundException >> readObject is called to restore the >> state of the >> - (@code BasicPermission} from a stream. >> + BasicPermission from a stream. >> >> Parameters: >> s - the ObjectInputStream from which data >> is read >> >> Notes >> - >> >> * I'm not an expert in any of the affected areas, except for jdk.javadoc, >> and I was merely after misused tags. Because of that, I would appreciate >> reviews from experts in other areas. >> * I discovered many more issues than I included in this PR. The excluded >> issues seem to occur in infrequently updated third-party code (e.g. >> javax.xml), which I assume we shouldn't touch unless necessary. >> * I will update copyright years after (and if) the fix had been approved, as >> required. > > Pavel Rappo has updated the pull request with a new target base due to a > merge or a rebase. The incremental webrev excludes the unrelated changes > brought in by the merge/rebase. The pull request contains two additional > commits since the last revision: > > - Merge branch 'master' into 8303480 > - Initial commit Marked as reviewed by lancea (Reviewer). - PR: https://git.openjdk.org/jdk/pull/12826
Re: RFR: 8303480: Miscellaneous fixes to mostly invisible doc comments [v2]
On Mon, 6 Mar 2023 20:22:48 GMT, Pavel Rappo wrote: >> Please review this superficial documentation cleanup that was triggered by >> unrelated analysis of doc comments in JDK API. >> >> The only effect that this multi-area PR has on the JDK API Documentation >> (i.e. the observable effect on the generated HTML pages) can be summarized >> as follows: >> >> >> diff -ur >> build/macosx-aarch64/images/docs-before/api/serialized-form.html >> build/macosx-aarch64/images/docs-after/api/serialized-form.html >> --- build/macosx-aarch64/images/docs-before/api/serialized-form.html >> 2023-03-02 11:47:44 >> +++ build/macosx-aarch64/images/docs-after/api/serialized-form.html >> 2023-03-02 11:48:45 >> @@ -17084,7 +17084,7 @@ >> throws > href="java.base/java/io/IOException.html" title="class in >> java.io">IOException, >> ClassNotFoundException >> readObject is called to restore the >> state of the >> - (@code BasicPermission} from a stream. >> + BasicPermission from a stream. >> >> Parameters: >> s - the ObjectInputStream from which data >> is read >> >> Notes >> - >> >> * I'm not an expert in any of the affected areas, except for jdk.javadoc, >> and I was merely after misused tags. Because of that, I would appreciate >> reviews from experts in other areas. >> * I discovered many more issues than I included in this PR. The excluded >> issues seem to occur in infrequently updated third-party code (e.g. >> javax.xml), which I assume we shouldn't touch unless necessary. >> * I will update copyright years after (and if) the fix had been approved, as >> required. > > Pavel Rappo has updated the pull request with a new target base due to a > merge or a rebase. The incremental webrev excludes the unrelated changes > brought in by the merge/rebase. The pull request contains two additional > commits since the last revision: > > - Merge branch 'master' into 8303480 > - Initial commit Marked as reviewed by rriggs (Reviewer). - PR: https://git.openjdk.org/jdk/pull/12826
RFR: 8303607: SunMSCAPI provider leaks memory and keys
The message from this sender included one or more files which could not be scanned for virus detection; do not open these files unless you are certain of the sender's intent. -- Use the correct API for freeing key handles when directed to by the output of CryptAcquireCertificatePrivateKey [1]. Specifically when [out] pfCallerFreeProvOrNCryptKey is true we test [out] pdwKeySpec for the CERT_NCRYPT_KEY_SPEC flag. When flag bit is set we now call NCryptFreeObject, otherwise we continue to call CryptReleaseContext (as before) [1] https://learn.microsoft.com/en-us/windows/win32/api/wincrypt/nf-wincrypt-cryptacquirecertificateprivatekey - Commit messages: - Merge branch 'openjdk:master' into ncrypt - Fix handle leak Changes: https://git.openjdk.org/jdk/pull/12891/files Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=12891&range=00 Issue: https://bugs.openjdk.org/browse/JDK-8303607 Stats: 5 lines in 1 file changed: 4 ins; 0 del; 1 mod Patch: https://git.openjdk.org/jdk/pull/12891.diff Fetch: git fetch https://git.openjdk.org/jdk pull/12891/head:pull/12891 PR: https://git.openjdk.org/jdk/pull/12891
Re: RFR: 8303607: SunMSCAPI provider leaks memory and keys
Weijun, Would you be so kind as to review and sponsor this change for me given that you are familiar with my previous changes [1] (although this issue existed prior) Once this is in tip, I'll look to backport to 19, 17 and 11 Thanks in advance Mat [1] https://bugs.openjdk.org/browse/JDK-8284850 Sent from Outlook From: security-dev on behalf of Mat Carter Sent: Monday, March 6, 2023 1:35 PM To: security-dev@openjdk.org Subject: RFR: 8303607: SunMSCAPI provider leaks memory and keys [Some people who received this message don't often get email from maca...@openjdk.org. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ] The message from this sender included one or more files which could not be scanned for virus detection; do not open these files unless you are certain of the sender's intent. -- Use the correct API for freeing key handles when directed to by the output of CryptAcquireCertificatePrivateKey [1]. Specifically when [out] pfCallerFreeProvOrNCryptKey is true we test [out] pdwKeySpec for the CERT_NCRYPT_KEY_SPEC flag. When flag bit is set we now call NCryptFreeObject, otherwise we continue to call CryptReleaseContext (as before) [1] https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Flearn.microsoft.com%2Fen-us%2Fwindows%2Fwin32%2Fapi%2Fwincrypt%2Fnf-wincrypt-cryptacquirecertificateprivatekey&data=05%7C01%7Cmatthew.carter%40microsoft.com%7Ce26c0d2b15e8424b988f08db1e8ac459%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C638137353598481138%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=DkDIdugqfKkENfLVcInWsdaN8mQHZk%2FMEzMo8a%2FofzQ%3D&reserved=0 - Commit messages: - Merge branch 'openjdk:master' into ncrypt - Fix handle leak Changes: https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgit.openjdk.org%2Fjdk%2Fpull%2F12891%2Ffiles&data=05%7C01%7Cmatthew.carter%40microsoft.com%7Ce26c0d2b15e8424b988f08db1e8ac459%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C638137353598481138%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=arWU6sUJYifADSnvxbxSbBhXixoV%2BfKQmERkfeleFWU%3D&reserved=0 Webrev: https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwebrevs.openjdk.org%2F%3Frepo%3Djdk%26pr%3D12891%26range%3D00&data=05%7C01%7Cmatthew.carter%40microsoft.com%7Ce26c0d2b15e8424b988f08db1e8ac459%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C638137353598481138%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=yVDaTyyNAasXuhsjGlKUE%2BybO%2FKYh853N54ONVfYUeE%3D&reserved=0 Issue: https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugs.openjdk.org%2Fbrowse%2FJDK-8303607&data=05%7C01%7Cmatthew.carter%40microsoft.com%7Ce26c0d2b15e8424b988f08db1e8ac459%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C638137353598637370%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=HtZ9QLDhraylQ2H%2FNnuwMahhYI8MoMRP7A5zWGrXRgg%3D&reserved=0 Stats: 5 lines in 1 file changed: 4 ins; 0 del; 1 mod Patch: https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgit.openjdk.org%2Fjdk%2Fpull%2F12891.diff&data=05%7C01%7Cmatthew.carter%40microsoft.com%7Ce26c0d2b15e8424b988f08db1e8ac459%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C638137353598637370%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=RnFSicrj4EDheFgBQ%2Fr7RszE%2FzS30gS2ykxpP%2FaSC10%3D&reserved=0 Fetch: git fetch https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgit.openjdk.org%2Fjdk&data=05%7C01%7Cmatthew.carter%40microsoft.com%7Ce26c0d2b15e8424b988f08db1e8ac459%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C638137353598637370%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=ObH7qip2rYafj%2FLaCXVz9Fq6ZmIzaPLycQe1AcYR%2Bv4%3D&reserved=0 pull/12891/head:pull/12891 PR: https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgit.openjdk.org%2Fjdk%2Fpull%2F12891&data=05%7C01%7Cmatthew.carter%40microsoft.com%7Ce26c0d2b15e8424b988f08db1e8ac459%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C638137353598637370%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=SQm05Uv%2B8Dde%2FUGdyDV%2FJl4be2vyYVMjuA6c2aLyVXk%3D&reserved=0
RFR: JDK-8287061: Support for rematerializing scalar replaced objects participating in allocation merges
Can I please get reviews for this PR to add support for the rematerialization of scalar-replaced objects that participate in allocation merges? The most common and frequent use of NonEscaping Phis merging object allocations is for debugging information. The two graphs below show numbers for Renaissance and DaCapo benchmarks - similar results are obtained for all other applications that I tested. With what frequency does each IR node type occurs as an allocation merge user? I.e., if the same node type uses a Phi N times the counter is incremented by N:  What are the most common users of allocation merges? I.e., if the same node type uses a Phi N times the counter is incremented by 1:  This PR adds support scalar replacing allocations participating in merges that are used *only* as debug information in SafePointNode and its subclasses. Although there is a performance benefit in doing scalar replacement in this scenario only, the goal of this PR is mainly to add infrastructure to support the rematerialization of SR objects participating in merges. I plan to create subsequent PRs to enable scalar replacement of merges used by other node types (CmpP, Load+AddP, primarily) subsequently. The approach I used is pretty straightforward. It consists basically in: 1) Extend SafePointScalarObjectNode to represent multiple SR objects; 2) Add a new Class to support rematerialization of SR objects part of merges; 3) Patch HotSpot to be able to serialize and deserialize debug information related to allocation merges; 4) Patch C2 to generate unique types for SR objects participating in allocation merges used only as debug information. I tested this with JTREG tests tier 1-4 (Windows, Linux, and Mac) and didn't see regression that might be related. I also tested with several applications and didn't see any failure. - Commit messages: - Add support for rematerializing scalar replaced objects participating in allocation merges Changes: https://git.openjdk.org/jdk/pull/12897/files Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=12897&range=00 Issue: https://bugs.openjdk.org/browse/JDK-8287061 Stats: 1803 lines in 18 files changed: 1653 ins; 9 del; 141 mod Patch: https://git.openjdk.org/jdk/pull/12897.diff Fetch: git fetch https://git.openjdk.org/jdk pull/12897/head:pull/12897 PR: https://git.openjdk.org/jdk/pull/12897
Re: RFR: 8303607: SunMSCAPI provider leaks memory and keys
On Mon, 6 Mar 2023 21:27:07 GMT, Mat Carter wrote: > Use the correct API for freeing key handles when directed to by the output of > CryptAcquireCertificatePrivateKey [1]. > Specifically when [out] pfCallerFreeProvOrNCryptKey is true we test [out] > pdwKeySpec for the CERT_NCRYPT_KEY_SPEC flag. When flag bit is set we now > call NCryptFreeObject, otherwise we continue to call CryptReleaseContext (as > before) > > [1] > https://learn.microsoft.com/en-us/windows/win32/api/wincrypt/nf-wincrypt-cryptacquirecertificateprivatekey Looks fine to me. Thanks. - Marked as reviewed by weijun (Reviewer). PR: https://git.openjdk.org/jdk/pull/12891
Integrated: 8303607: SunMSCAPI provider leaks memory and keys
On Mon, 6 Mar 2023 21:27:07 GMT, Mat Carter wrote: > Use the correct API for freeing key handles when directed to by the output of > CryptAcquireCertificatePrivateKey [1]. > Specifically when [out] pfCallerFreeProvOrNCryptKey is true we test [out] > pdwKeySpec for the CERT_NCRYPT_KEY_SPEC flag. When flag bit is set we now > call NCryptFreeObject, otherwise we continue to call CryptReleaseContext (as > before) > > [1] > https://learn.microsoft.com/en-us/windows/win32/api/wincrypt/nf-wincrypt-cryptacquirecertificateprivatekey This pull request has now been integrated. Changeset: c51d40cf Author:Mat Carter Committer: Weijun Wang URL: https://git.openjdk.org/jdk/commit/c51d40cfebe793b2e979db0f2d91ac3b136311bb Stats: 5 lines in 1 file changed: 4 ins; 0 del; 1 mod 8303607: SunMSCAPI provider leaks memory and keys Reviewed-by: weijun - PR: https://git.openjdk.org/jdk/pull/12891
