Hi Andrew,
On Fri, 2020-09-18 at 06:40 +0100, Andrew Hughes wrote:
> On 20:16 Wed 09 Sep , Severin Gehwolf wrote:
> > Hi,
> >
> > Please review this 8u (jdk8u/jdk8u-dev tree) fix for JDK-8252395 that
> > I've pushed today. Thanks for Zhengyu Gu for noticing it. The pushed
> > fix added the java.debuginfo and unpack.debuginfo make targets on the
> > condition of ENABLE_DEBUG_SYMBOLS=true, which is insufficient. It needs
> > another check on POST_STRIP_CMD which is set to non-empty for --with-
> > native-debug-symbols={external,zipped}, and is indeed empty for --with-
> > native-debug-symbols=internal. For the --with-native-debug-symbols=none
> > case we have ENABLE_DEBUG_SYMBOLS=false.
> >
> > Bug: https://bugs.openjdk.java.net/browse/JDK-8252975
> > webrev: https://cr.openjdk.java.net/~sgehwolf/webrevs/JDK-8252975/01/webrev/
> >
> > Testing: Builds with
> > --with-native-debug-symbols={none,internal,external,zipped} on Linux x86_64
> >
> > OK?
> >
> > Thanks,
> > Severin
> >
>
> Build is still broken for me with this patch:
>
> /usr/bin/cp /home/ahughes/builder/8u-dev/jdk/objs/java_objs/java.diz
> /home/ahughes/builder/8u-dev/jdk/bin/java.diz
> /usr/bin/cp: cannot stat
> '/home/ahughes/builder/8u-dev/jdk/objs/java_objs/java.diz': No such file or
> directory
>
> where --with-native-debug-symbols is not set (pre-JDK-8207324)
Hmm, I'm not sure how you are building. I've checked builds with:
$ bash configure \
--with-boot-jdk="/some/boot/jdk" \
--with-extra-cflags=-Wno-error \
--disable-zip-debug-info
$ make images
and
$ bash configure \
--with-boot-jdk="/some/boot/jdk" \
--with-extra-cflags=-Wno-error
$ make images
and
$ bash configure \
--with-boot-jdk="/some/boot/jdk" \
--with-extra-cflags=-Wno-error \
--disable-debug-symbols
$ make images
All seem to work fine for me. Are you sure you are not
setting POST_STRIP_CMD or the like explicitly somewhere else. Please
post the exact configure/make invocations you are using.
FWIW, that bug you've referenced seems wrong:
8207324: aarch64 jtreg: assert in TestOptionsWithRanges.jtr
You probably meant JDK-8036003:
8036003: Add --with-native-debug-symbols=[none|internal|external|zipped]
> The use of these two conditionals seems odd to me. What we want to know
> is whether zipped debuginfo is in use.
No. We want to know whether or not external debug symbols (zipped or
otherwise) are in use.
> This is not the same as debug symbols
> in general being enabled.
Yes. Correctly so.
> Also, DEBUGINFO_EXT is only being set when
> ZIP_DEBUGINFO_FILES is true!
How so?
ifeq ($(ZIP_DEBUGINFO_FILES), true)
DEBUGINFO_EXT := .diz
else ifeq ($(OPENJDK_TARGET_OS), macosx)
DEBUGINFO_EXT := .dSYM
else ifeq ($(OPENJDK_TARGET_OS), windows)
DEBUGINFO_EXT := .pdb
else
DEBUGINFO_EXT := .debuginfo
endif
On Linux with --with-native-debug-symbols=external this ends up with:
DEBUGINFO_EXT := .debuginfo
> POST_STRIP_CMD seems to only be used in image creation, so I don't
> think checking this is appropriate either.
Seems debatable.
> Instead, we should mirror what is done in make/common/NativeCompilation.gmk
> when
> creating the .diz files:
>
> -ifeq ($(ENABLE_DEBUG_SYMBOLS), true)
> - ifneq ($(POST_STRIP_CMD), )
> +ifeq ($(ZIP_DEBUGINFO_FILES), true)
> + ifneq ($(STRIP_POLICY), no_strip)
>
> The second check is necessary because ZIP_DEBUGINFO_FILES is set by default,
> and so may be true even if STRIP_POLICY has been set to no_strip.
Your patch won't work for --with-native-debug-symbols=external. In that
case no *.debuginfo files for the java and unpack200 launchers would be
created.
With your patch:
$ find build/linux-x86_64-normal-server-release/images/j2sdk-image/bin/ | grep
-E 'java\.debuginfo|unpack200\.debuginfo'
<nothing>
Expected:
$ find build/linux-x86_64-normal-server-release/images/j2sdk-image/bin/ | grep
-E 'java\.debuginfo|unpack200\.debuginfo'
build/linux-x86_64-normal-server-release/images/j2sdk-image/bin/java.debuginfo
build/linux-x86_64-normal-server-release/images/j2sdk-image/bin/unpack200.debuginfo
> The attached patch fixed the build for me.
Sorry, I don't seem to be able to reproduce your build failure.
Thanks,
Severin
