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

[dean.long at oracle.com]

Looks OK, but after the Condy change, the label "Long" might be better 
named "NotDouble".

dl

On 8/10/19 11:54 PM, christoph.goettschkes at microdoc.com wrote:
> 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
>>