[8u] RFR: 8037825: Fix warnings and enable "warnings as errors" in serviceability native libraries

[Volker Simonis]

Hi Paul,

have you intentionally only downported the "jdk/" part of this change
and omitted the "top-level" part
(http://hg.openjdk.java.net/jdk9/jdk9/rev/1cf2abab835f)?

If yes, that should probably be mentioned in the commit message and
the downport request. Otherwise, you'll also have to add the
"top-level" part. But that might be a little tricky/risky because it
could potentially introduce new build failures because of the new
"CFLAGS_WARNINGS_ARE_ERRORS" settings in conjunction with the jdk8u
compiler toolchains.

Besides that I've spotted two other small issues:

 - I can't see that the change in InvocationAdapter.c was already applied:
http://hg.openjdk.java.net/jdk8u/jdk8u-dev/jdk/file/tip/src/share/instrument/InvocationAdapter.c#l376

Have you maybe confused it with this occurrence?
http://hg.openjdk.java.net/jdk8u/jdk8u-dev/jdk/file/tip/src/share/instrument/InvocationAdapter.c#l204

- The following hunk looks pretty much like an error (notice the use
of "CFLAGS_CFLAGS_WARNINGS_ARE_ERRORS" instead of
"CFLAGS_WARNINGS_ARE_ERRORS" ), but it was already an error in the
initial patch so I'm not sure if it's worth fixing in the downport:

make/lib/ServiceabilityLibraries.gmk
-----------------------------------------------
@@ -81,11 +81,11 @@
     OUTPUT_DIR := $(INSTALL_LIBRARIES_HERE), \
     SRC := $(JDK_TOPDIR)/src/share/transport/socket \
         $(JDK_TOPDIR)/src/$(OPENJDK_TARGET_OS_API_DIR)/transport/socket, \
     LANG := C, \
     OPTIMIZATION := LOW, \
-    CFLAGS := $(CFLAGS_JDKLIB) -DUSE_MMAP \
+    CFLAGS := $(CFLAGS_JDKLIB) $(CFLAGS_CFLAGS_WARNINGS_ARE_ERRORS)
-DUSE_MMAP \

Thank you and best regards,
Volker

On Thu, Apr 29, 2021 at 11:58 PM Hohensee, Paul <hohensee at amazon.com> wrote:
>
> Please review this backport to help 8u build using Xcode 12.
>
> Original issue: https://bugs.openjdk.java.net/browse/JDK-8037825
> Original commit: http://hg.openjdk.java.net/jdk9/jdk9/jdk/rev/f5a18100873c
> 8u webrev: https://cr.openjdk.java.net/~phh/8037825/webrev.8u.jdk.00/
>
> The patch applied cleanly, except that the change to InvocationAdapter.c was already in place, so was left out of the backport. There are no semantic changes.
>
> Thanks,
> Paul
>