[8u] RFR: 8252975: [8u] JDK-8252395 breaks the build for --with-native-debug-symbols=internal

[Severin Gehwolf]

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