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

Timestamp:

Dec 18, 2015, 6:32:46 PM (9 years ago)

Author:

[email protected]

Message:

Make JSString::SafeView less of a footgun.
<​https://webkit.org/b/152376>

Reviewed by Darin Adler.

Remove the "operator StringView()" convenience helper on JSString::SafeString since that
made it possible to casually turn the return value from JSString::view() into an unsafe
StringView local on the stack with this pattern:

StringView view = someJSValue.toString(exec)->view(exec);

The JSString* returned by toString() above will go out of scope by the end of the statement
and does not stick around to protect itself from garbage collection.

It will now look like this instead:

JSString::SafeView view = someJSValue.toString(exec)->view(exec);

To be extra clear, the following is not safe:

StringView view = someJSValue.toString(exec)->view(exec).get();

By the end of that statement, the JSString::SafeView goes out of scope, and the JSString*
is no longer protected from GC.

I added a couple of forwarding helpers to the SafeView class, and if you need a StringView
object from it, you can call .get() just like before.

Finally I also removed the JSString::SafeView() constructor, since nobody was instantiating
empty SafeView objects anyway. This way we don't have to worry about null members.

• runtime/ArrayPrototype.cpp:

(JSC::arrayProtoFuncJoin):

• runtime/FunctionConstructor.cpp:

(JSC::constructFunctionSkippingEvalEnabledCheck):

• runtime/JSGenericTypedArrayViewPrototypeFunctions.h:

(JSC::genericTypedArrayViewProtoFuncJoin):

• runtime/JSGlobalObjectFunctions.cpp:

(JSC::decode):
(JSC::globalFuncParseInt):
(JSC::globalFuncParseFloat):
(JSC::globalFuncEscape):
(JSC::globalFuncUnescape):

• runtime/JSONObject.cpp:

(JSC::JSONProtoFuncParse):

• runtime/JSString.cpp:

(JSC::JSString::getPrimitiveNumber):
(JSC::JSString::toNumber):

• runtime/JSString.h:

(JSC::JSString::SafeView::is8Bit):
(JSC::JSString::SafeView::length):
(JSC::JSString::SafeView::characters8):
(JSC::JSString::SafeView::characters16):
(JSC::JSString::SafeView::operator[]):
(JSC::JSString::SafeView::SafeView):
(JSC::JSString::SafeView::get):
(JSC::JSString::SafeView::operator StringView): Deleted.

• runtime/StringPrototype.cpp:

(JSC::stringProtoFuncCharAt):
(JSC::stringProtoFuncCharCodeAt):
(JSC::stringProtoFuncIndexOf):
(JSC::stringProtoFuncNormalize):

File:

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

trunk/Source/JavaScriptCore/runtime/JSString.h

@@ -433 +433 @@
 class JSString::SafeView {
 public:
-    SafeView();
     explicit SafeView(ExecState&, const JSString&);
-    operator StringView() const;
     StringView get() const;
 
+    bool is8Bit() const { return m_string->is8Bit(); }
+    unsigned length() const { return m_string->length(); }
+    const LChar* characters8() const { return get().characters8(); }
+    const UChar* characters16() const { return get().characters16(); }
+    UChar operator[](unsigned index) const { return get()[index]; }
+
 private:
-    ExecState* m_state { nullptr };
+    ExecState& m_state;
 
     // The following pointer is marked "volatile" to make the compiler leave it on the stack
@@ -445 +449 @@
     // 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 };
+    const JSString* volatile m_string;
 };
 
@@ -708 +712 @@
 }
 
-inline JSString::SafeView::SafeView()
-{
-}
-
 inline JSString::SafeView::SafeView(ExecState& state, const JSString& string)
-    : m_state(&state)
+    : 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;
+    return m_string->unsafeView(m_state);
 }