RFR: 8332368: ubsan aarch64: immediate_aarch64.cpp:298:31: runtime error: shift exponent 32 is too large for 32-bit type 'int'

[Boris Ulasevich]

Running a linux-aarch64-server-fastdebug build with UBSAN (--enable-ubsan configure option) gives the following runtime error immediately on JVM start:


ad_aarch64.hpp:7114:11: runtime error: shift exponent 100 is too large for 32-bit type 'unsigned int'
# Pipeline_Use_Cycle_Mask::operator<<=(int) make/hotspot/ad_aarch64.hpp:7114
# Pipeline_Use_Element::step(unsigned int) make/hotspot/ad_aarch64.hpp:7168
# Pipeline_Use::step(unsigned int) make/hotspot/ad_aarch64.hpp:7216
# Scheduling::step(unsigned int) src/hotspot/share/opto/output.cpp:2116
# Scheduling::AddNodeToBundle(Node*, Block const*) src/hotspot/share/opto/output.cpp:2553


The value of 100 comes from fixed_latency, defined in AD files:

// Pipeline class for call.
pipe_class pipe_class_call()
%{
  single_instruction;
  fixed_latency(100);
%}


The fixed_latency value is used by the scheduler to model the occupancy of functional units over time. The occupancy is tracked using a uint mask value:

  fprintf(fp_hpp, "class Pipeline_Use_Element {\n");
  fprintf(fp_hpp, "protected:\n");
  fprintf(fp_hpp, "  // Mask of used functional units\n");
  fprintf(fp_hpp, "  uint _used;\n\n");

When the scheduler virtually steps over the instruction, it shifts the masks left by the instruction's latency. The problem is that 100 is greater than sizeof(uint), and left-shifting by 100 effectively zeroes the _mask, but, according to the C++ standard, this is undefined behavior.

We can find a number of fixed_latency(100) expressions in aarch64.ad, arm.ad, ppc.ad, riscv.ad, x86_64.ad files. Perhaps all of them deserve correction. I suggest leaving the AD files as they are, but limiting the shift value in case it exceeds the allowed maximum in a generated code:

 void step(uint cycles) {
   _used = 0;
-  _mask <<= cycles;
+  uint max_shift = 8 * sizeof(_mask) - 1;
+  _mask <<= (cycles < max_shift) ? cycles : max_shift;
 }

In fact, this change does not affect the current behavior; we just eliminate the undefined behavior while preserving the intended semantics.

-------------

Commit messages:
 - 8332368: ubsan aarch64: immediate_aarch64.cpp:298:31: runtime error: shift exponent 32 is too large for 32-bit type 'int'

Changes: https://git.openjdk.org/jdk/pull/24696/files
  Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=24696&range=00
  Issue: https://bugs.openjdk.org/browse/JDK-8332368
  Stats: 2 lines in 1 file changed: 1 ins; 0 del; 1 mod
  Patch: https://git.openjdk.org/jdk/pull/24696.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/24696/head:pull/24696

PR: https://git.openjdk.org/jdk/pull/24696