Enable OMR_WARNINGS_AS_ERRORS in the JIT on Power for clang and GCC builds

[dylanjtuttle]

"Warnings as errors" is enabled on a component-by-component basis throughout OpenJ9 and OMR. For the JIT, warnings as errors is enabled on every platform in OMR and on x86, Z, and Aarch64 in OpenJ9.

This PR enables warnings as errors on Power in OpenJ9 by turning the OMR_WARNINGS_AS_ERRORS flag on if OMR_ARCH_POWER is true. This will ensure all builds on Power compile code with the -Werror flag, which will halt compilation if a warning is reported.

Currently, OpenJ9 JDK 8 builds on AIX are built with pure xlC, while other JDK 11+ builds on AIX are built with xlC with a clang frontend and plinux builds are built with GCC. Because of this, different warnings appear during OpenJ9 JDK 8 on AIX builds that have not been fixed yet. For that reason, warnings as errors will not be enabled on that build yet.

[keithc-ca]

Have you tested with gcc 13 (which is the default version on Ubuntu 24.04)?

[dylanjtuttle]

@keithc-ca I don't believe I did, but once I've cleaned up any warnings that appeared while I was gone I'll make sure to do that.

[dylanjtuttle]

@keithc-ca I can confirm that no warnings arise from the JIT when building JDK 11 on plinux Ubuntu 20.04 with gcc 13. I actually had to disable warnings-as-errors to complete the build because there were a handful of warnings outside of the JIT that crashed it. I find that a little odd, because I ran plenty of warnings-as-errors builds last year that had non-JIT warnings and had no issues.

To be fair, the only warning that I know crashed the build for certain was the first one, because after that I configured with --disable-warnings-as-errors, --disable-warnings-as-errors-openj9, and --disable-warnings-as-errors-omr, so the others just appeared as regular warnings. I have a feeling that this warning crashes because it arises from openj9/runtime, whereas most (or potentially all) of the non-JIT warnings I saw last year came from places like java.base. Nevertheless, I'm still confused that turning on warnings as errors in runtime/compiler/CMakeLists.txt seems to potentially have an effect on code outside of runtime/compiler.

The warnings in question: 1.

/root/openj9-openjdk-jdk11/openj9/runtime/rastrace/trcmain.c: In function 'initializeTrace':
/root/openj9-openjdk-jdk11/openj9/runtime/rastrace/trcmain.c:1045:14: warning: storing the address of local variable 'tempThr' in '*thr' [-Wdangling-pointer=]
 1045 |         *thr = &tempThr;
      |         ~~~~~^~~~~~~~~~
/root/openj9-openjdk-jdk11/openj9/runtime/rastrace/trcmain.c:1033:23: note: 'tempThr' declared here
 1033 |         UtThreadData  tempThr;
      |                       ^~~~~~~
/root/openj9-openjdk-jdk11/openj9/runtime/rastrace/trcmain.c:1026:32: note: 'thr' declared here
 1026 | initializeTrace(UtThreadData **thr, void **gbl,
      |                 ~~~~~~~~~~~~~~~^~~

/root/openj9-openjdk-jdk11/openj9/runtime/shared_common/CacheMap.cpp: In member function 'const bool SH_CacheMap::matchAotMethod(MethodSpecTable*, IDATA, J9UTF8*, J9UTF8*, J9UTF8*)':
/root/openj9-openjdk-jdk11/openj9/runtime/shared_common/CacheMap.cpp:6330:33: warning: the address of 'J9UTF8::data' will never be NULL [-Waddress]
 6330 |                 } else if (NULL == J9UTF8_DATA(romMethodSig)) {
In file included from /root/openj9-openjdk-jdk11/openj9/runtime/oti/j9generated.h:69,
                 from /root/openj9-openjdk-jdk11/openj9/runtime/oti/j9.h:79,
                 from /root/openj9-openjdk-jdk11/openj9/runtime/shared_common/CacheMapStats.hpp:28,
                 from /root/openj9-openjdk-jdk11/openj9/runtime/shared_common/CacheMap.hpp:27,
                 from /root/openj9-openjdk-jdk11/openj9/runtime/shared_common/CacheMap.cpp:22:
/root/openj9-openjdk-jdk11/openj9/runtime/oti/j9nonbuilder.h:3520:13: note: 'J9UTF8::data' declared here
 3520 |         U_8 data[];
      |             ^~~~

/root/openj9-openjdk-jdk11/openj9/runtime/shared_common/CacheMap.cpp: In member function 'IDATA SH_CacheMap::startupLowerLayerForStats(J9VMThread*, const char*, UDATA, SH_OSCache*, J9Pool**)':
/root/openj9-openjdk-jdk11/openj9/runtime/shared_common/CacheMap.cpp:5991:65: warning: dangling pointer 'cacheName' to 'cacheNameBuf' may be used [-Wdangling-pointer=]
 5991 |                         rc = ccToUse->startupNonTopLayerForStats(currentThread, ctrlDirName, cacheName, cacheType, layer, _runtimeFlags, 0);
      |                              ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/root/openj9-openjdk-jdk11/openj9/runtime/shared_common/CacheMap.cpp:5974:30: note: 'cacheNameBuf' declared here
 5974 |                         char cacheNameBuf[USER_SPECIFIED_CACHE_NAME_MAXLEN];
      |                              ^~~~~~~~~~~~

/root/openj9-openjdk-jdk11/openj9/runtime/shared_common/ScopeManagerImpl.cpp: In static member function 'static UDATA SH_ScopeManagerImpl::scHashEqualFn(void*, void*, void*)':
/root/openj9-openjdk-jdk11/openj9/runtime/shared_common/ScopeManagerImpl.cpp:99:34: warning: the address of 'J9UTF8::data' will never be NULL [-Waddress]
   99 |         if (J9UTF8_DATA(utf8left)==NULL || J9UTF8_DATA(utf8right)==NULL) {
In file included from /root/openj9-openjdk-jdk11/openj9/runtime/oti/j9generated.h:69,
                 from /root/openj9-openjdk-jdk11/openj9/runtime/oti/j9.h:79,
                 from /root/openj9-openjdk-jdk11/openj9/runtime/oti/shcdatatypes.h:26,
                 from /root/openj9-openjdk-jdk11/openj9/runtime/shared_common/sharedconsts.h:26,
                 from /root/openj9-openjdk-jdk11/openj9/runtime/shared_common/Manager.hpp:32,
                 from /root/openj9-openjdk-jdk11/openj9/runtime/shared_common/ScopeManager.hpp:32,
                 from /root/openj9-openjdk-jdk11/openj9/runtime/shared_common/ScopeManagerImpl.hpp:32,
                 from /root/openj9-openjdk-jdk11/openj9/runtime/shared_common/ScopeManagerImpl.cpp:28:
/root/openj9-openjdk-jdk11/openj9/runtime/oti/j9nonbuilder.h:3520:13: note: 'J9UTF8::data' declared here
 3520 |         U_8 data[];
      |             ^~~~
/root/openj9-openjdk-jdk11/openj9/runtime/shared_common/ScopeManagerImpl.cpp:99:66: warning: the address of 'J9UTF8::data' will never be NULL [-Waddress]
   99 |         if (J9UTF8_DATA(utf8left)==NULL || J9UTF8_DATA(utf8right)==NULL) {
/root/openj9-openjdk-jdk11/openj9/runtime/oti/j9nonbuilder.h:3520:13: note: 'J9UTF8::data' declared here
 3520 |         U_8 data[];
      |             ^~~~

/root/openj9-openjdk-jdk11/openj9/runtime/libffi/closures.c: In function 'dlmmap_locked':
/root/openj9-openjdk-jdk11/openj9/runtime/libffi/closures.c:495:7: warning: ignoring return value of 'ftruncate' declared with attribute 'warn_unused_result' [-Wunused-result]
  495 |       ftruncate (execfd, offset);
      |       ^~~~~~~~~~~~~~~~~~~~~~~~~~
/root/openj9-openjdk-jdk11/openj9/runtime/libffi/closures.c:507:7: warning: ignoring return value of 'ftruncate' declared with attribute 'warn_unused_result' [-Wunused-result]
  507 |       ftruncate (execfd, offset);
      |       ^~~~~~~~~~~~~~~~~~~~~~~~~~

/root/openj9-openjdk-jdk11/openj9/runtime/tests/shared/ByteDataTest.cpp: In function 'IDATA testByteDataManager(J9JavaVM*)':
/root/openj9-openjdk-jdk11/openj9/runtime/tests/shared/ByteDataTest.cpp:112:55: warning: 'cachePointers.CachePointers::testCache2' may be used uninitialized [-Wmaybe-uninitialized]
  112 |         if (cachePointers.testCache1 && cachePointers.testCache2) {
      |                                         ~~~~~~~~~~~~~~^~~~~~~~~~
/root/openj9-openjdk-jdk11/openj9/runtime/tests/shared/ByteDataTest.cpp:98:30: note: 'cachePointers' declared here
   98 |         struct CachePointers cachePointers;
      |                              ^~~~~~~~~~~~~

/root/openj9-openjdk-jdk11/src/java.base/unix/native/libjli/java_md_common.c: In function 'Resolve':
/root/openj9-openjdk-jdk11/src/java.base/unix/native/libjli/java_md_common.c:131:43: warning: '%s' directive output may be truncated writing up to 4095 bytes into a region of size between 2 and 4097 [-Wformat-truncation=]
  131 |     JLI_Snprintf(name, sizeof(name), "%s%c%s", indir, FILE_SEPARATOR, cmd);
      |                                           ^~
In file included from /usr/include/stdio.h:867,
                 from /root/openj9-openjdk-jdk11/src/java.base/share/native/libjli/java.h:29,
                 from /root/openj9-openjdk-jdk11/src/java.base/unix/native/libjli/java_md_common.c:25:
In function 'snprintf',
    inlined from 'Resolve' at /root/openj9-openjdk-jdk11/src/java.base/unix/native/libjli/java_md_common.c:131:5:
/usr/include/powerpc64le-linux-gnu/bits/stdio2.h:67:10: note: '__builtin___snprintf_chk' output between 2 and 8192 bytes into a destination of size 4098
   67 |   return __builtin___snprintf_chk (__s, __n, __USE_FORTIFY_LEVEL - 1,
      |          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   68 |                                    __bos (__s), __fmt, __va_arg_pack ());
      |                                    ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

[keithc-ca]

I saw (1) in my xlinux build with gcc 13; that one will require some more thought.

Warnings (2) and (4) should be easy to address: simply remove the dead code.

Warning (3) may just require moving the declaration to a larger scope so the target survives as long as the pointer.

I tried to address warning (5) some time ago, but I remember having trouble telling the compiler to ignore the return value. That is code we inherit from https://github.com/libffi/libffi so any improvements should be shared there as well.

Warning (6) should be straight-forward: add initialization visible to the compiler.

Issue (7) is in openjdk code; I'm not sure jdk11 has been kept current with newer compliers. Even the head stream, the minimum gcc version is 10.

[dylanjtuttle]

@keithc-ca Do you know how it's possible for a change in the JIT's CMakeLists.txt to be affecting non-JIT code? That seems dangerous. From an offline conversation with @hzongaro, neither of us seem to know why this would be happening, so as far as we know, any change to the JIT's CMake configuration could theoretically break any other part of the code for seemingly no reason (kind of like it did in this case with OMR_WARNINGS_AS_ERRORS).

With a better understanding of why this CMake dependency between the JIT and other components exists, perhaps we could figure out how to fix it, or at least be better informed about when it's safe to change the JIT's CMakeLists.txt and when it isn't.

[keithc-ca]

Do you know how it's possible for a change in the JIT's CMakeLists.txt to be affecting non-JIT code?

The only thing that I can think of is that OMR_WARNINGS_AS_ERRORS is a CACHE variable, which effectively lives in a scope outside the project, and so affects the whole project. In the end, we want all warnings to be fatal, so we should just fix any we find.

[dylanjtuttle]

Understood. I'll start working on those warnings!

[dylanjtuttle]

I hadn't actually tested this at the time, but it turns out that even the warning from the openjdk code is treated as an error. Would that be a PR to https://github.com/ibmruntimes/openj9-openjdk-jdk11? If so, I suppose I should check all of our JDK versions...

[keithc-ca]

I hadn't actually tested this at the time, but it turns out that even the warning from the openjdk code is treated as an error. Would that be a PR to https://github.com/ibmruntimes/openj9-openjdk-jdk11? If so, I suppose I should check all of our JDK versions...

We have three independent configuration options:

• --disable-warnings-as-errors for openjdk code
• --disable-warnings-as-errors-openj9 for OpenJ9 code
• --disable-warnings-as-errors-omr for OMR code

jdk11 may not be ready for newer compilers, in which case we should just use the first option. OpenJ9 code and OMR code should not have warnings even without using the latter two options.

[dylanjtuttle]

I see, I'll just focus on the OpenJ9 and OMR warnings then.

[dylanjtuttle]

@keithc-ca @pshipton @zl-wang Now that the last warning has been disabled, could I get a review on this when one or more of you has a chance?

[dylanjtuttle]

@keithc-ca How would I run a build of IBM Java 8 on AIX? That is the last build we have that still uses the xlC frontend and I would like to confirm that this change doesn't break that build.

[dylanjtuttle]

Done!

[keithc-ca]

Jenkins test sanity plinux jdk23

[keithc-ca]

The test failure appears to be https://github.com/eclipse-openj9/openj9/issues/17474.