From fff81462c6475e9d10776d8c443ef3a1f3438942 Mon Sep 17 00:00:00 2001
From: Jan de Mooij <jdemooij@mozilla.com>
Date: Mon, 24 Jun 2024 07:41:48 +0000
Subject: [PATCH] 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                        | 22 +++++++------------
 js/src/jit-test/tests/basic/stringbuffer-2.js |  5 +++++
 js/src/vm/StringType.h                        | 22 ++++++++++++-------
 3 files changed, 27 insertions(+), 22 deletions(-)
 create mode 100644 js/src/jit-test/tests/basic/stringbuffer-2.js

diff --git a/js/src/gc/Tenuring.cpp b/js/src/gc/Tenuring.cpp
index 3f3d18946b07..8b846a82c678 100644
--- a/js/src/gc/Tenuring.cpp
+++ b/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);
       }
diff --git a/js/src/jit-test/tests/basic/stringbuffer-2.js b/js/src/jit-test/tests/basic/stringbuffer-2.js
new file mode 100644
index 000000000000..1a46d0e9e07e
--- /dev/null
+++ b/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");
diff --git a/js/src/vm/StringType.h b/js/src/vm/StringType.h
index 8ebec7a1b096..65c0c8e2e0ba 100644
--- a/js/src/vm/StringType.h
+++ b/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;