[vm/compiler] Fix dart fuzzer CalculateElementAddress crash on X64C.

In the backend, handle the following cases that were assumed not to
happen before:
* If the index and offset are both 0, then the operation is a no-op
  and so the output register should be the same as the first input.
  (Should only happen if the instruction is used in a non-optimizing
  context, as otherwise it is removed by canonicalization.)
* If the scaled index can be used as an instruction immediate and the
  offset is 0, then emit the appropriate instruction(s).
* If the scaled index and offset can both be used as immediates to
  instructions, but their sum (the total offset in bytes) cannot, then
  allocate a register for the index and fall back to the non-constant
  index case.

CalculateElementAddress::Canonicalize now only performs removal of
no-op instructions.

This CL also fixes a switch on the instruction tag in
FlowGraph::RenameRecursive to appropriately convert UnboxedConstant
instructions to initial definitions of the FlowGraph as it already does
for Constant instructions.

TEST=vm/dart/regress_55877

Issue: https://github.com/dart-lang/sdk/issues/55877
Cq-Include-Trybots: luci.dart.try:vm-aot-linux-debug-simarm_x64-try,vm-aot-linux-debug-simriscv64-try,vm-aot-linux-debug-x64-try,vm-aot-linux-debug-x64c-try,vm-linux-debug-ia32-try,vm-linux-debug-x64-try,vm-linux-debug-x64c-try,vm-linux-debug-simriscv64-try,vm-mac-debug-arm64-try,vm-aot-mac-release-arm64-try
Change-Id: I613d6c8770fe02facf6bbdb3d2b11f842b51540d
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/369642
Commit-Queue: Tess Strickland <sstrickl@google.com>
Reviewed-by: Ryan Macnak <rmacnak@google.com>

runtime/tests/vm/dart/regress_55877_test.dart

@@ -0,0 +1,29 @@
+// Copyright (c) 2024, the Dart project authors.  Please see the AUTHORS file
+// for details. All rights reserved. Use of this source code is governed by a
+// BSD-style license that can be found in the LICENSE file.
+
+// Regression test for https://github.com/dart-lang/sdk/issues/55877.
+
+// VMOptions=--optimization_level=3
+
+import 'dart:typed_data';
+
+Uint8List var7 = Uint8List(37);
+Int32List? var16 = Int32List(2);
+
+@pragma("vm:entry-point")
+foo(int par3) {
+  switch (par3) {
+    case 308366437:
+      var16 = Int32List(2).sublist(par3, 8);
+      break;
+  }
+}
+
+main() {
+  try {
+    foo(var7[-9223372032559808513]);
+  } catch (e, st) {
+    print('X2() throws');
+  }
+}

runtime/vm/compiler/backend/flow_graph.cc

@@ -1592,10 +1592,11 @@ void FlowGraph::RenameRecursive(
         break;
       }
 
-      case Instruction::kConstant: {
+      case Instruction::kConstant:
+      case Instruction::kUnboxedConstant: {
         ConstantInstr* constant = current->Cast<ConstantInstr>();
         if (constant->HasTemp()) {
-          result = GetConstant(constant->value());
+          result = GetConstant(constant->value(), constant->representation());
         }
         break;
       }

runtime/vm/compiler/backend/il.cc

@@ -3785,26 +3785,8 @@ Definition* EqualityCompareInstr::Canonicalize(FlowGraph* flow_graph) {
 }
 
 Definition* CalculateElementAddressInstr::Canonicalize(FlowGraph* flow_graph) {
-  if (index()->BindsToSmiConstant() && offset()->BindsToSmiConstant()) {
-    const intptr_t offset_in_bytes = Utils::AddWithWrapAround(
-        Utils::MulWithWrapAround<intptr_t>(index()->BoundSmiConstant(),
-                                           index_scale()),
-        offset()->BoundSmiConstant());
-
-    if (offset_in_bytes == 0) return base()->definition();
-
-    if (compiler::target::IsSmi(offset_in_bytes)) {
-      auto* const Z = flow_graph->zone();
-      auto* const new_adjust = new (Z) CalculateElementAddressInstr(
-          base()->CopyWithType(Z),
-          new (Z) Value(
-              flow_graph->GetConstant(Object::smi_zero(), kUnboxedIntPtr)),
-          /*index_scale=*/1,
-          new (Z) Value(flow_graph->GetConstant(
-              Smi::Handle(Smi::New(offset_in_bytes)), kUnboxedIntPtr)));
-      flow_graph->InsertBefore(this, new_adjust, env(), FlowGraph::kValue);
-      return new_adjust;
-    }
+  if (IsNoop()) {
+    return base()->definition();
   }
   return this;
 }

runtime/vm/compiler/backend/il.h

@@ -8038,6 +8038,11 @@ class CalculateElementAddressInstr : public TemplateDefinition<3, NoThrow> {
 #undef FIELD_LIST
 
  private:
+  bool IsNoop() const {
+    return index()->BindsToSmiConstant() && index()->BoundSmiConstant() == 0 &&
+           offset()->BindsToSmiConstant() && offset()->BoundSmiConstant() == 0;
+  }
+
   DISALLOW_COPY_AND_ASSIGN(CalculateElementAddressInstr);
 };
 

runtime/vm/compiler/backend/il_arm.cc

@@ -458,7 +458,6 @@ LocationSummary* CalculateElementAddressInstr::MakeLocationSummary(
       LocationSummary(zone, kNumInputs, kNumTemps, LocationSummary::kNoCall);
 
   summary->set_in(kBasePos, Location::RequiresRegister());
-  summary->set_in(kIndexPos, Location::RequiresRegister());
   // Only use a Smi constant for the index if multiplying it by the index
   // scale would be an int32 constant.
   const intptr_t scale_shift = Utils::ShiftForPowerOfTwo(index_scale());
@@ -466,7 +465,25 @@ LocationSummary* CalculateElementAddressInstr::MakeLocationSummary(
                                  index(), kMinInt32 >> scale_shift,
                                  kMaxInt32 >> scale_shift));
   summary->set_in(kOffsetPos, LocationRegisterOrSmiConstant(offset()));
-  summary->set_out(0, Location::RequiresRegister());
+  // Special case for when both inputs are appropriate constants.
+  if (summary->in(kIndexPos).IsConstant() &&
+      summary->in(kOffsetPos).IsConstant()) {
+    const int64_t offset_in_bytes = Utils::AddWithWrapAround<int64_t>(
+        Utils::MulWithWrapAround<int64_t>(index()->BoundSmiConstant(),
+                                          index_scale()),
+        offset()->BoundSmiConstant());
+    if (!Utils::IsInt(32, offset_in_bytes)) {
+      // The offset in bytes calculated from the index and offset cannot
+      // fit in a 32-bit immediate, so pass the index as a register instead.
+      summary->set_in(kIndexPos, Location::RequiresRegister());
+    }
+  }
+
+  if (IsNoop()) {
+    summary->set_out(0, Location::SameAsFirstInput());
+  } else {
+    summary->set_out(0, Location::RequiresRegister());
+  }
 
   return summary;
 }
@@ -477,19 +494,22 @@ void CalculateElementAddressInstr::EmitNativeCode(FlowGraphCompiler* compiler) {
   const Location& offset_loc = locs()->in(kOffsetPos);
   const Register result_reg = locs()->out(0).reg();
 
+  if (IsNoop()) {
+    ASSERT_EQUAL(base_reg, result_reg);
+    return;
+  }
+
   if (index_loc.IsConstant()) {
+    const intptr_t scaled_index =
+        Smi::Cast(index_loc.constant()).Value() * index_scale();
     if (offset_loc.IsConstant()) {
-      ASSERT_EQUAL(Smi::Cast(index_loc.constant()).Value(), 0);
-      ASSERT(Smi::Cast(offset_loc.constant()).Value() != 0);
-      // No index involved at all.
-      const int32_t offset_value = Smi::Cast(offset_loc.constant()).Value();
-      __ AddImmediate(result_reg, base_reg, offset_value);
+      const intptr_t offset_in_bytes =
+          scaled_index + Smi::Cast(offset_loc.constant()).Value();
+      __ AddImmediate(result_reg, base_reg, offset_in_bytes);
     } else {
       __ add(result_reg, base_reg, compiler::Operand(offset_loc.reg()));
       // Don't need wrap-around as the index is constant only if multiplying
       // it by the scale is an int32.
-      const int32_t scaled_index =
-          Smi::Cast(index_loc.constant()).Value() * index_scale();
       __ AddImmediate(result_reg, scaled_index);
     }
   } else {

runtime/vm/compiler/backend/il_arm64.cc

@@ -389,7 +389,12 @@ LocationSummary* CalculateElementAddressInstr::MakeLocationSummary(
                                  kMaxInt64 >> scale_shift));
   // Any possible int64 value is okay as a constant here.
   summary->set_in(kOffsetPos, LocationRegisterOrConstant(offset()));
-  summary->set_out(0, Location::RequiresRegister());
+
+  if (IsNoop()) {
+    summary->set_out(0, Location::SameAsFirstInput());
+  } else {
+    summary->set_out(0, Location::RequiresRegister());
+  }
 
   return summary;
 }
@@ -400,20 +405,20 @@ void CalculateElementAddressInstr::EmitNativeCode(FlowGraphCompiler* compiler) {
   const Location& offset_loc = locs()->in(kOffsetPos);
   const Register result_reg = locs()->out(0).reg();
 
+  if (IsNoop()) {
+    ASSERT_EQUAL(base_reg, result_reg);
+    return;
+  }
+
   if (index_loc.IsConstant()) {
+    const intptr_t scaled_index =
+        Smi::Cast(index_loc.constant()).Value() * index_scale();
     if (offset_loc.IsConstant()) {
-      ASSERT_EQUAL(Smi::Cast(index_loc.constant()).Value(), 0);
-      ASSERT(Integer::Cast(offset_loc.constant()).AsInt64Value() != 0);
-      // No index involved at all.
-      const int64_t offset_value =
-          Integer::Cast(offset_loc.constant()).AsInt64Value();
-      __ AddImmediate(result_reg, base_reg, offset_value);
+      const intptr_t offset_in_bytes =
+          scaled_index + Smi::Cast(offset_loc.constant()).Value();
+      __ AddImmediate(result_reg, base_reg, offset_in_bytes);
     } else {
       __ add(result_reg, base_reg, compiler::Operand(offset_loc.reg()));
-      // Don't need wrap-around as the index is constant only if multiplying
-      // it by the scale is an int64.
-      const int64_t scaled_index =
-          Smi::Cast(index_loc.constant()).Value() * index_scale();
       __ AddImmediate(result_reg, scaled_index);
     }
   } else {

runtime/vm/compiler/backend/il_ia32.cc

@@ -266,7 +266,25 @@ LocationSummary* CalculateElementAddressInstr::MakeLocationSummary(
                                  index(), kMinInt32 >> scale_shift,
                                  kMaxInt32 >> scale_shift));
   summary->set_in(kOffsetPos, LocationRegisterOrSmiConstant(offset()));
-  summary->set_out(0, Location::RequiresRegister());
+  // Special case for when both inputs are appropriate constants.
+  if (summary->in(kIndexPos).IsConstant() &&
+      summary->in(kOffsetPos).IsConstant()) {
+    const int64_t offset_in_bytes = Utils::AddWithWrapAround<int64_t>(
+        Utils::MulWithWrapAround<int64_t>(index()->BoundSmiConstant(),
+                                          index_scale()),
+        offset()->BoundSmiConstant());
+    if (!Utils::IsInt(32, offset_in_bytes)) {
+      // The offset in bytes calculated from the index and offset cannot
+      // fit in a 32-bit immediate, so pass the index as a register instead.
+      summary->set_in(kIndexPos, Location::RequiresRegister());
+    }
+  }
+
+  if (IsNoop()) {
+    summary->set_out(0, Location::SameAsFirstInput());
+  } else {
+    summary->set_out(0, Location::RequiresRegister());
+  }
 
   return summary;
 }
@@ -277,18 +295,19 @@ void CalculateElementAddressInstr::EmitNativeCode(FlowGraphCompiler* compiler) {
   const Location& offset_loc = locs()->in(kOffsetPos);
   const Register result_reg = locs()->out(0).reg();
 
+  if (IsNoop()) {
+    ASSERT_EQUAL(base_reg, result_reg);
+    return;
+  }
+
   if (index_loc.IsConstant()) {
+    const intptr_t scaled_index =
+        Smi::Cast(index_loc.constant()).Value() * index_scale();
     if (offset_loc.IsConstant()) {
-      ASSERT_EQUAL(Smi::Cast(index_loc.constant()).Value(), 0);
-      ASSERT(Smi::Cast(offset_loc.constant()).Value() != 0);
-      // No index involved at all.
-      const int32_t offset_value = Smi::Cast(offset_loc.constant()).Value();
-      __ leal(result_reg, compiler::Address(base_reg, offset_value));
+      const intptr_t offset_in_bytes =
+          scaled_index + Smi::Cast(offset_loc.constant()).Value();
+      __ leal(result_reg, compiler::Address(base_reg, offset_in_bytes));
     } else {
-      // Don't need wrap-around as the index is constant only if multiplying
-      // it by the scale is an int32.
-      const int32_t scaled_index =
-          Smi::Cast(index_loc.constant()).Value() * index_scale();
       __ leal(result_reg, compiler::Address(base_reg, offset_loc.reg(), TIMES_1,
                                             scaled_index));
     }

runtime/vm/compiler/backend/il_riscv.cc

@@ -455,10 +455,28 @@ LocationSummary* CalculateElementAddressInstr::MakeLocationSummary(
                                                kMaxIntX >> scale_shift));
 #if XLEN == 32
   summary->set_in(kOffsetPos, LocationRegisterOrSmiConstant(offset()));
+  // Special case for when both inputs are appropriate constants.
+  if (summary->in(kIndexPos).IsConstant() &&
+      summary->in(kOffsetPos).IsConstant()) {
+    const int64_t offset_in_bytes = Utils::AddWithWrapAround<int64_t>(
+        Utils::MulWithWrapAround<int64_t>(index()->BoundSmiConstant(),
+                                          index_scale()),
+        offset()->BoundSmiConstant());
+    if (!Utils::IsInt(32, offset_in_bytes)) {
+      // The offset in bytes calculated from the index and offset cannot
+      // fit in a 32-bit immediate, so pass the index as a register instead.
+      summary->set_in(kIndexPos, Location::RequiresRegister());
+    }
+  }
 #else
   summary->set_in(kOffsetPos, LocationRegisterOrConstant(offset()));
 #endif
-  summary->set_out(0, Location::RequiresRegister());
+
+  if (IsNoop()) {
+    summary->set_out(0, Location::SameAsFirstInput());
+  } else {
+    summary->set_out(0, Location::RequiresRegister());
+  }
 
   return summary;
 }
@@ -469,20 +487,20 @@ void CalculateElementAddressInstr::EmitNativeCode(FlowGraphCompiler* compiler) {
   const Location& offset_loc = locs()->in(kOffsetPos);
   const Register result_reg = locs()->out(0).reg();
 
+  if (IsNoop()) {
+    ASSERT_EQUAL(base_reg, result_reg);
+    return;
+  }
+
   if (index_loc.IsConstant()) {
+    const intptr_t scaled_index =
+        Smi::Cast(index_loc.constant()).Value() * index_scale();
     if (offset_loc.IsConstant()) {
-      ASSERT_EQUAL(Smi::Cast(index_loc.constant()).Value(), 0);
-      ASSERT(Integer::Cast(offset_loc.constant()).AsInt64Value() != 0);
-      // No index involved at all.
-      const intx_t offset_value =
-          Integer::Cast(offset_loc.constant()).AsInt64Value();
-      __ AddImmediate(result_reg, base_reg, offset_value);
+      const intx_t offset_in_bytes =
+          scaled_index + Smi::Cast(offset_loc.constant()).Value();
+      __ AddImmediate(result_reg, base_reg, offset_in_bytes);
     } else {
       __ add(result_reg, base_reg, offset_loc.reg());
-      // Don't need wrap-around as the index is constant only if multiplying
-      // it by the scale is an intx.
-      const intx_t scaled_index =
-          Smi::Cast(index_loc.constant()).Value() * index_scale();
       __ AddImmediate(result_reg, scaled_index);
     }
   } else {

runtime/vm/compiler/backend/il_x64.cc

@@ -366,7 +366,25 @@ LocationSummary* CalculateElementAddressInstr::MakeLocationSummary(
   // Only use a Smi constant for the offset if it is an int32 constant.
   summary->set_in(kOffsetPos, LocationRegisterOrSmiConstant(offset(), kMinInt32,
                                                             kMaxInt32));
-  summary->set_out(0, Location::RequiresRegister());
+  // Special case for when both inputs are appropriate constants.
+  if (summary->in(kIndexPos).IsConstant() &&
+      summary->in(kOffsetPos).IsConstant()) {
+    const int64_t offset_in_bytes = Utils::AddWithWrapAround<int64_t>(
+        Utils::MulWithWrapAround<int64_t>(index()->BoundSmiConstant(),
+                                          index_scale()),
+        offset()->BoundSmiConstant());
+    if (!Utils::IsInt(32, offset_in_bytes)) {
+      // The offset in bytes calculated from the index and offset cannot
+      // fit in a 32-bit immediate, so pass the index as a register instead.
+      summary->set_in(kIndexPos, Location::RequiresRegister());
+    }
+  }
+
+  if (IsNoop()) {
+    summary->set_out(0, Location::SameAsFirstInput());
+  } else {
+    summary->set_out(0, Location::RequiresRegister());
+  }
 
   return summary;
 }
@@ -377,18 +395,21 @@ void CalculateElementAddressInstr::EmitNativeCode(FlowGraphCompiler* compiler) {
   const Location& offset_loc = locs()->in(kOffsetPos);
   const Register result_reg = locs()->out(0).reg();
 
+  if (IsNoop()) {
+    ASSERT_EQUAL(base_reg, result_reg);
+    return;
+  }
+
   if (index_loc.IsConstant()) {
+    const intptr_t scaled_index =
+        Smi::Cast(index_loc.constant()).Value() * index_scale();
+    ASSERT(Utils::IsInt(32, scaled_index));
     if (offset_loc.IsConstant()) {
-      ASSERT_EQUAL(Smi::Cast(index_loc.constant()).Value(), 0);
-      ASSERT(Smi::Cast(offset_loc.constant()).Value() != 0);
-      // No index involved at all.
-      const int32_t offset_value = Smi::Cast(offset_loc.constant()).Value();
-      __ leaq(result_reg, compiler::Address(base_reg, offset_value));
+      const intptr_t offset_in_bytes =
+          scaled_index + Smi::Cast(offset_loc.constant()).Value();
+      ASSERT(Utils::IsInt(32, offset_in_bytes));
+      __ leaq(result_reg, compiler::Address(base_reg, offset_in_bytes));
     } else {
-      // Don't need wrap-around as the index is constant only if multiplying
-      // it by the scale is an int32.
-      const int32_t scaled_index =
-          Smi::Cast(index_loc.constant()).Value() * index_scale();
       __ leaq(result_reg, compiler::Address(base_reg, offset_loc.reg(), TIMES_1,
                                             scaled_index));
     }
@@ -408,7 +429,8 @@ void CalculateElementAddressInstr::EmitNativeCode(FlowGraphCompiler* compiler) {
     }
     auto const scale = ToScaleFactor(index_scale(), index_unboxed);
     if (offset_loc.IsConstant()) {
-      const int32_t offset_value = Smi::Cast(offset_loc.constant()).Value();
+      const intptr_t offset_value = Smi::Cast(offset_loc.constant()).Value();
+      ASSERT(Utils::IsInt(32, offset_value));
       __ leaq(result_reg,
               compiler::Address(base_reg, index_reg, scale, offset_value));
     } else {