Changeset 244996 in webkit

Timestamp:

May 6, 2019 5:29:09 PM (5 years ago)

Author:

ysuzuki@apple.com

Message:

[JSC] We should check OOM for description string of Symbol
​https://bugs.webkit.org/show_bug.cgi?id=197634

Reviewed by Keith Miller.

JSTests:

• stress/check-symbol-description-oom.js: Added.

(shouldThrow):

Source/JavaScriptCore:

When resoling JSString for description of Symbol, we should check OOM error.
We also change JSValueMakeSymbol(..., nullptr) to returning a symbol value
without description, (1) to simplify the code and (2) give a way for JSC API
to create a symbol value without description.

• API/JSValueRef.cpp:

(JSValueMakeSymbol):

• API/tests/testapi.cpp:

(TestAPI::symbolsTypeof):
(TestAPI::symbolsDescription):
(testCAPIViaCpp):

• dfg/DFGOperations.cpp:
• runtime/Symbol.cpp:

(JSC::Symbol::createWithDescription):

• runtime/Symbol.h:
• runtime/SymbolConstructor.cpp:

(JSC::callSymbol):

Location:

trunk

Files:

• JSTests/ChangeLog (modified) (1 diff)
• JSTests/stress/check-symbol-description-oom.js (added)
• Source/JavaScriptCore/API/JSValueRef.cpp (modified) (1 diff)
• Source/JavaScriptCore/API/tests/testapi.cpp (modified) (4 diffs)
• Source/JavaScriptCore/ChangeLog (modified) (1 diff)
• Source/JavaScriptCore/dfg/DFGOperations.cpp (modified) (1 diff)
• Source/JavaScriptCore/runtime/Symbol.cpp (modified) (1 diff)
• Source/JavaScriptCore/runtime/Symbol.h (modified) (1 diff)
• Source/JavaScriptCore/runtime/SymbolConstructor.cpp (modified) (1 diff)

trunk/JSTests/ChangeLog

@@ +1 @@
+2019-05-06  Yusuke Suzuki  <ysuzuki@apple.com>
+
+        [JSC] We should check OOM for description string of Symbol
+        https://bugs.webkit.org/show_bug.cgi?id=197634
+
+        Reviewed by Keith Miller.
+
+        * stress/check-symbol-description-oom.js: Added.
+        (shouldThrow):
+
 2019-05-06  Yusuke Suzuki  <ysuzuki@apple.com>
 

trunk/Source/JavaScriptCore/API/JSValueRef.cpp

@@ -332 +332 @@
     }
     ExecState* exec = toJS(ctx);
-    JSLockHolder locker(exec);
-    auto scope = DECLARE_CATCH_SCOPE(exec->vm());
-
-    JSString* jsDescription = jsString(exec, description ? description->string() : String());
-    RETURN_IF_EXCEPTION(scope, nullptr);
-
-    return toRef(exec, Symbol::create(exec, jsDescription));
+    VM& vm = exec->vm();
+    JSLockHolder locker(exec);
+
+    if (!description)
+        return toRef(exec, Symbol::create(vm));
+    return toRef(exec, Symbol::createWithDescription(vm, description->string()));
 }
 

trunk/Source/JavaScriptCore/API/tests/testapi.cpp

@@ -131 +131 @@
     void basicSymbol();
     void symbolsTypeof();
+    void symbolsDescription();
     void symbolsGetPropertyForKey();
     void symbolsSetPropertyForKey();
@@ -269 +270 @@
 
 static const char* isSymbolFunction = "(function isSymbol(symbol) { return typeof(symbol) === 'symbol'; })";
+static const char* getSymbolDescription = "(function getSymbolDescription(symbol) { return symbol.description; })";
 static const char* getFunction = "(function get(object, key) { return object[key]; })";
 static const char* setFunction = "(function set(object, key, value) { object[key] = value; })";
@@ -282 +284 @@
 void TestAPI::symbolsTypeof()
 {
-    APIString description("dope");
-    JSValueRef symbol = JSValueMakeSymbol(context, description);
-    check(functionReturnsTrue(isSymbolFunction, symbol), "JSValueMakeSymbol makes a symbol value");
+    {
+        JSValueRef symbol = JSValueMakeSymbol(context, nullptr);
+        check(functionReturnsTrue(isSymbolFunction, symbol), "JSValueMakeSymbol makes a symbol value");
+    }
+    {
+        APIString description("dope");
+        JSValueRef symbol = JSValueMakeSymbol(context, description);
+        check(functionReturnsTrue(isSymbolFunction, symbol), "JSValueMakeSymbol makes a symbol value");
+    }
+}
+
+void TestAPI::symbolsDescription()
+{
+    {
+        JSValueRef symbol = JSValueMakeSymbol(context, nullptr);
+        auto result = callFunction(getSymbolDescription, symbol);
+        check(JSValueIsStrictEqual(context, result.value(), JSValueMakeUndefined(context)), "JSValueMakeSymbol with nullptr description produces a symbol value without description");
+    }
+    {
+        APIString description("dope");
+        JSValueRef symbol = JSValueMakeSymbol(context, description);
+        auto result = callFunction(getSymbolDescription, symbol);
+        check(JSValueIsStrictEqual(context, result.value(), JSValueMakeString(context, description)), "JSValueMakeSymbol with description string produces a symbol value with description");
+    }
 }
 
@@ -492 +515 @@
     RUN(basicSymbol());
     RUN(symbolsTypeof());
+    RUN(symbolsDescription());
     RUN(symbolsGetPropertyForKey());
     RUN(symbolsSetPropertyForKey());

trunk/Source/JavaScriptCore/ChangeLog

@@ +1 @@
+2019-05-06  Yusuke Suzuki  <ysuzuki@apple.com>
+
+        [JSC] We should check OOM for description string of Symbol
+        https://bugs.webkit.org/show_bug.cgi?id=197634
+
+        Reviewed by Keith Miller.
+
+        When resoling JSString for description of Symbol, we should check OOM error.
+        We also change JSValueMakeSymbol(..., nullptr) to returning a symbol value
+        without description, (1) to simplify the code and (2) give a way for JSC API
+        to create a symbol value without description.
+
+        * API/JSValueRef.cpp:
+        (JSValueMakeSymbol):
+        * API/tests/testapi.cpp:
+        (TestAPI::symbolsTypeof):
+        (TestAPI::symbolsDescription):
+        (testCAPIViaCpp):
+        * dfg/DFGOperations.cpp:
+        * runtime/Symbol.cpp:
+        (JSC::Symbol::createWithDescription):
+        * runtime/Symbol.h:
+        * runtime/SymbolConstructor.cpp:
+        (JSC::callSymbol):
+
 2019-05-06  Keith Rollin  <krollin@apple.com>
 

trunk/Source/JavaScriptCore/dfg/DFGOperations.cpp

@@ -2292 +2292 @@
     VM& vm = exec->vm();
     NativeCallFrameTracer tracer(&vm, exec);
-
-    return Symbol::create(exec, description);
+    auto scope = DECLARE_THROW_SCOPE(vm);
+
+    String string = description->value(exec);
+    RETURN_IF_EXCEPTION(scope, nullptr);
+
+    return Symbol::createWithDescription(vm, string);
 }
 

trunk/Source/JavaScriptCore/runtime/Symbol.cpp

@@ -117 +117 @@
 }
 
-Symbol* Symbol::create(ExecState* exec, JSString* description)
+Symbol* Symbol::createWithDescription(VM& vm, const String& description)
 {
-    VM& vm = exec->vm();
-    String desc = description->value(exec);
-    Symbol* symbol = new (NotNull, allocateCell<Symbol>(vm.heap)) Symbol(vm, desc);
+    Symbol* symbol = new (NotNull, allocateCell<Symbol>(vm.heap)) Symbol(vm, description);
     symbol->finishCreation(vm);
     return symbol;

trunk/Source/JavaScriptCore/runtime/Symbol.h

@@ -47 +47 @@
 
     static Symbol* create(VM&);
-    static Symbol* create(ExecState*, JSString* description);
+    static Symbol* createWithDescription(VM&, const String&);
     JS_EXPORT_PRIVATE static Symbol* create(VM&, SymbolImpl& uid);
 

trunk/Source/JavaScriptCore/runtime/SymbolConstructor.cpp

@@ -80 +80 @@
 static EncodedJSValue JSC_HOST_CALL callSymbol(ExecState* exec)
 {
+    VM& vm = exec->vm();
+    auto scope = DECLARE_THROW_SCOPE(vm);
+
     JSValue description = exec->argument(0);
     if (description.isUndefined())
-        return JSValue::encode(Symbol::create(exec->vm()));
-    return JSValue::encode(Symbol::create(exec, description.toString(exec)));
+        return JSValue::encode(Symbol::create(vm));
+
+    String string = description.toWTFString(exec);
+    RETURN_IF_EXCEPTION(scope, { });
+    return JSValue::encode(Symbol::createWithDescription(vm, string));
 }