RFR: 8326936: RISC-V: Shenandoah GC crashes due to incorrect atomic memory operations [v2]
Gui Cao
gcao at openjdk.org
Thu Feb 29 06:42:52 UTC 2024
On Thu, 29 Feb 2024 03:36:07 GMT, MaxXing <duke at openjdk.org> wrote:
>> With compressed oops enabled, Shenandoah GC crashes in the concurrent marking phase due to some incorrect atomic memory operations. This resulted in the failure of multiple related tests, including `gc/shenandoah/TestSmallHeap.java`, `gc/metaspace/TestMetaspacePerfCounters.java#Shenandoah-64`, and so on, tested on XuanTie C910 and QEMU.
>>
>> This failure is related to a GCC bug we recently discovered: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114130.
>>
>> In detail, there's a pattern in method `ShenandoahHeap::conc_update_with_forwarded`:
>>
>>
>> if (in_collection_set(obj)) {
>> // Dereferences `obj` without explicit null check.
>> oop fwd = ShenandoahBarrierSet::resolve_forwarded_not_null(obj);
>> // Then calls atomic built-in.
>> atomic_update_oop(fwd, p, obj);
>> }
>>
>>
>> `atomic_update_oop` then compresses `obj` into a 32-bit word and calls the built-in `__atomic_compare_exchange`. The latter produces incorrect assembly that comparing this unsigned 32-bit word with the sign-extended result of `lr.w` instructions.
>>
>> This bug can be bypassed by adding an explicit null check (like `if (obj && in_collection_set(obj))`), or adding compiler flag `-fno-delete-null-pointer-checks`.
>>
>> In previous commits, JDK-8316186 removed RISC-V's `PlatformCmpxchg<4>` but left the flag `-fno-delete-null-pointer-checks` enabled. Then JDK-8316893 removed the flag and made the bug visible. This patch adds `PlatformCmpxchg<4>` back.
>
> MaxXing has updated the pull request incrementally with one additional commit since the last revision:
>
> Remove unnecessary sign-extension.
Hi, When I use this patch, it doesn't compile, and reports the following error msg:
/home/zifeihan/jdk/src/hotspot/os_cpu/linux_riscv/atomic_linux_riscv.hpp:190:27: note: the comparison reduces to ‘(1 >= 8)’
190 | STATIC_ASSERT(byte_size >= 8);
/home/zifeihan/jdk/src/hotspot/share/utilities/debug.hpp:289:44: note: in definition of macro ‘STATIC_ASSERT’
... (rest of output omitted)
* For target hotspot_variant-server_libjvm_objs_arraycopynode.o:
In file included from /home/zifeihan/jdk/src/hotspot/share/memory/allocation.hpp:29,
from /home/zifeihan/jdk/src/hotspot/share/classfile/classLoaderData.hpp:28,
from /home/zifeihan/jdk/src/hotspot/share/precompiled/precompiled.hpp:34:
/home/zifeihan/jdk/src/hotspot/os_cpu/linux_riscv/atomic_linux_riscv.hpp: In instantiation of ‘T Atomic::PlatformCmpxchg<byte_size>::operator()(volatile T*, T, T, atomic_memory_order) const [with T = unsigned char; long unsigned int byte_size = 1]’:
/home/zifeihan/jdk/src/hotspot/share/runtime/atomic.hpp:1026:40: required from ‘T Atomic::CmpxchgImpl<T, T, T, typename std::enable_if<std::is_integral<_Tp>::value, void>::type>::operator()(volatile T*, T, T, atomic_memory_order) const [with T = unsigned char; typename std::enable_if<std::is_integral<_Tp>::value, void>::type = void]’
/home/zifeihan/jdk/src/hotspot/share/runtime/atomic.hpp:1002:32: required from ‘static D Atomic::cmpxchg(volatile D*, U, T, atomic_memory_order) [with D = unsigned char; U = unsigned char; T = unsigned char]’
/home/zifeihan/jdk/src/hotspot/share/oops/methodData.hpp:216:46: required from here
/home/zifeihan/jdk/src/hotspot/os_cpu/linux_riscv/atomic_linux_riscv.hpp:190:27: error: static assertion failed: byte_size >= 8
190 | STATIC_ASSERT(byte_size >= 8);
/home/zifeihan/jdk/src/hotspot/share/utilities/debug.hpp:289:44: note: in definition of macro ‘STATIC_ASSERT’
289 | #define STATIC_ASSERT(Cond) static_assert((Cond), #Cond)
| ^~~~
/home/zifeihan/jdk/src/hotspot/os_cpu/linux_riscv/atomic_linux_riscv.hpp:190:27: note: the comparison reduces to ‘(1 >= 8)’
190 | STATIC_ASSERT(byte_size >= 8);
/home/zifeihan/jdk/src/hotspot/share/utilities/debug.hpp:289:44: note: in definition of macro ‘STATIC_ASSERT’
... (rest of output omitted)
* For target hotspot_variant-server_libjvm_objs_assembler.o:
In file included from /home/zifeihan/jdk/src/hotspot/share/memory/allocation.hpp:29,
from /home/zifeihan/jdk/src/hotspot/share/classfile/classLoaderData.hpp:28,
from /home/zifeihan/jdk/src/hotspot/share/precompiled/precompiled.hpp:34:
/home/zifeihan/jdk/src/hotspot/os_cpu/linux_riscv/atomic_linux_riscv.hpp: In instantiation of ‘T Atomic::PlatformCmpxchg<byte_size>::operator()(volatile T*, T, T, atomic_memory_order) const [with T = unsigned char; long unsigned int byte_size = 1]’:
/home/zifeihan/jdk/src/hotspot/share/runtime/atomic.hpp:1026:40: required from ‘T Atomic::CmpxchgImpl<T, T, T, typename std::enable_if<std::is_integral<_Tp>::value, void>::type>::operator()(volatile T*, T, T, atomic_memory_order) const [with T = unsigned char; typename std::enable_if<std::is_integral<_Tp>::value, void>::type = void]’
/home/zifeihan/jdk/src/hotspot/share/runtime/atomic.hpp:1002:32: required from ‘static D Atomic::cmpxchg(volatile D*, U, T, atomic_memory_order) [with D = unsigned char; U = unsigned char; T = unsigned char]’
/home/zifeihan/jdk/src/hotspot/share/runtime/atomic.hpp:750:38: required from ‘T Atomic::PrefetchBitopsUsingCmpxchg::bitop(volatile T*, atomic_memory_order, Op) const [with T = unsigned char; Op = Atomic::PrefetchBitopsUsingCmpxchg::fetch_then_or<unsigned char>(volatile unsigned char*, unsigned char, atomic_memory_order) const::<lambda(unsigned char)>]’
/home/zifeihan/jdk/src/hotspot/share/runtime/atomic.hpp:763:17: required from ‘T Atomic::PrefetchBitopsUsingCmpxchg::fetch_then_or(volatile T*, T, atomic_memory_order) const [with T = unsigned char]’
/home/zifeihan/jdk/src/hotspot/share/runtime/atomic.hpp:194:53: required from ‘static T Atomic::fetch_then_or(volatile T*, T, atomic_memory_order) [with T = unsigned char]’
/home/zifeihan/jdk/src/hotspot/share/oops/instanceKlassFlags.hpp:126:58: required from here
/home/zifeihan/jdk/src/hotspot/os_cpu/linux_riscv/atomic_linux_riscv.hpp:190:27: error: static assertion failed: byte_size >= 8
190 | STATIC_ASSERT(byte_size >= 8);
/home/zifeihan/jdk/src/hotspot/share/utilities/debug.hpp:289:44: note: in definition of macro ‘STATIC_ASSERT’
289 | #define STATIC_ASSERT(Cond) static_assert((Cond), #Cond)
| ^~~~
... (rest of output omitted)
* All command lines available in /home/zifeihan/jdk/build/linux-riscv64-server-release/make-support/failure-logs.
=== End of repeated output ===
No indication of failed target found.
HELP: Try searching the build log for '] Error'.
HELP: Run 'make doctor' to diagnose build problems.
make[1]: *** [/home/zifeihan/jdk/make/Init.gmk:323: main] Error 2
make: *** [/home/zifeihan/jdk/make/Init.gmk:189: default] Error 2
gcc version:
[zifeihan at fedora-riscv jdk]$ gcc -v
Using built-in specs.
COLLECT_GCC=gcc
COLLECT_LTO_WRAPPER=/usr/libexec/gcc/riscv64-redhat-linux/13/lto-wrapper
Target: riscv64-redhat-linux
Configured with: ../configure --enable-bootstrap --enable-languages=c,c++,fortran,objc,obj-c++,ada,go,d,m2,lto --prefix=/usr --mandir=/usr/share/man --infodir=/usr/share/info --with-bugurl=http://bugzilla.redhat.com/bugzilla --enable-shared --enable-threads=posix --enable-checking=release --enable-multilib --with-system-zlib --enable-__cxa_atexit --disable-libunwind-exceptions --enable-gnu-unique-object --enable-linker-build-id --with-gcc-major-version-only --enable-libstdcxx-backtrace --with-libstdcxx-zoneinfo=/usr/share/zoneinfo --with-linker-hash-style=gnu --enable-plugin --enable-initfini-array --with-isl=/builddir/build/BUILD/gcc-13.2.1-20230728/obj-riscv64-redhat-linux/isl-install --enable-gnu-indirect-function --with-arch=rv64gc --with-abi=lp64d --with-multilib-list=lp64d --build=riscv64-redhat-linux --with-build-config=bootstrap-lto --enable-link-serialization=1
Thread model: posix
Supported LTO compression algorithms: zlib zstd
gcc version 13.2.1 20230728 (Red Hat 13.2.1-1) (GCC)
-------------
PR Comment: https://git.openjdk.org/jdk/pull/18039#issuecomment-1970502994
More information about the hotspot-runtime-dev
mailing list