JDK-8307197 : Signed overflow in multiply_high_signed()
  • Type: Bug
  • Component: hotspot
  • Sub-Component: compiler
  • Affected Version: 21
  • Priority: P4
  • Status: Closed
  • Resolution: Duplicate
  • Submitted: 2023-05-01
  • Updated: 2023-05-03
  • Resolved: 2023-05-03
The Version table provides details related to the release that this issue/RFE will be addressed.

Unresolved : Release in which this issue/RFE will be addressed.
Resolved: Release in which this issue/RFE has been resolved.
Fixed : Release in which this issue/RFE has been fixed. The release containing this fix may be available for download as an Early Access Release or a General Availability Release.

To download the current JDK release, click here.
JDK 21
21Resolved
Related Reports
Duplicate :  
JDK-8307139 - Fix signed integer overflow in compiler code, part 1
Relates :  
JDK-8307230 - Fix signed overflow issues found with -ftrapv
Relates :  
JDK-8299546 - C2: MulLNode::mul_ring() wrongly returns bottom type due to casting errors with large numbers
Description
I ran into an overflow with multiply (running with -ftrapv) but it looks like the add can also overflow.  We could fix this with java_multiply() and java_add(), but it might make the code more readable if we use operator overloading.
Comments
I'm closing this as dup of JDK-8307139 as the PR for it already fixes multiply_high_signed().
03-05-2023
I could reproduce this when building with -ftrapv. Seems like this multiplication is the problem, for example with x = y = -1 = 0xFF...F, it overflows: const uint64_t z2 = x2 * y2; But we actually want to have an unsigned multiplication here. So, I think we should just first cast x2 and y2 to uint64_t. Since x2 and y2 are clamped to 32-bit values, the multiplication should not overflow uint64_t. The other operations seem to be safe. I've actually hit more crashes with this GCC flag when simply running a HelloWorld program with -Xcomp. We could now either fix multiply_high_signed() separately or have a look at all of the other found issues with -ftrapv as well and fix them together with this one here. Since JDK-8299546 is new code it might make sense to fix this now. I filed another sub-task to clean everything else found with -ftrapv together (JDK-8307230). About using multiply_high_unsigned() for the signed case, this does not work unfortunately. For example, when multiplying -1L with 1L, we get -1L. The high part would be -1L in that case. But when doing the same with unsigned, we interpret -1 as uint64_t (i.e. 0xFFFF FFFF FFFF FFFF) and multiply it with 1. The result is 1 (i.e. 0x0000 0000 0000 0001) and the high part is 0: jshell> Math.unsignedMultiplyHigh(-1L, 1L) $1 ==> 0 jshell> Math.multiplyHigh(-1L, 1L) $2 ==> -1
02-05-2023
ILW = Undefined behavior in multiply_high_signed, no crash/wrong execution observed, no workaround = MLH = P4
02-05-2023
Is multiply_high_signed(x, y) equivalent to (int64_t)multiply_high_unsigned((uint64_t)x, (uint64_t)y)?
02-05-2023