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

[Andrew Dinn]

On Sun, 13 Jul 2025 08:40:45 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
>>    0xffff25c...
>
> Yadong Wang has updated the pull request incrementally with one additional commit since the last revision:
> 
>   8361892: AArch64: Incorrect matching rule leading to improper oop instruction encoding

The proposed solution is not simply going to work when the Leyden project introduces code save/restore. In the assembly phase for an AOT cache (i.e when compiling code to store in the cache) we need to recognize that an incoming address is the byte map base address and generate an lea with an external address relocation. So, the current code in premain relies on matching against immByteMapBase.

I believe we can retain the current approach if we make the immByteMapBase predicate test the operand type for a RawPtr rather than an OopPtr. That is actually the key distinction that separates the cases we are dealing with. I believe it would work for both immByteMapBase and immPollPage and is a much smaller change to the status quo.

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

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