Skip to content

8354668: Missing REX2 prefix accounting in ZGC barriers leads to incorrect encoding #24664

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 3 commits into from
Closed

8354668: Missing REX2 prefix accounting in ZGC barriers leads to incorrect encoding #24664

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
1a5a73c
8354668: Missing REX2 prefix accounting in ZGC barriers leads to inco…
jatin-bhateja Apr 15, 2025
ffd92c3
review comment resolutions
jatin-bhateja Apr 16, 2025
dc2b2b1
Review comments resolutions
jatin-bhateja Apr 17, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 7 additions & 7 deletions src/hotspot/cpu/x86/gc/z/zBarrierSetAssembler_x86.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -391,8 +391,8 @@ void ZBarrierSetAssembler::store_barrier_fast(MacroAssembler* masm,
if (rnew_zaddress != noreg) {
// noreg means null; no need to color
__ movptr(rnew_zpointer, rnew_zaddress);
__ relocate(barrier_Relocation::spec(), ZBarrierRelocationFormatLoadGoodBeforeShl);
__ shlq(rnew_zpointer, barrier_Relocation::unpatched);
__ relocate(barrier_Relocation::spec(), ZBarrierRelocationFormatLoadGoodAfterShX);
__ orq_imm32(rnew_zpointer, barrier_Relocation::unpatched);
__ relocate(barrier_Relocation::spec(), ZBarrierRelocationFormatStoreGoodAfterOr);
}
Expand Down Expand Up @@ -969,13 +969,13 @@ void ZBarrierSetAssembler::try_resolve_jobject_in_native(MacroAssembler* masm,
#define __ ce->masm()->

static void z_uncolor(LIR_Assembler* ce, LIR_Opr ref) {
__ relocate(barrier_Relocation::spec(), ZBarrierRelocationFormatLoadGoodBeforeShl);
__ shrq(ref->as_register(), barrier_Relocation::unpatched);
__ relocate(barrier_Relocation::spec(), ZBarrierRelocationFormatLoadGoodAfterShX);
}

static void z_color(LIR_Assembler* ce, LIR_Opr ref) {
__ relocate(barrier_Relocation::spec(), ZBarrierRelocationFormatLoadGoodBeforeShl);
__ shlq(ref->as_register(), barrier_Relocation::unpatched);
__ relocate(barrier_Relocation::spec(), ZBarrierRelocationFormatLoadGoodAfterShX);
__ orq_imm32(ref->as_register(), barrier_Relocation::unpatched);
__ relocate(barrier_Relocation::spec(), ZBarrierRelocationFormatStoreGoodAfterOr);
}
Expand Down Expand Up @@ -1278,8 +1278,8 @@ void ZBarrierSetAssembler::generate_c2_store_barrier_stub(MacroAssembler* masm,

static int patch_barrier_relocation_offset(int format) {
switch (format) {
case ZBarrierRelocationFormatLoadGoodBeforeShl:
return 3;
case ZBarrierRelocationFormatLoadGoodAfterShX:
return -1;

case ZBarrierRelocationFormatStoreGoodAfterCmp:
return -2;
Expand All @@ -1300,7 +1300,7 @@ static int patch_barrier_relocation_offset(int format) {

static uint16_t patch_barrier_relocation_value(int format) {
switch (format) {
case ZBarrierRelocationFormatLoadGoodBeforeShl:
case ZBarrierRelocationFormatLoadGoodAfterShX:
return (uint16_t)ZPointerLoadShift;

case ZBarrierRelocationFormatMarkBadAfterTest:
Expand All @@ -1327,7 +1327,7 @@ void ZBarrierSetAssembler::patch_barrier_relocation(address addr, int format) {
const int offset = patch_barrier_relocation_offset(format);
const uint16_t value = patch_barrier_relocation_value(format);
uint8_t* const patch_addr = (uint8_t*)addr + offset;
if (format == ZBarrierRelocationFormatLoadGoodBeforeShl) {
if (format == ZBarrierRelocationFormatLoadGoodAfterShX) {
*patch_addr = (uint8_t)value;
} else {
*(uint16_t*)patch_addr = value;
Expand Down
2 changes: 1 addition & 1 deletion src/hotspot/cpu/x86/gc/z/zBarrierSetAssembler_x86.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ class ZLoadBarrierStubC2;
class ZStoreBarrierStubC2;
#endif // COMPILER2

const int ZBarrierRelocationFormatLoadGoodBeforeShl = 0;
const int ZBarrierRelocationFormatLoadGoodAfterShX = 0;
const int ZBarrierRelocationFormatLoadBadAfterTest = 1;
const int ZBarrierRelocationFormatMarkBadAfterTest = 2;
const int ZBarrierRelocationFormatStoreGoodAfterCmp = 3;
Expand Down
4 changes: 2 additions & 2 deletions src/hotspot/cpu/x86/gc/z/z_x86_64.ad
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,15 +35,15 @@ source %{
#include "gc/z/zBarrierSetAssembler.hpp"

static void z_color(MacroAssembler* masm, const MachNode* node, Register ref) {
__ relocate(barrier_Relocation::spec(), ZBarrierRelocationFormatLoadGoodBeforeShl);
__ shlq(ref, barrier_Relocation::unpatched);
__ relocate(barrier_Relocation::spec(), ZBarrierRelocationFormatLoadGoodAfterShX);
__ orq_imm32(ref, barrier_Relocation::unpatched);
__ relocate(barrier_Relocation::spec(), ZBarrierRelocationFormatStoreGoodAfterOr);
}

static void z_uncolor(MacroAssembler* masm, const MachNode* node, Register ref) {
__ relocate(barrier_Relocation::spec(), ZBarrierRelocationFormatLoadGoodBeforeShl);
__ shrq(ref, barrier_Relocation::unpatched);
__ relocate(barrier_Relocation::spec(), ZBarrierRelocationFormatLoadGoodAfterShX);
}

static void z_keep_alive_load_barrier(MacroAssembler* masm, const MachNode* node, Address ref_addr, Register ref) {
Expand Down
2 changes: 1 addition & 1 deletion src/hotspot/cpu/x86/jvmciCodeInstaller_x86.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -221,7 +221,7 @@ bool CodeInstaller::pd_relocate(address pc, jint mark) {
return true;
#if INCLUDE_ZGC
case Z_BARRIER_RELOCATION_FORMAT_LOAD_GOOD_BEFORE_SHL:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should probably communicate with the JVMCI / Graal @dougxc so we can both update this exported symbol name to reflect the new behaviour, and give them the opportunity to adapt to the new relocation patching.

_instructions->relocate(pc, barrier_Relocation::spec(), ZBarrierRelocationFormatLoadGoodBeforeShl);
_instructions->relocate(pc, barrier_Relocation::spec(), ZBarrierRelocationFormatLoadGoodAfterShX);
return true;
case Z_BARRIER_RELOCATION_FORMAT_LOAD_BAD_AFTER_TEST:
_instructions->relocate(pc, barrier_Relocation::spec(), ZBarrierRelocationFormatLoadBadAfterTest);
Expand Down