Changeset 267113 in webkit


Ignore:
Timestamp:
Sep 15, 2020 4:38:23 PM (4 years ago)
Author:
sbarati@apple.com
Message:

CustomFunctionEquivalence PropertyCondition needs to check if the structure has the property
https://bugs.webkit.org/show_bug.cgi?id=216575
<rdar://problem/68286930>

Reviewed by Yusuke Suzuki.

JSTests:

  • stress/has-static-property-property-condition-needs-to-check-if-structure-has-property.js: Added.

(foo):

Source/JavaScriptCore:

The CustomFunctionEquivalence PropertyCondition would only return false to
isStillValidAssumingImpurePropertyWatchpoint if the Structure's static
property table was reified or if the static property table did not contain the
property. However, this missed the obvious case of where we store to this
property in normal object storage without reifying the static property table.
The fix here is simple: we first check if the Structure's property table
has this property, and if so, return false.

This patch also renames CustomFunctionEquivalence to HasStaticProperty to
better capture what we're doing.

  • bytecode/ObjectPropertyCondition.h:

(JSC::ObjectPropertyCondition::hasStaticProperty):
(JSC::ObjectPropertyCondition::customFunctionEquivalence): Deleted.

  • bytecode/ObjectPropertyConditionSet.cpp:

(JSC::ObjectPropertyConditionSet::hasOneSlotBaseCondition const):
(JSC::ObjectPropertyConditionSet::slotBaseCondition const):
(JSC::generateConditionsForPrototypePropertyHitCustom):

  • bytecode/PropertyCondition.cpp:

(JSC::PropertyCondition::dumpInContext const):
(JSC::PropertyCondition::isStillValidAssumingImpurePropertyWatchpoint const):
(JSC::PropertyCondition::validityRequiresImpurePropertyWatchpoint const):
(JSC::PropertyCondition::isStillValid const):
(JSC::PropertyCondition::isWatchableWhenValid const):
(WTF::printInternal):

  • bytecode/PropertyCondition.h:

(JSC::PropertyCondition::hasStaticProperty):
(JSC::PropertyCondition::hash const):
(JSC::PropertyCondition::operator== const):
(JSC::PropertyCondition::customFunctionEquivalence): Deleted.

  • tools/JSDollarVM.cpp:

(JSC::functionCreateStaticCustomValue):
(JSC::JSDollarVM::finishCreation):

Location:
trunk
Files:
1 added
7 edited

Legend:

Unmodified
Added
Removed
  • trunk/JSTests/ChangeLog

    r267108 r267113  
     12020-09-15  Saam Barati  <sbarati@apple.com>
     2
     3        CustomFunctionEquivalence PropertyCondition needs to check if the structure has the property
     4        https://bugs.webkit.org/show_bug.cgi?id=216575
     5        <rdar://problem/68286930>
     6
     7        Reviewed by Yusuke Suzuki.
     8
     9        * stress/has-static-property-property-condition-needs-to-check-if-structure-has-property.js: Added.
     10        (foo):
     11
    1122020-09-15  Yusuke Suzuki  <ysuzuki@apple.com>
    213
  • trunk/Source/JavaScriptCore/ChangeLog

    r267108 r267113  
     12020-09-15  Saam Barati  <sbarati@apple.com>
     2
     3        CustomFunctionEquivalence PropertyCondition needs to check if the structure has the property
     4        https://bugs.webkit.org/show_bug.cgi?id=216575
     5        <rdar://problem/68286930>
     6
     7        Reviewed by Yusuke Suzuki.
     8
     9        The CustomFunctionEquivalence PropertyCondition would only return false to
     10        isStillValidAssumingImpurePropertyWatchpoint if the Structure's static
     11        property table was reified or if the static property table did not contain the
     12        property. However, this missed the obvious case of where we store to this
     13        property in normal object storage without reifying the static property table.
     14        The fix here is simple: we first check if the Structure's property table
     15        has this property, and if so, return false.
     16       
     17        This patch also renames CustomFunctionEquivalence to HasStaticProperty to
     18        better capture what we're doing.
     19
     20        * bytecode/ObjectPropertyCondition.h:
     21        (JSC::ObjectPropertyCondition::hasStaticProperty):
     22        (JSC::ObjectPropertyCondition::customFunctionEquivalence): Deleted.
     23        * bytecode/ObjectPropertyConditionSet.cpp:
     24        (JSC::ObjectPropertyConditionSet::hasOneSlotBaseCondition const):
     25        (JSC::ObjectPropertyConditionSet::slotBaseCondition const):
     26        (JSC::generateConditionsForPrototypePropertyHitCustom):
     27        * bytecode/PropertyCondition.cpp:
     28        (JSC::PropertyCondition::dumpInContext const):
     29        (JSC::PropertyCondition::isStillValidAssumingImpurePropertyWatchpoint const):
     30        (JSC::PropertyCondition::validityRequiresImpurePropertyWatchpoint const):
     31        (JSC::PropertyCondition::isStillValid const):
     32        (JSC::PropertyCondition::isWatchableWhenValid const):
     33        (WTF::printInternal):
     34        * bytecode/PropertyCondition.h:
     35        (JSC::PropertyCondition::hasStaticProperty):
     36        (JSC::PropertyCondition::hash const):
     37        (JSC::PropertyCondition::operator== const):
     38        (JSC::PropertyCondition::customFunctionEquivalence): Deleted.
     39        * tools/JSDollarVM.cpp:
     40        (JSC::functionCreateStaticCustomValue):
     41        (JSC::JSDollarVM::finishCreation):
     42
    1432020-09-15  Yusuke Suzuki  <ysuzuki@apple.com>
    244
  • trunk/Source/JavaScriptCore/bytecode/ObjectPropertyCondition.h

    r264488 r267113  
    123123    }
    124124
    125     static ObjectPropertyCondition customFunctionEquivalence(
     125    static ObjectPropertyCondition hasStaticProperty(
    126126        VM& vm, JSCell* owner, JSObject* object, UniquedStringImpl* uid)
    127127    {
    128128        ObjectPropertyCondition result;
    129129        result.m_object = object;
    130         result.m_condition = PropertyCondition::customFunctionEquivalence(uid);
     130        result.m_condition = PropertyCondition::hasStaticProperty(uid);
    131131        if (owner)
    132132            vm.heap.writeBarrier(owner);
  • trunk/Source/JavaScriptCore/bytecode/ObjectPropertyConditionSet.cpp

    r266496 r267113  
    6868        case PropertyCondition::Presence:
    6969        case PropertyCondition::Equivalence:
    70         case PropertyCondition::CustomFunctionEquivalence:
     70        case PropertyCondition::HasStaticProperty:
    7171            if (sawBase)
    7272                return false;
     
    8888        if (condition.kind() == PropertyCondition::Presence
    8989            || condition.kind() == PropertyCondition::Equivalence
    90             || condition.kind() == PropertyCondition::CustomFunctionEquivalence) {
     90            || condition.kind() == PropertyCondition::HasStaticProperty) {
    9191            result = condition;
    9292            numFound++;
     
    245245        break;
    246246    }
    247     case PropertyCondition::CustomFunctionEquivalence: {
     247    case PropertyCondition::HasStaticProperty: {
    248248        auto entry = object->findPropertyHashEntry(vm, uid);
    249249        if (!entry)
    250250            return ObjectPropertyCondition();
    251         result = ObjectPropertyCondition::customFunctionEquivalence(vm, owner, object, uid);
     251        result = ObjectPropertyCondition::hasStaticProperty(vm, owner, object, uid);
    252252        break;
    253253    }
     
    410410                    kind = PropertyCondition::Equivalence;
    411411                } else if (structure->findPropertyHashEntry(uid))
    412                     kind = PropertyCondition::CustomFunctionEquivalence;
     412                    kind = PropertyCondition::HasStaticProperty;
    413413                else if (attributes & PropertyAttribute::DontDelete) {
    414414                    // This can't change, so we can blindly cache it.
  • trunk/Source/JavaScriptCore/bytecode/PropertyCondition.cpp

    r261895 r267113  
    5555        out.print(m_header.type(), " of ", m_header.pointer(), " with ", inContext(requiredValue(), context));
    5656        return;
    57     case CustomFunctionEquivalence:
     57    case HasStaticProperty:
    5858        out.print(m_header.type(), " of ", m_header.pointer());
    5959        return;
     
    9090    case AbsenceOfSetEffect:
    9191    case Equivalence:
    92     case CustomFunctionEquivalence:
     92    case HasStaticProperty:
    9393        if (!structure->propertyAccessesAreCacheable()) {
    9494            if (PropertyConditionInternal::verbose)
     
    254254        return true;
    255255    }
    256     case CustomFunctionEquivalence: {
     256    case HasStaticProperty: {
     257        if (isValidOffset(structure->getConcurrently(uid())))
     258            return false;
    257259        if (structure->staticPropertiesReified())
    258260            return false;
     
    274276    case Absence:
    275277    case Equivalence:
    276     case CustomFunctionEquivalence:
     278    case HasStaticProperty:
    277279        return structure->needImpurePropertyWatchpoint();
    278280    case AbsenceOfSetEffect:
     
    300302    case Presence:
    301303    case Equivalence:
    302     case CustomFunctionEquivalence:
     304    case HasStaticProperty:
    303305        if (structure->typeInfo().getOwnPropertySlotIsImpure())
    304306            return false;
     
    343345    }
    344346
    345     case CustomFunctionEquivalence: {
     347    case HasStaticProperty: {
    346348        // We just use the structure transition watchpoint for this. A structure S starts
    347349        // off with a property P in the static property hash table. If S transitions to
     
    431433        out.print("Equivalence");
    432434        return;
    433     case JSC::PropertyCondition::CustomFunctionEquivalence:
    434         out.print("CustomFunctionEquivalence");
     435    case JSC::PropertyCondition::HasStaticProperty:
     436        out.print("HasStaticProperty");
    435437        return;
    436438    case JSC::PropertyCondition::HasPrototype:
  • trunk/Source/JavaScriptCore/bytecode/PropertyCondition.h

    r264488 r267113  
    4141        AbsenceOfSetEffect,
    4242        Equivalence, // An adaptive watchpoint on this will be a pair of watchpoints, and when the structure transitions, we will set the replacement watchpoint on the new structure.
    43         CustomFunctionEquivalence, // Custom value or accessor.
     43        HasStaticProperty, // Custom value or accessor.
    4444        HasPrototype
    4545    };
     
    125125    }
    126126
    127     static PropertyCondition customFunctionEquivalence(UniquedStringImpl* uid)
    128     {
    129         PropertyCondition result;
    130         result.m_header = Header(uid, CustomFunctionEquivalence);
     127    static PropertyCondition hasStaticProperty(UniquedStringImpl* uid)
     128    {
     129        PropertyCondition result;
     130        result.m_header = Header(uid, HasStaticProperty);
    131131        return result;
    132132    }
     
    202202            result ^= EncodedJSValueHash::hash(u.equivalence.value);
    203203            break;
    204         case CustomFunctionEquivalence:
     204        case HasStaticProperty:
    205205            break;
    206206        }
     
    224224        case Equivalence:
    225225            return u.equivalence.value == other.u.equivalence.value;
    226         case CustomFunctionEquivalence:
     226        case HasStaticProperty:
    227227            return true;
    228228        }
  • trunk/Source/JavaScriptCore/tools/JSDollarVM.cpp

    r266969 r267113  
    740740};
    741741
     742static EncodedJSValue testStaticValueGetter(JSGlobalObject*, EncodedJSValue, PropertyName)
     743{
     744    DollarVMAssertScope assertScope;
     745    return JSValue::encode(jsUndefined());
     746}
     747
     748static bool testStaticValuePutter(JSGlobalObject* globalObject, EncodedJSValue thisValue, EncodedJSValue value)
     749{
     750    DollarVMAssertScope assertScope;
     751    VM& vm = globalObject->vm();
     752   
     753    JSObject* thisObject = jsDynamicCast<JSObject*>(vm, JSValue::decode(thisValue));
     754    RELEASE_ASSERT(thisObject);
     755
     756    return thisObject->putDirect(vm, PropertyName(Identifier::fromString(vm, "testStaticValue")), JSValue::decode(value));
     757}
     758
     759static const struct CompactHashIndex staticCustomValueTableIndex[2] = {
     760    { 0, -1 },
     761    { -1, -1 },
     762};
     763
     764static const struct HashTableValue staticCustomValueTableValues[1] = {
     765    { "testStaticValue", static_cast<unsigned>(PropertyAttribute::CustomAccessor), NoIntrinsic, { (intptr_t)static_cast<PropertySlot::GetValueFunc>(testStaticValueGetter), (intptr_t)static_cast<PutPropertySlot::PutValueFunc>(testStaticValuePutter) } },
     766};
     767
     768static const struct HashTable staticCustomValueTable =
     769    { 1, 1, true, nullptr, staticCustomValueTableValues, staticCustomValueTableIndex };
     770
     771class StaticCustomValue : public JSNonFinalObject {
     772    using Base = JSNonFinalObject;
     773public:
     774    StaticCustomValue(VM& vm, Structure* structure)
     775        : Base(vm, structure)
     776    {
     777        DollarVMAssertScope assertScope;
     778    }
     779
     780    DECLARE_INFO;
     781
     782    static constexpr unsigned StructureFlags = Base::StructureFlags | HasStaticPropertyTable;
     783
     784    template<typename CellType, SubspaceAccess>
     785    static CompleteSubspace* subspaceFor(VM& vm)
     786    {
     787        return &vm.cellSpace;
     788    }
     789
     790    static Structure* createStructure(VM& vm, JSGlobalObject* globalObject, JSValue prototype)
     791    {
     792        DollarVMAssertScope assertScope;
     793        return Structure::create(vm, globalObject, prototype, TypeInfo(ObjectType, StructureFlags), info());
     794    }
     795
     796    static StaticCustomValue* create(VM& vm, Structure* structure)
     797    {
     798        DollarVMAssertScope assertScope;
     799        StaticCustomValue* accessor = new (NotNull, allocateCell<StaticCustomValue>(vm.heap)) StaticCustomValue(vm, structure);
     800        accessor->finishCreation(vm);
     801        return accessor;
     802    }
     803};
     804
    742805class ObjectDoingSideEffectPutWithoutCorrectSlotStatus : public JSNonFinalObject {
    743806    using Base = JSNonFinalObject;
     
    15461609
    15471610const ClassInfo StaticCustomAccessor::s_info = { "StaticCustomAccessor", &Base::s_info, &staticCustomAccessorTable, nullptr, CREATE_METHOD_TABLE(StaticCustomAccessor) };
     1611const ClassInfo StaticCustomValue::s_info = { "StaticCustomValue", &Base::s_info, &staticCustomValueTable, nullptr, CREATE_METHOD_TABLE(StaticCustomValue) };
    15481612const ClassInfo ObjectDoingSideEffectPutWithoutCorrectSlotStatus::s_info = { "ObjectDoingSideEffectPutWithoutCorrectSlotStatus", &Base::s_info, &staticCustomAccessorTable, nullptr, CREATE_METHOD_TABLE(ObjectDoingSideEffectPutWithoutCorrectSlotStatus) };
    15491613
     
    25332597}
    25342598
     2599static EncodedJSValue JSC_HOST_CALL functionCreateStaticCustomValue(JSGlobalObject* globalObject, CallFrame*)
     2600{
     2601    DollarVMAssertScope assertScope;
     2602    VM& vm = globalObject->vm();
     2603    JSLockHolder lock(vm);
     2604    Structure* structure = StaticCustomValue::createStructure(vm, globalObject, jsNull());
     2605    auto* result = StaticCustomValue::create(vm, structure);
     2606    return JSValue::encode(result);
     2607}
     2608
    25352609static EncodedJSValue JSC_HOST_CALL functionCreateObjectDoingSideEffectPutWithoutCorrectSlotStatus(JSGlobalObject* globalObject, CallFrame* callFrame)
    25362610{
     
    32513325#endif
    32523326    addFunction(vm, "createStaticCustomAccessor", functionCreateStaticCustomAccessor, 0);
     3327    addFunction(vm, "createStaticCustomValue", functionCreateStaticCustomValue, 0);
    32533328    addFunction(vm, "createObjectDoingSideEffectPutWithoutCorrectSlotStatus", functionCreateObjectDoingSideEffectPutWithoutCorrectSlotStatus, 0);
    32543329    addFunction(vm, "createEmptyFunctionWithName", functionCreateEmptyFunctionWithName, 1);
Note: See TracChangeset for help on using the changeset viewer.