RFR: 8369947: Bytecode rewriting causes Java heap corruption on RISC-V

[Martin Doerr]

On Fri, 17 Oct 2025 07:10:05 GMT, Aleksey Shipilev <shade at openjdk.org> wrote:

>> Hi, I am having some difficulty in understanding the issue.
>> @shipilev @theRealAph : For the aarch64 counterpart, shouldn't the `ldarb` at [1] prevent the reordering of `STR` of PBC and `STLR` of RFE? It's a load instruction with acquire semantics.
>> 
>> [1] https://github.com/openjdk/jdk/blob/master/src/hotspot/cpu/aarch64/templateTable_aarch64.cpp#L200
>
>> @shipilev @theRealAph : For the aarch64 counterpart, shouldn't the `ldarb` at [1] prevent the reordering of `STR` of PBC and `STLR` of RFE? It's a load instruction with acquire semantics.
> 
> Yes, I was confused about this myself. The key thing for this particular issue: the _reader_ we need to sync up with is not `patch_bytecode`, it is the thread that _executes_ the patched bytecode. In other words, the _writer_ is `patch_bytecode`, and _reader_ is executing thread. 
> 
> So acquire barrier in `patch_bytecode` does not help this case, because it is a write path, it needs release, which aarch64 fix did. The read path needs some other synchronization for acquire-like semantics; in aarch64 we reasoned the control dependency on bytecode itself and the barrier in RFE resolution is already enough to do this. See my writeup here: https://bugs.openjdk.org/browse/JDK-8369506?focusedId=14824157#comment-14824157 -- and the comments after it.

> > > @shipilev @theRealAph : For the aarch64 counterpart, shouldn't the `ldarb` at [1] prevent the reordering of `STR` of PBC and `STLR` of RFE? It's a load instruction with acquire semantics.
> > 
> > 
> > Yes, I was confused about this myself. The key thing for this particular issue: the _reader_ we need to sync up with is not `patch_bytecode`, it is the thread that _executes_ the patched bytecode. In other words, the _writer_ is `patch_bytecode`, and _reader_ is executing thread.
> > So acquire barrier in `patch_bytecode` does not help this case, because it is a write path, it needs release, which aarch64 fix did. The read path needs some other synchronization for acquire-like semantics; in aarch64 we reasoned the control dependency on bytecode itself and the barrier in RFE resolution is already enough to do this.
> 
> RISCV is good on the read side, we just need this patch to fix the write:
> 
> ```
> void InterpreterMacroAssembler::load_field_entry(Register cache, Register index, int bcp_offset) {
> ...
>   // Get address of field entries array
>   ld(cache, Address(xcpool, ConstantPoolCache::field_entries_offset()));
>   addi(cache, cache, Array<ResolvedIndyEntry>::base_offset_in_bytes());
>   add(cache, cache, index);
>   // Prevents stale data from being read after the bytecode is patched to the fast bytecode
>   membar(MacroAssembler::LoadLoad);
> }
> ```

I just made a similar proposal for PPC64. But, I only use a barrier for fast Bytecodes:
https://github.com/TheRealMDoerr/jdk/blob/138df669209ae58676e0559cf825d0a0cc81ee1b/src/hotspot/cpu/ppc/interp_masm_ppc_64.cpp#L491

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

PR Comment: https://git.openjdk.org/jdk/pull/27850#issuecomment-3415352767