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

[serguei.spitsyn at oracle.com]

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
>