Re: RFR: 8214796: Create a jlink plugin for stripping debug info symbols from native libraries

2019-02-07 Thread Erik Joelsson



On 2019-02-07 11:09, Severin Gehwolf wrote:

Hi Erik,

On Thu, 2019-02-07 at 09:39 -0800, Erik Joelsson wrote:

Hello Severin,

There is a macro for automatically finding all source dirs for a module.
So in Gensrc-jdk.jlink.gmk, I think it would be better expressed using
that macro, like this:

JLINK_RESOURCE_DIRS := $(wildcard $(addsuffix
/jdk/tools/jlink/resources, $(call FindModuleSrcSdirs, jdk.jlink)))

The above could/should even be inlined.

I've considered this. It seems, though, that FindModuleSrcDirs comes
from make/common/Modules.gmk which isn't included in
make/gensrc/Gensrc-jdk.jlink.gmk. Given that it has already caused
problems with multiple includes of Modules.gmk (JDK-8213736) I was
reluctant to include it here too. Without the new include the above
won't work.

The approach I've taken here seems to be the lesser evil. Thoughts?


I appreciate your concern, but JDK-8213736 was a problem in Main.gmk, 
which is part of where Modules.gmk gets bootstrapped. In any normal 
makefile (as in where stuff is just being built), the bootstrapping is 
done and including Modules.gmk is completely fine. Any 
-.gmk file certainly qualifies here.


/Erik


Thanks,
Severin


Otherwise build changes look ok.

/Erik

On 2019-02-07 09:09, Severin Gehwolf wrote:

Hi,

Could I please get reviews for this enhancement? It adds a debug
symbols stripping plug-in to jlink for Linux and Unix platforms. It's
the first platform specific jlink plugin and the approach taken for
keeping it contained is to use a plugin specific ResourceBundle.
Discussion for this happened in [1].

The test uses a native library which should never get debug symbols
stripped during the test library build. As such, tiny modifications
were needed to make/common/TestFilesCompilation.gmk. Hence, build-dev
being on the list for this RFR. The test currently only runs on Linux
and requires objcopy to be available. Otherwise the test is being
skipped.

Example usage of this plugin is described in the bug.

webrev: http://cr.openjdk.java.net/~sgehwolf/webrevs/JDK-8214796/04/webrev/
Bug: https://bugs.openjdk.java.net/browse/JDK-8214796

Testing: test/jdk/tools/jlink test/jdk/jdk/modules tests on Linux
x86_64 (with good and broken objcopy) and the newly added test. It's
currently running through jdk/submit too.

Thoughts?

Thanks,
Severin

[1] http://mail.openjdk.java.net/pipermail/jigsaw-dev/2019-January/014109.html



Re: RFR: 8214796: Create a jlink plugin for stripping debug info symbols from native libraries

2019-02-07 Thread Mandy Chung

Hi Severin,

Using the plugin specific ResourceBundle is good.  Thanks for making
the change.

I see that Alan's comment [1] on the plugin options and I assume
you will investigate the plugin options and bring back a proposal.
Did I miss the discussion on these options?

Option: 
--strip-native-debug-symbols=][:debuginfo-file-ext=][:include-debug-syms=true]>


Examples: --strip-native-debug-symbols=defaults

I suggest the above be simplified and drop the "=defaults".  Simply:
   --strip-native-debug-symbols

Examples: 
--strip-native-debug-symbols=options:objcopy-cmd=/usr/bin/objcopy:debuginfo-file-ext=dbg:include-debug-syms=true 



If include-debug-syms=true then it will run
  objcopy --only-keep-debug foo foo. to create debug symbols file
  objcopy --add-gnu-debuglink=foo.dbg foo

So what about simplifying the options to:

--strip-native-debug-symbols=keep-debug-symbols=dbg,objcopy-path=/usr/bin/objcopy

We could also drop the word "symbols":


--strip-native-debug=[keep-debug=,][objcopy-path=]

By default, `--strip-native-debug` will strip native debug symbols
and don't keep debug symbols.

keep-debug= creates the debug symbols file.
 specifies the file extension.  It would be
nice to use the default `debuginfo` extension and simply
accept `keep-debug` option.

`objcopy-cmd` may be okay too.  Others may have opinion.

I think we should agree on the plugin options first before
doing the code review.

W.r.t. the test:

> The test currently only runs on Linux
> and requires objcopy to be available. Otherwise the test is being
> skipped.

We can create a fake objcopy utility for testing purpose
such that the test will validate if the options are passed
properly to the test utility.

> webrev: 
http://cr.openjdk.java.net/~sgehwolf/webrevs/JDK-8214796/04/webrev/


I haven't reviewed the new files.  Just notice that very long
lines in the new files that you may want to fix the formatting
that will help further side-by-side review.

The --list-plugin output is very very long.  The existing plugins
didn't set a good example to keep the well formatted (I recorded
that they were cleaned up at one point to keep 80 chars width).

One other question: should this plugin be moved to linux/classes
rather than unix/classes given that that's the platform it
intends to support?

Mandy
[1] 
http://mail.openjdk.java.net/pipermail/jigsaw-dev/2019-January/014099.html




Re: RFR(13): JDK-8216263: Add historical data for JDK 12

2019-02-07 Thread Joe Darcy

Hi Jan,

Thanks for looking into this. The generated information is much smaller 
as you note, 1087 inserted lines vs 16328 inserted lines before.


From looking over data for several modules, the revised changes look 
plausible in terms of corresponding to known work in those areas.


Looks good!

Cheers,

-Joe

On 2/7/2019 9:21 AM, Jan Lahoda wrote:

Hello,

I've regenerated the patch using JDK 12b30, and also using the patch 
from:
https://mail.openjdk.java.net/pipermail/compiler-dev/2019-February/012938.html 



(which makes the historical record smaller).

The updated patch is here:
http://cr.openjdk.java.net/~jlahoda/8216263/webrev.04/

How does that look?

Thanks,
    Jan

On 18.1.2019 18:06, Jan Lahoda wrote:

Adding build-dev.

Jan

On 17.1.2019 19:53, Jan Lahoda wrote:

On 17.1.2019 17:58, Jonathan Gibbons wrote:

Re:

   36 #   The checkout must not have any local changes that could
interfere with the new data. In particular,
   37 #   there must be absolutely no changed, new or removed files
under the ${JDK_CHECKOUT}/make/data/symbols
   38 #   directory.

That seems like a simple check that could be incorporated into the
script itself.

But, it also seems reasonable to separate enhancements like that from
the specific issue here, to add historical data for 12.


Ok. Sent as:
https://mail.openjdk.java.net/pipermail/compiler-dev/2019-January/012879.html 





And limited this patch to the historical data only:
http://cr.openjdk.java.net/~jlahoda/8216263/webrev.03/

Thanks,
    Jan



-- Jon

On 1/17/19 8:51 AM, Jan Lahoda wrote:

Hi Joe,

On 17.1.2019 02:13, Joseph D. Darcy wrote:

Hi Jan,

On 1/8/2019 2:13 AM, Jan Lahoda wrote:

Hi Joe,

Thanks for the comments, some responses inline. Updated webrev:
http://cr.openjdk.java.net/~jlahoda/8216263/webrev.01/

On 7.1.2019 23:51, Joseph D. Darcy wrote:

Hi Jan,

On 1/7/2019 4:48 AM, Jan Lahoda wrote:

Hi,

To make --release 12 work, we need to add historical data for JDK
12.
The current historical data is based on:
$ javac  -fullversion
javac full version "12-ea+26"

We may need to update the data once JDK 12 is out to cover any
changes
to the APIs that might happen.

The patch also adds a simple (shell) script to generate the
data, so
all that was needed to generate the data was:
$ cd make/data/symbols
$ sh generate-data 

JBS: https://bugs.openjdk.java.net/browse/JDK-8216263
Webrev: http://cr.openjdk.java.net/~jlahoda/8216263/webrev.00/

How does this look?


If the overhead is small enough and we don't mind a few updates to
the
JDK N+1 data, than perhaps the generation of this information
could be
included as part of the start of JDK N+1 activities.


That would be perfect!



I suggest a more extensive description of how to
make/data/symbols/generate-data. For example, this builds the data
for
the current release independent of whether or not the current
release
has already had its data generated, correct?


Yes.



Concretely, if JDK N+1 has already forked, what is the proper way
to get
data for JDK N? Likewise, how should a re-build of the data for 
JDK

N be
done?


What exactly should be there? To the existing description:
# to generate (add) data for JDK N, do:
# cd make/data/symbols
# sh generate-data 

I've added (in .01):
# to regenerate the data for JDK N, run the above commands again

What more should be there?



I'd appreciate somewhere to have a textual discussion of how
make/data/symbols/symbols should be updated, the A = 10, B = 11, 
etc.
convention, what else need to be done to generate the contents of 
this

changeset.

The  is the contents of the build directory?

In general, a description of how the data of JDK N should be 
added to

JDK (N+1).


I've added more textual description to the script, an updated webrev
is here:
http://cr.openjdk.java.net/~jlahoda/8216263/webrev.02/

Jan



Thanks,

-Joe




Re: RFR: 8214796: Create a jlink plugin for stripping debug info symbols from native libraries

2019-02-07 Thread Severin Gehwolf
Hi Erik,

On Thu, 2019-02-07 at 09:39 -0800, Erik Joelsson wrote:
> Hello Severin,
> 
> There is a macro for automatically finding all source dirs for a module. 
> So in Gensrc-jdk.jlink.gmk, I think it would be better expressed using 
> that macro, like this:
> 
> JLINK_RESOURCE_DIRS := $(wildcard $(addsuffix 
> /jdk/tools/jlink/resources, $(call FindModuleSrcSdirs, jdk.jlink)))
> 
> The above could/should even be inlined.

I've considered this. It seems, though, that FindModuleSrcDirs comes
from make/common/Modules.gmk which isn't included in
make/gensrc/Gensrc-jdk.jlink.gmk. Given that it has already caused
problems with multiple includes of Modules.gmk (JDK-8213736) I was
reluctant to include it here too. Without the new include the above
won't work.

The approach I've taken here seems to be the lesser evil. Thoughts?

Thanks,
Severin

> Otherwise build changes look ok.
> 
> /Erik
> 
> On 2019-02-07 09:09, Severin Gehwolf wrote:
> > Hi,
> > 
> > Could I please get reviews for this enhancement? It adds a debug
> > symbols stripping plug-in to jlink for Linux and Unix platforms. It's
> > the first platform specific jlink plugin and the approach taken for
> > keeping it contained is to use a plugin specific ResourceBundle.
> > Discussion for this happened in [1].
> > 
> > The test uses a native library which should never get debug symbols
> > stripped during the test library build. As such, tiny modifications
> > were needed to make/common/TestFilesCompilation.gmk. Hence, build-dev
> > being on the list for this RFR. The test currently only runs on Linux
> > and requires objcopy to be available. Otherwise the test is being
> > skipped.
> > 
> > Example usage of this plugin is described in the bug.
> > 
> > webrev: http://cr.openjdk.java.net/~sgehwolf/webrevs/JDK-8214796/04/webrev/
> > Bug: https://bugs.openjdk.java.net/browse/JDK-8214796
> > 
> > Testing: test/jdk/tools/jlink test/jdk/jdk/modules tests on Linux
> > x86_64 (with good and broken objcopy) and the newly added test. It's
> > currently running through jdk/submit too.
> > 
> > Thoughts?
> > 
> > Thanks,
> > Severin
> > 
> > [1] 
> > http://mail.openjdk.java.net/pipermail/jigsaw-dev/2019-January/014109.html
> > 



Re: RFR: JDK-8218630: CreateSymbols includes class and module headers unnecessarily.

2019-02-07 Thread Joe Darcy

PS I see that data is already out for review on a different thread:

https://mail.openjdk.java.net/pipermail/compiler-dev/2019-February/012939.html
https://mail.openjdk.java.net/pipermail/build-dev/2019-February/024936.html

Thanks,

-Joe

On 2/7/2019 11:04 AM, Joe Darcy wrote:

Hi Jan,

Is the revised historical data also available for review?

Thanks,

-Joe

On 2/7/2019 9:15 AM, Jan Lahoda wrote:

Hi,

Joe noted that the proposed historical data for JDK 12:
http://cr.openjdk.java.net/~jlahoda/8216263/webrev.00/
may contain unnecessary items, and it indeed is the case.

The main problems appear to be:
-when the class headers are being compared, the nestMembers for the 
current version may be null, and the nestMembers for the previous 
version may be an empty list. So, currently, such headers won't match 
and the header will be generated again to the historical record. 
listMatch can be used to consider null and empty list to be the same


-module header descriptions may refer to 
RequiresDescription/ProvidesDescription, which are missing 
hashCode/equals, which means that headers that have either requires 
or provides won't match any of the existing headers. The fix is to 
add hashCode and equals


I generated the ct.sym for the original JDK 12 historical data from
http://cr.openjdk.java.net/~jlahoda/8216263/webrev.00/

and the historical data generated by this patch, and the resulting 
content appears to be equivalent.


Proposed webrev: http://cr.openjdk.java.net/~jlahoda/8218630/webrev.00/
Bug: https://bugs.openjdk.java.net/browse/JDK-8218630

How does this look?

Thanks,
    Jan


Re: compiling openJdk 11 on windows 7 32bits fail

2019-02-07 Thread Franco Gastón Pellegrini
I tried compiling JDK 12 for 32 bits, and I get similar errors:

=== Output from failing command(s) repeated here ===
* For target hotspot_variant-client_libjvm_objs_classFileParser.obj:
classFileParser.cpp
c:/cygwin64/home/franc/java/jdk12/src/hotspot/share/classfile/classFileParser.cpp(312):
error C2220: warning treated as error - no 'object' file generated
c:/cygwin64/home/franc/java/jdk12/src/hotspot/share/classfile/classFileParser.cpp(312):
warning C4267: '=': conversion from 'size_t' to 'u2', possible loss of data
   ... (rest of output omitted)

* All command lines available in
/cygdrive/c/cygwin64/home/franc/java/jdk12/build/windows-x86-client-fastdebug/make-support/failure-logs.
=== End of repeated output ===

No indication of failed target found.
Hint: Try searching the build log for '] Error'.
Hint: See doc/building.html#troubleshooting for assistance.

make[1]: *** [/home/franc/java/jdk12/make/Init.gmk:310: main] Error 2
make: *** [/home/franc/java/jdk12/make/Init.gmk:186: default] Error 2


El mié., 6 de feb. de 2019 a la(s) 19:23, Franco Gastón Pellegrini (
francogpellegr...@gmail.com) escribió:

> I just tried  --disable-warnings-as-errors, and JDK 11 64bits as a
> bootjdk, but I get a lots of errors, and it refuse to build, like this:
>
> c:/cygwin64/home/franc/java/jdk11/src/hotspot/share/classfile/classFileParser.cpp(310):
> warning C4267: '=': conversion from 'size_t' to 'u2', possible loss of data
> c:/cygwin64/home/franc/java/jdk11/src/hotspot/share/code/codeBlob.cpp(229):
> error C2956: sized deallocation function 'operator delete(void*, size_t)'
> would be chosen as placement deallocation function.
> predefined C++ types (compiler internal)(44): note: see declaration of
> 'operator delete'
> c:/cygwin64/home/franc/java/jdk11/src/hotspot/share/code/codeBlob.cpp(250):
> error C2956: sized deallocation function 'operator delete(void*, size_t)'
> would be chosen as placement deallocation function.
> predefined C++ types (compiler internal)(44): note: see declaration of
> 'operator delete'
> c:/cygwin64/home/franc/java/jdk11/src/hotspot/share/code/codeBlob.cpp(289):
> error C2956: sized deallocation function 'operator delete(void*, size_t)'
> would be chosen as placement deallocation function.
> predefined C++ types (compiler internal)(44): note: see declaration of
> 'operator delete'
> c:/cygwin64/home/franc/java/jdk11/src/hotspot/share/code/codeBlob.cpp(312):
> error C2956: sized deallocation function 'operator delete(void*, size_t)'
> would be chosen as placement deallocation function.
> predefined C++ types (compiler internal)(44): note: see declaration of
> 'operator delete'
> c:/cygwin64/home/franc/java/jdk11/src/hotspot/share/code/codeBlob.cpp(333):
> error C2956: sized deallocation function 'operator delete(void*, size_t)'
> would be chosen as placement deallocation function.
> predefined C++ types (compiler internal)(44): note: see declaration of
> 'operator delete'
> c:/cygwin64/home/franc/java/jdk11/src/hotspot/share/code/codeBlob.cpp(372):
> error C2956: sized deallocation function 'operator delete(void*, size_t)'
> would be chosen as placement deallocation function.
> predefined C++ types (compiler internal)(44): note: see declaration of
> 'operator delete'
> c:/cygwin64/home/franc/java/jdk11/src/hotspot/share/code/codeBlob.cpp(437):
> error C2956: sized deallocation function 'operator delete(void*, size_t)'
> would be chosen as placement deallocation function.
> predefined C++ types (compiler internal)(44): note: see declaration of
> 'operator delete'
> c:/cygwin64/home/franc/java/jdk11/src/hotspot/share/code/codeBlob.cpp(541):
> error C2956: sized deallocation function 'operator delete(void*, size_t)'
> would be chosen as placement deallocation function.
> predefined C++ types (compiler internal)(44): note: see declaration of
> 'operator delete'
>
>
>
> El mié., 6 de feb. de 2019 a la(s) 16:15, Erik Joelsson (
> erik.joels...@oracle.com) escribió:
>
>> If you are running into warnings treated as errors, please try
>> configuring with --disable-warnings-as-errors. That is often needed on
>> less tested platforms.
>>
>> Note that as long as you run the build on a 64 bit system, you can use a
>> 64 bit bootjdk to build a 32 bit JDK.
>>
>> /Erik
>>
>> On 2019-02-06 11:07, Franco Gastón Pellegrini wrote:
>> > Can this be fixed before JDK 12? I will not able to compile java 12
>> 32bits
>> > because I will not have java 11 32bits as a boot-jdk.
>> >
>> > El sáb., 24 de nov. de 2018 a la(s) 03:59, David Holmes (
>> > david.hol...@oracle.com) escribió:
>> >
>> >> On 23/11/2018 7:10 pm, Magnus Ihse Bursie wrote:
>> >>> On 2018-11-23 08:35, Franco Gastón Pellegrini wrote:
>>  Using the same command as before, and then using
>>  make CONF=windows-x86-normal-client-fastdebug clean;
>>  make CONF=windows-x86-normal-client-fastdebug;
>> 
>>  I get warnings as error, and cannot compile. The output is (and I
>>  attached the logs):
>> 
>>  $ make 

Re: RFR: JDK-8218630: CreateSymbols includes class and module headers unnecessarily.

2019-02-07 Thread Joe Darcy

Hi Jan,

Is the revised historical data also available for review?

Thanks,

-Joe

On 2/7/2019 9:15 AM, Jan Lahoda wrote:

Hi,

Joe noted that the proposed historical data for JDK 12:
http://cr.openjdk.java.net/~jlahoda/8216263/webrev.00/
may contain unnecessary items, and it indeed is the case.

The main problems appear to be:
-when the class headers are being compared, the nestMembers for the 
current version may be null, and the nestMembers for the previous 
version may be an empty list. So, currently, such headers won't match 
and the header will be generated again to the historical record. 
listMatch can be used to consider null and empty list to be the same


-module header descriptions may refer to 
RequiresDescription/ProvidesDescription, which are missing 
hashCode/equals, which means that headers that have either requires or 
provides won't match any of the existing headers. The fix is to add 
hashCode and equals


I generated the ct.sym for the original JDK 12 historical data from
http://cr.openjdk.java.net/~jlahoda/8216263/webrev.00/

and the historical data generated by this patch, and the resulting 
content appears to be equivalent.


Proposed webrev: http://cr.openjdk.java.net/~jlahoda/8218630/webrev.00/
Bug: https://bugs.openjdk.java.net/browse/JDK-8218630

How does this look?

Thanks,
    Jan


Re: RFR(13): JDK-8216263: Add historical data for JDK 12

2019-02-07 Thread Jonathan Gibbons

Looks good to me.

-- Jon

On 2/7/19 9:21 AM, Jan Lahoda wrote:

Hello,

I've regenerated the patch using JDK 12b30, and also using the patch 
from:
https://mail.openjdk.java.net/pipermail/compiler-dev/2019-February/012938.html 



(which makes the historical record smaller).

The updated patch is here:
http://cr.openjdk.java.net/~jlahoda/8216263/webrev.04/

How does that look?

Thanks,
    Jan

On 18.1.2019 18:06, Jan Lahoda wrote:

Adding build-dev.

Jan

On 17.1.2019 19:53, Jan Lahoda wrote:

On 17.1.2019 17:58, Jonathan Gibbons wrote:

Re:

   36 #   The checkout must not have any local changes that could
interfere with the new data. In particular,
   37 #   there must be absolutely no changed, new or removed files
under the ${JDK_CHECKOUT}/make/data/symbols
   38 #   directory.

That seems like a simple check that could be incorporated into the
script itself.

But, it also seems reasonable to separate enhancements like that from
the specific issue here, to add historical data for 12.


Ok. Sent as:
https://mail.openjdk.java.net/pipermail/compiler-dev/2019-January/012879.html 





And limited this patch to the historical data only:
http://cr.openjdk.java.net/~jlahoda/8216263/webrev.03/

Thanks,
    Jan



-- Jon

On 1/17/19 8:51 AM, Jan Lahoda wrote:

Hi Joe,

On 17.1.2019 02:13, Joseph D. Darcy wrote:

Hi Jan,

On 1/8/2019 2:13 AM, Jan Lahoda wrote:

Hi Joe,

Thanks for the comments, some responses inline. Updated webrev:
http://cr.openjdk.java.net/~jlahoda/8216263/webrev.01/

On 7.1.2019 23:51, Joseph D. Darcy wrote:

Hi Jan,

On 1/7/2019 4:48 AM, Jan Lahoda wrote:

Hi,

To make --release 12 work, we need to add historical data for JDK
12.
The current historical data is based on:
$ javac  -fullversion
javac full version "12-ea+26"

We may need to update the data once JDK 12 is out to cover any
changes
to the APIs that might happen.

The patch also adds a simple (shell) script to generate the
data, so
all that was needed to generate the data was:
$ cd make/data/symbols
$ sh generate-data 

JBS: https://bugs.openjdk.java.net/browse/JDK-8216263
Webrev: http://cr.openjdk.java.net/~jlahoda/8216263/webrev.00/

How does this look?


If the overhead is small enough and we don't mind a few updates to
the
JDK N+1 data, than perhaps the generation of this information
could be
included as part of the start of JDK N+1 activities.


That would be perfect!



I suggest a more extensive description of how to
make/data/symbols/generate-data. For example, this builds the data
for
the current release independent of whether or not the current
release
has already had its data generated, correct?


Yes.



Concretely, if JDK N+1 has already forked, what is the proper way
to get
data for JDK N? Likewise, how should a re-build of the data for 
JDK

N be
done?


What exactly should be there? To the existing description:
# to generate (add) data for JDK N, do:
# cd make/data/symbols
# sh generate-data 

I've added (in .01):
# to regenerate the data for JDK N, run the above commands again

What more should be there?



I'd appreciate somewhere to have a textual discussion of how
make/data/symbols/symbols should be updated, the A = 10, B = 11, 
etc.
convention, what else need to be done to generate the contents of 
this

changeset.

The  is the contents of the build directory?

In general, a description of how the data of JDK N should be 
added to

JDK (N+1).


I've added more textual description to the script, an updated webrev
is here:
http://cr.openjdk.java.net/~jlahoda/8216263/webrev.02/

Jan



Thanks,

-Joe




Re: RFR: JDK-8218630: CreateSymbols includes class and module headers unnecessarily.

2019-02-07 Thread Jonathan Gibbons

Looks good to me.

-- Jon

On 2/7/19 9:15 AM, Jan Lahoda wrote:

Hi,

Joe noted that the proposed historical data for JDK 12:
http://cr.openjdk.java.net/~jlahoda/8216263/webrev.00/
may contain unnecessary items, and it indeed is the case.

The main problems appear to be:
-when the class headers are being compared, the nestMembers for the 
current version may be null, and the nestMembers for the previous 
version may be an empty list. So, currently, such headers won't match 
and the header will be generated again to the historical record. 
listMatch can be used to consider null and empty list to be the same


-module header descriptions may refer to 
RequiresDescription/ProvidesDescription, which are missing 
hashCode/equals, which means that headers that have either requires or 
provides won't match any of the existing headers. The fix is to add 
hashCode and equals


I generated the ct.sym for the original JDK 12 historical data from
http://cr.openjdk.java.net/~jlahoda/8216263/webrev.00/

and the historical data generated by this patch, and the resulting 
content appears to be equivalent.


Proposed webrev: http://cr.openjdk.java.net/~jlahoda/8218630/webrev.00/
Bug: https://bugs.openjdk.java.net/browse/JDK-8218630

How does this look?

Thanks,
    Jan


Re: RFR: 8214796: Create a jlink plugin for stripping debug info symbols from native libraries

2019-02-07 Thread Erik Joelsson

Hello Severin,

There is a macro for automatically finding all source dirs for a module. 
So in Gensrc-jdk.jlink.gmk, I think it would be better expressed using 
that macro, like this:


JLINK_RESOURCE_DIRS := $(wildcard $(addsuffix 
/jdk/tools/jlink/resources, $(call FindModuleSrcSdirs, jdk.jlink)))


The above could/should even be inlined.

Otherwise build changes look ok.

/Erik

On 2019-02-07 09:09, Severin Gehwolf wrote:

Hi,

Could I please get reviews for this enhancement? It adds a debug
symbols stripping plug-in to jlink for Linux and Unix platforms. It's
the first platform specific jlink plugin and the approach taken for
keeping it contained is to use a plugin specific ResourceBundle.
Discussion for this happened in [1].

The test uses a native library which should never get debug symbols
stripped during the test library build. As such, tiny modifications
were needed to make/common/TestFilesCompilation.gmk. Hence, build-dev
being on the list for this RFR. The test currently only runs on Linux
and requires objcopy to be available. Otherwise the test is being
skipped.

Example usage of this plugin is described in the bug.

webrev: http://cr.openjdk.java.net/~sgehwolf/webrevs/JDK-8214796/04/webrev/
Bug: https://bugs.openjdk.java.net/browse/JDK-8214796

Testing: test/jdk/tools/jlink test/jdk/jdk/modules tests on Linux
x86_64 (with good and broken objcopy) and the newly added test. It's
currently running through jdk/submit too.

Thoughts?

Thanks,
Severin

[1] http://mail.openjdk.java.net/pipermail/jigsaw-dev/2019-January/014109.html



Re: RFR(13): JDK-8216263: Add historical data for JDK 12

2019-02-07 Thread Jan Lahoda

Hello,

I've regenerated the patch using JDK 12b30, and also using the patch from:
https://mail.openjdk.java.net/pipermail/compiler-dev/2019-February/012938.html

(which makes the historical record smaller).

The updated patch is here:
http://cr.openjdk.java.net/~jlahoda/8216263/webrev.04/

How does that look?

Thanks,
Jan

On 18.1.2019 18:06, Jan Lahoda wrote:

Adding build-dev.

Jan

On 17.1.2019 19:53, Jan Lahoda wrote:

On 17.1.2019 17:58, Jonathan Gibbons wrote:

Re:

   36 #   The checkout must not have any local changes that could
interfere with the new data. In particular,
   37 #   there must be absolutely no changed, new or removed files
under the ${JDK_CHECKOUT}/make/data/symbols
   38 #   directory.

That seems like a simple check that could be incorporated into the
script itself.

But, it also seems reasonable to separate enhancements like that from
the specific issue here, to add historical data for 12.


Ok. Sent as:
https://mail.openjdk.java.net/pipermail/compiler-dev/2019-January/012879.html



And limited this patch to the historical data only:
http://cr.openjdk.java.net/~jlahoda/8216263/webrev.03/

Thanks,
Jan



-- Jon

On 1/17/19 8:51 AM, Jan Lahoda wrote:

Hi Joe,

On 17.1.2019 02:13, Joseph D. Darcy wrote:

Hi Jan,

On 1/8/2019 2:13 AM, Jan Lahoda wrote:

Hi Joe,

Thanks for the comments, some responses inline. Updated webrev:
http://cr.openjdk.java.net/~jlahoda/8216263/webrev.01/

On 7.1.2019 23:51, Joseph D. Darcy wrote:

Hi Jan,

On 1/7/2019 4:48 AM, Jan Lahoda wrote:

Hi,

To make --release 12 work, we need to add historical data for JDK
12.
The current historical data is based on:
$ javac  -fullversion
javac full version "12-ea+26"

We may need to update the data once JDK 12 is out to cover any
changes
to the APIs that might happen.

The patch also adds a simple (shell) script to generate the
data, so
all that was needed to generate the data was:
$ cd make/data/symbols
$ sh generate-data 

JBS: https://bugs.openjdk.java.net/browse/JDK-8216263
Webrev: http://cr.openjdk.java.net/~jlahoda/8216263/webrev.00/

How does this look?


If the overhead is small enough and we don't mind a few updates to
the
JDK N+1 data, than perhaps the generation of this information
could be
included as part of the start of JDK N+1 activities.


That would be perfect!



I suggest a more extensive description of how to
make/data/symbols/generate-data. For example, this builds the data
for
the current release independent of whether or not the current
release
has already had its data generated, correct?


Yes.



Concretely, if JDK N+1 has already forked, what is the proper way
to get
data for JDK N? Likewise, how should a re-build of the data for JDK
N be
done?


What exactly should be there? To the existing description:
# to generate (add) data for JDK N, do:
# cd make/data/symbols
# sh generate-data 

I've added (in .01):
# to regenerate the data for JDK N, run the above commands again

What more should be there?



I'd appreciate somewhere to have a textual discussion of how
make/data/symbols/symbols should be updated, the A = 10, B = 11, etc.
convention, what else need to be done to generate the contents of this
changeset.

The  is the contents of the build directory?

In general, a description of how the data of JDK N should be added to
JDK (N+1).


I've added more textual description to the script, an updated webrev
is here:
http://cr.openjdk.java.net/~jlahoda/8216263/webrev.02/

Jan



Thanks,

-Joe




RFR: JDK-8218630: CreateSymbols includes class and module headers unnecessarily.

2019-02-07 Thread Jan Lahoda

Hi,

Joe noted that the proposed historical data for JDK 12:
http://cr.openjdk.java.net/~jlahoda/8216263/webrev.00/
may contain unnecessary items, and it indeed is the case.

The main problems appear to be:
-when the class headers are being compared, the nestMembers for the 
current version may be null, and the nestMembers for the previous 
version may be an empty list. So, currently, such headers won't match 
and the header will be generated again to the historical record. 
listMatch can be used to consider null and empty list to be the same


-module header descriptions may refer to 
RequiresDescription/ProvidesDescription, which are missing 
hashCode/equals, which means that headers that have either requires or 
provides won't match any of the existing headers. The fix is to add 
hashCode and equals


I generated the ct.sym for the original JDK 12 historical data from
http://cr.openjdk.java.net/~jlahoda/8216263/webrev.00/

and the historical data generated by this patch, and the resulting 
content appears to be equivalent.


Proposed webrev: http://cr.openjdk.java.net/~jlahoda/8218630/webrev.00/
Bug: https://bugs.openjdk.java.net/browse/JDK-8218630

How does this look?

Thanks,
Jan


RFR: 8214796: Create a jlink plugin for stripping debug info symbols from native libraries

2019-02-07 Thread Severin Gehwolf
Hi,

Could I please get reviews for this enhancement? It adds a debug
symbols stripping plug-in to jlink for Linux and Unix platforms. It's
the first platform specific jlink plugin and the approach taken for
keeping it contained is to use a plugin specific ResourceBundle.
Discussion for this happened in [1].

The test uses a native library which should never get debug symbols
stripped during the test library build. As such, tiny modifications
were needed to make/common/TestFilesCompilation.gmk. Hence, build-dev
being on the list for this RFR. The test currently only runs on Linux
and requires objcopy to be available. Otherwise the test is being
skipped.

Example usage of this plugin is described in the bug.

webrev: http://cr.openjdk.java.net/~sgehwolf/webrevs/JDK-8214796/04/webrev/
Bug: https://bugs.openjdk.java.net/browse/JDK-8214796

Testing: test/jdk/tools/jlink test/jdk/jdk/modules tests on Linux
x86_64 (with good and broken objcopy) and the newly added test. It's
currently running through jdk/submit too.

Thoughts?

Thanks,
Severin

[1] http://mail.openjdk.java.net/pipermail/jigsaw-dev/2019-January/014109.html



Re: RFR [XS] : 8218562: handle HOTSPOT_BUILD_COMPILER for clang/xlclang and cleanup HOTSPOT_BUILD_COMPILER settings

2019-02-07 Thread David Holmes

On 7/02/2019 9:38 pm, Magnus Ihse Bursie wrote:

On 2019-02-07 10:36, David Holmes wrote:

Hi Matthias,

On 7/02/2019 1:17 am, Baesken, Matthias wrote:

Hello, please review this small change .

I noticed (when looking into AIX xlc16 / xlclang++)  the 
following:    in the case that  clang is used for compilation, 
HOTSPOT_BUILD_COMPILER   claims to be a gcc .

This is a bit misleading.
This change  adjusts the setting for clang usage on  AIX  (for future 
usage with xlclang++)   and macOSX.


Additionally I removed some  old  HOTSPOT_BUILD_COMPILER   for legacy 
Oracle / Sun Studio versions ).


Possibly more could be removed :)

Changes seem fine.

I've always been irked by the fact we can access an actual version 
string from the compilers themselves, so we wouldn't need this code.

How do you do that?


Sorry typo s/we can access/we can't access/

I'm having this kind of background low-intensity itch to fix the 
duplication between hotspot and build system in this (and related system 
detection code). It never rises to enough annoyance to actually get me 
to do anything about it, though.


I guess configure could generate a version string for hotspot to use, 
but it's probably not the brief version string we prefer to have.


David


/Magnus




Thanks,
David



Bug/webrev :

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

http://cr.openjdk.java.net/~mbaesken/webrevs/8218562.0/


Thanks, Matthias





Re: RFR [XS] : 8218562: handle HOTSPOT_BUILD_COMPILER for clang/xlclang and cleanup HOTSPOT_BUILD_COMPILER settings

2019-02-07 Thread Magnus Ihse Bursie




On 2019-02-07 10:36, David Holmes wrote:

Hi Matthias,

On 7/02/2019 1:17 am, Baesken, Matthias wrote:

Hello, please review this small change .

I noticed (when looking into AIX xlc16 / xlclang++)  the 
following:    in the case that  clang is used for compilation,    
HOTSPOT_BUILD_COMPILER   claims to be a gcc .

This is a bit misleading.
This change  adjusts the setting for clang usage on  AIX  (for future 
usage with xlclang++)   and macOSX.


Additionally I removed some  old  HOTSPOT_BUILD_COMPILER   for legacy 
Oracle / Sun Studio versions ).


Possibly more could be removed :)

Changes seem fine.

I've always been irked by the fact we can access an actual version 
string from the compilers themselves, so we wouldn't need this code.

How do you do that?

I'm having this kind of background low-intensity itch to fix the 
duplication between hotspot and build system in this (and related system 
detection code). It never rises to enough annoyance to actually get me 
to do anything about it, though.


/Magnus




Thanks,
David



Bug/webrev :

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

http://cr.openjdk.java.net/~mbaesken/webrevs/8218562.0/


Thanks, Matthias





Re: RFR [XS] : 8218562: handle HOTSPOT_BUILD_COMPILER for clang/xlclang and cleanup HOTSPOT_BUILD_COMPILER settings

2019-02-07 Thread David Holmes

Hi Matthias,

On 7/02/2019 1:17 am, Baesken, Matthias wrote:

Hello, please review this small change .

I noticed (when looking into AIX xlc16 / xlclang++)  the following:in the 
case that  clang is used for compilation,HOTSPOT_BUILD_COMPILER   claims to 
be a gcc .
This is a bit misleading.
This change  adjusts the setting for clang usage on  AIX  (for future usage 
with xlclang++)   and macOSX.

Additionally I removed some  old  HOTSPOT_BUILD_COMPILER   for  legacy Oracle / 
Sun Studio versions ).


Possibly more could be removed :)

Changes seem fine.

I've always been irked by the fact we can access an actual version 
string from the compilers themselves, so we wouldn't need this code.


Thanks,
David



Bug/webrev :

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

http://cr.openjdk.java.net/~mbaesken/webrevs/8218562.0/


Thanks, Matthias



RE: RFR [XS] : 8218562: handle HOTSPOT_BUILD_COMPILER for clang/xlclang and cleanup HOTSPOT_BUILD_COMPILER settings

2019-02-07 Thread Doerr, Martin
Hi Matthias,

looks good to me, too.

Best regards,
Martin


-Original Message-
From: hotspot-dev  On Behalf Of Baesken, 
Matthias
Sent: Donnerstag, 7. Februar 2019 10:13
To: Magnus Ihse Bursie ; 
'hotspot-...@openjdk.java.net' ; 
'build-dev@openjdk.java.net' 
Subject: RE: RFR [XS] : 8218562: handle HOTSPOT_BUILD_COMPILER for 
clang/xlclang and cleanup HOTSPOT_BUILD_COMPILER settings

Thanks .  
A second review would be great .

Best regards, Matthias

> -Original Message-
> From: Magnus Ihse Bursie 
> Sent: Mittwoch, 6. Februar 2019 17:38
> To: Baesken, Matthias ; 'hotspot-
> d...@openjdk.java.net' ; 'build-
> d...@openjdk.java.net' 
> Subject: Re: RFR [XS] : 8218562: handle HOTSPOT_BUILD_COMPILER for
> clang/xlclang and cleanup HOTSPOT_BUILD_COMPILER settings
> 
> On 2019-02-06 16:17, Baesken, Matthias wrote:
> > Hello, please review this small change .
> >
> > I noticed (when looking into AIX xlc16 / xlclang++)  the following:in 
> > the
> case that  clang is used for compilation,HOTSPOT_BUILD_COMPILER   claims
> to be a gcc .
> > This is a bit misleading.
> > This change  adjusts the setting for clang usage on  AIX  (for future usage
> with xlclang++)   and macOSX.
> >
> > Additionally I removed some  old  HOTSPOT_BUILD_COMPILER   for  legacy
> Oracle / Sun Studio versions ).
> >
> >
> > Bug/webrev :
> >
> > https://bugs.openjdk.java.net/browse/JDK-8218562
> >
> > http://cr.openjdk.java.net/~mbaesken/webrevs/8218562.0/
> Looks good to me.
> 
> /Magnus
> >
> >
> > Thanks, Matthias



RE: RFR [XS] : 8218562: handle HOTSPOT_BUILD_COMPILER for clang/xlclang and cleanup HOTSPOT_BUILD_COMPILER settings

2019-02-07 Thread Baesken, Matthias
Thanks .  
A second review would be great .

Best regards, Matthias

> -Original Message-
> From: Magnus Ihse Bursie 
> Sent: Mittwoch, 6. Februar 2019 17:38
> To: Baesken, Matthias ; 'hotspot-
> d...@openjdk.java.net' ; 'build-
> d...@openjdk.java.net' 
> Subject: Re: RFR [XS] : 8218562: handle HOTSPOT_BUILD_COMPILER for
> clang/xlclang and cleanup HOTSPOT_BUILD_COMPILER settings
> 
> On 2019-02-06 16:17, Baesken, Matthias wrote:
> > Hello, please review this small change .
> >
> > I noticed (when looking into AIX xlc16 / xlclang++)  the following:in 
> > the
> case that  clang is used for compilation,HOTSPOT_BUILD_COMPILER   claims
> to be a gcc .
> > This is a bit misleading.
> > This change  adjusts the setting for clang usage on  AIX  (for future usage
> with xlclang++)   and macOSX.
> >
> > Additionally I removed some  old  HOTSPOT_BUILD_COMPILER   for  legacy
> Oracle / Sun Studio versions ).
> >
> >
> > Bug/webrev :
> >
> > https://bugs.openjdk.java.net/browse/JDK-8218562
> >
> > http://cr.openjdk.java.net/~mbaesken/webrevs/8218562.0/
> Looks good to me.
> 
> /Magnus
> >
> >
> > Thanks, Matthias