[turbofan] Harden BuildElementAccess against potential typer bugs

Bug: chromium:1051017
Change-Id: Id300c6d5f88b762e465383ac78ed037d3bc958d5
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2089938
Commit-Queue: Georg Neis <neis@chromium.org>
Reviewed-by: Tobias Tebbi <tebbi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#66627}

src/compiler/js-native-context-specialization.cc

@@ -2636,6 +2636,15 @@ JSNativeContextSpecialization::BuildElementAccess(
           Node* etrue = effect;
           Node* vtrue;
           {
+            // Do a real bounds check against {length}. This is in order to
+            // protect against a potential typer bug leading to the elimination
+            // of the NumberLessThan above.
+            index = etrue = graph()->NewNode(
+                simplified()->CheckBounds(
+                    FeedbackSource(),
+                    CheckBoundsParameters::kAbortOnOutOfBounds),
+                index, length, etrue, if_true);
+
             // Perform the actual load
             vtrue = etrue = graph()->NewNode(
                 simplified()->LoadTypedElement(external_array_type),
@@ -2697,6 +2706,15 @@ JSNativeContextSpecialization::BuildElementAccess(
           Node* if_true = graph()->NewNode(common()->IfTrue(), branch);
           Node* etrue = effect;
           {
+            // Do a real bounds check against {length}. This is in order to
+            // protect against a potential typer bug leading to the elimination
+            // of the NumberLessThan above.
+            index = etrue = graph()->NewNode(
+                simplified()->CheckBounds(
+                    FeedbackSource(),
+                    CheckBoundsParameters::kAbortOnOutOfBounds),
+                index, length, etrue, if_true);
+
             // Perform the actual store.
             etrue = graph()->NewNode(
                 simplified()->StoreTypedElement(external_array_type),
@@ -2829,6 +2847,14 @@ JSNativeContextSpecialization::BuildElementAccess(
         Node* etrue = effect;
         Node* vtrue;
         {
+          // Do a real bounds check against {length}. This is in order to
+          // protect against a potential typer bug leading to the elimination of
+          // the NumberLessThan above.
+          index = etrue = graph()->NewNode(
+              simplified()->CheckBounds(
+                  FeedbackSource(), CheckBoundsParameters::kAbortOnOutOfBounds),
+              index, length, etrue, if_true);
+
           // Perform the actual load
           vtrue = etrue =
               graph()->NewNode(simplified()->LoadElement(element_access),
@@ -3097,13 +3123,18 @@ Node* JSNativeContextSpecialization::BuildIndexedStringLoad(
                                           IsSafetyCheck::kCriticalSafetyCheck),
                          check, *control);
 
-    Node* masked_index = graph()->NewNode(simplified()->PoisonIndex(), index);
-
     Node* if_true = graph()->NewNode(common()->IfTrue(), branch);
-    Node* etrue;
+    // Do a real bounds check against {length}. This is in order to protect
+    // against a potential typer bug leading to the elimination of the
+    // NumberLessThan above.
+    Node* etrue = index = graph()->NewNode(
+        simplified()->CheckBounds(FeedbackSource(),
+                                  CheckBoundsParameters::kAbortOnOutOfBounds),
+        index, length, *effect, if_true);
+    Node* masked_index = graph()->NewNode(simplified()->PoisonIndex(), index);
     Node* vtrue = etrue =
         graph()->NewNode(simplified()->StringCharCodeAt(), receiver,
-                         masked_index, *effect, if_true);
+                         masked_index, etrue, if_true);
     vtrue = graph()->NewNode(simplified()->StringFromSingleCharCode(), vtrue);
 
     Node* if_false = graph()->NewNode(common()->IfFalse(), branch);

src/compiler/simplified-lowering.cc

@@ -1687,7 +1687,7 @@ class RepresentationSelector {
         }
       }
     } else {
-      DCHECK(length_type.Is(type_cache_->kPositiveSafeInteger));
+      CHECK(length_type.Is(type_cache_->kPositiveSafeInteger));
       VisitBinop(node,
                  UseInfo::CheckedSigned64AsWord64(kIdentifyZeros, p.feedback()),
                  UseInfo::Word64(), MachineRepresentation::kWord64);

src/compiler/simplified-operator.cc

@@ -822,7 +822,6 @@ bool operator==(CheckMinusZeroParameters const& lhs,
   V(CheckedUint32Mod, 2, 1)
 
 #define CHECKED_WITH_FEEDBACK_OP_LIST(V)    \
-  V(CheckBounds, 2, 1)                      \
   V(CheckNumber, 1, 1)                      \
   V(CheckSmi, 1, 1)                         \
   V(CheckString, 1, 1)                      \
@@ -840,7 +839,9 @@ bool operator==(CheckMinusZeroParameters const& lhs,
   V(CheckedUint64ToInt32, 1, 1)             \
   V(CheckedUint64ToTaggedSigned, 1, 1)
 
-#define CHECKED_BOUNDS_OP_LIST(V) V(CheckedUint32Bounds)
+#define CHECKED_BOUNDS_OP_LIST(V) \
+  V(CheckBounds)                  \
+  V(CheckedUint32Bounds)
 
 struct SimplifiedOperatorGlobalCache final {
 #define PURE(Name, properties, value_input_count, control_input_count)     \
@@ -1616,7 +1617,8 @@ std::ostream& operator<<(std::ostream& os, CheckParameters const& p) {
 }
 
 CheckParameters const& CheckParametersOf(Operator const* op) {
-  if (op->opcode() == IrOpcode::kCheckedUint32Bounds) {
+  if (op->opcode() == IrOpcode::kCheckBounds ||
+      op->opcode() == IrOpcode::kCheckedUint32Bounds) {
     return OpParameter<CheckBoundsParameters>(op).check_parameters();
   }
 #define MAKE_OR(name, arg2, arg3) op->opcode() == IrOpcode::k##name ||

src/compiler/simplified-operator.h

@@ -781,7 +781,9 @@ class V8_EXPORT_PRIVATE SimplifiedOperatorBuilder final
   const Operator* CompareMaps(ZoneHandleSet<Map>);
   const Operator* MapGuard(ZoneHandleSet<Map> maps);
 
-  const Operator* CheckBounds(const FeedbackSource& feedback);
+  const Operator* CheckBounds(const FeedbackSource& feedback,
+                              CheckBoundsParameters::Mode mode =
+                                  CheckBoundsParameters::kDeoptOnOutOfBounds);
   const Operator* CheckClosure(const Handle<FeedbackCell>& feedback_cell);
   const Operator* CheckEqualsInternalizedString();
   const Operator* CheckEqualsSymbol();