RFR: 8305895: Implement JEP 450: Compact Object Headers (Experimental) [v46]

[Roman Kennke]

On Mon, 21 Oct 2024 18:00:14 GMT, Roman Kennke <rkennke at openjdk.org> wrote:

>>> I've managed to reproduce the ECoreIndexOf crash locally by running with -XX:+UseSerialGC -XX:+UnlockExperimentalVMOptions -XX:+UseCompactObjectHeaders. The crash happens on line 773 when reading past the needle.
>>> 
>>> ```
>>> │      762     __ movq(index, needle_len);
>>> │
>>> │      763     __ andq(index, 0xf);  // nLen % 16
>>> │      764     __ movq(offset, 0x10);
>>> │      765     __ subq(offset, index);  // 16 - (nLen % 16)
>>> │      766     __ movq(index, offset);
>>> │      767     __ shlq(offset, 1);  // * 2
>>> │      768     __ negq(index);      // -(16 - (nLen % 16))
>>> │
>>> │      769     __ xorq(wr_index, wr_index);
>>> │      770
>>> │      771     __ bind(L_top);
>>> │      772     // load needle and expand 
>>> │      773     __ vpmovzxbw(xmm0, Address(needle, index, Address::times_1), Assembler::AVX_256bit);
>>> ```
>>> 
>>> We're reading this address:
>>> 
>>> ```
>>> (SEGV_MAPERR), si_addr: 0x00000007cffffffe
>>> ```
>>> 
>>> which is just before the start of the heap:
>>> 
>>> ```
>>> Heap address: 0x00000007d0000000, size: 768 MB, Compressed Oops mode: Zero based, Oop shift amount: 3
>>> ```
>>> 
>>> When this crashed I had:
>>> 
>>> ```
>>> needle: 0x00000007d000000c
>>> needle_len = 0x12
>>> index = 0xfffffffffffffffe
>>> ```
>>> 
>>> There has been previous fix to not read past the haystack: Fix header < 16 bytes in indexOf intrinsic, by @sviswa7 [f65ef5d](https://github.com/openjdk/jdk/commit/f65ef5dc325212155a50a2fc3a7f4aad18b8d9d0)
>>> 
>>> maybe we need something similar for the needle.
>> 
>> @sviswa7 @vpaprotsk could you have a look? If we can have a reasonable fix for this soon, we could ship it in this PR, otherwise I'd defer it to a follow-up issue and disable indexOf intrinsic when running with +UseCompactObjectHeaders.
>
>> @rkennke Could you post the full command you used please? And perhaps also the seed that gets printed.. having trouble getting it to fail..
>> 
>> So far I added a few options and perrmitations of: `./build/linux-x86_64-server-fastdebug/images/jdk/bin/java -Xcomp -XX:-TieredCompilation -XX:+UnlockDiagnosticVMOptions -XX:+EnableX86ECoreOpts -XX:+UseSerialGC -XX:+UnlockExperimentalVMOptions -XX:+UseCompactObjectHeaders test/jdk/java/lang/StringBuffer/ECoreIndexOf.java` and lo luck.. IndexOf.java test checks "all interesting" lengths of haystack and needle and can't get it to fail either.
> 
> I could reproduce on 3rd try with a fastdebug build with:
> 
> make test TEST=java/lang/StringBuffer/ECoreIndexOf.java TEST_VM_OPTS="-XX:+UnlockExperimentalVMOptions -XX:+UseCompactObjectHeaders -XX:+UseSerialGC"
> 
> 
> It prints:
> 
> Seed set to 636754923980405411
> 
> 
> It probably depends on GC operation: It would only fail when the array happens to be the very first object in the heap. The relevant GC/heap configs would be:
> 
> InitialHeapSize                          = 805306368
> MaxHeapSize                              = 805306368
> MaxNewSize                               = 268435456
> 
> 
> So you should probably also add `-Xmx805306368 -Xms805306368 -Xmn268435456`

> > Thanks @rkennke able to reproduce now.. Sandhya will have a patch soon and I will re-verify
> 
> @rkennke @vpaprotsk Please find attached the patch which should fix the problem.
> 
> [smallneedlefix.patch](https://github.com/user-attachments/files/17466073/smallneedlefix.patch)

Testing now. Runs the reproducer in a loop since half an hour without crashing. I'll let it run overnight, and if @vpaprotsk approves the changes, then I'll intergrate them tomorrow morning.

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

PR Comment: https://git.openjdk.org/jdk/pull/20677#issuecomment-2427660618