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
