Diff
Modified: trunk/Source/_javascript_Core/ChangeLog (208766 => 208767)
--- trunk/Source/_javascript_Core/ChangeLog 2016-11-16 00:09:06 UTC (rev 208766)
+++ trunk/Source/_javascript_Core/ChangeLog 2016-11-16 00:19:54 UTC (rev 208767)
@@ -1,3 +1,48 @@
+2016-11-15 Mark Lam <[email protected]>
+
+ Remove JSString::SafeView and replace its uses with StringViewWithUnderlyingString.
+ https://bugs.webkit.org/show_bug.cgi?id=164777
+
+ Reviewed by Geoffrey Garen.
+
+ JSString::SafeView no longer achieves its intended goal to make it easier to
+ handle strings safely. Its clients still need to do explicit exception checks in
+ order to be correct. We'll remove it and replace its uses with
+ StringViewWithUnderlyingString instead which serves to gets the a StringView
+ (which is what we really wanted from SafeView) and keeps the backing String alive
+ while the view is in use.
+
+ Also added some missing exception checks.
+
+ * jsc.cpp:
+ (printInternal):
+ (functionDebug):
+ * runtime/ArrayPrototype.cpp:
+ (JSC::arrayProtoFuncJoin):
+ * runtime/FunctionConstructor.cpp:
+ (JSC::constructFunctionSkippingEvalEnabledCheck):
+ * runtime/IntlCollatorPrototype.cpp:
+ (JSC::IntlCollatorFuncCompare):
+ * runtime/JSGenericTypedArrayViewPrototypeFunctions.h:
+ (JSC::genericTypedArrayViewProtoFuncJoin):
+ * runtime/JSGlobalObjectFunctions.cpp:
+ (JSC::toStringView):
+ (JSC::globalFuncParseFloat):
+ * runtime/JSONObject.cpp:
+ (JSC::JSONProtoFuncParse):
+ * runtime/JSString.h:
+ (JSC::JSString::SafeView::is8Bit): Deleted.
+ (JSC::JSString::SafeView::length): Deleted.
+ (JSC::JSString::SafeView::SafeView): Deleted.
+ (JSC::JSString::SafeView::get): Deleted.
+ (JSC::JSString::view): Deleted.
+ * runtime/StringPrototype.cpp:
+ (JSC::stringProtoFuncRepeatCharacter):
+ (JSC::stringProtoFuncCharAt):
+ (JSC::stringProtoFuncCharCodeAt):
+ (JSC::stringProtoFuncIndexOf):
+ (JSC::stringProtoFuncNormalize):
+
2016-11-15 Filip Pizlo <[email protected]>
Unreviewed, remove bogus assertion.
Modified: trunk/Source/_javascript_Core/jsc.cpp (208766 => 208767)
--- trunk/Source/_javascript_Core/jsc.cpp 2016-11-16 00:09:06 UTC (rev 208766)
+++ trunk/Source/_javascript_Core/jsc.cpp 2016-11-16 00:19:54 UTC (rev 208767)
@@ -1531,6 +1531,9 @@
static EncodedJSValue printInternal(ExecState* exec, FILE* out)
{
+ VM& vm = exec->vm();
+ auto scope = DECLARE_THROW_SCOPE(vm);
+
if (test262AsyncTest) {
JSValue value = exec->argument(0);
if (value.isString() && WTF::equal(asString(value)->value(exec).impl(), "Test262:AsyncTestComplete"))
@@ -1543,7 +1546,9 @@
if (EOF == fputc(' ', out))
goto fail;
- if (fprintf(out, "%s", exec->uncheckedArgument(i).toString(exec)->view(exec).get().utf8().data()) < 0)
+ auto viewWithString = exec->uncheckedArgument(i).toString(exec)->viewWithUnderlyingString(*exec);
+ RETURN_IF_EXCEPTION(scope, encodedJSValue());
+ if (fprintf(out, "%s", viewWithString.view.utf8().data()) < 0)
goto fail;
}
@@ -1569,7 +1574,11 @@
EncodedJSValue JSC_HOST_CALL functionDebug(ExecState* exec)
{
- fprintf(stderr, "--> %s\n", exec->argument(0).toString(exec)->view(exec).get().utf8().data());
+ VM& vm = exec->vm();
+ auto scope = DECLARE_THROW_SCOPE(vm);
+ auto viewWithString = exec->argument(0).toString(exec)->viewWithUnderlyingString(*exec);
+ RETURN_IF_EXCEPTION(scope, encodedJSValue());
+ fprintf(stderr, "--> %s\n", viewWithString.view.utf8().data());
return JSValue::encode(jsUndefined());
}
Modified: trunk/Source/_javascript_Core/runtime/ArrayPrototype.cpp (208766 => 208767)
--- trunk/Source/_javascript_Core/runtime/ArrayPrototype.cpp 2016-11-16 00:09:06 UTC (rev 208766)
+++ trunk/Source/_javascript_Core/runtime/ArrayPrototype.cpp 2016-11-16 00:19:54 UTC (rev 208767)
@@ -667,7 +667,11 @@
return JSValue::encode(slowJoin(*exec, thisObject, jsSeparator, length64));
}
- return JSValue::encode(fastJoin(*exec, thisObject, jsSeparator->view(exec).get(), length));
+ auto viewWithString = jsSeparator->viewWithUnderlyingString(*exec);
+ RETURN_IF_EXCEPTION(scope, encodedJSValue());
+
+ scope.release();
+ return JSValue::encode(fastJoin(*exec, thisObject, viewWithString.view, length));
}
EncodedJSValue JSC_HOST_CALL arrayProtoFuncPop(ExecState* exec)
Modified: trunk/Source/_javascript_Core/runtime/FunctionConstructor.cpp (208766 => 208767)
--- trunk/Source/_javascript_Core/runtime/FunctionConstructor.cpp 2016-11-16 00:09:06 UTC (rev 208766)
+++ trunk/Source/_javascript_Core/runtime/FunctionConstructor.cpp 2016-11-16 00:19:54 UTC (rev 208767)
@@ -123,13 +123,19 @@
builder.append(prefix);
builder.append(functionName.string());
builder.append('(');
- builder.append(args.at(0).toString(exec)->view(exec).get());
+ auto viewWithString = args.at(0).toString(exec)->viewWithUnderlyingString(*exec);
+ RETURN_IF_EXCEPTION(scope, nullptr);
+ builder.append(viewWithString.view);
for (size_t i = 1; i < args.size() - 1; i++) {
builder.appendLiteral(", ");
- builder.append(args.at(i).toString(exec)->view(exec).get());
+ auto viewWithString = args.at(i).toString(exec)->viewWithUnderlyingString(*exec);
+ RETURN_IF_EXCEPTION(scope, nullptr);
+ builder.append(viewWithString.view);
}
builder.appendLiteral(") {\n");
- builder.append(args.at(args.size() - 1).toString(exec)->view(exec).get());
+ viewWithString = args.at(args.size() - 1).toString(exec)->viewWithUnderlyingString(*exec);
+ RETURN_IF_EXCEPTION(scope, nullptr);
+ builder.append(viewWithString.view);
builder.appendLiteral("\n}}");
program = builder.toString();
}
Modified: trunk/Source/_javascript_Core/runtime/IntlCollatorPrototype.cpp (208766 => 208767)
--- trunk/Source/_javascript_Core/runtime/IntlCollatorPrototype.cpp 2016-11-16 00:09:06 UTC (rev 208766)
+++ trunk/Source/_javascript_Core/runtime/IntlCollatorPrototype.cpp 2016-11-16 00:19:54 UTC (rev 208767)
@@ -98,7 +98,12 @@
RETURN_IF_EXCEPTION(scope, encodedJSValue());
// 9. Return CompareStrings(collator, X, Y).
- return JSValue::encode(collator->compareStrings(*state, x->view(state).get(), y->view(state).get()));
+ auto xViewWithString = x->viewWithUnderlyingString(*state);
+ RETURN_IF_EXCEPTION(scope, encodedJSValue());
+ auto yViewWithString = y->viewWithUnderlyingString(*state);
+ RETURN_IF_EXCEPTION(scope, encodedJSValue());
+ scope.release();
+ return JSValue::encode(collator->compareStrings(*state, xViewWithString.view, yViewWithString.view));
}
EncodedJSValue JSC_HOST_CALL IntlCollatorPrototypeGetterCompare(ExecState* state)
Modified: trunk/Source/_javascript_Core/runtime/JSGenericTypedArrayViewPrototypeFunctions.h (208766 => 208767)
--- trunk/Source/_javascript_Core/runtime/JSGenericTypedArrayViewPrototypeFunctions.h 2016-11-16 00:09:06 UTC (rev 208766)
+++ trunk/Source/_javascript_Core/runtime/JSGenericTypedArrayViewPrototypeFunctions.h 2016-11-16 00:19:54 UTC (rev 208767)
@@ -287,7 +287,9 @@
if (thisObject->isNeutered())
return throwVMTypeError(exec, scope, typedArrayBufferHasBeenDetachedErrorMessage);
- return joinWithSeparator(separatorString->view(exec).get());
+ auto viewWithString = separatorString->viewWithUnderlyingString(*exec);
+ RETURN_IF_EXCEPTION(scope, encodedJSValue());
+ return joinWithSeparator(viewWithString.view);
}
template<typename ViewClass>
Modified: trunk/Source/_javascript_Core/runtime/JSGlobalObjectFunctions.cpp (208766 => 208767)
--- trunk/Source/_javascript_Core/runtime/JSGlobalObjectFunctions.cpp 2016-11-16 00:09:06 UTC (rev 208766)
+++ trunk/Source/_javascript_Core/runtime/JSGlobalObjectFunctions.cpp 2016-11-16 00:19:54 UTC (rev 208767)
@@ -65,10 +65,9 @@
JSString* string = value.toStringOrNull(exec);
if (UNLIKELY(!string))
return { };
- JSString::SafeView view = string->view(exec);
- StringView stringView = view.get();
+ auto viewWithString = string->viewWithUnderlyingString(*exec);
RETURN_IF_EXCEPTION(scope, { });
- return callback(stringView);
+ return callback(viewWithString.view);
}
template<unsigned charactersCount>
@@ -718,7 +717,8 @@
EncodedJSValue JSC_HOST_CALL globalFuncParseFloat(ExecState* exec)
{
- return JSValue::encode(jsNumber(parseFloat(exec->argument(0).toString(exec)->view(exec).get())));
+ auto viewWithString = exec->argument(0).toString(exec)->viewWithUnderlyingString(*exec);
+ return JSValue::encode(jsNumber(parseFloat(viewWithString.view)));
}
EncodedJSValue JSC_HOST_CALL globalFuncDecodeURI(ExecState* exec)
Modified: trunk/Source/_javascript_Core/runtime/JSONObject.cpp (208766 => 208767)
--- trunk/Source/_javascript_Core/runtime/JSONObject.cpp 2016-11-16 00:09:06 UTC (rev 208766)
+++ trunk/Source/_javascript_Core/runtime/JSONObject.cpp 2016-11-16 00:19:54 UTC (rev 208767)
@@ -761,10 +761,9 @@
if (!exec->argumentCount())
return throwVMError(exec, scope, createError(exec, ASCIILiteral("JSON.parse requires at least one parameter")));
- JSString::SafeView source = exec->uncheckedArgument(0).toString(exec)->view(exec);
+ auto viewWithString = exec->uncheckedArgument(0).toString(exec)->viewWithUnderlyingString(*exec);
RETURN_IF_EXCEPTION(scope, encodedJSValue());
- StringView view = source.get();
- RETURN_IF_EXCEPTION(scope, encodedJSValue());
+ StringView view = viewWithString.view;
JSValue unfiltered;
LocalScope localScope(vm);
Modified: trunk/Source/_javascript_Core/runtime/JSString.h (208766 => 208767)
--- trunk/Source/_javascript_Core/runtime/JSString.h 2016-11-16 00:09:06 UTC (rev 208766)
+++ trunk/Source/_javascript_Core/runtime/JSString.h 2016-11-16 00:19:54 UTC (rev 208767)
@@ -150,8 +150,6 @@
AtomicString toAtomicString(ExecState*) const;
RefPtr<AtomicStringImpl> toExistingAtomicString(ExecState*) const;
- class SafeView;
- SafeView view(ExecState*) const;
StringViewWithUnderlyingString viewWithUnderlyingString(ExecState&) const;
inline bool equal(ExecState*, JSString* other) const;
@@ -475,24 +473,6 @@
friend JSString* jsString(ExecState*, const String&, const String&, const String&);
};
-class JSString::SafeView {
-public:
- explicit SafeView(ExecState&, const JSString&);
- StringView get() const;
-
- bool is8Bit() const { return m_string->is8Bit(); }
- unsigned length() const { return m_string->length(); }
-
-private:
- ExecState& m_state;
-
- // 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;
-};
-
JS_EXPORT_PRIVATE JSString* jsStringWithCacheSlowCase(VM&, StringImpl&);
inline const StringImpl* JSString::tryGetValueImpl() const
@@ -759,22 +739,6 @@
return isRope() && static_cast<const JSRopeString*>(this)->isSubstring();
}
-inline JSString::SafeView::SafeView(ExecState& state, const JSString& string)
- : m_state(state)
- , m_string(&string)
-{
-}
-
-inline StringView JSString::SafeView::get() const
-{
- return m_string->unsafeView(m_state);
-}
-
-ALWAYS_INLINE JSString::SafeView JSString::view(ExecState* exec) const
-{
- return SafeView(*exec, *this);
-}
-
// --- JSValue inlines ----------------------------
inline bool JSValue::toBoolean(ExecState* exec) const
Modified: trunk/Source/_javascript_Core/runtime/StringPrototype.cpp (208766 => 208767)
--- trunk/Source/_javascript_Core/runtime/StringPrototype.cpp 2016-11-16 00:09:06 UTC (rev 208766)
+++ trunk/Source/_javascript_Core/runtime/StringPrototype.cpp 2016-11-16 00:19:54 UTC (rev 208767)
@@ -812,8 +812,8 @@
ASSERT(repeatCount >= 0);
ASSERT(!repeatCountValue.isDouble() || repeatCountValue.asDouble() == repeatCount);
- JSString::SafeView safeView = string->view(exec);
- StringView view = safeView.get();
+ auto viewWithString = string->viewWithUnderlyingString(*exec);
+ StringView view = viewWithString.view;
ASSERT(view.length() == 1 && !scope.exception());
UChar character = view[0];
scope.release();
@@ -906,10 +906,9 @@
JSValue thisValue = exec->thisValue();
if (!checkObjectCoercible(thisValue))
return throwVMTypeError(exec, scope);
- JSString::SafeView string = thisValue.toString(exec)->view(exec);
+ auto viewWithString = thisValue.toString(exec)->viewWithUnderlyingString(*exec);
RETURN_IF_EXCEPTION(scope, encodedJSValue());
- StringView view = string.get();
- RETURN_IF_EXCEPTION(scope, encodedJSValue());
+ StringView view = viewWithString.view;
JSValue a0 = exec->argument(0);
if (a0.isUInt32()) {
uint32_t i = a0.asUInt32();
@@ -931,11 +930,9 @@
JSValue thisValue = exec->thisValue();
if (!checkObjectCoercible(thisValue))
return throwVMTypeError(exec, scope);
- JSString* jsString = thisValue.toString(exec);
+ auto viewWithString = thisValue.toString(exec)->viewWithUnderlyingString(*exec);
RETURN_IF_EXCEPTION(scope, encodedJSValue());
- JSString::SafeView string = jsString->view(exec);
- StringView view = string.get();
- RETURN_IF_EXCEPTION(scope, encodedJSValue());
+ StringView view = viewWithString.view;
JSValue a0 = exec->argument(0);
if (a0.isUInt32()) {
uint32_t i = a0.asUInt32();
@@ -1035,7 +1032,11 @@
if (thisJSString->length() < otherJSString->length() + pos)
return JSValue::encode(jsNumber(-1));
- size_t result = thisJSString->view(exec).get().find(otherJSString->view(exec).get(), pos);
+ auto thisViewWithString = thisJSString->viewWithUnderlyingString(*exec);
+ RETURN_IF_EXCEPTION(scope, encodedJSValue());
+ auto otherViewWithString = otherJSString->viewWithUnderlyingString(*exec);
+ RETURN_IF_EXCEPTION(scope, encodedJSValue());
+ size_t result = thisViewWithString.view.find(otherViewWithString.view, pos);
if (result == notFound)
return JSValue::encode(jsNumber(-1));
return JSValue::encode(jsNumber(result));
@@ -2016,10 +2017,9 @@
JSValue thisValue = exec->thisValue();
if (!checkObjectCoercible(thisValue))
return throwVMTypeError(exec, scope);
- JSString::SafeView source = thisValue.toString(exec)->view(exec);
+ auto viewWithString = thisValue.toString(exec)->viewWithUnderlyingString(*exec);
RETURN_IF_EXCEPTION(scope, encodedJSValue());
- StringView view = source.get();
- RETURN_IF_EXCEPTION(scope, encodedJSValue());
+ StringView view = viewWithString.view;
UNormalizationMode form = UNORM_NFC;
// Verify that the argument is provided and is not undefined.