Bug 1523888 - Fix rooting hazard in BigInt::toString when called by ValueToID<NoGC> r=arai,terpri
authorAndy Wingo <wingo@igalia.com>
Wed, 30 Jan 2019 12:09:09 +0000
changeset 456049 59b627e13858
parent 456048 f2371ea2c1c7
child 456050 fab926361170
push id35468
push userrgurzau@mozilla.com
push dateWed, 30 Jan 2019 17:02:03 +0000
treeherdermozilla-central@37103bfa3dca [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersarai, terpri
bugs1523888
milestone67.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 1523888 - Fix rooting hazard in BigInt::toString when called by ValueToID<NoGC> r=arai,terpri Differential Revision: https://phabricator.services.mozilla.com/D18059
js/src/builtin/BigInt.cpp
js/src/util/StringBuffer.cpp
js/src/vm/BigIntType.cpp
js/src/vm/BigIntType.h
js/src/vm/JSAtom.cpp
js/src/vm/StringType.cpp
--- a/js/src/builtin/BigInt.cpp
+++ b/js/src/builtin/BigInt.cpp
@@ -119,17 +119,17 @@ bool BigIntObject::toString_impl(JSConte
     if (d < 2 || d > 36) {
       JS_ReportErrorNumberASCII(cx, GetErrorMessage, nullptr, JSMSG_BAD_RADIX);
       return false;
     }
     radix = d;
   }
 
   // Steps 6-7.
-  JSLinearString* str = BigInt::toString(cx, bi, radix);
+  JSLinearString* str = BigInt::toString<CanGC>(cx, bi, radix);
   if (!str) {
     return false;
   }
   args.rval().setString(str);
   return true;
 }
 
 bool BigIntObject::toString(JSContext* cx, unsigned argc, Value* vp) {
@@ -142,17 +142,17 @@ bool BigIntObject::toString(JSContext* c
 // for it to return the same thing as toString."
 bool BigIntObject::toLocaleString_impl(JSContext* cx, const CallArgs& args) {
   HandleValue thisv = args.thisv();
   MOZ_ASSERT(IsBigInt(thisv));
   RootedBigInt bi(cx, thisv.isBigInt()
                           ? thisv.toBigInt()
                           : thisv.toObject().as<BigIntObject>().unbox());
 
-  RootedString str(cx, BigInt::toString(cx, bi, 10));
+  RootedString str(cx, BigInt::toString<CanGC>(cx, bi, 10));
   if (!str) {
     return false;
   }
   args.rval().setString(str);
   return true;
 }
 
 bool BigIntObject::toLocaleString(JSContext* cx, unsigned argc, Value* vp) {
--- a/js/src/util/StringBuffer.cpp
+++ b/js/src/util/StringBuffer.cpp
@@ -165,17 +165,17 @@ bool js::ValueToStringBufferSlow(JSConte
   if (v.isSymbol()) {
     JS_ReportErrorNumberASCII(cx, GetErrorMessage, nullptr,
                               JSMSG_SYMBOL_TO_STRING);
     return false;
   }
 #ifdef ENABLE_BIGINT
   if (v.isBigInt()) {
     RootedBigInt i(cx, v.toBigInt());
-    JSLinearString* str = BigInt::toString(cx, i, 10);
+    JSLinearString* str = BigInt::toString<CanGC>(cx, i, 10);
     if (!str) {
       return false;
     }
     return sb.append(str);
   }
 #endif
   MOZ_ASSERT(v.isUndefined());
   return sb.append(cx->names().undefined);
--- a/js/src/vm/BigIntType.cpp
+++ b/js/src/vm/BigIntType.cpp
@@ -1070,16 +1070,17 @@ size_t BigInt::calculateMaximumCharacter
   uint64_t maximumCharactersRequired =
       CeilDiv(static_cast<uint64_t>(bitsPerCharTableMultiplier) * bitLength,
               maxBitsPerChar - 1);
   maximumCharactersRequired += x->isNegative();
 
   return AssertedCast<size_t>(maximumCharactersRequired);
 }
 
+template <AllowGC allowGC>
 JSLinearString* BigInt::toStringBasePowerOfTwo(JSContext* cx, HandleBigInt x,
                                                unsigned radix) {
   MOZ_ASSERT(mozilla::IsPowerOfTwo(radix));
   MOZ_ASSERT(radix >= 2 && radix <= 32);
   MOZ_ASSERT(!x->isZero());
 
   const unsigned length = x->digitLength();
   const bool sign = x->isNegative();
@@ -1146,17 +1147,48 @@ JSLinearString* BigInt::toStringBasePowe
   }
 
   if (sign) {
     MOZ_ASSERT(pos);
     resultChars[--pos] = '-';
   }
 
   MOZ_ASSERT(pos == 0);
-  return NewStringCopyN<CanGC>(cx, resultChars.get(), charsRequired);
+  return NewStringCopyN<allowGC>(cx, resultChars.get(), charsRequired);
+}
+
+template<AllowGC allowGC>
+JSLinearString* BigInt::toStringSingleDigitBaseTen(JSContext* cx, Digit digit, bool isNegative) {
+  if (digit <= Digit(INT32_MAX)) {
+    int32_t val = AssertedCast<int32_t>(digit);
+    return Int32ToString<allowGC>(cx, isNegative ? -val : val);
+  }
+
+  MOZ_ASSERT(digit != 0, "zero case should have been handled in toString");
+
+  const size_t charsRequired = CeilDiv(digit, 10) + isNegative;
+  auto resultChars = cx->make_pod_array<char>(charsRequired);
+  if (!resultChars) {
+    return nullptr;
+  }
+
+  size_t pos = charsRequired;
+  while (digit != 0) {
+    MOZ_ASSERT(pos);
+    resultChars[--pos] = radixDigits[digit % 10];
+    digit /= 10;
+  }
+
+  if (isNegative) {
+    MOZ_ASSERT(pos);
+    resultChars[--pos] = '-';
+  }
+
+  MOZ_ASSERT(pos == 0);
+  return NewStringCopyN<allowGC>(cx, resultChars.get(), charsRequired);
 }
 
 static constexpr BigInt::Digit MaxPowerInDigit(uint8_t radix) {
   BigInt::Digit result = 1;
   while (result < BigInt::Digit(-1) / radix) {
     result *= radix;
   }
   return result;
@@ -3013,30 +3045,43 @@ bool BigInt::lessThan(JSContext* cx, Han
     return lessThan(cx, lhsString, rhsBigInt, res);
   }
 
   MOZ_ASSERT(lhs.isNumber());
   res = lessThan(lhs.toNumber(), rhs.toBigInt());
   return true;
 }
 
+template <js::AllowGC allowGC>
 JSLinearString* BigInt::toString(JSContext* cx, HandleBigInt x, uint8_t radix) {
   MOZ_ASSERT(2 <= radix && radix <= 36);
 
   if (x->isZero()) {
     return cx->staticStrings().getInt(0);
   }
 
   if (mozilla::IsPowerOfTwo(radix)) {
-    return toStringBasePowerOfTwo(cx, x, radix);
+    return toStringBasePowerOfTwo<allowGC>(cx, x, radix);
+  }
+
+  if (radix == 10 && x->digitLength() == 1) {
+    return toStringSingleDigitBaseTen<allowGC>(cx, x->digit(0), x->isNegative());
+  }
+
+  // Punt on doing generic toString without GC.
+  if (!allowGC) {
+    return nullptr;
   }
 
   return toStringGeneric(cx, x, radix);
 }
 
+template JSLinearString* BigInt::toString<js::CanGC>(JSContext* cx, HandleBigInt x, uint8_t radix);
+template JSLinearString* BigInt::toString<js::NoGC>(JSContext* cx, HandleBigInt x, uint8_t radix);
+
 template <typename CharT>
 static inline BigInt* ParseStringBigIntLiteral(JSContext* cx,
                                                Range<const CharT> range,
                                                bool* haveParseError) {
   auto start = range.begin();
   auto end = range.end();
 
   while (start < end && unicode::IsSpace(start[0])) {
@@ -3108,24 +3153,28 @@ BigInt* js::ParseBigIntLiteral(JSContext
   BigInt* res = BigInt::parseLiteral(cx, chars, &parseError);
   if (!res) {
     return nullptr;
   }
   MOZ_RELEASE_ASSERT(!parseError);
   return res;
 }
 
+template <js::AllowGC allowGC>
 JSAtom* js::BigIntToAtom(JSContext* cx, HandleBigInt bi) {
-  JSString* str = BigInt::toString(cx, bi, 10);
+  JSString* str = BigInt::toString<allowGC>(cx, bi, 10);
   if (!str) {
     return nullptr;
   }
   return AtomizeString(cx, str);
 }
 
+template JSAtom* js::BigIntToAtom<js::CanGC>(JSContext* cx, HandleBigInt bi);
+template JSAtom* js::BigIntToAtom<js::NoGC>(JSContext* cx, HandleBigInt bi);
+
 JS::ubi::Node::Size JS::ubi::Concrete<BigInt>::size(
     mozilla::MallocSizeOf mallocSizeOf) const {
   BigInt& bi = get();
   MOZ_ASSERT(bi.isTenured());
   size_t size = js::gc::Arena::thingSize(bi.asTenured().getAllocKind());
   size += bi.sizeOfExcludingThis(mallocSizeOf);
   return size;
 }
--- a/js/src/vm/BigIntType.h
+++ b/js/src/vm/BigIntType.h
@@ -143,16 +143,17 @@ class BigInt final : public js::gc::Tenu
                      MutableHandle<Value> res);
   static bool bitOr(JSContext* cx, Handle<Value> lhs, Handle<Value> rhs,
                     MutableHandle<Value> res);
   static bool bitNot(JSContext* cx, Handle<Value> operand,
                      MutableHandle<Value> res);
 
   static double numberValue(BigInt* x);
 
+  template <js::AllowGC allowGC>
   static JSLinearString* toString(JSContext* cx, Handle<BigInt*> x,
                                   uint8_t radix);
   template <typename CharT>
   static BigInt* parseLiteral(JSContext* cx,
                               const mozilla::Range<const CharT> chars,
                               bool* haveParseError);
   template <typename CharT>
   static BigInt* parseLiteralDigits(JSContext* cx,
@@ -312,18 +313,22 @@ class BigInt final : public js::gc::Tenu
 
   // If `|x| < |y|` return -1; if `|x| == |y|` return 0; otherwise return 1.
   static int8_t absoluteCompare(BigInt* lhs, BigInt* rhs);
 
   static int8_t compare(BigInt* lhs, double rhs);
 
   static bool equal(BigInt* lhs, double rhs);
 
+  template <js::AllowGC allowGC>
   static JSLinearString* toStringBasePowerOfTwo(JSContext* cx, Handle<BigInt*>,
                                                 unsigned radix);
+  template <js::AllowGC allowGC>
+  static JSLinearString* toStringSingleDigitBaseTen(JSContext* cx, Digit digit,
+                                                    bool isNegative);
   static JSLinearString* toStringGeneric(JSContext* cx, Handle<BigInt*>,
                                          unsigned radix);
 
   static BigInt* trimHighZeroDigits(JSContext* cx, Handle<BigInt*> x);
   static BigInt* destructivelyTrimHighZeroDigits(JSContext* cx,
                                                  Handle<BigInt*> x);
 
   friend struct ::JSStructuredCloneReader;
@@ -344,16 +349,17 @@ static_assert(
 static_assert(
     sizeof(BigInt) == js::gc::MinCellSize,
     "sizeof(BigInt) intended to be the same as the minimum allocation size");
 
 }  // namespace JS
 
 namespace js {
 
+template <AllowGC allowGC>
 extern JSAtom* BigIntToAtom(JSContext* cx, JS::HandleBigInt bi);
 
 extern JS::BigInt* NumberToBigInt(JSContext* cx, double d);
 
 // Parse a BigInt from a string, using the method specified for StringToBigInt.
 // Used by the BigInt constructor among other places.
 extern JS::Result<JS::BigInt*, JS::OOM&> StringToBigInt(
     JSContext* cx, JS::Handle<JSString*> str);
--- a/js/src/vm/JSAtom.cpp
+++ b/js/src/vm/JSAtom.cpp
@@ -1077,17 +1077,17 @@ static JSAtom* ToAtomSlow(
       JS_ReportErrorNumberASCII(cx, GetErrorMessage, nullptr,
                                 JSMSG_SYMBOL_TO_STRING);
     }
     return nullptr;
   }
 #ifdef ENABLE_BIGINT
   if (v.isBigInt()) {
     RootedBigInt i(cx, v.toBigInt());
-    JSAtom* atom = BigIntToAtom(cx, i);
+    JSAtom* atom = BigIntToAtom<allowGC>(cx, i);
     if (!allowGC && !atom) {
       cx->recoverFromOutOfMemory();
     }
     return atom;
   }
 #endif
   MOZ_ASSERT(v.isUndefined());
   return cx->names().undefined;
--- a/js/src/vm/StringType.cpp
+++ b/js/src/vm/StringType.cpp
@@ -2165,17 +2165,17 @@ JSString* js::ToStringSlow(
     return nullptr;
   }
 #ifdef ENABLE_BIGINT
   else if (v.isBigInt()) {
     if (!allowGC) {
       return nullptr;
     }
     RootedBigInt i(cx, v.toBigInt());
-    str = BigInt::toString(cx, i, 10);
+    str = BigInt::toString<CanGC>(cx, i, 10);
   }
 #endif
   else {
     MOZ_ASSERT(v.isUndefined());
     str = cx->names().undefined;
   }
   return str;
 }