JDK-8294031 : GCC 12 fails to compile AArch64 due to -Wstringop-overflow
  • Type: Bug
  • Component: hotspot
  • Sub-Component: runtime
  • Affected Version: 20
  • Priority: P4
  • Status: In Progress
  • Resolution: Unresolved
  • Submitted: 2022-09-19
  • Updated: 2023-01-14
The Version table provides details related to the release that this issue/RFE will be addressed.

Unresolved : Release in which this issue/RFE will be addressed.
Resolved: Release in which this issue/RFE has been resolved.
Fixed : Release in which this issue/RFE has been fixed. The release containing this fix may be available for download as an Early Access Release or a General Availability Release.

To download the current JDK release, click here.
JDK 21
21Unresolved
Related Reports
Duplicate :  
JDK-8295986 - [lworld] compilation problem on aarch64 Ubuntu 22.10
Relates :  
JDK-8299580 - RISC-V: fail to build with GCC 12 due to stringop-overflow warning
Description
Cross-building AArch64 with GCC 12.1.0, 12.2.0 fails like this:

* For target hotspot_variant-server_libjvm_objs_classLoader.o:
In file included from /home/buildbot/worker/build-jdkX-debian12/build/src/hotspot/share/runtime/atomic.hpp:603,
                 from /home/buildbot/worker/build-jdkX-debian12/build/src/hotspot/share/oops/oop.hpp:33,
                 from /home/buildbot/worker/build-jdkX-debian12/build/src/hotspot/share/cds/metaspaceShared.hpp:31,
                 from /home/buildbot/worker/build-jdkX-debian12/build/src/hotspot/share/cds/filemap.hpp:28,
                 from /home/buildbot/worker/build-jdkX-debian12/build/src/hotspot/share/classfile/classLoader.cpp:29:
In member function 'T Atomic::PlatformOrderedLoad<byte_size, X_ACQUIRE>::operator()(const volatile T*) const [with T = ClassPathEntry*; long unsigned int byte_size = 8]',
    inlined from 'T Atomic::LoadImpl<T, PlatformOp, typename std::enable_if<(IsIntegral<T>::value || IsPointer<T>::value), void>::type>::operator()(const volatile T*) const [with T = ClassPathEntry*; PlatformOp = Atomic::PlatformOrderedLoad<8, X_ACQUIRE>]' at /home/buildbot/worker/build-jdkX-debian12/build/src/hotspot/share/runtime/atomic.hpp:391:24,
    inlined from 'static T Atomic::load_acquire(const volatile T*) [with T = ClassPathEntry*]' at /home/buildbot/worker/build-jdkX-debian12/build/src/hotspot/share/runtime/atomic.hpp:628:67,
    inlined from 'ClassPathEntry* ClassPathEntry::next() const' at /home/buildbot/worker/build-jdkX-debian12/build/src/hotspot/share/classfile/classLoader.inline.hpp:34:82,
    inlined from 'static ClassPathEntry* ClassLoader::classpath_entry(int)' at /home/buildbot/worker/build-jdkX-debian12/build/src/hotspot/share/classfile/classLoader.inline.hpp:55:18,
    inlined from 'static ClassPathEntry* ClassLoader::classpath_entry(int)' at /home/buildbot/worker/build-jdkX-debian12/build/src/hotspot/share/classfile/classLoader.inline.hpp:41:24,
    inlined from 'static oop ClassLoader::get_system_package(const char*, JavaThread*)' at /home/buildbot/worker/build-jdkX-debian12/build/src/hotspot/share/classfile/classLoader.cpp:1004:37:
/home/buildbot/worker/build-jdkX-debian12/build/src/hotspot/os_cpu/linux_aarch64/atomic_linux_aarch64.hpp:203:66: error: 'long unsigned int __atomic_load_8(const volatile void*, int)' writing 8 bytes into a region of size 0 overflows the destination [-Werror=stringop-overflow=]
  203 |   T operator()(const volatile T* p) const { T data; __atomic_load(const_cast<T*>(p), &data, __ATOMIC_ACQUIRE); return data; }
      |                                                     ~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~


Release builds notably do not fail. Only fastdebugs builds fail.
Comments
Based on my (non-expert) understanding of gcc bug 104475, I think this arises because gcc12 has changed some of the value/range propagation to be more precise. But it runs into problems in the face of nullptr checks that don't lead to __builtin_unreachable or the like. What happens is that the branch taken for a nullptr value is predicted to be unlikely but not impossible, which leads later code to think a nullptr is explicitly (though perhaps rarely) possible. And oops... A release build doesn't have this problem because without the assert providing the additional precision the compiler has no information about whether there's a nullptr possibility or not, and so can't complain. The HotSpot assert macro, in the failure case, calls a function that isn't marked [[noreturn]] and that call isn't followed by __builtin_unreachable, so we run afoul of that gcc12 change. We can't mark that function [[noreturn]] because it *can* return, due to Debugging (involved in recursive error handling), SuppressErrorAt (a not-product feature), and UseOSErrorReporting (Windows-only). As an experiment, I was able to suppress the warnings by changing the BREAKPOINT macro to expand to __builtin_unreachable(). That provides further evidence for the above analysis. Note that we've run into problems with doing that sort of thing in the past though. See, for example, https://mail.openjdk.org/pipermail/hotspot-dev/2015-April/017990.html. Maybe we need to take another run at this?
14-01-2023
An experiment that failed was to apply __builtin_expect_with_probability to the assert condition. That is, use __builtin_expect_with_probability(!(p), false, 1.0). Still get the same warning.
08-01-2023
Reproduces on RISC-V - see JDK-8299580. Also reproduces on linux-x64 if we make it use __atomic_load for PlatformLoad. So it appears to be a generic gcc problem. Searching for gcc bugs, I see these that look relevant: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107694 https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104475 << this one looks like the most relevant https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106297 << this one seems to involve loop-unrolling, so might not be relevant The discussion of gcc bug 104475 is interesting, though hard to follow in detail for someone (like me) not intimately familiar with gcc internals.
06-01-2023
The alternative would be: ``` diff --git a/make/hotspot/lib/CompileJvm.gmk b/make/hotspot/lib/CompileJvm.gmk index 8e12c22c9ee..4ba38771587 100644 --- a/make/hotspot/lib/CompileJvm.gmk +++ b/make/hotspot/lib/CompileJvm.gmk @@ -156,6 +156,9 @@ $(eval $(call SetupJdkLibrary, BUILD_LIBJVM, \ abstract_vm_version.cpp_CXXFLAGS := $(CFLAGS_VM_VERSION), \ arguments.cpp_CXXFLAGS := $(CFLAGS_VM_VERSION), \ DISABLED_WARNINGS_gcc := $(DISABLED_WARNINGS_gcc), \ + DISABLED_WARNINGS_gcc_classLoader.cpp := stringop-overflow, \ + DISABLED_WARNINGS_gcc_jfrThreadIterator.cpp := stringop-overflow, \ + DISABLED_WARNINGS_gcc_klass.cpp := stringop-overflow, \ DISABLED_WARNINGS_clang := $(DISABLED_WARNINGS_clang), \ DISABLED_WARNINGS_xlc := $(DISABLED_WARNINGS_xlc), \ DISABLED_WARNINGS_microsoft := $(DISABLED_WARNINGS_microsoft), \ ```
28-09-2022
This looks like a bonafide GCC analysis trouble: apparently, if we have asserts that verify NULL-ity, stringop-overflow assumes the variable we are touching can be NULL? This hack makes aarch64 fastdebug build pass: ``` diff --git a/src/hotspot/os_cpu/linux_aarch64/vm_version_linux_aarch64.cpp b/src/hotspot/os_cpu/linux_aarch64/vm_version_linux_aarch64.cpp index e1ace9cd07b..d33c433284b 100644 --- a/src/hotspot/os_cpu/linux_aarch64/vm_version_linux_aarch64.cpp +++ b/src/hotspot/os_cpu/linux_aarch64/vm_version_linux_aarch64.cpp @@ -183,3 +183,3 @@ void VM_Version::get_os_cpu_info() { static bool read_fully(const char *fname, char *buf, size_t buflen) { - assert(buf != NULL, "invalid argument"); +// assert(buf != NULL, "invalid argument"); assert(buflen >= 1, "invalid argument"); diff --git a/src/hotspot/share/classfile/classLoader.inline.hpp b/src/hotspot/share/classfile/classLoader.inline.hpp index 183024252cd..5b633c81bec 100644 --- a/src/hotspot/share/classfile/classLoader.inline.hpp +++ b/src/hotspot/share/classfile/classLoader.inline.hpp @@ -53,3 +53,3 @@ inline ClassPathEntry* ClassLoader::classpath_entry(int n) { while (--n >= 1) { - assert(e != NULL, "Not that many classpath entries."); +// assert(e != NULL, "Not that many classpath entries."); e = e->next(); diff --git a/src/hotspot/share/jfr/utilities/jfrThreadIterator.cpp b/src/hotspot/share/jfr/utilities/jfrThreadIterator.cpp index 956224a80d9..c37610d15b4 100644 --- a/src/hotspot/share/jfr/utilities/jfrThreadIterator.cpp +++ b/src/hotspot/share/jfr/utilities/jfrThreadIterator.cpp @@ -36,3 +36,3 @@ static bool thread_inclusion_predicate(Thread* t) { static bool java_thread_inclusion_predicate(JavaThread* jt, bool live_only) { - assert(jt != NULL, "invariant"); +// assert(jt != NULL, "invariant"); if (live_only && jt->thread_state() == _thread_new) { diff --git a/src/hotspot/share/oops/klass.cpp b/src/hotspot/share/oops/klass.cpp index d0b74dd0e16..335dd90f156 100644 --- a/src/hotspot/share/oops/klass.cpp +++ b/src/hotspot/share/oops/klass.cpp @@ -415,3 +415,3 @@ Klass* Klass::next_sibling(bool log) const { void Klass::set_subklass(Klass* s) { - assert(s != this, "sanity check"); +// assert(s != this, "sanity check"); Atomic::release_store(&_subklass, s); ```
19-09-2022