[15] RFR(XXS): 7107012: sun.jvm.hostspot.code.CompressedReadStream readDouble() conversion to long mishandled

[Chris Plummer]

Thanks Serguei!

Can I get one more reviewer please? The change is very simple.

thanks,

Chris

On 6/26/20 5:51 PM, serguei.spitsyn at oracle.com wrote:
> Hi Chris,
>
> The fix looks good.
> I would most likely overlook such a bug with my eyes. :)
>
>
> Thanks,
> Serguei
>
>
> On 6/26/20 16:03, Chris Plummer wrote:
>> Hello,
>>
>> Please help review the following:
>>
>> http://cr.openjdk.java.net/~cjplummer/7107012/webrev.00/index.html
>> https://bugs.openjdk.java.net/browse/JDK-7107012
>>
>> This bug is filed as confidential, although the issue is trivial. In 
>> the following line of code:
>>
>>     return Double.longBitsToDouble((h << 32) | ((long)l & 
>> 0x00000000FFFFFFFFL));
>>
>> Since h is an int, it's subject to the following:
>>
>> https://docs.oracle.com/javase/specs/jls/se14/html/jls-15.html#jls-15.19
>>
>> "If the promoted type of the left-hand operand is int, then only the 
>> five lowest-order bits of the right-hand operand are used as the 
>> shift distance. It is as if the right-hand operand were subjected to 
>> a bitwise logical AND operator & (§15.22.1) with the mask value 0x1f 
>> (0b11111). The shift distance actually used is therefore always in 
>> the range 0 to 31, inclusive."
>>
>> So (h << 32) is the same as (h << 0), which is not what was intended. 
>> The spec also calls out another issue:
>>
>> "The type of the shift expression is the promoted type of the 
>> left-hand operand."
>>
>> So even if it did left shift 32 bits, the result would have been 
>> truncated to an int, meaning the result would always be 0. The fix is 
>> to first cast h to a long. Doing this addresses both these problems, 
>> allowing a full 32 bit left shift to be done, and leaving the result 
>> as an untruncated long.
>>
>> I was unable to trigger use of this code in SA. It seems to be used 
>> to pull locals out of a CompiledVFrame. I don't see any clhsdb paths 
>> to this code. It appears the GUI hsdb uses it via a complex call path 
>> I could not fully decipher, but I could not trigger its use from 
>> hsdb. In any case, the fix is straight forward and trivial, so I'd 
>> rather not have to spend more time digging deeper into its use and 
>> providing a test case.
>>
>> thanks,
>>
>> Chris
>>
>