Ignore:
Timestamp:
Sep 29, 2017, 4:48:10 PM (8 years ago)
Author:
[email protected]
Message:

Custom GetterSetterAccessCase does not use the correct slotBase when making call
https://bugs.webkit.org/show_bug.cgi?id=177639

Reviewed by Geoffrey Garen.

JSTests:

  • stress/custom-get-set-inline-caching-one-level-up-proto-chain.js: Added.

(assert):
(Class):
(items.forEach):
(set get for):

Source/JavaScriptCore:

The bug occurred when you had a custom set value. Custom set/get
values are passed the property holder, not the base of the access.
If we had an object chain like this:
o = {proto: thingWithCustomSetValue}

We would end up not providing thingWithCustomSetValue as the argument
to the PutValueFunc. The reason is, we would use generateConditionsForPrototypePropertyHitCustom
for custom sets. This would return to us an empty ConditionSet, because
the property holder was only one level up the prototype chain. The reason
is, it didn't generate a condition for the slot holder, because the
protocol for custom set/get is that if an object responds to a custom
setter/getter, it will continue to respond to that getter/setter for
the lifetime of that object. Therefore, it's not strictly necessary to
generate an OPC for the slot base for custom accesses. However, AccessCase
uses !m_conditionSet.isEmtpy() to indicate that the IC is doing a prototype
access. With the above object "o", we were doing a prototype access, but we
had an empty condition set. This lead us to passing the base instead of
the property holder to the custom set value function, which is incorrect.

With custom getters, we never called to into the generateConditionsForPrototypePropertyHitCustom
API. Gets would always call into generateConditionsForPrototypePropertyHit, which
will generate an OPC on the slot base, even if it isn't strictly necessary for custom accessors.
This patch simply removes generateConditionsForPrototypePropertyHitCustom
and aligns the set case with the get case. It makes us properly detect
when we're doing a prototype access with the above object "o". If we find
that generateConditionsForPrototypePropertyHitCustom was a worthwhile
optimization to have, we can re-introduce it. We'll just need to pipe through
a new notion of when we're doing prototype accesses that doesn't rely solely
on !m_conditionSet.isEmpty().

  • bytecode/ObjectPropertyConditionSet.cpp:

(JSC::generateConditionsForPrototypePropertyHitCustom): Deleted.

  • bytecode/ObjectPropertyConditionSet.h:
  • jit/Repatch.cpp:

(JSC::tryCachePutByID):

  • jsc.cpp:

(JSTestCustomGetterSetter::JSTestCustomGetterSetter):
(JSTestCustomGetterSetter::create):
(JSTestCustomGetterSetter::createStructure):
(customGetAccessor):
(customGetValue):
(customSetAccessor):
(customSetValue):
(JSTestCustomGetterSetter::finishCreation):
(GlobalObject::finishCreation):
(functionLoadGetterFromGetterSetter):
(functionCreateCustomTestGetterSetter):

  • runtime/PropertySlot.h:

(JSC::PropertySlot::setCustomGetterSetter):

File:
1 edited

Legend:

Unmodified
Added
Removed
  • trunk/Source/JavaScriptCore/runtime/PropertySlot.h

    r222473 r222671  
    2727#include "ScopeOffset.h"
    2828#include <wtf/Assertions.h>
     29#include <wtf/ForbidHeapAllocation.h>
    2930
    3031namespace JSC {
     
    8182
    8283class PropertySlot {
     84
     85    // We rely on PropertySlot being stack allocated when used. This is needed
     86    // because we rely on some of its fields being a GC root. For example, it
     87    // may be the only thing that points to the CustomGetterSetter property it has.
     88    WTF_FORBID_HEAP_ALLOCATION;
     89
    8390    enum PropertyType : uint8_t {
    8491        TypeUnset,
     
    291298    {
    292299        ASSERT(attributes == attributesForStructure(attributes));
     300
     301        disableCaching();
    293302
    294303        ASSERT(getterSetter);
Note: See TracChangeset for help on using the changeset viewer.