Bug 1899160 part 2 - Fix string arena assertion for dependent strings…

… backed by a StringBuffer. r=sfink

We already avoid calling `AssertJSStringBufferInCorrectArena` when creating a JS string with
a `StringBuffer`, but we weren't handling this case when moving dependent strings.

This patch changes `relocateNonInlineChars` to `relocateBaseAndChars` so we can check
for this case there.

Differential Revision: https://phabricator.services.mozilla.com/D213950

js/src/gc/Tenuring.cpp

@@ -558,9 +558,7 @@ void JSDependentString::sweepTypedAfterMinorGC() {
   MOZ_ASSERT(offset < tenuredBase->length());
 
   const CharT* newBaseChars = tenuredBase->JSString::nonInlineCharsRaw<CharT>();
-  relocateNonInlineChars(newBaseChars, offset);
-  MOZ_ASSERT(tenuredBase->assertIsValidBase());
-  d.s.u3.base = tenuredBase;
+  relocateBaseAndChars(tenuredBase, newBaseChars, offset);
 }
 
 inline void JSDependentString::sweepAfterMinorGC() {
@@ -1093,10 +1091,9 @@ void js::gc::TenuringTracer::relocateDependentStringChars(
         const CharT* rootBaseChars = relocOverlay->savedNurseryChars<CharT>();
         *offset = dependentStrChars - rootBaseChars;
         MOZ_ASSERT(*offset < tenuredRootBase->length());
-        tenuredDependentStr->relocateNonInlineChars<const CharT*>(
-            tenuredRootBase->nonInlineChars<CharT>(nogc), *offset);
-        tenuredDependentStr->setBase(tenuredRootBase);
-        MOZ_ASSERT(tenuredRootBase->assertIsValidBase());
+        tenuredDependentStr->relocateBaseAndChars<const CharT*>(
+            tenuredRootBase, tenuredRootBase->nonInlineChars<CharT>(nogc),
+            *offset);
 
         if (tenuredDependentStr->isTenured() && !tenuredRootBase->isTenured()) {
           runtime()->gc.storeBuffer().putWholeCell(tenuredDependentStr);
@@ -1220,15 +1217,12 @@ void js::gc::TenuringTracer::collectToStringFixedPoint() {
       MOZ_ASSERT(offset < tenuredRootBase->length());
 
       if (str->hasTwoByteChars()) {
-        str->asDependent().relocateNonInlineChars<const char16_t*>(
-            tenuredRootBase->twoByteChars(nogc), offset);
+        str->asDependent().relocateBaseAndChars<const char16_t*>(
+            tenuredRootBase, tenuredRootBase->twoByteChars(nogc), offset);
       } else {
-        str->asDependent().relocateNonInlineChars<const JS::Latin1Char*>(
-            tenuredRootBase->latin1Chars(nogc), offset);
+        str->asDependent().relocateBaseAndChars<const JS::Latin1Char*>(
+            tenuredRootBase, tenuredRootBase->latin1Chars(nogc), offset);
       }
-
-      str->setBase(tenuredRootBase);
-      MOZ_ASSERT(tenuredRootBase->assertIsValidBase());
       if (str->isTenured() && !tenuredRootBase->isTenured()) {
         runtime()->gc.storeBuffer().putWholeCell(str);
       }

js/src/jit-test/tests/basic/stringbuffer-2.js

@@ -0,0 +1,5 @@
+var s = newString("abcdefg".repeat(5), {newStringBuffer: true});
+for (var i = 0; i < 10; i++) {
+  s = s.substring(1);
+}
+assertEq(s, "defgabcdefgabcdefgabcdefg");

js/src/vm/StringType.h

@@ -537,7 +537,8 @@ class JSString : public js::gc::CellWithLengthAndFlags {
 
  protected:
   template <typename CharT>
-  MOZ_ALWAYS_INLINE void setNonInlineChars(const CharT* chars);
+  MOZ_ALWAYS_INLINE void setNonInlineChars(const CharT* chars,
+                                           bool checkArena = true);
 
   template <typename CharT>
   static MOZ_ALWAYS_INLINE void checkStringCharsArena(const CharT* chars) {
@@ -1236,8 +1237,12 @@ class JSDependentString : public JSLinearString {
                                      js::gc::Heap heap);
 
   template <typename T>
-  void relocateNonInlineChars(T chars, size_t offset) {
-    setNonInlineChars(chars + offset);
+  void relocateBaseAndChars(JSLinearString* base, T chars, size_t offset) {
+    // StringBuffers are not yet allocated in the jemalloc string arena.
+    bool checkArena = !base->hasStringBuffer();
+    MOZ_ASSERT(base->assertIsValidBase());
+    setNonInlineChars(chars + offset, checkArena);
+    setBase(base);
   }
 
   inline JSLinearString* rootBaseDuringMinorGC();
@@ -2291,19 +2296,20 @@ MOZ_ALWAYS_INLINE bool JSAtom::lengthFitsInline<char16_t>(size_t length) {
 }
 
 template <>
-MOZ_ALWAYS_INLINE void JSString::setNonInlineChars(const char16_t* chars) {
+MOZ_ALWAYS_INLINE void JSString::setNonInlineChars(const char16_t* chars,
+                                                   bool checkArena) {
   // Check that the new buffer is located in the StringBufferArena
-  if (!(isAtomRef() && atom()->isInline())) {
+  if (checkArena && !(isAtomRef() && atom()->isInline())) {
     checkStringCharsArena(chars);
   }
   d.s.u2.nonInlineCharsTwoByte = chars;
 }
 
 template <>
-MOZ_ALWAYS_INLINE void JSString::setNonInlineChars(
-    const JS::Latin1Char* chars) {
+MOZ_ALWAYS_INLINE void JSString::setNonInlineChars(const JS::Latin1Char* chars,
+                                                   bool checkArena) {
   // Check that the new buffer is located in the StringBufferArena
-  if (!(isAtomRef() && atom()->isInline())) {
+  if (checkArena && !(isAtomRef() && atom()->isInline())) {
     checkStringCharsArena(chars);
   }
   d.s.u2.nonInlineCharsLatin1 = chars;