RFR: 8237192: Generate stripped/public pdbs on Windows for jdk images
Erik Joelsson
erik.joelsson at oracle.com
Tue Feb 18 17:08:31 UTC 2020
Hello Christoph,
Thanks for hanging in there, this is now looking good to me. Nice to see
a more general solution to the java.pdb problem.
/Erik
On 2020-02-18 06:47, Langer, Christoph wrote:
>
> Hi,
>
> I had to update my change a little bit – I forgot to exclude
> jimage.map, java.map and jpackage.map when copying the cmd debug
> symbols in Images.gmk.
>
> Furthermore, I needed to modify tests
> jdk/tools/launcher/TestHelper.java and
> jdk/tools/launcher/VersionCheck.java to cope with debug symbol files
> in images/jdk/bin on non Windows platforms.
>
> http://cr.openjdk.java.net/~clanger/webrevs/8237192.2/
>
> Cheers
>
> Christoph
>
> *From:*Langer, Christoph
> *Sent:* Montag, 17. Februar 2020 10:17
> *To:* Erik Joelsson <erik.joelsson at oracle.com>; Magnus Ihse Bursie
> <magnus.ihse.bursie at oracle.com>; 'build-dev at openjdk.java.net'
> <build-dev at openjdk.java.net>
> *Cc:* 'hotspot-dev at openjdk.java.net' <hotspot-dev at openjdk.java.net>;
> Baesken, Matthias <matthias.baesken at sap.com>
> *Subject:* RE: RFR: 8237192: Generate stripped/public pdbs on Windows
> for jdk images
>
> Hi Erik,
>
> thanks for your review. While I’ve addressed all your points (thanks
> for the great hints regarding usage of SetupCopyFiles), I also
> enhanced the configure option a little bit.
>
> What you can now pass is
> --with-external-symbols-in-bundles=<none|public|full>. Default setting
> is none. It’ll require --with-native-debug-symbols=external obviously
> and is currently only implemented for Windows. This is checked in
> configure.
>
> Depending on the value used, the bundles and jmods will contain either
> no pdbs, public pdbs or the full pdbs. This configure option could
> easily be enhanced to work for Linux/Unix/Mac as well – but I didn’t
> want to go too far with my change now.
>
> What also comes with my change is debug files for executables
> (launchers, cmds) in images/<jdk|jre> which weren’t copied so far in
> Images.gmk. It’s however needed because bundles are created from these
> images and with the current bundle logic, stripped pdb files need to
> exist in images/… to get bundled for option ‘public’.
>
> I also removed the special handling of the pdb file for java.exe in
> Launcher-java.base.gmk, as the filtering to make sure it’s not
> overwriting the pdb for java.dll has to be done in CreateJmods.gmk and
> Images.gmk anyway.
>
> Eventually, when this is all refactored, one should probably generate
> the pdb files to ship into support/modules_libs and
> support/modules_cmds and only in the case of public pdbs redirect
> generation of the full pdbs to something like
> support/modules_libs_full_debug_info or something like that. Then,
> from support/modules_libs and support/modules_cmds, one should
> generate the base image via jlink which can simply be packed into the
> shipment bundle. And the other image with all debug/demos can be
> created by copying the base image and then applying the stuff from
> support/ modules_libs_full_debug_info etc.
>
> Here's the new webrev:
> http://cr.openjdk.java.net/~clanger/webrevs/8237192.1/
> <http://cr.openjdk.java.net/~clanger/webrevs/8237192.1/>
>
> Best regards
>
> Christoph
>
> *From:*Erik Joelsson <erik.joelsson at oracle.com
> <mailto:erik.joelsson at oracle.com>>
> *Sent:* Mittwoch, 12. Februar 2020 23:17
> *To:* Langer, Christoph <christoph.langer at sap.com
> <mailto:christoph.langer at sap.com>>; Magnus Ihse Bursie
> <magnus.ihse.bursie at oracle.com
> <mailto:magnus.ihse.bursie at oracle.com>>; 'build-dev at openjdk.java.net'
> <build-dev at openjdk.java.net <mailto:build-dev at openjdk.java.net>>
> *Cc:* 'hotspot-dev at openjdk.java.net' <hotspot-dev at openjdk.java.net
> <mailto:hotspot-dev at openjdk.java.net>>; Baesken, Matthias
> <matthias.baesken at sap.com <mailto:matthias.baesken at sap.com>>
> *Subject:* Re: RFR: 8237192: Generate stripped/public pdbs on Windows
> for jdk images
>
> Hello Christoph,
>
> This patch certainly looks better to me, though I agree it's a bit
> hackish to have to filter and rename the stripped.pdb files twice,
> once for jmods and again for bundles. I think I'm ok with it for now
> though. The future improvement I would like to make would be to create
> two sets of jdk images, one that contains debug symbols and demos,
> which we continue to use for testing, and another which only contains
> exactly what we bundle up, including the correctly named top dir. The
> latter would be created first and used as input for the former. I
> think lots of things would be cleaner then, especially Bundles.gmk.
>
> But, that can wait for later. With your current proposal, my proposed
> future change will apply seamlessly in regard to the stripped pdb
> files and our distribution bundles.
>
> The clash between launchers and libs is annoying, and we also have it
> for java.exe and java.dll already, though the build is explicitly
> handling that one somewhere else.
>
> Now to review, there are some details I will nitpick about.
>
> CreateJmods.gmk:
>
> 174, 185: Rule declarations should be indented like any other line
> inside conditionals. But, I have a problem with these rules as the
> target is the directory. This will not work well with incremental
> builds. I would recommend doing a SetupCopyFiles construct instead so
> you get individual rules for each file. It would look something like this:
>
> rename-stripped-pdb = \
> $(patsubst %.stripped.pdb,%.pdb,$1)
> $(eval $(call SetupCopyFiles, COPY_STRIPPED_LIBS, \
> SRC := $(LIBS_DIR), \
> DEST := $(LIBS_DIR_STRIPPED), \
> FILES := $(call FindFiles, $(LIBS_DIR)), \
> NAME_MACRO := rename-stripped-pdb, \
> ))
>
> DEPS += $(COPY_STRIPPED_LIBS)
>
> For the corresponding CMD_DIR, the removal of jimage and friends can
> be done with $(filter ) around The FindFiles call.
>
> GenerateLinkOptData.gmk:
>
> Please indent inside ifeq block. I would prefer having the TARGETS +=
> inside the conditional block. Seems you also left a commented out
> endif there.
>
> NativeCompilation.gmk
>
> 919: You changed the continuation indent from 4 to 2, please revert.
>
> /Erik
>
> On 2020-02-12 05:26, Langer, Christoph wrote:
>
> Hi Magnus, Erik,
>
> I started off with Matthias’ patch and tried to address your concerns.
> Especially I explored adding the stripped pdbs to the jmods as well.
> Here is what I came up with:
>
> http://cr.openjdk.java.net/~clanger/webrevs/8237192.0/
>
> It’s a bit hacky in that it’ll copy support/modules_libs to
> support/modules_libs_stripped and fix the pdbs to ship in there. The
> same goes for modules_cmds. The problem with that approach is that
> probably not all dependencies are placed correctly and also there’s a
> bit more of duplication of binaries in the build directories. I think,
> it could be repaired eventually by refactoring, e.g. have a
> support/modules_dbgsymbols folder where the real debug symbol files
> get placed and used from there.
>
> There’s also two additional caveats, one is that jimage.pdb and
> jpackage.pdb exist twice. Once for the libs and once for the launchers
> with the same name. This will cause failures when jlinking. I decided
> to keep the pdbs for the libs. And I also had to take care of the
> classlist generation, to have the resulting classlist placed in
> support/modules_libs_stripped as well.
>
> I furthermore updated the naming of options and variables, hopefully
> to your like. And I made the debug output logInfo.
>
> I tested (on Windows), both, with --enable-public-debug-symbols and
> without. Without the option, everything seems as before. With the
> option enabled, the stripped debug symbols will be installed in the
> bundles and also in the jmods. 😊
>
> Please let me know what you think.
>
> Thanks & Best regards
>
> Christoph
>
> *From:*Magnus Ihse Bursie <magnus.ihse.bursie at oracle.com>
> <mailto:magnus.ihse.bursie at oracle.com>
> *Sent:* Freitag, 7. Februar 2020 14:09
> *To:* Baesken, Matthias <matthias.baesken at sap.com>
> <mailto:matthias.baesken at sap.com>; Erik Joelsson
> <erik.joelsson at oracle.com> <mailto:erik.joelsson at oracle.com>; Langer,
> Christoph <christoph.langer at sap.com>
> <mailto:christoph.langer at sap.com>; David Holmes
> <david.holmes at oracle.com> <mailto:david.holmes at oracle.com>;
> 'build-dev at openjdk.java.net <mailto:build-dev at openjdk.java.net>'
> <build-dev at openjdk.java.net> <mailto:build-dev at openjdk.java.net>;
> 'hotspot-dev at openjdk.java.net <mailto:hotspot-dev at openjdk.java.net>'
> <hotspot-dev at openjdk.java.net> <mailto:hotspot-dev at openjdk.java.net>
> *Subject:* Re: RFR: 8237192: Generate stripped/public pdbs on Windows
> for jdk images
>
> 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 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
>
>
>
> -----Original Message-----
>
> From: build-dev<build-dev-bounces at openjdk.java.net> <mailto:build-dev-bounces at openjdk.java.net> On Behalf Of
>
> Erik
>
> Joelsson
>
> Sent: Donnerstag, 23. Januar 2020 18:49
>
> To: Baesken, Matthias<matthias.baesken at sap.com> <mailto:matthias.baesken at sap.com>; David Holmes
>
> <david.holmes at oracle.com> <mailto:david.holmes at oracle.com>; 'build-dev at openjdk.java.net <mailto:build-dev at openjdk.java.net>' <build-
>
> dev at openjdk.java.net <mailto:dev at openjdk.java.net>>; 'hotspot-dev at openjdk.java.net <mailto:hotspot-dev at openjdk.java.net>' <hotspot-
>
> dev at openjdk.java.net <mailto:dev at openjdk.java.net>>
>
> Subject: Re: RFR: 8237192: Generate stripped/public pdbs on Windows
>
> for
>
> jdk images
>
>
>
>
>
> On 2020-01-23 00:03, Baesken, Matthias wrote:
>
> Hi Erik, yes true sorry for answering your comments a bit late .
>
>
>
> If a user runs jlink and includes all the jmods we ship with the JDK, the
>
> result
>
> should be essentially equivalent to the original JDK image. The way
>
> the
>
> stripped pdb files are included in the bundles sort of at the last
>
> second of the build here breaks this property.
>
> I think we should address this in a separate bug/CR .
>
> Maybe. I realize that my proposal below is quite a big task. But on the
>
> other hand, I don't think breaking the relationship between the jmods
>
> and the distribution bundles is on the table really.
>
> Looking for example into a Linux build, I see a lot of debuginfo files in
>
> the
>
> jdk image (more or less for every shared lib) .
>
> But when looking into the jmods of that jdk image , no debuginfo files
>
> are
>
> in there ( I checked the java.base jmod).
>
> So putting the files with debug information into the jmods seems to
>
> be
>
> something that was not done so far cross platform (or is there some
>
> build
>
> switch for it that I did not check?) .
>
> Maybe to keep the jmods as small as possible .
>
> No, we do not put the debuginfo files in the jmods nor the bundles
>
> because those are not intended to be shipped to customers. We are
>
> currently overlaying them into images/jdk in the build output to make
>
> them available for local debugging. (This is convoluted and I would very
>
> much like to get away from this practice at some point so that there is
>
> a 1-1 mapping between images/jdk and bundles/jdk*-bin.tar.gz.) The
>
> stripped pdb files you are proposing are on the contrary intended for
>
> shipping to customers (as I understand your proposal) so comparing
>
> them
>
> with the debuginfo files is not relevant.
>
>
>
> Now if MS had been kind enough to define a separate file type for the
>
> stripped pdbs, so that they could live alongside the full pdbs, we
>
> wouldn't have this issue. The heart of the problem is that only one set
>
> of files (either stripped or full) can be present and usable in
>
> images/jdk at a time. We have 2 main uses for images/jdk.
>
>
>
> 1. Developer running and debugging locally
>
>
>
> 2. Serve as the source for generating the distribution bundles
>
>
>
> We currently have one image serving both of these purposes, which is
>
> already creating complicated and convoluted build steps. To properly
>
> solve this I would argue for splitting these two apart into two
>
> different images for the two different purposes. The build procedure
>
> would then be, first build the images for distribution, only containing
>
> what should go into each bundle. Then create the developer jdk image
>
> by
>
> copying files from the distribution images. On Windows, the first image
>
> would contain the stripped pdbs and when building the second, those
>
> would get overwritten with the full pdbs.
>
>
>
> Now that I figured out a working model that would solve a bunch of
>
> other
>
> problems as well, I would love to implement it, but I doubt I will have
>
> time in the near future.
>
>
>
> /Erik
>
>
>
> To properly implement this, care will need to be taken to juggle the
>
> two
>
> sets of pdb files around, making sure each build and test use case has
>
> the correct one in place where and when it's needed. Quite possibly,
>
> we
>
> cannot cover all use cases with one build configuration. Developers
>
> needing the full debug symbols when debugging locally would likely
>
> need
>
> to disable the stripped symbols so they get the full symbols
>
> everywhere.
>
> Possibly this would need to be the default for debug builds and
>
> configurable for release builds.
>
> From my limited experience , the developers do not work with the
>
> bundles (that would contain now after my patch the stripped pds) but
>
> with
>
> a "normal" jdk image that is created my make all.
>
> Best regards, Matthias
>
>
>
>
>
>
>
> This still does not address anything in my objection.
>
>
>
> /Erik
>
>
>
> On 2020-01-22 07:46, Baesken, Matthias wrote:
>
> Hello, here is an updated version :
>
>
>
> http://cr.openjdk.java.net/~mbaesken/webrevs/8237192.3/
>
>
>
> this one supports a configure switch "--enable-stripped-pdbs" to
>
> enable
>
> the feature .
>
> Best regards, Matthias
>
>
>
>
>
> -----Original Message-----
>
> From: Baesken, Matthias
>
> Sent: Dienstag, 21. Januar 2020 11:03
>
> To: 'David Holmes'<david.holmes at oracle.com> <mailto:david.holmes at oracle.com>; 'build-
>
> dev at openjdk.java.net <mailto:dev at openjdk.java.net>'<build-dev at openjdk.java.net> <mailto:build-dev at openjdk.java.net>; 'hotspot-
>
> dev at openjdk.java.net <mailto:dev at openjdk.java.net>'<hotspot-dev at openjdk.java.net> <mailto:hotspot-dev at openjdk.java.net>
>
> Subject: RE: RFR: 8237192: Generate stripped/public pdbs on
>
> Windows
>
> for
>
> jdk images
>
>
>
>
>
> Hi David , yes I think it makes sense to have a configure option for
>
> this .
>
> Not everyone would like to have a larger JDK (even it is only a bit
>
> larger).
>
>
>
More information about the build-dev
mailing list