Diff
Modified: trunk/JSTests/ChangeLog (268655 => 268656)
--- trunk/JSTests/ChangeLog 2020-10-18 12:48:08 UTC (rev 268655)
+++ trunk/JSTests/ChangeLog 2020-10-18 13:24:38 UTC (rev 268656)
@@ -1,3 +1,14 @@
+2020-10-18 Caio Lima <ticaiol...@gmail.com>
+
+ [ESNext][JIT] Add support for UntypedUse on PutPrivateName's base operand
+ https://bugs.webkit.org/show_bug.cgi?id=217373
+
+ Reviewed by Yusuke Suzuki.
+
+ * stress/get-private-name-with-primitive.js: Added.
+ * stress/put-private-name-untyped-use.js: Added.
+ * stress/put-private-name-with-primitive.js: Added.
+
2020-10-17 Ross Kirsling <ross.kirsl...@sony.com>
Ensure %TypedArray% essential internal methods adhere to spec
Added: trunk/JSTests/stress/get-private-name-with-primitive.js (0 => 268656)
--- trunk/JSTests/stress/get-private-name-with-primitive.js (rev 0)
+++ trunk/JSTests/stress/get-private-name-with-primitive.js 2020-10-18 13:24:38 UTC (rev 268656)
@@ -0,0 +1,51 @@
+//@ requireOptions("--usePrivateClassFields=true")
+
+let assert = {
+ shouldThrow: function(exception, functor) {
+ let threwException;
+ try {
+ functor();
+ threwException = false;
+ } catch(e) {
+ threwException = true;
+ if (!e instanceof exception)
+ throw new Error("Expected to throw: " + exception.name + " but it throws: " + e.name);
+ }
+
+ if (!threwException)
+ throw new Error("Expected to throw: " + exception.name + " but executed without exception");
+ }
+}
+
+class C {
+ #field = 'test';
+
+ getField(v) {
+ this.#field;
+ }
+}
+noInline(C.constructor);
+noInline(C.prototype.getField);
+
+
+let c = new C();
+
+assert.shouldThrow(TypeError, () => {
+ c.getField.call(15);
+});
+assert.shouldThrow(TypeError, () => {
+ c.getField.call('test');
+});
+assert.shouldThrow(TypeError, () => {
+ c.getField.call(Symbol);
+});
+assert.shouldThrow(TypeError, () => {
+ c.getField.call(null);
+});
+assert.shouldThrow(TypeError, () => {
+ c.getField.call(undefined);
+});
+assert.shouldThrow(TypeError, () => {
+ c.getField.call(10n);
+});
+
Added: trunk/JSTests/stress/put-private-name-untyped-use.js (0 => 268656)
--- trunk/JSTests/stress/put-private-name-untyped-use.js (rev 0)
+++ trunk/JSTests/stress/put-private-name-untyped-use.js 2020-10-18 13:24:38 UTC (rev 268656)
@@ -0,0 +1,55 @@
+//@ requireOptions("--allowUnsupportedTiers=true", "--usePrivateClassFields=true", "--useAccessInlining=false")
+
+let assert = {
+ shouldThrow: function(exception, functor) {
+ let threwException;
+ try {
+ functor();
+ threwException = false;
+ } catch(e) {
+ threwException = true;
+ if (!e instanceof exception)
+ throw new Error("Expected to throw: " + exception.name + " but it throws: " + e.name);
+ }
+
+ if (!threwException)
+ throw new Error("Expected to throw: " + exception.name + " but executed without exception");
+ }
+}
+
+class C {
+ #field;
+
+ setField(v) {
+ let o = this;
+ // This branch is here to avoid PutPrivateNameById to be folded as
+ // PutByOffset.
+ if (v > 100)
+ o = {};
+ o.#field = v;
+ }
+}
+noInline(C.constructor);
+noInline(C.prototype.setField);
+
+let c = new C();
+
+// Let's trigger JIT compilation for `C.prototype.setField`
+for (let i = 0; i < 10000; i++) {
+ c.setField(15);
+}
+
+for (let i = 0; i < 10000; i++) {
+ assert.shouldThrow(TypeError, () => {
+ c.setField.call(15, 'test');
+ });
+
+ assert.shouldThrow(TypeError, () => {
+ c.setField.call('string', 'test');
+ });
+
+ assert.shouldThrow(TypeError, () => {
+ c.setField.call(Symbol('symbol'), 'test');
+ });
+}
+
Added: trunk/JSTests/stress/put-private-name-with-primitive.js (0 => 268656)
--- trunk/JSTests/stress/put-private-name-with-primitive.js (rev 0)
+++ trunk/JSTests/stress/put-private-name-with-primitive.js 2020-10-18 13:24:38 UTC (rev 268656)
@@ -0,0 +1,53 @@
+//@ requireOptions("--usePrivateClassFields=true")
+
+let assert = {
+ shouldThrow: function(exception, functor) {
+ let threwException;
+ try {
+ functor();
+ threwException = false;
+ } catch(e) {
+ threwException = true;
+ if (!e instanceof exception)
+ throw new Error("Expected to throw: " + exception.name + " but it throws: " + e.name);
+ }
+
+ if (!threwException)
+ throw new Error("Expected to throw: " + exception.name + " but executed without exception");
+ }
+}
+
+class C {
+ #field = 'test';
+
+ setField(v) {
+ this.#field = v;
+ }
+}
+noInline(C.constructor);
+noInline(C.prototype.setField);
+
+let c = new C();
+
+// We repeat this to make operationPutPrivateNameOptimize repatch to operationPutPrivateNameGeneric
+for (let i = 0; i < 5; i++) {
+ assert.shouldThrow(TypeError, () => {
+ c.setField.call(15, 0);
+ });
+ assert.shouldThrow(TypeError, () => {
+ c.setField.call('test', 0);
+ });
+ assert.shouldThrow(TypeError, () => {
+ c.setField.call(Symbol, 0);
+ });
+ assert.shouldThrow(TypeError, () => {
+ c.setField.call(null, 0);
+ });
+ assert.shouldThrow(TypeError, () => {
+ c.setField.call(undefined, 0);
+ });
+ assert.shouldThrow(TypeError, () => {
+ c.setField.call(10n, 0);
+ });
+}
+
Modified: trunk/Source/_javascript_Core/ChangeLog (268655 => 268656)
--- trunk/Source/_javascript_Core/ChangeLog 2020-10-18 12:48:08 UTC (rev 268655)
+++ trunk/Source/_javascript_Core/ChangeLog 2020-10-18 13:24:38 UTC (rev 268656)
@@ -1,3 +1,43 @@
+2020-10-18 Caio Lima <ticaiol...@gmail.com>
+
+ [ESNext][JIT] Add support for UntypedUse on PutPrivateName's base operand
+ https://bugs.webkit.org/show_bug.cgi?id=217373
+
+ Reviewed by Yusuke Suzuki.
+
+ This patch is adding UntypedUse for `PutPrivateName`'s base operand to
+ avoid a OSR when we have a non-cell base.
+ Also, it is fixing a bug on private field operations `get_private_name` and
+ `put_private_name` to call `ToObject` on base to properly support
+ class fields spec text[1][2].
+
+ [1] - https://tc39.es/proposal-class-fields/#sec-getvalue
+ [2] - https://tc39.es/proposal-class-fields/#sec-putvalue
+
+ * dfg/DFGFixupPhase.cpp:
+ (JSC::DFG::FixupPhase::fixupNode):
+ * dfg/DFGSpeculativeJIT.cpp:
+ (JSC::DFG::SpeculativeJIT::compilePutPrivateName):
+ * ftl/FTLLowerDFGToB3.cpp:
+ (JSC::FTL::DFG::LowerDFGToB3::compilePutPrivateName):
+ * jit/JITOperations.cpp:
+ (JSC::setPrivateField):
+ (JSC::definePrivateField):
+ (JSC::JSC_DEFINE_JIT_OPERATION):
+ (JSC::getPrivateName):
+ * jit/JITPropertyAccess.cpp:
+ (JSC::JIT::emit_op_put_private_name):
+ * jit/JITPropertyAccess32_64.cpp:
+ (JSC::JIT::emit_op_put_private_name):
+ * llint/LLIntSlowPaths.cpp:
+ (JSC::LLInt::LLINT_SLOW_PATH_DECL):
+ * runtime/CommonSlowPaths.cpp:
+
+ Previous implementation was wrongly considering that base was always
+ an object, causing segmentation fault when base was not an object.
+ We changed this to handle cases when base is not and object, following
+ what spec text specifies.
+
2020-10-17 Ross Kirsling <ross.kirsl...@sony.com>
Unreviewed fix for r268640
Modified: trunk/Source/_javascript_Core/dfg/DFGFixupPhase.cpp (268655 => 268656)
--- trunk/Source/_javascript_Core/dfg/DFGFixupPhase.cpp 2020-10-18 12:48:08 UTC (rev 268655)
+++ trunk/Source/_javascript_Core/dfg/DFGFixupPhase.cpp 2020-10-18 13:24:38 UTC (rev 268656)
@@ -1907,7 +1907,6 @@
case PutPrivateName: {
- fixEdge<CellUse>(node->child1());
fixEdge<SymbolUse>(node->child2());
break;
}
Modified: trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT.cpp (268655 => 268656)
--- trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT.cpp 2020-10-18 12:48:08 UTC (rev 268655)
+++ trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT.cpp 2020-10-18 13:24:38 UTC (rev 268656)
@@ -3526,19 +3526,20 @@
void SpeculativeJIT::compilePutPrivateName(Node* node)
{
- SpeculateCellOperand base(this, node->child1());
+ ASSERT(node->child1().useKind() == UntypedUse);
+ JSValueOperand base(this, node->child1());
SpeculateCellOperand propertyValue(this, node->child2());
JSValueOperand value(this, node->child3());
JSValueRegs valueRegs = value.jsValueRegs();
+ JSValueRegs baseRegs = base.jsValueRegs();
- GPRReg baseGPR = base.gpr();
GPRReg propertyGPR = propertyValue.gpr();
speculateSymbol(node->child2(), propertyGPR);
flushRegisters();
- callOperation(operationPutPrivateNameGeneric, TrustedImmPtr::weakPointer(m_graph, m_graph.globalObjectFor(node->origin.semantic)), CCallHelpers::CellValue(baseGPR), CCallHelpers::CellValue(propertyGPR), valueRegs, TrustedImmPtr(nullptr), TrustedImm32(node->privateFieldPutKind().value()));
+ callOperation(operationPutPrivateNameGeneric, TrustedImmPtr::weakPointer(m_graph, m_graph.globalObjectFor(node->origin.semantic)), baseRegs, CCallHelpers::CellValue(propertyGPR), valueRegs, TrustedImmPtr(nullptr), TrustedImm32(node->privateFieldPutKind().value()));
m_jit.exceptionCheck();
noResult(node);
Modified: trunk/Source/_javascript_Core/ftl/FTLLowerDFGToB3.cpp (268655 => 268656)
--- trunk/Source/_javascript_Core/ftl/FTLLowerDFGToB3.cpp 2020-10-18 12:48:08 UTC (rev 268655)
+++ trunk/Source/_javascript_Core/ftl/FTLLowerDFGToB3.cpp 2020-10-18 13:24:38 UTC (rev 268656)
@@ -3971,10 +3971,10 @@
void compilePutPrivateName()
{
- DFG_ASSERT(m_graph, m_node, m_node->child1().useKind() == CellUse, m_node->child1().useKind());
+ DFG_ASSERT(m_graph, m_node, m_node->child1().useKind() == UntypedUse, m_node->child1().useKind());
JSGlobalObject* globalObject = m_graph.globalObjectFor(m_node->origin.semantic);
- LValue base = lowCell(m_node->child1());
+ LValue base = lowJSValue(m_node->child1());
LValue property = lowSymbol(m_node->child2());
LValue value = lowJSValue(m_node->child3());
Modified: trunk/Source/_javascript_Core/jit/JITOperations.cpp (268655 => 268656)
--- trunk/Source/_javascript_Core/jit/JITOperations.cpp 2020-10-18 12:48:08 UTC (rev 268655)
+++ trunk/Source/_javascript_Core/jit/JITOperations.cpp 2020-10-18 13:24:38 UTC (rev 268656)
@@ -677,7 +677,8 @@
Identifier ident = Identifier::fromUid(vm, identifier.uid());
ASSERT(ident.isPrivateName());
- JSObject* baseObject = asObject(baseValue);
+ JSObject* baseObject = baseValue.toObject(globalObject);
+ RETURN_IF_EXCEPTION(scope, void());
CodeBlock* codeBlock = callFrame->codeBlock();
Structure* oldStructure = baseObject->structure(vm);
@@ -696,7 +697,8 @@
Identifier ident = Identifier::fromUid(vm, identifier.uid());
ASSERT(ident.isPrivateName());
- JSObject* baseObject = asObject(baseValue);
+ JSObject* baseObject = baseValue.toObject(globalObject);
+ RETURN_IF_EXCEPTION(scope, void());
CodeBlock* codeBlock = callFrame->codeBlock();
Structure* oldStructure = baseObject->structure(vm);
@@ -730,9 +732,10 @@
CacheableIdentifier identifier = CacheableIdentifier::createFromRawBits(rawCacheableIdentifier);
AccessType accessType = static_cast<AccessType>(stubInfo->accessType);
JSValue value = JSValue::decode(encodedValue);
- JSObject* baseObject = asObject(JSValue::decode(encodedBase));
-
- definePrivateField(vm, globalObject, callFrame, baseObject, identifier, value, [=](VM& vm, CodeBlock* codeBlock, Structure* oldStructure, PutPropertySlot& putSlot, const Identifier& ident) {
+ JSValue baseValue = JSValue::decode(encodedBase);
+
+ definePrivateField(vm, globalObject, callFrame, baseValue, identifier, value, [=](VM& vm, CodeBlock* codeBlock, Structure* oldStructure, PutPropertySlot& putSlot, const Identifier& ident) {
+ JSObject* baseObject = asObject(baseValue);
LOG_IC((ICEvent::OperationPutByIdDefinePrivateFieldFieldStrictOptimize, baseObject->classInfo(vm), ident, putSlot.base() == baseObject));
ASSERT_UNUSED(accessType, accessType == static_cast<AccessType>(stubInfo->accessType));
@@ -765,9 +768,10 @@
CacheableIdentifier identifier = CacheableIdentifier::createFromRawBits(rawCacheableIdentifier);
AccessType accessType = static_cast<AccessType>(stubInfo->accessType);
JSValue value = JSValue::decode(encodedValue);
- JSObject* baseObject = asObject(JSValue::decode(encodedBase));
+ JSValue baseValue = JSValue::decode(encodedBase);
- setPrivateField(vm, globalObject, callFrame, baseObject, identifier, value, [&](VM& vm, CodeBlock* codeBlock, Structure* oldStructure, PutPropertySlot& putSlot, const Identifier& ident) {
+ setPrivateField(vm, globalObject, callFrame, baseValue, identifier, value, [&](VM& vm, CodeBlock* codeBlock, Structure* oldStructure, PutPropertySlot& putSlot, const Identifier& ident) {
+ JSObject* baseObject = asObject(baseValue);
LOG_IC((ICEvent::OperationPutByIdPutPrivateFieldFieldStrictOptimize, baseObject->classInfo(vm), ident, putSlot.base() == baseObject));
ASSERT_UNUSED(accessType, accessType == static_cast<AccessType>(stubInfo->accessType));
@@ -1101,6 +1105,9 @@
JSValue subscript = JSValue::decode(encodedSubscript);
JSValue value = JSValue::decode(encodedValue);
+ auto baseObject = baseValue.toObject(globalObject);
+ RETURN_IF_EXCEPTION(scope, void());
+
auto propertyName = subscript.toPropertyKey(globalObject);
EXCEPTION_ASSERT(!scope.exception());
@@ -1149,7 +1156,6 @@
// Private fields can only be accessed within class lexical scope
// and class methods are always in strict mode
const bool isStrictMode = true;
- auto baseObject = asObject(baseValue);
PutPropertySlot slot(baseObject, isStrictMode);
if (putKind.isDefine())
baseObject->definePrivateField(globalObject, propertyName, value, slot);
@@ -1170,6 +1176,9 @@
JSValue subscript = JSValue::decode(encodedSubscript);
JSValue value = JSValue::decode(encodedValue);
+ auto baseObject = baseValue.toObject(globalObject);
+ RETURN_IF_EXCEPTION(scope, void());
+
auto propertyName = subscript.toPropertyKey(globalObject);
EXCEPTION_ASSERT(!scope.exception());
@@ -1178,7 +1187,6 @@
// Private fields can only be accessed within class lexical scope
// and class methods are always in strict mode
const bool isStrictMode = true;
- auto baseObject = asObject(baseValue);
PutPropertySlot slot(baseObject, isStrictMode);
if (privateFieldPutKind.isDefine())
baseObject->definePrivateField(globalObject, propertyName, value, slot);
@@ -2241,12 +2249,11 @@
VM& vm = globalObject->vm();
auto scope = DECLARE_THROW_SCOPE(vm);
- baseValue.requireObjectCoercible(globalObject);
+ JSObject* base = baseValue.toObject(globalObject);
RETURN_IF_EXCEPTION(scope, JSValue());
auto fieldName = fieldNameValue.toPropertyKey(globalObject);
RETURN_IF_EXCEPTION(scope, JSValue());
- JSObject* base = baseValue.toObject(globalObject);
PropertySlot slot(base, PropertySlot::InternalMethodType::GetOwnProperty);
base->getPrivateField(globalObject, fieldName, slot);
RETURN_IF_EXCEPTION(scope, JSValue());
Modified: trunk/Source/_javascript_Core/jit/JITPropertyAccess.cpp (268655 => 268656)
--- trunk/Source/_javascript_Core/jit/JITPropertyAccess.cpp 2020-10-18 12:48:08 UTC (rev 268655)
+++ trunk/Source/_javascript_Core/jit/JITPropertyAccess.cpp 2020-10-18 13:24:38 UTC (rev 268656)
@@ -380,6 +380,8 @@
emitGetVirtualRegister(base, regT0);
emitGetVirtualRegister(property, regT1);
+ emitJumpSlowCaseIfNotJSCell(regT0, base);
+
PatchableJump fastPathJmp = patchableJump();
addSlowCase(fastPathJmp);
Modified: trunk/Source/_javascript_Core/jit/JITPropertyAccess32_64.cpp (268655 => 268656)
--- trunk/Source/_javascript_Core/jit/JITPropertyAccess32_64.cpp 2020-10-18 12:48:08 UTC (rev 268655)
+++ trunk/Source/_javascript_Core/jit/JITPropertyAccess32_64.cpp 2020-10-18 13:24:38 UTC (rev 268656)
@@ -340,6 +340,7 @@
emitLoad2(base, regT1, regT0, property, regT3, regT2);
+ emitJumpSlowCaseIfNotJSCell(base, regT1);
PatchableJump fastPathJmp = patchableJump();
addSlowCase(fastPathJmp);
Modified: trunk/Source/_javascript_Core/llint/LLIntSlowPaths.cpp (268655 => 268656)
--- trunk/Source/_javascript_Core/llint/LLIntSlowPaths.cpp 2020-10-18 12:48:08 UTC (rev 268655)
+++ trunk/Source/_javascript_Core/llint/LLIntSlowPaths.cpp 2020-10-18 13:24:38 UTC (rev 268656)
@@ -1087,17 +1087,17 @@
JSValue subscript = getOperand(callFrame, bytecode.m_property);
ASSERT(subscript.isSymbol());
- baseValue.requireObjectCoercible(globalObject);
+ JSObject* baseObject = baseValue.toObject(globalObject);
LLINT_CHECK_EXCEPTION();
auto property = subscript.toPropertyKey(globalObject);
LLINT_CHECK_EXCEPTION();
ASSERT(property.isPrivateName());
- PropertySlot slot(baseValue, PropertySlot::InternalMethodType::GetOwnProperty);
- asObject(baseValue)->getPrivateField(globalObject, property, slot);
+ PropertySlot slot(baseObject, PropertySlot::InternalMethodType::GetOwnProperty);
+ baseObject->getPrivateField(globalObject, property, slot);
LLINT_CHECK_EXCEPTION();
- if (!LLINT_ALWAYS_ACCESS_SLOW && slot.isCacheable() && !slot.isUnset()) {
+ if (!LLINT_ALWAYS_ACCESS_SLOW && baseValue.isCell() && slot.isCacheable() && !slot.isUnset()) {
auto& metadata = bytecode.metadata(codeBlock);
{
StructureID oldStructureID = metadata.m_structureID;
@@ -1207,6 +1207,9 @@
JSValue subscript = getOperand(callFrame, bytecode.m_property);
JSValue value = getOperand(callFrame, bytecode.m_value);
+ JSObject* baseObject = baseValue.toObject(globalObject);
+ LLINT_CHECK_EXCEPTION();
+
auto property = subscript.toPropertyKey(globalObject);
LLINT_CHECK_EXCEPTION();
ASSERT(property.isPrivateName());
@@ -1216,7 +1219,6 @@
// Private fields can only be accessed within class lexical scope
// and class methods are always in strict mode
const bool isStrictMode = true;
- auto baseObject = asObject(baseValue);
PutPropertySlot slot(baseObject, isStrictMode);
if (bytecode.m_putKind.isDefine())
baseObject->definePrivateField(globalObject, property, value, slot);
Modified: trunk/Source/_javascript_Core/runtime/CommonSlowPaths.cpp (268655 => 268656)
--- trunk/Source/_javascript_Core/runtime/CommonSlowPaths.cpp 2020-10-18 12:48:08 UTC (rev 268655)
+++ trunk/Source/_javascript_Core/runtime/CommonSlowPaths.cpp 2020-10-18 13:24:38 UTC (rev 268656)
@@ -1340,28 +1340,6 @@
RETURN_PROFILED(baseValue.get(globalObject, property, slot));
}
-JSC_DEFINE_COMMON_SLOW_PATH(slow_path_get_private_name)
-{
- BEGIN();
-
- auto bytecode = pc->as<OpGetPrivateName>();
- JSValue baseValue = GET_C(bytecode.m_base).jsValue();
- JSValue subscript = GET_C(bytecode.m_property).jsValue();
- ASSERT(subscript.isSymbol());
-
- baseValue.requireObjectCoercible(globalObject);
- CHECK_EXCEPTION();
- auto property = subscript.toPropertyKey(globalObject);
- CHECK_EXCEPTION();
- ASSERT(property.isPrivateName());
-
- PropertySlot slot(baseValue, PropertySlot::InternalMethodType::GetOwnProperty);
- asObject(baseValue)->getPrivateField(globalObject, property, slot);
- CHECK_EXCEPTION();
-
- RETURN_PROFILED(slot.getValue(globalObject, property));
-}
-
JSC_DEFINE_COMMON_SLOW_PATH(slow_path_get_prototype_of)
{
BEGIN();
