RE: RFR: 8237192: Generate stripped/public pdbs on Windows for jdk images

[Baesken, Matthias]

Hello, here is  a slightly changed  new  webrev :

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

(adjusted $(JRE_STRIPPED_PDB_FILES)  in Bundles.gmk, that was in the wrong 
place )  .

> I think it needs to be a separate option as it's all orthogonal to the
> main debug symbols file creation.

Yes it is a separate option I agree that’s better .  One has to set  
--enable-stripped-pdbs=yes  .

Best regards , Matthias


> 
> On 2020-02-06 04:48, Magnus Ihse Bursie wrote:
> > On 2020-02-06 12:36, Langer, Christoph wrote:
> >> Hi,
> >>
> >> let me chime in to this discussion at this point. So, first of all, 
> >> Matthias,
> thanks for your work so far.
> >>
> >> Erik, I fully understand your points and I agree that it's probably a good
> idea to refactor this whole process of creating these different types of
> bundles.
> >>
> >> Our idea is to provide functionality in the build system to add "public" or
> stripped debug files to the delivery image of the JDK. This would provide
> better information in case of crashes and would hence allow for better
> supportability. That's something we'd probably want to enable in
> SapMachine binary distributions.
> I very much support the idea of using these stripped pdb files. It has
> been a long standing issue in hotspot on Windows to not have access to
> stacktraces.
> >> So, can we get to add a configure option like the one proposed by
> Matthias into the current code base?
> >> The option should be turned off by default. If it is switched on, the
> images/jdk folder in the build directory will have both, the *stripped.pdb
> files and the standard *.pdb files. However, having *stripped.pdb files
> around should not matter in terms of functionality (for developers and
> testing) as they'd not be used in the presence of the "real" pdb files anyway.
> The actual JDK bundle for delivery would then contain the *stripped.pdb
> files, renamed to *.pdb. And the debug symbols bundle would have the full
> *.pdb files only. Both could then be overlaid as needed.
> >>
> >> I think you raised two concerns.
> >> One is that you'd rather like to refactor bundling for several reasons. 
> >> But I
> guess, should you eventually get to your refactoring, it shouldn't be a
> problem to take the functionality of this new option along.
> >> The other was regarding JMODs. I believe it's correct, that JMODs have
> never carried external debug information. For platforms other than MS
> Windows that's actually not a big problem because debug information can be
> internalized. And jlink has gotten several additions to set flags for 
> stripping
> that data to the right level. I assume if jlinked images on Windows should
> ever be enabled to carry debug information, inclusion of external debug files
> would have to be added to JMODs and jlink. But that's definitely out of scope
> here.
> The argument "jmods have never carried external debug information" just
> doesn't apply here. Neither has the distribution bundles, for the exact
> same reason. You really should not compare these new stripped pdb files
> to the existing debug symbol files, they are different files with
> different purposes. One is meant to be shipped to customers, the other
> is not. You say you want to ship these new stripped pdb files with your
> distribution so that customers have them present when they use your
> distribution. If you then omit these new files from the jmods, any
> customer created jlinked image will not have these new stripped pdb
> files, which IMO is a very weird and unexpected behavior from a customer
> point of view. Jlinking new images is an integral and promoted way of
> using a JDK, so any mismatch between the original JDK distribution and
> what you are able to jlink is a serious discrepancy.
> >> So, what do you think? What speaks against adding this option (that is off
> by default)?
> 
> My main objective is that you introduce further discrepancies between
> the original distribution JDK image and what's possible to generate
> using jlink from that distribution JDK image. My second objective is
> that the already convoluted bundles creation logic becomes even more
> convoluted. I believe there is a better possible solution in the build
> but it will require a lot more work to figure out.
> 
> All that said, if you still wish to continue, I will stop standing in
> the way.
> 
> > While Erik will need to comment on this himself, from my POV this
> > looks okay. The conditions are:
> >
> > * This is controlled by a separate option (or perhaps even better as a
> > new argument to --with-native-debug-symbols), so without this, nothing
> > will change.
> >
> I think it needs to be a separate option as it's all orthogonal to the
> main debug symbols file creation.
> > * The code need to make sure to separate stripped.pdb and normal pdb
> > files, when enabled.
> >
> > * And there must not be a heavy penalty in added code complexity.
> >
> /Erik
> > /Magnus
> >
> >> Thanks
> >> Christoph
> >

RFR: JDK-8238534

[René Schünemann]

For the Apple notarization process, the whole bundle in its final form
has to be signed with the codesign tool.
See the discussion here: https://bugs.openjdk.java.net/browse/JDK-8238225

This change copies all JDK/JRE files to a temporary directory, which
is then passed to the codesign tool. The temporary directory is then
used as the base directory for the bundle archive and is getting
removed after the archive has been created. This only applies when a
valid code signing identity is set and the build type is release.

Bug: https://bugs.openjdk.java.net/browse/JDK-8238534
WebRev: 
http://cr.openjdk.java.net/~rschuenemann/wr/2020/8238534-macos_sign_bundles/01/

Rene

Re: RFR: 8237192: Generate stripped/public pdbs on Windows for jdk images

[Magnus Ihse Bursie]

On 2020-02-07 09:50, Baesken, Matthias wrote:

Hello, here is  a slightly changed  new  webrev :

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

In Bundles.gmk, this line:

$(ECHO) found stripped pdb file {f}, we rename it to: 
{f%stripped.pdb}pdb; \


It looks almost like left-over debug output. If you want to keep it, 
please rephrase to something more terse, that fits better with the 
existing style of build messages. Also, it should probably be on the 
LOG=info level, so add a $(LOG_INFO).


In NativeCompilation.gmk:
Why not just a simple,
  ifeq ($(ENABLE_STRIPPED_PDBS), true)
    $1_EXTRA_LDFLAGS += 
"-pdbstripped:$$($1_SYMBOLS_DIR)/$$($1_NOSUFFIX).stripped.pdb"

  endif
?
I see no reason to duplicate code.

In jdk-options.m4:

I'm not 100% sure about the name of the new option. 
--enable-stripped-pdb does not fully convey the fact that we do not 
strip the *existing* pdb:s, but instead also add a new type. Maybe 
--enable-bundle-stripped-pdb?


/Magnus


(adjusted $(JRE_STRIPPED_PDB_FILES)  in Bundles.gmk, that was in the wrong 
place )  .


I think it needs to be a separate option as it's all orthogonal to the
main debug symbols file creation.

Yes it is a separate option I agree that’s better .  One has to set  
--enable-stripped-pdbs=yes  .

Best regards , Matthias



On 2020-02-06 04:48, Magnus Ihse Bursie wrote:

On 2020-02-06 12:36, Langer, Christoph wrote:

Hi,

let me chime in to this discussion at this point. So, first of all, Matthias,

thanks for your work so far.

Erik, I fully understand your points and I agree that it's probably a good

idea to refactor this whole process of creating these different types of
bundles.

Our idea is to provide functionality in the build system to add "public" or

stripped debug files to the delivery image of the JDK. This would provide
better information in case of crashes and would hence allow for better
supportability. That's something we'd probably want to enable in
SapMachine binary distributions.
I very much support the idea of using these stripped pdb files. It has
been a long standing issue in hotspot on Windows to not have access to
stacktraces.

So, can we get to add a configure option like the one proposed by

Matthias into the current code base?

The option should be turned off by default. If it is switched on, the

images/jdk folder in the build directory will have both, the *stripped.pdb
files and the standard *.pdb files. However, having *stripped.pdb files
around should not matter in terms of functionality (for developers and
testing) as they'd not be used in the presence of the "real" pdb files anyway.
The actual JDK bundle for delivery would then contain the *stripped.pdb
files, renamed to *.pdb. And the debug symbols bundle would have the full
*.pdb files only. Both could then be overlaid as needed.

I think you raised two concerns.
One is that you'd rather like to refactor bundling for several reasons. But I

guess, should you eventually get to your refactoring, it shouldn't be a
problem to take the functionality of this new option along.

The other was regarding JMODs. I believe it's correct, that JMODs have

never carried external debug information. For platforms other than MS
Windows that's actually not a big problem because debug information can be
internalized. And jlink has gotten several additions to set flags for stripping
that data to the right level. I assume if jlinked images on Windows should
ever be enabled to carry debug information, inclusion of external debug files
would have to be added to JMODs and jlink. But that's definitely out of scope
here.
The argument "jmods have never carried external debug information" just
doesn't apply here. Neither has the distribution bundles, for the exact
same reason. You really should not compare these new stripped pdb files
to the existing debug symbol files, they are different files with
different purposes. One is meant to be shipped to customers, the other
is not. You say you want to ship these new stripped pdb files with your
distribution so that customers have them present when they use your
distribution. If you then omit these new files from the jmods, any
customer created jlinked image will not have these new stripped pdb
files, which IMO is a very weird and unexpected behavior from a customer
point of view. Jlinking new images is an integral and promoted way of
using a JDK, so any mismatch between the original JDK distribution and
what you are able to jlink is a serious discrepancy.

So, what do you think? What speaks against adding this option (that is off

by default)?

My main objective is that you introduce further discrepancies between
the original distribution JDK image and what's possible to generate
using jlink from that distribution JDK image. My second objective is
that the already convoluted bundles creation logic becomes even more
convoluted. I believe there is a better possible solution in the build
but it will require a lo

Re: RFR: JDK-8238534

[Erik Joelsson]

Hello René,

It's good to see an open solution to this, but I have some opinions on 
the patch.


The concept of building into "temp dirs" that are then removed is a 
practice we try to avoid in the build. Whenever possible, each rule 
should be a well defined transformation from a set of source files to a 
target file. There is just no reason to remove the jdk-signed dir here. 
If something goes wrong, you would want the dir around to investigate. 
This also keeps incremental builds working as expected. Your current 
patch will always rebuild the bundles, which is not ok.


I would recommend putting the jdk-signed dir in 
$(IMAGES_OUTPUTDIR)/jdk-signed and just leave it there. I would create a 
separate rule for the signing part, where the target file is the 
CodeResources file that codesign actually creates, and the prerequisite 
files simply $(COPY_SIGNED_JDK_BUNDLE).


Separate rules for creating a top level directory are not needed. The 
rules generated from SetupCopyFiles will create all directories needed.


I would also keep using the existing SetupBundleFile for the actual 
bundling, even if most of the functionality in it is not used, just to 
avoid more separate code paths than necessary.


/Erik

On 2020-02-07 02:05, René Schünemann wrote:

For the Apple notarization process, the whole bundle in its final form
has to be signed with the codesign tool.
See the discussion here: https://bugs.openjdk.java.net/browse/JDK-8238225

This change copies all JDK/JRE files to a temporary directory, which
is then passed to the codesign tool. The temporary directory is then
used as the base directory for the bundle archive and is getting
removed after the archive has been created. This only applies when a
valid code signing identity is set and the build type is release.

Bug: https://bugs.openjdk.java.net/browse/JDK-8238534
WebRev: 
http://cr.openjdk.java.net/~rschuenemann/wr/2020/8238534-macos_sign_bundles/01/

Rene

ARM build broken?

[Marc Hoffmann]

Dear JDK developers,

it looks like building http://hg.openjdk.java.net/jdk/jdk/ 
 in ARM is broken since yesterday. This 
commit seems to be the first one that broke the build:
changeset:   57948:4a4d185098e2
tag: tip
user:lfoltan
date:Thu Feb 06 14:29:57 2020 +
summary: 8230199: consolidate signature parsing code in HotSpot sources
this one now leads to another error (see below):
changeset:   57949:d3caf06ac9ae
tag: tip
user:lfoltan
date:Thu Feb 06 15:28:37 2020 +
summary: 8238600: Remove runtime/fieldType.hpp and fieldType.cpp

Is this a problem with the source tree or do we need to adjust the build setup 
on ARM?

Background info: I’m a developer of the JaCoCo project and we do CI builds on 
all JDKs to test them agains the JaCoCo integration tests.

Best regards,
-marc






ERROR: Build failed for target 'images' in configuration 
'linux-arm-server-release' (exit code 2) 
Stopping sjavac server

=== Output from failing command(s) repeated here ===
* For target hotspot_variant-server_libjvm_objs_interpreterRT_arm.o:
/workspace/src/hotspot/cpu/arm/interpreterRT_arm.cpp:54:27: error: 'int_parm' 
is not a member of 'SignatureIterator'
   (u1) SignatureIterator::int_parm, // bool
   ^~~~
/workspace/src/hotspot/cpu/arm/interpreterRT_arm.cpp:55:27: error: 'int_parm' 
is not a member of 'SignatureIterator'
   (u1) SignatureIterator::int_parm, // byte
   ^~~~
/workspace/src/hotspot/cpu/arm/interpreterRT_arm.cpp:56:27: error: 'int_parm' 
is not a member of 'SignatureIterator'
   (u1) SignatureIterator::int_parm, // char
   ^~~~
/workspace/src/hotspot/cpu/arm/interpreterRT_arm.cpp:57:27: error: 'int_parm' 
is not a member of 'SignatureIterator'
   (u1) SignatureIterator::int_parm, // short
   ^~~~
/workspace/src/hotspot/cpu/arm/interpreterRT_arm.cpp:58:27: error: 'int_parm' 
is not a member of 'SignatureIterator'
   (u1) SignatureIterator::int_parm, // int
   ^~~~
   ... (rest of output omitted)

Re: ARM build broken?

[Erik Joelsson]

Adding hotspot-dev since this is caused by a source change in hotspot 
and not really a build issue. We see it in our internal arm builds as well.


/Erik

On 2020-02-07 13:38, Marc Hoffmann wrote:

Dear JDK developers,

it looks like building http://hg.openjdk.java.net/jdk/jdk/ 
 in ARM is broken since yesterday. This 
commit seems to be the first one that broke the build:
changeset:   57948:4a4d185098e2
tag: tip
user:lfoltan
date:Thu Feb 06 14:29:57 2020 +
summary: 8230199: consolidate signature parsing code in HotSpot sources
this one now leads to another error (see below):
changeset:   57949:d3caf06ac9ae
tag: tip
user:lfoltan
date:Thu Feb 06 15:28:37 2020 +
summary: 8238600: Remove runtime/fieldType.hpp and fieldType.cpp

Is this a problem with the source tree or do we need to adjust the build setup 
on ARM?

Background info: I’m a developer of the JaCoCo project and we do CI builds on 
all JDKs to test them agains the JaCoCo integration tests.

Best regards,
-marc






ERROR: Build failed for target 'images' in configuration 
'linux-arm-server-release' (exit code 2)
Stopping sjavac server

=== Output from failing command(s) repeated here ===
* For target hotspot_variant-server_libjvm_objs_interpreterRT_arm.o:
/workspace/src/hotspot/cpu/arm/interpreterRT_arm.cpp:54:27: error: 'int_parm' 
is not a member of 'SignatureIterator'
(u1) SignatureIterator::int_parm, // bool
^~~~
/workspace/src/hotspot/cpu/arm/interpreterRT_arm.cpp:55:27: error: 'int_parm' 
is not a member of 'SignatureIterator'
(u1) SignatureIterator::int_parm, // byte
^~~~
/workspace/src/hotspot/cpu/arm/interpreterRT_arm.cpp:56:27: error: 'int_parm' 
is not a member of 'SignatureIterator'
(u1) SignatureIterator::int_parm, // char
^~~~
/workspace/src/hotspot/cpu/arm/interpreterRT_arm.cpp:57:27: error: 'int_parm' 
is not a member of 'SignatureIterator'
(u1) SignatureIterator::int_parm, // short
^~~~
/workspace/src/hotspot/cpu/arm/interpreterRT_arm.cpp:58:27: error: 'int_parm' 
is not a member of 'SignatureIterator'
(u1) SignatureIterator::int_parm, // int
^~~~
... (rest of output omitted)

Re: ARM build broken?

[David Holmes]

Hi,

It is a known breakage:

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

David

On 8/02/2020 7:49 am, Erik Joelsson wrote:
Adding hotspot-dev since this is caused by a source change in hotspot 
and not really a build issue. We see it in our internal arm builds as well.


/Erik

On 2020-02-07 13:38, Marc Hoffmann wrote:

Dear JDK developers,

it looks like building http://hg.openjdk.java.net/jdk/jdk/ 
 in ARM is broken since 
yesterday. This commit seems to be the first one that broke the build:

changeset:   57948:4a4d185098e2
tag: tip
user:    lfoltan
date:    Thu Feb 06 14:29:57 2020 +
summary: 8230199: consolidate signature parsing code in HotSpot 
sources

this one now leads to another error (see below):
changeset:   57949:d3caf06ac9ae
tag: tip
user:    lfoltan
date:    Thu Feb 06 15:28:37 2020 +
summary: 8238600: Remove runtime/fieldType.hpp and fieldType.cpp

Is this a problem with the source tree or do we need to adjust the 
build setup on ARM?


Background info: I’m a developer of the JaCoCo project and we do CI 
builds on all JDKs to test them agains the JaCoCo integration tests.


Best regards,
-marc






ERROR: Build failed for target 'images' in configuration 
'linux-arm-server-release' (exit code 2)

Stopping sjavac server

=== Output from failing command(s) repeated here ===
* For target hotspot_variant-server_libjvm_objs_interpreterRT_arm.o:
/workspace/src/hotspot/cpu/arm/interpreterRT_arm.cpp:54:27: error: 
'int_parm' is not a member of 'SignatureIterator'

    (u1) SignatureIterator::int_parm, // bool
    ^~~~
/workspace/src/hotspot/cpu/arm/interpreterRT_arm.cpp:55:27: error: 
'int_parm' is not a member of 'SignatureIterator'

    (u1) SignatureIterator::int_parm, // byte
    ^~~~
/workspace/src/hotspot/cpu/arm/interpreterRT_arm.cpp:56:27: error: 
'int_parm' is not a member of 'SignatureIterator'

    (u1) SignatureIterator::int_parm, // char
    ^~~~
/workspace/src/hotspot/cpu/arm/interpreterRT_arm.cpp:57:27: error: 
'int_parm' is not a member of 'SignatureIterator'

    (u1) SignatureIterator::int_parm, // short
    ^~~~
/workspace/src/hotspot/cpu/arm/interpreterRT_arm.cpp:58:27: error: 
'int_parm' is not a member of 'SignatureIterator'

    (u1) SignatureIterator::int_parm, // int
    ^~~~
    ... (rest of output omitted)