RFR: 8263425: AArch64: two potential bugs in C1 LIRGenerator::generate_address()

[Andrew Haley]

On Fri, 12 Mar 2021 08:24:19 GMT, Nick Gasson <ngasson at openjdk.org> wrote:

> Around line 177 we have:
> 
>   LIR_Opr tmp = new_pointer_register();
>   if (Assembler::operand_valid_for_add_sub_immediate(large_disp)) {
>     __ add(tmp, tmp, LIR_OprFact::intptrConst(large_disp));    <----
>     index = tmp;
>    } else {
> 
> This is supposed to be calculating "tmp = index + large_disp" where tmp
> is a freshly allocated register but it actually does "large_disp = tmp +
> tmp".
> 
> On line 155 we check if index is a constant and if so accumulate its
> value into large_disp. Then on line 194:
> 
>   // at this point we either have base + index or base + displacement
>   if (large_disp == 0) {
>     return new LIR_Address(base, index, type);      <----
>   } else {
> 
> LIR_Address::verify() asserts that index is either a register or an
> illegal value, but if the displacement and constant index sum to zero we
> pass a constant in here.
> 
> C1 in mainline JDK doesn't hit these code paths, but I don't see any
> reason why it couldn't in the future, and indeed it does on the lworld
> branch. Tested tier1 on AArch64 with TieredStopAtLevel=1.

Oops. Thank you.

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

Marked as reviewed by aph (Reviewer).

PR: https://git.openjdk.java.net/jdk/pull/2961