Re: RFR: JDK-8212780: JEP 343: Packaging Tool Implementation
> I believe otherwise it is an accurate incremental webrev of the jpackage changes since EA-5. It is also not very incremental. * *src/jdk.incubator.jpackage/share/classes/jdk/incubator/jpackage/main/Main.java is definitely not "new" ... -phil. On 11/26/19, 2:17 PM, Andy Herrick wrote: yes, this incremental webrev contains the JNLPConverter code, which it should not. I believe otherwise it is an accurate incremental webrev of the jpackage changes since EA-5. /Andy On 11/26/2019 4:53 PM, Phil Race wrote: Andy, I am puzzled by what I see here > The webrev at [3] shows the changes since EA-06 (Build 13-jpackage+1-49 ) up to the current point. > [3] http://cr.openjdk.java.net/~herrick/8212780/webrev.06-10/ This includes the JNLPConverter which isn't what I expected to see and (correctly) isn't in the cumulative webrev Since [3] seemed like the most obvious thing to do a review of the recent changes I'd like to be sure it has the correct content and I am not sure it does ... -phil. On 11/26/19 1:36 PM, Kevin Rushforth wrote: This all looks good. +1 -- Kevin On 11/26/2019 12:56 PM, Erik Joelsson wrote: (adding build-dev) Build changes look good. /Erik On 2019-11-20 09:37, Andy Herrick wrote: I posted new composite webrev [7], and git patch [8] after pushing fix for JDK-8234402 [6]. [7] http://cr.openjdk.java.net/~herrick/8212780/webrev.EA-11/ [8] http://cr.openjdk.java.net/~herrick/8212780/JDK-EA-11.git.patch /Andy On 11/19/2019 3:13 PM, Kevin Rushforth wrote: I took the "git diff" patch [5] that you uploaded yesterday, applied it, and verified that it is the same as what is in the JDK-8200758-branch branch of the sandbox. It builds and runs fine for me. I ran jcheck and it found the following three files with whitespace errors that will need to be fixed before you push: src/jdk.incubator.jpackage/linux/classes/jdk/incubator/jpackage/internal/PackageProperty.java:49: Trailing whitespace src/jdk.incubator.jpackage/share/classes/jdk/incubator/jpackage/ToolProviderFactory.java:35: Trailing whitespace test/jdk/tools/jpackage/share/AddLauncherBase.java:137: Trailing whitespace The second of these will go away with the fix for JDK-8234402 [6], so you don't need to do anything there. Once the fix for JDK-8234402 is pushed to sandbox, I presume you will update the webrev, right? Pending the fix for JDK-8234402 and the needed white-space fixes, this is a +1 from me (although I am not a jdk Project Reviewer, so you will need at least one review from someone who is). -- Kevin [5] http://cr.openjdk.java.net/~herrick/8212780/JDK-EA-10.git.patch [6] https://bugs.openjdk.java.net/browse/JDK-8234402 On 11/13/2019 4:23 PM, Andy Herrick wrote: Please review changes for [1] which is the implementation bug for JEP-343. The webrev at [2] is the total cumulative webrev of changes for the jpackage tool, currently in the JDK-8200758-branch branch of the open sandbox repository. The webrev at [3] shows the changes since EA-06 (Build 13-jpackage+1-49 ) up to the current point. The latest EA (Build 14-jpackage+1-49 ) is posted at [4]. Please send feedback to core-libs-...@openjdk.java.net [1] https://bugs.openjdk.java.net/browse/JDK-8212780 [2] http://cr.openjdk.java.net/~herrick/8212780/webrev.EA-10/ [3] http://cr.openjdk.java.net/~herrick/8212780/webrev.06-10/ [4] http://jdk.java.net/jpackage/
Re: RFR 8234697: Generate sun.security.util.math.intpoly classes during build
> On Nov 27, 2019, at 12:14 AM, Erik Joelsson wrote: > > On 2019-11-25 16:42, Weijun Wang wrote: >> >>> On Nov 26, 2019, at 12:36 AM, Erik Joelsson >>> wrote: >>> >>> Build change looks good. >> Thanks. >> >> One question: I see the output to stdout from FieldGen.java shown in build. >> Is it possible to hide it but still store the output in the log file? > > No, we are not able to support different log levels to the main log file and > console. What you can do is add $(LOG_INFO) or $(LOG_DEBUG) last on the > command line. That macro resolves to "> /dev/null" when we don't want the > output printed. How much output and how interesting is it to see? You can > also wrap the whole command in a call to ExecuteWithLog to have it being > logged to an individual file, which gets repeated at the end of the build in > case it fails. > > I think you should be able to combine the two with something like this: > > $(call ExecuteWithLog, ...) $(LOG_DEBUG) > > That should print everything to the special log file as well as letting it > pass to stdout when LOG=debug, but I haven't tested it. The log shows on the console with LOG=debug but whatever LOG level I choose, the output is always collected in the log file. This is exactly what I am looking for. Thanks, Max > > /Erik > >> I copied everything from CLDR_GEN_DONE but that one does not log anything >> actually. >> >> I can remove all output too, not really important. >> >> Thanks, >> Max >> >>> /Erik >>> >>> On 2019-11-22 18:59, Weijun Wang wrote: Please review the change at http://cr.openjdk.java.net/~weijun/8234697/webrev.00/ The new lines in Gensrc-java.base.gmk mimics the one for CLDR_GEN_DONE at the beginning of the same file. I changed the BigInteger parameter in the FieldParams constructor (the one not reading terms) to its HEX string form and is now using the inverse. This makes sure the strings appeared here are exactly the same as the one used in CurveDB.java. The generated source code is identical to before. Other smaller changes made to FieldGen.jsh: 1. Package name 2. No more jshell, but now Java 3. Input/output paths as args for main() 4. White spaces, wrapping and indentation Thanks, Max
Re: RFR: JDK-8212780: JEP 343: Packaging Tool Implementation
yes, this incremental webrev contains the JNLPConverter code, which it should not. I believe otherwise it is an accurate incremental webrev of the jpackage changes since EA-5. /Andy On 11/26/2019 4:53 PM, Phil Race wrote: Andy, I am puzzled by what I see here > The webrev at [3] shows the changes since EA-06 (Build 13-jpackage+1-49 ) up to the current point. > [3] http://cr.openjdk.java.net/~herrick/8212780/webrev.06-10/ This includes the JNLPConverter which isn't what I expected to see and (correctly) isn't in the cumulative webrev Since [3] seemed like the most obvious thing to do a review of the recent changes I'd like to be sure it has the correct content and I am not sure it does ... -phil. On 11/26/19 1:36 PM, Kevin Rushforth wrote: This all looks good. +1 -- Kevin On 11/26/2019 12:56 PM, Erik Joelsson wrote: (adding build-dev) Build changes look good. /Erik On 2019-11-20 09:37, Andy Herrick wrote: I posted new composite webrev [7], and git patch [8] after pushing fix for JDK-8234402 [6]. [7] http://cr.openjdk.java.net/~herrick/8212780/webrev.EA-11/ [8] http://cr.openjdk.java.net/~herrick/8212780/JDK-EA-11.git.patch /Andy On 11/19/2019 3:13 PM, Kevin Rushforth wrote: I took the "git diff" patch [5] that you uploaded yesterday, applied it, and verified that it is the same as what is in the JDK-8200758-branch branch of the sandbox. It builds and runs fine for me. I ran jcheck and it found the following three files with whitespace errors that will need to be fixed before you push: src/jdk.incubator.jpackage/linux/classes/jdk/incubator/jpackage/internal/PackageProperty.java:49: Trailing whitespace src/jdk.incubator.jpackage/share/classes/jdk/incubator/jpackage/ToolProviderFactory.java:35: Trailing whitespace test/jdk/tools/jpackage/share/AddLauncherBase.java:137: Trailing whitespace The second of these will go away with the fix for JDK-8234402 [6], so you don't need to do anything there. Once the fix for JDK-8234402 is pushed to sandbox, I presume you will update the webrev, right? Pending the fix for JDK-8234402 and the needed white-space fixes, this is a +1 from me (although I am not a jdk Project Reviewer, so you will need at least one review from someone who is). -- Kevin [5] http://cr.openjdk.java.net/~herrick/8212780/JDK-EA-10.git.patch [6] https://bugs.openjdk.java.net/browse/JDK-8234402 On 11/13/2019 4:23 PM, Andy Herrick wrote: Please review changes for [1] which is the implementation bug for JEP-343. The webrev at [2] is the total cumulative webrev of changes for the jpackage tool, currently in the JDK-8200758-branch branch of the open sandbox repository. The webrev at [3] shows the changes since EA-06 (Build 13-jpackage+1-49 ) up to the current point. The latest EA (Build 14-jpackage+1-49 ) is posted at [4]. Please send feedback to core-libs-...@openjdk.java.net [1] https://bugs.openjdk.java.net/browse/JDK-8212780 [2] http://cr.openjdk.java.net/~herrick/8212780/webrev.EA-10/ [3] http://cr.openjdk.java.net/~herrick/8212780/webrev.06-10/ [4] http://jdk.java.net/jpackage/
Re: RFR: JDK-8212780: JEP 343: Packaging Tool Implementation
Andy, I am puzzled by what I see here > The webrev at [3] shows the changes since EA-06 (Build 13-jpackage+1-49 ) up to the current point. > [3] http://cr.openjdk.java.net/~herrick/8212780/webrev.06-10/ This includes the JNLPConverter which isn't what I expected to see and (correctly) isn't in the cumulative webrev Since [3] seemed like the most obvious thing to do a review of the recent changes I'd like to be sure it has the correct content and I am not sure it does ... -phil. On 11/26/19 1:36 PM, Kevin Rushforth wrote: This all looks good. +1 -- Kevin On 11/26/2019 12:56 PM, Erik Joelsson wrote: (adding build-dev) Build changes look good. /Erik On 2019-11-20 09:37, Andy Herrick wrote: I posted new composite webrev [7], and git patch [8] after pushing fix for JDK-8234402 [6]. [7] http://cr.openjdk.java.net/~herrick/8212780/webrev.EA-11/ [8] http://cr.openjdk.java.net/~herrick/8212780/JDK-EA-11.git.patch /Andy On 11/19/2019 3:13 PM, Kevin Rushforth wrote: I took the "git diff" patch [5] that you uploaded yesterday, applied it, and verified that it is the same as what is in the JDK-8200758-branch branch of the sandbox. It builds and runs fine for me. I ran jcheck and it found the following three files with whitespace errors that will need to be fixed before you push: src/jdk.incubator.jpackage/linux/classes/jdk/incubator/jpackage/internal/PackageProperty.java:49: Trailing whitespace src/jdk.incubator.jpackage/share/classes/jdk/incubator/jpackage/ToolProviderFactory.java:35: Trailing whitespace test/jdk/tools/jpackage/share/AddLauncherBase.java:137: Trailing whitespace The second of these will go away with the fix for JDK-8234402 [6], so you don't need to do anything there. Once the fix for JDK-8234402 is pushed to sandbox, I presume you will update the webrev, right? Pending the fix for JDK-8234402 and the needed white-space fixes, this is a +1 from me (although I am not a jdk Project Reviewer, so you will need at least one review from someone who is). -- Kevin [5] http://cr.openjdk.java.net/~herrick/8212780/JDK-EA-10.git.patch [6] https://bugs.openjdk.java.net/browse/JDK-8234402 On 11/13/2019 4:23 PM, Andy Herrick wrote: Please review changes for [1] which is the implementation bug for JEP-343. The webrev at [2] is the total cumulative webrev of changes for the jpackage tool, currently in the JDK-8200758-branch branch of the open sandbox repository. The webrev at [3] shows the changes since EA-06 (Build 13-jpackage+1-49 ) up to the current point. The latest EA (Build 14-jpackage+1-49 ) is posted at [4]. Please send feedback to core-libs-...@openjdk.java.net [1] https://bugs.openjdk.java.net/browse/JDK-8212780 [2] http://cr.openjdk.java.net/~herrick/8212780/webrev.EA-10/ [3] http://cr.openjdk.java.net/~herrick/8212780/webrev.06-10/ [4] http://jdk.java.net/jpackage/
Re: RFR: JDK-8212780: JEP 343: Packaging Tool Implementation
This all looks good. +1 -- Kevin On 11/26/2019 12:56 PM, Erik Joelsson wrote: (adding build-dev) Build changes look good. /Erik On 2019-11-20 09:37, Andy Herrick wrote: I posted new composite webrev [7], and git patch [8] after pushing fix for JDK-8234402 [6]. [7] http://cr.openjdk.java.net/~herrick/8212780/webrev.EA-11/ [8] http://cr.openjdk.java.net/~herrick/8212780/JDK-EA-11.git.patch /Andy On 11/19/2019 3:13 PM, Kevin Rushforth wrote: I took the "git diff" patch [5] that you uploaded yesterday, applied it, and verified that it is the same as what is in the JDK-8200758-branch branch of the sandbox. It builds and runs fine for me. I ran jcheck and it found the following three files with whitespace errors that will need to be fixed before you push: src/jdk.incubator.jpackage/linux/classes/jdk/incubator/jpackage/internal/PackageProperty.java:49: Trailing whitespace src/jdk.incubator.jpackage/share/classes/jdk/incubator/jpackage/ToolProviderFactory.java:35: Trailing whitespace test/jdk/tools/jpackage/share/AddLauncherBase.java:137: Trailing whitespace The second of these will go away with the fix for JDK-8234402 [6], so you don't need to do anything there. Once the fix for JDK-8234402 is pushed to sandbox, I presume you will update the webrev, right? Pending the fix for JDK-8234402 and the needed white-space fixes, this is a +1 from me (although I am not a jdk Project Reviewer, so you will need at least one review from someone who is). -- Kevin [5] http://cr.openjdk.java.net/~herrick/8212780/JDK-EA-10.git.patch [6] https://bugs.openjdk.java.net/browse/JDK-8234402 On 11/13/2019 4:23 PM, Andy Herrick wrote: Please review changes for [1] which is the implementation bug for JEP-343. The webrev at [2] is the total cumulative webrev of changes for the jpackage tool, currently in the JDK-8200758-branch branch of the open sandbox repository. The webrev at [3] shows the changes since EA-06 (Build 13-jpackage+1-49 ) up to the current point. The latest EA (Build 14-jpackage+1-49 ) is posted at [4]. Please send feedback to core-libs-...@openjdk.java.net [1] https://bugs.openjdk.java.net/browse/JDK-8212780 [2] http://cr.openjdk.java.net/~herrick/8212780/webrev.EA-10/ [3] http://cr.openjdk.java.net/~herrick/8212780/webrev.06-10/ [4] http://jdk.java.net/jpackage/
Re: RFR: JDK-8212780: JEP 343: Packaging Tool Implementation
(adding build-dev) Build changes look good. /Erik On 2019-11-20 09:37, Andy Herrick wrote: I posted new composite webrev [7], and git patch [8] after pushing fix for JDK-8234402 [6]. [7] http://cr.openjdk.java.net/~herrick/8212780/webrev.EA-11/ [8] http://cr.openjdk.java.net/~herrick/8212780/JDK-EA-11.git.patch /Andy On 11/19/2019 3:13 PM, Kevin Rushforth wrote: I took the "git diff" patch [5] that you uploaded yesterday, applied it, and verified that it is the same as what is in the JDK-8200758-branch branch of the sandbox. It builds and runs fine for me. I ran jcheck and it found the following three files with whitespace errors that will need to be fixed before you push: src/jdk.incubator.jpackage/linux/classes/jdk/incubator/jpackage/internal/PackageProperty.java:49: Trailing whitespace src/jdk.incubator.jpackage/share/classes/jdk/incubator/jpackage/ToolProviderFactory.java:35: Trailing whitespace test/jdk/tools/jpackage/share/AddLauncherBase.java:137: Trailing whitespace The second of these will go away with the fix for JDK-8234402 [6], so you don't need to do anything there. Once the fix for JDK-8234402 is pushed to sandbox, I presume you will update the webrev, right? Pending the fix for JDK-8234402 and the needed white-space fixes, this is a +1 from me (although I am not a jdk Project Reviewer, so you will need at least one review from someone who is). -- Kevin [5] http://cr.openjdk.java.net/~herrick/8212780/JDK-EA-10.git.patch [6] https://bugs.openjdk.java.net/browse/JDK-8234402 On 11/13/2019 4:23 PM, Andy Herrick wrote: Please review changes for [1] which is the implementation bug for JEP-343. The webrev at [2] is the total cumulative webrev of changes for the jpackage tool, currently in the JDK-8200758-branch branch of the open sandbox repository. The webrev at [3] shows the changes since EA-06 (Build 13-jpackage+1-49 ) up to the current point. The latest EA (Build 14-jpackage+1-49 ) is posted at [4]. Please send feedback to core-libs-...@openjdk.java.net [1] https://bugs.openjdk.java.net/browse/JDK-8212780 [2] http://cr.openjdk.java.net/~herrick/8212780/webrev.EA-10/ [3] http://cr.openjdk.java.net/~herrick/8212780/webrev.06-10/ [4] http://jdk.java.net/jpackage/
Fwd: Re: RFR: JDK-8212780: JEP 343: Packaging Tool Implementation
Forwarding review thread to build-dev. /Andy Forwarded Message Subject:Re: RFR: JDK-8212780: JEP 343: Packaging Tool Implementation Date: Wed, 20 Nov 2019 12:37:20 -0500 From: Andy Herrick Organization: Oracle Corporation To: Kevin Rushforth , core-libs-dev I posted new composite webrev [7], and git patch [8] after pushing fix for JDK-8234402 [6]. [7] http://cr.openjdk.java.net/~herrick/8212780/webrev.EA-11/ [8] http://cr.openjdk.java.net/~herrick/8212780/JDK-EA-11.git.patch /Andy On 11/19/2019 3:13 PM, Kevin Rushforth wrote: I took the "git diff" patch [5] that you uploaded yesterday, applied it, and verified that it is the same as what is in the JDK-8200758-branch branch of the sandbox. It builds and runs fine for me. I ran jcheck and it found the following three files with whitespace errors that will need to be fixed before you push: src/jdk.incubator.jpackage/linux/classes/jdk/incubator/jpackage/internal/PackageProperty.java:49: Trailing whitespace src/jdk.incubator.jpackage/share/classes/jdk/incubator/jpackage/ToolProviderFactory.java:35: Trailing whitespace test/jdk/tools/jpackage/share/AddLauncherBase.java:137: Trailing whitespace The second of these will go away with the fix for JDK-8234402 [6], so you don't need to do anything there. Once the fix for JDK-8234402 is pushed to sandbox, I presume you will update the webrev, right? Pending the fix for JDK-8234402 and the needed white-space fixes, this is a +1 from me (although I am not a jdk Project Reviewer, so you will need at least one review from someone who is). -- Kevin [5] http://cr.openjdk.java.net/~herrick/8212780/JDK-EA-10.git.patch [6] https://bugs.openjdk.java.net/browse/JDK-8234402 On 11/13/2019 4:23 PM, Andy Herrick wrote: Please review changes for [1] which is the implementation bug for JEP-343. The webrev at [2] is the total cumulative webrev of changes for the jpackage tool, currently in the JDK-8200758-branch branch of the open sandbox repository. The webrev at [3] shows the changes since EA-06 (Build 13-jpackage+1-49 ) up to the current point. The latest EA (Build 14-jpackage+1-49 ) is posted at [4]. Please send feedback to core-libs-...@openjdk.java.net [1] https://bugs.openjdk.java.net/browse/JDK-8212780 [2] http://cr.openjdk.java.net/~herrick/8212780/webrev.EA-10/ [3] http://cr.openjdk.java.net/~herrick/8212780/webrev.06-10/ [4] http://jdk.java.net/jpackage/
Re: RFR [XS] 8234809: set relro in linker flags when building with gcc - was RE: binary Hardening on linux using Relocation Read-Only (relro)
* Matthias Baesken: > Florian, may I add you as reviewer ? Sure, but I don't have a formal reviewer role. Thanks, Florian
RE: RFR [XS] 8234809: set relro in linker flags when building with gcc - was RE: binary Hardening on linux using Relocation Read-Only (relro)
Thanks ! Florian, may I add you as reviewer ? Best regards, Matthias > Looks good. > > /Erik > > On 2019-11-26 05:07, Baesken, Matthias wrote: > >> Hello Erik, Florian , currently relro is set already for libjvm. > >> I think if this works nicely for libjvm, it shouldn't do any harm to set > >> it as > well > >> in the BASIC_LDFLAGS for other binaries . > >> I would propose a patch like : > > Hello, here is my webrev , please review . > > > > Bug/webrev : > > > > https://bugs.openjdk.java.net/browse/JDK-8234809 > > > > http://cr.openjdk.java.net/~mbaesken/webrevs/8234809.0/ > > > > > > Thanks, Matthias > > > >>> I would involve at least hotspot-dev for a wider discussion on this as > libjvm > >> is > >>> the most affected library. > >> Hello Erik, Florian , currently relro is set already for libjvm. > >> I think if this works nicely for libjvm, it shouldn't do any harm to set > >> it as > well > >> in the BASIC_LDFLAGS for other binaries . > >> I would propose a patch like : > >> > >> diff -r 80e1201f6c9a make/autoconf/flags-ldflags.m4 > >> --- a/make/autoconf/flags-ldflags.m4Fri Nov 22 09:06:35 2019 -0500 > >> +++ b/make/autoconf/flags-ldflags.m4Tue Nov 26 13:05:42 2019 +0100 > >> @@ -70,10 +70,9 @@ > >> fi > >> > >> # Add -z defs, to forbid undefined symbols in object files. > >> -BASIC_LDFLAGS="$BASIC_LDFLAGS -Wl,-z,defs" > >> - > >> -BASIC_LDFLAGS_JVM_ONLY="-Wl,-O1 -Wl,-z,relro" > >> - > >> +# add relro (mark relocations read only) for all libs > >> +BASIC_LDFLAGS="$BASIC_LDFLAGS -Wl,-z,defs -Wl,-z,relro" > >> +BASIC_LDFLAGS_JVM_ONLY="-Wl,-O1" > >> > >> > >> If I understand > >> https://bugzilla.redhat.com/show_bug.cgi?id=1571359 > >> correct, RedHat is setting those flags already via the build system . > >> > >> Regarding "bindnow" (ld -z now) , this might be set additionally by > using -- > >> with-extra-ldflags . > >> > >> > >> Best regards, Matthias > >> > >> > >>> Hello, > >>> > >>> I wasn't directly involved in introducing these flags, but my > >>> understanding is that it's always a performance compromise. I would > >>> involve at least hotspot-dev for a wider discussion on this as libjvm is > >>> the most affected library. > >>> > >>> /Erik > >>> > >>> On 2019-11-25 06:42, Baesken, Matthias wrote: > Hello, I wonder why the binary hardening on linux using Relocation > >>> Read-Only (relro) is not enabled by default. > Some info can be found here : > > https://wiki.debian.org/Hardening > > https://www.redhat.com/en/blog/hardening-elf-binaries-using- > >>> relocation-read-only-relro > > Currently I notice the settings only for debug / fastdebug builds , > see > >>> flags-ldflags.m4 : > # Setup debug level-dependent LDFLAGS > if test "x$TOOLCHAIN_TYPE" = xgcc; then > if test "x$OPENJDK_TARGET_OS" = xlinux; then > if test x$DEBUG_LEVEL = xrelease; then > > >>> > DEBUGLEVEL_LDFLAGS_JDK_ONLY="$DEBUGLEVEL_LDFLAGS_JDK_ONLY - > >>> Wl,-O1" > else > # mark relocations read only on (fast/slow) debug builds > DEBUGLEVEL_LDFLAGS_JDK_ONLY="-Wl,-z,relro" > fi > if test x$DEBUG_LEVEL = xslowdebug; then > # do relocations at load > DEBUGLEVEL_LDFLAGS="-Wl,-z,now" > fi > fi > > Shouldn't we use at least "-Wl,-z,relro" also on product builds ? > > For "-Wl,-z,now" some startup performance hits are mentioned in > >>> articles/blogs - any experiences / performance-measurements with > this > >> in > >>> the OpenJDK context ? > Best regards, Matthias >
Re: RFR [XS] 8234809: set relro in linker flags when building with gcc - was RE: binary Hardening on linux using Relocation Read-Only (relro)
Looks good. /Erik On 2019-11-26 05:07, Baesken, Matthias wrote: Hello Erik, Florian , currently relro is set already for libjvm. I think if this works nicely for libjvm, it shouldn't do any harm to set it as well in the BASIC_LDFLAGS for other binaries . I would propose a patch like : Hello, here is my webrev , please review . Bug/webrev : https://bugs.openjdk.java.net/browse/JDK-8234809 http://cr.openjdk.java.net/~mbaesken/webrevs/8234809.0/ Thanks, Matthias I would involve at least hotspot-dev for a wider discussion on this as libjvm is the most affected library. Hello Erik, Florian , currently relro is set already for libjvm. I think if this works nicely for libjvm, it shouldn't do any harm to set it as well in the BASIC_LDFLAGS for other binaries . I would propose a patch like : diff -r 80e1201f6c9a make/autoconf/flags-ldflags.m4 --- a/make/autoconf/flags-ldflags.m4Fri Nov 22 09:06:35 2019 -0500 +++ b/make/autoconf/flags-ldflags.m4Tue Nov 26 13:05:42 2019 +0100 @@ -70,10 +70,9 @@ fi # Add -z defs, to forbid undefined symbols in object files. -BASIC_LDFLAGS="$BASIC_LDFLAGS -Wl,-z,defs" - -BASIC_LDFLAGS_JVM_ONLY="-Wl,-O1 -Wl,-z,relro" - +# add relro (mark relocations read only) for all libs +BASIC_LDFLAGS="$BASIC_LDFLAGS -Wl,-z,defs -Wl,-z,relro" +BASIC_LDFLAGS_JVM_ONLY="-Wl,-O1" If I understand https://bugzilla.redhat.com/show_bug.cgi?id=1571359 correct, RedHat is setting those flags already via the build system . Regarding "bindnow" (ld -z now) , this might be set additionally by using -- with-extra-ldflags . Best regards, Matthias Hello, I wasn't directly involved in introducing these flags, but my understanding is that it's always a performance compromise. I would involve at least hotspot-dev for a wider discussion on this as libjvm is the most affected library. /Erik On 2019-11-25 06:42, Baesken, Matthias wrote: Hello, I wonder why the binary hardening on linux using Relocation Read-Only (relro) is not enabled by default. Some info can be found here : https://wiki.debian.org/Hardening https://www.redhat.com/en/blog/hardening-elf-binaries-using- relocation-read-only-relro Currently I notice the settings only for debug / fastdebug builds , see flags-ldflags.m4 : # Setup debug level-dependent LDFLAGS if test "x$TOOLCHAIN_TYPE" = xgcc; then if test "x$OPENJDK_TARGET_OS" = xlinux; then if test x$DEBUG_LEVEL = xrelease; then DEBUGLEVEL_LDFLAGS_JDK_ONLY="$DEBUGLEVEL_LDFLAGS_JDK_ONLY - Wl,-O1" else # mark relocations read only on (fast/slow) debug builds DEBUGLEVEL_LDFLAGS_JDK_ONLY="-Wl,-z,relro" fi if test x$DEBUG_LEVEL = xslowdebug; then # do relocations at load DEBUGLEVEL_LDFLAGS="-Wl,-z,now" fi fi Shouldn't we use at least "-Wl,-z,relro" also on product builds ? For "-Wl,-z,now" some startup performance hits are mentioned in articles/blogs - any experiences / performance-measurements with this in the OpenJDK context ? Best regards, Matthias
Re: RFR: 8234525: enable link-time section-gc for linux s390x to remove unused code
On 2019-11-26 04:53, Baesken, Matthias wrote: Erik, may I add you as a reviewer too ? Yes. /Erik
Re: RFR: 8234535: Cross compilation fails due to missing CFLAGS for the BUILD_CC
Will do. /Erik On 2019-11-26 01:05, christoph.goettsch...@microdoc.com wrote: Hi Erik, thanks for looking into this. I created a new webrev which includes the changeset and you as a reviewer: http://cr.openjdk.java.net/~cgo/8234535/webrev.02/ Could you please sponsor this changeset and commit it into the repository for me? Thanks, Christoph Erik Joelsson wrote on 2019-11-22 17:19:08: From: Erik Joelsson To: christoph.goettsch...@microdoc.com, build-dev@openjdk.java.net Date: 2019-11-22 17:19 Subject: Re: RFR: 8234535: Cross compilation fails due to missing CFLAGS for the BUILD_CC Hello Christoph, On 2019-11-20 10:22, christoph.goettsch...@microdoc.com wrote: Hi, please review the following change. Bug: https://bugs.openjdk.java.net/browse/JDK-8234535 Webrev: https://cr.openjdk.java.net/~cgo/8234535/webrev.00/ The removed lines are completely out of place and have no use at all. for the CC variable, for instance, the script does the following: CC_OLD="$CC" CC="$BUILD_CC" CC="$CC_OLD" without executing any function between those lines. I think that, while restructuring the build system, those lines have been copied to the wrong location. I copied them around the "FLAGS_SETUP_CFLAGS_CPU_DEP" call for the BUILD_CC, in order to make configure not use the target CFLAGS while discovering possible CFLAGS for the BUILD_CC, which was happening in our case. Wow, I had not noticed that. Your fix looks correct. I am also not sure if the variable BUILD_CC_DISABLE_WARNING_PREFIX is actually used anywhere. I could not find any user and it might be possible to simply remove it. It's used to override DISABLE_WARNING_PREFIX in buildjdk-spec.gmk so I think it serves a purpose, even if we don't actually support different toolchain types for cross and native compiler. If we ever will, then this variable will be needed, so I think it should stay. I tried the change and compiled on an amd64 linux machine for amd64, and cross-compiled for linux on armv7 and linux on aarch64. I don't have access to other cross-compilation environments and would like to ask others to review and try out the change. I applied the patch and tried a few of our configurations. Since we use the same (or close to the same) compiler versions across architectures, I don't actually see any difference in the generated spec files. The reason you see this is the large version difference between your compilers. Looks good, thanks for finding and fixing this! /Erik Thanks, Christoph
Re: RFR 8234697: Generate sun.security.util.math.intpoly classes during build
On 2019-11-25 16:42, Weijun Wang wrote: On Nov 26, 2019, at 12:36 AM, Erik Joelsson wrote: Build change looks good. Thanks. One question: I see the output to stdout from FieldGen.java shown in build. Is it possible to hide it but still store the output in the log file? No, we are not able to support different log levels to the main log file and console. What you can do is add $(LOG_INFO) or $(LOG_DEBUG) last on the command line. That macro resolves to "> /dev/null" when we don't want the output printed. How much output and how interesting is it to see? You can also wrap the whole command in a call to ExecuteWithLog to have it being logged to an individual file, which gets repeated at the end of the build in case it fails. I think you should be able to combine the two with something like this: $(call ExecuteWithLog, ...) $(LOG_DEBUG) That should print everything to the special log file as well as letting it pass to stdout when LOG=debug, but I haven't tested it. /Erik I copied everything from CLDR_GEN_DONE but that one does not log anything actually. I can remove all output too, not really important. Thanks, Max /Erik On 2019-11-22 18:59, Weijun Wang wrote: Please review the change at http://cr.openjdk.java.net/~weijun/8234697/webrev.00/ The new lines in Gensrc-java.base.gmk mimics the one for CLDR_GEN_DONE at the beginning of the same file. I changed the BigInteger parameter in the FieldParams constructor (the one not reading terms) to its HEX string form and is now using the inverse. This makes sure the strings appeared here are exactly the same as the one used in CurveDB.java. The generated source code is identical to before. Other smaller changes made to FieldGen.jsh: 1. Package name 2. No more jshell, but now Java 3. Input/output paths as args for main() 4. White spaces, wrapping and indentation Thanks, Max
Re: binary Hardening on linux using Relocation Read-Only (relro)
* Matthias Baesken: > If I understand > https://bugzilla.redhat.com/show_bug.cgi?id=1571359 > correct, RedHat is setting those flags already via the build system . BFD ld in binutils defaults to relro, except perhaps on s390x where your version might not implement the partial RELRO variant that you get without -z now (BIND_NOW is not enabled by default). > Regarding "bindnow" (ld -z now) , this might be set additionally by > using --with-extra-ldflags . Yes, that is usually more controversial because it can have an impact on startup time. But even the AWT libraries have relatively few function references, so it probably does not matter. On the other hand, all this security hardening is typically not very effective because part of classes.jsa is mapped rwx at a fixed address, so you can just abuse that (if you want to inject machine code directly, I'm sure there are other options for bytecode). Thanks, Florian
RFR [XS] 8234809: set relro in linker flags when building with gcc - was RE: binary Hardening on linux using Relocation Read-Only (relro)
> Hello Erik, Florian , currently relro is set already for libjvm. > I think if this works nicely for libjvm, it shouldn't do any harm to set it > as well > in the BASIC_LDFLAGS for other binaries . > I would propose a patch like : Hello, here is my webrev , please review . Bug/webrev : https://bugs.openjdk.java.net/browse/JDK-8234809 http://cr.openjdk.java.net/~mbaesken/webrevs/8234809.0/ Thanks, Matthias > > > I would involve at least hotspot-dev for a wider discussion on this as > > libjvm > is > > the most affected library. > > Hello Erik, Florian , currently relro is set already for libjvm. > I think if this works nicely for libjvm, it shouldn't do any harm to set it > as well > in the BASIC_LDFLAGS for other binaries . > I would propose a patch like : > > diff -r 80e1201f6c9a make/autoconf/flags-ldflags.m4 > --- a/make/autoconf/flags-ldflags.m4Fri Nov 22 09:06:35 2019 -0500 > +++ b/make/autoconf/flags-ldflags.m4Tue Nov 26 13:05:42 2019 +0100 > @@ -70,10 +70,9 @@ > fi > > # Add -z defs, to forbid undefined symbols in object files. > -BASIC_LDFLAGS="$BASIC_LDFLAGS -Wl,-z,defs" > - > -BASIC_LDFLAGS_JVM_ONLY="-Wl,-O1 -Wl,-z,relro" > - > +# add relro (mark relocations read only) for all libs > +BASIC_LDFLAGS="$BASIC_LDFLAGS -Wl,-z,defs -Wl,-z,relro" > +BASIC_LDFLAGS_JVM_ONLY="-Wl,-O1" > > > If I understand > https://bugzilla.redhat.com/show_bug.cgi?id=1571359 > correct, RedHat is setting those flags already via the build system . > > Regarding "bindnow" (ld -z now) , this might be set additionally by > using -- > with-extra-ldflags . > > > Best regards, Matthias > > > > Hello, > > > > I wasn't directly involved in introducing these flags, but my > > understanding is that it's always a performance compromise. I would > > involve at least hotspot-dev for a wider discussion on this as libjvm is > > the most affected library. > > > > /Erik > > > > On 2019-11-25 06:42, Baesken, Matthias wrote: > > > Hello, I wonder why the binary hardening on linux using Relocation > > Read-Only (relro) is not enabled by default. > > > > > > Some info can be found here : > > > > > > https://wiki.debian.org/Hardening > > > > > > https://www.redhat.com/en/blog/hardening-elf-binaries-using- > > relocation-read-only-relro > > > > > > > > > Currently I notice the settings only for debug / fastdebug builds , > > > see > > flags-ldflags.m4 : > > > > > ># Setup debug level-dependent LDFLAGS > > >if test "x$TOOLCHAIN_TYPE" = xgcc; then > > > if test "x$OPENJDK_TARGET_OS" = xlinux; then > > >if test x$DEBUG_LEVEL = xrelease; then > > > > > DEBUGLEVEL_LDFLAGS_JDK_ONLY="$DEBUGLEVEL_LDFLAGS_JDK_ONLY - > > Wl,-O1" > > >else > > > # mark relocations read only on (fast/slow) debug builds > > > DEBUGLEVEL_LDFLAGS_JDK_ONLY="-Wl,-z,relro" > > >fi > > >if test x$DEBUG_LEVEL = xslowdebug; then > > > # do relocations at load > > > DEBUGLEVEL_LDFLAGS="-Wl,-z,now" > > >fi > > > fi > > > > > > Shouldn't we use at least "-Wl,-z,relro" also on product builds ? > > > > > > For "-Wl,-z,now" some startup performance hits are mentioned in > > articles/blogs - any experiences / performance-measurements with this > in > > the OpenJDK context ? > > > > > > Best regards, Matthias > > >
RE: RFR: 8234525: enable link-time section-gc for linux s390x to remove unused code
Hi Martin , > Hi Matthias and Erik, > > I also think this is an interesting option. > > I like the idea to generate smaller libraries. In addition to that, I could > also > imagine building with -Os (size optimized) by default and only select -O3 for > performance critical files (e.g. C2's register allocation, some gc code, ...). > This is a good idea . However I would like to look into this separately in another change . ( Maybe there should also be a configure setting to select for size vs. speed optimization ) > If we want to go into such a direction for all linux platforms and want to use > this s390 only change as some kind of pipe cleaner, I think this change is > fine > and can get pushed. Yes , that was my intention. Erik, may I add you as a reviewer too ? Best regards, Matthias > Otherwise, I think building s390 differently and not intending to do the same > for other linux platforms would be not so good. > > We should only make sure the exported symbols are set up properly to avoid > that this optimization throws out too much. > > My 50 Cents. > > Best regards, > Martin >
RE: RFR [XS]: 8234795: ix build fails on macOS lower than 10.13 after 8214578 was: build fails on macOS 10.12 after 8214578: [macos] Problem with backslashes on macOS/JIS keyboard: Java ignores system
Thanks for the update on this . Do you plan to push it today or tomorrow ? Best regards, Matthias > -Original Message- > From: Prasanta Sadhukhan > Sent: Dienstag, 26. November 2019 10:26 > To: Baesken, Matthias ; Erik Joelsson > ; 'build-dev@openjdk.java.net' d...@openjdk.java.net> > Cc: 2d-...@openjdk.java.net; Lindenmaier, Goetz > > Subject: Re: RFR [XS]: 8234795: ix build fails on macOS lower than 10.13 after > 8214578 was: build fails on macOS 10.12 after 8214578: [macos] Problem with > backslashes on macOS/JIS keyboard: Java ignores system settings > > I have already raised a similar fix sometime back for JDK-8234786 > > Regards > > Prasanta > > On 26-Nov-19 2:49 PM, Baesken, Matthias wrote: > > Hello, here is a small adjustment that uses the typealias for > NSTextInputSourceIdentifier . This fixes the build on macOS < 10.13 . > > > > > > Bug/webrev : > > > > https://bugs.openjdk.java.net/browse/JDK-8234795 > > > > > > http://cr.openjdk.java.net/~mbaesken/webrevs/8234795.0/ > > > > > > Thanks, Matthias > > > > > >> If there is a simple fix, I would very much like to see it done. I'm not > >> familiar enough with this area to know what the implications would be > >> though. > >> > >> /Erik > >> > >> On 2019-11-25 04:48, Baesken, Matthias wrote: > >>> Hello, any comments on the issue ? > >>> > >>> Could we maybe switch from using > >>> > >>> NSTextInputSourceIdentifier > >>> > >>> to > >>> > >>> String (NSString* ?) , because > >> > https://developer.apple.com/documentation/appkit/nstextinputsourceiden > >> tifier > >>> says NSTextInputSourceIdentifieris a typealias for String ? > >>> > >>> Best regards ,Matthias > >>> > >>> > >>> > >>> > >>> Hello, I noticed that since today our jdk/jdk build fails on macOS . We > run > >> it on macOS 10.12 . > >>> It seems > >>> https://hg.openjdk.java.net/jdk/jdk/rev/d0bfaae2ff33 > >>> > >>> 8214578: [macos] Problem with backslashes on macOS/JIS keyboard: > Java > >> ignores system settings > >>> Brought a dependency on 10.13. Was that intended ? Could we keep > 10.12 > >> compatibility ? > >>> At least the doc of NSTextInputSourceIdentifier : > >> > https://developer.apple.com/documentation/appkit/nstextinputsourceiden > >> tifier > >>> mentions macOS 10.13+ . > >>> > >>> > >>> > >>> Build errors are : > >>> > >>> > >>> > /jdk/src/java.desktop/macosx/native/libawt_lwawt/awt/AWTView.h:41:5: > >> error: unknown type name 'NSTextInputSourceIdentifier' > >>> NSTextInputSourceIdentifier kbdLayout; > >>> ^ > >>> > >> > /jdk/src/java.desktop/macosx/native/libawt_lwawt/awt/AWTView.m:93:23: > >> error: assignment to readonly property > >>> self.cglLayer = windowLayer; > >>> ~ ^ ~~~ > >>> > >> > /jdk/src/java.desktop/macosx/native/libawt_lwawt/awt/AWTView.m:110:1 > >> 9: error: assignment to readonly property > >>> self.cglLayer = nil; > >>> ~ ^ ~~~ > >>> 3 errors generated. > >>> > >>> > >>> ... > >>> > >> > /jdk/src/java.desktop/macosx/native/libawt_lwawt/awt/AWTWindow.m:45 > >> 4:18: error: incompatible pointer to integer conversion initializing 'BOOL' > (aka > >> 'signed char') with an expression of type 'id' [-Werror,-Wint-conversion] > >>> BOOL mouseIsOver = [[window contentView] mouseIsOver]; > >>>^ ~~ > >>> 2 errors generated. > >>> > >>> > >>> > >>> Best regards, Matthias
RE: binary Hardening on linux using Relocation Read-Only (relro)
> I would involve at least hotspot-dev for a wider discussion on this as > libjvm is > the most affected library. Hello Erik, Florian , currently relro is set already for libjvm. I think if this works nicely for libjvm, it shouldn't do any harm to set it as well in the BASIC_LDFLAGS for other binaries . I would propose a patch like : diff -r 80e1201f6c9a make/autoconf/flags-ldflags.m4 --- a/make/autoconf/flags-ldflags.m4Fri Nov 22 09:06:35 2019 -0500 +++ b/make/autoconf/flags-ldflags.m4Tue Nov 26 13:05:42 2019 +0100 @@ -70,10 +70,9 @@ fi # Add -z defs, to forbid undefined symbols in object files. -BASIC_LDFLAGS="$BASIC_LDFLAGS -Wl,-z,defs" - -BASIC_LDFLAGS_JVM_ONLY="-Wl,-O1 -Wl,-z,relro" - +# add relro (mark relocations read only) for all libs +BASIC_LDFLAGS="$BASIC_LDFLAGS -Wl,-z,defs -Wl,-z,relro" +BASIC_LDFLAGS_JVM_ONLY="-Wl,-O1" If I understand https://bugzilla.redhat.com/show_bug.cgi?id=1571359 correct, RedHat is setting those flags already via the build system . Regarding "bindnow" (ld -z now) , this might be set additionally by using --with-extra-ldflags . Best regards, Matthias > Hello, > > I wasn't directly involved in introducing these flags, but my > understanding is that it's always a performance compromise. I would > involve at least hotspot-dev for a wider discussion on this as libjvm is > the most affected library. > > /Erik > > On 2019-11-25 06:42, Baesken, Matthias wrote: > > Hello, I wonder why the binary hardening on linux using Relocation > Read-Only (relro) is not enabled by default. > > > > Some info can be found here : > > > > https://wiki.debian.org/Hardening > > > > https://www.redhat.com/en/blog/hardening-elf-binaries-using- > relocation-read-only-relro > > > > > > Currently I notice the settings only for debug / fastdebug builds , see > flags-ldflags.m4 : > > > ># Setup debug level-dependent LDFLAGS > >if test "x$TOOLCHAIN_TYPE" = xgcc; then > > if test "x$OPENJDK_TARGET_OS" = xlinux; then > >if test x$DEBUG_LEVEL = xrelease; then > > > DEBUGLEVEL_LDFLAGS_JDK_ONLY="$DEBUGLEVEL_LDFLAGS_JDK_ONLY - > Wl,-O1" > >else > > # mark relocations read only on (fast/slow) debug builds > > DEBUGLEVEL_LDFLAGS_JDK_ONLY="-Wl,-z,relro" > >fi > >if test x$DEBUG_LEVEL = xslowdebug; then > > # do relocations at load > > DEBUGLEVEL_LDFLAGS="-Wl,-z,now" > >fi > > fi > > > > Shouldn't we use at least "-Wl,-z,relro" also on product builds ? > > > > For "-Wl,-z,now" some startup performance hits are mentioned in > articles/blogs - any experiences / performance-measurements with this in > the OpenJDK context ? > > > > Best regards, Matthias > >
RE: RFR: 8234525: enable link-time section-gc for linux s390x to remove unused code
Hi Matthias and Erik, I also think this is an interesting option. I like the idea to generate smaller libraries. In addition to that, I could also imagine building with -Os (size optimized) by default and only select -O3 for performance critical files (e.g. C2's register allocation, some gc code, ...). If we want to go into such a direction for all linux platforms and want to use this s390 only change as some kind of pipe cleaner, I think this change is fine and can get pushed. Otherwise, I think building s390 differently and not intending to do the same for other linux platforms would be not so good. We should only make sure the exported symbols are set up properly to avoid that this optimization throws out too much. My 50 Cents. Best regards, Martin > -Original Message- > From: build-dev On Behalf Of > Baesken, Matthias > Sent: Freitag, 22. November 2019 16:01 > To: Erik Joelsson ; 'build-dev@openjdk.java.net' > ; core-libs-...@openjdk.java.net > Subject: RE: RFR: 8234525: enable link-time section-gc for linux > s390x to remove unused code > > Hi Erik, >yes it makes the build more chatty - > our linux s390 product build log of jdk/jdk goes from ~ 60.000 > lines to ~ > 123.000 lines with the patch. > > However the change is linux s390 only so I guess it will not disturb other > people ( in case we bring it to more platforms later on, we could remove the > printing or make it configurable ). > > Additionally the "chatty" output is used currently for eliminating unused > functions + datacross-platform > (see for example https://bugs.openjdk.java.net/browse/JDK-8234629 ). > > Best regards, Matthias > > > > > > > Hello Matthias, > > > > This looks like an interesting change. If you want to enable this for > > s390x, I'm ok with it. If it works out well, perhaps we can find a way > > to expand this to other architectures. > > > > Do you really want to set --print-gc-sections by default? I would assume > > that makes the build quite chatty. > > > > /Erik > > > > On 2019-11-21 00:54, Baesken, Matthias wrote: > > > Hello, > > > > > > gcc and ld can be instructed to work together to "garbage collect" unused > > input sections. > > > This feature eliminates unused code from native libraries. As a > prerequisite > > to take full advantage of the feature, > > > the source files need to be compiled with "-ffunction-sections -fdata- > > sections". > > > > > > Details on what happens can be found in the ld documentation: > > https://linux.die.net/man/1/ld . > > > See the description of --gc-sections and --print-gc-sections therein. > > > For more detailed insights there is a talk available from ELC2010: > > https://elinux.org/images/2/2d/ELC2010-gc-sections_Denys_Vlasenko.pdf > > > > > > My change enables the unused code elimination on linux s390x . > > > (on the other Linux platforms, there are still issues to be solved with > > > the > > serviceability agent, but we do not have the serviceability agent on linux > > s390x). > > > > > > The change has 2 benefits : > > > - native libs with unused code get smaller (some get alot smaller) > > > some example lib sizes from linuxs390x (product build) : > > > default settings / link-time gc-sections > > > libmlib_image.so556K 536K > > > libjavajpeg.so 300K 292K > > > libsplashscreen.so 412K 268K > > > libfontmanager.so 1.4M 864K > > > libjvm.so19M 17M > > > > > > - the flag --print-gc-sections outputs the removed sections when calling > > the linker; > > > this helps a lot to find coding "waiting for" cross-platform removal. > > > > > > > > > Here is an example output of --print-gc-sections for the libnet-build > > > (linux > > s390x) : > > > > > > /bin/ld: Removing unused section '.bss.my_gconf_init_func' in file > > '/builddir/support/native/java.base/libnet/DefaultProxySelector.o' <--- > > seems to be dead > > > /bin/ld: Removing unused section '.text.NET_ReadV' in file > > '/builddir/support/native/java.base/libnet/linux_close.o' > > <--- > seems > > to be dead, I requested cross-platform removal with > > https://bugs.openjdk.java.net/browse/JDK-8234501 > > > /bin/ld: Removing unused section '.text.getInet6Address_scopeifname' > in > > file '/builddir/support/native/java.base/libnet/net_util.o'<--- seems to > be > > dead > > > /bin/ld: Removing unused section '.text.getInet6Address_scopeid_set' in > > file '/builddir/support/native/java.base/libnet/net_util.o'<--- seems to > be > > dead > > > /bin/ld: Removing unused section '.text.getInetAddress_hostName' in > file > > '/builddir/support/native/java.base/libnet/net_util.o'<--- seems to > > be > > dead > > > /bin/ld: Removing unused section '.text.setDefaultScopeID' in file > > '/builddir/support/native/java.base/libnet/net_util_md.o' <--- > > seems
RE: RFR [XS]: 8234795: ix build fails on macOS lower than 10.13 after 8214578 was: build fails on macOS 10.12 after 8214578: [macos] Problem with backslashes on macOS/JIS keyboard: Java ignores system
Hi Matthias, the change looks good. Thanks for fixing this. Best regards, Goetz. > -Original Message- > From: Prasanta Sadhukhan > Sent: Dienstag, 26. November 2019 10:26 > To: Baesken, Matthias ; Erik Joelsson > ; 'build-dev@openjdk.java.net' d...@openjdk.java.net> > Cc: 2d-...@openjdk.java.net; Lindenmaier, Goetz > > Subject: Re: RFR [XS]: 8234795: ix build fails on macOS lower than 10.13 after > 8214578 was: build fails on macOS 10.12 after 8214578: [macos] Problem with > backslashes on macOS/JIS keyboard: Java ignores system settings > > I have already raised a similar fix sometime back for JDK-8234786 > > Regards > > Prasanta > > On 26-Nov-19 2:49 PM, Baesken, Matthias wrote: > > Hello, here is a small adjustment that uses the typealias for > NSTextInputSourceIdentifier . This fixes the build on macOS < 10.13 . > > > > > > Bug/webrev : > > > > https://bugs.openjdk.java.net/browse/JDK-8234795 > > > > > > http://cr.openjdk.java.net/~mbaesken/webrevs/8234795.0/ > > > > > > Thanks, Matthias > > > > > >> If there is a simple fix, I would very much like to see it done. I'm not > >> familiar enough with this area to know what the implications would be > >> though. > >> > >> /Erik > >> > >> On 2019-11-25 04:48, Baesken, Matthias wrote: > >>> Hello, any comments on the issue ? > >>> > >>> Could we maybe switch from using > >>> > >>> NSTextInputSourceIdentifier > >>> > >>> to > >>> > >>> String (NSString* ?) , because > >> https://developer.apple.com/documentation/appkit/nstextinputsourceiden > >> tifier > >>> says NSTextInputSourceIdentifieris a typealias for String ? > >>> > >>> Best regards ,Matthias > >>> > >>> > >>> > >>> > >>> Hello, I noticed that since today our jdk/jdk build fails on macOS . > >>> We run > >> it on macOS 10.12 . > >>> It seems > >>> https://hg.openjdk.java.net/jdk/jdk/rev/d0bfaae2ff33 > >>> > >>> 8214578: [macos] Problem with backslashes on macOS/JIS keyboard: Java > >> ignores system settings > >>> Brought a dependency on 10.13. Was that intended ? Could we keep > 10.12 > >> compatibility ? > >>> At least the doc of NSTextInputSourceIdentifier : > >> https://developer.apple.com/documentation/appkit/nstextinputsourceiden > >> tifier > >>> mentions macOS 10.13+ . > >>> > >>> > >>> > >>> Build errors are : > >>> > >>> > >>> /jdk/src/java.desktop/macosx/native/libawt_lwawt/awt/AWTView.h:41:5: > >> error: unknown type name 'NSTextInputSourceIdentifier' > >>> NSTextInputSourceIdentifier kbdLayout; > >>> ^ > >>> > >> > /jdk/src/java.desktop/macosx/native/libawt_lwawt/awt/AWTView.m:93:23: > >> error: assignment to readonly property > >>> self.cglLayer = windowLayer; > >>> ~ ^ ~~~ > >>> > >> /jdk/src/java.desktop/macosx/native/libawt_lwawt/awt/AWTView.m:110:1 > >> 9: error: assignment to readonly property > >>> self.cglLayer = nil; > >>> ~ ^ ~~~ > >>> 3 errors generated. > >>> > >>> > >>> ... > >>> > >> /jdk/src/java.desktop/macosx/native/libawt_lwawt/awt/AWTWindow.m:45 > >> 4:18: error: incompatible pointer to integer conversion initializing 'BOOL' > (aka > >> 'signed char') with an expression of type 'id' [-Werror,-Wint-conversion] > >>> BOOL mouseIsOver = [[window contentView] mouseIsOver]; > >>>^ ~~ > >>> 2 errors generated. > >>> > >>> > >>> > >>> Best regards, Matthias
Re: RFR [XS]: 8234795: ix build fails on macOS lower than 10.13 after 8214578 was: build fails on macOS 10.12 after 8214578: [macos] Problem with backslashes on macOS/JIS keyboard: Java ignores system
I have already raised a similar fix sometime back for JDK-8234786 Regards Prasanta On 26-Nov-19 2:49 PM, Baesken, Matthias wrote: Hello, here is a small adjustment that uses the typealias for NSTextInputSourceIdentifier . This fixes the build on macOS < 10.13 . Bug/webrev : https://bugs.openjdk.java.net/browse/JDK-8234795 http://cr.openjdk.java.net/~mbaesken/webrevs/8234795.0/ Thanks, Matthias If there is a simple fix, I would very much like to see it done. I'm not familiar enough with this area to know what the implications would be though. /Erik On 2019-11-25 04:48, Baesken, Matthias wrote: Hello, any comments on the issue ? Could we maybe switch from using NSTextInputSourceIdentifier to String (NSString* ?) , because https://developer.apple.com/documentation/appkit/nstextinputsourceiden tifier says NSTextInputSourceIdentifieris a typealias for String ? Best regards ,Matthias Hello, I noticed that since today our jdk/jdk build fails on macOS . We run it on macOS 10.12 . It seems https://hg.openjdk.java.net/jdk/jdk/rev/d0bfaae2ff33 8214578: [macos] Problem with backslashes on macOS/JIS keyboard: Java ignores system settings Brought a dependency on 10.13. Was that intended ? Could we keep 10.12 compatibility ? At least the doc of NSTextInputSourceIdentifier : https://developer.apple.com/documentation/appkit/nstextinputsourceiden tifier mentions macOS 10.13+ . Build errors are : /jdk/src/java.desktop/macosx/native/libawt_lwawt/awt/AWTView.h:41:5: error: unknown type name 'NSTextInputSourceIdentifier' NSTextInputSourceIdentifier kbdLayout; ^ /jdk/src/java.desktop/macosx/native/libawt_lwawt/awt/AWTView.m:93:23: error: assignment to readonly property self.cglLayer = windowLayer; ~ ^ ~~~ /jdk/src/java.desktop/macosx/native/libawt_lwawt/awt/AWTView.m:110:1 9: error: assignment to readonly property self.cglLayer = nil; ~ ^ ~~~ 3 errors generated. ... /jdk/src/java.desktop/macosx/native/libawt_lwawt/awt/AWTWindow.m:45 4:18: error: incompatible pointer to integer conversion initializing 'BOOL' (aka 'signed char') with an expression of type 'id' [-Werror,-Wint-conversion] BOOL mouseIsOver = [[window contentView] mouseIsOver]; ^ ~~ 2 errors generated. Best regards, Matthias
RFR [XS]: 8234795: ix build fails on macOS lower than 10.13 after 8214578 was: build fails on macOS 10.12 after 8214578: [macos] Problem with backslashes on macOS/JIS keyboard: Java ignores system se
Hello, here is a small adjustment that uses the typealias for NSTextInputSourceIdentifier . This fixes the build on macOS < 10.13 . Bug/webrev : https://bugs.openjdk.java.net/browse/JDK-8234795 http://cr.openjdk.java.net/~mbaesken/webrevs/8234795.0/ Thanks, Matthias > > If there is a simple fix, I would very much like to see it done. I'm not > familiar enough with this area to know what the implications would be > though. > > /Erik > > On 2019-11-25 04:48, Baesken, Matthias wrote: > > Hello, any comments on the issue ? > > > > Could we maybe switch from using > > > > NSTextInputSourceIdentifier > > > > to > > > > String (NSString* ?) , because > https://developer.apple.com/documentation/appkit/nstextinputsourceiden > tifier > > > > says NSTextInputSourceIdentifieris a typealias for String ? > > > > Best regards ,Matthias > > > > > > > > > > Hello, I noticed that since today our jdk/jdk build fails on macOS . We > > run > it on macOS 10.12 . > > > > It seems > > https://hg.openjdk.java.net/jdk/jdk/rev/d0bfaae2ff33 > > > > 8214578: [macos] Problem with backslashes on macOS/JIS keyboard: Java > ignores system settings > > > > Brought a dependency on 10.13. Was that intended ? Could we keep 10.12 > compatibility ? > > At least the doc of NSTextInputSourceIdentifier : > https://developer.apple.com/documentation/appkit/nstextinputsourceiden > tifier > > mentions macOS 10.13+ . > > > > > > > > Build errors are : > > > > > > /jdk/src/java.desktop/macosx/native/libawt_lwawt/awt/AWTView.h:41:5: > error: unknown type name 'NSTextInputSourceIdentifier' > > NSTextInputSourceIdentifier kbdLayout; > > ^ > > > /jdk/src/java.desktop/macosx/native/libawt_lwawt/awt/AWTView.m:93:23: > error: assignment to readonly property > > self.cglLayer = windowLayer; > > ~ ^ ~~~ > > > /jdk/src/java.desktop/macosx/native/libawt_lwawt/awt/AWTView.m:110:1 > 9: error: assignment to readonly property > > self.cglLayer = nil; > > ~ ^ ~~~ > > 3 errors generated. > > > > > > ... > > > /jdk/src/java.desktop/macosx/native/libawt_lwawt/awt/AWTWindow.m:45 > 4:18: error: incompatible pointer to integer conversion initializing 'BOOL' > (aka > 'signed char') with an expression of type 'id' [-Werror,-Wint-conversion] > > BOOL mouseIsOver = [[window contentView] mouseIsOver]; > > ^ ~~ > > 2 errors generated. > > > > > > > > Best regards, Matthias
Re: RFR: 8234535: Cross compilation fails due to missing CFLAGS for the BUILD_CC
Hi Erik, thanks for looking into this. I created a new webrev which includes the changeset and you as a reviewer: http://cr.openjdk.java.net/~cgo/8234535/webrev.02/ Could you please sponsor this changeset and commit it into the repository for me? Thanks, Christoph Erik Joelsson wrote on 2019-11-22 17:19:08: > From: Erik Joelsson > To: christoph.goettsch...@microdoc.com, build-dev@openjdk.java.net > Date: 2019-11-22 17:19 > Subject: Re: RFR: 8234535: Cross compilation fails due to missing > CFLAGS for the BUILD_CC > > Hello Christoph, > > On 2019-11-20 10:22, christoph.goettsch...@microdoc.com wrote: > > Hi, > > > > please review the following change. > > > > Bug: https://bugs.openjdk.java.net/browse/JDK-8234535 > > Webrev: https://cr.openjdk.java.net/~cgo/8234535/webrev.00/ > > > > The removed lines are completely out of place and have no use at all. for > > the CC variable, for instance, the script does the following: > > > > CC_OLD="$CC" > > CC="$BUILD_CC" > > CC="$CC_OLD" > > > > without executing any function between those lines. I think that, while > > restructuring the build system, those lines have been copied to the wrong > > location. I copied them around the "FLAGS_SETUP_CFLAGS_CPU_DEP" call for > > the BUILD_CC, in order to make configure not use the target CFLAGS while > > discovering possible CFLAGS for the BUILD_CC, which was happening in our > > case. > Wow, I had not noticed that. Your fix looks correct. > > I am also not sure if the variable BUILD_CC_DISABLE_WARNING_PREFIX is > > actually used anywhere. I could not find any user and it might be possible > > to simply remove it. > It's used to override DISABLE_WARNING_PREFIX in buildjdk-spec.gmk so I > think it serves a purpose, even if we don't actually support different > toolchain types for cross and native compiler. If we ever will, then > this variable will be needed, so I think it should stay. > > I tried the change and compiled on an amd64 linux machine for amd64, and > > cross-compiled for linux on armv7 and linux on aarch64. I don't have > > access to other cross-compilation environments and would like to ask > > others to review and try out the change. > > I applied the patch and tried a few of our configurations. Since we use > the same (or close to the same) compiler versions across architectures, > I don't actually see any difference in the generated spec files. The > reason you see this is the large version difference between your compilers. > > Looks good, thanks for finding and fixing this! > > /Erik > > > Thanks, > > Christoph > > >
Re: binary Hardening on linux using Relocation Read-Only (relro)
* Claes Redestad: > On 2019-11-25 18:30, Florian Weimer wrote: >> That being said, relocation processing for libjvm.so adds a couple of >> milliseconds to startup, and it looks like their number is growing with >> each release. > > This piqued my interest, so I took a quick look: > > readelf --relocs libjvm.so | wc -l > > 8: 85635 > 9: 112645 > 11: 105607 > 13: 107912 > jdk/jdk: 106175 > > 9 saw a big jump, yes, but things look pretty stable since, even > improving a bit (various cleanups and feature removals..?). I see slightly higher numbers with the default build flags. The recent drop by ~1000 relocations is due to the CMS removal. > Of course improvements in this area would be most welcome (not an area > I've been paying attention to - maybe I should?) Unfortunately, I'm not aware of a good tool to gather relocation statistics with a goal towards avoiding them. Some cases may be easy changes (e.g., rewriting arrays of character strings). I suspect that quite a bit is related to C++ vtables. Thanks, Florian
