On Mon, 21 Oct 2024 13:53:58 GMT, Roman Kennke <rken...@openjdk.org> wrote:

>> Roman Kennke has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   Compact header riscv (#3)
>>   
>>   Implement compact headers on RISCV
>>   ---------
>>   
>>   Co-authored-by: hamlin <ham...@rivosinc.com>
>
>> 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.

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

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

Reply via email to