RFR: 8327647: Occasional SIGSEGV in markWord::displaced_mark_helper() for SPECjvm2008 sunflow

[Coleen Phillimore]

On Mon, 25 Mar 2024 19:41:02 GMT, Matias Saavedra Silva <matsaave at openjdk.org> wrote:

> A misplaced memory barrier causes a very intermittent crash on on some aarch64 systems. This patch adds an appropriate LoadLoad barrier after a constant pool cache field entry is loaded.

Changes requested by coleenp (Reviewer).

src/hotspot/cpu/aarch64/templateTable_aarch64.cpp line 2360:

> 2358:   if (tos_state != noreg) {
> 2359:     __ load_unsigned_byte(tos_state, Address(cache, in_bytes(ResolvedFieldEntry::type_offset())));
> 2360:   }

This handling of tos_state seems fine to me.  Add a comment that the caller might not want to set type_offset as tos.

src/hotspot/cpu/aarch64/templateTable_aarch64.cpp line 2559:

> 2557:     // Must prevent reordering of the following cp cache loads with bytecode load
> 2558:   __ membar(MacroAssembler::LoadLoad);
> 2559: 

I'm wondering if this can be in load_field_entry at the end so we don't miss any callers.  It might be a bit redundant with the ldar in the resolve_cache_and_index_for_field(), but that's for only the first time the field is resolved and in the interpreter, should not be an issue for performance.

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

PR Review: https://git.openjdk.org/jdk/pull/18477#pullrequestreview-1995535220
PR Review Comment: https://git.openjdk.org/jdk/pull/18477#discussion_r1561790322
PR Review Comment: https://git.openjdk.org/jdk/pull/18477#discussion_r1561787370