RFR 8229382 [lworld][c1] TestLWorld.java crashes with -XX:+UseSerialGC

[Tobias Hartmann]

Hi Ioi,

good catch! Looks good to me.

Thanks,
Tobias

On 13.08.19 05:50, Ioi Lam wrote:
> https://bugs.openjdk.java.net/browse/JDK-8229382
> http://cr.openjdk.java.net/~iklam/valhalla/8229382-c1-dont-cache-constants-in-conditional-code.v01/
> 
> Background
> ----------
> 
> When c1_LIRGenerator.cpp translates the HIR of a basic block to LIR, and it
> loads a constant C into a virtual register R, it assumes that this must happen
> for all possible paths through this basic block. Thus, it uses a cache
> of {constant} -> {register} to avoid loading the same constant C again
> in the same basic block. See LIRGenerator::load_constant().
> 
> However, when translating aastore, we end up having code like this:
> 
>  0x7f46c544a260: test $0x1,%bl            ; is this array flattened?
>  0x7f46c544a263: jne 0x7f46c544a376       ; call store_flattened_array
>  ....
>  0x7f46c544a28b: mov $0x7f03335dc000,%rsi ; card_table_base
>  0x7f46c544a295: movb $0x0,(%rax,%rsi,1)  ; mark card table
> 
> so the constant card_table_base is stored into %rsi only if the array
> is not flattened.
> 
> When a subsequent aastore is translated, we mistakenly assume that
> %rsi already contains the desired value. The code will crash when
> using %rsi to mark the card table.
> 
> 
> Fix
> ---
> 
> The fix is to avoid caching constants used in conditionally executed LIR.
> 
> I also added assertions so we don't call LIRItem::load_item() while
> generating conditionally executed LIR. Otherwise we will create
> {Value} -> {LIR_Opr} mappings (i.e., from HIR values to LIR virtual
> registers) that are not always valid.
> 
> I looked at every line of "__ branch_destination" added by the
> valhalla code and hopefully have caught all the bugs introduced by me.
> 
> Thanks
> - Ioi
>