[jdk8u-dev] RFR: 8186787: clang-4.0 SIGSEGV in Unsafe_PutByte

Andrew Dinn adinn at openjdk.org
Wed Feb 26 10:04:07 UTC 2025


On Tue, 25 Feb 2025 16:40:40 GMT, Severin Gehwolf <sgehwolf at openjdk.org> wrote:

>> This backport fixes [failures](https://github.com/zzambers/jdk8u-dev/actions/runs/9858657012/job/27221579883) (segfaults) in following tests which appeared after [macos update](https://github.com/openjdk/jdk8u-dev/pull/544):
>> 
>> sun/misc/CopyMemory.java 
>> compiler/unsafe/OpaqueAccesses.java
>> 
>> Backport differs from original changeset, because there were significant changes/refactoring in unsafe.
>> 
>> **Notes:**
>> - [original changeset](https://github.com/openjdk/jdk11u-dev/commit/6dc1d8c06d98e127b022886172e16b90bf357c97) changes pointer returned by `addr` (`MemoryAccess` class), to volatile. Otherwise it is basically just refactoring.
>> - `MemoryAccess` is used by `Unsafe_{Set,Put}*` and `Unsafe_{Set,Put}*Volatile` functions, defined using `DEFINE_GETSETOOP` and `DEFINE_GETSETOOP_VOLATILE` macros
>> - jdk8 does not have `MemoryAccess` class, so equivalent pointers, in functions mentioned higher, are cast to volatile, to achieve same effect
>> 
>> Testing:
>> Tier1: OK (fixes `sun/misc/CopyMemory.java` and `compiler/unsafe/OpaqueAccesses.java` tests on macos, 1 failure on Linux x86 is timeout - seems unrelated, macos failures explained here: https://github.com/openjdk/jdk8u-dev/pull/544#issuecomment-2250636257)
>
> @adinn @tstuefe @theRealAph Could you please help review this? We see crashes of this in GHA, so it looks to fix a real bug. Then again it's fairly late to touch this area in JDK 8u. Thoughts?

@jerboaa I think this is fine to backport. Adding a volatile qualifier to the access cannot really do any harm as it has the very limited effect of stopping the C++ compiler from reordering the volatile field access write relative to other volatile field accesses within the current thread's instruction stream. Since that is actually what is required here I cannot see any real risk. My real concern is that the introduction of volatile is clearly flagged in comments and happens at a point where it is most obvious what is going on and why.

-------------

PR Comment: https://git.openjdk.org/jdk8u-dev/pull/553#issuecomment-2684486415


More information about the jdk8u-dev mailing list