Re: zlib before 1.2.12 allows memory corruption (CVE-2018-25032)

2022-04-27 Thread Volker Simonis
Hi Bernd, Vitaly,

Amazon Corretto [1] also includes the fixes for CVE-2018-25032. This
is our statement:

"Based upon our analysis, OpenJDK/Corretto is not affected by
CVE-2018-25032, because the zlib "memLevel" parameter is not settable
and is fixed at 8, and the usage of the Z_FIXED strategy is prevented.
With these settings there is no way to invoke the issue described in
the CVE and we only include this fix out of an abundance of caution."

You're right that the vulnerability can also be exploited without the
Z_FIXED strategy, but in that case only with memLevel set to "1" (see
[2] for more details).

Given all the currently available information, I don't think there's a
reason to worry because of CVE-2018-25032 in the context of Java.

Best regards,
Volker

[1] https://github.com/corretto/corretto-8/blob/release-8.332.08.1/CHANGELOG.md
[2] https://www.openwall.com/lists/oss-security/2022/03/28/1

On Wed, Apr 27, 2022 at 1:21 AM Bernd Eckenfels  wrote:
>
> Hello Vitaly,
>
> (Personal answer not affiliated with OpenJDK members)
>
> I had also asked about this before, but there was no answer (which is however 
> not surprising, since it is the policy of OpenJDK and Oracle to not comment 
> on unfixed security issues).
>
> My hope was, that by reporting it before the April update, the (trivial?) 
> zlib update would be merged, but it is still the old version according to the 
> source files. So it depends on build parameters and exploitability of the 
> weakness if you are still in danger (I guess:).
>
> BTW while I can understand to not publish unfixed problems, it does really 
> not do the java users a favor to not comment on generally known/published 
> problems, especially not for 2 quarters.
>
> There is however a ray of light on the horizon, I see CVE-2018-25032 fixed in 
> the Azul April Release  Notes and asume they provide the update out of band. 
> (Probably only for Windows binaries, haven’t analysed them yet)
>
> They state:
> > Our analysis shows that Azul Zulu and OpenJDK are not affected by 
> > CVE-2018-25032.
> > In OpenJDK, the Zlib "memLevel" parameter is always set to 8 and can not be 
> > changed by a
> > Java code, and the Z_FIXED strategy is permanently disabled. The CVE does 
> > not apply to Azul
> > Zulu and OpenJDK with these settings. However, Azul decided to include the 
> > corresponding
> > patch to the Zlib library in Azul products just in case someone chooses to 
> > use Zlib from Azul
> > Zulu outside of Java applications.
>
> (I am not sure of the analysis is complete I think the z_fixed was not a 
> strict requirement, but I could be wrong.)
>
> Hopefully the vulnerability group will share their finding in a few month.
>
> Gruss
> Bernd
> --
> http://bernd.eckenfels.net
> 
> Von: security-dev  im Auftrag von Vitaly 
> Provodin 
> Gesendet: Thursday, April 21, 2022 2:06:57 AM
> An: security-...@openjdk.java.net ; 
> build-dev@openjdk.java.net 
> Cc: Vitaly Provodin 
> Betreff: zlib before 1.2.12 allows memory corruption (CVE-2018-25032)
>
> Hi all,
>
> Recently we (at JetBrains) were faced with the vulnerability issue 
> CVE-2018-25032 (zlib before 1.2.12 allows memory corruption…)
> It is known that Linux, macOS builds uses system’s zlib but Windows - bundled 
> one (by default).
> On Linux and macOS users can work around the issue by installing proper zlib 
> on their systems.
> Are there any ideas for Windows? - the way building (under Cygwin!) with 
> system zlib looks unworkable in case if Cygwin is not installed on user's 
> machines.
>
> It looks like after implementing 
> https://bugs.openjdk.java.net/browse/JDK-8249963 (which also discussed here 
> https://mail.openjdk.java.net/pipermail/core-libs-dev/2020-July/067868.html) 
> the resolution of such issues can be shifted to users but what can be done 
> now?
>
> Thanks,
> Vitaly


Re: CFV: New Build Group Member: Aleksey Shipilev

2021-11-10 Thread Volker Simonis
Vote: yes


Magnus Ihse Bursie  schrieb am Mi., 10. Nov.
2021, 15:07:

> I hereby nominate Aleksey Shipilev (shade) to Membership in the Build
> Group.
>
> Aleksey is a long standing member of the OpenJDK Members Group and the
> Hotspot
> Group. He is a prolific contributor, and has committed more than 600
> changes to
> the JDK over the years. Lately, Aleksey has made a number of improvement
> to the
> build system, which is indicated by the more than 140 commits made to
> files in
> the "make" directory since 2018.
>
> Votes are due by 24 November 2021, 16:00 CET.
>
> Only current Members of the Build Group [2] are eligible
> to vote on this nomination.  Votes must be cast in the open by
> replying to this mailing list
>
> For Lazy Consensus voting instructions, see [3].
>
> /Magnus
>
> [1]
> https://github.com/openjdk/jdk/search?q=author-name%3A%22Aleksey+Shipilev%22=commits
> [2]http://openjdk.java.net/census#build
> [3]http://openjdk.java.net/groups/#member-vote
>
>


Re: RFR: 8256127: Add cross-compiled foreign architectures builds to submit workflow [v5]

2020-11-12 Thread Volker Simonis
On Thu, 12 Nov 2020 09:34:51 GMT, Aleksey Shipilev  wrote:

>> Maybe it would be possible to build hotspot only as this is the part where 
>> things often break? This would save a lot of CPU time.
>> E.g. I think it'd be valueable to have at least one Big Endian platform 
>> included.
>> I noticed that some Oracle people use cross builds to check their changes on 
>> these platforms. It would save a bit of their time as well.
>
>> I'm not at all sure I want to accept these changes. I understand that you 
>> care about these exotic platforms, and so do I, but adding them to the 
>> submit workflow for _all_ changes seem like the wrong way to proceed.
> 
> I understand the first reaction, but please read the updated description. It 
> includes the discussion what bugs it tries to catch, and updated cost 
> estimates. 
> 
> Submit workflow is a convenience thing for better testing coverage. Having 
> the early warnings for fixable issues in foreign architectures helps to avoid 
> follow-up churn without increasing the costs all too much. Do note that it 
> does not require contributors to fix the runtime issues in foreign 
> architectures, it only asks for passing a rather low bar of _not breaking the 
> builds_ for them, which is, in my experience, an easily achievable goal. It 
> is easily achievable because --  I am speaking from experience of fixing a 
> lot of them over the years! -- the overwhelming majority of arch-specific 
> build breakages are about silly things that are obvious from the build logs.
> 
> So I am leaving this PR open, letting others to chime in. If there is really 
> a need for a wider discussion here, I could start a thread at jdk-dev@, but 
> so far it does not look as a good use of our collective time. Please read 
> through the description, and see if that soothes your concerns.
> 
> Aside: the issues about costs were [raised 
> before](https://mail.openjdk.java.net/pipermail/skara-dev/2020-October/003581.html),
>  but I think all of them resolve as "enjoy your free tier". If we target to 
> optimize the GH action costs, IMO we should instead consider to avoid 
> triggering them on every push, as [I hinted 
> before](https://mail.openjdk.java.net/pipermail/jdk-dev/2020-September/004739.html),
>  and which I think requires better Skara integration (i.e. for `/test` 
> command). Meanwhile, this PR gives us extended build coverage at fraction of 
> the additional cost.

I think this is a really useful feature and I'm a little surprised that it gets 
rejected. Please find my comments inline.

> Aleksey,
> 
> I'm not at all sure I want to accept these changes. I understand that you 
> care about these exotic platforms, and so do I, but adding them to the submit 
> workflow for _all_ changes seem like the wrong way to proceed.
> 
> My concern vary all the way from increased build time, to increased usage of 
> the Github CPU quota (which, as I understand it, is fairly large but not 
> unlimited), to the elephant in the room: the question on who the burden lays 
> to fix bugs in these exotic platforms. It is not at all obvious that these 
> builds should be tested before commit.
> 

I think "on who's the burden to fix issues" on *exotic* platforms and "testing 
changes on all platforms" at  commit time are two totally distinct questions. 
While I agree that problems on a *exotic* platform shouldn't block reviewed 
changes for an unreasonable amount of time, I also think it is very valuable 
for both, authors of changes and maintainers of *exotic* platforms to at least 
get early notifications of problems on other platforms.

My experience as a maintainer of the AIX7PowerPC/s390 ports has always been 
very positive with regards to the collaboration with authors of changes which 
impact these ports. Authors are anxious to not break other platforms but 
usually don't have the possibility to test on these platforms. Aleksey's change 
comes in very handy here. It will help authors to fix trivial build problems 
right in their initial submission which I'm sure they'll appreciate. For more 
complex problems it gives Authors the possibility to notify port maintainers 
early, before a change gets pushed. I'm sure this alerting could even be 
automatized in the future if builds/tests fail on certain platforms.

Of course such problems shouldn't block changes which are otherwise reviewed 
and ready to push for an unreasonable amount of time, so I have nothing against 
making the test on such platform optional submit requirements.

But overall I think Aleksey's enhancement are a great means to improve the 
development experience and to keep the OpenJDK repos clean of trivial follow-up 
fixes.


PS: I specially emphasized *exotic platforms* because it seems to me that your 
comment implies that all not Oracle-supported platforms are considered *exotic* 
for you. I think in an open source project it should be decided by the 
community which platforms are considered *main* (or *primary*) and which 

Re: RFR: 8255128: linux x86 build failure with libJNIPoint.c [v2]

2020-11-04 Thread Volker Simonis
On Tue, 3 Nov 2020 20:12:35 GMT, Magnus Ihse Bursie  wrote:

>> Ok, will do. (FWIW, you can expand the context of the diff with the arrow 
>> buttons on the left side of the view. Above or below the line numbers)
>
> (Yes, I know.  I just didn't think that doing so would reveal anything about 
> AIX. I just wish I had gotten a bit more context automatically.)

Sorry, but I'm neither workin on AIX any more nor do I have access to an AIX 
machine :)

Pulling in @TheRealMDoerr  or @tstuefe

-

PR: https://git.openjdk.java.net/jdk/pull/1017


Re: RFC: JEP JDK-8208089: Implement C++14 Language Features

2020-03-03 Thread Volker Simonis
On Tue, Mar 3, 2020 at 10:26 AM Andrew Haley  wrote:
>
> On 3/2/20 10:46 PM, Volker Simonis wrote:
>
> > As lead of the 8 and 11 update projects you probably know best, if this fix
> > will eventually be considered for backporting and choose your means wisely
> > :)
>
> Yeah, I know. Srsly. :-)
>
> But one of the few things of which I am certain is that we must not
> allow the needs of backporting to determine the future of Java. That's
> the way of staying in the past.
>
> As Patrick Head put it, “Some people tell me that Formula 1 would be
> better if the drivers still used stick shifts, but that’s a bit like
> saying, 'isn’t it a pity we don’t still walk around in clogs!'”
>

Totally agree.

> --
> Andrew Haley  (he/him)
> Java Platform Lead Engineer
> Red Hat UK Ltd. <https://www.redhat.com>
> https://keybase.io/andrewhaley
> EAC8 43EB D3EF DB98 CC77 2FAD A5CD 6035 332F A671
>


Re: RFC: JEP JDK-8208089: Implement C++14 Language Features

2020-03-02 Thread Volker Simonis
Andrew Haley  schrieb am Mo., 2. März 2020, 13:00:

> On 11/19/18 8:39 PM, Kim Barrett wrote:
> > I don't see any benefit to making the first C++ code change that uses
> > a new feature also incorporate the needed build system changes.
> > Indeed, how does one develop that feature usage unless one has the
> > build system changes already in hand?
> >
> > It seems to me that if the documentation says one can use some new
> > language feature but the build system hasn't been updated to actually
> > support doing so, then the documentation is wrong. And recall that
> > this JEP includes making some new features available immediately.
> > (That initial list might be modified as part of this JEP discussion.)
>
> What is the status of this? I thought we'd decide, but I still see
>
>$1_CXXSTD_CXXFLAG="-std=gnu++98"
>
> in flags-cflags.m4
>
> I need std::make_unsigned in order to fix some undefined behaviour in
> HotSpot. I could implement it myself in share/metaprogramming but if
> we've agreed to allow C++14, can we please get it done now? Thx.
>

As lead of the 8 and 11 update projects you probably know best, if this fix
will eventually be considered for backporting and choose your means wisely
:)


> --
> Andrew Haley  (he/him)
> Java Platform Lead Engineer
> Red Hat UK Ltd. 
> https://keybase.io/andrewhaley
> EAC8 43EB D3EF DB98 CC77 2FAD A5CD 6035 332F A671
>
>


Re: [11u] RFR: 8240073: Fix 'test-make' build target in 11u

2020-02-28 Thread Volker Simonis
On Wed, Feb 26, 2020 at 5:21 PM Hohensee, Paul  wrote:
>
> Looks to me like the replaced store to KEYWORDS is dead (i.e., no uses of 
> KEYWORDS that I could find in make and test/make). Lgtm.

Yes, that's exactly why we need this patch :)

"ParseKeywordVariableBody" in "make/common/MakeBase.gmk" initially
used "KEYWORDS" but when it was updated by "8212028: Use run-test
makefile framework for testing in Oracle's Mach5" to use
"SINGLE_KEYWORDS" instead, these two places were forgotten.

Thanks for reviewing,
Volker

>
> Thanks,
> Paul
>
> On 2/26/20, 3:19 AM, "build-dev on behalf of Volker Simonis" 
>  
> wrote:
>
> Hi,
>
> can I please have a review for the following tiny change which fixes
> the 'test-make' build target in 11u:
>
> http://cr.openjdk.java.net/~simonis/webrevs/2020/8240073/
> https://bugs.openjdk.java.net/browse/JDK-8240073
>
> This is a tiny fix for a problem introduced by JDK-8212028 (which has
> already been downported to 11u) and fixed in jdk 12 as part of
> JDK-8210958 (which hasn't been downported yet and probably won't be
> donwported at all).
>
> Instead of smuggling this fix into another downport, it has been
> argued 
> (https://mail.openjdk.java.net/pipermail/jdk-updates-dev/2020-February/002533.html)
> that it is cleaner to create an explicit issue for fixing it in 11u.
>
> The fix is trivial (that's why it was done as part of JDK-8210958
> without an extra bug ID) and only affects running "make test-make".
>
> Thank you and best regards,
> Volker
>
>


Re: [11u] RFR: 8223678: Add Visual Studio Code workspace generation support (for native code)

2020-02-28 Thread Volker Simonis
On Wed, Feb 26, 2020 at 3:49 PM Andrew Hughes  wrote:
>
> On 26/02/2020 10:59, Volker Simonis wrote:
> > Hi,
> > I'd like to downport the support for Visual Studio Code project
> > creation to 11u. I think we will have to support 11u for quite some
> > years and it makes sense to have as good as possible tool support in
> > 11u as well:
> >
> > https://bugs.openjdk.java.net/browse/JDK-8223678
> > http://cr.openjdk.java.net/~simonis/webrevs/2020/8223678/
> >
> > This change is the third in a series of three changes (8210459,
> > 8218807, 8223678) which are required to get VS Code support in 11u.
> > The first two changes (8210459, 8218807) have already been approved
> > and pushed.
> >
> > This change only touch make files and actually only add new features
> > to the make system, so the normal build results shouldn't be affected.
> > I've tested that the build still works on Linux and Windows and "make
> > test-make" succeeds.
> >
> > This change actually creates the VS Code project files (i.e. "make
> > vscode-project-ccls"). See
> > http://hg.openjdk.java.net/jdk/jdk/raw-file/tip/doc/ide.html for more
> > details.
> >
> > It applies cleanly except the following two hunks. The first one had to
> > be moved back from "make/common/Utils.gmk" to
> > "make/common/MakeBase.gmk" where it still applies cleanly, because the
> > "RelativePath" macro is still in "MakeBase.gmk" in  jdk11u.
> >
> > ORIGINAL
> > 
> > diff -r a82a367b2d8c -r 2ae056696b15 make/common/Utils.gmk
> > --- a/make/common/Utils.gmkMon Jun 03 17:14:23 2019 -0700
> > +++ b/make/common/Utils.gmkMon Jun 03 10:28:03 2019 +0200
> > @@ -122,7 +122,8 @@
> >  # $2 - Directory to compute the relative path from
> >  RelativePath = \
> >  $(eval $1_prefix := $(call FindCommonPathPrefix, $1, $2)) \
> > -$(eval $1_dotdots := $(call DirToDotDot, $(patsubst $($(strip
> > $1)_prefix)/%, %, $2))) \
> > +$(eval $1_dotdots := $(call DirToDotDot, $(patsubst $($(strip
> > $1)_prefix)%, %, $2))) \
> > +$(eval $1_dotdots := $(if $($(strip $1)_dotdots),$($(strip
> > $1)_dotdots),.)) \
> >  $(eval $1_suffix := $(patsubst $($(strip $1)_prefix)/%, %, $1)) \
> >  $($(strip $1)_dotdots)/$($(strip $1)_suffix)
> >
> > JDK11u
> > ==
> > diff -r 8599975f5b33 make/common/MakeBase.gmk
> > --- a/make/common/MakeBase.gmkTue Feb 12 08:40:43 2019 +0100
> > +++ b/make/common/MakeBase.gmkMon Dec 23 22:10:46 2019 +0100
> > @@ -611,7 +611,8 @@
> >  # $2 - Directory to compute the relative path from
> >  RelativePath = \
> >  $(eval $1_prefix := $(call FindCommonPathPrefix, $1, $2)) \
> > -$(eval $1_dotdots := $(call DirToDotDot, $(patsubst $($(strip
> > $1)_prefix)/%, %, $2))) \
> > +$(eval $1_dotdots := $(call DirToDotDot, $(patsubst $($(strip
> > $1)_prefix)%, %, $2))) \
> > +$(eval $1_dotdots := $(if $($(strip $1)_dotdots),$($(strip
> > $1)_dotdots),.)) \
> >  $(eval $1_suffix := $(patsubst $($(strip $1)_prefix)/%, %, $1)) \
> >  $($(strip $1)_dotdots)/$($(strip $1)_suffix)
> >
> > For the second changed hunk, the simple call of the "AssertEquals"
> > macro had to be renamed to "assert-equals" and wrapped with "eval"
> > because the corresponding macros haven't been updated in jdk11u yet.
> > Notice that these are only test changes (for testing the make system
> > itself) and don't effect the build at all.
> >
> > ORIGINAL
> > 
> > diff -r a82a367b2d8c -r 2ae056696b15 test/make/TestMakeBase.gmk
> > --- a/test/make/TestMakeBase.gmkMon Jun 03 17:14:23 2019 -0700
> > +++ b/test/make/TestMakeBase.gmkMon Jun 03 10:28:03 2019 +0200
> > @@ -361,6 +361,18 @@
> >  RelativePath, \
> >  )
> >
> > +$(call AssertEquals, \
> > +$(call RelativePath, /foo/bar/baz/banan/kung, /foo/bar/baz), \
> > +./banan/kung, \
> > +RelativePath, \
> > +)
> > +
> > +$(call AssertEquals, \
> > +$(call RelativePath, /foo/bar/baz/banan/kung, /foo/bar/baz/), \
> > +./banan/kung, \
> > +RelativePath, \
> > +)
> >
> > JDK11u
> > ==
> > diff -r 8599975f5b33 test/make/TestMakeBase.gmk
> > --- a/test/make/TestMakeBase.gmkTue Feb 12 08:40:43 2019 +0100
> > +++ b/test/make/TestMakeBase.gmkMon Dec 23 22:10:46 2019 +0100
> > @@ -325,13 +325,25 @@
> >  RelativePath, \
> >  ))
> >
> > +$(eval $(call

Re: [RFR] [11u] JDK-8232748: "Build static versions of certain JDK libraries"

2020-02-28 Thread Volker Simonis
On Thu, Feb 27, 2020 at 3:44 PM Severin Gehwolf  wrote:
>
> Hi Andrew,
>
> On Thu, 2020-02-27 at 04:52 +, Andrew Hughes wrote:
> > Bug: https://bugs.openjdk.java.net/browse/JDK-8232748
> > Webrev: https://cr.openjdk.java.net/~andrew/openjdk11/8232748/webrev.01/
> >
> > This patch adds targets to the build so that static versions of the JDK
> > native libraries can be built, using static-libs-image. Such static
> > versions of the libraries are required for consumption by Graal.
> >
> > With JDK-8210459 now in 11u, this is a largely clean backport, aside
> > from some context changes, due to additional targets (JCov, JMH) being
> > present in trunk:
> >
> > * In make/Bundles.gmk, 11u does not have jcov-bundles
> > * In make/Main.gmk, 11u does not have jcov-image or jcov-bundles
> > * In make/autoconf/spec.gmk.in, 11u does not have JMH_CORE_JAR, etc or
> > JCOV_BUNDLE_NAME.
> > * In make/conf/jib-profiles.js, the dependencies list in 11u doesn't
> > include jmh and jcov.
> >
> > Building the new target, static-libs-image, succeeded. This should have
> > no effect on other targets.
>
> Unfortunately this patch doesn't work. While a build of static-libs-
> image succeeds, it doesn't create the image in build/ name>/images. Expected is this:
>
> $ find build/linux-x86_64-normal-server-release/images/static-libs/
> build/linux-x86_64-normal-server-release/images/static-libs/
> build/linux-x86_64-normal-server-release/images/static-libs/lib
> build/linux-x86_64-normal-server-release/images/static-libs/lib/libj2pkcs11.a
> build/linux-x86_64-normal-server-release/images/static-libs/lib/libj2pcsc.a
> build/linux-x86_64-normal-server-release/images/static-libs/lib/libnio.a
> build/linux-x86_64-normal-server-release/images/static-libs/lib/libprefs.a
> build/linux-x86_64-normal-server-release/images/static-libs/lib/libjava.a
> build/linux-x86_64-normal-server-release/images/static-libs/lib/libjli.a
> build/linux-x86_64-normal-server-release/images/static-libs/lib/libnet.a
> build/linux-x86_64-normal-server-release/images/static-libs/lib/libjimage.a
> build/linux-x86_64-normal-server-release/images/static-libs/lib/libjaas.a
> build/linux-x86_64-normal-server-release/images/static-libs/lib/libfdlibm.a
> build/linux-x86_64-normal-server-release/images/static-libs/lib/libj2gss.a
> build/linux-x86_64-normal-server-release/images/static-libs/lib/libsunec.a
> build/linux-x86_64-normal-server-release/images/static-libs/lib/libjsig.a
> build/linux-x86_64-normal-server-release/images/static-libs/lib/libextnet.a
> build/linux-x86_64-normal-server-release/images/static-libs/lib/libverify.a
> build/linux-x86_64-normal-server-release/images/static-libs/lib/libzip.a
>
> The reason for this is that FindFiles isn't available in JDK 11u. I had
> to do these modifications to your patch to make it work:
>
> diff --git a/make/Bundles.gmk b/make/Bundles.gmk
> --- a/make/Bundles.gmk
> +++ b/make/Bundles.gmk
> @@ -367,7 +367,7 @@
>  
> 
>
>  ifneq ($(filter static-libs-bundles, $(MAKECMDGOALS)), )
> -  STATIC_LIBS_BUNDLE_FILES := $(call FindFiles, $(STATIC_LIBS_IMAGE_DIR))
> +  STATIC_LIBS_BUNDLE_FILES := $(shell $(FIND) $(STATIC_LIBS_IMAGE_DIR))
>
>ifeq ($(OPENJDK_TARGET_OS)-$(DEBUG_LEVEL), macosx-release)
>  STATIC_LIBS_BUNDLE_SUBDIR := $(JDK_MACOSX_CONTENTS_SUBDIR)/Home
> diff --git a/make/StaticLibsImage.gmk b/make/StaticLibsImage.gmk
> --- a/make/StaticLibsImage.gmk
> +++ b/make/StaticLibsImage.gmk
> @@ -42,7 +42,7 @@
>SRC := $(SUPPORT_OUTPUTDIR)/native/$m, \
>DEST := $(STATIC_LIBS_IMAGE_DIR)/lib, \
>FILES := $(filter %$(STATIC_LIBRARY_SUFFIX), \
> -  $(call FindFiles, $(SUPPORT_OUTPUTDIR)/native/$m/*/static)), \
> +  $(shell $(FIND) $(SUPPORT_OUTPUTDIR)/native/$m/*/static)), \
>)) \
>$(eval TARGETS += $$(COPY_STATIC_LIBS_$m)) \
>  )
>
> This uses FIND directly, but there ought to be a (convoluted?) way to
> use CacheFind instead of FindFiles in 11u. Maybe build-dev folks could
> help there?
>

Hi Severin, Andrew,

there's no magic behind calling "CacheFind", you can just replace
FindFiles by CacheFind. I've just did it and a test build with "make
static-libs-image" resulted in the following output which seems to be
correct:

$ find images/static-libs/
images/static-libs/
images/static-libs/lib
images/static-libs/lib/libjaas.a
images/static-libs/lib/libnet.a
images/static-libs/lib/libzip.a
images/static-libs/lib/libfdlibm.a
images/static-libs/lib/libprefs.a
images/static-libs/lib/libnio.a
images/static-libs/lib/libextnet.a
images/static-libs/lib/libj2pcsc.a
images/static-libs/lib/libjimage.a
images/static-libs/lib/libsunec.a
images/static-libs/lib/libjava.a
images/static-libs/lib/libjsig.a
images/static-libs/lib/libj2gss.a
images/static-libs/lib/libverify.a
images/static-libs/lib/libjli.a
images/static-libs/lib/libj2pkcs11.a

I've uploaded the updated webrev for your convenience to:


Re: [11u] RFR: 8210459, 8218807, 8223678: Support for creating Visual Studio Code projects

2020-02-26 Thread Volker Simonis
On Sat, Feb 22, 2020 at 12:08 AM Langer, Christoph 
wrote:

> Hi,
>
> Thanks, Volker for backporting this to JDK11 and thanks Andrew for
> reviewing it.
>
> > On 30/12/2019 20:18, Volker Simonis wrote:
> > > Hi,
> > > I'd like to downport the support for Visual Studio Code project
> > > creation to 11u. I think we will have to support 11u for quite some
> > > years and it makes sense to have as good as possible tool support in
> > > 11u as well.
> >
> > I just came across this proposal in the process of backporting
> > JDK-8232748: "Build static versions of certain JDK libraries". The only
> > hunk of that changeset which isn't a simple context difference is that
> > which applies to JdkNativeCompilation.gmk and the FindLib &
> > FindStaticLib rules it introduces.
> >
> > I was going to just drop this hunk, as I didn't see any reason to
> > backport JDK-8210459, but then I saw your existing RFR referenced on the
> > bug. If JDK-8210459 is to be backported, I thus need this in place
> > before posting JDK-8232748 for review.
>
> Thanks for waiting, Andrew. I'll approve JDK-8210459 now in order to get
> it pushed to unblock your work on JDK-8210459.
>

Thanks, pushed.

>
> > > Finally, I've added an additional fix to this last change, whch fixes
> > > the make tests (i.e. "make test-make"). These tests are currently
> > > broken in jdk11u. They have been broken by JDK-8212028 (which has
> > > already been downported to jdk11u) and fixed in jdk 12 as part of
> > > "8210958: Rename "make run-test" to "make test"" (which hasn't been
> > > downported yet and probably won't be donwported at all). Because the
> > > fix is trivial (that's why it was done as part of 8210958 without an
> > > extra bug ID) and because I wanted to run "make test-make" in jdk11u
> > > as well to verify my changes, I've decided to downport this fix as
> > > part of 8223678:
> > >
> > > diff -r 8599975f5b33 test/make/TestMakeBase.gmk
> > > --- a/test/make/TestMakeBase.gmkTue Feb 12 08:40:43 2019 +0100
> > > +++ b/test/make/TestMakeBase.gmkMon Dec 23 22:10:46 2019 +0100
> > >  KWBASE :=
> > APA=banan;GURKA=tomat;COUNT=1%202%203%204%205;SUM=1+2+3+4+5
> > ;MANY_WORDS=I
> > > have the best words.
> > >
> > >  $(eval $(call ParseKeywordVariable, KWBASE, \
> > > -KEYWORDS := APA GURKA SUM, \
> > > +SINGLE_KEYWORDS := APA GURKA SUM, \
> > >  STRING_KEYWORDS := COUNT MANY_WORDS, \
> > >  ))
> > >
> > > @@ -364,7 +376,7 @@
> > >  KWBASE_WEIRD := ;;APA=banan;;;GURKA=apelsin;APA=skansen;;
> > >
> > >  $(eval $(call ParseKeywordVariable, KWBASE_WEIRD, \
> > > -KEYWORDS := APA GURKA SUM, \
> > > +SINGLE_KEYWORDS := APA GURKA SUM, \
> > >  STRING_KEYWORDS := COUNT, \
> > >  ))
> > >
> >
> > I agree it doesn't make sense to backport JDK-8210958, as that's a major
> > change to the test infrastructure. However, I think this warrants its
> > own 11u bug rather than being smuggled in under JDK-8223678. Otherwise,
> > you're just mirroring the same problem that was created by mixing this
> > in with JDK-8210958. An independent bug can describe the breakage and
> > link to the original issue that caused it.
>
> Hm, I think Volker's way of adding this trivial correction to the backport
> of 8223678 is okay. But, of course a separate bug could also be a good way
> to document this fix. Maybe we need a tiebreaker here? 
>

No problem. I've created "JDK-8240073: Fix 'test-make' build target in 11u"
for this issue now and sent out a RFR here:

https://mail.openjdk.java.net/pipermail/jdk-updates-dev/2020-February/002589.html

Please take a look if possible :)


> So I'll approve the series of backports now, except for JDK-8223678.
>
>
Thanks, I've pushed 8210459 and 8218807 now and  sent out a new RFR for
"JDK-8223678: Add Visual Studio Code workspace generation support (for
native code)" without the extra fix which has been moved to JDK-8240073:

https://mail.openjdk.java.net/pipermail/jdk-updates-dev/2020-February/002588.html

Please take a look if possible :)

Thank you and best regards,
Volker



> Cheers
> Christoph
>
>


Re: [11u] RFR: 8210459, 8218807, 8223678: Support for creating Visual Studio Code projects

2020-02-26 Thread Volker Simonis
On Wed, Feb 19, 2020 at 6:14 AM Andrew Hughes  wrote:
>
>
>
> On 30/12/2019 20:18, Volker Simonis wrote:
> > Hi,
> > I'd like to downport the support for Visual Studio Code project
> > creation to 11u. I think we will have to support 11u for quite some
> > years and it makes sense to have as good as possible tool support in
> > 11u as well.
>
> I just came across this proposal in the process of backporting
> JDK-8232748: "Build static versions of certain JDK libraries". The only
> hunk of that changeset which isn't a simple context difference is that
> which applies to JdkNativeCompilation.gmk and the FindLib &
> FindStaticLib rules it introduces.
>
> I was going to just drop this hunk, as I didn't see any reason to
> backport JDK-8210459, but then I saw your existing RFR referenced on the
> bug. If JDK-8210459 is to be backported, I thus need this in place
> before posting JDK-8232748 for review.
>
> >
> > This mail is especially intended to get a review from the build team
> > that the few changes to the patches are OK:
> >
> > https://bugs.openjdk.java.net/browse/JDK-8210459
> > http://cr.openjdk.java.net/~simonis/webrevs/2019/8210459/
> >
> > https://bugs.openjdk.java.net/browse/JDK-8218807
> > http://cr.openjdk.java.net/~simonis/webrevs/2019/8218807/
> >
> > https://bugs.openjdk.java.net/browse/JDK-8223678
> > http://cr.openjdk.java.net/~simonis/webrevs/2019/8223678/
> >
> > The downport consists of three changes which are additive (i.e. later
> > changes depend on earlier ones). That's why I decided to post just one
> > RFR, because looking at a single change makes no sense without the
> > corresponding context. The changes mostly apply cleanly except for
> > very few places where we need manual adjustments because of changed
> > context.
>
> I'm just going to stick to reviewing JDK-8210459 here. I feel an RFR
> should stick to just the one single change, which a reviewer can then
> apply and test on the current state of the repository. Having three
> changes in one RFR is quite overwhelming and the later changes are being
> applied against a repository state which doesn't yet exist. I suspect
> that may be one of the reasons this review has not been undertaken thus far.
>

I'm always torn between the two approaches here. On one side I
completely understand and respect your position. On the other hand, I
find it hard to judge and review backport changes which are only
required as dependencies for other backports out of their context. And
we don't have any good possibilities of somehow grouping such changes
together logically. It also increases the latency considerably,
because you can't post the second change for review before the first
one has been approved and pushed. This is exactly for the reason you
mention, that a reviewer will first have to find and apply the first
change, before he can look at the second one. Having all the related
changes in one RFR at least keeps the context together and allows a
review to apply and review them one after another.

Anyway, thanks for looking at JDK-8210459 :)

> >
> > These changes only touch make files and actually only add new features
> > to the make system, so the normal build results shouldn't be affected.
> > I've tested that the build still works on Linux and Windows and "make
> > test-make" succeeds.
> >
> > Following some details on the three changes:
> >
> > 8210459: Add support for generating compile_commands.json
> >
> > The initial change which creates the compile_commands.json file (i.e.
> > "make compile_commands"). This file can be used for many tools (e.g.
> > CLion IDE, clangd, etc..)
> > Applies cleanly except for the following hunk, which has a different 
> > context:
> >
> > ORIGINAL
> > 
> > CFLAGS_solaris := -KPIC, \
> >  CFLAGS_macosx := -fPIC, \
> >  DISABLED_WARNINGS_clang := format-nonliteral, \
> > -LDFLAGS := $(UNPACKEXE_ZIPOBJS) \
> > -$(LDFLAGS_JDKEXE) $(LDFLAGS_CXX_JDK) \
> > +LDFLAGS := $(LDFLAGS_JDKEXE) $(LDFLAGS_CXX_JDK) \
> >
> > JDK11u
> > ==
> > CFLAGS_linux := -fPIC, \
> >  CFLAGS_solaris := -KPIC, \
> >  CFLAGS_macosx := -fPIC, \
> > -LDFLAGS := $(UNPACKEXE_ZIPOBJS) \
> > -$(LDFLAGS_JDKEXE) $(LDFLAGS_CXX_JDK) \
> > +LDFLAGS := $(LDFLAGS_JDKEXE) $(LDFLAGS_CXX_JDK) \
> >
>
> This looks fine. I compared your patch and the original changeset and
> this is indeed the only difference, and only occurs because of the
> additional disabled warning for Clang.
>

Thanks. I've pushed JDK-82104

[11u] RFR: 8240073: Fix 'test-make' build target in 11u

2020-02-26 Thread Volker Simonis
Hi,

can I please have a review for the following tiny change which fixes
the 'test-make' build target in 11u:

http://cr.openjdk.java.net/~simonis/webrevs/2020/8240073/
https://bugs.openjdk.java.net/browse/JDK-8240073

This is a tiny fix for a problem introduced by JDK-8212028 (which has
already been downported to 11u) and fixed in jdk 12 as part of
JDK-8210958 (which hasn't been downported yet and probably won't be
donwported at all).

Instead of smuggling this fix into another downport, it has been
argued 
(https://mail.openjdk.java.net/pipermail/jdk-updates-dev/2020-February/002533.html)
that it is cleaner to create an explicit issue for fixing it in 11u.

The fix is trivial (that's why it was done as part of JDK-8210958
without an extra bug ID) and only affects running "make test-make".

Thank you and best regards,
Volker


[11u] RFR: 8223678: Add Visual Studio Code workspace generation support (for native code)

2020-02-26 Thread Volker Simonis
Hi,
I'd like to downport the support for Visual Studio Code project
creation to 11u. I think we will have to support 11u for quite some
years and it makes sense to have as good as possible tool support in
11u as well:

https://bugs.openjdk.java.net/browse/JDK-8223678
http://cr.openjdk.java.net/~simonis/webrevs/2020/8223678/

This change is the third in a series of three changes (8210459,
8218807, 8223678) which are required to get VS Code support in 11u.
The first two changes (8210459, 8218807) have already been approved
and pushed.

This change only touch make files and actually only add new features
to the make system, so the normal build results shouldn't be affected.
I've tested that the build still works on Linux and Windows and "make
test-make" succeeds.

This change actually creates the VS Code project files (i.e. "make
vscode-project-ccls"). See
http://hg.openjdk.java.net/jdk/jdk/raw-file/tip/doc/ide.html for more
details.

It applies cleanly except the following two hunks. The first one had to
be moved back from "make/common/Utils.gmk" to
"make/common/MakeBase.gmk" where it still applies cleanly, because the
"RelativePath" macro is still in "MakeBase.gmk" in  jdk11u.

ORIGINAL

diff -r a82a367b2d8c -r 2ae056696b15 make/common/Utils.gmk
--- a/make/common/Utils.gmkMon Jun 03 17:14:23 2019 -0700
+++ b/make/common/Utils.gmkMon Jun 03 10:28:03 2019 +0200
@@ -122,7 +122,8 @@
 # $2 - Directory to compute the relative path from
 RelativePath = \
 $(eval $1_prefix := $(call FindCommonPathPrefix, $1, $2)) \
-$(eval $1_dotdots := $(call DirToDotDot, $(patsubst $($(strip
$1)_prefix)/%, %, $2))) \
+$(eval $1_dotdots := $(call DirToDotDot, $(patsubst $($(strip
$1)_prefix)%, %, $2))) \
+$(eval $1_dotdots := $(if $($(strip $1)_dotdots),$($(strip
$1)_dotdots),.)) \
 $(eval $1_suffix := $(patsubst $($(strip $1)_prefix)/%, %, $1)) \
 $($(strip $1)_dotdots)/$($(strip $1)_suffix)

JDK11u
==
diff -r 8599975f5b33 make/common/MakeBase.gmk
--- a/make/common/MakeBase.gmkTue Feb 12 08:40:43 2019 +0100
+++ b/make/common/MakeBase.gmkMon Dec 23 22:10:46 2019 +0100
@@ -611,7 +611,8 @@
 # $2 - Directory to compute the relative path from
 RelativePath = \
 $(eval $1_prefix := $(call FindCommonPathPrefix, $1, $2)) \
-$(eval $1_dotdots := $(call DirToDotDot, $(patsubst $($(strip
$1)_prefix)/%, %, $2))) \
+$(eval $1_dotdots := $(call DirToDotDot, $(patsubst $($(strip
$1)_prefix)%, %, $2))) \
+$(eval $1_dotdots := $(if $($(strip $1)_dotdots),$($(strip
$1)_dotdots),.)) \
 $(eval $1_suffix := $(patsubst $($(strip $1)_prefix)/%, %, $1)) \
 $($(strip $1)_dotdots)/$($(strip $1)_suffix)

For the second changed hunk, the simple call of the "AssertEquals"
macro had to be renamed to "assert-equals" and wrapped with "eval"
because the corresponding macros haven't been updated in jdk11u yet.
Notice that these are only test changes (for testing the make system
itself) and don't effect the build at all.

ORIGINAL

diff -r a82a367b2d8c -r 2ae056696b15 test/make/TestMakeBase.gmk
--- a/test/make/TestMakeBase.gmkMon Jun 03 17:14:23 2019 -0700
+++ b/test/make/TestMakeBase.gmkMon Jun 03 10:28:03 2019 +0200
@@ -361,6 +361,18 @@
 RelativePath, \
 )

+$(call AssertEquals, \
+$(call RelativePath, /foo/bar/baz/banan/kung, /foo/bar/baz), \
+./banan/kung, \
+RelativePath, \
+)
+
+$(call AssertEquals, \
+$(call RelativePath, /foo/bar/baz/banan/kung, /foo/bar/baz/), \
+./banan/kung, \
+RelativePath, \
+)

JDK11u
==
diff -r 8599975f5b33 test/make/TestMakeBase.gmk
--- a/test/make/TestMakeBase.gmkTue Feb 12 08:40:43 2019 +0100
+++ b/test/make/TestMakeBase.gmkMon Dec 23 22:10:46 2019 +0100
@@ -325,13 +325,25 @@
 RelativePath, \
 ))

+$(eval $(call assert-equals, \
+$(call RelativePath, /foo/bar/baz/banan/kung, /foo/bar/baz), \
+./banan/kung, \
+RelativePath, \
+))
+
+$(eval $(call assert-equals, \
+$(call RelativePath, /foo/bar/baz/banan/kung, /foo/bar/baz/), \
+./banan/kung, \
+RelativePath, \
+))


Re: RFR [jdk11]: 8201349: build broken when configured with --with-zlib=bundled on gcc 7.3

2020-02-21 Thread Volker Simonis
Looks good!

Thanks,
Volker

On Fri, Feb 21, 2020 at 2:50 PM Baesken, Matthias
 wrote:
>
> Hello, please review this small downport of  8201349 .
> Reason is that we run into  warnings as errors, when building  jdk11 on Linux 
> with gcc-7.X  and  with-zlib=bundled .
> This adds  the  compiler options
>
> -Wno-unused-function -Wno-implicit-fallthrough
>
> To some compilations e.g. zlib-files   ; these 2 flags are at least supported 
> with gcc-4.7 , gcc-4.8 ,gcc-7 and gcc-8 .
>
>
>
> Bug :
>
> https://bugs.openjdk.java.net/browse/JDK-8201349
>
> jdk/jdk webrev :
>
> https://hg.openjdk.java.net/jdk/jdk/rev/c35eac313084
>
>
> adjusted Jdk11 webrev :
>
> http://cr.openjdk.java.net/~mbaesken/webrevs/8201349.0_jdk11/
>
>
> Thanks, Matthias


Re: RFR: JDK-8201349 build broken when configured with --with-zlib=bundled on gcc 7.3

2020-02-09 Thread Volker Simonis
I've also seen this recently but haven't managed to file a bug report yet.
If I remember correctly, I had to exclude two warnings, but that can also
be because I've experimented with different libz version.

The one proposed here is definitely required. So the change looks good to
me.

Erik Joelsson  schrieb am Mi., 5. Feb. 2020,
15:26:

> Looks good.
>
> /Erik
>
> On 2020-02-05 03:33, Magnus Ihse Bursie wrote:
> > From the bug report:
> >
> > "I'm seeing errors like this now:
> >
> > /ws/dev1/open/src/java.base/share/native/libzip/zlib/inflate.c:766:25:
> > error: this statement may fall through [-Werror=implicit-fallthrough=]
> >state->mode = EXLEN;
> >
> > It suspect it is related to the new gcc 7.3 devkit combined with
> > --with-zlib=bundled."
> >
> > The patch belows disables the warning. It is needed twice, since not
> > only libzip but also libjli includes the bundleed zlib code.
> >
> > Bug: https://bugs.openjdk.java.net/browse/JDK-8201349
> > Patch inline:
> > diff --git a/make/lib/CoreLibraries.gmk b/make/lib/CoreLibraries.gmk
> > --- a/make/lib/CoreLibraries.gmk
> > +++ b/make/lib/CoreLibraries.gmk
> > @@ -144,7 +144,7 @@
> >  CFLAGS := $(CFLAGS_JDKLIB) \
> >  $(LIBZ_CFLAGS), \
> >  CFLAGS_unix := $(BUILD_LIBZIP_MMAP) -UDEBUG, \
> > -DISABLED_WARNINGS_gcc := unused-function, \
> > +DISABLED_WARNINGS_gcc := unused-function implicit-fallthrough, \
> >  LDFLAGS := $(LDFLAGS_JDKLIB) \
> >  $(call SET_SHARED_LIBRARY_ORIGIN), \
> >  LIBS_unix := -ljvm -ljava $(LIBZ_LIBS), \
> > @@ -210,7 +210,7 @@
> >  EXTRA_FILES := $(LIBJLI_EXTRA_FILES), \
> >  OPTIMIZATION := HIGH, \
> >  CFLAGS := $(CFLAGS_JDKLIB) $(LIBJLI_CFLAGS), \
> > -DISABLED_WARNINGS_gcc := unused-function, \
> > +DISABLED_WARNINGS_gcc := unused-function implicit-fallthrough, \
> >  DISABLED_WARNINGS_clang := sometimes-uninitialized
> > format-nonliteral, \
> >  LDFLAGS := $(LDFLAGS_JDKLIB) \
> >  $(call SET_SHARED_LIBRARY_ORIGIN), \
> >
> > /Magnus
>


Re: serviceability agent : problems when using gcc LTO (link time optimization)

2020-01-15 Thread Volker Simonis
Aleksei, Matthias,

thanks for the numbers. The size reduction on libjvm.so looks not bad,
indeed.

Do you know if newer versions of GCC use the gold linker by default? I
remember from some experiments which I did many years ago that gold was
considerably faster compared to the default ld linker.

Unfortunately, the documentation I found about LTO/ld/gold [1,2] seems to
be quite old and not very precise. Do you have gained any experience with
LTO/gold and know if gold could maybe improve linking times with LTO?

[1] https://gcc.gnu.org/wiki/LinkTimeOptimization
[2] https://stackoverflow.com/questions/31688069/requirements-to-use-flto


Baesken, Matthias  schrieb am Mi., 15. Jan. 2020,
07:02:

> Hello , I can comment on   the  code size .  This is what I get when
> comparing  a build  without  and  with  -flto .
>
>
>
> gcc7 linux x86_64  product build, normal / with -flto
>
>
> --
>
>
>
> du -sh on the *.so files gives :
>
>
>
> 16K / 16K  ./lib/libattach.so
>
> 48K / 44K  ./lib/libawt_headless.so
>
> 752K / 760K./lib/libawt.so<-- this one
> gets a bit larger with flto
>
> 472K / 456K./lib/libawt_xawt.so   <-- small gain
>
> 36K / 32K  ./lib/libdt_socket.so
>
> 16K /16K   ./lib/libextnet.so
>
> 1.5M / 824K./lib/libfontmanager.so <-- HUGE gain
>
> 784K / 792K./lib/libfreetype.so<-- this one
> gets a bit larger with flto
>
> 56K / 56K  ./lib/libinstrument.so
>
> 52K / 52K  ./lib/libj2gss.so
>
> 20K / 20K  ./lib/libj2pcsc.so
>
> 92K / 84K  ./lib/libj2pkcs11.so
>
> 12K / 12k  ./lib/libjaas.so
>
> 260K / 244K./lib/libjavajpeg.so <- small gain
>
> 196K / 188K./lib/libjava.so
>
> 12K / 12K  ./lib/libjawt.so
>
> 280K / 256K./lib/libjdwp.so <- small gain
>
> 144K / 140K./lib/libjimage.so
>
> 84K / 76K  ./lib/libjli.so
>
> 16K / 16K  ./lib/libjsig.so
>
> 88K / 80K  ./lib/libjsound.so
>
> 564K / 420K./lib/liblcms.so <- large gain
>
> 12K / 12K  ./lib/libmanagement_agent.so
>
> 40K / 36K  ./lib/libmanagement_ext.so
>
> 36K / 32K  ./lib/libmanagement.so
>
> 576K / 496K./lib/libmlib_image.so   <- large gain
>
> 112K / 108K./lib/libnet.so
>
> 100K / 100K./lib/libnio.so
>
> 16K  / 16K ./lib/libprefs.so
>
> 8.0K / 8.0K./lib/librmi.so
>
> 60K / 60K  ./lib/libsaproc.so
>
> 36K / 32K  ./lib/libsctp.so
>
> 368K / 212K./lib/libsplashscreen.so <- large gain
>
> 320K / 296K./lib/libsunec.so<- medium gain
>
> 72K / 72K  ./lib/libverify.so
>
> 44K / 44K  ./lib/libzip.so
>
> 16K / 16K  ./lib/server/libjsig.so
>
> 23M / 17M  ./lib/server/libjvm.so<- big gain
> maybe because it is C++ ?
>
>
>
>
>
> So  for  some libs  you see  10% and more , but not for all .   But  most
> large  libs  like   libjvm.so,  libfontmanager.so  or   liblcms.so
> we see good results regarding reduced code size.
>
>
>
> I Cannot say much about performance improvements , probably it would be
> small .
>
>
>
> For SPEC  you find something at
>
>
>
>
> http://hubicka.blogspot.com/2019/05/gcc-9-link-time-and-inter-procedural.html
>
>
>
> (not that these results would say too much about  JVM performance ).
>
>
>
>
>
> Best regards, Matthias
>
>
>
> *From:* Volker Simonis 
> *Sent:* Mittwoch, 15. Januar 2020 14:40
> *To:* Aleksei Voitylov 
> *Cc:* Baesken, Matthias ; Magnus Ihse Bursie <
> magnus.ihse.bur...@oracle.com>; serviceability-...@openjdk.java.net;
> build-dev ; hotspot-...@openjdk.java.net
> *Subject:* Re: serviceability agent : problems when using gcc LTO (link
> time optimization)
>
>
>
> While we are speaking about all the drawbacks of LTO, it's still not clear
> what the benefits are? In the very first mail Matthias mentioned that there
> might be performance improvements but that performance is not the main
> driving factor behind this initiative. So is it the reduced code size
> (Matthias mentioned something around ~10%)?
>
>
>
> It would be nice to see some real numbers on various platform for both,
> the performance improvements for native parts like JIT/GC as well as for
> the size reduction.
>
> Aleksei Voitylov  schrieb am Di., 14. Jan.
> 2020, 09:54:
>

Re: serviceability agent : problems when using gcc LTO (link time optimization)

2020-01-15 Thread Volker Simonis
While we are speaking about all the drawbacks of LTO, it's still not clear
what the benefits are? In the very first mail Matthias mentioned that there
might be performance improvements but that performance is not the main
driving factor behind this initiative. So is it the reduced code size
(Matthias mentioned something around ~10%)?

It would be nice to see some real numbers on various platform for both, the
performance improvements for native parts like JIT/GC as well as for the
size reduction.

Aleksei Voitylov  schrieb am Di., 14. Jan.
2020, 09:54:

>
> On 14/01/2020 19:57, Baesken, Matthias wrote:
> > Hello  Magnus and Aleksei,  thanks for the input .
> >
> > The times you  provided really look like they make a big difference  at
> least for people  often  building   minimal-vm  .
> > Guess I have to measure myself a bit  (maybe the difference is not that
> big on our linux s390x / ppc64(le) ) .
> >
> >> If the change to enable lto by default is proposed, what would be the
> >> recommended strategy for development?
> >>
> > Probably  we should a)   do not enable it by default but just make sure
> it can be enabled easily and works  for  the minimal-vm
> That would be welcome. I have high hopes to LTO the VM some time by
> default, and the tendency observed is that the compiler time overhead
> for GCC becomes smaller. At the same time there is no reason why vendors
> that invested in testing and can absorb the build time hit could provide
> binaries with LTO built VMs by passing an additional option flag.
> >   or  b)  take it easy to disable it for local development.
> >
> > Best regards, Matthias
> >
> >
> >
> >> Magnus, Matthias,
> >>
> >> for me, lto is a little heavyweight for development. x86_64 build time
> >> with gcc 7:
> >>
> >> Server 1m32.484s
> >> Server+Minimal 1m42.166s
> >> Server+Minimal (--with-jvm-features="link-time-opt") 5m29.422s
> >>
> >> If the change to enable lto by default is proposed, what would be the
> >> recommended strategy for development?
> >>
> >> For ARM32 Minimal, please keep in mind that it's not uncommon to disable
> >> LTO plugin in commodity ARM32 gcc compiler distributions, so for some it
> >> does not matter what settings we have in OpenJDK. I believe there could
> >> be other reasons for that on top of build time (bugs?).
> >>
>
>


[11u] RFR: 8210459, 8218807, 8223678: Support for creating Visual Studio Code projects

2019-12-30 Thread Volker Simonis
Hi,
I'd like to downport the support for Visual Studio Code project
creation to 11u. I think we will have to support 11u for quite some
years and it makes sense to have as good as possible tool support in
11u as well.

This mail is especially intended to get a review from the build team
that the few changes to the patches are OK:

https://bugs.openjdk.java.net/browse/JDK-8210459
http://cr.openjdk.java.net/~simonis/webrevs/2019/8210459/

https://bugs.openjdk.java.net/browse/JDK-8218807
http://cr.openjdk.java.net/~simonis/webrevs/2019/8218807/

https://bugs.openjdk.java.net/browse/JDK-8223678
http://cr.openjdk.java.net/~simonis/webrevs/2019/8223678/

The downport consists of three changes which are additive (i.e. later
changes depend on earlier ones). That's why I decided to post just one
RFR, because looking at a single change makes no sense without the
corresponding context. The changes mostly apply cleanly except for
very few places where we need manual adjustments because of changed
context.

These changes only touch make files and actually only add new features
to the make system, so the normal build results shouldn't be affected.
I've tested that the build still works on Linux and Windows and "make
test-make" succeeds.

Following some details on the three changes:

8210459: Add support for generating compile_commands.json

The initial change which creates the compile_commands.json file (i.e.
"make compile_commands"). This file can be used for many tools (e.g.
CLion IDE, clangd, etc..)
Applies cleanly except for the following hunk, which has a different context:

ORIGINAL

CFLAGS_solaris := -KPIC, \
 CFLAGS_macosx := -fPIC, \
 DISABLED_WARNINGS_clang := format-nonliteral, \
-LDFLAGS := $(UNPACKEXE_ZIPOBJS) \
-$(LDFLAGS_JDKEXE) $(LDFLAGS_CXX_JDK) \
+LDFLAGS := $(LDFLAGS_JDKEXE) $(LDFLAGS_CXX_JDK) \

JDK11u
==
CFLAGS_linux := -fPIC, \
 CFLAGS_solaris := -KPIC, \
 CFLAGS_macosx := -fPIC, \
-LDFLAGS := $(UNPACKEXE_ZIPOBJS) \
-$(LDFLAGS_JDKEXE) $(LDFLAGS_CXX_JDK) \
+LDFLAGS := $(LDFLAGS_JDKEXE) $(LDFLAGS_CXX_JDK) \

8218807: Compilation database (compile_commands.json) may contain obsolete items

This is a simple fix for 8210459 and applies cleanly

8223678: Add Visual Studio Code workspace generation support (for native code)

This change actually creates the VS Code project files (i.e. "make
vscode-project-ccls"). See
http://hg.openjdk.java.net/jdk/jdk/raw-file/tip/doc/ide.html for more
details.

Applies cleanly except the following two hunks. The first one had to
be moved back from "make/common/Utils.gmk" to
"make/common/MakeBase.gmk" where it still applies cleanly, because the
"RelativePath" macro is still in "MakeBase.gmk" in  jdk11u.

ORIGINAL

diff -r a82a367b2d8c -r 2ae056696b15 make/common/Utils.gmk
--- a/make/common/Utils.gmkMon Jun 03 17:14:23 2019 -0700
+++ b/make/common/Utils.gmkMon Jun 03 10:28:03 2019 +0200
@@ -122,7 +122,8 @@
 # $2 - Directory to compute the relative path from
 RelativePath = \
 $(eval $1_prefix := $(call FindCommonPathPrefix, $1, $2)) \
-$(eval $1_dotdots := $(call DirToDotDot, $(patsubst $($(strip
$1)_prefix)/%, %, $2))) \
+$(eval $1_dotdots := $(call DirToDotDot, $(patsubst $($(strip
$1)_prefix)%, %, $2))) \
+$(eval $1_dotdots := $(if $($(strip $1)_dotdots),$($(strip
$1)_dotdots),.)) \
 $(eval $1_suffix := $(patsubst $($(strip $1)_prefix)/%, %, $1)) \
 $($(strip $1)_dotdots)/$($(strip $1)_suffix)

JDK11u
==
diff -r 8599975f5b33 make/common/MakeBase.gmk
--- a/make/common/MakeBase.gmkTue Feb 12 08:40:43 2019 +0100
+++ b/make/common/MakeBase.gmkMon Dec 23 22:10:46 2019 +0100
@@ -611,7 +611,8 @@
 # $2 - Directory to compute the relative path from
 RelativePath = \
 $(eval $1_prefix := $(call FindCommonPathPrefix, $1, $2)) \
-$(eval $1_dotdots := $(call DirToDotDot, $(patsubst $($(strip
$1)_prefix)/%, %, $2))) \
+$(eval $1_dotdots := $(call DirToDotDot, $(patsubst $($(strip
$1)_prefix)%, %, $2))) \
+$(eval $1_dotdots := $(if $($(strip $1)_dotdots),$($(strip
$1)_dotdots),.)) \
 $(eval $1_suffix := $(patsubst $($(strip $1)_prefix)/%, %, $1)) \
 $($(strip $1)_dotdots)/$($(strip $1)_suffix)

For the second changed hunk, the simple call of the "AssertEquals"
macro had to be renamed to "assert-equals" and wrapped with "eval"
because the corresponding macros haven't been updated in jdk11u yet.
Notice that these are only test changes (for testing the make system
iteslf) and don't effect the build at all.

ORIGINAL

diff -r a82a367b2d8c -r 2ae056696b15 test/make/TestMakeBase.gmk
--- a/test/make/TestMakeBase.gmkMon Jun 03 17:14:23 2019 -0700
+++ b/test/make/TestMakeBase.gmkMon Jun 03 10:28:03 2019 +0200
@@ -361,6 +361,18 @@
 RelativePath, \
 )

+$(call AssertEquals, \
+$(call RelativePath, /foo/bar/baz/banan/kung, /foo/bar/baz), \
+./banan/kung, \
+RelativePath, \
+)
+
+$(call AssertEquals, \
+$(call 

Result: New Build Group Member: Matthias Baesken

2019-07-12 Thread Volker Simonis
Voting for Matthias Baesken [1] is now closed.

Yes: 5
Veto: 0
Abstain: 0

According to the Bylaws definition of Lazy Consensus voting, this is
sufficient to approve the nomination.

Best regards,
Volker

[1]
https://mail.openjdk.java.net/pipermail/build-dev/2019-June/025839.html


CFV: New Build Group Member: Matthias Baesken

2019-06-26 Thread Volker Simonis
I hereby nominate Matthias Baesken (mbaesken) to Membership in the Build Group.

Matthias is a long standing member of the JVM team at SAP. He's main
areas of expertise are the build system, compilers/porting and
security updates. He's a JDK Reviewer who has contributed more than
90 changes within the last two years [1], many of which were build
system related.

Votes are due by 10 July 2019, 12:00 CET.

Only current Members of the Build Group [2] are eligible
to vote on this nomination.  Votes must be cast in the open by
replying to this mailing list

For Lazy Consensus voting instructions, see [3].

Volker Simonis

[1] 
http://hg.openjdk.java.net/jdk/jdk/search/?rev=((author(%22mbaesken%22)%20and%20not%20desc(%22Contributed-by%22))%20or%20desc(%22Contributed-by%3A%20matthias.baesken%40sap.com%22))%20and%20not%20merge()=100
[2] http://openjdk.java.net/census#build
[3] http://openjdk.java.net/groups/#member-vote


Re: CFV: New Build Group Member: Matthias Baesken

2019-06-26 Thread Volker Simonis
Vote: yes

On Wed, Jun 26, 2019 at 10:29 AM Volker Simonis
 wrote:
>
> I hereby nominate Matthias Baesken (mbaesken) to Membership in the Build 
> Group.
>
> Matthias is a long standing member of the JVM team at SAP. He's main
> areas of expertise are the build system, compilers/porting and
> security updates. He's a JDK Reviewer who has contributed more than
> 90 changes within the last two years [1], many of which were build
> system related.
>
> Votes are due by 10 July 2019, 12:00 CET.
>
> Only current Members of the Build Group [2] are eligible
> to vote on this nomination.  Votes must be cast in the open by
> replying to this mailing list
>
> For Lazy Consensus voting instructions, see [3].
>
> Volker Simonis
>
> [1] 
> http://hg.openjdk.java.net/jdk/jdk/search/?rev=((author(%22mbaesken%22)%20and%20not%20desc(%22Contributed-by%22))%20or%20desc(%22Contributed-by%3A%20matthias.baesken%40sap.com%22))%20and%20not%20merge()=100
> [2] http://openjdk.java.net/census#build
> [3] http://openjdk.java.net/groups/#member-vote


Re: RFR: 8223678: Add Visual Studio Code workspace generation support (for native code)

2019-05-29 Thread Volker Simonis
On Wed, May 29, 2019 at 3:43 PM Robin Westberg
 wrote:
>
> Hi Volker,
>
> > On 28 May 2019, at 17:33, Volker Simonis  wrote:
> >
> > Hi Robin,
> >
> > thanks a lot for doing this!
> >
> > I have just a quick question. Do you know if any of the VSC indexers
> > (default, clangd) support call hierarchies (i.e. "called by",
> > "callers" of a function/method) and "used by" for variables/class
> > fields?
>
> Sure, I can make a quick summary of the various pros and cons of the indexers 
> that I’ve found so far. They are all moving pretty fast though, so didn’t 
> think it was a good fit for the documentation file.
>
> In general, VS Code itself only just recently gained proper support for 
> displaying call hierarchies: 
> https://code.visualstudio.com/updates/v1_33#_call-hierarchy - but alternative 
> indexers have worked around this by showing them differently.
>
> Default (Microsoft - C/C++ for Visual Studio Code)
> + Easy to setup, as no additional dependencies are needed
> + Good “go to definition”, the only one that can make sense of the 
> template+macro stuff in log.hpp for example
> - Find references (used by) not done yet (said to be coming in the June 
> update: https://github.com/microsoft/vscode-cpptools/issues/15)
> - Call hierarchies also not there (scheduled for September apparently: 
> https://github.com/microsoft/vscode-cpptools/issues/16)
>
> clangd
> + Actively developed and part of the llvm/clang project
> + Support for find references
> - ..but not call hierarchies yet
> - Full project indexing is still flagged as experimental, and currently seems 
> to fail when editing commonly used header files for example
>
> Full feature list: 
> https://clang.llvm.org/extra/clangd/Features.html#complete-list-of-features
>
> rtags
> This is currently the most feature-complete indexer I think. But the VS Code 
> integration is a bit minimal and not part of the rtags project itself, and it 
> is missing things like indexer progress.
> + The indexer is actively developed and has been around for quite some time
> - You will probably have to build the indexer from source
> + Full support for call hierarchies and find references
> - VS Code integration a bit limited, missing convenience features like switch 
> between header/source file, showing method/class documentation on hover.
>
> There are even more indexers of course, a promising one used to be “cquery", 
> but that project seems to be defunct now. It lives on in the “ccls" indexer, 
> but ithat one is a bit tricky to configure unless you use clang for building 
> the JDK as well.
>
> So in summary, after the summer the default indexer might be the obvious best 
> choice, but right now it depends on which features you think are the most 
> important I guess..
>

Hi Robin,

thanks a lot for the great summary! Incidentally I've just did some
Google research as well and basically came to the same conclusion.
"Cquery" seemed quite promising and also pretends to have call
hierarchies. But it seems to be defunct and I've also found some bug
reports about problems with the call hierarchies.

For me "Called Hierarchy", and "Find References" (in this order) are
really the most important IDE features. I'm using Eclipse and if you
setup your HotSpot project correctly, these features work pretty well
and reliably.

Have you personally tried the rtags "call hierarchies" / "find
references" support in VSC? From your summary it sounds like it's
worth giving it a try.

Thanks again for caring about these topics! Hotspot development with
IDE support has always been a pain and every improvement in this area
will be highly welcome :)

Best regards,
Volker

> Best regards,
> Robin
>
> >
> > Regards,
> > Volker
> >
> > On Mon, May 27, 2019 at 6:03 PM Robin Westberg
> >  wrote:
> >>
> >> Hi all,
> >>
> >> Please review this change that adds build system support for generating a 
> >> Visual Studio Code workspace configured for working with the JDK native 
> >> code. It configures the default C/C++ IntelliSense Engine to allow code 
> >> completion/navigation and similar features. It also configures two 
> >> executable targets (gtestLauncher and java) that can be built and debugged 
> >> from the IDE.
> >>
> >> The main target is "make vscode-project”, additional information can be 
> >> found in doc/ide.[md|html].
> >>
> >> Issue:
> >> https://bugs.openjdk.java.net/browse/JDK-8223678
> >>
> >> Webrev:
> >> https://cr.openjdk.java.net/~rwestberg/8223678/webrev.01/
> >>
> >> Testing:
> >> Manual testing on Linux, MacOS and Windows
> >>
> >> Thanks Erik Joelsson for taking a look at an earlier version of it!
> >>
> >> Best regards,
> >> Robin
>


Re: RFR: 8223678: Add Visual Studio Code workspace generation support (for native code)

2019-05-28 Thread Volker Simonis
Hi Robin,

thanks a lot for doing this!

I have just a quick question. Do you know if any of the VSC indexers
(default, clangd) support call hierarchies (i.e. "called by",
"callers" of a function/method) and "used by" for variables/class
fields?

Regards,
Volker

On Mon, May 27, 2019 at 6:03 PM Robin Westberg
 wrote:
>
> Hi all,
>
> Please review this change that adds build system support for generating a 
> Visual Studio Code workspace configured for working with the JDK native code. 
> It configures the default C/C++ IntelliSense Engine to allow code 
> completion/navigation and similar features. It also configures two executable 
> targets (gtestLauncher and java) that can be built and debugged from the IDE.
>
> The main target is "make vscode-project”, additional information can be found 
> in doc/ide.[md|html].
>
> Issue:
> https://bugs.openjdk.java.net/browse/JDK-8223678
>
> Webrev:
> https://cr.openjdk.java.net/~rwestberg/8223678/webrev.01/
>
> Testing:
> Manual testing on Linux, MacOS and Windows
>
> Thanks Erik Joelsson for taking a look at an earlier version of it!
>
> Best regards,
> Robin


Re: RFR: 8224087: Compile C code for at least C99 Standard compliance

2019-05-20 Thread Volker Simonis
Sergey Bylokhov  schrieb am Mo. 20. Mai 2019 um
20:14:

> Hi, David.
> Should not the minimum version of c standard mentioned in the
> doc/building.html?
>

We actually have a Wiki page for that because it’s much easier to maintain:

https://wiki.openjdk.java.net/display/Build/Supported+Build+Platforms


> On 20/05/2019 00:40, David Holmes wrote:
> > Bug: https://bugs.openjdk.java.net/browse/JDK-8224087
> > webrev: http://cr.openjdk.java.net/~dholmes/8224087/webrev/
> >
> > The need to remove a for-loop declaration expression to appease gcc 4.8
> annoyed me enough to investigate setting C99 as our minimum allow
> C-language level when compiling. It turned out to be a lot more complex a
> situation than I thought due to toolchain quirks. See lots of details in
> the bug report.
> >
> > To summarise the changes:
> > - gcc: force to -std=gnu99
> > - clang force to -std=gnu99
> > - Solaris studio - no effective change
> > - Visual Studio - no change
> > - xlc - no effective change (but we use the explicit flag rather than
> accepting it as default)
> >
> > I've checked how this works with all the toolchains except xlc as I have
> no access to that. Some assistance from someone who can verify the
> correctness on xlc would be appreciated.
> >
> > Thanks,
> > David
>
>
> --
> Best regards, Sergey.
>


Re: RFR: 8224087: Compile C code for at least C99 Standard compliance

2019-05-20 Thread Volker Simonis
Hi David,

thanks for considering AIX in your change.

With OpenJDK 13 we've moved to XLC 16 which has a new Clang based
frontend. The "ucs" feature-suboption [1] you're using in your change
is only supported in the old, xlc compiler but not in the new xlclang
any more [2]. However, the good news is that the new xlclang now
supports the same "-std=gn99" option like gcc and clang, so you could
easily handle the xlc case now together with gcc and clang as shown in
this webrev:

http://cr.openjdk.java.net/~simonis/webrevs/2019/8224087/

Otherwise, your change looks good!

Please let me know once you've pushed it. I opened "8224214: [AIX]
Remove support for legacy xlc compiler" [3] for further cleannups
because we still set some "-qlanglvl" options for C++ which aren't
supported by the new compiler either.

Best regards,
Volker

[1] 
https://www.ibm.com/support/knowledgecenter/SSGH3R_12.1.0/com.ibm.xlcpp121.aix.doc/compiler_ref/opt_langlvl.html
[2] 
https://www.ibm.com/support/knowledgecenter/SSGH3R_16.1.0/com.ibm.xlcpp161.aix.doc/compiler_ref/opt_langlvl_aix.html


On Mon, May 20, 2019 at 9:40 AM David Holmes  wrote:
>
> Bug: https://bugs.openjdk.java.net/browse/JDK-8224087
> webrev: http://cr.openjdk.java.net/~dholmes/8224087/webrev/
>
> The need to remove a for-loop declaration expression to appease gcc 4.8
> annoyed me enough to investigate setting C99 as our minimum allow
> C-language level when compiling. It turned out to be a lot more complex
> a situation than I thought due to toolchain quirks. See lots of details
> in the bug report.
>
> To summarise the changes:
> - gcc: force to -std=gnu99
> - clang force to -std=gnu99
> - Solaris studio - no effective change
> - Visual Studio - no change
> - xlc - no effective change (but we use the explicit flag rather than
> accepting it as default)
>
> I've checked how this works with all the toolchains except xlc as I have
> no access to that. Some assistance from someone who can verify the
> correctness on xlc would be appreciated.
>
> Thanks,
> David


Re: RFR [XS] : 8223685: used bundled zlib on AIX by default

2019-05-10 Thread Volker Simonis
Looks good!

Thanks,
Volker

On Fri, May 10, 2019 at 12:23 PM Baesken, Matthias
 wrote:
>
> Hello, please review this small  AIX - specific change .
>
> Currently we use by default the bundled zlib  (= zlib coming with OpenJDK)   
> on Windows, on other platforms the system zlib.
> However it is possible to switch to the bundled zlib  in the configure 
> process .
>
> We had some build issues on AIX  build systems with ancient system-zlibs ,  
> so I propose to use as default the bundled zlib on AIX too .
> (would also mean that  we use a current 1.2.11  in the build on AIX which is 
> not bad, on some AIX machines I've seen older zlibs)
>
>
> Bug/webrev  :
>
> https://bugs.openjdk.java.net/browse/JDK-8223685
>
> http://cr.openjdk.java.net/~mbaesken/webrevs/8223685.0/webrev/
>
> Thanks, Matthias


Re: RFR(XS): 8222627: Remove sneaky token 'Name' in jdk-version.m4

2019-04-16 Thread Volker Simonis
Looks good.

Nice to see how „fault tolerant“ m4 is :)

Langer, Christoph  schrieb am Mi. 17. Apr. 2019
um 08:46:

> Hi,
>
> please review this little revert of a token that accidentally sneaked in
> when I pushed JDK-8222522 (Add configure options for Mac Bundle creation)
> yesterday. I don't know how that happened but fortunately it didn't break
> the build...
>
> diff -r 15f2aae40bc8 -r ae1be0d04777 make/autoconf/jdk-version.m4
> --- a/make/autoconf/jdk-version.m4  Tue Apr 16 20:47:11 2019 -0700
> +++ b/make/autoconf/jdk-version.m4  Wed Apr 17 06:25:14 2019 +0100
> @@ -1,4 +1,4 @@
> -Name#
> +#
> # Copyright (c) 2015, 2019, Oracle and/or its affiliates. All rights
> reserved.
> # DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
> #
>
> Thanks & Sorry
> Christoph
>
>


Re: RFR(XS): 8220164: Fix build instructions for AIX

2019-03-05 Thread Volker Simonis
Thanks Martin!

On Tue, Mar 5, 2019 at 5:06 PM Doerr, Martin  wrote:
>
> Hi Volker,
>
> the wiki is already up to date, so I like your new version which just refers 
> to it. Reviewed.
>
> Best regards,
> Martin
>
>
> -Original Message-
> From: build-dev  On Behalf Of Volker 
> Simonis
> Sent: Dienstag, 5. März 2019 16:17
> To: build-dev 
> Subject: RFR(XS): 8220164: Fix build instructions for AIX
>
> Hi,
>
> can I please have a review for the following trivial change which
> updates the AIX/XLC information in the the building.{md, html} files:
>
> https://bugs.openjdk.java.net/browse/JDK-8220164
> http://cr.openjdk.java.net/~simonis/webrevs/2019/8220164/
>
> The build instructions for AIX in doc/building.{md, html} still
> reference our old AIX build results page which isn't active since
> quite some time and refers to old OS/compiler versions.
>
> Remove the old information and link to the OpenJDK build Wiki instead
> (https://wiki.openjdk.java.net/display/Build/Supported+Build+Platforms)
> which should always have the current information.
>
> Thank you and best regards,
> Volker


RFR(XS): 8220164: Fix build instructions for AIX

2019-03-05 Thread Volker Simonis
Hi,

can I please have a review for the following trivial change which
updates the AIX/XLC information in the the building.{md, html} files:

https://bugs.openjdk.java.net/browse/JDK-8220164
http://cr.openjdk.java.net/~simonis/webrevs/2019/8220164/

The build instructions for AIX in doc/building.{md, html} still
reference our old AIX build results page which isn't active since
quite some time and refers to old OS/compiler versions.

Remove the old information and link to the OpenJDK build Wiki instead
(https://wiki.openjdk.java.net/display/Build/Supported+Build+Platforms)
which should always have the current information.

Thank you and best regards,
Volker


Re: RFR: JDK-8217723 Switch ld from bfd to gold on gcc toolchain

2019-01-29 Thread Volker Simonis
When I last tried gold a few years ago on ppc64 (and ia64, just for
the reference :) it didn't work because it didn't support all the
required relocations.

However, a current experiment with "GNU gold (GNU Binutils for Ubuntu
2.26.1) 1.11" on Ubuntu 16.04/ppc64le succeeded and improved the link
time of libjvm.so (with full debug info) from ~15 to ~5 seconds which
is pretty nice.

I'd still appreciate if the usage of gold would be guarded by a configure check.

Thank you and best regards,
Volker

On Sat, Jan 26, 2019 at 6:00 PM Martin Buchholz  wrote:
>
> Another, more productionized, version of my benchmark:
>
> processors=12
> g++ (Debian 7.3.0-5) 7.3.0
> --- -fuse-ld=bfd ---
> 6.559 user 1.180 system 7.740 total
> --- -fuse-ld=gold ---
> 4.575 user 0.600 system 5.176 total
> --- -fuse-ld=gold -Wl,-threads ---
> 9.355 user 5.062 system 4.289 total
> --- -fuse-ld=lld ---
> 2.700 user 1.058 system 1.157 total
> --- -fuse-ld=lld -Wl,-threads ---
> 2.572 user 1.128 system 1.107 total
>
>
> #!/bin/bash
> set -eu
> echo processors=$(nproc)
> read -a CMDLINE < $(find . -name BUILD_LIBJVM_link.cmdline -print)
>
> readonly DRIVER="${CMDLINE[0]}"
> "$DRIVER" --version | head -1
>
> benchmark() {
>   echo --- "$@" ---
>   local -r TIMEFORMAT="%U user %S system %R total"
>   time "$DRIVER" "$@" "${CMDLINE[@]:1}"
> }
>
> benchmark -fuse-ld=bfd
> benchmark -fuse-ld=gold
> benchmark -fuse-ld=gold -Wl,-threads
> benchmark -fuse-ld=lld
> benchmark -fuse-ld=lld -Wl,-threads


Re: Is anyone using msys to build OpenJDK?

2018-12-15 Thread Volker Simonis
It is a long time ago (~2012) that I added and fixed the MinGW/Msys build
[1] on Windows. We at SAP used it productively for some years because at
that time Cygwin was terrible slow, buggy, had no 64-bit version, wasn’t
really maintained very well, etc...

But things have changed to the opposite since then and we‘ve switched to
Cygwin at least 3 years ago (can‘t find the exact date now).

As far as I know, there haven‘t been any serious users of the MinGW/Msys
build since then. At that time MinGW/Msys had some serious problems which
made it impossible to do parallel builds. Not sure if that has been fixed
by now?

I personally always liked MinGW/Msys compared to Cygwin because of the (at
least in my eyes) more intuitive way of handling Unix/Windows paths it’s
simplicity and speed. I’m not sure however if it is currently actively
supported, and if the old problems have been fixed.

So to finally answer your question :) I don’t think anybody is currently
using the MinGW/Msys build and I doubt that it will work without bigger
adaptions and fixes.

Regards,
Volker

[1]
http://mail.openjdk.java.net/pipermail/build-dev/2012-March/005729.html

Andrew Luo  schrieb am Sa. 15. Dez. 2018
um 10:11:

> From the source history, it appears that with JDK9 we dropped support for
> msys:
>
> http://hg.openjdk.java.net/jdk/jdk/rev/2a2e56f4c03b
>
> This was committed about 1.5 years ago - are you suggesting that perhaps
> we added support for msys2 later on (perhaps unofficial or undocumented)?
>
> Thanks,
>
> -Andrew
>
> -Original Message-
> From: build-dev  On Behalf Of Erik
> Joelsson
> Sent: Friday, December 14, 2018 5:39 PM
> To: build-dev 
> Subject: Is anyone using msys to build OpenJDK?
>
> If anyone is currently using msys to build OpenJDK, I'm curious to know
> what version so I can replicate that environment.
>
> As we are trying to add WSL support, we are touching all sorts of places
> in configure, and there is a big risk that we break the existing msys
> support. I would like to be able to setup an environment so I can at least
> do basic verification of such changes.
>
> I know we initially set this up for msys, but later I tried msys2, which
> seemed a lot more modern and easy to use. I do remember there were several
> options on how to setup it up exactly though.
>
> /Erik
>
>


Re: Proposal: Use new JDK_EXPORT decorator instead of JNIEXPORT

2018-12-11 Thread Volker Simonis
On Tue, Dec 11, 2018 at 11:47 PM David Holmes  wrote:
>
> On 12/12/2018 12:34 am, Magnus Ihse Bursie wrote:
> >
> >
> > On 2018-12-11 00:23, David Holmes wrote:
> >> Hi Magnus,
> >>
> >> On 10/12/2018 11:19 pm, Magnus Ihse Bursie wrote:
> >>> I propose that we introduce a new define, available to all JDK native
> >>> files (Hotspot included), called JDK_EXPORT. The behavior of this
> >>> symbol will be very similar (as of now, in fact identical) to
> >>> JNIEXPORT; however, the semantics will not.
> >>>
> >>> Currently, we "mis-use" the JNIEXPORT define to mark a function for
> >>> exporting from the library. The problem with this is that JNIEXPORT
> >>> is part of the JNI interface, and is supposed to be used when C
> >>> programs interact with Java. And, when doing this, the function
> >>> should be fully decorated like this: "JNIEXPORT foo JNICALL".
> >>
> >> I've seen a lot of the emails on this issue and I don't fully
> >> understand what has been going wrong. But the intent is obviously the
> >> JNIEXPORT represents what is needed to export this function for use by
> >> JNI, while JNICALL defines the calling convention. I agree there may
> >> be some mistmatch when functions are actually not intended for general
> >> export outside the JDK but are only for internal JDK use.
> >>
> >>> We do have many such JNI exports in our native libraries, but we also
> >>> have a lot of other, non-JNI exports, where one native library just
> >>> provides an interface to other libraries. In these cases, we have
> >>> still used JNIEXPORT for the functionality of getting the function
> >>> exported, but we have not been consistent in our use of JNICALL. This
> >>> has caused us way too much trouble for something that should Just
> >>> Work.
> >>
> >> Are you suggesting that the interface between different libraries in
> >> the JDK should not be a JNI interface? Is this because you think the
> >> functions in these libraries are only for JDK internal use or ... ??
> >>
> >>> I therefore propose that we define "JDK_EXPORT", with the same
> >>> behavior as JNIEXPORT (that is, flagging the function for external
> >>> visibility in the resulting native library), but which is *not*
> >>> supposed to be exported to Java code using JNI, nor supposed to be
> >>> decorated with
> >>
> >> Just a clarification there. JNI functions are not exported to Java
> >> code, they are exported to native code. Java code can declare native
> >> methods and those native methods must be written as JNI functions, but
> >> that's not what we are discussing. Libraries expose a JNI interface (a
> >> set of functions in the library) that can be called by application
> >> native code, using JNI.
> > We're apparently looking at "JNI" and "exporting" from two opposite
> > sides here. :-) Just to make everything clear: If I have a Java class
> > class MyClass {
> >public static void native myNativeFunc();
> > }
> >
> > then I have one half of the JNI function, the Java half. This must be
> > matched by a corresponding implementation in native, like this:
> > JNIEXPORT void JNICALL
> > Java_MyClass_myNativeFunc(void) {
> > // ... do stuff
> > }
> >
> > And this is the native half of the JNI function. Right? Let's leave
> > aside which side is "exporting" to the other for now. :-)
> >
> > This way of setting up native functions that can be called from Java is
> > what I refer to as JNI. And when you declare a native JNI function, you
> > *must* use both JNIEXPORT and JNICALL. Alright?
> >
> > We do have a lot of those functions in our native libraries. And they
> > are correct just the way they are.
>
> Yep all well and good. A function declared native in Java must have an
> implementation as you describe. But not all native functions exist to
> provide the native-half of a Java native function!
>
> > However, we also have *other* native functions, that are exported, not
> > as JNI functions that should be called from Java, but as normal native
> > library functions that should be called by other native code. Okay so
> > far? And *those* functions have been problematic in how we decorate
>
> But there are again two cases. Those functions exported from a library
> that are expected to be called from external code using the JNI
> interface mechanism - such as all the JNI functions and JVM TI functions
> we export from the JVM - and those "exported" for access between
> libraries within the JDK (such as all the JVM_* functions in libjvm).
>
> I think it is only the second group that should be addressed by your
> JDK_EXPORT proposal - though I'm not completely clear exactly how to
> identify them.
>
> > them. My proposal is that we *refrain* from using JNIEXPORT for those
> > functions, and instead use JDK_EXPORT as name for the macro that
> > decorates them as exported. That way, we can clearly see that a function
> > like this:
> >
> > JDK_EXPORT void
> > JLI_ReadEnv(char* env);
> >
> > is correctly declared, and will be exported to other native 

Re: Proposal: Use new JDK_EXPORT decorator instead of JNIEXPORT

2018-12-11 Thread Volker Simonis
On Tue, Dec 11, 2018 at 3:35 PM Magnus Ihse Bursie
 wrote:
>
>
>
> On 2018-12-11 00:23, David Holmes wrote:
> > Hi Magnus,
> >
> > On 10/12/2018 11:19 pm, Magnus Ihse Bursie wrote:
> >> I propose that we introduce a new define, available to all JDK native
> >> files (Hotspot included), called JDK_EXPORT. The behavior of this
> >> symbol will be very similar (as of now, in fact identical) to
> >> JNIEXPORT; however, the semantics will not.
> >>
> >> Currently, we "mis-use" the JNIEXPORT define to mark a function for
> >> exporting from the library. The problem with this is that JNIEXPORT
> >> is part of the JNI interface, and is supposed to be used when C
> >> programs interact with Java. And, when doing this, the function
> >> should be fully decorated like this: "JNIEXPORT foo JNICALL".
> >
> > I've seen a lot of the emails on this issue and I don't fully
> > understand what has been going wrong. But the intent is obviously the
> > JNIEXPORT represents what is needed to export this function for use by
> > JNI, while JNICALL defines the calling convention. I agree there may
> > be some mistmatch when functions are actually not intended for general
> > export outside the JDK but are only for internal JDK use.
> >
> >> We do have many such JNI exports in our native libraries, but we also
> >> have a lot of other, non-JNI exports, where one native library just
> >> provides an interface to other libraries. In these cases, we have
> >> still used JNIEXPORT for the functionality of getting the function
> >> exported, but we have not been consistent in our use of JNICALL. This
> >> has caused us way too much trouble for something that should Just
> >> Work.
> >
> > Are you suggesting that the interface between different libraries in
> > the JDK should not be a JNI interface? Is this because you think the
> > functions in these libraries are only for JDK internal use or ... ??
> >
> >> I therefore propose that we define "JDK_EXPORT", with the same
> >> behavior as JNIEXPORT (that is, flagging the function for external
> >> visibility in the resulting native library), but which is *not*
> >> supposed to be exported to Java code using JNI, nor supposed to be
> >> decorated with
> >
> > Just a clarification there. JNI functions are not exported to Java
> > code, they are exported to native code. Java code can declare native
> > methods and those native methods must be written as JNI functions, but
> > that's not what we are discussing. Libraries expose a JNI interface (a
> > set of functions in the library) that can be called by application
> > native code, using JNI.
> We're apparently looking at "JNI" and "exporting" from two opposite
> sides here. :-) Just to make everything clear: If I have a Java class
> class MyClass {
>public static void native myNativeFunc();
> }
>
> then I have one half of the JNI function, the Java half. This must be
> matched by a corresponding implementation in native, like this:
> JNIEXPORT void JNICALL
> Java_MyClass_myNativeFunc(void) {
> // ... do stuff
> }
>
> And this is the native half of the JNI function. Right? Let's leave
> aside which side is "exporting" to the other for now. :-)
>
> This way of setting up native functions that can be called from Java is
> what I refer to as JNI. And when you declare a native JNI function, you
> *must* use both JNIEXPORT and JNICALL. Alright?
>
> We do have a lot of those functions in our native libraries. And they
> are correct just the way they are.
>
> However, we also have *other* native functions, that are exported, not
> as JNI functions that should be called from Java, but as normal native
> library functions that should be called by other native code. Okay so
> far? And *those* functions have been problematic in how we decorate
> them. My proposal is that we *refrain* from using JNIEXPORT for those
> functions, and instead use JDK_EXPORT as name for the macro that
> decorates them as exported. That way, we can clearly see that a function
> like this:
>
> JDK_EXPORT void
> JLI_ReadEnv(char* env);
>
> is correctly declared, and will be exported to other native libraries,
> but not to Java.
>

Hi Magnus,

I agree with your explanation and I think your proposal is sound.

Have you checked how many of the occurrences of "#include "jni.h"" can
really be replaced by "#include "jdk.h""?

I think native code also "misuses" jni.h in order to agree on common
types like jint, jlong, etc.. across different native libraries. We
could of course also define such common types in "jdk.h", but I'm not
sure if it's worth the effort?

Regards,
Volker

> Just to clarify, this has nothing to do with if this is a officially
> supported API or not. In general though, I assume that most (if not
> all?) of our exported functions (apart from the JNI_* stuff) is supposed
> to be consumed by other libraries in the JDK, and is not a public API.
>
> /Magnus
>
>
>
> >
> >> JNICALL. All current instances of JNIEXPORT which is not pure JNI
> >> native functions 

Re: RFR(XS): 8214063: [AIX] Disable symbol visibility flags

2018-12-04 Thread Volker Simonis
Hi Adam,

I've just pushed the change:

http://hg.openjdk.java.net/jdk/jdk/rev/fc54d27e58d8

Best regards,
Volker
On Thu, Nov 29, 2018 at 5:54 PM Adam Farley8  wrote:
>
> Hi All,
>
> The build passed on xlC 13.1 with the makefile patch I proposed (good catch 
> on the comments Volkar!).
>
> With Volkar, Erik, Matthias, and Magnus all approving the change, it sounds 
> like we're good to merge!
>
> Volkar, can you do the honours?
>
> Best Regards
>
> Adam Farley
> IBM Runtimes
>
> P.S. I approve the change too. ;)
>
>
> Volker Simonis  wrote on 29/11/2018 11:54:33:
>
> > From: Volker Simonis 
> > To: Magnus Ihse Bursie 
> > Cc: build-dev , ppc-aix-port-
> > d...@openjdk.java.net, adam.far...@uk.ibm.com
> > Date: 29/11/2018 11:54
> > Subject: Re: RFR(XS): 8214063: [AIX] Disable symbol visibility flags
> >
> > On Thu, Nov 29, 2018 at 12:20 PM Magnus Ihse Bursie
> >  wrote:
> > >
> > > On 2018-11-27 16:33, Volker Simonis wrote:
> > >
> > > > Hi,
> > > >
> > > > can I please have a review for the following trivial change which
> > > > simply disables the symbol visibility flags on AIX:
> > > >
> > > > https://urldefense.proofpoint.com/v2/url?
> > u=http-3A__cr.openjdk.java.net_-7Esimonis_webrevs_2018_8214063_=DwIBaQ=jf_iaSHvJObTbx-
> > siA1ZOg=P5m8KWUXJf-
> > CeVJc0hDGD9AQ2LkcXDC0PMV9ntVw5Ho=6y4Npxy6aG4q8E9Xca--
> > YxF4UGVrVEIqu_wVvivFVUA=DptrWUUtJCcpUCbCWkkBOeFJCVk5im3hm9T_DcD0Jd8=
> > > > https://urldefense.proofpoint.com/v2/url?
> > u=https-3A__bugs.openjdk.java.net_browse_JDK-2D8214063=DwIBaQ=jf_iaSHvJObTbx-
> > siA1ZOg=P5m8KWUXJf-
> > CeVJc0hDGD9AQ2LkcXDC0PMV9ntVw5Ho=6y4Npxy6aG4q8E9Xca--
> > YxF4UGVrVEIqu_wVvivFVUA=jBFABkJb5E5W9K8pMX794-3gnpLfPyi3oASA1kizQ7A=
> > > Looks good to me. I am sorry for the mess I caused by optimisically
> > > trying to fix things on a platform I could not compile on... :(
> > >
> >
> > Thanks for the review and don't worry! We really appreciate your
> > continued help. It's really us who should have tested and spotted the
> > problems earlier :)
> >
> > Regards,
> > Volker
> >
> > > This also reminds me that the visibility flags *really* should move into
> > > configure/spec, not be sprinkled like this in the make files.
> > >
> > > /Magnus
> > > >
> > > > Change "8202322: AIX: symbol visibility flags not support on xlc 12.1"
> > > > [1] blindly introduced these flags for all xlC compiler versions >
> > > > 12.1 without ever testing it (which should not have happened). Now
> > > > that people are starting to really use xlC 13 it turns out that there
> > > > is more to do than just enabling the flags. This future work is
> > > > covered by "8204541: Correctly support AIX xlC 13.1 symbol visibility
> > > > flags".
> > > >
> > > > Thank you and best regards,
> > > > Volker
> > > >
> > > > [1] https://urldefense.proofpoint.com/v2/url?
> > u=https-3A__bugs.openjdk.java.net_browse_JDK-2D8202322=DwIBaQ=jf_iaSHvJObTbx-
> > siA1ZOg=P5m8KWUXJf-
> > CeVJc0hDGD9AQ2LkcXDC0PMV9ntVw5Ho=6y4Npxy6aG4q8E9Xca--
> > YxF4UGVrVEIqu_wVvivFVUA=pd7-rH7OPxeaq2g6S0dQPmb_3-8PLi8JZFKcP_Abp6Q=
> > > > [2] https://urldefense.proofpoint.com/v2/url?
> > u=https-3A__bugs.openjdk.java.net_browse_JDK-2D8204541=DwIBaQ=jf_iaSHvJObTbx-
> > siA1ZOg=P5m8KWUXJf-
> > CeVJc0hDGD9AQ2LkcXDC0PMV9ntVw5Ho=6y4Npxy6aG4q8E9Xca--
> > YxF4UGVrVEIqu_wVvivFVUA=q7KHUASpF-opdcLXbTTUT1bPoKrkTeaHTtd7c2jN4rc=
> > >
> >
>
> Unless stated otherwise above:
> IBM United Kingdom Limited - Registered in England and Wales with number 
> 741598.
> Registered office: PO Box 41, North Harbour, Portsmouth, Hampshire PO6 3AU


Re: RFR(XS): 8214534: Setting of THIS_FILE in the build is broken

2018-12-03 Thread Volker Simonis
Cool, thanks!
I'll push then...
On Mon, Dec 3, 2018 at 2:37 PM Magnus Ihse Bursie
 wrote:
>
> On 2018-12-03 14:31, Volker Simonis wrote:
>
> Hi Magnus, Erik,
>
> do I understand you correctly that you both are fine with my proposed
> fix and that we leave all the other enhancements/discussion for the
> future?
>
> Yes, sorry I was not clear. Your fix looks good to me.
>
> We can save the rest of the discussion of reproducible builds for a rainy day.
>
> /Magnus
>
> Thank you and best regards,
> Volker
>
> On Mon, Dec 3, 2018 at 12:27 PM Magnus Ihse Bursie
>  wrote:
>
> On 2018-11-30 19:03, Volker Simonis wrote:
>
> On Fri, Nov 30, 2018 at 6:37 PM Erik Joelsson  
> wrote:
>
> Hello Volker,
>
> The fix looks good. Thanks for finding and fixing it!
>
> Thanks!
>
> Now for some history on why THIS_FILE. The short story is that it's for
> more reproducible and comparable builds.
>
> When we started the build infra project, one of the design decisions was
> to use absolute paths everywhere to avoid having to keep track of the
> current directory, and to make all command lines in the build be simply
> copy and paste in a terminal to rerun.
>
> A consequence of this was that the __FILE__ macro then also expands to
> absolute paths. This made binary build comparisons much harder. Very
> often (especially in the build infra project itself) we use elaborate
> comparison methods to verify that a build change does not change the
> output of the build in any unwanted way. We then introduced the
> THIS_FILE macro to get rid of the absolute paths baked into our binaries
> which got rid of a huge source of binary noise preventing reproducible
> builds.
>
> Note that two different builds with slightly different output
> directories (or in the build-infra project case, completely different
> output structure for generated sources) will generate absolute source
> paths of different lengths. This will cause otherwise equivalent
> binaries to differ greatly due to different alignment, not just because
> of different contents in those strings.
>
> With this change, we could count on object files (at least for GCC) to
> always end up binary equivalent.
>
> In my long term vision, I would like to get the OpenJDK build even more
> reproducible, but it's currently not a high priority task. I would be
> very hard to convince of reducing the level of reproducible output we
> have though.
>
> Thanks for the background information. But as far as I can see, this
> currently only works because "THIS_FILE" is always empty which of
> course makes builds to various locations highly comparable :) On the
> other hand, HotSpot is not using THIS_FILE at all and "__FILE__" quite
> a lot.
>
> No, it's not. It will work just as well when THIS_FILE once more is fixed, 
> since
> /tmp/foo/src/java.base/.../fooimpl.c will have -DTHIS_FILE="fooimpl.c"
> just as
> /home/chthulu/puny_humans_projects/jdk/src/java.base/.../fooimpl.c will have 
> -DTHIS_FILE="fooimpl.c"
>
> So the builds of fooimpl.c will be identical. Or, at least, not dependent on 
> His R'lyehian Highness choice of directory names.
>
> /Magnus
>
>
>
> Don't get me wrong. I highly appreciate the feature of having absolute
> path names in the build to make all command lines in the build
> self-contained (I use that feature every day :). And I also support
> the goal of making builds even more reproducible. But does this goal
> not apply to hotspot (or is it just on the TODO list ?).
>
> In the end, I'm happy with the current, minimal fix which at least
> gets the logs working again.
>
> And maybe for the follow up change we should then better move all
> "__FILE__" occurrences in HotSpot to "THIS_FILE" instead of getting
> rid of "THIS_FILE"?
>
> Best regards,
> Volker
>
> /Erik
>
> On 2018-11-30 09:05, Volker Simonis wrote:
>
> Hi,
>
> can I please have a review for the following trivial fix:
>
> http://cr.openjdk.java.net/~simonis/webrevs/2018/8214534/
> https://bugs.openjdk.java.net/browse/JDK-8214534
>
> DISCLAIMER: "XS" refers to the size of the fix, not the size of the
> explanation :)
>
> Currently the compilation of native files defines "THIS_FILE" to hold
> the name of the current compilation unit. However, setting "THIS_FILE"
> in NativeCompilation.gmk is broken and results in "THIS_FILE" always
> being the empty string. I first thought that this is just a simple
> quoting issue, but after I couldn't get it working, I found the
> following explanation in the GNU Make manual [1]:
>
> "A common mistake is a

Re: RFR(XS): 8214534: Setting of THIS_FILE in the build is broken

2018-12-03 Thread Volker Simonis
Hi Magnus, Erik,

do I understand you correctly that you both are fine with my proposed
fix and that we leave all the other enhancements/discussion for the
future?

Thank you and best regards,
Volker

On Mon, Dec 3, 2018 at 12:27 PM Magnus Ihse Bursie
 wrote:
>
> On 2018-11-30 19:03, Volker Simonis wrote:
>
> On Fri, Nov 30, 2018 at 6:37 PM Erik Joelsson  
> wrote:
>
> Hello Volker,
>
> The fix looks good. Thanks for finding and fixing it!
>
> Thanks!
>
> Now for some history on why THIS_FILE. The short story is that it's for
> more reproducible and comparable builds.
>
> When we started the build infra project, one of the design decisions was
> to use absolute paths everywhere to avoid having to keep track of the
> current directory, and to make all command lines in the build be simply
> copy and paste in a terminal to rerun.
>
> A consequence of this was that the __FILE__ macro then also expands to
> absolute paths. This made binary build comparisons much harder. Very
> often (especially in the build infra project itself) we use elaborate
> comparison methods to verify that a build change does not change the
> output of the build in any unwanted way. We then introduced the
> THIS_FILE macro to get rid of the absolute paths baked into our binaries
> which got rid of a huge source of binary noise preventing reproducible
> builds.
>
> Note that two different builds with slightly different output
> directories (or in the build-infra project case, completely different
> output structure for generated sources) will generate absolute source
> paths of different lengths. This will cause otherwise equivalent
> binaries to differ greatly due to different alignment, not just because
> of different contents in those strings.
>
> With this change, we could count on object files (at least for GCC) to
> always end up binary equivalent.
>
> In my long term vision, I would like to get the OpenJDK build even more
> reproducible, but it's currently not a high priority task. I would be
> very hard to convince of reducing the level of reproducible output we
> have though.
>
> Thanks for the background information. But as far as I can see, this
> currently only works because "THIS_FILE" is always empty which of
> course makes builds to various locations highly comparable :) On the
> other hand, HotSpot is not using THIS_FILE at all and "__FILE__" quite
> a lot.
>
> No, it's not. It will work just as well when THIS_FILE once more is fixed, 
> since
> /tmp/foo/src/java.base/.../fooimpl.c will have -DTHIS_FILE="fooimpl.c"
> just as
> /home/chthulu/puny_humans_projects/jdk/src/java.base/.../fooimpl.c will have 
> -DTHIS_FILE="fooimpl.c"
>
> So the builds of fooimpl.c will be identical. Or, at least, not dependent on 
> His R'lyehian Highness choice of directory names.
>
> /Magnus
>
>
>
> Don't get me wrong. I highly appreciate the feature of having absolute
> path names in the build to make all command lines in the build
> self-contained (I use that feature every day :). And I also support
> the goal of making builds even more reproducible. But does this goal
> not apply to hotspot (or is it just on the TODO list ?).
>
> In the end, I'm happy with the current, minimal fix which at least
> gets the logs working again.
>
> And maybe for the follow up change we should then better move all
> "__FILE__" occurrences in HotSpot to "THIS_FILE" instead of getting
> rid of "THIS_FILE"?
>
> Best regards,
> Volker
>
> /Erik
>
> On 2018-11-30 09:05, Volker Simonis wrote:
>
> Hi,
>
> can I please have a review for the following trivial fix:
>
> http://cr.openjdk.java.net/~simonis/webrevs/2018/8214534/
> https://bugs.openjdk.java.net/browse/JDK-8214534
>
> DISCLAIMER: "XS" refers to the size of the fix, not the size of the
> explanation :)
>
> Currently the compilation of native files defines "THIS_FILE" to hold
> the name of the current compilation unit. However, setting "THIS_FILE"
> in NativeCompilation.gmk is broken and results in "THIS_FILE" always
> being the empty string. I first thought that this is just a simple
> quoting issue, but after I couldn't get it working, I found the
> following explanation in the GNU Make manual [1]:
>
> "A common mistake is attempting to use $@ within the prerequisites
> list; this will not work. However, there is a special feature of GNU
> make, secondary expansion (see Secondary Expansion), which will allow
> automatic variable values to be used in prerequisite lists."
>
> I'm not a Make expert, but I think this quote doesn't apply to "$@"
> only, but to all 

Re: RFR(XS): 8214534: Setting of THIS_FILE in the build is broken

2018-11-30 Thread Volker Simonis
On Fri, Nov 30, 2018 at 6:37 PM Erik Joelsson  wrote:
>
> Hello Volker,
>
> The fix looks good. Thanks for finding and fixing it!
>

Thanks!

> Now for some history on why THIS_FILE. The short story is that it's for
> more reproducible and comparable builds.
>
> When we started the build infra project, one of the design decisions was
> to use absolute paths everywhere to avoid having to keep track of the
> current directory, and to make all command lines in the build be simply
> copy and paste in a terminal to rerun.
>
> A consequence of this was that the __FILE__ macro then also expands to
> absolute paths. This made binary build comparisons much harder. Very
> often (especially in the build infra project itself) we use elaborate
> comparison methods to verify that a build change does not change the
> output of the build in any unwanted way. We then introduced the
> THIS_FILE macro to get rid of the absolute paths baked into our binaries
> which got rid of a huge source of binary noise preventing reproducible
> builds.
>
> Note that two different builds with slightly different output
> directories (or in the build-infra project case, completely different
> output structure for generated sources) will generate absolute source
> paths of different lengths. This will cause otherwise equivalent
> binaries to differ greatly due to different alignment, not just because
> of different contents in those strings.
>
> With this change, we could count on object files (at least for GCC) to
> always end up binary equivalent.
>
> In my long term vision, I would like to get the OpenJDK build even more
> reproducible, but it's currently not a high priority task. I would be
> very hard to convince of reducing the level of reproducible output we
> have though.
>

Thanks for the background information. But as far as I can see, this
currently only works because "THIS_FILE" is always empty which of
course makes builds to various locations highly comparable :) On the
other hand, HotSpot is not using THIS_FILE at all and "__FILE__" quite
a lot.

Don't get me wrong. I highly appreciate the feature of having absolute
path names in the build to make all command lines in the build
self-contained (I use that feature every day :). And I also support
the goal of making builds even more reproducible. But does this goal
not apply to hotspot (or is it just on the TODO list ?).

In the end, I'm happy with the current, minimal fix which at least
gets the logs working again.

And maybe for the follow up change we should then better move all
"__FILE__" occurrences in HotSpot to "THIS_FILE" instead of getting
rid of "THIS_FILE"?

Best regards,
Volker

> /Erik
>
> On 2018-11-30 09:05, Volker Simonis wrote:
> > Hi,
> >
> > can I please have a review for the following trivial fix:
> >
> > http://cr.openjdk.java.net/~simonis/webrevs/2018/8214534/
> > https://bugs.openjdk.java.net/browse/JDK-8214534
> >
> > DISCLAIMER: "XS" refers to the size of the fix, not the size of the
> > explanation :)
> >
> > Currently the compilation of native files defines "THIS_FILE" to hold
> > the name of the current compilation unit. However, setting "THIS_FILE"
> > in NativeCompilation.gmk is broken and results in "THIS_FILE" always
> > being the empty string. I first thought that this is just a simple
> > quoting issue, but after I couldn't get it working, I found the
> > following explanation in the GNU Make manual [1]:
> >
> > "A common mistake is attempting to use $@ within the prerequisites
> > list; this will not work. However, there is a special feature of GNU
> > make, secondary expansion (see Secondary Expansion), which will allow
> > automatic variable values to be used in prerequisite lists."
> >
> > I'm not a Make expert, but I think this quote doesn't apply to "$@"
> > only, but to all automatic variables. The proposed solution (i.e.
> > "Secondary Expansion" [2]) seems overly complex for this problem. I
> > think the solution in the patch is much simpler and works "just as
> > well" :)
> >
> > The other question is of course why do we need "THIS_FILE" at all? It
> > is used for various native logs in the class library (not in HotSpot)
> > which use the value of "THIS_FILE" to decorate their output with the
> > current file name. On the other hand, we already have the standard,
> > predefined "__FILE__" macro which could be used instead (and indeed,
> > if "THIS_FILE" isn't defined, the various logging routines fall back
> > to using "__FILE__"

RFR(XS): 8214534: Setting of THIS_FILE in the build is broken

2018-11-30 Thread Volker Simonis
Hi,

can I please have a review for the following trivial fix:

http://cr.openjdk.java.net/~simonis/webrevs/2018/8214534/
https://bugs.openjdk.java.net/browse/JDK-8214534

DISCLAIMER: "XS" refers to the size of the fix, not the size of the
explanation :)

Currently the compilation of native files defines "THIS_FILE" to hold
the name of the current compilation unit. However, setting "THIS_FILE"
in NativeCompilation.gmk is broken and results in "THIS_FILE" always
being the empty string. I first thought that this is just a simple
quoting issue, but after I couldn't get it working, I found the
following explanation in the GNU Make manual [1]:

"A common mistake is attempting to use $@ within the prerequisites
list; this will not work. However, there is a special feature of GNU
make, secondary expansion (see Secondary Expansion), which will allow
automatic variable values to be used in prerequisite lists."

I'm not a Make expert, but I think this quote doesn't apply to "$@"
only, but to all automatic variables. The proposed solution (i.e.
"Secondary Expansion" [2]) seems overly complex for this problem. I
think the solution in the patch is much simpler and works "just as
well" :)

The other question is of course why do we need "THIS_FILE" at all? It
is used for various native logs in the class library (not in HotSpot)
which use the value of "THIS_FILE" to decorate their output with the
current file name. On the other hand, we already have the standard,
predefined "__FILE__" macro which could be used instead (and indeed,
if "THIS_FILE" isn't defined, the various logging routines fall back
to using "__FILE__").

The only explanation I could come up for having "THIS_FILE" until now
is that "__FILE__" may contain the full path name of the compilation
unit while we only want the simple file name (without path) in the
log. However, in order to solve this "path" problem, we can use
simpler solutions.

Some call sites (e.g.
"src/jdk.jdwp.agent/share/native/libjdwp/log_messages.h") already use
helper functions like "file_basename()" to cut off a potential path
component from "THIS_FILE". Other call sites (e.g.
"src/java.instrument/share/native/libinstrument/JPLISAssert.h" or
"src/jdk.jdwp.agent/share/native/libjdwp/error_messages.h") currently
simply use the value of "THIS_FILE" directly. But they could be easily
fixed by either using "file_basename()" there as well or even simpler,
wrapping "__FILE__" into another macro which calls "strrchr()" to do
the same work.

So as a follow up to this change, I'd like to propose another change
which completely removes "THIS_FILE" and changes all users of
"THIS_FILE" to use "__FILE__" instead. This would also shorten our
compile command lines (which doesn't happen too often :) What do you
think?

Thank you and best regards,
Volker

[1] https://www.gnu.org/software/make/manual/html_node/Automatic-Variables.html
[2] 
https://www.gnu.org/software/make/manual/html_node/Secondary-Expansion.html#Secondary-Expansion


Re: RFR(XS): 8214063: [AIX] Disable symbol visibility flags

2018-11-29 Thread Volker Simonis
On Thu, Nov 29, 2018 at 12:20 PM Magnus Ihse Bursie
 wrote:
>
> On 2018-11-27 16:33, Volker Simonis wrote:
>
> > Hi,
> >
> > can I please have a review for the following trivial change which
> > simply disables the symbol visibility flags on AIX:
> >
> > http://cr.openjdk.java.net/~simonis/webrevs/2018/8214063/
> > https://bugs.openjdk.java.net/browse/JDK-8214063
> Looks good to me. I am sorry for the mess I caused by optimisically
> trying to fix things on a platform I could not compile on... :(
>

Thanks for the review and don't worry! We really appreciate your
continued help. It's really us who should have tested and spotted the
problems earlier :)

Regards,
Volker

> This also reminds me that the visibility flags *really* should move into
> configure/spec, not be sprinkled like this in the make files.
>
> /Magnus
> >
> > Change "8202322: AIX: symbol visibility flags not support on xlc 12.1"
> > [1] blindly introduced these flags for all xlC compiler versions >
> > 12.1 without ever testing it (which should not have happened). Now
> > that people are starting to really use xlC 13 it turns out that there
> > is more to do than just enabling the flags. This future work is
> > covered by "8204541: Correctly support AIX xlC 13.1 symbol visibility
> > flags".
> >
> > Thank you and best regards,
> > Volker
> >
> > [1] https://bugs.openjdk.java.net/browse/JDK-8202322
> > [2] https://bugs.openjdk.java.net/browse/JDK-8204541
>


Re: RFR(XS): 8214063: [AIX] Disable symbol visibility flags

2018-11-27 Thread Volker Simonis
On Tue, Nov 27, 2018 at 4:56 PM Baesken, Matthias
 wrote:
>
> > Change "8202322: AIX: symbol visibility flags not support on xlc 12.1"
> > [1] blindly introduced these flags for all xlC compiler versions >
>
> Hello,  probably you did not look correctly at  my  change 8202322  .
>
> It   did NOT "blindly"  introduce  visibility flags  for   all xlC  
> compilers, but Instead it   ***removed***   the flags  in cases  where   xlc 
> 12.1 was used
> (because they generated  a TON of warnings with xlc 12.1 ,  which  messed up 
> the build log  and made  build  operations/analysis a pain).
>

You're right. They were "blindly integrated" by "8200358: Remove
mapfiles for JDK executables" and not completely removed by 8202322.
Sorry for the confusion.

> See the description :
> https://bugs.openjdk.java.net/browse/JDK-8202322
>
> "For a while the OpenJDK build on AIX attempts to set symbol visibility 
> related flags; however they are not supported on our standard xlc version 
> 12.1 so they generate a lot of warnings."
>
>
> Please see :
>
> http://hg.openjdk.java.net/jdk/jdk/rev/98f57dff16f3
>
>
> > Now  that people are starting to really use xlC 13 it turns out that there
> > is more to do than just enabling the flags
>
> It was pretty  clear from the  beginning  that  with  higher xlc versions  
> than 12.1   the chance  that it works "out of the box"  is low .
>
> Regards, Matthias
>
>
>
> > -Original Message-
> > From: ppc-aix-port-dev  On
> > Behalf Of Volker Simonis
> > Sent: Dienstag, 27. November 2018 16:34
> > To: build-dev ; ppc-aix-port-
> > d...@openjdk.java.net
> > Cc: adam.far...@uk.ibm.com
> > Subject: RFR(XS): 8214063: [AIX] Disable symbol visibility flags
> >
> > Hi,
> >
> > can I please have a review for the following trivial change which
> > simply disables the symbol visibility flags on AIX:
> >
> > http://cr.openjdk.java.net/~simonis/webrevs/2018/8214063/
> > https://bugs.openjdk.java.net/browse/JDK-8214063
> >
> > Change "8202322: AIX: symbol visibility flags not support on xlc 12.1"
> > [1] blindly introduced these flags for all xlC compiler versions >
> > 12.1 without ever testing it (which should not have happened). Now
> > that people are starting to really use xlC 13 it turns out that there
> > is more to do than just enabling the flags. This future work is
> > covered by "8204541: Correctly support AIX xlC 13.1 symbol visibility
> > flags".
> >
> > Thank you and best regards,
> > Volker
> >
> > [1] https://bugs.openjdk.java.net/browse/JDK-8202322
> > [2] https://bugs.openjdk.java.net/browse/JDK-8204541


RFR(XS): 8214063: [AIX] Disable symbol visibility flags

2018-11-27 Thread Volker Simonis
Hi,

can I please have a review for the following trivial change which
simply disables the symbol visibility flags on AIX:

http://cr.openjdk.java.net/~simonis/webrevs/2018/8214063/
https://bugs.openjdk.java.net/browse/JDK-8214063

Change "8202322: AIX: symbol visibility flags not support on xlc 12.1"
[1] blindly introduced these flags for all xlC compiler versions >
12.1 without ever testing it (which should not have happened). Now
that people are starting to really use xlC 13 it turns out that there
is more to do than just enabling the flags. This future work is
covered by "8204541: Correctly support AIX xlC 13.1 symbol visibility
flags".

Thank you and best regards,
Volker

[1] https://bugs.openjdk.java.net/browse/JDK-8202322
[2] https://bugs.openjdk.java.net/browse/JDK-8204541


RFR(S): 8214343: Handle the absence of Xrandr more generically

2018-11-27 Thread Volker Simonis
Hi,

can I please have a review for the following trivial change which
handles the absence of Xrandr more generically:

http://cr.openjdk.java.net/~simonis/webrevs/2018/8214343/
https://bugs.openjdk.java.net/browse/JDK-8214343

Change JDK-8213944 fixed the build on AIX (which has no Xrandr) by
conditionally excluding the relevant parts on AIX with the help of
preprocessor defines. On the mailing lists the wish was expressed to
handle the absence of Xrandr more generically during the configure
step, so here comes the corresponding change.

In contrast to the suggestions on the previous mail thread I define
"NO_XRENDER" if we're configuring on AIX. This avoids clobbering all
the other platforms which support Xrandr with yet another command line
define of the form "-DHAVE_XRANDR". Instead, only the corresponding
compiler options on AIX will now contain "-DNO_XRANDR". Other
platforms which don't support Xrandr can now easily define NO_XRANDR
as well.

Thank you and best regards,
Volker


Re: RFR(S): 8213698: Improve devkit creation and add support for linux/ppc64/ppc64le/s390x

2018-11-22 Thread Volker Simonis
On Tue, Nov 20, 2018 at 1:46 PM Magnus Ihse Bursie
 wrote:
>
> Hi Volker,
>
> Glad to see you fix and clean up the devkit scripts! And you're more
> than welcome to add documentation to the building.md.
>

OK, I've added a section about devkits to build.{md,html}. Please have
a look and let me know what you think. Any comments are welcome :)

http://cr.openjdk.java.net/~simonis/webrevs/2018/8213698.v2/

Please find my comments to the other changes inline

>
> I think all your changes are good, and can go in as-is, but I see some
> further potential for cleanup, so if you feel like some more fixes while
> you're at it, please go ahead. Otherwise you can ignore the rest of what
> I'm writing. :)
>
> > make cross_compile_target="ppc64-linux-gnu" BASE_OS=Fedora 
> > BASE_OS_VERSION=17
> For consistency, maybe we should rename cross_compile_target to TARGET?
> At least as written on the make command line; you can keep the
> cross_compile_target name in the Makefile itself, and only assign to it
> from TARGET.
>

I've renamed it to TARGETS because it can actually take a whole list
of targets (and also to distinguish it from the TARGET parameter used
in Tools.gmk which only accepts a single TARGET).

> > OEL_URL :=
> > https://archives.fedoraproject.org/pub/archive/$(FEDORA_TYPE)/releases/$(BASE_OS_VERSION)/Everything/$(ARCH)/os/Packages/
> Maybe it's time to rename this to something more distro-agnostic than
> OEL_URL...
>

Renamed it to BASE_URL

> > PREFIX=$(RESULT)/$@-to-$$p
> I really like this! Devkits are much more better described as X-to-Y
> (even if that was not originally the case, as Erik said).
>

Thanks :)

> > Notice that we have to build "ccache" for each target because ccache
> > will be installed into the directory specified by "--prefix" at
> > configure time and this is now different for every target. However
> > that's not a big problem, because the time for compiling ccache is
> > negligible compared to the download time of the RPMs and the build
> > time of GCC.
> Is it still worth the effort of compiling ccache? At least in Oracle,
> we're not using it anymore. OTOH, if it works, we can just keep it
> there. It's no big cost.
>

I'm not using it either, but I leave it in for now.

>
> > - Cleanup arch selection:
> >
> >   ifeq ($(ARCH),x86_64)
> > -  RPM_ARCHS := x86_64 noarch
> > +  RPM_ARCHS := $(ARCH) noarch
> > ifeq ($(BUILD),$(HOST))
> >   ifeq ($(TARGET),$(HOST))
> > # When building the native compiler for x86_64, enable mixed mode.
> > @@ -199,7 +206,7 @@
> >   endif
> > endif
> >   else ifeq ($(ARCH),i686)
> > -  RPM_ARCHS := i386 i686 noarch
> > +  RPM_ARCHS := $(ARCH) i386 noarch
> >   else ifeq ($(ARCH), armhfp)
> Looks like you could always to RPM_ARCHS := $(ARCH) noarch, and then use
> += for the few platforms that need additionals RPM archs.
>

Done.

> There was something more I thought of when I reviewed your changes, but
> it slipped my mind now.
>

There's still room for improvements :) For example the GCC version
could be made configurable, as well as the OEL version. But I leave
that to the person who updates the compiler the next time :)

> /Magnus
>
>
> On 2018-11-12 12:19, Volker Simonis wrote:
> > Hi,
> >
> > can I please have a review for the following change which ads support
> > for linux/ppc64/ppc64le/s390x devkits and hopefully improves the
> > creation of devkits a little bit :)
> >
> > http://cr.openjdk.java.net/~simonis/webrevs/2018/8213698/
> > https://bugs.openjdk.java.net/browse/JDK-8213698
> >
> > With these changes it becomes possible to say any of the following:
> >
> > make cross_compile_target="ppc64le-linux-gnu s390x-linux-gnu" BASE_OS=Fedora
> > make cross_compile_target="ppc64-linux-gnu" BASE_OS=Fedora 
> > BASE_OS_VERSION=17
> > make onlytars cross_compile_target="ppc64-linux-gnu ppc64le-linux-gnu
> > s390x-linux-gnu"
> >
> > and get the following devkits under "build/devkit/result":
> >
> > sdk-x86_64-unknown-linux-gnu-to-ppc64le-linux-gnu-20181112.tar.gz
> > sdk-x86_64-unknown-linux-gnu-to-ppc64-linux-gnu-20181112.tar.gz
> > sdk-x86_64-unknown-linux-gnu-to-s390x-linux-gnu-20181112.tar.gz
> > x86_64-unknown-linux-gnu-to-ppc64le-linux-gnu/
> > x86_64-unknown-linux-gnu-to-ppc64-linux-gnu/
> > x86_64-unknown-linux-gnu-to-s390x-linux-gnu/
> >
> > Below you can find a more detailed description of the various changes.
> > Once we've discussed and agreed on the changes I'd like to add a small
> > documentation about how 

Re: RFR: JDK-8214063: OpenJDK will not build on AIX while using the xlc 13.1 compiler

2018-11-21 Thread Volker Simonis
On Wed, Nov 21, 2018 at 12:44 PM Magnus Ihse Bursie
 wrote:
>
>
>
> On 2018-11-20 18:50, Volker Simonis wrote:
> > On Tue, Nov 20, 2018 at 6:15 PM Thomas Stüfe  
> > wrote:
> >> On Tue, Nov 20, 2018 at 6:12 PM Adam Farley8  
> >> wrote:
> >>> Heya Tom,
> >>>
> >>> "In JDK11 and JDK12, source files are compiled with -qvisibility=hidden
> >>> when using xlc version other than 12.1. That doesn't seem to play well
> >>> with link option -bexpall. "
> >>>
> >>> Found that buried in one of the associated Git issues. It appears that
> >>> it's OpenJDK's use of that option that's causing the problem, though
> >>> I couldn't speculate as to why it was added in the first place.
> >>>
> >>> I see this has also been noted in 
> >>> https://bugs.openjdk.java.net/browse/JDK-8204541
> >>>
> >>> Does that answer your question?
> >>>
> >> Yes, Thank you. Odd. Will have to do archeology on that one.
> >>
> > No I begin to understand the problem as well :)
> >
> > It was actually change "8202322: AIX: symbol visibility flags not
> > support on xlc 12.1" [1] which introduced "-qvisibility=hidden" for
> > XLC version not equal to 12.1. That's kind of a weak check and I
> > suppose nobody has ever tested this change with an XLC version other
> > than 12.1 (until you came along :). Maybe that check should be a more
> > precisly check for >= 13.1 (but I know such version checks are hard to
> > do in Makefile syntax)?
> In configure (where, ideally, all version checks should be made),
> there's the TOOLCHAIN_CHECK_COMPILER_VERSION function, which supports
> #   IF_AT_LEAST:   block to run if the compiler is at least this version
> (>=)
> #   IF_OLDER_THAN:   block to run if the compiler is older than this
> version (<)
> for normal, dot-separated version number schemes.
>

Thanks for the pointer. I was looking for this functionality yesterday
but couldn't find it :)

> /Magnus
>
> >
> > The thing I don't understand about your patch (the changes in
> > "jni_md.h" look good although I haven't tested them) is why you need
> > the extra changes in NativeImageBuffer.cpp?
> > "jdk_internal_jimage_NativeImageBuffer.h" is a plain, generated JNI
> > header file. If XLC 13 has problems to parse it, there should be much
> > more places which need fixing. I think that part of your change needs
> > a closer evaluation.
> >
> > Thank you and best regards,
> > Volker
> >
> > [1] https://bugs.openjdk.java.net/browse/JDK-8202322
> >
> >> ..Thomas
> >>
> >>> Best Regards
> >>>
> >>> Adam Farley
> >>> IBM Runtimes
> >>>
> >>>
> >>> "Thomas Stüfe"  wrote on 20/11/2018 16:44:07:
> >>>
> >>>> From: "Thomas Stüfe" 
> >>>> To: Adam Farley8 
> >>>> Cc: Java Core Libs 
> >>>> Date: 20/11/2018 16:48
> >>>> Subject: Re: RFR: JDK-8214063: OpenJDK will not build on AIX while
> >>>> using the xlc 13.1 compiler
> >>>>
> >>>> Hi Adam,
> >>>>
> >>>> On Tue, Nov 20, 2018 at 5:12 PM Adam Farley8  
> >>>> wrote:
> >>>>> Hi Tom,
> >>>>>
> >>>>> Sounds reasonable. I've added a webex to the bug, and here's a
> >>>> link to the bug.
> >>>>> https://urldefense.proofpoint.com/v2/url?
> >>>> u=https-3A__bugs.openjdk.java.net_browse_JDK-2D8214063=DwIFaQ=jf_iaSHvJObTbx-
> >>>> siA1ZOg=P5m8KWUXJf-
> >>>> CeVJc0hDGD9AQ2LkcXDC0PMV9ntVw5Ho=z8YYwBXEfN7UtX1suPjpp9CZSHf8v0GrIMK3XGIC9VY=81TP9mIjhYD2Hmt8g7p2EHWRZXgiep21hxKLYRU7zIQ=
> >>>>> This patch is required because otherwise, when building on AIX
> >>>> using xlc 3.1,
> >>>>> the build fails with this error:
> >>>>>
> >>>>> "Visibility is not allowed on a reference to an imported symbol."
> >>>>>
> >>>>> We believe this is caused by JNIEXPORT and JNIIMPORT not being
> >>>> defined. Without
> >>>>> this, almost no symbols are exported from shared libraries due to use of
> >>>>> -qvisibility=hidden as specified in make/lib/LibCommon.gmk.
> >>>> Yes but what I try to understand is why does this happen now with
> >>>> xlc13? Did xlc change th

Re: [OpenJDK 2D-Dev] RFR(XS): 8213944: Fix AIX build after the removal of Xrandr.h and add a configure check for it

2018-11-20 Thread Volker Simonis
Thanks everybody for the reviews.

If nobody raises a "Veto" (Phil?) I plan to push this fix tomorrow in
its current form.

I've also run it through the submit repo and got an error on Windows
for the test "runtime/modules/JVMDefineModule.java" which seems
completely unrelated to my change which only touches the X11
implementation on Unix. Can somebody please confirm that?

[Mach5] mach5-one-simonis-JDK-8213944-20181120-1629-11082: FAILED,
Failed tests: 1

runtime/modules/JVMDefineModule.java tier1 windows-x64-debug othervm
driver ExitCode: -1073741819

Mach5 Tasks Results Summary

UNABLE_TO_RUN: 0
FAILED: 0
EXECUTED_WITH_FAILURE: 1
KILLED: 0
PASSED: 75
NA: 0

Thank you and best regards,
Volker

On Tue, Nov 20, 2018 at 12:05 PM Magnus Ihse Bursie
 wrote:
>
>
>
> On 2018-11-19 18:56, Volker Simonis wrote:
> > Hi Phil,
> >
> > I'd like to kindly ask you to suggest how we can proceed with this issue.
> >
> > As I wrote before, Xrandr is not officially supported on AIX and there
> > are no official packages available for it. There are some OpenSource
> > sites for AIX which provide Xrandr, but they are all not compatible
> > with the default native libraries (e.g. the open source Xrandr package
> > depends on another open source package of Xrender.so.1 but the system
> > only provides Xrender.so.0 natively). We can't compile the whole JDK
> > (i.e. libawt_xawt.so) against some open source package of Xrender.so.1
> > because that simply won't be available on the majority of systems.
> >
> > Remember that forcing people to install these open-source packages
> > even as a build dependency is like expecting Linux users to install
> > some non-official packages just to be able to build. Especially in
> > corporate environments that's not easy. Moreover the benefit would be
> > really minimal, because the Xrandr functionality won't be available at
> > runtime anyway.
> >
> > So to cut a long story short, I see two options:
> >
> > 1. Go with my current patch (ugly but efficient)
> >
> > 2. Check-in in an AIX specific version of XRander.h/randr.h under
> > src/java.desktop/aix (OK for me, but that would actually negate the
> > initial purpose of "8210863: Remove Xrandr include files")
> >
> > Do you have a better proposal?
> I think the change look good, and I vote for strategy 1. As Thomas
> suggested, if the AIX ifdefs look bad we can create a new define, but
> I'm not sure that's really helpful - after all, it's just on AIX we
> currently have no r Having a define would mostly be needed if it was
> multiple OSes, or similar more complex situations, that would have/not
> have the r extension.
>
> Yet another solution, to get rid of the ifdefs, is to move the relevant
> Xranrd dependent functions into a new, separate file, like
> awt_GraphicsEnv_randr.c, and then in the build exclude it on AIX (or,
> perhaps if it's worth the trouble, on all platforms where configure did
> not find Xrandr).
>
> /Magnus
>
> >
> > Thank you and best regards,
> > Volker
> >
> > On Fri, Nov 16, 2018 at 11:22 AM Volker Simonis
> >  wrote:
> >> On Thu, Nov 15, 2018 at 6:01 PM Philip Race  wrote:
> >>> PS I am not sure why xrandr headers would not be available for AIX.
> >>> They are a standard part of the xdistribution.
> >>>
> >> I'm not an X11 guru, but as far as I understand, xrandr is an
> >> extension and as such it doesn't have to be supported by every
> >> implementation. To the best of my knowledge (I've just started another
> >> poll among some experts) AIX doesn't support Xrandr and does not have
> >> the corresponding headers.
> >>
> >>> If true, think what you are going to have to do is add a
> >>> --with-xrandr-include option
> >>> and provide it that way.
> >>>
> >> What if there are no standard Xrandr headers on a platform? Do you
> >> really want to force users to get them from some dubious sources just
> >> for building the OpenJDK? Sorry, but I don't think that's a good
> >> solution. Than I'd rather prefer the ugly ifdefs (or I check the
> >> headers back in again in an AIX-specific directory :)
> >>
> >> Thank you and best regards,
> >> Volker
> >>
> >>> -phil.
> >>>
> >>> On 11/15/18, 8:55 AM, Philip Race wrote:
> >>>> Hmm. I don't like the ifdefs.
> >>>>
> >>>> Xrandr is a requirement for the build. If its not there at runtime
> >>>> that's OK.
> >>>>
> >>>> -phil.
> >&g

RFR(XS): 8214120: [REDO] Fix sun.awt.nativedebug on X11 platforms

2018-11-20 Thread Volker Simonis
Hi,

so here's a reworked version of my previous change:

http://cr.openjdk.java.net/~simonis/webrevs/2018/8214120/
https://bugs.openjdk.java.net/browse/JDK-8214120

In addition to my initial change (which unfortunately broke the
Solaris build) the new version has to JNIEXPORT the following methods
from libawt.so otherwise they won't be visible in libawt_xawt.so on
some platforms which make symbols local by default (e.g. Solaris):

DAssert_Impl
DTrace_PrintFunction
DTrace_VPrint
DTrace_VPrintln

Local tests on Linux/AIX and Solaris succeeded. The patch is currently
building in the submit repo.

Thank you and best regards,
Volker


Re: [OpenJDK 2D-Dev] RFR(XS): 8213944: Fix AIX build after the removal of Xrandr.h and add a configure check for it

2018-11-19 Thread Volker Simonis
Hi Phil,

I'd like to kindly ask you to suggest how we can proceed with this issue.

As I wrote before, Xrandr is not officially supported on AIX and there
are no official packages available for it. There are some OpenSource
sites for AIX which provide Xrandr, but they are all not compatible
with the default native libraries (e.g. the open source Xrandr package
depends on another open source package of Xrender.so.1 but the system
only provides Xrender.so.0 natively). We can't compile the whole JDK
(i.e. libawt_xawt.so) against some open source package of Xrender.so.1
because that simply won't be available on the majority of systems.

Remember that forcing people to install these open-source packages
even as a build dependency is like expecting Linux users to install
some non-official packages just to be able to build. Especially in
corporate environments that's not easy. Moreover the benefit would be
really minimal, because the Xrandr functionality won't be available at
runtime anyway.

So to cut a long story short, I see two options:

1. Go with my current patch (ugly but efficient)

2. Check-in in an AIX specific version of XRander.h/randr.h under
src/java.desktop/aix (OK for me, but that would actually negate the
initial purpose of "8210863: Remove Xrandr include files")

Do you have a better proposal?

Thank you and best regards,
Volker

On Fri, Nov 16, 2018 at 11:22 AM Volker Simonis
 wrote:
>
> On Thu, Nov 15, 2018 at 6:01 PM Philip Race  wrote:
> >
> > PS I am not sure why xrandr headers would not be available for AIX.
> > They are a standard part of the xdistribution.
> >
>
> I'm not an X11 guru, but as far as I understand, xrandr is an
> extension and as such it doesn't have to be supported by every
> implementation. To the best of my knowledge (I've just started another
> poll among some experts) AIX doesn't support Xrandr and does not have
> the corresponding headers.
>
> > If true, think what you are going to have to do is add a
> > --with-xrandr-include option
> > and provide it that way.
> >
>
> What if there are no standard Xrandr headers on a platform? Do you
> really want to force users to get them from some dubious sources just
> for building the OpenJDK? Sorry, but I don't think that's a good
> solution. Than I'd rather prefer the ugly ifdefs (or I check the
> headers back in again in an AIX-specific directory :)
>
> Thank you and best regards,
> Volker
>
> > -phil.
> >
> > On 11/15/18, 8:55 AM, Philip Race wrote:
> > > Hmm. I don't like the ifdefs.
> > >
> > > Xrandr is a requirement for the build. If its not there at runtime
> > > that's OK.
> > >
> > > -phil.
> > >
> > > On 11/15/18, 8:06 AM, Volker Simonis wrote:
> > >> Hi,
> > >>
> > >> can I please have a review for the following small change:
> > >>
> > >> http://cr.openjdk.java.net/~simonis/webrevs/2018/8213944/
> > >> https://bugs.openjdk.java.net/browse/JDK-8213944
> > >>
> > >> Change JDK-8210863 removed the Xrandr.h/randr.h headers from the
> > >> OpenJDK sources but forgot to add a configure check for the Xrandr
> > >> extension which is now a build dependency.
> > >>
> > >> The change also broke the AIX build. AIX never supported Xrandr, but
> > >> that was only detected at runtime, when the JDK was unable to
> > >> dynamically load libXrand.so. Now, without Xrandr.h/randr.h in the
> > >> source tree any more, we have to conditionally compile some parts of
> > >> src/java.desktop/unix/native/libawt_xawt/awt/awt_GraphicsEnv.c such
> > >> that it doesn't require the definitions and declarations from
> > >> Xrandr.h/randr.h any more.
> > >>
> > >> Thank you and best regards,
> > >> Volker


Re: RFC: JEP JDK-8208089: Implement C++14 Language Features

2018-11-19 Thread Volker Simonis
On Mon, Nov 19, 2018 at 9:00 AM Kim Barrett  wrote:
>
> > On Nov 19, 2018, at 2:04 AM, Kim Barrett  wrote:
> >
> >> On Nov 19, 2018, at 1:31 AM, David Holmes  wrote:
> >> I think it is important that all the port owners buy into this.
> >
> > At least one port (aix_ppc) presently seems to have no way to support this 
> > change, because
> > the compiler being used is seriously deficient and appears to be 
> > languishing.  (It doesn’t even
> > support C++11, let alone C++14.)  I think the community could (and in my 
> > opinion, should)
> > chose to move ahead despite that.  If a new and adequate compiler is 
> > expected “soon” then
> > the community might choose to wait, or might proceed and let that port 
> > languish until the new
> > compiler is available.  I think that’s all part of the discussion that 
> > should happen around the
> > targeting of this JEP.  I hope this inadequate compiler on a relatively 
> > niche platform won’t be
> > an indefinite blocker in this area.
>
> Here’s what Volker said on the build-dev list, 2017-07-19, subject 
> “C++11/C++14 support in XLC ?”
>
> 
> - SAP is currently maintaining the AIX port in the OpenJDK and we're
> willing to do that in the future. But we're not IBM and we can not
> decide about the XLC feature list. If Oracle and the OpenJDK community
> finally decide to use C++11/14 features which are not available in XLC
> we have to live with that. We can either escalate the XLC deficiencies
> to IBM and suspend the port until the compiler gets fixed. Or we can
> switch the port to use the GCC tool chain with all the pros (bigger
> compatibility with Linux platforms) and cons (porting effort, testing,
> compatibility with other AIX software compiled with XLC, compiler
> support). While the GCC alternative sounds very appealing at a first
> glance it really isn't that perfect in reality, especially not for our
> commercial SAP JVM version of OpenJDK. One problem is the fact that
> there's no official support for GCC on AIX, the other is
> compatibility. Just think you had to replace Solaris Studio by GCC on
> Solaris :)
> 
>
> He had more to say on the general topic at the time.  I don’t know if anything
> has changed in the intervening nearly 1 1/2 years.
>

Not really. IBM is working on a new compiler which is in Beta now. But
neither have I tested it nor do I know exactly if it will have full
C++11/14 support.

As I wrote before, this is a chicken/egg problem which I can not solve
and I agree that it shouldn't be a blocker for OpenJDK. Therefore,
please go ahead and use whichever feature the OpenJDK community agrees
upon.

Regards,
Volker


RFR(XXS): 8214007: Fix sun.awt.nativedebug on X11 platforms

2018-11-16 Thread Volker Simonis
Hi,

can I please have a review for the following trivial fix:

http://cr.openjdk.java.net/~simonis/webrevs/2018/8214007/
https://bugs.openjdk.java.net/browse/JDK-8214007

AWT supports some kind of native logging which can be enabled with
"-Dsun.awt.nativedebug=true -Dawtdebug.ctrace=true".

Unfortunately this doesn't work on X platforms any more because both,
libawt and libawt_xawt end up with a copy of debug_trace.o. Among
others, debug_trace.o contains the static variable
GlobalTracingEnabled which denotes the tracing state. This obviously
can't work if the final executable contains several instances of this
variable.

The fix is trivial. Remove "common/awt/debug" from the set of sources
for libawt_xawt. libawt_xawt is linked against libawt (which already
contains debug_trace.o) anyway, so this is no problem.

Thank you and best regards,
Volker


Re: [OpenJDK 2D-Dev] RFR(XS): 8213944: Fix AIX build after the removal of Xrandr.h and add a configure check for it

2018-11-16 Thread Volker Simonis
On Thu, Nov 15, 2018 at 6:01 PM Philip Race  wrote:
>
> PS I am not sure why xrandr headers would not be available for AIX.
> They are a standard part of the xdistribution.
>

I'm not an X11 guru, but as far as I understand, xrandr is an
extension and as such it doesn't have to be supported by every
implementation. To the best of my knowledge (I've just started another
poll among some experts) AIX doesn't support Xrandr and does not have
the corresponding headers.

> If true, think what you are going to have to do is add a
> --with-xrandr-include option
> and provide it that way.
>

What if there are no standard Xrandr headers on a platform? Do you
really want to force users to get them from some dubious sources just
for building the OpenJDK? Sorry, but I don't think that's a good
solution. Than I'd rather prefer the ugly ifdefs (or I check the
headers back in again in an AIX-specific directory :)

Thank you and best regards,
Volker

> -phil.
>
> On 11/15/18, 8:55 AM, Philip Race wrote:
> > Hmm. I don't like the ifdefs.
> >
> > Xrandr is a requirement for the build. If its not there at runtime
> > that's OK.
> >
> > -phil.
> >
> > On 11/15/18, 8:06 AM, Volker Simonis wrote:
> >> Hi,
> >>
> >> can I please have a review for the following small change:
> >>
> >> http://cr.openjdk.java.net/~simonis/webrevs/2018/8213944/
> >> https://bugs.openjdk.java.net/browse/JDK-8213944
> >>
> >> Change JDK-8210863 removed the Xrandr.h/randr.h headers from the
> >> OpenJDK sources but forgot to add a configure check for the Xrandr
> >> extension which is now a build dependency.
> >>
> >> The change also broke the AIX build. AIX never supported Xrandr, but
> >> that was only detected at runtime, when the JDK was unable to
> >> dynamically load libXrand.so. Now, without Xrandr.h/randr.h in the
> >> source tree any more, we have to conditionally compile some parts of
> >> src/java.desktop/unix/native/libawt_xawt/awt/awt_GraphicsEnv.c such
> >> that it doesn't require the definitions and declarations from
> >> Xrandr.h/randr.h any more.
> >>
> >> Thank you and best regards,
> >> Volker


Re: [OpenJDK 2D-Dev] RFR(XS): 8213944: Fix AIX build after the removal of Xrandr.h and add a configure check for it

2018-11-16 Thread Volker Simonis
On Thu, Nov 15, 2018 at 5:55 PM Philip Race  wrote:
>
> Hmm. I don't like the ifdefs.
>

Mee too, but what else can we do, if there are no Xrandr headers
available on a platform?

Please see my answer to your other mail...

> Xrandr is a requirement for the build. If its not there at runtime
> that's OK.
>
> -phil.
>
> On 11/15/18, 8:06 AM, Volker Simonis wrote:
> > Hi,
> >
> > can I please have a review for the following small change:
> >
> > http://cr.openjdk.java.net/~simonis/webrevs/2018/8213944/
> > https://bugs.openjdk.java.net/browse/JDK-8213944
> >
> > Change JDK-8210863 removed the Xrandr.h/randr.h headers from the
> > OpenJDK sources but forgot to add a configure check for the Xrandr
> > extension which is now a build dependency.
> >
> > The change also broke the AIX build. AIX never supported Xrandr, but
> > that was only detected at runtime, when the JDK was unable to
> > dynamically load libXrand.so. Now, without Xrandr.h/randr.h in the
> > source tree any more, we have to conditionally compile some parts of
> > src/java.desktop/unix/native/libawt_xawt/awt/awt_GraphicsEnv.c such
> > that it doesn't require the definitions and declarations from
> > Xrandr.h/randr.h any more.
> >
> > Thank you and best regards,
> > Volker


Re: RFR(XS): 8213944: Fix AIX build after the removal of Xrandr.h and add a configure check for it

2018-11-16 Thread Volker Simonis
On Thu, Nov 15, 2018 at 5:31 PM Aleksey Shipilev  wrote:
>
> On 11/15/2018 05:06 PM, Volker Simonis wrote:
> > can I please have a review for the following small change:
> >
> > http://cr.openjdk.java.net/~simonis/webrevs/2018/8213944/
>
> *) I tested it on platform without libxrandr-dev installed, and configure 
> reported the failure
> appopriately.
>

Thanks for testing!

> *) Indent looks off at L2207 here:
>
> 2205 static char *get_output_screen_name(JNIEnv *env, int screen) {
> 2206 #ifdef _AIX
> 2207 return NULL;
> 2208 #else
> 2209 if (!awt_XRRGetScreenResources || !awt_XRRGetOutputInfo) {
>

You're right. Fixed it.

Thank you and best regards,
Volker

>
> Thanks,
> -Aleksey
>


Re: RFR(S): 8213698: Improve devkit creation and add support for linux/ppc64/ppc64le/s390x

2018-11-15 Thread Volker Simonis
Hi Erik,

thanks for looking at my changes (and providing some history, which is
always interesting) Can I consider your answer a review from your
site? I'd like to wait for Magnus to double check my patch but it's
definitely good to know if you're fine with my changes :)

Thank you and best regards,
Volker

On Tue, Nov 13, 2018 at 6:34 PM Erik Joelsson  wrote:
>
> Hello Volker,
>
> I think your changes look reasonable and I'm happy to see some external
> interest in the devkits!
>
> To provide some history on the structure of these makefiles. They used
> to support building full cross toolchains from and to all listed
> platforms (creating one mega kit for each host platform with all the
> others as targets), which was also the default behavior. Since we
> weren't using this, I simplified it a bit. I think your design with
> base-to-target directories makes more sense. You get separate kits for
> each base and target combination, which is how we use them in practice
> anyway.
>
> The presence of a top level dir in the tars is something I have debated
> with myself a lot. The different devkit scripts aren't even behaving the
> same in this regard. I agree that it's cleaner with a top level dir, but
> in the world of automated installs, it adds yet another level of
> directories to already deep paths. I'm ok with changing it however.
>
> /Erik
>
> On 2018-11-13 08:25, Volker Simonis wrote:
> > As it seems that nobody has looked at this until now, I take the
> > liberty to slightly update my proposal :)
> >
> > http://cr.openjdk.java.net/~simonis/webrevs/2018/8213698.v1/
> >
> > - add the Fedora version to the sub-directory under
> > build/devkit/download/rpms/ which contains the target specific .rpm
> > files. This saves a lot of time if experimenting with various sysroot
> > versions:
> >
> > endif
> > -  LINUX_VERSION := Fedora $(BASE_OS_VERSION)
> > +  LINUX_VERSION := Fedora_$(BASE_OS_VERSION)
> >   else
> >   DOWNLOAD := $(OUTPUT_ROOT)/download
> > -DOWNLOAD_RPMS := $(DOWNLOAD)/rpms/$(TARGET)
> > +DOWNLOAD_RPMS := $(DOWNLOAD)/rpms/$(TARGET)-$(LINUX_VERSION)
> >   SRCDIR := $(OUTPUT_ROOT)/src
> >
> > - remove "unknown" from the host platform triplet. According to the
> > "Autobook, §3.4 Configuration Names" [1] "..it is normally not
> > necessary to specify an entire name. In particular, the middle field
> > (manufacturer - which defaults to "unknown") is often omitted."
> >
> >   # Figure out what platform this is building on.
> > -me := $(cpu)-$(if $(findstring Linux,$(os)),unknown-linux-gnu)
> > +me := $(cpu)-$(if $(findstring Linux,$(os)),linux-gnu)
> >
> > Thank you and best regards,
> > Volker
> >
> > [1] https://sourceware.org/autobook/autobook/autobook_17.html
> > On Mon, Nov 12, 2018 at 12:19 PM Volker Simonis
> >  wrote:
> >> Hi,
> >>
> >> can I please have a review for the following change which ads support
> >> for linux/ppc64/ppc64le/s390x devkits and hopefully improves the
> >> creation of devkits a little bit :)
> >>
> >> http://cr.openjdk.java.net/~simonis/webrevs/2018/8213698/
> >> https://bugs.openjdk.java.net/browse/JDK-8213698
> >>
> >> With these changes it becomes possible to say any of the following:
> >>
> >> make cross_compile_target="ppc64le-linux-gnu s390x-linux-gnu" 
> >> BASE_OS=Fedora
> >> make cross_compile_target="ppc64-linux-gnu" BASE_OS=Fedora 
> >> BASE_OS_VERSION=17
> >> make onlytars cross_compile_target="ppc64-linux-gnu ppc64le-linux-gnu
> >> s390x-linux-gnu"
> >>
> >> and get the following devkits under "build/devkit/result":
> >>
> >> sdk-x86_64-unknown-linux-gnu-to-ppc64le-linux-gnu-20181112.tar.gz
> >> sdk-x86_64-unknown-linux-gnu-to-ppc64-linux-gnu-20181112.tar.gz
> >> sdk-x86_64-unknown-linux-gnu-to-s390x-linux-gnu-20181112.tar.gz
> >> x86_64-unknown-linux-gnu-to-ppc64le-linux-gnu/
> >> x86_64-unknown-linux-gnu-to-ppc64-linux-gnu/
> >> x86_64-unknown-linux-gnu-to-s390x-linux-gnu/
> >>
> >> Below you can find a more detailed description of the various changes.
> >> Once we've discussed and agreed on the changes I'd like to add a small
> >> documentation about how to build and use devkits to
> >> "doc/building.{md,html}" which describes the creation and usage of
> >> devkits. This documentation should be right at the top of the
> >> "Cross-compiling" section which is quite comp

Re: RFR(S): 8213698: Improve devkit creation and add support for linux/ppc64/ppc64le/s390x

2018-11-13 Thread Volker Simonis
As it seems that nobody has looked at this until now, I take the
liberty to slightly update my proposal :)

http://cr.openjdk.java.net/~simonis/webrevs/2018/8213698.v1/

- add the Fedora version to the sub-directory under
build/devkit/download/rpms/ which contains the target specific .rpm
files. This saves a lot of time if experimenting with various sysroot
versions:

   endif
-  LINUX_VERSION := Fedora $(BASE_OS_VERSION)
+  LINUX_VERSION := Fedora_$(BASE_OS_VERSION)
 else
 DOWNLOAD := $(OUTPUT_ROOT)/download
-DOWNLOAD_RPMS := $(DOWNLOAD)/rpms/$(TARGET)
+DOWNLOAD_RPMS := $(DOWNLOAD)/rpms/$(TARGET)-$(LINUX_VERSION)
 SRCDIR := $(OUTPUT_ROOT)/src

- remove "unknown" from the host platform triplet. According to the
"Autobook, §3.4 Configuration Names" [1] "..it is normally not
necessary to specify an entire name. In particular, the middle field
(manufacturer - which defaults to "unknown") is often omitted."

 # Figure out what platform this is building on.
-me := $(cpu)-$(if $(findstring Linux,$(os)),unknown-linux-gnu)
+me := $(cpu)-$(if $(findstring Linux,$(os)),linux-gnu)

Thank you and best regards,
Volker

[1] https://sourceware.org/autobook/autobook/autobook_17.html
On Mon, Nov 12, 2018 at 12:19 PM Volker Simonis
 wrote:
>
> Hi,
>
> can I please have a review for the following change which ads support
> for linux/ppc64/ppc64le/s390x devkits and hopefully improves the
> creation of devkits a little bit :)
>
> http://cr.openjdk.java.net/~simonis/webrevs/2018/8213698/
> https://bugs.openjdk.java.net/browse/JDK-8213698
>
> With these changes it becomes possible to say any of the following:
>
> make cross_compile_target="ppc64le-linux-gnu s390x-linux-gnu" BASE_OS=Fedora
> make cross_compile_target="ppc64-linux-gnu" BASE_OS=Fedora BASE_OS_VERSION=17
> make onlytars cross_compile_target="ppc64-linux-gnu ppc64le-linux-gnu
> s390x-linux-gnu"
>
> and get the following devkits under "build/devkit/result":
>
> sdk-x86_64-unknown-linux-gnu-to-ppc64le-linux-gnu-20181112.tar.gz
> sdk-x86_64-unknown-linux-gnu-to-ppc64-linux-gnu-20181112.tar.gz
> sdk-x86_64-unknown-linux-gnu-to-s390x-linux-gnu-20181112.tar.gz
> x86_64-unknown-linux-gnu-to-ppc64le-linux-gnu/
> x86_64-unknown-linux-gnu-to-ppc64-linux-gnu/
> x86_64-unknown-linux-gnu-to-s390x-linux-gnu/
>
> Below you can find a more detailed description of the various changes.
> Once we've discussed and agreed on the changes I'd like to add a small
> documentation about how to build and use devkits to
> "doc/building.{md,html}" which describes the creation and usage of
> devkits. This documentation should be right at the top of the
> "Cross-compiling" section which is quite complex now. It was not clear
> to me until yet how trivial the creation and usage of a devkit can be
> :)
>
>
> - The changes required for supporting linux/ppc64/ppc64le/s390x are trivial:
>
> make/devkit/Tools.gmk
>
> +ifneq ($(filter ppc64 ppc64le s390x, $(ARCH)), )
> +  # We only support 64-bit on these platforms anyway
> +  CONFIG += --disable-multilib
> +endif
>
> This is required to prevent building of multilib toolchains which
> arent't needed anyway. The problem with the multilib toolchain build
> is that it requires some special 32-bit headers which arn't installed
> by default from the current RPM list.
>
> - The following change allows users to choose the version of Fedora
> which is used to create the sysroot environment by setting
> "BASE_OS_VERSION" (with "27" being the default). This works "BASE_OS"
> will be set to "Fedora" (as opposed to "Fedora27" before). Notice that
> older Fedora versions have a sligthly different download URL:
>
> make/devkit/Tools.gmk
>
>  ifeq ($(BASE_OS), OEL6)
>OEL_URL := http://yum.oracle.com/repo/OracleLinux/OL6/4/base/$(ARCH)/
>LINUX_VERSION := OEL6.4
> -else ifeq ($(BASE_OS), Fedora27)
> -  ifeq ($(ARCH), aarch64)
> +else ifeq ($(BASE_OS), Fedora)
> +  DEFAULT_OS_VERSION := 27
> +  ifeq ($(BASE_OS_VERSION), )
> +BASE_OS_VERSION := $(DEFAULT_OS_VERSION)
> +  endif
> +  ifeq ($(filter x86_64 armhfp, $(ARCH)), )
>  FEDORA_TYPE=fedora-secondary
>else
>  FEDORA_TYPE=fedora/linux
>endif
> -  OEL_URL := 
> https://dl.fedoraproject.org/pub/$(FEDORA_TYPE)/releases/27/Everything/$(ARCH)/os/Packages/
> -  LINUX_VERSION := Fedora 27
> +  ARCHIVED := $(shell [ $(BASE_OS_VERSION) -lt $(DEFAULT_OS_VERSION)
> ] && echo true)
> +  ifeq ($(ARCHIVED),true)
> +OEL_URL :=
> https://archives.fedoraproject.org/pub/archive/$(FEDORA_TYPE)/releases/$(BASE_OS_VERSION)/Everything/$(ARCH)/os/Packages/
> +  else
> +OEL_URL :=
> https://dl.fedoraproj

RFR(S): 8213698: Improve devkit creation and add support for linux/ppc64/ppc64le/s390x

2018-11-12 Thread Volker Simonis
Hi,

can I please have a review for the following change which ads support
for linux/ppc64/ppc64le/s390x devkits and hopefully improves the
creation of devkits a little bit :)

http://cr.openjdk.java.net/~simonis/webrevs/2018/8213698/
https://bugs.openjdk.java.net/browse/JDK-8213698

With these changes it becomes possible to say any of the following:

make cross_compile_target="ppc64le-linux-gnu s390x-linux-gnu" BASE_OS=Fedora
make cross_compile_target="ppc64-linux-gnu" BASE_OS=Fedora BASE_OS_VERSION=17
make onlytars cross_compile_target="ppc64-linux-gnu ppc64le-linux-gnu
s390x-linux-gnu"

and get the following devkits under "build/devkit/result":

sdk-x86_64-unknown-linux-gnu-to-ppc64le-linux-gnu-20181112.tar.gz
sdk-x86_64-unknown-linux-gnu-to-ppc64-linux-gnu-20181112.tar.gz
sdk-x86_64-unknown-linux-gnu-to-s390x-linux-gnu-20181112.tar.gz
x86_64-unknown-linux-gnu-to-ppc64le-linux-gnu/
x86_64-unknown-linux-gnu-to-ppc64-linux-gnu/
x86_64-unknown-linux-gnu-to-s390x-linux-gnu/

Below you can find a more detailed description of the various changes.
Once we've discussed and agreed on the changes I'd like to add a small
documentation about how to build and use devkits to
"doc/building.{md,html}" which describes the creation and usage of
devkits. This documentation should be right at the top of the
"Cross-compiling" section which is quite complex now. It was not clear
to me until yet how trivial the creation and usage of a devkit can be
:)


- The changes required for supporting linux/ppc64/ppc64le/s390x are trivial:

make/devkit/Tools.gmk

+ifneq ($(filter ppc64 ppc64le s390x, $(ARCH)), )
+  # We only support 64-bit on these platforms anyway
+  CONFIG += --disable-multilib
+endif

This is required to prevent building of multilib toolchains which
arent't needed anyway. The problem with the multilib toolchain build
is that it requires some special 32-bit headers which arn't installed
by default from the current RPM list.

- The following change allows users to choose the version of Fedora
which is used to create the sysroot environment by setting
"BASE_OS_VERSION" (with "27" being the default). This works "BASE_OS"
will be set to "Fedora" (as opposed to "Fedora27" before). Notice that
older Fedora versions have a sligthly different download URL:

make/devkit/Tools.gmk

 ifeq ($(BASE_OS), OEL6)
   OEL_URL := http://yum.oracle.com/repo/OracleLinux/OL6/4/base/$(ARCH)/
   LINUX_VERSION := OEL6.4
-else ifeq ($(BASE_OS), Fedora27)
-  ifeq ($(ARCH), aarch64)
+else ifeq ($(BASE_OS), Fedora)
+  DEFAULT_OS_VERSION := 27
+  ifeq ($(BASE_OS_VERSION), )
+BASE_OS_VERSION := $(DEFAULT_OS_VERSION)
+  endif
+  ifeq ($(filter x86_64 armhfp, $(ARCH)), )
 FEDORA_TYPE=fedora-secondary
   else
 FEDORA_TYPE=fedora/linux
   endif
-  OEL_URL := 
https://dl.fedoraproject.org/pub/$(FEDORA_TYPE)/releases/27/Everything/$(ARCH)/os/Packages/
-  LINUX_VERSION := Fedora 27
+  ARCHIVED := $(shell [ $(BASE_OS_VERSION) -lt $(DEFAULT_OS_VERSION)
] && echo true)
+  ifeq ($(ARCHIVED),true)
+OEL_URL :=
https://archives.fedoraproject.org/pub/archive/$(FEDORA_TYPE)/releases/$(BASE_OS_VERSION)/Everything/$(ARCH)/os/Packages/
+  else
+OEL_URL :=
https://dl.fedoraproject.org/pub/$(FEDORA_TYPE)/releases/$(BASE_OS_VERSION)/Everything/$(ARCH)/os/Packages/
+  endif
+  LINUX_VERSION := Fedora $(BASE_OS_VERSION)
 else
   $(error Unknown base OS $(BASE_OS))
 endif

- Enable the creation of several different devkits at once (e.g. 'make
 cross_compile_target="ppc64-linux-gnu ppc64le-linux-gnu
s390x-linux-gnu"') or one after another but all into the same
'build/devkit/result' directory. The result directory will contain
$HOST-to-$TARGET sub-directories with the corresponding devkits:

make/devkit/Makefile

-submakevars = HOST=$@ BUILD=$(me) \
-RESULT=$(RESULT) PREFIX=$(RESULT)/$@ \
-OUTPUT_ROOT=$(OUTPUT_ROOT)
+submakevars = HOST=$@ BUILD=$(me) RESULT=$(RESULT) OUTPUT_ROOT=$(OUTPUT_ROOT)
+
 $(host_platforms) :
@echo 'Building compilers for $@'
@echo 'Targets: $(target_platforms)'
for p in $(filter $@, $(target_platforms)) $(filter-out $@,
$(target_platforms)); do \
- $(MAKE) -f Tools.gmk download-rpms $(submakevars) TARGET=$$p && \
+ $(MAKE) -f Tools.gmk download-rpms $(submakevars) \
+  TARGET=$$p PREFIX=$(RESULT)/$@-to-$$p && \
  $(MAKE) -f Tools.gmk all $(submakevars) \
- TARGET=$$p || exit 1 ; \
+  TARGET=$$p PREFIX=$(RESULT)/$@-to-$$p && \
+ $(MAKE) -f Tools.gmk ccache $(submakevars) \
+  TARGET=$@ PREFIX=$(RESULT)/$@-to-$$p
BUILDDIR=$(OUTPUT_ROOT)/$@/$$p || exit 1 ; \
done
-   @echo 'Building ccache program for $@'
-   $(MAKE) -f Tools.gmk ccache $(submakevars) TARGET=$@
@echo 'All done"'

Notice that we have to build "ccache" for each target because ccache
will be installed into the directory specified by "--prefix" at
configure time and this is now different for every target. However
that's not a big 

Re: RFR(XS): 8213515: Improve freetype detection on linux/ppc64/ppc64le/s390x

2018-11-08 Thread Volker Simonis
Thanks for the quick reviews!

I've removed the comment as requested by Aleksey and pushed.

Regards,
Volker
On Thu, Nov 8, 2018 at 11:54 AM Thomas Stüfe  wrote:
>
> +1
>
> Gruss Thomas
> On Thu, Nov 8, 2018 at 10:25 AM Volker Simonis  
> wrote:
> >
> > Hi,
> >
> > can I please have a review for the following trivial enhancement of
> > the freetype detection on linux/ppc64/ppc64le/s390x:
> >
> > http://cr.openjdk.java.net/~simonis/webrevs/2018/8213515/
> > https://bugs.openjdk.java.net/browse/JDK-8213515
> >
> > On some more "exotic" Linux platforms, libfreetype.so is found under
> > /usr/lib64/libfreetype.so even if the platform only supports 64-bits.
> > However, we currently only check for libfreetype.so under "/usr/lib"
> > and "/usr/lib/-linux-gnu". Another check under "/usr/lib64" does
> > no harm and greatly improves the usability on the mentioned platforms.
> >
> > Thank you and best regards,
> > Volker


RFR(XS): 8213515: Improve freetype detection on linux/ppc64/ppc64le/s390x

2018-11-08 Thread Volker Simonis
Hi,

can I please have a review for the following trivial enhancement of
the freetype detection on linux/ppc64/ppc64le/s390x:

http://cr.openjdk.java.net/~simonis/webrevs/2018/8213515/
https://bugs.openjdk.java.net/browse/JDK-8213515

On some more "exotic" Linux platforms, libfreetype.so is found under
/usr/lib64/libfreetype.so even if the platform only supports 64-bits.
However, we currently only check for libfreetype.so under "/usr/lib"
and "/usr/lib/-linux-gnu". Another check under "/usr/lib64" does
no harm and greatly improves the usability on the mentioned platforms.

Thank you and best regards,
Volker


Re: gcc 7.3.1 build - warnings as errors in harfbuzz

2018-10-10 Thread Volker Simonis
Hi Matthias,

I've looked a little at this error and it is quite strange. First of
all, I can't reproduce it with a default gcc 7.3.0 and there doesn't
exist an official version of gcc 7.3.1 (at least I couldn't find it).
Also, I can't see the real error in the objected code. The values the
warning prints ("assuming directive output of 2147488582 bytes" and
"output 2 or more bytes (assuming 2147488583) into a destination of
size 127") seem to be excessively large. Every other reference for
this warning I could google reports much small numbers and I don't see
how a double could potentially use 2147488583 characters in the output
buffer.

Could this be a problem with the compiler you've used? Could you also
ask the people who compiled and patched it?

I did found the GCC bug "[7 Regression] -Wformat-overflow false
positive exceeding INT_MAX in glibc sysdeps/posix/tempname.c" [1]
which seem to resemble to the output you see, but that should have
been fixed  already in the 7.0 release.

Regards,
Volker

[1] https://gcc.gnu.org/bugzilla/show_bug.cgi?id=79275

On Wed, Oct 10, 2018 at 3:14 PM Baesken, Matthias
 wrote:
>
> Hello  , when  compiling   jdk/jdk   with  gcc 7.3.1on linux x86_64 (or 
> also on   linux ppc64)   I run into this build error :
>
>
> /open_jdk/jdk_just_clone/jdk/src/java.desktop/share/native/libfontmanager/harfbuzz/hb-common.cc:
>  In function 'void hb_variation_to_string(hb_variation_t*, char*, unsigned 
> int)':
> /open_jdk/jdk_just_clone/jdk/src/java.desktop/share/native/libfontmanager/harfbuzz/hb-common.cc:1066:27:
>  error: '%g' directive output between 1 and 18446744073709551615 bytes may 
> cause result to exceed 'INT_MAX' [-Werror=format-truncation=]
>len += MAX (0, snprintf (s + len, ARRAY_LENGTH (s) - len, "%g", 
> variation->value));
>   
> ~^
> /open_jdk/jdk_just_clone/jdk/src/java.desktop/share/native/libfontmanager/harfbuzz/hb-common.cc:1066:27:
>  note: assuming directive output of 2147488582 bytes
> cc1plus: all warnings being treated as errors
>
> (build is a product - build)
>
> Setting –disable-warnings-as-errors  works  as a workaround  ,  but of course 
> this is not really   what we want to do .
>
> Fixing  in theharfbuzz   sources  in OpenJDk  might be also not so nice 
> because it would clash with imports of new versions of harfbuzz .
> Do you think we could disable  the specific warning for the library 
> compilation ?
>
> Any other great suggestions ?
>
>
> Thanks, Matthias


Re: RFR(XS): 8211837: Creation of the default CDS Archive should depend on ENABLE_CDS

2018-10-09 Thread Volker Simonis
Hi Jiangli,

thanks for looking at the change. Unfortunately I've already pushed it
in order to fix our nightly builds.

I think the comment should only mention the variants which are
explicitly excluded by that function. All other cases are now handled
by the "ENABLE_CDS" flag which seems more natural to me. But
"ENABLE_CDS" is defined in hotspot.m4 where I put in some comments.

Regards,
Volker

On Mon, Oct 8, 2018 at 8:06 PM Jiangli Zhou  wrote:
>
> Hi Volker,
>
> Looks good. Thanks for fixing! It would be good to update the following
> comment in jdk-options.m4 to add AIX, minimal and core.
>
>   609
> 
>   610 #
>   611 # Disable the default CDS archive generation
>   612 #   cross compilation - disabled
>   613 #   zero  - off by default (not a tested configuration)
>   614 #
>
>
> Thanks!
>
> Jiangli
>
>
> On 10/8/18 3:10 AM, Volker Simonis wrote:
> > Hi Martin, Goetz,
> >
> > thanks for reviewing my patch! As Aleksey posted a similar patch for
> > fixing the Zero build I've extended my patch to handle
> > zero/minimal/core as well.
> >
> > In fact the patch now disables CDS on AIX, minimal, core and zero
> > unless the user explicitly requests it with the help of
> > `--with-jvm-features=cds`. The configuration variant with
> > `--with-jvm-features=-cds` should now also be handled correctly (I
> > hope caught all possible combinations :)
> >
> > If CDS is disabled, the creation of the default class list and default
> > archive will now be disabled as well.
> >
> > @Aleksey Shipilev : can you please check if this new version of my
> > patch also works for you?
> >
> > http://cr.openjdk.java.net/~simonis/webrevs/2018/8211837.v1/
> >
> > Thank you and best regards,
> > Volker
> > On Mon, Oct 8, 2018 at 11:10 AM Doerr, Martin  wrote:
> >> Hi Volker,
> >>
> >> looks good. Thanks for fixing.
> >> Of course, it would be great if this could be used to fix minimal/zero 
> >> build, too.
> >>
> >> Best regards,
> >> Martin
> >>
> >>
> >> -Original Message-
> >> From: hotspot-runtime-dev  
> >> On Behalf Of Volker Simonis
> >> Sent: Montag, 8. Oktober 2018 10:19
> >> To: build-dev ; 
> >> hotspot-runtime-...@openjdk.java.net runtime 
> >> 
> >> Subject: RFR(XS): 8211837: Creation of the default CDS Archive should 
> >> depend on ENABLE_CDS
> >>
> >> Hi,
> >>
> >> can I please have a review for the following trivial build fix:
> >>
> >> http://cr.openjdk.java.net/~simonis/webrevs/2018/8211837/
> >> https://bugs.openjdk.java.net/browse/JDK-8211837
> >>
> >> It makes no sense to try to create a default CDS archive on VMs which
> >> don't support CDS at all. We already have '--enable_cds' which
> >> defaults to 'true' on all platforms except AIX.
> >>
> >> The check for '--enable_cds_archive' should use the result of
> >> '--enable_cds' (which is saved in "ENABLE_CDS") and only enable
> >> default archive creation if CDS is enabled.
> >>
> >> Failure to do so currently breaks the AIX build (after JDK-)8202951 was 
> >> pushed).
> >>
> >> Thank you and best regards,
> >> Volker
>


Re: RFR(XS): 8211837: Creation of the default CDS Archive should depend on ENABLE_CDS

2018-10-09 Thread Volker Simonis
Hi Erik,

thanks for reviewing and sorry but I've already pushed this yesterday
evening before leaving because I wanted to fix our nightly builds.

I agree that the long line in jdk-options.m4 is ugly and I put it on
my TODO list. Taking into account the current cadence of AIX build
fixes I think it wont take long until I'll have to revisit that file
anyway :)

Regards,
Volker
On Mon, Oct 8, 2018 at 6:30 PM Erik Joelsson  wrote:
>
> Hello,
>
> Looks good. My only minor nit is the rather long line in jdk-options.m4
> that I would appreciate if it was broken up (no need for respin if you do).
>
> /Erik
>
>
> On 2018-10-08 03:10, Volker Simonis wrote:
> > Hi Martin, Goetz,
> >
> > thanks for reviewing my patch! As Aleksey posted a similar patch for
> > fixing the Zero build I've extended my patch to handle
> > zero/minimal/core as well.
> >
> > In fact the patch now disables CDS on AIX, minimal, core and zero
> > unless the user explicitly requests it with the help of
> > `--with-jvm-features=cds`. The configuration variant with
> > `--with-jvm-features=-cds` should now also be handled correctly (I
> > hope caught all possible combinations :)
> >
> > If CDS is disabled, the creation of the default class list and default
> > archive will now be disabled as well.
> >
> > @Aleksey Shipilev : can you please check if this new version of my
> > patch also works for you?
> >
> > http://cr.openjdk.java.net/~simonis/webrevs/2018/8211837.v1/
> >
> > Thank you and best regards,
> > Volker
> > On Mon, Oct 8, 2018 at 11:10 AM Doerr, Martin  wrote:
> >> Hi Volker,
> >>
> >> looks good. Thanks for fixing.
> >> Of course, it would be great if this could be used to fix minimal/zero 
> >> build, too.
> >>
> >> Best regards,
> >> Martin
> >>
> >>
> >> -Original Message-
> >> From: hotspot-runtime-dev  
> >> On Behalf Of Volker Simonis
> >> Sent: Montag, 8. Oktober 2018 10:19
> >> To: build-dev ; 
> >> hotspot-runtime-...@openjdk.java.net runtime 
> >> 
> >> Subject: RFR(XS): 8211837: Creation of the default CDS Archive should 
> >> depend on ENABLE_CDS
> >>
> >> Hi,
> >>
> >> can I please have a review for the following trivial build fix:
> >>
> >> http://cr.openjdk.java.net/~simonis/webrevs/2018/8211837/
> >> https://bugs.openjdk.java.net/browse/JDK-8211837
> >>
> >> It makes no sense to try to create a default CDS archive on VMs which
> >> don't support CDS at all. We already have '--enable_cds' which
> >> defaults to 'true' on all platforms except AIX.
> >>
> >> The check for '--enable_cds_archive' should use the result of
> >> '--enable_cds' (which is saved in "ENABLE_CDS") and only enable
> >> default archive creation if CDS is enabled.
> >>
> >> Failure to do so currently breaks the AIX build (after JDK-)8202951 was 
> >> pushed).
> >>
> >> Thank you and best regards,
> >> Volker
>


Re: RFC: JEP JDK-8208089: Implement C++14 Language Features

2018-10-08 Thread Volker Simonis
On Mon, Oct 8, 2018 at 12:49 PM Thomas Stüfe  wrote:
>
> Hi Kim,
>
> is this JEP only about C++14 features or shall we discuss older
> features too? The reason I am asking is that I would like us to
> officially endorse namespaces. Not inline namespaces, just plain old
> namespaces.
>
> HotSpot makes very limited use of namespaces.
>
> Not really true, we already use them. E.g. in metaspace coding, I used
> them to keep the global name space clean and to keep internals
> internal. This was met with positive reviews, and it works on all
> toolchains, so compiler support should not be a problem. Using
> namespaces, we could get slowly replace the "AllStatic" classes, which
> are namespaces in all but name. In contrast to classes, namespaces can
> be spread over multiple files and compilation units, and allow for
> cleaner separation of internal and external coding.
>
> It also would allow us to get rid the middle-of-header-platform-inclusions:
>

That's overdue since a long time!
It would allow us to finally get correct code navigation in IDEs.

> For example, today we have:
>
> [os.hpp]
> class os: AllStatic {
> 
> (platform independent, outward facing os:: functions)
> #include "os_linux.hpp"
> >> (Inner class "Linux" with platform specific os functions)
> ...
> }
>
> Not only is the inclusion in the middle of a class terrifying, it also
> means the shared, outward facing os:: namespace contains class Linux
> and lots of platform specific internals.
>
> With namespaces one could:
>
> [os.hpp]
> namespace os {
> 
> (platform independent, outward facing os:: functions)
> 
> }
>
> [os_linux.hpp]
> namespace os {
> namespace Linux {
> (linux specific os functions)
> }
> }
>
> I think this is way cleaner, and keeps platform specifics from
> including files which only care for the shared os interface.
>
> --
>
> Note that I would prefer forbidding the "using" directive for callers
> of namespace functions, but rather force them to spell out the
> namespace:
>

As far as I saw, "using" directives are already on the "Exclude" list
in Kim's proposal.

> So, instead of this:
>
> using os;
> jlong m = available_memory();
>
> I would prefer this, which is our current practice with AllStatic childs:
>
> jlong m = os::available_memory();
>
> The latter form would keep the code grepable.
>
> Best Regards, Thomas
> On Wed, Oct 3, 2018 at 9:13 PM Kim Barrett  wrote:
> >
> > I've submitted a JEP for
> >
> > (1) enabling the use of C++14 Language Features when building the JDK,
> >
> > (2) define a process for deciding and documenting which new features
> > can be used or are forbidden in HotSpot code,
> >
> > (3) provide an initial list of permitted and forbidden new features.
> >
> > https://bugs.openjdk.java.net/browse/JDK-8208089
> >


Re: RFR(XS): 8211837: Creation of the default CDS Archive should depend on ENABLE_CDS

2018-10-08 Thread Volker Simonis
On Mon, Oct 8, 2018 at 12:59 PM Aleksey Shipilev  wrote:
>
> On 10/08/2018 12:10 PM, Volker Simonis wrote:
> > @Aleksey Shipilev : can you please check if this new version of my
> > patch also works for you?
> >
> > http://cr.openjdk.java.net/~simonis/webrevs/2018/8211837.v1/
>
> Checked Linux x86_64 {server, minimal, zero}, all build fine with this patch!
>

Thanks for checking !

> Closed my JDK-8211838 as duplicate of this bug.
>
> This comment is now obsolete:
>
>  613 #   zero  - off by default (not a tested configuration)
>

I've removed that line from my local change.

I'd appreciate if I could get one more review from a build-group
member before pushing.

Regards,
Volker

> Thanks,
> -Aleksey
>
>


Re: RFR 8211838 (XS): Minimal VM build is broken after JDK-8202951 (Implementation of JEP 341: Default CDS Archives)

2018-10-08 Thread Volker Simonis
On Mon, Oct 8, 2018 at 10:38 AM Aleksey Shipilev  wrote:
>
> On 10/08/2018 10:34 AM, Volker Simonis wrote:
> > Instead of checking the JVM_VARIANT, I check for ENABLE_CDS. But as
> > far as I see, that still doesn't help for the 'minmal' variant. I
> > think the right fix would be to set ENABLE_CDS to false for the
> > minimal build. I could add that to my fix if you're all right with it?
>
> Sure, I am okay with that, as long as it unbreaks the build. The other thing 
> is
> --with-jvm-features=-cds. I can test Minimal/Zero once you post the patch.
>

Cool, I've just posted a new webrev on the other thread. Can you
please verify if it fixes your problems as well and reply on that
thread?

http://mail.openjdk.java.net/pipermail/build-dev/2018-October/023564.html

Thanks,
Volker

> -Aleksey
>


Re: RFR(XS): 8211837: Creation of the default CDS Archive should depend on ENABLE_CDS

2018-10-08 Thread Volker Simonis
Hi Martin, Goetz,

thanks for reviewing my patch! As Aleksey posted a similar patch for
fixing the Zero build I've extended my patch to handle
zero/minimal/core as well.

In fact the patch now disables CDS on AIX, minimal, core and zero
unless the user explicitly requests it with the help of
`--with-jvm-features=cds`. The configuration variant with
`--with-jvm-features=-cds` should now also be handled correctly (I
hope caught all possible combinations :)

If CDS is disabled, the creation of the default class list and default
archive will now be disabled as well.

@Aleksey Shipilev : can you please check if this new version of my
patch also works for you?

http://cr.openjdk.java.net/~simonis/webrevs/2018/8211837.v1/

Thank you and best regards,
Volker
On Mon, Oct 8, 2018 at 11:10 AM Doerr, Martin  wrote:
>
> Hi Volker,
>
> looks good. Thanks for fixing.
> Of course, it would be great if this could be used to fix minimal/zero build, 
> too.
>
> Best regards,
> Martin
>
>
> -Original Message-
> From: hotspot-runtime-dev  On 
> Behalf Of Volker Simonis
> Sent: Montag, 8. Oktober 2018 10:19
> To: build-dev ; 
> hotspot-runtime-...@openjdk.java.net runtime 
> 
> Subject: RFR(XS): 8211837: Creation of the default CDS Archive should depend 
> on ENABLE_CDS
>
> Hi,
>
> can I please have a review for the following trivial build fix:
>
> http://cr.openjdk.java.net/~simonis/webrevs/2018/8211837/
> https://bugs.openjdk.java.net/browse/JDK-8211837
>
> It makes no sense to try to create a default CDS archive on VMs which
> don't support CDS at all. We already have '--enable_cds' which
> defaults to 'true' on all platforms except AIX.
>
> The check for '--enable_cds_archive' should use the result of
> '--enable_cds' (which is saved in "ENABLE_CDS") and only enable
> default archive creation if CDS is enabled.
>
> Failure to do so currently breaks the AIX build (after JDK-)8202951 was 
> pushed).
>
> Thank you and best regards,
> Volker


Re: RFR 8211838 (XS): Minimal VM build is broken after JDK-8202951 (Implementation of JEP 341: Default CDS Archives)

2018-10-08 Thread Volker Simonis
Hi Aleksey,

I've just posted a fix for this issue as well (the AIX build is broken as well).

http://mail.openjdk.java.net/pipermail/build-dev/2018-October/023559.html

Instead of checking the JVM_VARIANT, I check for ENABLE_CDS. But as
far as I see, that still doesn't help for the 'minmal' variant. I
think the right fix would be to set ENABLE_CDS to false for the
minimal build. I could add that to my fix if you're all right with it?

Regards,
Volker
On Mon, Oct 8, 2018 at 9:54 AM Aleksey Shipilev  wrote:
>
> Bug:
>   https://bugs.openjdk.java.net/browse/JDK-8211838
>
> Seems like Minimal VM has CDS disabled as VM feature, which means it would 
> fail during CDS archive
> generation at the end of the build. It looks like the situation Zero is in, 
> so the quick fix is:
>
> diff -r f697ba5b18d2 make/autoconf/jdk-options.m4
> --- a/make/autoconf/jdk-options.m4  Mon Oct 08 13:25:39 2018 +0800
> +++ b/make/autoconf/jdk-options.m4  Mon Oct 08 09:34:16 2018 +0200
> @@ -609,10 +609,11 @@
>  
> 
>  #
>  # Disable the default CDS archive generation
>  #   cross compilation - disabled
>  #   zero  - off by default (not a tested configuration)
> +#   minimal   - off by default (not a tested configuration)
>  #
>  AC_DEFUN([JDKOPT_ENABLE_DISABLE_CDS_ARCHIVE],
>  [
>AC_ARG_ENABLE([cds-archive], [AS_HELP_STRING([--disable-cds-archive],
>[Set to disable generation of a default CDS archive in the product 
> image @<:@enabled@:>@])])
> @@ -625,10 +626,13 @@
>  AC_MSG_RESULT([yes, forced])
>  BUILD_CDS_ARCHIVE="true"
>elif HOTSPOT_CHECK_JVM_VARIANT(zero); then
>  AC_MSG_RESULT([no])
>  BUILD_CDS_ARCHIVE="false"
> +  elif HOTSPOT_CHECK_JVM_VARIANT(minimal); then
> +AC_MSG_RESULT([no])
> +BUILD_CDS_ARCHIVE="false"
>elif test "x$enable_cds_archive" = "x"; then
>  AC_MSG_RESULT([yes])
>  BUILD_CDS_ARCHIVE="true"
>elif test "x$enable_cds_archive" = "xno"; then
>  AC_MSG_RESULT([no, forced])
>
> I think a good follow-up would be actually checking for "cds" as feature, and 
> disabling cds-archive
> based on that. I think --with-jvm-features=-cds fails the same way.
>
> Testing: x86_64 Minimal/Server builds
>
> Thanks,
> -Aleksey
>


RFR(XS): 8211837: Creation of the default CDS Archive should depend on ENABLE_CDS

2018-10-08 Thread Volker Simonis
Hi,

can I please have a review for the following trivial build fix:

http://cr.openjdk.java.net/~simonis/webrevs/2018/8211837/
https://bugs.openjdk.java.net/browse/JDK-8211837

It makes no sense to try to create a default CDS archive on VMs which
don't support CDS at all. We already have '--enable_cds' which
defaults to 'true' on all platforms except AIX.

The check for '--enable_cds_archive' should use the result of
'--enable_cds' (which is saved in "ENABLE_CDS") and only enable
default archive creation if CDS is enabled.

Failure to do so currently breaks the AIX build (after JDK-)8202951 was pushed).

Thank you and best regards,
Volker


Re: RFC: JEP JDK-8208089: Implement C++14 Language Features

2018-10-04 Thread Volker Simonis
Hi Tim, Jeff,

Kim has assembled a quite detailed list of new C++ features intended for
use in HotSpot/OpenJDK (with links to the corresponding C++ standard)

https://bugs.openjdk.java.net/browse/JDK-8208089

Could you please provide a summary of which of these features are already
available since which version of xlC for AIX and potentially escalate the
implementation of the missing features within IBM’s xlC on AIX team.

Thank you and best regards,
Volker


Kim Barrett  schrieb am Mi. 3. Okt. 2018 um 22:13:

> I've submitted a JEP for
>
> (1) enabling the use of C++14 Language Features when building the JDK,
>
> (2) define a process for deciding and documenting which new features
> can be used or are forbidden in HotSpot code,
>
> (3) provide an initial list of permitted and forbidden new features.
>
> https://bugs.openjdk.java.net/browse/JDK-8208089
>
>


Re: RFR(S): 8211145: [ppc] [s390]: Build fails due to -Werror=switch (introduced with JDK-8211029)

2018-10-01 Thread Volker Simonis
Hi Lutz,

the change looks good.

Thanks for cleaning this up,
Volker

On Fri, Sep 28, 2018 at 12:30 PM Aleksey Shipilev  wrote:
>
> On 09/27/2018 04:28 PM, Schmidt, Lutz wrote:
> > re break vs. ShouldNotReachHere(), I tried to change semantics as little as 
> > possible. After
> > discussion with colleagues, we concluded that ShouldNotReachHere() is the 
> > better choice. Code was
> > modified accordingly. Your concerns re. coding style are reflected in the 
> > new webrev as well. It
> > can be found here: http://cr.openjdk.java.net/~lucy/webrevs/8211145.02/
>
> Looks good! Ship it.
>
> -Aleksey
>


Re: RFR 8208183: update HSDIS plugin license to UPL

2018-08-21 Thread Volker Simonis
On Tue, Aug 21, 2018 at 12:14 PM, Magnus Ihse Bursie
 wrote:
>
>> Yes, please. And it would be absolutely fantastic to actually build it and
>> ship it in default
>> OpenJDK images :)
>
>
> Am I correct in understanding that there are no more legal barriers towards
> doing such a thing anymore?
>
> I can easily add a new make target to build hsdis.
>
> Adding a new binary to the OpenJDK images will require a CSR.

Do we really need a CSR for hsdis? hsdis is a shared library/DLL and
not a "binary" which can be called by the user or executed
stand-alone. The functionality offered by hsdis is only accessible by
already existing command line parameters (like -XX:+PrintAssembly).

The CSR FAQ [1] only mentions (among others):

- Adding or removing a command in $JDK/bin
- Adding, removing, or changing a command line option

which both don't seem to be applicable to hsdis.

Regards,
Volker

PS: and just to be clear - I'm all for having hsdis in the default images :)

[1] https://wiki.openjdk.java.net/display/csr/CSR+FAQs

> This in turn
> will likely require that there are proper tests that can be run (I have no
> idea of the current status of hsdis testing). I can't help you with either
> of these two.
>
> /Magnus


Re: RFR: 8206106: [solaris sparc] jck tests api/javax_print/PrintService failing

2018-07-06 Thread Volker Simonis
Hi Phil,

I've actually thought about the same trick yesterday in the evening
and just wanted to try it out when I saw your mail. It indeed works
quite nicely and I can confirm that it solves the problem on our side
as well. I've also opened an issue in the CUPS bug tracker:
https://github.com/apple/cups/issues/5349

I'f you don't mind, I'd suggest to wrap this workaround into "#ifdef
__solaris__" to not affect any other platforms at all. I also suggest
to slightly change the comment to "unless __GNUC__ is defined" because
there are compilers like Clang which are not GNU C but define
"__GNUC__". Finally, I'd also put a link to the CUPS issue into the
comment:

diff -r 93043d28f8fa src/java.desktop/unix/native/common/awt/CUPSfuncs.c
--- a/src/java.desktop/unix/native/common/awt/CUPSfuncs.c   Tue
Jun 12 13:00:50 2018 +0530
+++ b/src/java.desktop/unix/native/common/awt/CUPSfuncs.c   Fri
Jul 06 10:17:39 2018 +0200
@@ -30,6 +30,15 @@
 #include 
 #include 

+#ifdef __solaris__
+  /*
+   * CUPS #define's __attribute__(x) to be empty unless __GNUC__ is defined.
+   * We need to #undef this else it breaks use of this keyword used
by JNIEXPORT.
+   * See: https://github.com/apple/cups/issues/5349
+   */
+  #undef __attribute__
+#endif
+
 //#define CUPS_DEBUG

 #ifdef CUPS_DEBUG

Thank you and best regards,
Volker


On Thu, Jul 5, 2018 at 11:09 PM, Erik Joelsson  wrote:
> Looks good to me.
>
> I would have thought __attribute__ was a macro originally, but since it's
> not, this looks like a very simple solution. Would be nice if Volker could
> verify this as well.
>
> /Erik
>
>
>
> On 2018-07-05 14:06, Phil Race wrote:
>>
>> bug: https://bugs.openjdk.java.net/browse/JDK-8206106
>>
>> Current CUPS include files are defining __attribute__ to be empty
>> We need to undo that to build. Details in the bug report.
>> built + tested on Solaris, Linux, Mac.
>>
>> fix :
>> hg diff src/java.desktop/unix/native/common/awt/CUPSfuncs.c
>> diff --git a/src/java.desktop/unix/native/common/awt/CUPSfuncs.c
>> b/src/java.desktop/unix/native/common/awt/CUPSfuncs.c
>> --- a/src/java.desktop/unix/native/common/awt/CUPSfuncs.c
>> +++ b/src/java.desktop/unix/native/common/awt/CUPSfuncs.c
>> @@ -29,6 +29,12 @@
>>  #include 
>>  #include 
>>  #include 
>> +/*
>> + * CUPS #define's __attribute__(x) to be empty unless the compiler is GNU
>> C.
>> + * We need to #undef this else it breaks use of this keyword used by
>> JNIEXPORT.
>> + */
>> +#undef __attribute__
>> +
>>
>> -phil.
>
>


Re: 11 RFR (XS) 8205956: Fix usage of “OpenJDK” in build and test instructions

2018-06-29 Thread Volker Simonis
On Thu, Jun 28, 2018 at 8:02 PM,   wrote:
> 2018/6/27 23:21:34 -0700, volker.simo...@gmail.com:
>> On Thu, Jun 28, 2018 at 12:08 AM, mark.reinh...@oracle.com wrote:
>>> ...
>>>
>>> “OpenJDK” is a trademarked term, per the OpenJDK Trademark Notice [1].
>>> As such it should be used only as an adjective, and not as a noun.
>>> Phrases such as “the OpenJDK” could be replaced by the more correct,
>>> and much more verbose, “the OpenJDK JDK,” or “the open-source JDK,”
>>> but in most cases the context is sufficiently clear that we can just
>>> write “the JDK.”
>>>
>>
>> Sorry, but I don't see any sense in this change!
>
> Trademark law doesn’t always make a lot of sense.
>
>> Do you plan to do the same for the OpenJDK web pages and the OpenJDK
>> Wiki.
>
> Yes, on a best-effort basis, but priority will be given to more-prominent
> documents such as these.
>
>>   Do you plan to scan all the OpenJDK mails and reject them if
>> they use "OpenJDK" inappropriately?
>
> No, of course not.  That’d be silly.
>
>> And by the way, "JDK" is an Oracle trademark as well (see [1]) so this
>> change is basically a NOP.
>
> Trademark law doesn’t always make a lot of sense.
>

This still doesn't explain why replacing one trademark with another
one is helpful here. "Building the JDK" is clearly using the trademark
"JDK" as a noun and thus infringing the "sens-less" trademark laws.
After Phil's remark, OpenJDK doesn't even seem to be registered as a
trademark, so in that sense the old version "Building the OpenJDK"
seemed to be even more trademark law compliant.

And by the way, I totally agree with Aleksey that changes shouldn't be
pushed if reviewers raise concerns (at least not before these concerns
have been addressed). Just to quote your words about the new release
cadence: "..if your changes don't make it into this release, they will
make it in to the next one which is just six month away" :)

> - Mark


Re: RFR(XXS): 8205916: [test] Fix jdk/tools/launcher/RunpathTest to handle both, RPATH and RUNPATH

2018-06-28 Thread Volker Simonis
Hi Martin, Erik,

thanks for the reviews!

Regards,
Volker


On Wed, Jun 27, 2018 at 5:53 PM, Erik Joelsson  wrote:
> Looks ok to me.
>
> /Erik
>
>
>
> On 2018-06-27 03:26, Volker Simonis wrote:
>>
>> Hi,
>>
>> can I please have a review for the following tiny test fix (I'm
>> actually not sure which would be the appropriate mailing list for this
>> fix so please redirect if necessary):
>>
>> http://cr.openjdk.java.net/~simonis/webrevs/2018/8205916/
>> https://bugs.openjdk.java.net/browse/JDK-8205916
>>
>> The test currently only checks for RPATH in the dynamic section of the
>> launcher, but some linkers / Linux distributions (notably SLES) use
>> RUNPATH instead.
>>
>> Following are the gory details:
>>
>> The test jdk/tools/launcher/RunpathTest.java checks that the java
>> launcher on Linux and Solaris has the correct RPATH path baked into
>> the executable.
>>
>> Unfortunately, the situation with RPATH is a little weird:
>>
>>- in order to bake a runtime path into a dynamically linked library
>> or executable one has to use the "-rpath " linker option (from
>> the C/C++ compiler this is usually available as "-Wl,-rpath,").
>>- depending on the dynamic linker version and Linux distribution,
>> the "-rpath" linker option adds either a "RPATH" entry (Ubuntu 16.04,
>> RHEL 7.4) or a "RUNPATH" entry (SLES 12.1, SLES 15) or both entries
>> together (SLES 11.3) to the dynamic section of the shared
>> library/executable.
>>- the semantics of "RPATH" and "RUNPATH" are slightly different:
>> "RPATH" is evaluated at runtime before LD_LIBRARY_PATH (if "RUNPATH"
>> isn't present) while "RUNPATH" is evaluated after LD_LIBRARY_PATH
>> (i.e. RUNPATH can be overridden at runtime by setting
>> LD_LIBRARY_PATH).
>>- "RPATH" is considered obsolete and should be replaced by "RUNPATH"
>> according to the man-page of "ld.so (8)".
>>- the linker option "--enable-new-dtags"/"--disable-new-dtags" (or
>> the corresponding compiler flags
>> "-Wl,--enable-new-dtags"/"-Wl,--disable-new-dtags") can be used to
>> enforce the generation of "RUNPATH"/"RPATH" respectively (except for
>> systems like SLES 11.3 where "--enable-new-dtags" generates both
>> "RPATH" and "RUNPATH" while "--disable-new-dtags" only generates
>> "RPATH").
>>
>> But this issue is not about fixing the build so to cut a long story
>> short - the test RunpathTest.java should be able to handle both
>> runtime path variants equally well. This can be easily achieved by
>> extending the match pattern from ".*RPATH.*\\$ORIGIN/../lib.*" to
>> ".*R(UN)?PATH.*\\$ORIGIN/../lib.*"
>>
>> Thank you and best regards,
>> Volker
>
>


Re: 11 RFR (XS) 8205956: Fix usage of “OpenJDK” in build and test instructions

2018-06-28 Thread Volker Simonis
On Thu, Jun 28, 2018 at 12:08 AM,   wrote:
> Bug: https://bugs.openjdk.java.net/browse/JDK-8205956
> Webrev: http://cr.openjdk.java.net/~mr/rev/8205956/
>
> Quick links to handier HTML diffs:
>
>   http://cr.openjdk.java.net/~mr/rev/8205956/doc/building.html.hdiff.html
>   http://cr.openjdk.java.net/~mr/rev/8205956/doc/testing.html.hdiff.html
>
> “OpenJDK” is a trademarked term, per the OpenJDK Trademark Notice [1].
> As such it should be used only as an adjective, and not as a noun.
> Phrases such as “the OpenJDK” could be replaced by the more correct,
> and much more verbose, “the OpenJDK JDK,” or “the open-source JDK,”
> but in most cases the context is sufficiently clear that we can just
> write “the JDK.”
>

Sorry, but I don't see any sense in this change!

Do you plan to do the same for the OpenJDK web pages and the OpenJDK
Wiki. Do you plan to scan all the OpenJDK mails and reject them if
they use "OpenJDK" inappropriately?

And by the way, "JDK" is an Oracle trademark as well (see [1]) so this
change is basically a NOP.

Best regards,
Volker

[1] http://tmsearch.uspto.gov/bin/showfield?f=doc=4807:9evmnn.2.10

> - Mark
>
>
> [1] http://openjdk.java.net/legal/openjdk-trademark-notice.html


RFR(XXS): 8205916: [test] Fix jdk/tools/launcher/RunpathTest to handle both, RPATH and RUNPATH

2018-06-27 Thread Volker Simonis
Hi,

can I please have a review for the following tiny test fix (I'm
actually not sure which would be the appropriate mailing list for this
fix so please redirect if necessary):

http://cr.openjdk.java.net/~simonis/webrevs/2018/8205916/
https://bugs.openjdk.java.net/browse/JDK-8205916

The test currently only checks for RPATH in the dynamic section of the
launcher, but some linkers / Linux distributions (notably SLES) use
RUNPATH instead.

Following are the gory details:

The test jdk/tools/launcher/RunpathTest.java checks that the java
launcher on Linux and Solaris has the correct RPATH path baked into
the executable.

Unfortunately, the situation with RPATH is a little weird:

  - in order to bake a runtime path into a dynamically linked library
or executable one has to use the "-rpath " linker option (from
the C/C++ compiler this is usually available as "-Wl,-rpath,").
  - depending on the dynamic linker version and Linux distribution,
the "-rpath" linker option adds either a "RPATH" entry (Ubuntu 16.04,
RHEL 7.4) or a "RUNPATH" entry (SLES 12.1, SLES 15) or both entries
together (SLES 11.3) to the dynamic section of the shared
library/executable.
  - the semantics of "RPATH" and "RUNPATH" are slightly different:
"RPATH" is evaluated at runtime before LD_LIBRARY_PATH (if "RUNPATH"
isn't present) while "RUNPATH" is evaluated after LD_LIBRARY_PATH
(i.e. RUNPATH can be overridden at runtime by setting
LD_LIBRARY_PATH).
  - "RPATH" is considered obsolete and should be replaced by "RUNPATH"
according to the man-page of "ld.so (8)".
  - the linker option "--enable-new-dtags"/"--disable-new-dtags" (or
the corresponding compiler flags
"-Wl,--enable-new-dtags"/"-Wl,--disable-new-dtags") can be used to
enforce the generation of "RUNPATH"/"RPATH" respectively (except for
systems like SLES 11.3 where "--enable-new-dtags" generates both
"RPATH" and "RUNPATH" while "--disable-new-dtags" only generates
"RPATH").

But this issue is not about fixing the build so to cut a long story
short - the test RunpathTest.java should be able to handle both
runtime path variants equally well. This can be easily achieved by
extending the match pattern from ".*RPATH.*\\$ORIGIN/../lib.*" to
".*R(UN)?PATH.*\\$ORIGIN/../lib.*"

Thank you and best regards,
Volker


Re: RFR(M): 8204965: Fix '--disable-cds' and disable CDS on AIX by default

2018-06-19 Thread Volker Simonis
On Tue, Jun 19, 2018 at 9:25 AM, David Holmes  wrote:
> Hi Volker,
>
> On 19/06/2018 4:50 PM, Volker Simonis wrote:
>>
>> On Tue, Jun 19, 2018 at 6:54 AM, David Holmes 
>> wrote:
>>>
>>> Hi Volker,
>>>
>>> v3 looks much cleaner - thanks.
>>>
>>> But AFAICS the change to jvmtiEnv.cpp is also not needed.
>>> ClassLoaderExt::append_boot_classpath exists regardless of INCLUDE_CDS
>>> but
>>> operates differently (just calling
>>> ClassLoader::add_to_boot_append_entries).
>>>
>>
>> That's not entirely true because the whole compilation unit (i.e.
>> classLoaderExt.cpp) which contains
>> 'ClassLoaderExt::append_boot_classpath()' is excluded from the
>> compilation if CDS is disabled (see make/hotspot/lib/JvmFeatures.gmk).
>
>
> Hmmm. There's a CDS bug there. Either classLoaderExt.cpp should not be
> excluded from a non-CDS build, or it should not contain any INCLUDE_CDS
> guards! I suspect it should not be excluded.
>
>> So I can either move the whole implementation of
>> 'ClassLoaderExt::append_boot_classpath()' into classLoaderExt.hpp in
>> which case things would work as you explained and my changes to
>> jvmtiEnv.cpp could be removed or leave the whole code and change as
>> is. Please let me know what you think?
>
>
> In the interest of moving forward you can push what you have and I will file
> a bug against CDS to sort out classLoaderExt.cpp.
>

Thanks! As the current version also passed the submit-repo tests I've pushed it.

Regarding classLoaderExt.cpp, I think it is OK to exclude it for
non-CDS builds. If my IDE doesn't cheat on me (see [1]),
ClassLoaderExt is mostly used from other CDS-only files
(classListParser.cpp, systemDictionaryShared.cpp, filemap.cpp,
metaspaceShared.cpp). The only references from non-CDS files are from
classLoader.cpp an jvmtiEnv.cpp. The ones from classLoader.cpp are all
guarded with 'INCLUDE_CDS' or they only use functions defined in
classLoaderExt.hpp. The single remaining reference from jvmtiEnv.cpp
has been guarded with 'INCLUDE_CDS' by my change.

I think it is a matter of taste if we leave this as is or move the
offending function from classLoaderExt.cpp to classLoaderExt.hpp and
remove the new guard from jvmtiEnv.cpp.

Regards,
Volker

[1] http://cr.openjdk.java.net/~simonis/webrevs/2018/ClassLoaderExt.html

> Thanks,
> David
>
>
>> Regards,
>> Volker
>>
>>> Thanks,
>>> David
>>>
>>>
>>> On 19/06/2018 2:04 AM, Volker Simonis wrote:
>>>>
>>>>
>>>> On Mon, Jun 18, 2018 at 8:17 AM, David Holmes 
>>>> wrote:
>>>>>
>>>>>
>>>>> Hi Volker,
>>>>>
>>>>> src/hotspot/share/runtime/globals.hpp
>>>>>
>>>>> This change should not be needed! We do minimal VM builds without CDS
>>>>> and
>>>>> we
>>>>> don't have to touch the UseSharedSpaces defaults (unless recent change
>>>>> have
>>>>> broken this - in which case that needs to be addressed in its own
>>>>> right!)
>>>>>
>>>>
>>>> Yes, you're right, CDS_ONLY/NOT_CDS isn't really required here,
>>>> because UseSharedSpaces is reseted later on at the end of
>>>> Arguments::parse(). I just thought it would be cleaner to disable it
>>>> statically, if the VM doesn't support it. But anyway I don't really
>>>> mind and I've reverted that change in globals.hpp.
>>>>
>>>>> src/hotspot/share/classfile/javaClasses.cpp
>>>>>
>>>>> AFAICS you should be using INCLUDE_CDS in the ifdefs not
>>>>> INCLUDE_CDS_JAVA_HEAP. But again I'm unclear (as was Thomas) why this
>>>>> should
>>>>> be needed as we have not needed it before. As Thomas notes we have:
>>>>>
>>>>> ./hotspot/share/memory/metaspaceShared.hpp:  static bool
>>>>> is_archive_object(oop p) NOT_CDS_JAVA_HEAP_RETURN_(false);
>>>>> ./hotspot/share/classfile/stringTable.hpp:  static oop
>>>>> create_archived_string(oop s, Thread* THREAD)
>>>>> NOT_CDS_JAVA_HEAP_RETURN_(NULL);
>>>>>
>>>>> so these methods should be defined when CDS is not available.
>>>>>
>>>>
>>>> Thomas and you are right. Must have been a mis-configuration on AIX
>>>> where I saw undefined symbols at link time. I've removed the ifdefs
>>>> from javaClasses.cpp now.
>>>>
>>>> Finally, I've also wrapped all the

Re: RFR(M): 8204965: Fix '--disable-cds' and disable CDS on AIX by default

2018-06-19 Thread Volker Simonis
On Tue, Jun 19, 2018 at 6:54 AM, David Holmes  wrote:
> Hi Volker,
>
> v3 looks much cleaner - thanks.
>
> But AFAICS the change to jvmtiEnv.cpp is also not needed.
> ClassLoaderExt::append_boot_classpath exists regardless of INCLUDE_CDS but
> operates differently (just calling ClassLoader::add_to_boot_append_entries).
>

That's not entirely true because the whole compilation unit (i.e.
classLoaderExt.cpp) which contains
'ClassLoaderExt::append_boot_classpath()' is excluded from the
compilation if CDS is disabled (see make/hotspot/lib/JvmFeatures.gmk).

So I can either move the whole implementation of
'ClassLoaderExt::append_boot_classpath()' into classLoaderExt.hpp in
which case things would work as you explained and my changes to
jvmtiEnv.cpp could be removed or leave the whole code and change as
is. Please let me know what you think?

Regards,
Volker

> Thanks,
> David
>
>
> On 19/06/2018 2:04 AM, Volker Simonis wrote:
>>
>> On Mon, Jun 18, 2018 at 8:17 AM, David Holmes 
>> wrote:
>>>
>>> Hi Volker,
>>>
>>> src/hotspot/share/runtime/globals.hpp
>>>
>>> This change should not be needed! We do minimal VM builds without CDS and
>>> we
>>> don't have to touch the UseSharedSpaces defaults (unless recent change
>>> have
>>> broken this - in which case that needs to be addressed in its own right!)
>>>
>>
>> Yes, you're right, CDS_ONLY/NOT_CDS isn't really required here,
>> because UseSharedSpaces is reseted later on at the end of
>> Arguments::parse(). I just thought it would be cleaner to disable it
>> statically, if the VM doesn't support it. But anyway I don't really
>> mind and I've reverted that change in globals.hpp.
>>
>>> src/hotspot/share/classfile/javaClasses.cpp
>>>
>>> AFAICS you should be using INCLUDE_CDS in the ifdefs not
>>> INCLUDE_CDS_JAVA_HEAP. But again I'm unclear (as was Thomas) why this
>>> should
>>> be needed as we have not needed it before. As Thomas notes we have:
>>>
>>> ./hotspot/share/memory/metaspaceShared.hpp:  static bool
>>> is_archive_object(oop p) NOT_CDS_JAVA_HEAP_RETURN_(false);
>>> ./hotspot/share/classfile/stringTable.hpp:  static oop
>>> create_archived_string(oop s, Thread* THREAD)
>>> NOT_CDS_JAVA_HEAP_RETURN_(NULL);
>>>
>>> so these methods should be defined when CDS is not available.
>>>
>>
>> Thomas and you are right. Must have been a mis-configuration on AIX
>> where I saw undefined symbols at link time. I've removed the ifdefs
>> from javaClasses.cpp now.
>>
>> Finally, I've also wrapped all the FileMapInfo fields in vmStructs.cpp
>> into CDS_ONLY macros as suggested by Jiangli because the really only
>> make sense for a CDS-enabled VM.
>>
>> Here's the new webrev:
>>
>> http://cr.openjdk.java.net/~simonis/webrevs/2018/8204965.v3/
>>
>> Please let me know if you think there's still something missing.
>>
>> Regards,
>> Volker
>>
>>
>>> ??
>>>
>>> Thanks,
>>> David
>>> -
>>>
>>>
>>>
>>>
>>>
>>> On 15/06/2018 12:26 AM, Volker Simonis wrote:
>>>>
>>>>
>>>> Hi,
>>>>
>>>> can I please have a review for the following fix:
>>>>
>>>> http://cr.openjdk.java.net/~simonis/webrevs/2018/8204965/
>>>> https://bugs.openjdk.java.net/browse/JDK-8204965
>>>>
>>>> CDS does currently not work on AIX because of the way how we
>>>> reserve/commit memory on AIX. The problem is that we're using a
>>>> combination of shmat/mmap depending on the page size and the size of
>>>> the memory chunk to reserve. This makes it impossible to reliably
>>>> reserve the memory for the CDS archive and later on map the various
>>>> parts of the archive into these regions.
>>>>
>>>> In order to fix this we would have to completely rework the memory
>>>> reserve/commit/uncommit logic on AIX which is currently out of our
>>>> scope because of resource limitations.
>>>>
>>>> Unfortunately, I could not simply disable CDS in the configure step
>>>> because some of the shared code apparently relies on parts of the CDS
>>>> code which gets excluded from the build when CDS is disabled. So I
>>>> also fixed the offending parts in hotspot and cleaned up the configure
>>>> logic for CDS.
>>>>
>>>> Thank you and best regards,
>>>> Volker
>>>>
>>>> PS: I did run the job through the submit forest
>>>> (mach5-one-simonis-JDK-8204965-20180613-1946-26349) but the results
>>>> weren't really useful because they mention build failures on linux-x64
>>>> which I can't reproduce locally.
>>>>
>>>
>


Re: RFR(M): 8204965: Fix '--disable-cds' and disable CDS on AIX by default

2018-06-18 Thread Volker Simonis
On Mon, Jun 18, 2018 at 8:17 AM, David Holmes  wrote:
> Hi Volker,
>
> src/hotspot/share/runtime/globals.hpp
>
> This change should not be needed! We do minimal VM builds without CDS and we
> don't have to touch the UseSharedSpaces defaults (unless recent change have
> broken this - in which case that needs to be addressed in its own right!)
>

Yes, you're right, CDS_ONLY/NOT_CDS isn't really required here,
because UseSharedSpaces is reseted later on at the end of
Arguments::parse(). I just thought it would be cleaner to disable it
statically, if the VM doesn't support it. But anyway I don't really
mind and I've reverted that change in globals.hpp.

> src/hotspot/share/classfile/javaClasses.cpp
>
> AFAICS you should be using INCLUDE_CDS in the ifdefs not
> INCLUDE_CDS_JAVA_HEAP. But again I'm unclear (as was Thomas) why this should
> be needed as we have not needed it before. As Thomas notes we have:
>
> ./hotspot/share/memory/metaspaceShared.hpp:  static bool
> is_archive_object(oop p) NOT_CDS_JAVA_HEAP_RETURN_(false);
> ./hotspot/share/classfile/stringTable.hpp:  static oop
> create_archived_string(oop s, Thread* THREAD)
> NOT_CDS_JAVA_HEAP_RETURN_(NULL);
>
> so these methods should be defined when CDS is not available.
>

Thomas and you are right. Must have been a mis-configuration on AIX
where I saw undefined symbols at link time. I've removed the ifdefs
from javaClasses.cpp now.

Finally, I've also wrapped all the FileMapInfo fields in vmStructs.cpp
into CDS_ONLY macros as suggested by Jiangli because the really only
make sense for a CDS-enabled VM.

Here's the new webrev:

http://cr.openjdk.java.net/~simonis/webrevs/2018/8204965.v3/

Please let me know if you think there's still something missing.

Regards,
Volker


> ??
>
> Thanks,
> David
> -
>
>
>
>
>
> On 15/06/2018 12:26 AM, Volker Simonis wrote:
>>
>> Hi,
>>
>> can I please have a review for the following fix:
>>
>> http://cr.openjdk.java.net/~simonis/webrevs/2018/8204965/
>> https://bugs.openjdk.java.net/browse/JDK-8204965
>>
>> CDS does currently not work on AIX because of the way how we
>> reserve/commit memory on AIX. The problem is that we're using a
>> combination of shmat/mmap depending on the page size and the size of
>> the memory chunk to reserve. This makes it impossible to reliably
>> reserve the memory for the CDS archive and later on map the various
>> parts of the archive into these regions.
>>
>> In order to fix this we would have to completely rework the memory
>> reserve/commit/uncommit logic on AIX which is currently out of our
>> scope because of resource limitations.
>>
>> Unfortunately, I could not simply disable CDS in the configure step
>> because some of the shared code apparently relies on parts of the CDS
>> code which gets excluded from the build when CDS is disabled. So I
>> also fixed the offending parts in hotspot and cleaned up the configure
>> logic for CDS.
>>
>> Thank you and best regards,
>> Volker
>>
>> PS: I did run the job through the submit forest
>> (mach5-one-simonis-JDK-8204965-20180613-1946-26349) but the results
>> weren't really useful because they mention build failures on linux-x64
>> which I can't reproduce locally.
>>
>


Re: RFR: 8205091: AIX: build errors in hotspot after 8203641: Refactor String Deduplication into shared

2018-06-15 Thread Volker Simonis
Hi Mattias,

the change looks good. Could you please also update the comment in the
line above which still reads "STAT".

Also, maybe "STATI" would be a better name choice (longer is better :)
because the probability of a clash is lower (and it would nicely align
with QUEUE in the comments :) But I leave that up to you...

Regards,
Volker


On Fri, Jun 15, 2018 at 9:47 AM, Baesken, Matthias
 wrote:
> Please review  this small  change  that fixes the AIX build after  "8203641: 
> Refactor String Deduplication into shared".
>
> We are getting this  compilation error  :
> /build_ci_jdk_jdk_rs6000_64/src/hotspot/share/gc/shared/stringdedup/stringDedup.hpp",
>  line 107.38: 1540-0063 (S) The text "1" is unexpected.
>
>
> Looks like the name of the second template parameter (STAT)
>
> template 
> static void initialize_impl();
>
> is clashing with defines from the AIX system headers  (where I find  #define 
> STAT  1 ) .
> Renaming  STAT  to something else fixes the build on AIX .
>
> Webrev :
>
> http://cr.openjdk.java.net/~mbaesken/webrevs/8205091/
>
> Bug :
>
> https://bugs.openjdk.java.net/browse/JDK-8205091
>
>
> Thanks, Matthias


Re: RFR(M): 8204965: Fix '--disable-cds' and disable CDS on AIX by default

2018-06-15 Thread Volker Simonis
On Thu, Jun 14, 2018 at 9:04 PM, Thomas Stüfe  wrote:
> Hi Volker,
>
> http://cr.openjdk.java.net/~simonis/webrevs/2018/8204965/make/autoconf/hotspot.m4.udiff.html
>
> Seems like a roundabout way to have a platform specific default value.
>
> Why not determine a default value beforehand:
>
> if test "x$OPENJDK_TARGET_OS" = "xaix"; then
>   ENABLE_CDS_DEFAULT="false"
> else
>   ENABLE_CDS_DEFAULT=true"
> fi
>
> AC_ARG_ENABLE([cds], [AS_HELP_STRING([--enable-cds@<:@=yes/no/auto@:>@],
>  [enable class data sharing feature in non-minimal VM. Default is
> ${ENABLE_CDS_DEFAULT}.])])
>
> and so on?
>

I've just followed the pattern used for '--enable-aot' right above the
code I changed.

Moreover, I don't think that we would save any code because we would
still have to check for AIX in the '--enable-cds=yes' case. Also, the
new reporting added later in the file (see "AC_MSG_CHECKING([if cds
should be enabled])" seems easier to me without the extra default
value. So if you don't mind I'd prefer to leave it as is.

> See also what we did for "8202325: [aix] disable warnings-as-errors by 
> default".
>
> --
>
> http://cr.openjdk.java.net/~simonis/webrevs/2018/8204965/src/hotspot/share/classfile/javaClasses.cpp.udiff.html
>
> Here, do we really need to exclude this from compiling,
> DumpSharedSpaces = false is not enough?
>

Yes, we need it because the excluded code references methods (e.g.
'StringTable::create_archived_string()') which are not compiled into
libjvm.so if we disable CDS.

>
> Best Regards, Thomas
>
> On Thu, Jun 14, 2018 at 4:26 PM, Volker Simonis
>  wrote:
>> Hi,
>>
>> can I please have a review for the following fix:
>>
>> http://cr.openjdk.java.net/~simonis/webrevs/2018/8204965/
>> https://bugs.openjdk.java.net/browse/JDK-8204965
>>
>> CDS does currently not work on AIX because of the way how we
>> reserve/commit memory on AIX. The problem is that we're using a
>> combination of shmat/mmap depending on the page size and the size of
>> the memory chunk to reserve. This makes it impossible to reliably
>> reserve the memory for the CDS archive and later on map the various
>> parts of the archive into these regions.
>>
>> In order to fix this we would have to completely rework the memory
>> reserve/commit/uncommit logic on AIX which is currently out of our
>> scope because of resource limitations.
>>
>> Unfortunately, I could not simply disable CDS in the configure step
>> because some of the shared code apparently relies on parts of the CDS
>> code which gets excluded from the build when CDS is disabled. So I
>> also fixed the offending parts in hotspot and cleaned up the configure
>> logic for CDS.
>>
>> Thank you and best regards,
>> Volker
>>
>> PS: I did run the job through the submit forest
>> (mach5-one-simonis-JDK-8204965-20180613-1946-26349) but the results
>> weren't really useful because they mention build failures on linux-x64
>> which I can't reproduce locally.


Re: RFR(M): 8204965: Fix '--disable-cds' and disable CDS on AIX by default

2018-06-15 Thread Volker Simonis
Hi Jiangli,

thanks for looking at the change.

'CDS_only' is only required for static fields because the
VMStructEntry for them contains a reference to the actual static field
which isn't present if we disable CDS, because the corresponding
compilations units (i.e. filemap.cpp) won't be part of libjvm.so. For
non-static fields, the VMStructEntry structure only contains the
offset of the corresponding field with regards to an object of that
type which is harmless.

Regards,
Volker


On Thu, Jun 14, 2018 at 6:42 PM, Jiangli Zhou  wrote:
> Hi Volker,
>
> The changes look good to me overall. I’ll refer to the JVMTI experts for
> jvmtiEnv.cpp change. I have a question for the change in vmStructs.cpp. Any
> reason why only _current_info needs CDS_ONLY?
>
>//
> \
>/* FileMapInfo fields (CDS archive related) */
> \
>//
> \
>
> \
>nonstatic_field(FileMapInfo, _header,
> FileMapInfo::FileMapHeader*)   \
> - static_field(FileMapInfo, _current_info,
> FileMapInfo*)  \
> + CDS_ONLY(static_field(FileMapInfo,_current_info,
> FileMapInfo*)) \
>nonstatic_field(FileMapInfo::FileMapHeader,  _space[0],
> FileMapInfo::FileMapHeader::space_info)\
>nonstatic_field(FileMapInfo::FileMapHeader::space_info, _addr._base,
> char*) \
>nonstatic_field(FileMapInfo::FileMapHeader::space_info, _used,
> size_t)        \
>
> \
>
> Thanks,
> Jiangli
>
> On Jun 14, 2018, at 7:26 AM, Volker Simonis 
> wrote:
>
> Hi,
>
> can I please have a review for the following fix:
>
> http://cr.openjdk.java.net/~simonis/webrevs/2018/8204965/
> https://bugs.openjdk.java.net/browse/JDK-8204965
>
> CDS does currently not work on AIX because of the way how we
> reserve/commit memory on AIX. The problem is that we're using a
> combination of shmat/mmap depending on the page size and the size of
> the memory chunk to reserve. This makes it impossible to reliably
> reserve the memory for the CDS archive and later on map the various
> parts of the archive into these regions.
>
> In order to fix this we would have to completely rework the memory
> reserve/commit/uncommit logic on AIX which is currently out of our
> scope because of resource limitations.
>
> Unfortunately, I could not simply disable CDS in the configure step
> because some of the shared code apparently relies on parts of the CDS
> code which gets excluded from the build when CDS is disabled. So I
> also fixed the offending parts in hotspot and cleaned up the configure
> logic for CDS.
>
> Thank you and best regards,
> Volker
>
> PS: I did run the job through the submit forest
> (mach5-one-simonis-JDK-8204965-20180613-1946-26349) but the results
> weren't really useful because they mention build failures on linux-x64
> which I can't reproduce locally.
>
>


RFR(M): 8204965: Fix '--disable-cds' and disable CDS on AIX by default

2018-06-14 Thread Volker Simonis
Hi,

can I please have a review for the following fix:

http://cr.openjdk.java.net/~simonis/webrevs/2018/8204965/
https://bugs.openjdk.java.net/browse/JDK-8204965

CDS does currently not work on AIX because of the way how we
reserve/commit memory on AIX. The problem is that we're using a
combination of shmat/mmap depending on the page size and the size of
the memory chunk to reserve. This makes it impossible to reliably
reserve the memory for the CDS archive and later on map the various
parts of the archive into these regions.

In order to fix this we would have to completely rework the memory
reserve/commit/uncommit logic on AIX which is currently out of our
scope because of resource limitations.

Unfortunately, I could not simply disable CDS in the configure step
because some of the shared code apparently relies on parts of the CDS
code which gets excluded from the build when CDS is disabled. So I
also fixed the offending parts in hotspot and cleaned up the configure
logic for CDS.

Thank you and best regards,
Volker

PS: I did run the job through the submit forest
(mach5-one-simonis-JDK-8204965-20180613-1946-26349) but the results
weren't really useful because they mention build failures on linux-x64
which I can't reproduce locally.


Re: RFR(XS): 8204684: [AIX] Build of libjli_static broken after change 8204572 (SetupJdkLibrary)

2018-06-11 Thread Volker Simonis
Hi Erik, Thomas,

thanks for the quick review!

Regards,
Volker

On Mon, Jun 11, 2018 at 5:59 PM, Thomas Stüfe  wrote:
> This looks good and I can confirm fixes our AIX build,
>
> Thanks for fixing!
>
>
> ..Thomas
>
>
> On Mon, Jun 11, 2018 at 4:48 PM, Volker Simonis
>  wrote:
>> Hi,
>>
>> can I please have a review for the following trivial build change
>> which fixes a build problem on AIX when building libjli_static:
>>
>> http://cr.openjdk.java.net/~simonis/webrevs/2018/8204684/
>> https://bugs.openjdk.java.net/browse/JDK-8204684
>>
>> The reason is that change 8204572 forgot to add LIBJLI_SRC_DIRS as
>> include path for the libjli_static build.
>>
>> Thank you and best regards,
>> Volker


RFR(XS): 8204684: [AIX] Build of libjli_static broken after change 8204572 (SetupJdkLibrary)

2018-06-11 Thread Volker Simonis
Hi,

can I please have a review for the following trivial build change
which fixes a build problem on AIX when building libjli_static:

http://cr.openjdk.java.net/~simonis/webrevs/2018/8204684/
https://bugs.openjdk.java.net/browse/JDK-8204684

The reason is that change 8204572 forgot to add LIBJLI_SRC_DIRS as
include path for the libjli_static build.

Thank you and best regards,
Volker


Re: RFR: JDK-8202384: Introduce altserver jvm variant with speculative execution disabled

2018-06-05 Thread Volker Simonis
On Tue, Jun 5, 2018 at 6:47 PM, Erik Joelsson  wrote:
> Hello Volker,
>
> On 2018-06-05 09:35, Volker Simonis wrote:
>>
>> Hi Erik,
>>
>> you wrote: "Note that this applies to the build time C++ flags, not
>> the compiler in the JVM itself." So what about the code generated by
>> the HotSpot (i.e. stubs, template interpreter, c1, c2, Graal)? Is this
>> code already "hardened" against speculative execution? If yes, that's
>> fine, if not, I don't see the point in hardening the HotSpot code
>> itself if the VM still generates potentially "insecure" code.
>
> Correct. These are just the build changes for the build time compiler
> options. Further work will be done by Hotspot engineers.
>>
>> And I still don't fully understand how things work if you build both
>> variants by default (as you intend to do it for Oracle builds). Will
>> you end up with two subdirectories (lib/server/ and lib/altserver)
>> where both contain a libjvm.so and the user can use "java -altserver"
>> on the command line to choose the hardened version?
>
> Correct, we use the old jvm variant mechanism, so this will work just like
> -server/-client used to work, two completely separate libjvm.so in separate
> sub directories.
>

OK. Thanks for the quick confirmation.

> /Erik
>
>> Thank you and best regards,
>> Volker
>>
>>
>> On Tue, Jun 5, 2018 at 5:47 PM, Erik Joelsson 
>> wrote:
>>>
>>> Hello,
>>>
>>> On 2018-06-04 23:10, David Holmes wrote:
>>>>
>>>> Sorry to be late to this party ...
>>>>
>>>> On 5/06/2018 6:10 AM, Erik Joelsson wrote:
>>>>>
>>>>> New webrev: http://cr.openjdk.java.net/~erikj/8202384/webrev.02/
>>>>>
>>>>> Renamed the new jvm variant to "hardened".
>>>>
>>>>
>>>> As it is a hardened server build I'd prefer if that were somehow
>>>> reflected
>>>> in the name. Though really I don't see why this should be restricted
>>>> this
>>>> way ... to be honest I don't see hardened as a variant of server vs.
>>>> client
>>>> vs. zero etc at all, you should be able to harden any of those.
>>>>
>>> I agree, and you sort of can. By adding the jvm feature
>>> "no-speculative-cti"
>>> to any jvm variant, you get the flags. The name of the predefined variant
>>> can be discussed. I initially suggested altserver because I, as you,
>>> thought
>>> it should include server in the name. But ultimately, I don't care that
>>> much
>>> about a name. There is also little point in defining a whole set of
>>> predefined variants that nobody has requested. If we ever need more
>>> specialized variants in the same image, we will add them.
>>>>
>>>> So IIUC with this change we will:
>>>> - always build JDK native code "hardened" (if toolchain supports it)
>>>
>>> Yes, this is correct. The reason being that no significant performance
>>> impact was detected, so there is no cost.
>>>>
>>>> - only build hotspot "hardened" if requested; and in that case
>>>>- jvm.cfg will list -server and -hardened with server as default
>>>
>>> Correct.
>>>>
>>>>
>>>> Is that right? I can see that we may choose to always build Oracle JDK
>>>> this way but it isn't clear to me that its suitable for OpenJDK. Nor why
>>>> hotspot is selectable but JDK is not. ??
>>>>
>>> We would prefer to always build with security features enabled, but the
>>> performance impact on the JVM is so high that we want to leave it to the
>>> user to decide, both at bulid time and at runtime. With these changes,
>>> Oracle will build OracleJDK, and OpenJDK, with dual JVMs by default, but
>>> any
>>> other person or entity just building the OpenJDK source will just get the
>>> server variant for now (as has been the default for a long time), unless
>>> they specifically ask for "hardened" or activate the new jvm feature
>>> (--with-jvm-feature=no-speculative-cti).
>>>
>>> We don't see the point in giving the choice on the JDK libraries simply
>>> because there is no drawback to enabling the flags.
>>>
>>> /Erik
>>>
>>>> Sorry.
>>>>
>>>> David
>>>> -
>>>>
>>>>> /Erik
>>&

Re: RFR: JDK-8202384: Introduce altserver jvm variant with speculative execution disabled

2018-06-05 Thread Volker Simonis
Hi Erik,

you wrote: "Note that this applies to the build time C++ flags, not
the compiler in the JVM itself." So what about the code generated by
the HotSpot (i.e. stubs, template interpreter, c1, c2, Graal)? Is this
code already "hardened" against speculative execution? If yes, that's
fine, if not, I don't see the point in hardening the HotSpot code
itself if the VM still generates potentially "insecure" code.

And I still don't fully understand how things work if you build both
variants by default (as you intend to do it for Oracle builds). Will
you end up with two subdirectories (lib/server/ and lib/altserver)
where both contain a libjvm.so and the user can use "java -altserver"
on the command line to choose the hardened version?

Thank you and best regards,
Volker


On Tue, Jun 5, 2018 at 5:47 PM, Erik Joelsson  wrote:
> Hello,
>
> On 2018-06-04 23:10, David Holmes wrote:
>>
>> Sorry to be late to this party ...
>>
>> On 5/06/2018 6:10 AM, Erik Joelsson wrote:
>>>
>>> New webrev: http://cr.openjdk.java.net/~erikj/8202384/webrev.02/
>>>
>>> Renamed the new jvm variant to "hardened".
>>
>>
>> As it is a hardened server build I'd prefer if that were somehow reflected
>> in the name. Though really I don't see why this should be restricted this
>> way ... to be honest I don't see hardened as a variant of server vs. client
>> vs. zero etc at all, you should be able to harden any of those.
>>
> I agree, and you sort of can. By adding the jvm feature "no-speculative-cti"
> to any jvm variant, you get the flags. The name of the predefined variant
> can be discussed. I initially suggested altserver because I, as you, thought
> it should include server in the name. But ultimately, I don't care that much
> about a name. There is also little point in defining a whole set of
> predefined variants that nobody has requested. If we ever need more
> specialized variants in the same image, we will add them.
>>
>> So IIUC with this change we will:
>> - always build JDK native code "hardened" (if toolchain supports it)
>
> Yes, this is correct. The reason being that no significant performance
> impact was detected, so there is no cost.
>>
>> - only build hotspot "hardened" if requested; and in that case
>>   - jvm.cfg will list -server and -hardened with server as default
>
> Correct.
>>
>>
>> Is that right? I can see that we may choose to always build Oracle JDK
>> this way but it isn't clear to me that its suitable for OpenJDK. Nor why
>> hotspot is selectable but JDK is not. ??
>>
> We would prefer to always build with security features enabled, but the
> performance impact on the JVM is so high that we want to leave it to the
> user to decide, both at bulid time and at runtime. With these changes,
> Oracle will build OracleJDK, and OpenJDK, with dual JVMs by default, but any
> other person or entity just building the OpenJDK source will just get the
> server variant for now (as has been the default for a long time), unless
> they specifically ask for "hardened" or activate the new jvm feature
> (--with-jvm-feature=no-speculative-cti).
>
> We don't see the point in giving the choice on the JDK libraries simply
> because there is no drawback to enabling the flags.
>
> /Erik
>
>> Sorry.
>>
>> David
>> -
>>
>>> /Erik
>>>
>>>
>>> On 2018-06-04 09:54, jesper.wilhelms...@oracle.com wrote:
>
> On 4 Jun 2018, at 17:52, Erik Joelsson 
> wrote:
>
> Hello,
>
> On 2018-06-01 14:00, Aleksey Shipilev wrote:
>>
>> On 06/01/2018 10:53 PM, Erik Joelsson wrote:
>>>
>>> This patch defines flags for disabling speculative execution for GCC
>>> and Visual Studio and applies
>>> them to all binaries except libjvm when available in the compiler. It
>>> defines a new jvm feature
>>> no-speculative-cti, which is used to control whether to use the flags
>>> for libjvm. It also defines a
>>> new jvm variant "altserver" which is the same as server, but with
>>> this new feature added.
>>
>> I think the classic name for such product configuration is "hardened",
>> no?
>
> I don't know. I'm open to suggestions on naming.

 "hardened" sounds good to me.

 The change looks good as well.
 /Jesper

> /Erik
>>
>> -Aleksey
>>
>>>
>


Re: License and Usage Terms of generated API documentation

2018-05-03 Thread Volker Simonis
On Thu, May 3, 2018 at 12:21 PM, Magnus Ihse Bursie
<magnus.ihse.bur...@oracle.com> wrote:
> On 2018-05-02 17:03, Volker Simonis wrote:
>>
>> Hi,
>>
>> we currently build OpenJDK and make it available from various sources
>> (e.g. GitHub, apt-get server, DockerHub). We also build the API
>> documentation (i.e. JavaDoc) and would like to make it available from
>> our project page as well. However the default API doc produced by the
>> build looks as follows:
>>
>>
>> http://cr.openjdk.java.net/~simonis/webrevs/2018/jdk10-api-doc/overview-summary.html
>>
>> Especially the footer seems to be weired and only valid for the API
>> doc hosted by Oracle itself. It reads as follows:
>>
>> Use is subject to "license terms" and the "documentation redistribution
>> policy".
>>
>> "license terms" links to
>>
>> http://www.oracle.com/technetwork/java/javase/terms/license/java10speclicense.html
>> which redirects to
>> http://download.oracle.com/otndocs/jcp/java_se-10-final-spec/license.html
>>
>> "documentation redistribution policy" links to
>> http://www.oracle.com/technetwork/java/redist-137594.html
>>
>> Especially the "documentation redistribution policy" is very strict. It
>> states:
>>
>> "The Java SE API Specification is not redistributable."
>>
>> This is a very strong statement. While it may be fine for the API
>> documentation hosted by Oracle (under
>> https://docs.oracle.com/javase/10/docs/api/overview-summary.html) I
>> doubt this can be valid for the OpenJDK API documentation which was
>> produced exclusively from GPLv2 licensed sources (actually even GPLv2
>> plus Classpath Exception). From my understanding the whole HTML tree
>> generated by "make docs-image" should be by default licensed under
>> GPLv2 as well.
>>
>> I would therefore like to propose to make the following variables from
>> "make/Docs.gmk" configurable with corresponding configure flags:
>>
>> JAVADOC_BASE_URL :=
>> http://www.oracle.com/pls/topic/lookup?ctx=javase10id=homepage
>> BUG_SUBMIT_URL := http://bugreport.java.com/bugreport/
>> COPYRIGHT_URL := {@docroot}/../legal/copyright.html
>> LICENSE_URL :=
>> http://www.oracle.com/technetwork/java/javase/terms/license/java10speclicense.html
>> REDISTRIBUTION_URL :=
>> http://www.oracle.com/technetwork/java/redist-137594.html
>>
>> "JAVADOC_BASE_URL" should by default point to an OpenJDK site
>> (although I'm not sure which one will be best suited). It seems
>> strange that the default documentation generated from an OpenSource
>> project like OpenJDK points to some Oracle-proprietary documentation.
>>
>> "BUG_SUBMIT_URL" should use the value of the already existing
>> "--with-vendor-bug-url" if that was set at configure time.
>>
>> "COPYRIGHT_URL" currently points to "copyright.html" which doesn't
>> exist neither in the OpenJDK sources nor in the generated images. Not
>> sure what would be a useful default value here. Maybe just leave it
>> empty? "Copyright © 1993, 2018, Oracle.." already seems
>> self-explanatory and clear enough.
>>
>> "LICENSE_URL" and "REDISTRIBUTION_URL" should both by default point to
>> the GPLv2+CPE LICENSE file and this LICENSE file should be copied into
>> the API doc HTML tree (much like it is copied into the various
>> subdirectories of the "legel/" directory of an OpenJDK image)
>>
>> I can open an issue for this topic and propose an implementation if
>> there's consensus on this topic.
>>
>> Thank you and best regards,
>> Volker
>
> Since you added build-dev, I'll chip in some of my cents.
>
> The code in the make file has been around like Forever. The current
> implementation is just what has been ported between frameworks and source
> control systems and updated for changes in release version and URL schemes.
>

Thanks for confirming this. That was my impression as well :)

> I agree that is odd that the Oracle URL is hard-coded. On the other hand,
> there is no common place where generated OpenJDK documents are published.
> (The Oracle site technically speaking documents the Java Platform API, not
> the OpenJDK, even if any actual difference is minimal to non-existant wrt
> docs.)
>
> It would make sense to me to have a OpenJDK-based documentation driven by
> the community. I'm guessing it's no easy thing to create this, though. A lot
> of links on the interwebz are po

License and Usage Terms of generated API documentation

2018-05-02 Thread Volker Simonis
Hi,

we currently build OpenJDK and make it available from various sources
(e.g. GitHub, apt-get server, DockerHub). We also build the API
documentation (i.e. JavaDoc) and would like to make it available from
our project page as well. However the default API doc produced by the
build looks as follows:

http://cr.openjdk.java.net/~simonis/webrevs/2018/jdk10-api-doc/overview-summary.html

Especially the footer seems to be weired and only valid for the API
doc hosted by Oracle itself. It reads as follows:

Use is subject to "license terms" and the "documentation redistribution policy".

"license terms" links to
http://www.oracle.com/technetwork/java/javase/terms/license/java10speclicense.html
which redirects to
http://download.oracle.com/otndocs/jcp/java_se-10-final-spec/license.html

"documentation redistribution policy" links to
http://www.oracle.com/technetwork/java/redist-137594.html

Especially the "documentation redistribution policy" is very strict. It states:

"The Java SE API Specification is not redistributable."

This is a very strong statement. While it may be fine for the API
documentation hosted by Oracle (under
https://docs.oracle.com/javase/10/docs/api/overview-summary.html) I
doubt this can be valid for the OpenJDK API documentation which was
produced exclusively from GPLv2 licensed sources (actually even GPLv2
plus Classpath Exception). From my understanding the whole HTML tree
generated by "make docs-image" should be by default licensed under
GPLv2 as well.

I would therefore like to propose to make the following variables from
"make/Docs.gmk" configurable with corresponding configure flags:

JAVADOC_BASE_URL :=
http://www.oracle.com/pls/topic/lookup?ctx=javase10id=homepage
BUG_SUBMIT_URL := http://bugreport.java.com/bugreport/
COPYRIGHT_URL := {@docroot}/../legal/copyright.html
LICENSE_URL := 
http://www.oracle.com/technetwork/java/javase/terms/license/java10speclicense.html
REDISTRIBUTION_URL := http://www.oracle.com/technetwork/java/redist-137594.html

"JAVADOC_BASE_URL" should by default point to an OpenJDK site
(although I'm not sure which one will be best suited). It seems
strange that the default documentation generated from an OpenSource
project like OpenJDK points to some Oracle-proprietary documentation.

"BUG_SUBMIT_URL" should use the value of the already existing
"--with-vendor-bug-url" if that was set at configure time.

"COPYRIGHT_URL" currently points to "copyright.html" which doesn't
exist neither in the OpenJDK sources nor in the generated images. Not
sure what would be a useful default value here. Maybe just leave it
empty? "Copyright © 1993, 2018, Oracle.." already seems
self-explanatory and clear enough.

"LICENSE_URL" and "REDISTRIBUTION_URL" should both by default point to
the GPLv2+CPE LICENSE file and this LICENSE file should be copied into
the API doc HTML tree (much like it is copied into the various
subdirectories of the "legel/" directory of an OpenJDK image)

I can open an issue for this topic and propose an implementation if
there's consensus on this topic.

Thank you and best regards,
Volker


Re: RFR : 8202322: AIX: symbol visibility flags not support on xlc 12.1

2018-04-26 Thread Volker Simonis
Hi Matthias,

after Bhaktavatsal Reddy's report about the problems with
"-qvisibility" with xlC 13 and taking into account that we can't test
this anyway because we don't currently have xlC 13
 on our machines I think it would be best to completely remove this
option for now on AIX. Once we get xlC 13 we can revisit the issue.

Thanks,
Volker


On Thu, Apr 26, 2018 at 4:59 PM, Bhaktavatsal R Maram
 wrote:
> Hi Matthias,
>
> At this point, I think we can remove the flag as you found that it is not 
> supported in XLC < 13. And with XLC 13, it require more work to use this flag.
>
> Thanks,
> Bhaktavatsal Reddy
>
>
>
> -"Baesken, Matthias"  wrote: -
> To: "Langer, Christoph" , 
> "'build-dev@openjdk.java.net'" , 
> "ppc-aix-port-...@openjdk.java.net" , 
> "core-libs-...@openjdk.java.net" 
> From: "Baesken, Matthias" 
> Date: 04/26/2018 08:23PM
> Cc: "Simonis, Volker" , Bhaktavatsal R Maram
> 
> Subject: RE: RFR : 8202322: AIX: symbol visibility flags not support on xlc 
> 12.1
>
>
>  Hello Christoph,   I think  all XLC versions  < 12.1   are unsupported  (and 
> probably not working anyway)  for the OpenJDK  build .
>  I am only aware   of  XLC  versions  12.1  and 13.1which  work / in case 
> of 13.1  “might” work   for OpenJDK compilation .
>  And for 12.1  I want to remove the flags  .
>
>  ( waiting for the feedback  of   Bhaktavatsal Reddy ,  in case he  prefers 
> it  I remove them for all xlC versions including 13.1 )
>
>  Best regards, Matthias
>
>
>
>
>
>
>  From: Langer, Christoph
>  Sent: Donnerstag, 26. April 2018 16:38
>  To: Baesken, Matthias ; 
> 'build-dev@openjdk.java.net' ; 
> ppc-aix-port-...@openjdk.java.net; core-libs-...@openjdk.java.net
>  Cc: Simonis, Volker 
>  Subject: RE: RFR : 8202322: AIX: symbol visibility flags not support on xlc 
> 12.1
>
>  Hi Matthias,
>
>  to me the change in principal looks good.
>
>  I’m wondering if it is possible to do a comparison like xlc < 13 (e.g. 
> extract major number before the first dot, then compare numerically) – but 
> maybe it is too complicated and the current single version compare suits  our 
> needs ?
>
>  Best regards
>  Christoph
>
>
>
>
>  From: Baesken, Matthias
>  Sent: Donnerstag, 26. April 2018 16:14
>  To: 'build-dev@openjdk.java.net' ; 
> ppc-aix-port-...@openjdk.java.net; core-libs-...@openjdk.java.net
>  Cc: Langer, Christoph ; Simonis, Volker 
> 
>  Subject: RFR : 8202322: AIX: symbol visibility flags not support on xlc 12.1
>
>  Hello  ,  could you please review this small adjustment to  the symbol 
> visibility compilation settings on AIX ?
>  Currently  we use  XLC 12.1  to compile  JDK on AIX .
>
>  However XLC 12.1   does not support  the “-qvisibility=hidden”  setting 
> currently set on AIX.
>  It was introduced with  XLC 13.1 . Christoph found some info about it here :
>
>  
> https://www.ibm.com/developerworks/aix/library/au-aix-symbol-visibility-part2/index.html
>
>  Setting it  only generates  hundreds  of warnings  in the build log , 
> warnings look like this :
>  XlC12.1
>
>  bash-4.4$ xlC -qversion
>  IBM XL C/C++ for AIX, V12.1 (5765-J02, 5725-C72)
>  Version: 12.01..0019
>
>  bash-4.4$ xlC -qvisibility=hidden sizeof.c -o sizeof_aixxlc
>  1506-173 (W) Option visibility=hidden is not valid. Enter xlC for list of 
> valid options.
>
>  Compare to XLC13.1
>
>  bash-3.00$ xlC -qversion
>  IBM XL C/C++ for AIX, V13.1 (5725-C72, 5765-J07)
>  Version: 13.01..0008
>  bash-3.00$ xlC -qvisibility=default sizeof.c -o sizeof_aixxlc
>  bash-3.00$ xlC -qvisibility=hidden sizeof.c -o sizeof_aixxlc
>
>
>  So it is better to avoid  setting these flags when using xlc12.1   .
>  Please review :
>
>  Bug :
>
>  https://bugs.openjdk.java.net/browse/JDK-8202322
>
>  Change :
>
>  http://cr.openjdk.java.net/~mbaesken/webrevs/8202322/
>
>
>  Best regards, Matthias
>
>
>
>


Re: RFR(XS): 8201584: Fix configure on SLES 11 after 8201483

2018-04-16 Thread Volker Simonis
Looks good!

Thanks for fixing,
Volker


On Mon, Apr 16, 2018 at 2:15 PM, Lindenmaier, Goetz
 wrote:
> Hi Magnus,
>
> yes, that works too:
> http://cr.openjdk.java.net/~goetz/wr18/8201584-fixSLES11configure/02/
> Can I push this right away if I get a second review?
>
> Best regards,
>  Goetz
>
>> -Original Message-
>> From: Magnus Ihse Bursie [mailto:magnus.ihse.bur...@oracle.com]
>> Sent: Montag, 16. April 2018 14:00
>> To: Lindenmaier, Goetz ; build-dev (build-
>> d...@openjdk.java.net) 
>> Subject: Re: RFR(XS): 8201584: Fix configure on SLES 11 after 8201483
>>
>> On 2018-04-16 11:16, Lindenmaier, Goetz wrote:
>> > Hi,
>> >
>> > could I please get reviewes for this tiny fix?
>> > Grep does not grok the syntax of the replacement of space to newline.
>> > It causes configure failures on SLES 11.
>> >
>> > http://cr.openjdk.java.net/~goetz/wr18/8201584-fixSLES11configure/01/
>> Aha, that was the reason for those odd NEEDLE and STACK constructs. :-)
>>
>> Goetz, please try this patch instead, which uses the new
>> BASIC_GET_NON_MATCHING_VALUES function. Hopefully, it works
>> correctly on
>> SLES 11 as well.
>>
>> diff --git a/make/autoconf/hotspot.m4 b/make/autoconf/hotspot.m4
>> --- a/make/autoconf/hotspot.m4
>> +++ b/make/autoconf/hotspot.m4
>> @@ -443,7 +443,7 @@
>>   AC_MSG_RESULT(["$JVM_FEATURES_FOR_VARIANT"])
>>
>>   # Validate features (for configure script errors, not user errors)
>> -INVALID_FEATURES=`$GREP -Fvx "${VALID_JVM_FEATURES// /$'\n'}" <<<
>> "${JVM_FEATURES_FOR_VARIANT// /$'\n'}"`
>> +BASIC_GET_NON_MATCHING_VALUES(INVALID_FEATURES,
>> $JVM_FEATURES_FOR_VARIANT, $VALID_JVM_FEATURES)
>>   if test "x$INVALID_FEATURES" != x; then
>> AC_MSG_ERROR([Internal configure script error. Invalid JVM
>> feature(s): $INVALID_FEATURES])
>>   fi
>>
>> /Magnus


Re: 8201495: [Zero] Reduce limits of max heap size for boot JDK on s390

2018-04-16 Thread Volker Simonis
On Mon, Apr 16, 2018 at 11:30 AM, Severin Gehwolf  wrote:
> Hi Andrew,
>
> On Mon, 2018-04-16 at 09:47 +0100, Andrew Haley wrote:
>> On 04/13/2018 02:40 PM, Severin Gehwolf wrote:
>> > ++ /usr/bin/tee 
>> > /builddir/build/BUILD/java-9-openjdk-9.0.4.12-5.openjdk9.el7.s390/openjdk/build/jdk/modules/java.base/_the.java.base_batch.log
>> > ++ /usr/bin/tee 
>> > /builddir/build/BUILD/java-9-openjdk-9.0.4.12-5.openjdk9.el7.s390/openjdk/build/jdk/modules/java.base/_the.java.base_batch.log
>> > Error occurred during initialization of VM
>> > Could not reserve enough space for 1048576KB object heap
>>
>> What is the root cause of this?  Is it that the system on which the build 
>> runs
>> cannot allocate all that memory?  Does not have that much memory?
>
> The configure code in JDK 9+ has this:
>
>   JVM_HEAP_LIMIT_32="1024"
>   # Running a 64 bit JVM allows for and requires a bigger heap
>   JVM_HEAP_LIMIT_64="1600"
>   STACK_SIZE_32=768
>   STACK_SIZE_64=1536
>   JVM_HEAP_LIMIT_GLOBAL=`expr $MEMORY_SIZE / 2`
>   if test "$JVM_HEAP_LIMIT_GLOBAL" -lt "$JVM_HEAP_LIMIT_32"; then
> JVM_HEAP_LIMIT_32=$JVM_HEAP_LIMIT_GLOBAL
>   fi
>   if test "$JVM_HEAP_LIMIT_GLOBAL" -lt "$JVM_HEAP_LIMIT_64"; then
> JVM_HEAP_LIMIT_64=$JVM_HEAP_LIMIT_GLOBAL
>   fi
>   if test "$JVM_HEAP_LIMIT_GLOBAL" -lt "512"; then
> JVM_HEAP_LIMIT_32=512
> JVM_HEAP_LIMIT_64=512
>   fi
>
>   if test "x$BOOT_JDK_BITS" = "x32"; then
> STACK_SIZE=$STACK_SIZE_32
> JVM_MAX_HEAP=$JVM_HEAP_LIMIT_32
>   else
> STACK_SIZE=$STACK_SIZE_64
> JVM_MAX_HEAP=$JVM_HEAP_LIMIT_64
>   fi
>
>
> Here's my reasoning:
>
> If I read this right then -Xmx will be bound above by 1/2 the hardware
> memory size. The set heap limit for that build was 1024M. It then
> follows that 1024M was less than 1/2 the hardware memory. Yet, it still
> failed to allocate required memory. So to answer your question: It
> looks like it was that the system wasn't able to allocate that much
> memory at the time.
>

Where do you get the "Before:" output from your initial mail:

Before:
checking flags for bootcycle boot jdk java command for big
workloads... -Xms64M -Xmx998M -XX:ThreadStackSize=768

with '-Xmx998M' if JVM_HEAP_LIMIT_32 was "1024" before your change ?

> Those builds run on systems I don't have access to, though. What's
> more, I don't have access to the build logs of that failed build any
> longer. From a successful s390 build with that patch I see:
>
> $ grep 'Memory limit' build.log
> * Memory limit:   5906 MB
>
> This seems suspiciously high for 32 bit (or 31 bit in this case). Maybe
> it gets the hardware memory size of the 64bit host system? We know this
> runs in mock chroots on s390x boxes.
>
> Thanks,
> Severin


Re: RFR(XS): 8201524: [AIX] Don't link libfontmanager against libawt_headless

2018-04-13 Thread Volker Simonis
Phil Race <philip.r...@oracle.com> schrieb am Fr. 13. Apr. 2018 um 19:21:

>
> I suppose this potentially helps the concurrency of the build ?
> I can't think of why this would be a problem now there is no
> compile-time linking
> involved and it seems Linux was already fine without this,
> but a jdk-submit would be prudent ..
>

I did start Solaris and AIX builds before I left the office. I can
certainly also submit a job to JDK-submit, but at least hs-submit wasn’t
working at all (i.e. didn’t return any results).


> -phil.
>
> On 04/13/2018 09:22 AM, Volker Simonis wrote:
> > Hi Erik,
> >
> > thanks for looking at the patch and good catch! You're right that the
> > dependency can now be removed. Here's the new webrev:
> >
> > http://cr.openjdk.java.net/~simonis/webrevs/2018/8201524.v1
> >
> > Regards,
> > Volker
> >
> > On Fri, Apr 13, 2018 at 6:00 PM, Erik Joelsson <erik.joels...@oracle.com>
> wrote:
> >> Hello Volker,
> >>
> >> The change looks good, but now that we no longer link against
> >> libawt_headless, we should also remove the make dependency a few lines
> down.
> >> (Should have been done already for Solaris.)
> >>
> >> /Erik
> >>
> >>
> >>
> >> On 2018-04-13 06:28, Volker Simonis wrote:
> >>> Hi,
> >>>
> >>> can I please have a review for this tiny AIX cleanup:
> >>>
> >>> http://cr.openjdk.java.net/~simonis/webrevs/2018/8201524/
> >>> https://bugs.openjdk.java.net/browse/JDK-8201524
> >>>
> >>> This is a follow up change of JDK-8196516 which discovered that on AIX
> >>> libfontmanager is always linked against libawt_headless at build time.
> >>> If we are running in a headfull environment, libfontmanager will
> >>> dynamically load libawt_xawt which is not good because libawt_headless
> >>> and libawt_xawt define some common symbols. If we're running in a
> >>> headless environment, libawt_headless may be loaded a second time (at
> >>> least on Linux/Solaris) which isn't good either.
> >>>
> >>> Both of these scenarios haven't caused any problems on AIX yet, but I
> >>> think it's good to cleanup the AIX implementation as well and don't
> >>> link libfontmanager against libawt_headless anymore. In order to
> >>> achieve this, we have to allow unresolved symbols during the linking
> >>> of libfontmanager. This can be easily achieved by adding the additions
> >>> linker flag "-Wl$(COMMA)-berok" through LDFLAGS_aix. This works fine
> >>> for AIX because options which come later on the command line take
> >>> precedence
> >>> over earlier ones.
> >>>
> >>> Thank you and best regards,
> >>> Volker
> >>
>
>


Re: RFR(XS): 8201524: [AIX] Don't link libfontmanager against libawt_headless

2018-04-13 Thread Volker Simonis
Hi Erik,

thanks for looking at the patch and good catch! You're right that the
dependency can now be removed. Here's the new webrev:

http://cr.openjdk.java.net/~simonis/webrevs/2018/8201524.v1

Regards,
Volker

On Fri, Apr 13, 2018 at 6:00 PM, Erik Joelsson <erik.joels...@oracle.com> wrote:
> Hello Volker,
>
> The change looks good, but now that we no longer link against
> libawt_headless, we should also remove the make dependency a few lines down.
> (Should have been done already for Solaris.)
>
> /Erik
>
>
>
> On 2018-04-13 06:28, Volker Simonis wrote:
>>
>> Hi,
>>
>> can I please have a review for this tiny AIX cleanup:
>>
>> http://cr.openjdk.java.net/~simonis/webrevs/2018/8201524/
>> https://bugs.openjdk.java.net/browse/JDK-8201524
>>
>> This is a follow up change of JDK-8196516 which discovered that on AIX
>> libfontmanager is always linked against libawt_headless at build time.
>> If we are running in a headfull environment, libfontmanager will
>> dynamically load libawt_xawt which is not good because libawt_headless
>> and libawt_xawt define some common symbols. If we're running in a
>> headless environment, libawt_headless may be loaded a second time (at
>> least on Linux/Solaris) which isn't good either.
>>
>> Both of these scenarios haven't caused any problems on AIX yet, but I
>> think it's good to cleanup the AIX implementation as well and don't
>> link libfontmanager against libawt_headless anymore. In order to
>> achieve this, we have to allow unresolved symbols during the linking
>> of libfontmanager. This can be easily achieved by adding the additions
>> linker flag "-Wl$(COMMA)-berok" through LDFLAGS_aix. This works fine
>> for AIX because options which come later on the command line take
>> precedence
>> over earlier ones.
>>
>> Thank you and best regards,
>> Volker
>
>


RFR(XS): 8201524: [AIX] Don't link libfontmanager against libawt_headless

2018-04-13 Thread Volker Simonis
Hi,

can I please have a review for this tiny AIX cleanup:

http://cr.openjdk.java.net/~simonis/webrevs/2018/8201524/
https://bugs.openjdk.java.net/browse/JDK-8201524

This is a follow up change of JDK-8196516 which discovered that on AIX
libfontmanager is always linked against libawt_headless at build time.
If we are running in a headfull environment, libfontmanager will
dynamically load libawt_xawt which is not good because libawt_headless
and libawt_xawt define some common symbols. If we're running in a
headless environment, libawt_headless may be loaded a second time (at
least on Linux/Solaris) which isn't good either.

Both of these scenarios haven't caused any problems on AIX yet, but I
think it's good to cleanup the AIX implementation as well and don't
link libfontmanager against libawt_headless anymore. In order to
achieve this, we have to allow unresolved symbols during the linking
of libfontmanager. This can be easily achieved by adding the additions
linker flag "-Wl$(COMMA)-berok" through LDFLAGS_aix. This works fine
for AIX because options which come later on the command line take
precedence
over earlier ones.

Thank you and best regards,
Volker


Re: RFR: JDK-8201483 Make it possible to disable JVM features

2018-04-13 Thread Volker Simonis
On Thu, Apr 12, 2018 at 11:30 PM, David Holmes  wrote:
> On 12/04/2018 11:33 PM, Magnus Ihse Bursie wrote:
>>
>> On 2018-04-12 14:15, David Holmes wrote:
>>>
>>> Hi Magnus,
>>>
>>> On 12/04/2018 9:39 PM, Magnus Ihse Bursie wrote:

 It is currently easy to add new JVM features to the JVM build, but it is
 not possible to remove features.

 With this change, features can be both added or removed from the default
 set. They are added using --with-jvm-features=f1,f2 and removed using
 --with-jvm-features=-f1,-f2. The syntax can be combined, so
 --with-jvm-features=dtrace,-nmt will enable dtrace but disable native 
 memory
 tracking.
>>>
>>>
>>> I need to point out that we have never tested disabling individual VM
>>> features likes this. They are either all on, or all off for the minimal VM!
>>> There may be implicit dependencies between features.
>>
>>
>> Well, I have. :-) However, I don't do that regularly, and changes might
>> very well have crept in. As always, if you build something non-standard that
>> is not regularly tested, you're on your own.
>
>
> Feels to me like you've taken away the safety-fence and are encouraging
> people to attempt these unsupported configurations. Whether that was your
> intent or not.

I think that would be great. If people would try out different
configurations AND fix them that would be a great code clean-up for
HotSpot. I've recently tried various "unsupported" configurations
(i.e. minimal plus some selected features) and found that the
resulting build failures are mostly artificial because the
corresponding features aren't correctly ifdefed.

>
>> In any case, the purpose of this is not so much to disable existing JVM
>> features (after all, no one has really been missing this functionality), as
>> to pave the way for the upcoming patch for including/excluding individual
>> GCs.
>
>
> Surely a GC selection flag would have sufficed.
>
> David
>
>
>> /Magnus
>>
>>>
>>> David
>>>
 I also included some additional code cleanup and fixes, such as printing
 the JVM feature set at the summary.

 Bug: https://bugs.openjdk.java.net/browse/JDK-8201483
 WebRev:
 http://cr.openjdk.java.net/~ihse/JDK-8201483-disable-JVM-features/webrev.01

>>
>


Re: RFR: 8196516: libfontmanager must be built with LDFLAGS allowing unresolved symbols

2018-04-13 Thread Volker Simonis
Hi Severin,

I'm currently looking at the AIX-side of this bug.

The problem I see with your solution is that it uses LDFLAGS (which is
generic) to filter out Linux specific linker flags. If we would extend
this to AIX, we would have to add yet another substitution for AIX
which filters out "-Wl,bernotok". This is not beautiful and gets
uglier with every new platform.

But as this change has already been pushed and because (fortunately)
on AIX options which come later on the command line take precedence
over earlier ones I can easily fix this with a specific LDFLAGS_aix
setting. I've opened "8201524: [AIX] Don't link libfontmanager against
libawt_headless" [1] for it and I'll send out a new RFR for it soon.

Regards,
Volker

[1] https://bugs.openjdk.java.net/browse/JDK-8201524


On Wed, Apr 11, 2018 at 10:17 AM, Severin Gehwolf  wrote:
> On Tue, 2018-04-10 at 14:51 -0700, Sergey Bylokhov wrote:
>> LIBS_aix := -lawt_headless,
>> I guess that AIX team should have a similar fix.
>
> Possibly. I have no way of testing it, though. So will leave it to AIX
> folk to have a look. My experience was that it isn't easily
> reproducible. Some observations:
>
>1. Run swing app such as SwingSet2 on a headfull system. Since
>   fontmanager will have a link dep on lawt_headless, and awt code
>   loads libawt_xawt (headfull) on a headfull system, both libraries
>   providing symbols needed by libfontmanager will be loaded. Then it
>   depends whether this is a problem on that particular system or not.
>   In my experience this worked on some systems and not on others.
>2. Solaris was build-time linking to libawt_headless causing bug
>   8194870. So build-time linking got removed with that bug. Not sure
>   why that bug is private :(
>
> Thanks,
> Severin
>
>> On 10/04/2018 09:34, Erik Joelsson wrote:
>> > Looks good. Thanks!
>> >
>> > /Erik
>> >
>> >
>> > On 2018-04-10 04:25, Severin Gehwolf wrote:
>> > > Hi Erik,
>> > >
>> > > On Mon, 2018-04-09 at 09:20 -0700, Erik Joelsson wrote:
>> > > > Hello Severin,
>> > > >
>> > > > I'm ok with this solution for now.
>> > >
>> > > Thanks for the review!
>> > >
>> > > > Could you please reduce the indentation on line 652. In the
>> > > > build system
>> > > > we like 4 spaces for continuation indent [1]
>> > >
>> > > Done. New webrev at:
>> > > http://cr.openjdk.java.net/~sgehwolf/webrevs/JDK-8196516/webrev.0
>> > > 2
>> > >
>> > > Could someone from awt-dev have a look at this too? Thanks!
>> > >
>> > > Cheers,
>> > > Severin
>> > >
>> > > > /Erik
>> > > >
>> > > > [1] http://openjdk.java.net/groups/build/doc/code-conventions.h
>> > > > tml
>> > > >
>> > > > On 2018-04-09 06:39, Severin Gehwolf wrote:
>> > > > > Hi,
>> > > > >
>> > > > > Could somebody please review this build fix for
>> > > > > libfontmanager.so. The
>> > > > > issue for us is that with some LDFLAGS the build breaks as
>> > > > > described in
>> > > > > bug JDK-8196218. However, we cannot link to a providing
>> > > > > library at
>> > > > > build-time since we don't know which one it should be:
>> > > > > libawt_headless
>> > > > > or libawt_xawt. That has to happen at runtime. The proposed
>> > > > > fix filters
>> > > > > out relevant linker flags when libfontmanager is being built.
>> > > > > More
>> > > > > details are in the bug.
>> > > > >
>> > > > > Bug: https://bugs.openjdk.java.net/browse/JDK-8196516
>> > > > > webrev:
>> > > > > http://cr.openjdk.java.net/~sgehwolf/webrevs/JDK-8196516/webr
>> > > > > ev.01/
>> > > > >
>> > > > > Testing: I've run this through submit[1] and got the
>> > > > > following results.
>> > > > > SwingSet2 works fine for me on F27. I'm currently running
>> > > > > some more
>> > > > > tests on RHEL 7.
>> > > > >
>> > > > > -
>> > > > > Mach5 mach5-one-sgehwolf-JDK-8196516-20180409-1036-17877:
>> > > > > Builds
>> > > > > PASSED. Testing FAILURE.
>> > > > >
>> > > > > 0 Failed Tests
>> > > > >
>> > > > > Mach5 Tasks Results Summary
>> > > > >
>> > > > > NA: 0
>> > > > > UNABLE_TO_RUN: 0
>> > > > > EXECUTED_WITH_FAILURE: 0
>> > > > > KILLED: 0
>> > > > > PASSED: 82
>> > > > > FAILED: 1
>> > > > > Test
>> > > > >
>> > > > > 1 Failed
>> > > > >
>> > > > > tier1-debug-jdk_open_test_hotspot_jtreg_tier1_compiler_2-
>> > > > > windows-x64-
>> > > > > debug-31 SetupFailedException in setup...profile run-test-
>> > > > > prebuilt' ,
>> > > > > return value: 10
>> > > > > 
>> > > > >
>> > > > > Not sure what this test failure means. Could somebody at
>> > > > > Oracle shed
>> > > > > some light on this?
>> > > > >
>> > > > > Thanks,
>> > > > > Severin
>>
>>


Re: Problem building openjdk using cygwin

2018-04-01 Thread Volker Simonis
Hi Alireza,

sorry if my answer (and especially the link which I provided) have been
unclear. You don’t have to create any junctions. You just have to enable
short (i.e. „8.3“ filenames) for the drive /directory where you have
installed Visual Studio. Afterwards you must copy “microsoft visual studio”
to a temporary directory and finally copy it back to “microsoft visual
studio”. This should create a short name for that directpry.

You can find more information about how to enable/disable short filenames
here:

https://docs.microsoft.com/en-us/previous-versions/windows/it-pro/windows-server-2012-R2-and-2012/ff621566(v=ws.11)

https://support.microsoft.com/en-us/help/121007/how-to-disable-8-3-file-name-creation-on-ntfs-partitions

Regards,
Volker


Alireza Mohamadi <dange...@gmail.com> schrieb am Sa. 31. März 2018 um 09:50:

> Hi Volker, gratitude for answering my question.
> After reading Q link you provided I realized how to make junctions but I
> don't know which source should be attached to the target?
> As you've mentioned `:/cygdrive/c/progra~2/microsoft: No such file or
> directory` means there are no links available for "C:\Program Files
> (x86)\Microsoft Visual Studio",
> so I tried
> ```
> C:\WINDOWS\system32>mklink /j "C:\progra~2\microsoft~7" "C:\Program Files
> (x86)\Microsoft Visual Studio"
> Junction created for C:\progra~2\microsoft~7 <<===>> C:\Program Files
> (x86)\Microsoft Visual Studio
> ```
> but it seems not working because I get the same error. Can you give some
> tips please? Sorry if I'm newbie, I'm just interested in some JEPs and want
> to test them in a project to see if I can deploy it later.
> Many thanks.
>
> On Thu, Mar 29, 2018 at 11:55 AM, Volker Simonis <volker.simo...@gmail.com
> > wrote:
>
>> Hi Alireza,
>>
>> it seems you don’t have short file names enabled for the “Program Files”
>> directory. Notice that you have short file names enabled for “c:\” (i.e.
>> you have “progra~2”).
>>
>> You have to first enable short file names for the “Program Files”
>> directory (see for example
>>
>> https://superuser.com/questions/681330/how-to-force-short-name-8dot3-generation
>> ).
>>
>> Afterwards you must copy “microsoft visual studio” to a temporary
>> directory and finally copy it back to “microsoft visual studio”. This
>> should create a short name for that directory.
>>
>> Regards,
>> Volker
>>
>> Alireza Mohamadi <dange...@gmail.com> schrieb am Do. 29. März 2018 um
>> 06:08:
>>
>>> Hi. I wanted to build OpenJDK in windows using cygwin, and I set VC
>>> compiler path, but due to whitespaces in the path I can't build, with the
>>> following error:
>>> ```
>>> configure: Rewriting CC to "/cygdrive/c/progra~2/microsoft visual
>>> studio/2017/community/vc/tools/msvc/14.13.26128/bin/hostx86/x64/cl"
>>> checking resolved symbolic links for CC... no symlink
>>> configure: The C compiler (located as /cygdrive/c/progra~2/microsoft
>>> visual
>>> studio/2017/community/vc/tools/msvc/14.13.26128/bin/hostx86/x64/cl) does
>>> not seem to be the required microsoft compiler.configure: error: A
>>> microsoft compiler is reconfigure: The result from running it was:
>>> "/home/SuNova/jdk/build/.configure-support/generated-configure.sh: line
>>> 35050: /cygdrive/c/progra~2/microsoft: No such file or directory"
>>> configure exiting with result code 1
>>> ```
>>> Well it's not in my hands to set compiler path while installing VS2017
>>> and
>>> it does this automatically, so what can I do in this case?
>>>
>>>
>>> --
>>> Sometimes the best results can be achieved in the worst conditions
>>> Wish you the bests :)
>>> Alireza
>>>
>>
>
>
> --
> Sometimes the best results can be achieved in the worst conditions
> Wish you the bests :)
> Alireza
>


Re: Problem building openjdk using cygwin

2018-03-29 Thread Volker Simonis
Hi Alireza,

it seems you don’t have short file names enabled for the “Program Files”
directory. Notice that you have short file names enabled for “c:\” (i.e.
you have “progra~2”).

You have to first enable short file names for the “Program Files” directory
(see for example
https://superuser.com/questions/681330/how-to-force-short-name-8dot3-generation
).

Afterwards you must copy “microsoft visual studio” to a temporary directory
and finally copy it back to “microsoft visual studio”. This should create a
short name for that directory.

Regards,
Volker

Alireza Mohamadi  schrieb am Do. 29. März 2018 um 06:08:

> Hi. I wanted to build OpenJDK in windows using cygwin, and I set VC
> compiler path, but due to whitespaces in the path I can't build, with the
> following error:
> ```
> configure: Rewriting CC to "/cygdrive/c/progra~2/microsoft visual
> studio/2017/community/vc/tools/msvc/14.13.26128/bin/hostx86/x64/cl"
> checking resolved symbolic links for CC... no symlink
> configure: The C compiler (located as /cygdrive/c/progra~2/microsoft visual
> studio/2017/community/vc/tools/msvc/14.13.26128/bin/hostx86/x64/cl) does
> not seem to be the required microsoft compiler.configure: error: A
> microsoft compiler is reconfigure: The result from running it was:
> "/home/SuNova/jdk/build/.configure-support/generated-configure.sh: line
> 35050: /cygdrive/c/progra~2/microsoft: No such file or directory"
> configure exiting with result code 1
> ```
> Well it's not in my hands to set compiler path while installing VS2017 and
> it does this automatically, so what can I do in this case?
>
>
> --
> Sometimes the best results can be achieved in the worst conditions
> Wish you the bests :)
> Alireza
>


Re: RFR: JDK-8200178 Remove mapfiles for JDK native libraries

2018-03-23 Thread Volker Simonis
Hi Magnus,

thanks for addressing this long standing issue! I haven't looked at
the changes, but just want to share some general and historical notes:

- Compiling with "-fvisibility=hidden" which hides all symbols expect
the ones explicitly exported with
"__attribute__((visibility("default")))" has been requested by SAP
back in 2007 even before we had OpenJDK (see "Use -fvisibility=hidden
for gcc compiles" https://bugs.openjdk.java.net/browse/JDK-6588413)
and finally pushed into the OpenJKD around 2010.
- "-fvisibility=hidden" gave us performance improvements of about 5%
(JBB2005) and 2% (JVM98) on Linux/IA64 and 1,5% (JBB2005) and 0,5%
(JVM98) on Linux/PPC64 because the compiler could use faster calls for
non exported symbols. This improvement was only very small on x86
tough.
- "-fvisibility=hidden"/"__attribute__((visibility("default")))"
applies BEFORE using the map files in the linking step (i.e. hidden
symbols can't be exported any more even if mentioned in the map file)
- because of the performance improvements we got by using
"-fvisibility=hidden" it was worth while using it even though we had
the mapfiles at the end of the process.

Then we had several mail threads (which you probably remember because
you were involved :) where we discussed to either remove the map files
completely or instead generate them automatically during the build:

http://mail.openjdk.java.net/pipermail/hotspot-dev/2014-February/thread.html#12412
http://mail.openjdk.java.net/pipermail/hotspot-dev/2014-February/thread.html#12628

The main arguments against removing the map files at that time were:

1. the danger to re-export all symbols of statically linked libraries
(notably libstdc++ at that time)
2. loosing exports of compiler generated symbols like vtables which
are required by the Serviceability Agent

Point 1 is not a problem today, because I don't think we do any static
linking any more. If we still do it under some circumstances, this
problem should be re-evaluated.

Point 2 is only relevant for HotSpot. But because of "8034065: GCC 4.3
and later doesn't export vtable symbols any more which seem to be
needed by SA" (https://bugs.openjdk.java.net/browse/JDK-8034065),
exporting such symbols trough a map files doesn't work any more
anyway. So this isn't a problem either.

So to cut a long story short - I think the time is ripe to get rid of
the map files. Thumbs up from me (meant as moral support, not as a
concrete review :)

Regards,
Volker

On Fri, Mar 23, 2018 at 5:05 PM, mandy chung  wrote:
> This is a very good change and no more mapfile to maintain!!
>
> Please do file JBS issues for the component teams to clean up their exports.
>
> Mandy
>
>
> On 3/23/18 7:30 AM, Erik Joelsson wrote:
>>
>> I have looked at the build changes and they look good.
>>
>> Will you file followups for each component team to look over their
>> exported symbols, at least for the libraries with $(EXPORT_ALL_SYMBOLS)? It
>> sure looks like there is some technical debt laying around here.
>>
>> /Erik
>>
>>
>> On 2018-03-23 06:56, Magnus Ihse Bursie wrote:
>>>
>>> With modern compilers, we can use compiler directives (such as
>>> _attribute__((visibility("default"))), or __declspec(dllexport)) to control
>>> symbol visibility, directly in the source code. This has historically not
>>> been present on all compilers, so we had to resort to using mapfiles (also
>>> known as linker scripts).
>>>
>>> This is no longer the case. Now all compilers we use support symbol
>>> visibility directives, in one form or another. We should start using this.
>>> Since this has been the only way to control symbol visibility on Windows,
>>> for most of the shared code, we already have proper JNIEXPORT decorations in
>>> place.
>>>
>>> If we fix the remaining platform-specific files to have proper JNIEXPORT
>>> tagging, then we can finally get rid of mapfiles.
>>>
>>> This fix removed mapfiles for all JDK libraries. It does not touch
>>> hotspot libraries nor JDK executables; they will have to wait for a future
>>> fix -- this was complex enough. This change will not have any impact on
>>> macosx, since we do not use mapfiles there, but instead export all symbols.
>>> (This is not a good idea, but I'll address that separately.) This change
>>> will also have a minimal impact on Windows. The only reason Windows is
>>> impacted at all, is that some changes needed by Solaris and Linux were
>>> simpler to fix for all platforms.
>>>
>>> I have strived for this change to have no impact on the actual generated
>>> code. Unfortunately, this was not possible to fully achieve. I do not
>>> believe that these changes will have any actual impact on the product,
>>> though. I will present the differences more in detail further down. Those
>>> who are not interested can probably skip that.
>>>
>>> The patch has passed tier1 testing and is currently running tier2 and
>>> tier3. Since the running code is more or less (see caveat below) unmodified,
>>> I 

Re: [XXS] 8199858 : solaris-x86_64 : unpack200 fails linking with SS12u4

2018-03-20 Thread Volker Simonis
Hi Matthias,

the change looks good. Can you please also update the copyright year
before pushing (no need for a new webrev).

Thanks,
Volker


On Tue, Mar 20, 2018 at 2:12 PM, Baesken, Matthias
 wrote:
> Hello , could you please review this small fix for Solaris x86_64 ?
>
> We were running into  build errors  for  unpack200   on Solaris x86_64  
> because of missing symbol  "environ"  .
> The patch adds  environ  to the mapfile  .
>
> ( seems this is a similar / same   problem  already  solved for Solaris Sparc 
>   with
>
> https://bugs.openjdk.java.net/browse/JDK-8172548
>
> JDK-8172548  :  unpack200 
> fails linking with new update of SS12u4)
>
>
>
> Bug :
>
> https://bugs.openjdk.java.net/browse/JDK-8199858
>
>
> Webrev :
>
>
> http://cr.openjdk.java.net/~mbaesken/webrevs/8199858/
>
>
>
> Thanks, Matthias


Friendly request: Update "Supported Build Platforms" Wiki page

2018-03-12 Thread Volker Simonis
Hi,

could somebody please be so kind and update the "Supported Build
Platforms" Wiki page [1] for OpenJDK 10 and possibly 11?

Thanks a lot,
Volker

[1] https://wiki.openjdk.java.net/display/Build/Supported+Build+Platforms


  1   2   3   4   5   >