JDK-8233364 : Fix undefined behavior in Canonicalizer::do_ShiftOp
  • Type: Bug
  • Component: hotspot
  • Sub-Component: compiler
  • Affected Version: 8,11,14
  • Priority: P4
  • Status: Resolved
  • Resolution: Fixed
  • Submitted: 2019-11-01
  • Updated: 2024-10-16
  • Resolved: 2019-11-07
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 11 JDK 13 JDK 14 JDK 7 JDK 8 Other
11.0.8-oracleFixed 13.0.4Fixed 14 b23Fixed 7u441Fixed 8u431Fixed openjdk8u432Fixed
Related Reports
Relates :  
Relates :  
Description
In do_ShiftOp, when both arguments are constants, it computes the result at compile-time, being careful to maintain Java semantics.  However, there are several problems with that computation.  There is a mask whose computation invokes undefined behavior.

The code is

jint mask = ~(~0 << (32 - shift));
if (shift == 0) mask = ~0;

The first problem is that the left shift of ~0 is a left shift of a negative number. C89 and C++98 don't explicitly specify the behavior, though one might infer there could be difficulties due to the different representations that are permitted. C99 and C++11 both explicitly specify this to be undefined. gcc warns about that shift when using -Wshift-negative-value (enabled by -Wextra) when compiling for C99 or C++11.  This isn't a problem now, but will be for JEP 347.

The second problem exists today.  If shift == 0, which is possible and is checked for by the second line, the shift quantity is 32, which is undefined behavior in any version of C/C++ when the left hand side has a size of 32 bits (or less).  This means that a sufficiently aggressive compiler might elide the second line entirely, since the test can't be true without having already invoked undefined behavior.

There is a third problem a couple lines later with this line:

case Bytecodes::_ishl:  set_constant(value << shift); return;

If value is negative, this is again a shift of a negative value. The C++ compiler probably isn't going to detect or be affected by this, but a runtime sanitizer could complain.

There is a second occurrence of essentially the same code with the same problems a few lines later for the jlong case.

Comments
[jdk8u-fix-request] Approval Request from Martin Balao Alonso Having this fix in 8u will be beneficial for parity between JDKs but, most importantly, will serve as a dependency for bug 8328544.
16-09-2024

A pull request was submitted for review. Branch: master URL: https://git.openjdk.org/jdk8u-dev/pull/577 Date: 2024-09-12 04:24:35 +0000
12-09-2024

Fix request (13u): Requesting backport to 13u for parity with 11u, the original change applies cleanly.
09-06-2020

jdk11 backport request I would like to have the patch in OpenJDK11 as well (for better parity with 11.0.8_oracle). The patch applies cleanly.
18-05-2020

URL: https://hg.openjdk.java.net/jdk/jdk/rev/291775bcf35d User: kbarrett Date: 2019-11-07 21:24:01 +0000
07-11-2019

ILW = Undefined behavior in compiler code, no known issues, no workaround = MLH = P4
04-11-2019