Re: RFR(S): 8231612: 100% cpu on arm32 in Service Thread
> On Dec 10, 2019, at 9:05 AM, Aleksei Voitylov > wrote: > > Kim, > > thank you for the suggestions. Here is an updated webrev: > > http://cr.openjdk.java.net/~avoitylov/webrev.8231612.04/ > > The assembly generated with inline functions is equivalent to that with > the macros, and the testing passed as well. This is mostly good. However, the new function names don't follow usual HotSpot style, which is lower case with underscore word separators. (Yes, there are *many* counterexamples.) Also, the minimal parenthesizing in SetByteInInt requires one to think about operator precedence order more than I would like. What's there is correct, but would be easier to read as 855 return (n & ~(0xff << bitsIdx)) | (b << bitsIdx); (We have some functions in globalDefinitions.hpp to supposedly help with these kinds of calculations, but the supported types are often wrong (as here), and there are missing operations (again, as here).)
RFR: JDK-8233112: Exclude SVG files from build comparison
This patch fixes the COMPARE_BUILD baseline. The SVG files generated by GraphViz are non-deterministic, so adding a sed filter to compensate. There is also an issue since jpackage was added, where we now have executables as resources in lib/modules. Those need to be compared as executables. With these changes, the baseline is now clean. Bug: https://bugs.openjdk.java.net/browse/JDK-8233112 Webrev: http://cr.openjdk.java.net/~erikj/8233112/webrev.01/ /Erik
RFR: JDK-8235686: Add more custom hooks in Bundles.gmk
We need to customize the image dirs in order to support the new Macos signing requirements at Oracle. To support this, we need Bundles.gmk to accept more variable overrides for where the various images are located. Bug: https://bugs.openjdk.java.net/browse/JDK-8235686 Webrev: http://cr.openjdk.java.net/~erikj/8235686/webrev.01/ /Erik
RFR: JDK-8235687: Contents/MacOS/libjli.dylib cannot be a symlink
To be able to fully sign and notarize a jdk-bundle on MacOS, the file Contents/MacOS/libjli.dylib cannot be a symlink. The easy fix is to simply make it a normal file. Longer term we will evaluate what this file is actually needed for, but for now, we just need to make the JDK compatible with the new requirements from Apple. Bug: https://bugs.openjdk.java.net/browse/JDK-8235687 Webrev: http://cr.openjdk.java.net/~erikj/8235687/webrev.01/ The patch also removes a bad make pattern where phony targets have recipes actually modifying files and replaced them with the touch file pattern. /Erik
Re: RFR: JDK-8235585: Enable macOS codesigning for all libraries and executables
Hi Erik, Christoph, thank you! Rene On Tue, Dec 10, 2019 at 4:27 PM Langer, Christoph wrote: > > Hi René, > > LGTM, too. > > I'll sponsor it for you. > > Cheers > Christoph > > > -Original Message- > > From: Erik Joelsson > > Sent: Dienstag, 10. Dezember 2019 15:35 > > To: René Schünemann ; Langer, Christoph > > > > Cc: build-dev@openjdk.java.net > > Subject: Re: RFR: JDK-8235585: Enable macOS codesigning for all libraries > > and > > executables > > > > Looks good. > > > > /Erik > > > > On 2019-12-10 03:44, René Schünemann wrote: > > > Thank you Christoph. > > > > > > I have fixed the indentation in NativeCompilation.gmk and removed the > > > "com.apple.security.cs.disable-executable-page-protection" > > > entitlement. > > > > > > Updated webrev: > > > http://cr.openjdk.java.net/~goetz/wr19/rene/8235585- > > mac_notarization/02/ > > > > > > Rene > > > > > > On Tue, Dec 10, 2019 at 11:25 AM Langer, Christoph > > > wrote: > > >> Hi René, > > >> > > >> thanks for doing this. > > >> > > >> I agree to Erik's findings, these should be addressed. Other than that, I > > have no further points. > > >> > > >> It would be good, if this little enhancement can be pushed before > > Thursday to make it into JDK14 without special approval. > > >> > > >> Best regards > > >> Christoph > > >> > > >> > > >>> -Original Message- > > >>> From: build-dev On Behalf Of > > René > > >>> Schünemann > > >>> Sent: Dienstag, 10. Dezember 2019 09:27 > > >>> To: Erik Joelsson > > >>> Cc: build-dev@openjdk.java.net > > >>> Subject: Re: RFR: JDK-8235585: Enable macOS codesigning for all > > >>> libraries > > and > > >>> executables > > >>> > > >>> Hello Erik, > > >>> > > >>> thank you for your review. > > >>> > > >>> On Mon, Dec 9, 2019 at 5:48 PM Erik Joelsson > > > > >>> wrote: > > Hello René, > > > > Nice to see an OpenJDK solution to this. (Our Oracle solution requires > > too much corp specific customization to really benefit from code > > sharing > > with a simple codesign based implementation) > > > > On 2019-12-09 08:06, René Schünemann wrote: > > > Here is the webrev: > > > http://cr.openjdk.java.net/~goetz/wr19/rene/8235585- > > >>> mac_notarization/01/ > > Generally looks good. > > > > NativeCompilation.gmk, line 1132 looks weirdly indented. The line > > could > > also benefit from being broken up. See [1] for guidance. > > > > >>> I agree. I will break it into two lines. > > >>> > > > On Mon, Dec 9, 2019 at 5:05 PM René Schünemann > > > wrote: > > >> Hi, > > >> > > >> for the macOS notarization process, all executables and libraries > > need > > >> to be codesigned with hardened runtime (--options runtime) and > > >>> secure > > >> timestamp (--timestamp) enabled. Additionally for the OpenJDK > > certain > > >> entitlements have to be set during codesigning: > > >> > > >> * com.apple.security.cs.allow-jit > > >> * com.apple.security.cs.allow-unsigned-executable-memory > > >> * com.apple.security.cs.disable-executable-page-protection > > In our testing, we saw no need for disable-executable-page- > > protection. > > Did you actually see missing this trigger any problems? > > >>> I'm actually not quite sure. We have used this set internally for > > notarization. > > >>> I will go back an do some additional testing with this specific > > >>> entitlement removed. > > >>> > > >> * com.apple.security.cs.allow-dyld-environment-variables > > >> * com.apple.security.cs.debugger > > >> > > >> With this change the macOS codesign tool is being run for all native > > >> executables and libraries. > > >> > > >> Additionally this change introduces a new configure option: > > >> --with-macosx-codesign-identity > > >> > > >> This options allows to specify a codesigning identity stored in the > > >> macOS keychain. > > >> When this option is not set it falls back to "openjdk_codesign". > > >> > > >> Thanks, > > >> Rene > > /Erik > > > > [1] http://openjdk.java.net/groups/build/doc/code-conventions.html > > > > >>> Rene
RE: RFR: JDK-8235585: Enable macOS codesigning for all libraries and executables
Hi René, LGTM, too. I'll sponsor it for you. Cheers Christoph > -Original Message- > From: Erik Joelsson > Sent: Dienstag, 10. Dezember 2019 15:35 > To: René Schünemann ; Langer, Christoph > > Cc: build-dev@openjdk.java.net > Subject: Re: RFR: JDK-8235585: Enable macOS codesigning for all libraries and > executables > > Looks good. > > /Erik > > On 2019-12-10 03:44, René Schünemann wrote: > > Thank you Christoph. > > > > I have fixed the indentation in NativeCompilation.gmk and removed the > > "com.apple.security.cs.disable-executable-page-protection" > > entitlement. > > > > Updated webrev: > > http://cr.openjdk.java.net/~goetz/wr19/rene/8235585- > mac_notarization/02/ > > > > Rene > > > > On Tue, Dec 10, 2019 at 11:25 AM Langer, Christoph > > wrote: > >> Hi René, > >> > >> thanks for doing this. > >> > >> I agree to Erik's findings, these should be addressed. Other than that, I > have no further points. > >> > >> It would be good, if this little enhancement can be pushed before > Thursday to make it into JDK14 without special approval. > >> > >> Best regards > >> Christoph > >> > >> > >>> -Original Message- > >>> From: build-dev On Behalf Of > René > >>> Schünemann > >>> Sent: Dienstag, 10. Dezember 2019 09:27 > >>> To: Erik Joelsson > >>> Cc: build-dev@openjdk.java.net > >>> Subject: Re: RFR: JDK-8235585: Enable macOS codesigning for all libraries > and > >>> executables > >>> > >>> Hello Erik, > >>> > >>> thank you for your review. > >>> > >>> On Mon, Dec 9, 2019 at 5:48 PM Erik Joelsson > > >>> wrote: > Hello René, > > Nice to see an OpenJDK solution to this. (Our Oracle solution requires > too much corp specific customization to really benefit from code > sharing > with a simple codesign based implementation) > > On 2019-12-09 08:06, René Schünemann wrote: > > Here is the webrev: > > http://cr.openjdk.java.net/~goetz/wr19/rene/8235585- > >>> mac_notarization/01/ > Generally looks good. > > NativeCompilation.gmk, line 1132 looks weirdly indented. The line > could > also benefit from being broken up. See [1] for guidance. > > >>> I agree. I will break it into two lines. > >>> > > On Mon, Dec 9, 2019 at 5:05 PM René Schünemann > > wrote: > >> Hi, > >> > >> for the macOS notarization process, all executables and libraries > need > >> to be codesigned with hardened runtime (--options runtime) and > >>> secure > >> timestamp (--timestamp) enabled. Additionally for the OpenJDK > certain > >> entitlements have to be set during codesigning: > >> > >> * com.apple.security.cs.allow-jit > >> * com.apple.security.cs.allow-unsigned-executable-memory > >> * com.apple.security.cs.disable-executable-page-protection > In our testing, we saw no need for disable-executable-page- > protection. > Did you actually see missing this trigger any problems? > >>> I'm actually not quite sure. We have used this set internally for > notarization. > >>> I will go back an do some additional testing with this specific > >>> entitlement removed. > >>> > >> * com.apple.security.cs.allow-dyld-environment-variables > >> * com.apple.security.cs.debugger > >> > >> With this change the macOS codesign tool is being run for all native > >> executables and libraries. > >> > >> Additionally this change introduces a new configure option: > >> --with-macosx-codesign-identity > >> > >> This options allows to specify a codesigning identity stored in the > >> macOS keychain. > >> When this option is not set it falls back to "openjdk_codesign". > >> > >> Thanks, > >> Rene > /Erik > > [1] http://openjdk.java.net/groups/build/doc/code-conventions.html > > >>> Rene
Re: RFR: JDK-8235585: Enable macOS codesigning for all libraries and executables
Looks good. /Erik On 2019-12-10 03:44, René Schünemann wrote: Thank you Christoph. I have fixed the indentation in NativeCompilation.gmk and removed the "com.apple.security.cs.disable-executable-page-protection" entitlement. Updated webrev: http://cr.openjdk.java.net/~goetz/wr19/rene/8235585-mac_notarization/02/ Rene On Tue, Dec 10, 2019 at 11:25 AM Langer, Christoph wrote: Hi René, thanks for doing this. I agree to Erik's findings, these should be addressed. Other than that, I have no further points. It would be good, if this little enhancement can be pushed before Thursday to make it into JDK14 without special approval. Best regards Christoph -Original Message- From: build-dev On Behalf Of René Schünemann Sent: Dienstag, 10. Dezember 2019 09:27 To: Erik Joelsson Cc: build-dev@openjdk.java.net Subject: Re: RFR: JDK-8235585: Enable macOS codesigning for all libraries and executables Hello Erik, thank you for your review. On Mon, Dec 9, 2019 at 5:48 PM Erik Joelsson wrote: Hello René, Nice to see an OpenJDK solution to this. (Our Oracle solution requires too much corp specific customization to really benefit from code sharing with a simple codesign based implementation) On 2019-12-09 08:06, René Schünemann wrote: Here is the webrev: http://cr.openjdk.java.net/~goetz/wr19/rene/8235585- mac_notarization/01/ Generally looks good. NativeCompilation.gmk, line 1132 looks weirdly indented. The line could also benefit from being broken up. See [1] for guidance. I agree. I will break it into two lines. On Mon, Dec 9, 2019 at 5:05 PM René Schünemann wrote: Hi, for the macOS notarization process, all executables and libraries need to be codesigned with hardened runtime (--options runtime) and secure timestamp (--timestamp) enabled. Additionally for the OpenJDK certain entitlements have to be set during codesigning: * com.apple.security.cs.allow-jit * com.apple.security.cs.allow-unsigned-executable-memory * com.apple.security.cs.disable-executable-page-protection In our testing, we saw no need for disable-executable-page-protection. Did you actually see missing this trigger any problems? I'm actually not quite sure. We have used this set internally for notarization. I will go back an do some additional testing with this specific entitlement removed. * com.apple.security.cs.allow-dyld-environment-variables * com.apple.security.cs.debugger With this change the macOS codesign tool is being run for all native executables and libraries. Additionally this change introduces a new configure option: --with-macosx-codesign-identity This options allows to specify a codesigning identity stored in the macOS keychain. When this option is not set it falls back to "openjdk_codesign". Thanks, Rene /Erik [1] http://openjdk.java.net/groups/build/doc/code-conventions.html Rene
Re: RFR(S): 8231612: 100% cpu on arm32 in Service Thread
Kim, thank you for the suggestions. Here is an updated webrev: http://cr.openjdk.java.net/~avoitylov/webrev.8231612.04/ The assembly generated with inline functions is equivalent to that with the macros, and the testing passed as well. -Aleksei On 07/12/2019 02:54, Kim Barrett wrote: >> On Dec 6, 2019, at 8:32 AM, Aleksei Voitylov >> wrote: >> >> Hi Kim, >> >> (cced hotspot-runtime as the updated webrev has code changes). >> >> Thank you for the suggestion to address this at the code level. The >> following change makes GCC generate correct code: >> >> webrev: http://cr.openjdk.java.net/~avoitylov/webrev.8231612.03/ >> issue: https://bugs.openjdk.java.net/browse/JDK-8231612 >> >> GCC alias analysis at RTL level has some limitations [1]. In particular, >> it sometimes cannot differentiate between pointer and integer and can >> make incorrect assumptions about whether two pointers actually alias >> each other. We are hitting this in >> Atomic::CmpxchgByteUsingInt::operator(), which is used on some platforms. >> >> The solution is to reference the same memory region using only one >> pointer. Anything with a second pointer here continues to confuse GCC, >> and would anyway be fragile in view of this GCC bug which surfaces >> easily on ARM32. >> >> I considered making get/set byte operations inline functions but >> preferred locality defines bring. >> >> Tested with JTReg, JCK and jcstress on ARM32 and Solaris SPARC with no >> regressions. The original problem also disappeared. >> No regressions in JCK VM,Lang, JTReg HotSpot on Linux x86, x86_64, >> AArch64, PPC, Mac. >> >> Thanks, >> >> -Aleksei >> >> [1] https://gcc.gnu.org/bugzilla/show_bug.cgi?id=49330 >> >> On 19/11/2019 19:01, Kim Barrett wrote: On Nov 19, 2019, at 10:18 AM, Aleksei Voitylov wrote: Kim, you are right, to play it safe we need to -fno-tree-pre all relevant bool occurrances. I'll get back with an updated webrev when testing is finished. >>> I think just sprinkling the build system with -fno-tree-pre to address this >>> doesn’t >>> scale in the face of people writing new code. >>> >>> I think we need to find a way to write CmpxchgByteUsingInt in such a way >>> that >>> it doesn’t tickle this bug, or forbid byte-sized cmpxchg (which would be >>> painful), >>> or provide some entirely other implementation approach. > This looks like a good solution to me. > > (I like this approach even without the gcc bug (which was an > entertaining read itself); this change lets me delete an item from my > followup list from templatizing Atomic.) > > A couple of really minor nits: > > (1) In the macros there are various expressions of the form > >x << BitsPerByte * idx > > Please put parens around the shift calculation, e.g. > >x << (BitsPerByte * idx) > > so readers don't need to remember the operator precedence order. > > (2) In this line: > > 871 uint32_t cur = SET_BYTE_IN_INT(*aligned_dest, canon_compare_value, > idx); > > I'd like "*aligned_dest" replaced with "Atomic::load(aligned_dest)". > > In addition, I think functions would be preferable to macros for the > helpers. Put them in the Atomic::CmpxchgByteUsingInt class, with > inline definitions where you put the macro definitions. > >
Re: RFR: JDK-8235585: Enable macOS codesigning for all libraries and executables
Thank you Christoph. I have fixed the indentation in NativeCompilation.gmk and removed the "com.apple.security.cs.disable-executable-page-protection" entitlement. Updated webrev: http://cr.openjdk.java.net/~goetz/wr19/rene/8235585-mac_notarization/02/ Rene On Tue, Dec 10, 2019 at 11:25 AM Langer, Christoph wrote: > > Hi René, > > thanks for doing this. > > I agree to Erik's findings, these should be addressed. Other than that, I > have no further points. > > It would be good, if this little enhancement can be pushed before Thursday to > make it into JDK14 without special approval. > > Best regards > Christoph > > > > -Original Message- > > From: build-dev On Behalf Of René > > Schünemann > > Sent: Dienstag, 10. Dezember 2019 09:27 > > To: Erik Joelsson > > Cc: build-dev@openjdk.java.net > > Subject: Re: RFR: JDK-8235585: Enable macOS codesigning for all libraries > > and > > executables > > > > Hello Erik, > > > > thank you for your review. > > > > On Mon, Dec 9, 2019 at 5:48 PM Erik Joelsson > > wrote: > > > > > > Hello René, > > > > > > Nice to see an OpenJDK solution to this. (Our Oracle solution requires > > > too much corp specific customization to really benefit from code sharing > > > with a simple codesign based implementation) > > > > > > On 2019-12-09 08:06, René Schünemann wrote: > > > > Here is the webrev: > > > > http://cr.openjdk.java.net/~goetz/wr19/rene/8235585- > > mac_notarization/01/ > > > > > > Generally looks good. > > > > > > NativeCompilation.gmk, line 1132 looks weirdly indented. The line could > > > also benefit from being broken up. See [1] for guidance. > > > > > > > I agree. I will break it into two lines. > > > > > > > > > > On Mon, Dec 9, 2019 at 5:05 PM René Schünemann > > > > wrote: > > > >> Hi, > > > >> > > > >> for the macOS notarization process, all executables and libraries need > > > >> to be codesigned with hardened runtime (--options runtime) and > > secure > > > >> timestamp (--timestamp) enabled. Additionally for the OpenJDK certain > > > >> entitlements have to be set during codesigning: > > > >> > > > >> * com.apple.security.cs.allow-jit > > > >> * com.apple.security.cs.allow-unsigned-executable-memory > > > >> * com.apple.security.cs.disable-executable-page-protection > > > In our testing, we saw no need for disable-executable-page-protection. > > > Did you actually see missing this trigger any problems? > > > > I'm actually not quite sure. We have used this set internally for > > notarization. > > I will go back an do some additional testing with this specific > > entitlement removed. > > > > > >> * com.apple.security.cs.allow-dyld-environment-variables > > > >> * com.apple.security.cs.debugger > > > >> > > > >> With this change the macOS codesign tool is being run for all native > > > >> executables and libraries. > > > >> > > > >> Additionally this change introduces a new configure option: > > > >> --with-macosx-codesign-identity > > > >> > > > >> This options allows to specify a codesigning identity stored in the > > > >> macOS keychain. > > > >> When this option is not set it falls back to "openjdk_codesign". > > > >> > > > >> Thanks, > > > >> Rene > > > /Erik > > > > > > [1] http://openjdk.java.net/groups/build/doc/code-conventions.html > > > > > > > Rene
RE: RFR: JDK-8235585: Enable macOS codesigning for all libraries and executables
Hi René, thanks for doing this. I agree to Erik's findings, these should be addressed. Other than that, I have no further points. It would be good, if this little enhancement can be pushed before Thursday to make it into JDK14 without special approval. Best regards Christoph > -Original Message- > From: build-dev On Behalf Of René > Schünemann > Sent: Dienstag, 10. Dezember 2019 09:27 > To: Erik Joelsson > Cc: build-dev@openjdk.java.net > Subject: Re: RFR: JDK-8235585: Enable macOS codesigning for all libraries and > executables > > Hello Erik, > > thank you for your review. > > On Mon, Dec 9, 2019 at 5:48 PM Erik Joelsson > wrote: > > > > Hello René, > > > > Nice to see an OpenJDK solution to this. (Our Oracle solution requires > > too much corp specific customization to really benefit from code sharing > > with a simple codesign based implementation) > > > > On 2019-12-09 08:06, René Schünemann wrote: > > > Here is the webrev: > > > http://cr.openjdk.java.net/~goetz/wr19/rene/8235585- > mac_notarization/01/ > > > > Generally looks good. > > > > NativeCompilation.gmk, line 1132 looks weirdly indented. The line could > > also benefit from being broken up. See [1] for guidance. > > > > I agree. I will break it into two lines. > > > > > > > On Mon, Dec 9, 2019 at 5:05 PM René Schünemann > > > wrote: > > >> Hi, > > >> > > >> for the macOS notarization process, all executables and libraries need > > >> to be codesigned with hardened runtime (--options runtime) and > secure > > >> timestamp (--timestamp) enabled. Additionally for the OpenJDK certain > > >> entitlements have to be set during codesigning: > > >> > > >> * com.apple.security.cs.allow-jit > > >> * com.apple.security.cs.allow-unsigned-executable-memory > > >> * com.apple.security.cs.disable-executable-page-protection > > In our testing, we saw no need for disable-executable-page-protection. > > Did you actually see missing this trigger any problems? > > I'm actually not quite sure. We have used this set internally for > notarization. > I will go back an do some additional testing with this specific > entitlement removed. > > > >> * com.apple.security.cs.allow-dyld-environment-variables > > >> * com.apple.security.cs.debugger > > >> > > >> With this change the macOS codesign tool is being run for all native > > >> executables and libraries. > > >> > > >> Additionally this change introduces a new configure option: > > >> --with-macosx-codesign-identity > > >> > > >> This options allows to specify a codesigning identity stored in the > > >> macOS keychain. > > >> When this option is not set it falls back to "openjdk_codesign". > > >> > > >> Thanks, > > >> Rene > > /Erik > > > > [1] http://openjdk.java.net/groups/build/doc/code-conventions.html > > > > Rene
Re: RFR: JDK-8235585: Enable macOS codesigning for all libraries and executables
Hello Erik, thank you for your review. On Mon, Dec 9, 2019 at 5:48 PM Erik Joelsson wrote: > > Hello René, > > Nice to see an OpenJDK solution to this. (Our Oracle solution requires > too much corp specific customization to really benefit from code sharing > with a simple codesign based implementation) > > On 2019-12-09 08:06, René Schünemann wrote: > > Here is the webrev: > > http://cr.openjdk.java.net/~goetz/wr19/rene/8235585-mac_notarization/01/ > > Generally looks good. > > NativeCompilation.gmk, line 1132 looks weirdly indented. The line could > also benefit from being broken up. See [1] for guidance. > I agree. I will break it into two lines. > > > > On Mon, Dec 9, 2019 at 5:05 PM René Schünemann > > wrote: > >> Hi, > >> > >> for the macOS notarization process, all executables and libraries need > >> to be codesigned with hardened runtime (--options runtime) and secure > >> timestamp (--timestamp) enabled. Additionally for the OpenJDK certain > >> entitlements have to be set during codesigning: > >> > >> * com.apple.security.cs.allow-jit > >> * com.apple.security.cs.allow-unsigned-executable-memory > >> * com.apple.security.cs.disable-executable-page-protection > In our testing, we saw no need for disable-executable-page-protection. > Did you actually see missing this trigger any problems? I'm actually not quite sure. We have used this set internally for notarization. I will go back an do some additional testing with this specific entitlement removed. > >> * com.apple.security.cs.allow-dyld-environment-variables > >> * com.apple.security.cs.debugger > >> > >> With this change the macOS codesign tool is being run for all native > >> executables and libraries. > >> > >> Additionally this change introduces a new configure option: > >> --with-macosx-codesign-identity > >> > >> This options allows to specify a codesigning identity stored in the > >> macOS keychain. > >> When this option is not set it falls back to "openjdk_codesign". > >> > >> Thanks, > >> Rene > /Erik > > [1] http://openjdk.java.net/groups/build/doc/code-conventions.html > Rene
