Diff
Modified: trunk/Source/_javascript_Core/ChangeLog (194309 => 194310)
--- trunk/Source/_javascript_Core/ChangeLog 2015-12-19 02:07:57 UTC (rev 194309)
+++ trunk/Source/_javascript_Core/ChangeLog 2015-12-19 02:32:46 UTC (rev 194310)
@@ -1,3 +1,68 @@
+2015-12-18 Andreas Kling <[email protected]>
+
+ 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):
+
2015-12-18 Saam barati <[email protected]>
BytecodeGenerator::pushLexicalScopeInternal and pushLexicalScope should use enums instead of bools
Modified: trunk/Source/_javascript_Core/runtime/ArrayPrototype.cpp (194309 => 194310)
--- trunk/Source/_javascript_Core/runtime/ArrayPrototype.cpp 2015-12-19 02:07:57 UTC (rev 194309)
+++ trunk/Source/_javascript_Core/runtime/ArrayPrototype.cpp 2015-12-19 02:32:46 UTC (rev 194310)
@@ -496,7 +496,7 @@
JSString* separator = separatorValue.toString(exec);
if (exec->hadException())
return JSValue::encode(jsUndefined());
- return JSValue::encode(join(*exec, thisObject, separator->view(exec)));
+ return JSValue::encode(join(*exec, thisObject, separator->view(exec).get()));
}
EncodedJSValue JSC_HOST_CALL arrayProtoFuncConcat(ExecState* exec)
Modified: trunk/Source/_javascript_Core/runtime/FunctionConstructor.cpp (194309 => 194310)
--- trunk/Source/_javascript_Core/runtime/FunctionConstructor.cpp 2015-12-19 02:07:57 UTC (rev 194309)
+++ trunk/Source/_javascript_Core/runtime/FunctionConstructor.cpp 2015-12-19 02:32:46 UTC (rev 194310)
@@ -105,13 +105,13 @@
builder.append('*');
builder.append(functionName.string());
builder.append('(');
- builder.append(args.at(0).toString(exec)->view(exec));
+ builder.append(args.at(0).toString(exec)->view(exec).get());
for (size_t i = 1; i < args.size() - 1; i++) {
builder.appendLiteral(", ");
- builder.append(args.at(i).toString(exec)->view(exec));
+ builder.append(args.at(i).toString(exec)->view(exec).get());
}
builder.appendLiteral(") {\n");
- builder.append(args.at(args.size() - 1).toString(exec)->view(exec));
+ builder.append(args.at(args.size() - 1).toString(exec)->view(exec).get());
builder.appendLiteral("\n}}");
program = builder.toString();
}
Modified: trunk/Source/_javascript_Core/runtime/IntlCollatorPrototype.cpp (194309 => 194310)
--- trunk/Source/_javascript_Core/runtime/IntlCollatorPrototype.cpp 2015-12-19 02:07:57 UTC (rev 194309)
+++ trunk/Source/_javascript_Core/runtime/IntlCollatorPrototype.cpp 2015-12-19 02:32:46 UTC (rev 194310)
@@ -105,7 +105,7 @@
return JSValue::encode(jsUndefined());
// 9. Return CompareStrings(collator, X, Y).
- return JSValue::encode(collator->compareStrings(*state, x->view(state), y->view(state)));
+ return JSValue::encode(collator->compareStrings(*state, x->view(state).get(), y->view(state).get()));
}
EncodedJSValue JSC_HOST_CALL IntlCollatorPrototypeGetterCompare(ExecState* state)
Modified: trunk/Source/_javascript_Core/runtime/JSGenericTypedArrayViewPrototypeFunctions.h (194309 => 194310)
--- trunk/Source/_javascript_Core/runtime/JSGenericTypedArrayViewPrototypeFunctions.h 2015-12-19 02:07:57 UTC (rev 194309)
+++ trunk/Source/_javascript_Core/runtime/JSGenericTypedArrayViewPrototypeFunctions.h 2015-12-19 02:32:46 UTC (rev 194310)
@@ -182,35 +182,31 @@
EncodedJSValue JSC_HOST_CALL genericTypedArrayViewProtoFuncJoin(ExecState* exec)
{
// 22.2.3.14
- ViewClass* thisObject = jsCast<ViewClass*>(exec->thisValue());
+ auto joinWithSeparator = [&] (StringView separator) -> EncodedJSValue {
+ ViewClass* thisObject = jsCast<ViewClass*>(exec->thisValue());
+ unsigned length = thisObject->length();
- unsigned length = thisObject->length();
+ JSStringJoiner joiner(*exec, separator, length);
+ if (exec->hadException())
+ return JSValue::encode(jsUndefined());
+ for (unsigned i = 0; i < length; i++) {
+ joiner.append(*exec, thisObject->getIndexQuickly(i));
+ if (exec->hadException())
+ return JSValue::encode(jsUndefined());
+ }
+ return JSValue::encode(joiner.join(*exec));
+ };
JSValue separatorValue = exec->argument(0);
- JSString* separatorString;
- StringView separator;
-
if (separatorValue.isUndefined()) {
const LChar* comma = reinterpret_cast<const LChar*>(",");
- separator = { comma, 1 };
- } else {
- separatorString = separatorValue.toString(exec);
- if (exec->hadException())
- return JSValue::encode(jsUndefined());
- separator = separatorString->view(exec);
+ return joinWithSeparator({ comma, 1 });
}
- JSStringJoiner joiner(*exec, separator, length);
+ JSString* separatorString = separatorValue.toString(exec);
if (exec->hadException())
return JSValue::encode(jsUndefined());
-
- for (unsigned i = 0; i < length; i++) {
- joiner.append(*exec, thisObject->getIndexQuickly(i));
- if (exec->hadException())
- return JSValue::encode(jsUndefined());
- }
-
- return JSValue::encode(joiner.join(*exec));
+ return joinWithSeparator(separatorString->view(exec).get());
}
template<typename ViewClass>
Modified: trunk/Source/_javascript_Core/runtime/JSGlobalObjectFunctions.cpp (194309 => 194310)
--- trunk/Source/_javascript_Core/runtime/JSGlobalObjectFunctions.cpp 2015-12-19 02:07:57 UTC (rev 194309)
+++ trunk/Source/_javascript_Core/runtime/JSGlobalObjectFunctions.cpp 2015-12-19 02:32:46 UTC (rev 194310)
@@ -150,7 +150,7 @@
static JSValue decode(ExecState* exec, const Bitmap<256>& doNotUnescape, bool strict)
{
- StringView str = exec->argument(0).toString(exec)->view(exec);
+ JSString::SafeView str = exec->argument(0).toString(exec)->view(exec);
if (str.is8Bit())
return decode(exec, str.characters8(), str.length(), doNotUnescape, strict);
@@ -617,16 +617,16 @@
}
// If ToString throws, we shouldn't call ToInt32.
- StringView s = value.toString(exec)->view(exec);
+ JSString::SafeView s = value.toString(exec)->view(exec);
if (exec->hadException())
return JSValue::encode(jsUndefined());
- return JSValue::encode(jsNumber(parseInt(s, radixValue.toInt32(exec))));
+ return JSValue::encode(jsNumber(parseInt(s.get(), radixValue.toInt32(exec))));
}
EncodedJSValue JSC_HOST_CALL globalFuncParseFloat(ExecState* exec)
{
- return JSValue::encode(jsNumber(parseFloat(exec->argument(0).toString(exec)->view(exec))));
+ return JSValue::encode(jsNumber(parseFloat(exec->argument(0).toString(exec)->view(exec).get())));
}
EncodedJSValue JSC_HOST_CALL globalFuncIsNaN(ExecState* exec)
@@ -689,7 +689,7 @@
);
JSStringBuilder builder;
- StringView str = exec->argument(0).toString(exec)->view(exec);
+ JSString::SafeView str = exec->argument(0).toString(exec)->view(exec);
if (str.is8Bit()) {
const LChar* c = str.characters8();
for (unsigned k = 0; k < str.length(); k++, c++) {
@@ -727,7 +727,7 @@
EncodedJSValue JSC_HOST_CALL globalFuncUnescape(ExecState* exec)
{
StringBuilder builder;
- StringView str = exec->argument(0).toString(exec)->view(exec);
+ JSString::SafeView str = exec->argument(0).toString(exec)->view(exec);
int k = 0;
int len = str.length();
Modified: trunk/Source/_javascript_Core/runtime/JSONObject.cpp (194309 => 194310)
--- trunk/Source/_javascript_Core/runtime/JSONObject.cpp 2015-12-19 02:07:57 UTC (rev 194309)
+++ trunk/Source/_javascript_Core/runtime/JSONObject.cpp 2015-12-19 02:32:46 UTC (rev 194310)
@@ -729,7 +729,7 @@
{
if (!exec->argumentCount())
return throwVMError(exec, createError(exec, ASCIILiteral("JSON.parse requires at least one parameter")));
- StringView source = exec->uncheckedArgument(0).toString(exec)->view(exec);
+ JSString::SafeView source = exec->uncheckedArgument(0).toString(exec)->view(exec);
if (exec->hadException())
return JSValue::encode(jsNull());
Modified: trunk/Source/_javascript_Core/runtime/JSString.cpp (194309 => 194310)
--- trunk/Source/_javascript_Core/runtime/JSString.cpp 2015-12-19 02:07:57 UTC (rev 194309)
+++ trunk/Source/_javascript_Core/runtime/JSString.cpp 2015-12-19 02:32:46 UTC (rev 194310)
@@ -378,13 +378,13 @@
bool JSString::getPrimitiveNumber(ExecState* exec, double& number, JSValue& result) const
{
result = this;
- number = jsToNumber(view(exec));
+ number = jsToNumber(unsafeView(*exec));
return false;
}
double JSString::toNumber(ExecState* exec) const
{
- return jsToNumber(view(exec));
+ return jsToNumber(unsafeView(*exec));
}
inline StringObject* StringObject::create(VM& vm, JSGlobalObject* globalObject, JSString* string)
Modified: trunk/Source/_javascript_Core/runtime/JSString.h (194309 => 194310)
--- trunk/Source/_javascript_Core/runtime/JSString.h 2015-12-19 02:07:57 UTC (rev 194309)
+++ trunk/Source/_javascript_Core/runtime/JSString.h 2015-12-19 02:32:46 UTC (rev 194310)
@@ -432,19 +432,23 @@
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
// 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 };
+ const JSString* volatile m_string;
};
JS_EXPORT_PRIVATE JSString* jsStringWithCacheSlowCase(VM&, StringImpl&);
@@ -707,24 +711,15 @@
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_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);
}
ALWAYS_INLINE JSString::SafeView JSString::view(ExecState* exec) const
Modified: trunk/Source/_javascript_Core/runtime/StringPrototype.cpp (194309 => 194310)
--- trunk/Source/_javascript_Core/runtime/StringPrototype.cpp 2015-12-19 02:07:57 UTC (rev 194309)
+++ trunk/Source/_javascript_Core/runtime/StringPrototype.cpp 2015-12-19 02:32:46 UTC (rev 194310)
@@ -798,7 +798,7 @@
JSValue thisValue = exec->thisValue();
if (!checkObjectCoercible(thisValue))
return throwVMTypeError(exec);
- StringView string = thisValue.toString(exec)->view(exec);
+ JSString::SafeView string = thisValue.toString(exec)->view(exec);
JSValue a0 = exec->argument(0);
if (a0.isUInt32()) {
uint32_t i = a0.asUInt32();
@@ -817,7 +817,7 @@
JSValue thisValue = exec->thisValue();
if (!checkObjectCoercible(thisValue))
return throwVMTypeError(exec);
- StringView string = thisValue.toString(exec)->view(exec);
+ JSString::SafeView string = thisValue.toString(exec)->view(exec);
JSValue a0 = exec->argument(0);
if (a0.isUInt32()) {
uint32_t i = a0.asUInt32();
@@ -909,7 +909,7 @@
if (thisJSString->length() < otherJSString->length() + pos)
return JSValue::encode(jsNumber(-1));
- size_t result = thisJSString->view(exec).get().find(otherJSString->view(exec), pos);
+ size_t result = thisJSString->view(exec).get().find(otherJSString->view(exec).get(), pos);
if (result == notFound)
return JSValue::encode(jsNumber(-1));
return JSValue::encode(jsNumber(result));
@@ -1930,7 +1930,7 @@
JSValue thisValue = exec->thisValue();
if (!checkObjectCoercible(thisValue))
return throwVMTypeError(exec);
- StringView source = thisValue.toString(exec)->view(exec);
+ JSString::SafeView source = thisValue.toString(exec)->view(exec);
if (exec->hadException())
return JSValue::encode(jsUndefined());
@@ -1953,7 +1953,7 @@
return throwVMError(exec, createRangeError(exec, ASCIILiteral("argument does not match any normalization form")));
}
- return JSValue::encode(normalize(exec, source.upconvertedCharacters(), source.length(), form));
+ return JSValue::encode(normalize(exec, source.get().upconvertedCharacters(), source.length(), form));
}
} // namespace JSC
