Bug 1853907 - Copy nursery chars for AutoStableStringChars r=jonco

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

js/public/StableStringChars.h

@@ -32,8 +32,8 @@ class JSLinearString;
 namespace JS {
 
 /**
- * This class provides safe access to a string's chars across a GC. If we ever
- * nursery allocate strings' out of line chars, this class will have to make a
+ * This class provides safe access to a string's chars across a GC. When it
+ * has nursery-allocated out of lines chars, this class will have to make a
  * copy, so it's best to avoid using this class unless you really need it. It's
  * usually more efficient to use the latin1Chars/twoByteChars JSString methods
  * and often the code can be rewritten so that only indexes instead of char
@@ -105,7 +105,6 @@ class MOZ_STACK_CLASS JS_PUBLIC_API AutoStableStringChars final {
   AutoStableStringChars(const AutoStableStringChars& other) = delete;
   void operator=(const AutoStableStringChars& other) = delete;
 
-  bool baseIsInline(Handle<JSLinearString*> linearString);
   template <typename T>
   T* allocOwnChars(JSContext* cx, size_t count);
   bool copyLatin1Chars(JSContext* cx, Handle<JSLinearString*> linearString);

js/src/vm/StringType-inl.h

@@ -226,6 +226,7 @@ inline size_t JSLinearString::maybeMallocCharsOnPromotion(
 
   return 0;
 }
+
 inline size_t JSLinearString::allocSize() const {
   MOZ_ASSERT(ownsMallocedChars());
 
@@ -432,6 +433,21 @@ inline js::PropertyName* JSLinearString::toPropertyName(JSContext* cx) {
   return atom->asPropertyName();
 }
 
+bool JSLinearString::hasMovableChars() const {
+  const JSLinearString* topBase = this;
+  while (topBase->hasBase()) {
+    topBase = topBase->base();
+  }
+  if (topBase->isInline()) {
+    return true;
+  }
+  if (topBase->isTenured()) {
+    return false;
+  }
+  return topBase->storeBuffer()->nursery().isInside(
+      topBase->nonInlineCharsRaw());
+}
+
 template <js::AllowGC allowGC>
 MOZ_ALWAYS_INLINE JSThinInlineString* JSThinInlineString::new_(
     JSContext* cx, js::gc::Heap heap) {

js/src/vm/StringType.cpp

@@ -445,7 +445,8 @@ JSExtensibleString& JSLinearString::makeExtensible(size_t capacity) {
 
 template <typename CharT>
 static MOZ_ALWAYS_INLINE bool AllocChars(JSString* str, size_t length,
-                                         CharT** chars, size_t* capacity) {
+                                         CharT** chars,
+                                         size_t* capacity) {
   /*
    * Grow by 12.5% if the buffer is very large. Otherwise, round up to the
    * next power of 2. This is similar to what we do with arrays; see
@@ -1302,9 +1303,11 @@ bool AutoStableStringChars::init(JSContext* cx, JSString* s) {
 
   MOZ_ASSERT(state_ == Uninitialized);
 
-  // If the chars are inline then we need to copy them since they may be moved
-  // by a compacting GC.
-  if (baseIsInline(linearString)) {
+  // Inline and nursery-allocated chars may move during a GC, so copy them
+  // out into a temporary malloced buffer. Note that we cannot update the
+  // string itself with a malloced buffer, because there may be dependent
+  // strings that are using the original chars.
+  if (linearString->hasMovableChars()) {
     return linearString->hasTwoByteChars() ? copyTwoByteChars(cx, linearString)
                                            : copyLatin1Chars(cx, linearString);
   }
@@ -1335,9 +1338,8 @@ bool AutoStableStringChars::initTwoByte(JSContext* cx, JSString* s) {
     return copyAndInflateLatin1Chars(cx, linearString);
   }
 
-  // If the chars are inline then we need to copy them since they may be moved
-  // by a compacting GC.
-  if (baseIsInline(linearString)) {
+  // Copy movable chars since they may be moved by GC (see above).
+  if (linearString->hasMovableChars()) {
     return copyTwoByteChars(cx, linearString);
   }
 
@@ -1350,14 +1352,6 @@ bool AutoStableStringChars::initTwoByte(JSContext* cx, JSString* s) {
   return true;
 }
 
-bool AutoStableStringChars::baseIsInline(Handle<JSLinearString*> linearString) {
-  JSString* base = linearString;
-  while (base->isDependent()) {
-    base = base->asDependent().base();
-  }
-  return base->isInline();
-}
-
 template <typename T>
 T* AutoStableStringChars::allocOwnChars(JSContext* cx, size_t count) {
   static_assert(

js/src/vm/StringType.h

@@ -925,6 +925,10 @@ class JSLinearString : public JSString {
   // be a string equal to this string.)
   inline bool isIndex(uint32_t* indexp) const;
 
+  // Return whether the characters of this string can be moved by minor or
+  // compacting GC.
+  inline bool hasMovableChars() const;
+
   void maybeInitializeIndexValue(uint32_t index, bool allowAtom = false) {
     MOZ_ASSERT(JSString::isLinear());
     MOZ_ASSERT_IF(hasIndexValue(), getIndexValue() == index);