Merged: [maglev] Fix clobbered register in LoadSignedIntDataViewElement

The load into the result register could clobber the is_little_endian_input register. Bug: v8:7700 Fixed: chromium:1467057 (cherry picked from commit f0d3d4a) Change-Id: I66698e9353a0bb40be1ec0d5b2c131c8a1bcd12a Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/4707917 Reviewed-by: Victor Gomes <victorgomes@chromium.org> Commit-Queue: Leszek Swirski <leszeks@chromium.org> Cr-Commit-Position: refs/branch-heads/11.6@{v8#24} Cr-Branched-From: e29c028-refs/heads/11.6.189@{v8#3} Cr-Branched-From: 95cbef2-refs/heads/main@{#88340}
hexagon-geo-surv · Jul 27, 2023 · b43e737 · b43e737
1 parent 578812b
commit b43e737
Show file tree

Hide file tree

Showing 2 changed files with 37 additions and 11 deletions.
diff --git a/src/maglev/arm64/maglev-ir-arm64.cc b/src/maglev/arm64/maglev-ir-arm64.cc
@@ -1146,6 +1146,7 @@ void LoadSignedIntDataViewElement::SetValueLocationConstraints() {
   } else {
     UseRegister(is_little_endian_input());
   }
+  set_temporaries_needed(1);
   DefineAsRegister(this);
 }
 void LoadSignedIntDataViewElement::GenerateCode(MaglevAssembler* masm,
@@ -1162,21 +1163,30 @@ void LoadSignedIntDataViewElement::GenerateCode(MaglevAssembler* masm,
 
   int element_size = ExternalArrayElementSize(type_);
 
-  // Load data pointer.
-  {
-    MaglevAssembler::ScratchRegisterScope temps(masm);
-    Register data_pointer = temps.Acquire();
-    __ LoadExternalPointerField(
-        data_pointer, FieldMemOperand(object, JSDataView::kDataPointerOffset));
+  MaglevAssembler::ScratchRegisterScope temps(masm);
+  Register data_pointer = temps.Acquire();
 
-    __ LoadSignedField(result_reg.W(), MemOperand(data_pointer, index),
-                       element_size);
+  // We need to make sure we don't clobber is_little_endian_input by writing to
+  // the result register.
+  Register reg_with_result = result_reg;
+  if (type_ != ExternalArrayType::kExternalInt8Array &&
+      !is_little_endian_constant() &&
+      result_reg == ToRegister(is_little_endian_input())) {
+    reg_with_result = data_pointer;
   }
 
+  // Load data pointer.
+  __ LoadExternalPointerField(
+      data_pointer, FieldMemOperand(object, JSDataView::kDataPointerOffset));
+
+  __ LoadSignedField(reg_with_result.W(), MemOperand(data_pointer, index),
+                     element_size);
+
   // We ignore little endian argument if type is a byte size.
   if (type_ != ExternalArrayType::kExternalInt8Array) {
     if (is_little_endian_constant()) {
       if (!FromConstantToBool(masm, is_little_endian_input().node())) {
+        DCHECK_EQ(reg_with_result, result_reg);
         __ ReverseByteOrder(result_reg, element_size);
       }
     } else {
@@ -1185,10 +1195,13 @@ void LoadSignedIntDataViewElement::GenerateCode(MaglevAssembler* masm,
                    CheckType::kCheckHeapObject, is_little_endian, is_big_endian,
                    false);
       __ Bind(*is_big_endian);
-      __ ReverseByteOrder(result_reg, element_size);
+      __ ReverseByteOrder(reg_with_result, element_size);
       __ Bind(*is_little_endian);
       // arm64 is little endian.
       static_assert(V8_TARGET_LITTLE_ENDIAN == 1);
+      if (reg_with_result != result_reg) {
+        __ Move(result_reg, reg_with_result);
+      }
     }
   }
 }

diff --git a/src/maglev/x64/maglev-ir-x64.cc b/src/maglev/x64/maglev-ir-x64.cc
@@ -280,18 +280,28 @@ void LoadSignedIntDataViewElement::GenerateCode(MaglevAssembler* masm,
     __ Assert(above_equal, AbortReason::kUnexpectedValue);
   }
 
+  // We need to make sure we don't clobber is_little_endian_input by writing to
+  // the result register.
+  Register reg_with_result = result_reg;
+  if (type_ != ExternalArrayType::kExternalInt8Array &&
+      !is_little_endian_constant() &&
+      result_reg == ToRegister(is_little_endian_input())) {
+    reg_with_result = data_pointer;
+  }
+
   // Load data pointer.
   __ LoadExternalPointerField(
       data_pointer, FieldOperand(object, JSDataView::kDataPointerOffset));
 
   int element_size = ExternalArrayElementSize(type_);
-  __ LoadSignedField(result_reg, Operand(data_pointer, index, times_1, 0),
+  __ LoadSignedField(reg_with_result, Operand(data_pointer, index, times_1, 0),
                      element_size);
 
   // We ignore little endian argument if type is a byte size.
   if (type_ != ExternalArrayType::kExternalInt8Array) {
     if (is_little_endian_constant()) {
       if (!FromConstantToBool(masm, is_little_endian_input().node())) {
+        DCHECK_EQ(reg_with_result, result_reg);
         __ ReverseByteOrder(result_reg, element_size);
       }
     } else {
@@ -300,10 +310,13 @@ void LoadSignedIntDataViewElement::GenerateCode(MaglevAssembler* masm,
                    CheckType::kCheckHeapObject, is_little_endian, is_big_endian,
                    false);
       __ bind(*is_big_endian);
-      __ ReverseByteOrder(result_reg, element_size);
+      __ ReverseByteOrder(reg_with_result, element_size);
       __ bind(*is_little_endian);
       // x64 is little endian.
       static_assert(V8_TARGET_LITTLE_ENDIAN == 1);
+      if (reg_with_result != result_reg) {
+        __ Move(result_reg, reg_with_result);
+      }
     }
   }
 }