RFR: 8355364: [REDO] Missing REX2 prefix accounting in ZGC barriers leads to incorrect encoding

[Jatin Bhateja]

On Mon, 28 Apr 2025 12:28:55 GMT, Jatin Bhateja <jbhateja at openjdk.org> wrote:

> This is a follow-up PR that fixes the crashes seen after the integration of PR #24664
> 
> ZGC bookkeeps multiple place holders in barrier code snippets through relocations, these are later used to patch appropriate contents (mostly immediate values) in instruction encoding to save costly comparisons against global state [1]. While most of the relocation records the patching offsets from the end of the instruction, SHL/R instructions used for pointer coloring/uncoloring, compute the patching offset from the starting address of the instruction. This was done to prevent accidental sharing of relocation information with subsequent relocatable instructions, e.g., static call. [2]
> 
> In case the destination register operand of SHL/R instruction is an extended GPR register, we miss accounting additional REX2 prefix byte in the patch offset, thereby corrupting the encoding since runtime patches the primary opcode byte, resulting in an ILLEGAL instruction exception.
> 
> This patch fixes reported failures by computing the relocation offset of the SHL/R instruction from the end of the instruction, thereby making the patch offset agnostic to the REX/REX2 prefix. To be safe, we emit a NOP instruction between the SHL/R and the subsequent relocatable instruction.
> 
> Please review and share your feedback.
> 
> Best Regards,
> Jatin
> 
> [1] https://openjdk.org/jeps/439#:~:text=we%20reduce%20this,changes%20phase%3B
> [2] https://github.com/openjdk/jdk/blob/master/src/hotspot/cpu/x86/x86_64.ad#L1873
> 
> 
> PS: Validations were performed using the latest Intel Software Development Emulator after modifying the static register allocation order in x86_64.ad file giving preference to EGPRs.

Please refer to following comments in relocInfo, which warns against recording relocation against exact patch site as it may pose problems in querying / iterating over relocations corresponding to particular instruction starting address.
https://github.com/openjdk/jdk/blob/master/src/hotspot/share/code/relocInfo.hpp#L85

@TobiHartmann confirmed that the patch fixed crashes.
https://bugs.openjdk.org/browse/JDK-8355363#:~:text=Sounds%20reasonable%2C%20maybe%20mention%20that%20in%20the%20PR%20as%20well.%20All%20testing%20passed.

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

PR Comment: https://git.openjdk.org/jdk/pull/24919#issuecomment-2839879447