RFR: 8229352: Use of an unitialized register in 32-bit ARM template interpreter

[christoph.goettschkes at microdoc.com]

Hello,

could someone please review the changes for 
https://bugs.openjdk.java.net/browse/JDK-8229352

webrev: http://cr.openjdk.java.net/~bulasevich/8229352/webrev.00/

The template interpreter for 32-bit arm generates code which accesses the 
uninitialized register Rtemp if the platform is softfp for the ldc2_w 
bytecode.
Because of that, loading a long constant might take the slow path and call 
into the runtime. In addition, loading a double constant might take the 
wrong path and not call into the runtime, but load it as if it would be of 
type long (never seen in real life).

I executed the tier1 tests of the hotspot JTreg test group and could not 
identify any regressions. Some tests were failing because of insufficient 
memory on the target platform.

Thanks,
Christoph

"hotspot-dev" <hotspot-dev-bounces at openjdk.java.net> wrote on 2019-07-16 
12:05:59:

> From: christoph.goettschkes at microdoc.com
> To: hotspot-dev at openjdk.java.net
> Date: 2019-07-16 12:08
> Subject: [PATCH] Use of an unitialized register in 32-bit ARM template 
interpreter
> Sent by: "hotspot-dev" <hotspot-dev-bounces at openjdk.java.net>
> 
> Hello,
> 
> while working with the OpenJDK 11 on a 32-bit ARMv7-A platform, I 
noticed
> something weird in the template interpreter, regarding the template for
> the bytecode instruction ldc2_w.
> 
> The type check for the operand is only done correctly if the ABI is a
> hard-float one. For soft-float, the check is done wrong, using an
> uninitialized register Rtemp. Please see the following diff:
> 
> diff -r 327d5994b2fb src/hotspot/cpu/arm/templateTable_arm.cpp
> --- a/src/hotspot/cpu/arm/templateTable_arm.cpp   Tue Mar 12 11:13:39 
2019 -0400
> +++ b/src/hotspot/cpu/arm/templateTable_arm.cpp   Tue Jul 16 11:22:14 
2019 +0200
> @@ -515,36 +515,37 @@
>  void TemplateTable::ldc2_w() {
>    transition(vtos, vtos);
>    const Register Rtags  = R2_tmp;
>    const Register Rindex = R3_tmp;
>    const Register Rcpool = R4_tmp;
>    const Register Rbase  = R5_tmp;
> 
>    __ get_unsigned_2_byte_index_at_bcp(Rindex, 1);
> 
>    __ get_cpool_and_tags(Rcpool, Rtags);
>    const int base_offset = ConstantPool::header_size() * wordSize;
>    const int tags_offset = Array<u1>::base_offset_in_bytes();
> 
>    __ add(Rbase, Rcpool, AsmOperand(Rindex, lsl, LogBytesPerWord));
> 
> +  // get type from tags
> +  __ add(Rtemp, Rtags, tags_offset);
> +  __ ldrb(Rtemp, Address(Rtemp, Rindex));
> +
>    Label Condy, exit;
>  #ifdef __ABI_HARD__
>    Label Long;
> -  // get type from tags
> -  __ add(Rtemp, Rtags, tags_offset);
> -  __ ldrb(Rtemp, Address(Rtemp, Rindex));
>    __ cmp(Rtemp, JVM_CONSTANT_Double);
>    __ b(Long, ne);
>    __ ldr_double(D0_tos, Address(Rbase, base_offset));
> 
>    __ push(dtos);
>    __ b(exit);
>    __ bind(Long);
>  #endif
> 
>    __ cmp(Rtemp, JVM_CONSTANT_Long);
>    __ b(Condy, ne);
>  #ifdef AARCH64
>    __ ldr(R0_tos, Address(Rbase, base_offset));
>  #else
>    __ ldr(R0_tos_lo, Address(Rbase, base_offset + 0 * wordSize));
> 
> If the check for the type of the operand is done correctly, the call to
> InterpreterRuntime::resolve_ldc should never happen. Currently, for 
32-bit
> soft-float arm, InterpreterRuntime::resolve_ldc is called if the operand 

> for ldc2_w is of type long.
> 
> Also, I find it weird that the "condy_helper" code is genarted for the
> ldc2_w bytecode instruction on 32-bit hard-float arm (and also on x86).
> Aren't the only two valid types for ldc2_w long and double?
> 
> -- Christoph
>