Changeset 186037 in webkit for trunk/Source/JavaScriptCore/runtime/JSString.h

Timestamp:

Jun 27, 2015, 3:53:12 PM (10 years ago)

Author:

Darin Adler

Message:

Make converting JSString to StringView idiomatically safe
​https://bugs.webkit.org/show_bug.cgi?id=146387

Reviewed by Anders Carlsson.

• jsc.cpp:

(functionPrint): Add explicit call to SafeView::get, needed since there
is no StringView temporary.
(functionDebug): Ditto.

• runtime/ArrayPrototype.cpp:

(JSC::holesMustForwardToPrototype): Refactored into helper function.
(JSC::join): Refactored so that StringView is a function argument, making
the lifetime simpler.
(JSC::arrayProtoFuncJoin): Ditto.
(JSC::arrayProtoFuncReverse): Use new holesMustForwardToPrototype helper.

• runtime/JSGlobalObjectFunctions.cpp:

(JSC::encode): Add explicit call to SafeView::get.

• runtime/JSString.h: Moved declarations of functions to the top of the

file instead of mixing them in with the function definitions. Changed
return type of the view function to return a JSString::SafeView so that
the JSString's lifetime will last as long as the StringView does in
typical coding idioms.
(JSC::JSString::getIndex): Use unsafeView so we can index into the
view; could also have used view.get but here in this class this seems fine.
(JSC::JSRopeString::unsafeView): Renamed existing view function to this.
(JSC::JSString::unsafeView): Ditto.
(JSC::JSString::SafeView::SafeView): Contains reference to an ExecState
and a JSString. The ExecState is needed to create the StringView, and the
JSString needs to be kept alive as long as the StringView is.
(JSC::JSString::SafeView::operator StringView): Call unsafeView.
(JSC::JSString::SafeView::get): Convenience for when we want to call
StringView member functions.
(JSC::JSString::view): Added. Returns a SafeView.

• runtime/StringPrototype.cpp:

(JSC::stringProtoFuncIndexOf): Add explicit call to SafeView::get.

File:

• trunk/Source/JavaScriptCore/runtime/JSString.h (modified) (11 diffs)

trunk/Source/JavaScriptCore/runtime/JSString.h

@@ -48 +48 @@
 JSString* jsSubstring(VM*, const String&, unsigned offset, unsigned length);
 JSString* jsSubstring(ExecState*, const String&, unsigned offset, unsigned length);
+JSString* jsSubstring8(VM*, const String&, unsigned offset, unsigned length);
+JSString* jsSubstring8(ExecState*, const String&, unsigned offset, unsigned length);
 
 // Non-trivial strings are two or more characters long.
@@ -62 +64 @@
 
 JSRopeString* jsStringBuilder(VM*);
+
+bool isJSString(JSValue);
+JSString* asString(JSValue);
 
 struct StringViewWithUnderlyingString {
@@ -149 +154 @@
     AtomicString toAtomicString(ExecState*) const;
     RefPtr<AtomicStringImpl> toExistingAtomicString(ExecState*) const;
-    StringView view(ExecState*) const;
+
+    class SafeView;
+    SafeView view(ExecState*) const;
     StringViewWithUnderlyingString viewWithUnderlyingString(ExecState&) const;
+
     const String& value(ExecState*) const;
     const String& tryGetValue() const;
@@ -221 +229 @@
 
     String& string() { ASSERT(!isRope()); return m_value; }
+    StringView unsafeView(ExecState&) const;
 
     friend JSValue jsString(ExecState*, JSString*, JSString*);
@@ -390 +399 @@
     void resolveRopeInternal16NoSubstring(UChar*) const;
     void clearFibers() const;
-    StringView view(ExecState*) const;
+    StringView unsafeView(ExecState&) const;
     StringViewWithUnderlyingString viewWithUnderlyingString(ExecState&) const;
 
@@ -436 +445 @@
 };
 
+class JSString::SafeView {
+public:
+    SafeView();
+    explicit SafeView(ExecState&, const JSString&);
+    operator StringView() const;
+    StringView get() const;
+
+private:
+    ExecState* m_state { nullptr };
+
+    // The following pointer is marked "volatile" to make the compiler leave it on the stack
+    // or in a register as long as this object is alive, even after the last use of the pointer.
+    // That's needed to prevent garbage collecting the string and possibly deleting the block
+    // with the characters in it, and then using the StringView after that.
+    const JSString* volatile m_string { nullptr };
+};
+
+JS_EXPORT_PRIVATE JSString* jsStringWithCacheSlowCase(VM&, StringImpl&);
 
 inline const StringImpl* JSString::tryGetValueImpl() const
@@ -441 +468 @@
     return m_value.impl();
 }
-
-JSString* asString(JSValue);
 
 inline JSString* asString(JSValue value)
@@ -512 +537 @@
 {
     ASSERT(canGetIndex(i));
-    return jsSingleCharacterString(exec, view(exec)[i]);
+    return jsSingleCharacterString(exec, unsafeView(*exec)[i]);
 }
 
@@ -598 +623 @@
 inline JSString* jsOwnedString(ExecState* exec, const String& s) { return jsOwnedString(&exec->vm(), s); }
 
-JS_EXPORT_PRIVATE JSString* jsStringWithCacheSlowCase(VM&, StringImpl&);
-
 ALWAYS_INLINE JSString* jsStringWithCache(ExecState* exec, const String& s)
 {
@@ -652 +675 @@
 }
 
-inline bool isJSString(JSValue v) { return v.isCell() && v.asCell()->type() == StringType; }
+inline bool isJSString(JSValue v)
+{
+    return v.isCell() && v.asCell()->type() == StringType;
+}
+
+ALWAYS_INLINE StringView JSRopeString::unsafeView(ExecState& state) const
+{
+    if (isSubstring()) {
+        if (is8Bit())
+            return StringView(substringBase()->m_value.characters8() + substringOffset(), m_length);
+        return StringView(substringBase()->m_value.characters16() + substringOffset(), m_length);
+    }
+    resolveRope(&state);
+    return m_value;
+}
+
+ALWAYS_INLINE StringViewWithUnderlyingString JSRopeString::viewWithUnderlyingString(ExecState& state) const
+{
+    if (isSubstring()) {
+        auto& base = substringBase()->m_value;
+        if (is8Bit())
+            return { { base.characters8() + substringOffset(), m_length }, base };
+        return { { base.characters16() + substringOffset(), m_length }, base };
+    }
+    resolveRope(&state);
+    return { m_value, m_value };
+}
+
+ALWAYS_INLINE StringView JSString::unsafeView(ExecState& state) const
+{
+    if (isRope())
+        return static_cast<const JSRopeString*>(this)->unsafeView(state);
+    return m_value;
+}
+
+ALWAYS_INLINE StringViewWithUnderlyingString JSString::viewWithUnderlyingString(ExecState& state) const
+{
+    if (isRope())
+        return static_cast<const JSRopeString&>(*this).viewWithUnderlyingString(state);
+    return { m_value, m_value };
+}
+
+inline bool JSString::isSubstring() const
+{
+    return isRope() && static_cast<const JSRopeString*>(this)->isSubstring();
+}
+
+inline JSString::SafeView::SafeView()
+{
+}
+
+inline JSString::SafeView::SafeView(ExecState& state, const JSString& string)
+    : m_state(&state)
+    , m_string(&string)
+{
+}
+
+inline JSString::SafeView::operator StringView() const
+{
+    return m_string->unsafeView(*m_state);
+}
+
+inline StringView JSString::SafeView::get() const
+{
+    return *this;
+}
+
+ALWAYS_INLINE JSString::SafeView JSString::view(ExecState* exec) const
+{
+    return SafeView(*exec, *this);
+}
 
 // --- JSValue inlines ----------------------------
@@ -681 +774 @@
 }
 
-ALWAYS_INLINE StringView JSRopeString::view(ExecState* exec) const
-{
-    if (isSubstring()) {
-        if (is8Bit())
-            return StringView(substringBase()->m_value.characters8() + substringOffset(), m_length);
-        return StringView(substringBase()->m_value.characters16() + substringOffset(), m_length);
-    }
-    resolveRope(exec);
-    return StringView(m_value);
-}
-
-ALWAYS_INLINE StringViewWithUnderlyingString JSRopeString::viewWithUnderlyingString(ExecState& state) const
-{
-    if (isSubstring()) {
-        auto& base = substringBase()->m_value;
-        if (is8Bit())
-            return { { base.characters8() + substringOffset(), m_length }, base };
-        return { { base.characters16() + substringOffset(), m_length }, base };
-    }
-    resolveRope(&state);
-    return { m_value, m_value };
-}
-
-ALWAYS_INLINE StringView JSString::view(ExecState* exec) const
-{
-    if (isRope())
-        return static_cast<const JSRopeString*>(this)->view(exec);
-    return StringView(m_value);
-}
-
-ALWAYS_INLINE StringViewWithUnderlyingString JSString::viewWithUnderlyingString(ExecState& state) const
-{
-    if (isRope())
-        return static_cast<const JSRopeString&>(*this).viewWithUnderlyingString(state);
-    return { m_value, m_value };
-}
-
-inline bool JSString::isSubstring() const
-{
-    return isRope() && static_cast<const JSRopeString*>(this)->isSubstring();
-}
-
 } // namespace JSC