⚠ Archived content — this site is no longer maintained.   Current WebKit documentation is at docs.webkit.org.

Changeset 196849 in webkit


Ignore:
Timestamp:
Feb 19, 2016, 5:51:48 PM (11 years ago)
Author:
barraclough@apple.com
Message:

JSObject::getPropertySlot - index-as-propertyname, override on prototype, & shadow
​https://bugs.webkit.org/show_bug.cgi?id=154416

Reviewed by Geoff Garen.

Source/JavaScriptCore:

Here's the bug. Suppose you call JSObject::getOwnProperty and -

  • PropertyName contains an index,
  • An object on the prototype chain overrides getOwnPropertySlot, and has that index property,
  • The base of the access (or another object on the prototype chain) shadows that property.

JSObject::getPropertySlot is written assuming the common case is that propertyName is not an
index, and as such walks up the prototype chain looking for non-index properties before it
tries calling parseIndex.

At the point we reach an object on the prototype chain overriding getOwnPropertySlot (which
would potentially return the property) we may have already skipped over non-overriding
objects that contain the property in index storage.

  • runtime/JSObject.h:

(JSC::JSObject::getOwnNonIndexPropertySlot):

  • renamed from inlineGetOwnPropertySlot to better describe behaviour; added ASSERT guarding that this method never returns index properties - if it ever does, this is unsafe for getPropertySlot.

(JSC::JSObject::getOwnPropertySlot):

  • inlineGetOwnPropertySlot -> getOwnNonIndexPropertySlot.

(JSC::JSObject::getPropertySlot):

  • In case of object overriding getOwnPropertySlot check if propertyName is an index.

(JSC::JSObject::getNonIndexPropertySlot):

  • called by getPropertySlot if we encounter an object that overrides getOwnPropertySlot, in order to avoid repeated calls to parseIndex.

(JSC::JSObject::inlineGetOwnPropertySlot): Deleted.

  • this was renamed to getOwnNonIndexPropertySlot.

(JSC::JSObject::fastGetOwnPropertySlot): Deleted.

  • this was folded back in to getPropertySlot.

Source/WebCore:

  • testing/Internals.cpp:

(WebCore::Internals::isReadableStreamDisturbed):

  • fastGetOwnPropertySlot -> getOwnPropertySlot (internal method removed; test shouldn't really have been using this anyway)

LayoutTests:

  • js/index-property-shadows-overriden-get-own-property-slot-expected.txt: Added.
  • js/index-property-shadows-overriden-get-own-property-slot.html: Added.
  • js/script-tests/index-property-shadows-overriden-get-own-property-slot.js: Added.

(test):

  • added test case.
Location:
trunk
Files:
3 added
5 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r196846 r196849  
     12016-02-18  Gavin Barraclough  <barraclough@apple.com>
     2
     3        JSObject::getPropertySlot - index-as-propertyname, override on prototype, & shadow
     4        https://bugs.webkit.org/show_bug.cgi?id=154416
     5
     6        Reviewed by Geoff Garen.
     7
     8        * js/index-property-shadows-overriden-get-own-property-slot-expected.txt: Added.
     9        * js/index-property-shadows-overriden-get-own-property-slot.html: Added.
     10        * js/script-tests/index-property-shadows-overriden-get-own-property-slot.js: Added.
     11        (test):
     12            - added test case.
     13
    1142016-02-19  Chris Dumez  <cdumez@apple.com>
    215

  • trunk/Source/JavaScriptCore/ChangeLog

    r196836 r196849  
     12016-02-18  Gavin Barraclough  <barraclough@apple.com>
     2
     3        JSObject::getPropertySlot - index-as-propertyname, override on prototype, & shadow
     4        https://bugs.webkit.org/show_bug.cgi?id=154416
     5
     6        Reviewed by Geoff Garen.
     7
     8        Here's the bug. Suppose you call JSObject::getOwnProperty and -
     9          - PropertyName contains an index,
     10          - An object on the prototype chain overrides getOwnPropertySlot, and has that index property,
     11          - The base of the access (or another object on the prototype chain) shadows that property.
     12
     13        JSObject::getPropertySlot is written assuming the common case is that propertyName is not an
     14        index, and as such walks up the prototype chain looking for non-index properties before it
     15        tries calling parseIndex.
     16
     17        At the point we reach an object on the prototype chain overriding getOwnPropertySlot (which
     18        would potentially return the property) we may have already skipped over non-overriding
     19        objects that contain the property in index storage.
     20
     21        * runtime/JSObject.h:
     22        (JSC::JSObject::getOwnNonIndexPropertySlot):
     23            - renamed from inlineGetOwnPropertySlot to better describe behaviour;
     24              added ASSERT guarding that this method never returns index properties -
     25              if it ever does, this is unsafe for getPropertySlot.
     26        (JSC::JSObject::getOwnPropertySlot):
     27            - inlineGetOwnPropertySlot -> getOwnNonIndexPropertySlot.
     28        (JSC::JSObject::getPropertySlot):
     29            - In case of object overriding getOwnPropertySlot check if propertyName is an index.
     30        (JSC::JSObject::getNonIndexPropertySlot):
     31            - called by getPropertySlot if we encounter an object that overrides getOwnPropertySlot,
     32              in order to avoid repeated calls to parseIndex.
     33        (JSC::JSObject::inlineGetOwnPropertySlot): Deleted.
     34            - this was renamed to getOwnNonIndexPropertySlot.
     35        (JSC::JSObject::fastGetOwnPropertySlot): Deleted.
     36            - this was folded back in to getPropertySlot.
     37
    1382016-02-19  Saam Barati  <sbarati@apple.com>
    239

  • trunk/Source/JavaScriptCore/runtime/JSObject.h

    r196772 r196849  
    114114    JSValue get(ExecState*, unsigned propertyName) const;
    115115
    116     bool fastGetOwnPropertySlot(ExecState*, VM&, Structure&, PropertyName, PropertySlot&);
    117116    bool getPropertySlot(ExecState*, PropertyName, PropertySlot&);
    118117    bool getPropertySlot(ExecState*, unsigned propertyName, PropertySlot&);
    … …  
    860859    JS_EXPORT_PRIVATE NEVER_INLINE void putInlineSlow(ExecState*, PropertyName, JSValue, PutPropertySlot&);
    861860
    862     bool inlineGetOwnPropertySlot(VM&, Structure&, PropertyName, PropertySlot&);
     861    bool getNonIndexPropertySlot(ExecState*, PropertyName, PropertySlot&);
     862    bool getOwnNonIndexPropertySlot(VM&, Structure&, PropertyName, PropertySlot&);
    863863    JS_EXPORT_PRIVATE void fillGetterPropertySlot(PropertySlot&, JSValue, unsigned, PropertyOffset);
    864864    void fillCustomGetterPropertySlot(PropertySlot&, JSValue, unsigned, Structure&);
    … …  
    10951095}
    10961096
    1097 ALWAYS_INLINE bool JSObject::inlineGetOwnPropertySlot(VM& vm, Structure& structure, PropertyName propertyName, PropertySlot& slot)
     1097// It is safe to call this method with a PropertyName that is actually an index,
     1098// but if so will always return false (doesn't search index storage).
     1099ALWAYS_INLINE bool JSObject::getOwnNonIndexPropertySlot(VM& vm, Structure& structure, PropertyName propertyName, PropertySlot& slot)
    10981100{
    10991101    unsigned attributes;
    … …  
    11011103    if (!isValidOffset(offset))
    11021104        return false;
     1105
     1106    // getPropertySlot relies on this method never returning index properties!
     1107    ASSERT(!parseIndex(propertyName));
    11031108
    11041109    JSValue value = getDirect(offset);
    … …  
    11391144    VM& vm = exec->vm();
    11401145    Structure& structure = *object->structure(vm);
    1141     if (object->inlineGetOwnPropertySlot(vm, structure, propertyName, slot))
     1146    if (object->getOwnNonIndexPropertySlot(vm, structure, propertyName, slot))
    11421147        return true;
    11431148    if (Optional<uint32_t> index = parseIndex(propertyName))
    … …  
    11461151}
    11471152
    1148 ALWAYS_INLINE bool JSObject::fastGetOwnPropertySlot(ExecState* exec, VM& vm, Structure& structure, PropertyName propertyName, PropertySlot& slot)
    1149 {
    1150     if (LIKELY(!TypeInfo::overridesGetOwnPropertySlot(inlineTypeFlags())))
    1151         return inlineGetOwnPropertySlot(vm, structure, propertyName, slot);
    1152     return structure.classInfo()->methodTable.getOwnPropertySlot(this, exec, propertyName, slot);
    1153 }
    1154 
    11551153// It may seem crazy to inline a function this large but it makes a big difference
    11561154// since this is function very hot in variable lookup
    … …  
    11611159    JSObject* object = this;
    11621160    while (true) {
     1161        if (UNLIKELY(TypeInfo::overridesGetOwnPropertySlot(object->inlineTypeFlags()))) {
     1162            // If propertyName is an index then we may have missed it (as this loop is using
     1163            // getOwnNonIndexPropertySlot), so we cannot safely call the overridden getOwnPropertySlot
     1164            // (lest we return a property from a prototype that is shadowed). Check now for an index,
     1165            // if so we need to start afresh from this object.
     1166            if (Optional<uint32_t> index = parseIndex(propertyName))
     1167                return getPropertySlot(exec, index.value(), slot);
     1168            // Safe to continue searching from current position; call getNonIndexPropertySlot to avoid
     1169            // parsing the int again.
     1170            return object->getNonIndexPropertySlot(exec, propertyName, slot);
     1171        }
    11631172        Structure& structure = *structureIDTable.get(object->structureID());
    1164         if (object->fastGetOwnPropertySlot(exec, vm, structure, propertyName, slot))
     1173        if (object->getOwnNonIndexPropertySlot(vm, structure, propertyName, slot))
    11651174            return true;
    11661175        JSValue prototype = structure.storedPrototype();
    … …  
    11831192        Structure& structure = *structureIDTable.get(object->structureID());
    11841193        if (structure.classInfo()->methodTable.getOwnPropertySlotByIndex(object, exec, propertyName, slot))
     1194            return true;
     1195        JSValue prototype = structure.storedPrototype();
     1196        if (!prototype.isObject())
     1197            return false;
     1198        object = asObject(prototype);
     1199    }
     1200}
     1201
     1202ALWAYS_INLINE bool JSObject::getNonIndexPropertySlot(ExecState* exec, PropertyName propertyName, PropertySlot& slot)
     1203{
     1204    // This method only supports non-index PropertyNames.
     1205    ASSERT(!parseIndex(propertyName));
     1206
     1207    VM& vm = exec->vm();
     1208    auto& structureIDTable = vm.heap.structureIDTable();
     1209    JSObject* object = this;
     1210    while (true) {
     1211        Structure& structure = *structureIDTable.get(object->structureID());
     1212        if (LIKELY(!TypeInfo::overridesGetOwnPropertySlot(object->inlineTypeFlags()))) {
     1213            if (object->getOwnNonIndexPropertySlot(vm, structure, propertyName, slot))
     1214                return true;
     1215        } else if (structure.classInfo()->methodTable.getOwnPropertySlot(object, exec, propertyName, slot))
    11851216            return true;
    11861217        JSValue prototype = structure.storedPrototype();

  • trunk/Source/WebCore/ChangeLog

    r196846 r196849  
     12016-02-18  Gavin Barraclough  <barraclough@apple.com>
     2
     3        JSObject::getPropertySlot - index-as-propertyname, override on prototype, & shadow
     4        https://bugs.webkit.org/show_bug.cgi?id=154416
     5
     6        Reviewed by Geoff Garen.
     7
     8        * testing/Internals.cpp:
     9        (WebCore::Internals::isReadableStreamDisturbed):
     10            - fastGetOwnPropertySlot -> getOwnPropertySlot
     11              (internal method removed; test shouldn't really have been using this anyway)
     12
    1132016-02-19  Chris Dumez  <cdumez@apple.com>
    214

  • trunk/Source/WebCore/testing/Internals.cpp

    r196833 r196849  
    34623462    JSValue value;
    34633463    PropertySlot propertySlot(value, PropertySlot::InternalMethodType::Get);
    3464     globalObject->fastGetOwnPropertySlot(&state, state.vm(), *globalObject->structure(), privateName, propertySlot);
     3464    globalObject->methodTable()->getOwnPropertySlot(globalObject, &state, privateName, propertySlot);
    34653465    value = propertySlot.getValue(&state, privateName);
    34663466    ASSERT(value.isFunction());
Note: See TracChangeset for help on using the changeset viewer.