RFR: 8361892: AArch64: Incorrect matching rule leading to improper oop instruction encoding

[Aleksey Shipilev]

On Thu, 10 Jul 2025 18:29:14 GMT, Yadong Wang <yadongwang at openjdk.org> wrote:

> The bug is that the predicate rule of immByteMapBase would cause a ConP Node for oop incorrect matching with byte_map_base when the placeholder jni handle address was just allocated to the address of byte_map_base.
> 
> C2 uses JNI handles as placeholders to encoding constant oops, and one of some handle maybe locate at the address of byte_map_base, which is not memory reserved by CardTable. It's possible because JNIHandleBlocks are allocated by malloc.
> 
>   // The assembler store_check code will do an unsigned shift of the oop,
>   // then add it to _byte_map_base, i.e.
>   //
>   //   _byte_map = _byte_map_base + (uintptr_t(low_bound) >> card_shift)
>   _byte_map = (CardValue*) rs.base();
>   _byte_map_base = _byte_map - (uintptr_t(low_bound) >> _card_shift);
> 
> In aarch64 port, C2 will incorrectly match ConP for oop to ConP for byte_map_base by the immByteMapBase operand.
> 
> // Card Table Byte Map Base
> operand immByteMapBase()
> %{
>   // Get base of card map
>   predicate((jbyte*)n->get_ptr() ==
>         ((CardTableModRefBS*)(Universe::heap()->barrier_set()))->byte_map_base);
>   match(ConP);
> 
>   op_cost(0);
>   format %{ %}
>   interface(CONST_INTER);
> %}
> 
> // Load Byte Map Base Constant
> instruct loadByteMapBase(iRegPNoSp dst, immByteMapBase con)
> %{
>   match(Set dst con);
> 
>   ins_cost(INSN_COST);
>   format %{ "adr  $dst, $con\t# Byte Map Base" %}
> 
>   ins_encode(aarch64_enc_mov_byte_map_base(dst, con));
> 
>   ins_pipe(ialu_imm);
> %}
> 
> As below, a typical incorrect instructions generated by C2 for java.lang.ref.Finalizer.register(Ljava/lang/Object;)V (10 bytes) @ 0x0000ffff25caf0bc [0x0000ffff25caee80+0x23c], where 0xffff21730000 is the byte_map_base address mistakenly used as an object address:
>    0xffff25caf08c:	ldaxr	x8, [x11]
>    0xffff25caf090:	cmp	x10, x8
>    0xffff25caf094:	b.ne	0xffff25caf0a0  // b.any
>    0xffff25caf098:	stlxr	w8, x28, [x11]
>    0xffff25caf09c:	cbnz	w8, 0xffff25caf08c
>    0xffff25caf0a0:	orr	x11, xzr, #0x3
>    0xffff25caf0a4:	str	x11, [x13]
>    0xffff25caf0a8:	b.eq	0xffff25caef80  // b.none
>    0xffff25caf0ac:	str	x14, [sp]
>    0xffff25caf0b0:	add	x2, sp, #0x20
>    0xffff25caf0b4:	adrp	x1, 0xffff21730000
>    0xffff25caf0b8:	bl	0xffff256fffc0
>    0xffff25caf0bc:	ldr	x14, [sp]
>    0xffff25caf0c0:	b	0xffff25caef80
>    0xffff25caf0c4:	add	x13, sp, #0x20
>    0xffff25caf0c8:	adrp	x12, 0xffff21730000
>    0xffff25caf0cc:	ldr	x10, [x13]
>    0xffff25caf0d0:	cmp	x10, xzr
>    0xffff25caf0d4:	b.eq	0xffff25caf130  // b.none
>    0xffff25caf0d8:	ldr	x11, [x12]
>    0xffff25caf0dc:	tbnz	w10, #1, 0xffff25caf0f...

It is such a beautiful bug to read about on Friday.

So the net effect of this mismatch is that we miss oop relocation/record when `ConP` accidentally mismatches to card table base, did I get that right?

> Yes, it maybe a better solution for jdk main line, because immPollPage was remove in https://bugs.openjdk.org/browse/JDK-8220051. But how about jdk8u backport?

I think we should do these things separately:
  1. `immByteMapBase` rule removal in AArch64, this PR, then backport it to 25, 21, 17, maybe to 11, 8
  2. `immByteMapBase` rule removal in RISC-V, separate PR, then backport it to 25, 21
  3. `immPollPage` rule removal in AArch64, in 11u and 8u specific PRs

The backports for (1) would not be clean, as Generational Shenandoah barrier checks would likely trigger technical conflicts in the code that is being removed. So there is doubly no point in going for clean backports, we should really slice them by the rule we are removing.

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

PR Comment: https://git.openjdk.org/jdk/pull/26249#issuecomment-3062188862