Bug 1593801: Fix conditional load of BigInt digits. r=jandem
authorAndré Bargull <andre.bargull@gmail.com>
Tue, 05 Nov 2019 15:28:34 +0000
changeset 500615 933f3452a368cddf7a67bf71e2956ee7cac9a718
parent 500614 70bd926c6ca95e796a9187a437993688ffae3c3d
child 500616 2f0447b228c762d88198398974e2818c0d3b5dc5
push id36768
push usershindli@mozilla.com
push dateTue, 05 Nov 2019 22:07:34 +0000
treeherdermozilla-central@e96c1ca93d25 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjandem
bugs1593801
milestone72.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1593801: Fix conditional load of BigInt digits. r=jandem Differential Revision: https://phabricator.services.mozilla.com/D51797
js/src/jit-test/tests/cacheir/store-typed-element-bigint.js
js/src/jit/MacroAssembler.cpp
js/src/jit/MacroAssembler.h
js/src/jit/arm/MacroAssembler-arm-inl.h
js/src/jit/arm64/MacroAssembler-arm64-inl.h
js/src/jit/x64/MacroAssembler-x64-inl.h
js/src/jit/x86/MacroAssembler-x86-inl.h
js/src/vm/BigIntType.h
--- a/js/src/jit-test/tests/cacheir/store-typed-element-bigint.js
+++ b/js/src/jit-test/tests/cacheir/store-typed-element-bigint.js
@@ -96,8 +96,36 @@ function storeHeapDigitsNegative() {
     var ta = xs[i & 1];
     ta[0] = value;
   }
 
   assertEq(xs[0][0], BigInt.asIntN(64, value));
   assertEq(xs[1][0], BigInt.asUintN(64, value));
 }
 storeHeapDigitsNegative();
+
+// Store BigInt with first number requiring heap digits.
+function storeFirstHeapDigits() {
+  var value = 2n ** 64n;
+
+  for (var i = 0; i < 100; ++i) {
+    var ta = xs[i & 1];
+    ta[0] = value;
+  }
+
+  assertEq(xs[0][0], BigInt.asIntN(64, value));
+  assertEq(xs[1][0], BigInt.asUintN(64, value));
+}
+storeFirstHeapDigits();
+
+// Store negative BigInt with first number requiring heap digits.
+function storeFirstHeapDigitsNegative() {
+  var value = -(2n ** 64n);
+
+  for (var i = 0; i < 100; ++i) {
+    var ta = xs[i & 1];
+    ta[0] = value;
+  }
+
+  assertEq(xs[0][0], BigInt.asIntN(64, value));
+  assertEq(xs[1][0], BigInt.asUintN(64, value));
+}
+storeFirstHeapDigitsNegative();
--- a/js/src/jit/MacroAssembler.cpp
+++ b/js/src/jit/MacroAssembler.cpp
@@ -1458,19 +1458,20 @@ void MacroAssembler::loadBigIntDigits(Re
   MOZ_ASSERT(digits != bigInt);
 
   // Load the inline digits.
   computeEffectiveAddress(Address(bigInt, BigInt::offsetOfInlineDigits()),
                           digits);
 
   // If inline digits aren't used, load the heap digits. Use a conditional move
   // to prevent speculative execution.
-  test32LoadPtr(Assembler::NonZero, Address(bigInt, BigInt::offsetOfLength()),
-                Imm32(int32_t(BigInt::nonInlineDigitsLengthMask())),
-                Address(bigInt, BigInt::offsetOfHeapDigits()), digits);
+  cmp32LoadPtr(Assembler::GreaterThan,
+               Address(bigInt, BigInt::offsetOfLength()),
+               Imm32(int32_t(BigInt::inlineDigitsLength())),
+               Address(bigInt, BigInt::offsetOfHeapDigits()), digits);
 }
 
 void MacroAssembler::loadBigInt64(Register bigInt, Register64 dest) {
   // This code follows the implementation of |BigInt::toUint64()|. We're also
   // using it for inline callers of |BigInt::toInt64()|, which works, because
   // all supported Jit architectures use a two's complement representation for
   // int64 values, which means the WrapToSigned call in toInt64() is a no-op.
 
--- a/js/src/jit/MacroAssembler.h
+++ b/js/src/jit/MacroAssembler.h
@@ -1660,16 +1660,20 @@ class MacroAssembler : public MacroAssem
   inline void cmp32Load32(Condition cond, Register lhs, const Address& rhs,
                           const Address& src, Register dest)
       DEFINED_ON(arm, arm64, mips_shared, x86_shared);
 
   inline void cmp32Load32(Condition cond, Register lhs, Register rhs,
                           const Address& src, Register dest)
       DEFINED_ON(arm, arm64, mips_shared, x86_shared);
 
+  inline void cmp32LoadPtr(Condition cond, const Address& lhs, Imm32 rhs,
+                           const Address& src, Register dest)
+      DEFINED_ON(arm, arm64, x86, x64);
+
   inline void cmp32MovePtr(Condition cond, Register lhs, Imm32 rhs,
                            Register src, Register dest)
       DEFINED_ON(arm, arm64, mips_shared, x86, x64);
 
   inline void test32LoadPtr(Condition cond, const Address& addr, Imm32 mask,
                             const Address& src, Register dest)
       DEFINED_ON(arm, arm64, mips_shared, x86, x64);
 
--- a/js/src/jit/arm/MacroAssembler-arm-inl.h
+++ b/js/src/jit/arm/MacroAssembler-arm-inl.h
@@ -1911,16 +1911,23 @@ void MacroAssembler::cmp32Load32(Conditi
 }
 
 void MacroAssembler::cmp32Load32(Condition cond, Register lhs, Register rhs,
                                  const Address& src, Register dest) {
   // This is never used, but must be present to facilitate linking on arm.
   MOZ_CRASH("No known use cases");
 }
 
+void MacroAssembler::cmp32LoadPtr(Condition cond, const Address& lhs, Imm32 rhs,
+                                  const Address& src, Register dest) {
+  cmp32(lhs, rhs);
+  ScratchRegisterScope scratch(*this);
+  ma_ldr(src, dest, scratch, Offset, cond);
+}
+
 void MacroAssembler::test32LoadPtr(Condition cond, const Address& addr,
                                    Imm32 mask, const Address& src,
                                    Register dest) {
   MOZ_ASSERT(cond == Assembler::Zero || cond == Assembler::NonZero);
   test32(addr, mask);
   ScratchRegisterScope scratch(*this);
   ma_ldr(src, dest, scratch, Offset, cond);
 }
--- a/js/src/jit/arm64/MacroAssembler-arm64-inl.h
+++ b/js/src/jit/arm64/MacroAssembler-arm64-inl.h
@@ -1609,16 +1609,29 @@ void MacroAssembler::cmp32Load32(Conditi
 
 void MacroAssembler::cmp32MovePtr(Condition cond, Register lhs, Imm32 rhs,
                                   Register src, Register dest) {
   cmp32(lhs, rhs);
   Csel(ARMRegister(dest, 64), ARMRegister(src, 64), ARMRegister(dest, 64),
        cond);
 }
 
+void MacroAssembler::cmp32LoadPtr(Condition cond, const Address& lhs, Imm32 rhs,
+                                  const Address& src, Register dest) {
+  // ARM64 does not support conditional loads, so we use a branch with a CSel
+  // (to prevent Spectre attacks).
+  vixl::UseScratchRegisterScope temps(this);
+  const ARMRegister scratch64 = temps.AcquireX();
+  Label done;
+  branch32(Assembler::InvertCondition(cond), lhs, rhs, &done);
+  loadPtr(src, scratch64.asUnsized());
+  Csel(ARMRegister(dest, 64), scratch64, ARMRegister(dest, 64), cond);
+  bind(&done);
+}
+
 void MacroAssembler::test32LoadPtr(Condition cond, const Address& addr,
                                    Imm32 mask, const Address& src,
                                    Register dest) {
   MOZ_ASSERT(cond == Assembler::Zero || cond == Assembler::NonZero);
 
   // ARM64 does not support conditional loads, so we use a branch with a CSel
   // (to prevent Spectre attacks).
   vixl::UseScratchRegisterScope temps(this);
--- a/js/src/jit/x64/MacroAssembler-x64-inl.h
+++ b/js/src/jit/x64/MacroAssembler-x64-inl.h
@@ -666,16 +666,22 @@ void MacroAssembler::branchToComputedAdd
 }
 
 void MacroAssembler::cmp32MovePtr(Condition cond, Register lhs, Imm32 rhs,
                                   Register src, Register dest) {
   cmp32(lhs, rhs);
   cmovCCq(cond, Operand(src), dest);
 }
 
+void MacroAssembler::cmp32LoadPtr(Condition cond, const Address& lhs, Imm32 rhs,
+                                  const Address& src, Register dest) {
+  cmp32(lhs, rhs);
+  cmovCCq(cond, Operand(src), dest);
+}
+
 void MacroAssembler::test32LoadPtr(Condition cond, const Address& addr,
                                    Imm32 mask, const Address& src,
                                    Register dest) {
   MOZ_ASSERT(cond == Assembler::Zero || cond == Assembler::NonZero);
   test32(addr, mask);
   cmovCCq(cond, Operand(src), dest);
 }
 
--- a/js/src/jit/x86/MacroAssembler-x86-inl.h
+++ b/js/src/jit/x86/MacroAssembler-x86-inl.h
@@ -887,16 +887,22 @@ void MacroAssembler::branchToComputedAdd
 }
 
 void MacroAssembler::cmp32MovePtr(Condition cond, Register lhs, Imm32 rhs,
                                   Register src, Register dest) {
   cmp32(lhs, rhs);
   cmovCCl(cond, Operand(src), dest);
 }
 
+void MacroAssembler::cmp32LoadPtr(Condition cond, const Address& lhs, Imm32 rhs,
+                                  const Address& src, Register dest) {
+  cmp32(lhs, rhs);
+  cmovCCl(cond, Operand(src), dest);
+}
+
 void MacroAssembler::test32LoadPtr(Condition cond, const Address& addr,
                                    Imm32 mask, const Address& src,
                                    Register dest) {
   MOZ_ASSERT(cond == Assembler::Zero || cond == Assembler::NonZero);
   test32(addr, mask);
   cmovCCl(cond, Operand(src), dest);
 }
 
--- a/js/src/vm/BigIntType.h
+++ b/js/src/vm/BigIntType.h
@@ -2,17 +2,16 @@
  * vim: set ts=8 sts=2 et sw=2 tw=80:
  * This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #ifndef vm_BigIntType_h
 #define vm_BigIntType_h
 
-#include "mozilla/MathAlgorithms.h"
 #include "mozilla/Range.h"
 #include "mozilla/Span.h"
 
 #include "jstypes.h"
 #include "gc/Barrier.h"
 #include "gc/GC.h"
 #include "gc/Heap.h"
 #include "js/AllocPolicy.h"
@@ -389,22 +388,16 @@ class BigInt final
   static size_t offsetOfInlineDigits() {
     return offsetof(BigInt, inlineDigits_);
   }
 
   static size_t offsetOfHeapDigits() { return offsetof(BigInt, heapDigits_); }
 
   static constexpr size_t inlineDigitsLength() { return InlineDigitsLength; }
 
-  static constexpr size_t nonInlineDigitsLengthMask() {
-    static_assert(mozilla::IsPowerOfTwo(InlineDigitsLength),
-                  "inline digits length is a power of two");
-    return ~(InlineDigitsLength - 1) & ~InlineDigitsLength;
-  }
-
   static constexpr size_t signBitMask() { return SignBit; }
 };
 
 static_assert(
     sizeof(BigInt) >= js::gc::MinCellSize,
     "sizeof(BigInt) must be greater than the minimum allocation size");
 
 static_assert(