Bug 1509542 part 3 - Increase JSString max length from |2**28 - 1| to |2**30 - 2|. r=jwalden
authorJan de Mooij <jdemooij@mozilla.com>
Mon, 26 Nov 2018 23:28:47 +0000
changeset 507456 cafc30a40d9f93762372ad805443ec8ba7efecdc
parent 507455 b3f239eccb59e2fe9632ee1106dfd78a176200c1
child 507457 5da532fab9bacb44b81ae0de6b7f026d187e54c7
push id1905
push userffxbld-merge
push dateMon, 21 Jan 2019 12:33:13 +0000
treeherdermozilla-release@c2fca1944d8c [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjwalden
bugs1509542
milestone65.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 1509542 part 3 - Increase JSString max length from |2**28 - 1| to |2**30 - 2|. r=jwalden We use |2**30 - 2| to ensure the size of a null-terminated char16_t buffer still fits in int32_t. The patch adds some tests. I tried to add similar tests for toUpperCase() and toLocaleUpperCase("lt") (calling into ICU) but it makes the test very slow in debug builds. Depends on D12878 Differential Revision: https://phabricator.services.mozilla.com/D12879
dom/base/nsJSUtils.h
js/src/builtin/SelfHostingDefines.h
js/src/builtin/String.js
js/src/jit-test/tests/basic/bug1355573.js
js/src/jit-test/tests/basic/max-string-length.js
js/src/jsfriendapi.h
--- a/dom/base/nsJSUtils.h
+++ b/dom/base/nsJSUtils.h
@@ -222,30 +222,30 @@ public:
   static void ResetTimeZone();
 };
 
 template<typename T>
 inline bool
 AssignJSString(JSContext *cx, T &dest, JSString *s)
 {
   size_t len = JS::GetStringLength(s);
-  static_assert(js::MaxStringLength < (1 << 28),
+  static_assert(js::MaxStringLength < (1 << 30),
                 "Shouldn't overflow here or in SetCapacity");
   if (MOZ_UNLIKELY(!dest.SetLength(len, mozilla::fallible))) {
     JS_ReportOutOfMemory(cx);
     return false;
   }
   return js::CopyStringChars(cx, dest.BeginWriting(), s, len);
 }
 
 inline void
 AssignJSFlatString(nsAString &dest, JSFlatString *s)
 {
   size_t len = js::GetFlatStringLength(s);
-  static_assert(js::MaxStringLength < (1 << 28),
+  static_assert(js::MaxStringLength < (1 << 30),
                 "Shouldn't overflow here or in SetCapacity");
   dest.SetLength(len);
   js::CopyFlatStringChars(dest.BeginWriting(), s, len);
 }
 
 class nsAutoJSString : public nsAutoString
 {
 public:
--- a/js/src/builtin/SelfHostingDefines.h
+++ b/js/src/builtin/SelfHostingDefines.h
@@ -18,18 +18,18 @@
 
 // Unforgeable version of Function.prototype.apply.
 #define FUN_APPLY(FUN, RECEIVER, ARGS) \
   callFunction(std_Function_apply, FUN, RECEIVER, ARGS)
 
 // NB: keep this in sync with the copy in vm/ArgumentsObject.h.
 #define MAX_ARGS_LENGTH (500 * 1000)
 
-// NB: keep this in sync with the copy in vm/String.h.
-#define MAX_STRING_LENGTH ((1 << 28) - 1)
+// NB: keep this in sync with js::MaxStringLength in jsfriendapi.h.
+#define MAX_STRING_LENGTH ((1 << 30) - 2)
 
 // Spread non-empty argument list of up to 15 elements.
 #define SPREAD(v, n) SPREAD_##n(v)
 #define SPREAD_1(v) v[0]
 #define SPREAD_2(v) SPREAD_1(v), v[1]
 #define SPREAD_3(v) SPREAD_2(v), v[2]
 #define SPREAD_4(v) SPREAD_3(v), v[3]
 #define SPREAD_5(v) SPREAD_4(v), v[4]
--- a/js/src/builtin/String.js
+++ b/js/src/builtin/String.js
@@ -508,20 +508,24 @@ function String_repeat(count) {
     // Steps 6-7.
     if (n < 0)
         ThrowRangeError(JSMSG_NEGATIVE_REPETITION_COUNT);
 
     // Inverted condition to handle |Infinity * 0 = NaN| correctly.
     if (!(n * S.length <= MAX_STRING_LENGTH))
         ThrowRangeError(JSMSG_RESULTING_STRING_TOO_LARGE);
 
-    // Communicate |n|'s possible range to the compiler.
-    assert((MAX_STRING_LENGTH & (MAX_STRING_LENGTH + 1)) === 0,
-           "MAX_STRING_LENGTH can be used as a bitmask");
-    n = n & MAX_STRING_LENGTH;
+    // Communicate |n|'s possible range to the compiler. We actually use
+    // MAX_STRING_LENGTH + 1 as range because that's a valid bit mask. That's
+    // fine because it's only used as optimization hint.
+    assert(TO_INT32(MAX_STRING_LENGTH + 1) == MAX_STRING_LENGTH + 1,
+           "MAX_STRING_LENGTH + 1 must fit in int32");
+    assert(((MAX_STRING_LENGTH + 1) & (MAX_STRING_LENGTH + 2)) === 0,
+           "MAX_STRING_LENGTH + 1 can be used as a bitmask");
+    n = n & (MAX_STRING_LENGTH + 1);
 
     // Steps 8-9.
     var T = "";
     for (;;) {
         if (n & 1)
             T += S;
         n >>= 1;
         if (n)
--- a/js/src/jit-test/tests/basic/bug1355573.js
+++ b/js/src/jit-test/tests/basic/bug1355573.js
@@ -1,10 +1,10 @@
 // |jit-test| skip-if: getBuildConfiguration().debug === true
 function f(){};
-Object.defineProperty(f, "name", {value: "a".repeat((1<<28)-1)});
+Object.defineProperty(f, "name", {value: "a".repeat((1<<30)-2)});
 var ex = null;
 try {
     len = f.bind().name.length;
 } catch (e) {
     ex = e;
 }
 assertEq(ex === "out of memory" || (ex instanceof InternalError), true);
new file mode 100644
--- /dev/null
+++ b/js/src/jit-test/tests/basic/max-string-length.js
@@ -0,0 +1,18 @@
+load(libdir + "asserts.js");
+
+const MaxStringLength = 2**30 - 2;
+
+// First check MaxStringLength is accurate.
+assertThrowsInstanceOf(() =>  "a".repeat(MaxStringLength + 1),
+                       RangeError);
+
+// escape() must handle uint32_t overflow correctly.
+var s = "\u0390".repeat(MaxStringLength);
+assertEq(s.length, MaxStringLength);
+var ex = null;
+try {
+    escape(s);
+} catch (e) {
+    ex = e;
+}
+assertEq(ex === "out of memory" || (ex instanceof InternalError), true);
--- a/js/src/jsfriendapi.h
+++ b/js/src/jsfriendapi.h
@@ -801,17 +801,23 @@ GetObjectSlot(JSObject* obj, size_t slot
 }
 
 MOZ_ALWAYS_INLINE size_t
 GetAtomLength(JSAtom* atom)
 {
     return reinterpret_cast<JS::shadow::String*>(atom)->length();
 }
 
-static const uint32_t MaxStringLength = (1 << 28) - 1;
+// Maximum length of a JS string. This is chosen so that the number of bytes
+// allocated for a null-terminated TwoByte string still fits in int32_t.
+static const uint32_t MaxStringLength = (1 << 30) - 2;
+
+static_assert((uint64_t(MaxStringLength) + 1) * sizeof(char16_t) <= INT32_MAX,
+              "size of null-terminated JSString char buffer must fit in "
+              "INT32_MAX");
 
 MOZ_ALWAYS_INLINE size_t
 GetFlatStringLength(JSFlatString* s)
 {
     return reinterpret_cast<JS::shadow::String*>(s)->length();
 }
 
 MOZ_ALWAYS_INLINE size_t