On Mon, 21 Oct 2024 18:00:14 GMT, Roman Kennke <rken...@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
