From 75266d1e43f9edb7a68dd50cf3ded2e8e3d35425 Mon Sep 17 00:00:00 2001 From: Steve Fink Date: Fri, 24 Nov 2023 05:33:20 +0000 Subject: [PATCH] Bug 1853907 - Copy nursery chars for AutoStableStringChars r=jonco Differential Revision: https://phabricator.services.mozilla.com/D191574 --- js/public/StableStringChars.h | 5 ++--- js/src/vm/StringType-inl.h | 16 ++++++++++++++++ js/src/vm/StringType.cpp | 24 +++++++++--------------- js/src/vm/StringType.h | 4 ++++ 4 files changed, 31 insertions(+), 18 deletions(-) diff --git a/js/public/StableStringChars.h b/js/public/StableStringChars.h index a12813d89c79..5c50df18cb53 100644 --- a/js/public/StableStringChars.h +++ b/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 linearString); template T* allocOwnChars(JSContext* cx, size_t count); bool copyLatin1Chars(JSContext* cx, Handle linearString); diff --git a/js/src/vm/StringType-inl.h b/js/src/vm/StringType-inl.h index 4ebc68214df8..63c2a04be56a 100644 --- a/js/src/vm/StringType-inl.h +++ b/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 MOZ_ALWAYS_INLINE JSThinInlineString* JSThinInlineString::new_( JSContext* cx, js::gc::Heap heap) { diff --git a/js/src/vm/StringType.cpp b/js/src/vm/StringType.cpp index 0226102c8dd6..666489c0f157 100644 --- a/js/src/vm/StringType.cpp +++ b/js/src/vm/StringType.cpp @@ -445,7 +445,8 @@ JSExtensibleString& JSLinearString::makeExtensible(size_t capacity) { template 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 linearString) { - JSString* base = linearString; - while (base->isDependent()) { - base = base->asDependent().base(); - } - return base->isInline(); -} - template T* AutoStableStringChars::allocOwnChars(JSContext* cx, size_t count) { static_assert( diff --git a/js/src/vm/StringType.h b/js/src/vm/StringType.h index 497d0cd95dc4..7bfcb6f62041 100644 --- a/js/src/vm/StringType.h +++ b/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);