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

[SUN Guoyun]

On Fri, 26 Apr 2024 21:10:00 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. Verified with tier 1-5 tests.
>
> Matias Saavedra Silva has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Improved comment

src/hotspot/cpu/aarch64/interp_masm_aarch64.cpp line 1787:

> 1785:   lea(cache, Address(cache, index));
> 1786:   // Prevents stale data from being read after the bytecode is patched to the fast bytecode
> 1787:   membar(MacroAssembler::LoadLoad);

if we put LoadLoad in here, maybe it is redundant for TemplateTable::patch_bytecode(..)
https://github.com/openjdk/jdk/pull/18477/files#diff-739c969c24180bfe592cd0a75940d3838503390d3f51c8362a56ec4903252b67R192

because there has a Load-acquire in L199.
can we change 
`load_field_entry(Register cache, Register index, int bcp_offset = 1) ` 
to 
`load_field_entry(Register cache, Register index, int bcp_offset = 1, bool needLoadLoad = 1) `?

then change L192 like this
<pre>
@@ -189,7 +189,7 @@ void TemplateTable::patch_bytecode(Bytecodes::Code bc, Register bc_reg,
       // additional, required work.
       assert(byte_no == f1_byte || byte_no == f2_byte, "byte_no out of range");
       assert(load_bc_into_bc_reg, "we use bc_reg as temp");
-      __ load_field_entry(temp_reg, bc_reg);
+      __ load_field_entry(temp_reg, bc_reg, 1 /*bcp_offset*/, false /*needLoadLoad*/);
       if (byte_no == f1_byte) {
         __ lea(temp_reg, Address(temp_reg, in_bytes(ResolvedFieldEntry::get_code_offset())));
       } else {
         __ lea(temp_reg, Address(temp_reg, in_bytes(ResolvedFieldEntry::put_code_offset())));
       }                                                                         
       // Load-acquire the bytecode to match store-release in ResolvedFieldEntry::fill_in()
       __ ldarb(temp_reg, temp_reg);
</pre>

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

PR Review Comment: https://git.openjdk.org/jdk/pull/18477#discussion_r1582002700