Changeset 251088 in webkit
- Timestamp:
- Oct 14, 2019 12:54:28 PM (4 years ago)
- Location:
- trunk
- Files:
-
- 1 added
- 11 edited
Legend:
- Unmodified
- Added
- Removed
-
trunk/JSTests/ChangeLog
r251085 r251088 1 2019-10-14 Yusuke Suzuki <ysuzuki@apple.com> 2 3 [JSC] GetterSetter should be JSCell, not JSObject 4 https://bugs.webkit.org/show_bug.cgi?id=202656 5 6 Reviewed by Tadeu Zagallo and Saam Barati. 7 8 * stress/getter-setter-should-be-cell.js: Added. 9 (foo.with.): 10 (foo.with.get for): 11 (foo.with.bar): 12 (foo): 13 1 14 2019-10-14 Saam Barati <sbarati@apple.com> 2 15 -
trunk/Source/JavaScriptCore/ChangeLog
r251085 r251088 1 2019-10-14 Yusuke Suzuki <ysuzuki@apple.com> 2 3 [JSC] GetterSetter should be JSCell, not JSObject 4 https://bugs.webkit.org/show_bug.cgi?id=202656 5 6 Reviewed by Tadeu Zagallo and Saam Barati. 7 8 Essentially, GetterSetter is not a JSObject. It is like a JSCell. But we made GetterSetter JSObject 9 to leverage existing strict-eq implementations for JSObject: pointer-comparison. But given the following 10 conditions, 11 12 1. GetterSetter strict-eq comparison only happens in builtin code when using @tryGetById. 13 2. RHS of that comparison is always folded into constant in DFG. 14 3. We already use pointer-comparison for cells that are neither JSString nor JSBigInt. 15 4. DFG strength reduction already has a rule which makes `CompareStrictEq(Cell-not-JSString/JSBigInt, Constant)` `ComparePtrEq`. 16 17 So we already support non-JSString/JSBigInt cell comparison in JSC JS code. We should use it instead of making GetterSetter JSObject. 18 This patch makes GetterSetter JSCell, and makes getterSetterStructure per-VM structure. 19 20 The attached test reported AI validation failure. AI assumed that GetterSetter's realm should be the same to the base object. But 21 this is incorrect in our runtime code: we are creating GetterSetter with lexical realm (JSGlobalObject). But the fundamental problem 22 is that GetterSetter is JSObject and tied to JSGlobalObject while it is not necessary. 23 24 * dfg/DFGAbstractInterpreterInlines.h: 25 (JSC::DFG::AbstractInterpreter<AbstractStateType>::executeEffects): 26 * dfg/DFGFixupPhase.cpp: 27 * runtime/GetterSetter.cpp: 28 * runtime/GetterSetter.h: 29 * runtime/JSGlobalObject.cpp: 30 (JSC::getGetterById): 31 (JSC::JSGlobalObject::init): 32 (JSC::JSGlobalObject::visitChildren): 33 * runtime/JSGlobalObject.h: 34 (JSC::JSGlobalObject::regExpProtoGlobalGetter const): 35 (JSC::JSGlobalObject::regExpProtoUnicodeGetter const): 36 (JSC::JSGlobalObject::customGetterSetterFunctionStructure const): 37 (JSC::JSGlobalObject::getterSetterStructure const): Deleted. 38 * runtime/JSType.h: 39 * runtime/VM.cpp: 40 (JSC::VM::VM): 41 * runtime/VM.h: 42 1 43 2019-10-14 Saam Barati <sbarati@apple.com> 2 44 -
trunk/Source/JavaScriptCore/dfg/DFGAbstractInterpreterInlines.h
r250932 r251088 3467 3467 } 3468 3468 3469 if (base.value() && base.value().isObject()) { 3470 setForNode(node, asObject(base.value())->globalObject()->getterSetterStructure()); 3471 break; 3472 } 3473 3474 setTypeForNode(node, SpecObjectOther); 3469 setForNode(node, m_vm.getterSetterStructure.get()); 3475 3470 break; 3476 3471 } -
trunk/Source/JavaScriptCore/dfg/DFGFixupPhase.cpp
r250932 r251088 35 35 #include "DFGPredictionPropagationPhase.h" 36 36 #include "DFGVariableAccessDataDump.h" 37 #include "GetterSetter.h" 37 38 #include "JSCInlines.h" 38 39 #include "TypeLocation.h" -
trunk/Source/JavaScriptCore/runtime/GetterSetter.cpp
r250932 r251088 34 34 STATIC_ASSERT_IS_TRIVIALLY_DESTRUCTIBLE(GetterSetter); 35 35 36 const ClassInfo GetterSetter::s_info = { "GetterSetter", &Base::s_info, nullptr, nullptr, CREATE_METHOD_TABLE(GetterSetter) };36 const ClassInfo GetterSetter::s_info = { "GetterSetter", nullptr, nullptr, nullptr, CREATE_METHOD_TABLE(GetterSetter) }; 37 37 38 38 void GetterSetter::visitChildren(JSCell* cell, SlotVisitor& visitor) -
trunk/Source/JavaScriptCore/runtime/GetterSetter.h
r250932 r251088 41 41 // that constant is observed to have a non-null setter (or getter) then we can 42 42 // constant fold that setter (or getter). 43 class GetterSetter final : public JS NonFinalObject{43 class GetterSetter final : public JSCell { 44 44 friend class JIT; 45 typedef JSNonFinalObject Base;45 using Base = JSCell; 46 46 private: 47 47 GetterSetter(VM& vm, JSGlobalObject* globalObject, JSObject* getter, JSObject* setter) 48 : Base(vm, globalObject->getterSetterStructure())48 : Base(vm, vm.getterSetterStructure.get()) 49 49 { 50 50 WTF::storeStoreFence(); -
trunk/Source/JavaScriptCore/runtime/JSGlobalObject.cpp
r250932 r251088 451 451 } 452 452 453 static JSObject* getGetterById(ExecState* exec, JSObject* base, const Identifier& ident)453 static GetterSetter* getGetterById(ExecState* exec, JSObject* base, const Identifier& ident) 454 454 { 455 455 JSValue baseValue = JSValue(base); 456 456 PropertySlot slot(baseValue, PropertySlot::InternalMethodType::VMInquiry); 457 457 baseValue.getPropertySlot(exec, ident, slot); 458 return slot.getPureResult().toObject(exec);458 return jsCast<GetterSetter*>(slot.getPureResult()); 459 459 } 460 460 … … 515 515 init.set(JSBoundFunction::createStructure(init.vm, init.owner, init.owner->m_functionPrototype.get())); 516 516 }); 517 m_getterSetterStructure.set(vm, this, GetterSetter::createStructure(vm, this, jsNull()));518 517 m_nativeStdFunctionStructure.initLater( 519 518 [] (const Initializer<Structure>& init) { … … 940 939 JSFunction* privateFuncSetBucketKey = JSFunction::create(vm, this, 0, String(), setPrivateFuncSetBucketKey, JSSetBucketKeyIntrinsic); 941 940 942 JSObject* regExpProtoFlagsGetterObject= getGetterById(exec, m_regExpPrototype.get(), vm.propertyNames->flags);941 GetterSetter* regExpProtoFlagsGetter = getGetterById(exec, m_regExpPrototype.get(), vm.propertyNames->flags); 943 942 catchScope.assertNoException(); 944 JSObject* regExpProtoGlobalGetterObject= getGetterById(exec, m_regExpPrototype.get(), vm.propertyNames->global);943 GetterSetter* regExpProtoGlobalGetter = getGetterById(exec, m_regExpPrototype.get(), vm.propertyNames->global); 945 944 catchScope.assertNoException(); 946 m_regExpProtoGlobalGetter.set(vm, this, regExpProtoGlobalGetter Object);947 JSObject* regExpProtoIgnoreCaseGetterObject= getGetterById(exec, m_regExpPrototype.get(), vm.propertyNames->ignoreCase);945 m_regExpProtoGlobalGetter.set(vm, this, regExpProtoGlobalGetter); 946 GetterSetter* regExpProtoIgnoreCaseGetter = getGetterById(exec, m_regExpPrototype.get(), vm.propertyNames->ignoreCase); 948 947 catchScope.assertNoException(); 949 JSObject* regExpProtoMultilineGetterObject= getGetterById(exec, m_regExpPrototype.get(), vm.propertyNames->multiline);948 GetterSetter* regExpProtoMultilineGetter = getGetterById(exec, m_regExpPrototype.get(), vm.propertyNames->multiline); 950 949 catchScope.assertNoException(); 951 JSObject* regExpProtoSourceGetterObject= getGetterById(exec, m_regExpPrototype.get(), vm.propertyNames->source);950 GetterSetter* regExpProtoSourceGetter = getGetterById(exec, m_regExpPrototype.get(), vm.propertyNames->source); 952 951 catchScope.assertNoException(); 953 JSObject* regExpProtoStickyGetterObject= getGetterById(exec, m_regExpPrototype.get(), vm.propertyNames->sticky);952 GetterSetter* regExpProtoStickyGetter = getGetterById(exec, m_regExpPrototype.get(), vm.propertyNames->sticky); 954 953 catchScope.assertNoException(); 955 JSObject* regExpProtoUnicodeGetterObject= getGetterById(exec, m_regExpPrototype.get(), vm.propertyNames->unicode);954 GetterSetter* regExpProtoUnicodeGetter = getGetterById(exec, m_regExpPrototype.get(), vm.propertyNames->unicode); 956 955 catchScope.assertNoException(); 957 m_regExpProtoUnicodeGetter.set(vm, this, regExpProtoUnicodeGetter Object);956 m_regExpProtoUnicodeGetter.set(vm, this, regExpProtoUnicodeGetter); 958 957 JSObject* builtinRegExpExec = asObject(m_regExpPrototype->getDirect(vm, vm.propertyNames->exec).asCell()); 959 958 m_regExpProtoExec.set(vm, this, builtinRegExpExec); … … 1025 1024 GlobalPropertyInfo(vm.propertyNames->builtinNames().isConstructorPrivateName(), JSFunction::create(vm, this, 1, String(), esSpecIsConstructor, NoIntrinsic), PropertyAttribute::DontEnum | PropertyAttribute::DontDelete | PropertyAttribute::ReadOnly), 1026 1025 1027 GlobalPropertyInfo(vm.propertyNames->builtinNames().regExpProtoFlagsGetterPrivateName(), regExpProtoFlagsGetter Object, PropertyAttribute::DontEnum | PropertyAttribute::DontDelete | PropertyAttribute::ReadOnly),1028 GlobalPropertyInfo(vm.propertyNames->builtinNames().regExpProtoGlobalGetterPrivateName(), regExpProtoGlobalGetter Object, PropertyAttribute::DontEnum | PropertyAttribute::DontDelete | PropertyAttribute::ReadOnly),1029 GlobalPropertyInfo(vm.propertyNames->builtinNames().regExpProtoIgnoreCaseGetterPrivateName(), regExpProtoIgnoreCaseGetter Object, PropertyAttribute::DontEnum | PropertyAttribute::DontDelete | PropertyAttribute::ReadOnly),1030 GlobalPropertyInfo(vm.propertyNames->builtinNames().regExpProtoMultilineGetterPrivateName(), regExpProtoMultilineGetter Object, PropertyAttribute::DontEnum | PropertyAttribute::DontDelete | PropertyAttribute::ReadOnly),1031 GlobalPropertyInfo(vm.propertyNames->builtinNames().regExpProtoSourceGetterPrivateName(), regExpProtoSourceGetter Object, PropertyAttribute::DontEnum | PropertyAttribute::DontDelete | PropertyAttribute::ReadOnly),1032 GlobalPropertyInfo(vm.propertyNames->builtinNames().regExpProtoStickyGetterPrivateName(), regExpProtoStickyGetter Object, PropertyAttribute::DontEnum | PropertyAttribute::DontDelete | PropertyAttribute::ReadOnly),1033 GlobalPropertyInfo(vm.propertyNames->builtinNames().regExpProtoUnicodeGetterPrivateName(), regExpProtoUnicodeGetter Object, PropertyAttribute::DontEnum | PropertyAttribute::DontDelete | PropertyAttribute::ReadOnly),1026 GlobalPropertyInfo(vm.propertyNames->builtinNames().regExpProtoFlagsGetterPrivateName(), regExpProtoFlagsGetter, PropertyAttribute::DontEnum | PropertyAttribute::DontDelete | PropertyAttribute::ReadOnly), 1027 GlobalPropertyInfo(vm.propertyNames->builtinNames().regExpProtoGlobalGetterPrivateName(), regExpProtoGlobalGetter, PropertyAttribute::DontEnum | PropertyAttribute::DontDelete | PropertyAttribute::ReadOnly), 1028 GlobalPropertyInfo(vm.propertyNames->builtinNames().regExpProtoIgnoreCaseGetterPrivateName(), regExpProtoIgnoreCaseGetter, PropertyAttribute::DontEnum | PropertyAttribute::DontDelete | PropertyAttribute::ReadOnly), 1029 GlobalPropertyInfo(vm.propertyNames->builtinNames().regExpProtoMultilineGetterPrivateName(), regExpProtoMultilineGetter, PropertyAttribute::DontEnum | PropertyAttribute::DontDelete | PropertyAttribute::ReadOnly), 1030 GlobalPropertyInfo(vm.propertyNames->builtinNames().regExpProtoSourceGetterPrivateName(), regExpProtoSourceGetter, PropertyAttribute::DontEnum | PropertyAttribute::DontDelete | PropertyAttribute::ReadOnly), 1031 GlobalPropertyInfo(vm.propertyNames->builtinNames().regExpProtoStickyGetterPrivateName(), regExpProtoStickyGetter, PropertyAttribute::DontEnum | PropertyAttribute::DontDelete | PropertyAttribute::ReadOnly), 1032 GlobalPropertyInfo(vm.propertyNames->builtinNames().regExpProtoUnicodeGetterPrivateName(), regExpProtoUnicodeGetter, PropertyAttribute::DontEnum | PropertyAttribute::DontDelete | PropertyAttribute::ReadOnly), 1034 1033 1035 1034 // RegExp.prototype helpers. … … 1761 1760 thisObject->m_customGetterSetterFunctionStructure.visit(visitor); 1762 1761 thisObject->m_boundFunctionStructure.visit(visitor); 1763 visitor.append(thisObject->m_getterSetterStructure);1764 1762 thisObject->m_nativeStdFunctionStructure.visit(visitor); 1765 1763 visitor.append(thisObject->m_regExpStructure); -
trunk/Source/JavaScriptCore/runtime/JSGlobalObject.h
r250932 r251088 307 307 WriteBarrier<JSObject> m_regExpProtoExec; 308 308 WriteBarrier<JSObject> m_regExpProtoSymbolReplace; 309 WriteBarrier< JSObject> m_regExpProtoGlobalGetter;310 WriteBarrier< JSObject> m_regExpProtoUnicodeGetter;309 WriteBarrier<GetterSetter> m_regExpProtoGlobalGetter; 310 WriteBarrier<GetterSetter> m_regExpProtoUnicodeGetter; 311 311 WriteBarrier<GetterSetter> m_throwTypeErrorArgumentsCalleeAndCallerGetterSetter; 312 312 … … 366 366 LazyProperty<JSGlobalObject, Structure> m_boundFunctionStructure; 367 367 LazyProperty<JSGlobalObject, Structure> m_customGetterSetterFunctionStructure; 368 WriteBarrier<Structure> m_getterSetterStructure;369 368 LazyProperty<JSGlobalObject, Structure> m_nativeStdFunctionStructure; 370 369 PropertyOffset m_functionNameOffset; … … 623 622 JSObject* regExpProtoExecFunction() const { return m_regExpProtoExec.get(); } 624 623 JSObject* regExpProtoSymbolReplaceFunction() const { return m_regExpProtoSymbolReplace.get(); } 625 JSObject* regExpProtoGlobalGetter() const { return m_regExpProtoGlobalGetter.get(); }626 JSObject* regExpProtoUnicodeGetter() const { return m_regExpProtoUnicodeGetter.get(); }624 GetterSetter* regExpProtoGlobalGetter() const { return m_regExpProtoGlobalGetter.get(); } 625 GetterSetter* regExpProtoUnicodeGetter() const { return m_regExpProtoUnicodeGetter.get(); } 627 626 GetterSetter* throwTypeErrorArgumentsCalleeAndCallerGetterSetter() 628 627 { … … 749 748 Structure* boundFunctionStructure() const { return m_boundFunctionStructure.get(this); } 750 749 Structure* customGetterSetterFunctionStructure() const { return m_customGetterSetterFunctionStructure.get(this); } 751 Structure* getterSetterStructure() const { return m_getterSetterStructure.get(); }752 750 Structure* nativeStdFunctionStructure() const { return m_nativeStdFunctionStructure.get(this); } 753 751 PropertyOffset functionNameOffset() const { return m_functionNameOffset; } -
trunk/Source/JavaScriptCore/runtime/JSType.h
r250932 r251088 30 30 BigIntType, 31 31 32 GetterSetterType, 32 33 CustomGetterSetterType, 33 34 APIValueWrapperType, … … 88 89 DataViewType, 89 90 // End JSArrayBufferView types. 90 91 GetterSetterType,92 91 93 92 // JSScope <- JSWithScope -
trunk/Source/JavaScriptCore/runtime/VM.cpp
r250932 r251088 343 343 terminatedExecutionErrorStructure.set(*this, TerminatedExecutionError::createStructure(*this, 0, jsNull())); 344 344 propertyNameEnumeratorStructure.set(*this, JSPropertyNameEnumerator::createStructure(*this, 0, jsNull())); 345 getterSetterStructure.set(*this, GetterSetter::createStructure(*this, 0, jsNull())); 345 346 customGetterSetterStructure.set(*this, CustomGetterSetter::createStructure(*this, 0, jsNull())); 346 347 domAttributeGetterSetterStructure.set(*this, DOMAttributeGetterSetter::createStructure(*this, 0, jsNull())); -
trunk/Source/JavaScriptCore/runtime/VM.h
r250932 r251088 505 505 Strong<Structure> stringStructure; 506 506 Strong<Structure> propertyNameEnumeratorStructure; 507 Strong<Structure> getterSetterStructure; 507 508 Strong<Structure> customGetterSetterStructure; 508 509 Strong<Structure> domAttributeGetterSetterStructure;
Note: See TracChangeset
for help on using the changeset viewer.