[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-serviceability-dev
Subject:    Re: [15] RFR(XXS): 7107012: sun.jvm.hostspot.code.CompressedReadStream readDouble() conversion to lo
From:       "Daniel D. Daugherty" <daniel.daugherty () oracle ! com>
Date:       2020-06-29 19:50:38
Message-ID: 0ac1f9fa-7ac3-27c7-3794-b27f307323cb () oracle ! com
[Download RAW message or body]

Hi Chris,

This one caught my eye just because it was such an old bug number... :-)


On 6/26/20 7:03 PM, Chris Plummer wrote:
> Hello,
>
> Please help review the following:
>
> http://cr.openjdk.java.net/~cjplummer/7107012/webrev.00/index.html

src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/code/CompressedReadStream.java
     Nice catch!

Thumbs up.

Dan


> 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
>

[prev in list] [next in list] [prev in thread] [next in thread]