Attachment #369199 for bug #197634

View Details Raw Unified Return to Bug 197634



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 NOBODY (OOPS!).

        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  Yusuke Suzuki  <ysuzuki@apple.com>

        TemplateObject passed to template literal tags are not always identical for the same source location.

Lines 331-343 JSValueRef JSValueMakeSymbol(JSContextRef ctx, JSStringRef description) a/Source/JavaScriptCore/API/JSValueRef.cpp_sec1
- a/Source/JavaScriptCore/API/JSValueRef.cpp -5 / +4 lines
331	return nullptr;	331	return nullptr;
332	}	332	}
333	ExecState* exec = toJS(ctx);	333	ExecState* exec = toJS(ctx);
		334	VM& vm = exec->vm();
334	JSLockHolder locker(exec);	335	JSLockHolder locker(exec);
335	auto scope = DECLARE_CATCH_SCOPE(exec->vm());
336
337	JSString* jsDescription = jsString(exec, description ? description->string() : String());
338	RETURN_IF_EXCEPTION(scope, nullptr);
339		336
340	return toRef(exec, Symbol::create(exec, jsDescription));	337	if (!description)
		338	return toRef(exec, Symbol::create(vm));
		339	return toRef(exec, Symbol::createWithDescription(vm, description->string()));
341	}	340	}
342		341
343	JSValueRef JSValueMakeString(JSContextRef ctx, JSStringRef string)	342	JSValueRef JSValueMakeString(JSContextRef ctx, JSStringRef string)




    void basicSymbol();
    void symbolsTypeof();
    void symbolsDescription();
    void symbolsGetPropertyForKey();
    void symbolsSetPropertyForKey();
    void symbolsHasPropertyForKey();

}

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; })";



void TestAPI::symbolsTypeof()
{
    {
        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");
    }
}

void TestAPI::symbolsGetPropertyForKey()


    RUN(basicSymbol());
    RUN(symbolsTypeof());
    RUN(symbolsDescription());
    RUN(symbolsGetPropertyForKey());
    RUN(symbolsSetPropertyForKey());
    RUN(symbolsHasPropertyForKey());

Lines 2291-2298 Symbol* JIT_OPERATION operationNewSymbolWithDescription(ExecState* exec, JSStrin a/Source/JavaScriptCore/dfg/DFGOperations.cpp_sec1
- a/Source/JavaScriptCore/dfg/DFGOperations.cpp -1 / +5 lines
2291	{	2291	{
2292	VM& vm = exec->vm();	2292	VM& vm = exec->vm();
2293	NativeCallFrameTracer tracer(&vm, exec);	2293	NativeCallFrameTracer tracer(&vm, exec);
		2294	auto scope = DECLARE_THROW_SCOPE(vm);
		2295
		2296	String string = description->value(exec);
		2297	RETURN_IF_EXCEPTION(scope, nullptr);
2294		2298
2295	return Symbol::create(exec, description);	2299	return Symbol::createWithDescription(vm, string);
2296	}	2300	}
2297		2301
2298	JSCell* JIT_OPERATION operationNewStringObject(ExecState* exec, JSString* string, Structure* structure)	2302	JSCell* JIT_OPERATION operationNewStringObject(ExecState* exec, JSString* string, Structure* structure)



    return symbol;
}

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

Lines 46-52 class Symbol final : public JSCell { a/Source/JavaScriptCore/runtime/Symbol.h_sec1
- a/Source/JavaScriptCore/runtime/Symbol.h -1 / +1 lines
46	}	46	}
47		47
48	static Symbol* create(VM&);	48	static Symbol* create(VM&);
49	static Symbol* create(ExecState, JSString description);	49	static Symbol* createWithDescription(VM&, const String&);
50	JS_EXPORT_PRIVATE static Symbol* create(VM&, SymbolImpl& uid);	50	JS_EXPORT_PRIVATE static Symbol* create(VM&, SymbolImpl& uid);
51		51
52	PrivateName privateName() const { return m_privateName; }	52	PrivateName privateName() const { return m_privateName; }

Lines 79-88 void SymbolConstructor::finishCreation(VM& vm, SymbolPrototype* prototype) a/Source/JavaScriptCore/runtime/SymbolConstructor.cpp_sec1
- a/Source/JavaScriptCore/runtime/SymbolConstructor.cpp -2 / +8 lines
79		79
80	static EncodedJSValue JSC_HOST_CALL callSymbol(ExecState* exec)	80	static EncodedJSValue JSC_HOST_CALL callSymbol(ExecState* exec)
81	{	81	{
		82	VM& vm = exec->vm();
		83	auto scope = DECLARE_THROW_SCOPE(vm);
		84
82	JSValue description = exec->argument(0);	85	JSValue description = exec->argument(0);
83	if (description.isUndefined())	86	if (description.isUndefined())
84	return JSValue::encode(Symbol::create(exec->vm()));	87	return JSValue::encode(Symbol::create(vm));
85	return JSValue::encode(Symbol::create(exec, description.toString(exec)));	88
		89	String string = description.toWTFString(exec);
		90	RETURN_IF_EXCEPTION(scope, { });
		91	return JSValue::encode(Symbol::createWithDescription(vm, string));
86	}	92	}
87		93
88	EncodedJSValue JSC_HOST_CALL symbolConstructorFor(ExecState* exec)	94	EncodedJSValue JSC_HOST_CALL symbolConstructorFor(ExecState* exec)



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 NOBODY (OOPS!).

        * stress/check-symbol-description-oom.js: Added.
        (shouldThrow):

2019-05-06  Yusuke Suzuki  <ysuzuki@apple.com>

        Unreviewed, land one more test



function shouldThrow(func, errorMessage) {
    var errorThrown = false;
    var error = null;
    try {
        func();
    } catch (e) {
        errorThrown = true;
        error = e;
    }
    if (!errorThrown)
        throw new Error('not thrown');
    if (String(error) !== errorMessage)
        throw new Error(`bad error: ${String(error)}`);
}

const s0 = (-1).toLocaleString();
const s1 = s0.padEnd(2147483647, '  ');
shouldThrow(() => Symbol(s1), `Error: Out of memory`);

Return to Bug 197634

Lines 130-135 class TestAPI { a/Source/JavaScriptCore/API/tests/testapi.cpp_sec1
- a/Source/JavaScriptCore/API/tests/testapi.cpp -3 / +27 lines
130		130
131	void basicSymbol();	131	void basicSymbol();
132	void symbolsTypeof();	132	void symbolsTypeof();
		133	void symbolsDescription();
133	void symbolsGetPropertyForKey();	134	void symbolsGetPropertyForKey();
134	void symbolsSetPropertyForKey();	135	void symbolsSetPropertyForKey();
135	void symbolsHasPropertyForKey();	136	void symbolsHasPropertyForKey();
Lines 268-273 APIVector<JSValueRef> TestAPI::interestingKeys() a/Source/JavaScriptCore/API/tests/testapi.cpp_sec2
268	}	269	}
269		270
270	static const char* isSymbolFunction = "(function isSymbol(symbol) { return typeof(symbol) === 'symbol'; })";	271	static const char* isSymbolFunction = "(function isSymbol(symbol) { return typeof(symbol) === 'symbol'; })";
		272	static const char* getSymbolDescription = "(function getSymbolDescription(symbol) { return symbol.description; })";
271	static const char* getFunction = "(function get(object, key) { return object[key]; })";	273	static const char* getFunction = "(function get(object, key) { return object[key]; })";
272	static const char* setFunction = "(function set(object, key, value) { object[key] = value; })";	274	static const char* setFunction = "(function set(object, key, value) { object[key] = value; })";
273		275
Lines 281-289 void TestAPI::basicSymbol() a/Source/JavaScriptCore/API/tests/testapi.cpp_sec3
281		283
282	void TestAPI::symbolsTypeof()	284	void TestAPI::symbolsTypeof()
283	{	285	{
284	APIString description("dope");	286	{
285	JSValueRef symbol = JSValueMakeSymbol(context, description);	287	JSValueRef symbol = JSValueMakeSymbol(context, nullptr);
286	check(functionReturnsTrue(isSymbolFunction, symbol), "JSValueMakeSymbol makes a symbol value");	288	check(functionReturnsTrue(isSymbolFunction, symbol), "JSValueMakeSymbol makes a symbol value");
		289	}
		290	{
		291	APIString description("dope");
		292	JSValueRef symbol = JSValueMakeSymbol(context, description);
		293	check(functionReturnsTrue(isSymbolFunction, symbol), "JSValueMakeSymbol makes a symbol value");
		294	}
		295	}
		296
		297	void TestAPI::symbolsDescription()
		298	{
		299	{
		300	JSValueRef symbol = JSValueMakeSymbol(context, nullptr);
		301	auto result = callFunction(getSymbolDescription, symbol);
		302	check(JSValueIsStrictEqual(context, result.value(), JSValueMakeUndefined(context)), "JSValueMakeSymbol with nullptr description produces a symbol value without description");
		303	}
		304	{
		305	APIString description("dope");
		306	JSValueRef symbol = JSValueMakeSymbol(context, description);
		307	auto result = callFunction(getSymbolDescription, symbol);
		308	check(JSValueIsStrictEqual(context, result.value(), JSValueMakeString(context, description)), "JSValueMakeSymbol with description string produces a symbol value with description");
		309	}
287	}	310	}
288		311
289	void TestAPI::symbolsGetPropertyForKey()	312	void TestAPI::symbolsGetPropertyForKey()
Lines 491-496 int testCAPIViaCpp(const char* filter) a/Source/JavaScriptCore/API/tests/testapi.cpp_sec4
491		514
492	RUN(basicSymbol());	515	RUN(basicSymbol());
493	RUN(symbolsTypeof());	516	RUN(symbolsTypeof());
		517	RUN(symbolsDescription());
494	RUN(symbolsGetPropertyForKey());	518	RUN(symbolsGetPropertyForKey());
495	RUN(symbolsSetPropertyForKey());	519	RUN(symbolsSetPropertyForKey());
496	RUN(symbolsHasPropertyForKey());	520	RUN(symbolsHasPropertyForKey());

Lines 116-126 Symbol* Symbol::create(VM& vm) a/Source/JavaScriptCore/runtime/Symbol.cpp_sec1
- a/Source/JavaScriptCore/runtime/Symbol.cpp -4 / +2 lines
116	return symbol;	116	return symbol;
117	}	117	}
118		118
119	Symbol* Symbol::create(ExecState* exec, JSString* description)	119	Symbol* Symbol::createWithDescription(VM& vm, const String& description)
120	{	120	{
121	VM& vm = exec->vm();	121	Symbol* symbol = new (NotNull, allocateCell<Symbol>(vm.heap)) Symbol(vm, description);
122	String desc = description->value(exec);
123	Symbol* symbol = new (NotNull, allocateCell<Symbol>(vm.heap)) Symbol(vm, desc);
124	symbol->finishCreation(vm);	122	symbol->finishCreation(vm);
125	return symbol;	123	return symbol;
126	}	124	}

Lines 1-3 a/Source/JavaScriptCore/ChangeLog_sec1
- a/Source/JavaScriptCore/ChangeLog +25 lines
		1	2019-05-06 Yusuke Suzuki <ysuzuki@apple.com>
		2
		3	[JSC] We should check OOM for description string of Symbol
		4	https://bugs.webkit.org/show_bug.cgi?id=197634
		5
		6	Reviewed by NOBODY (OOPS!).
		7
		8	When resoling JSString for description of Symbol, we should check OOM error.
		9	We also change JSValueMakeSymbol(..., nullptr) to returning a symbol value
		10	without description, (1) to simplify the code and (2) give a way for JSC API
		11	to create a symbol value without description.
		12
		13	* API/JSValueRef.cpp:
		14	(JSValueMakeSymbol):
		15	* API/tests/testapi.cpp:
		16	(TestAPI::symbolsTypeof):
		17	(TestAPI::symbolsDescription):
		18	(testCAPIViaCpp):
		19	* dfg/DFGOperations.cpp:
		20	* runtime/Symbol.cpp:
		21	(JSC::Symbol::createWithDescription):
		22	* runtime/Symbol.h:
		23	* runtime/SymbolConstructor.cpp:
		24	(JSC::callSymbol):
		25
1	2019-05-06 Yusuke Suzuki <ysuzuki@apple.com>	26	2019-05-06 Yusuke Suzuki <ysuzuki@apple.com>
2		27
3	TemplateObject passed to template literal tags are not always identical for the same source location.	28	TemplateObject passed to template literal tags are not always identical for the same source location.

Lines 1-3 a/JSTests/ChangeLog_sec1
- a/JSTests/ChangeLog +10 lines
		1	2019-05-06 Yusuke Suzuki <ysuzuki@apple.com>
		2
		3	[JSC] We should check OOM for description string of Symbol
		4	https://bugs.webkit.org/show_bug.cgi?id=197634
		5
		6	Reviewed by NOBODY (OOPS!).
		7
		8	* stress/check-symbol-description-oom.js: Added.
		9	(shouldThrow):
		10
1	2019-05-06 Yusuke Suzuki <ysuzuki@apple.com>	11	2019-05-06 Yusuke Suzuki <ysuzuki@apple.com>
2		12
3	Unreviewed, land one more test	13	Unreviewed, land one more test

Line 0 a/JSTests/stress/check-symbol-description-oom.js_sec1
- a/JSTests/stress/check-symbol-description-oom.js +18 lines
	1	function shouldThrow(func, errorMessage) {
	2	var errorThrown = false;
	3	var error = null;
	4	try {
	5	func();
	6	} catch (e) {
	7	errorThrown = true;
	8	error = e;
	9	}
	10	if (!errorThrown)
	11	throw new Error('not thrown');
	12	if (String(error) !== errorMessage)
	13	throw new Error(`bad error: ${String(error)}`);
	14	}
	15
	16	const s0 = (-1).toLocaleString();
	17	const s1 = s0.padEnd(2147483647, ' ');
	18	shouldThrow(() => Symbol(s1), `Error: Out of memory`);