Changeset 247173 in webkit


Ignore:
Timestamp:
Jul 5, 2019 1:26:02 PM (5 years ago)
Author:
commit-queue@webkit.org
Message:

[JSC] Clean up ArraySpeciesCreate
https://bugs.webkit.org/show_bug.cgi?id=182434

Patch by Alexey Shvayka <Alexey Shvayka> on 2019-07-05
Reviewed by Yusuke Suzuki.

JSTests:

Adjusts error message expectations in stress tests.

  • stress/array-flatmap.js:
  • stress/array-flatten.js:
  • stress/array-species-create-should-handle-masquerader.js:
  • test262/expectations.yaml: Mark 4 test cases as passing.

Source/JavaScriptCore:

We have duplicate code in arraySpeciesCreate, filter, map, concatSlowPath of ArrayPrototype.js
and speciesConstructArray of ArrayPrototype.cpp. This patch fixes cross-realm Array constructor
detection in native speciesConstructArray, upgrades length type to correctly handle large integers,
and exposes it as @arraySpeciesCreate. Also removes now unused @isArrayConstructor private function.
Native speciesConstructArray is preferred because it has fast path via speciesWatchpointIsValid.

Thoroughly benchmarked: this change progresses ARES-6 by 0-1%.

  • builtins/ArrayPrototype.js:

(filter):
(map):
(globalPrivate.concatSlowPath):
(globalPrivate.arraySpeciesCreate): Deleted.

  • builtins/BuiltinNames.h:
  • runtime/ArrayConstructor.cpp:

(JSC::arrayConstructorPrivateFuncIsArrayConstructor): Deleted.

  • runtime/ArrayConstructor.h:
  • runtime/ArrayPrototype.cpp:

(JSC::arrayProtoFuncSpeciesCreate):

  • runtime/ArrayPrototype.h:
  • runtime/JSGlobalObject.cpp:

(JSC::JSGlobalObject::init):

Location:
trunk
Files:
13 edited

Legend:

Unmodified
Added
Removed
  • trunk/JSTests/ChangeLog

    r247088 r247173  
     12019-07-05  Alexey Shvayka  <shvaikalesh@gmail.com>
     2
     3        [JSC] Clean up ArraySpeciesCreate
     4        https://bugs.webkit.org/show_bug.cgi?id=182434
     5
     6        Reviewed by Yusuke Suzuki.
     7
     8        Adjusts error message expectations in stress tests.
     9
     10        * stress/array-flatmap.js:
     11        * stress/array-flatten.js:
     12        * stress/array-species-create-should-handle-masquerader.js:
     13        * test262/expectations.yaml: Mark 4 test cases as passing.
     14
    1152019-07-02  Michael Saboff  <msaboff@apple.com>
    216
  • trunk/JSTests/stress/array-flatmap.js

    r228266 r247173  
    6767shouldThrow(() => {
    6868    flatMap.call(array2, () => {});
    69 }, `TypeError: 0 is not a constructor`);
     69}, `TypeError: Species construction did not get a valid constructor`);
    7070
    7171var array2 = new realm.Array;
  • trunk/JSTests/stress/array-flatten.js

    r232226 r247173  
    7878shouldThrow(() => {
    7979    flat.call(array2);
    80 }, `TypeError: 0 is not a constructor`);
     80}, `TypeError: Species construction did not get a valid constructor`);
    8181
    8282var array2 = new realm.Array;
  • trunk/JSTests/stress/array-species-create-should-handle-masquerader.js

    r239761 r247173  
    1818    shouldThrow(() => {
    1919        new (class extends Array { static get [Symbol.species]() { return makeMasquerader(); } })(1, 2, 3).flat().constructor
    20     }, `TypeError: Masquerader is not a constructor`);
     20    }, `TypeError: Species construction did not get a valid constructor`);
    2121}
  • trunk/JSTests/test262/expectations.yaml

    r246765 r247173  
    661661  default: 'Test262Error: Expected a StopReverse but got a Test262Error'
    662662  strict mode: 'Test262Error: Expected a StopReverse but got a Test262Error'
    663 test/built-ins/Array/prototype/slice/create-proto-from-ctor-realm-non-array.js:
    664   default: 'Test262Error: Expected SameValue(«», «[object Object]») to be true'
    665   strict mode: 'Test262Error: Expected SameValue(«», «[object Object]») to be true'
    666663test/built-ins/Array/prototype/slice/length-exceeding-integer-limit-proxied-array.js:
    667664  default: 'Test262Error: Expected [] and [9007199254740989, 9007199254740990] to have the same contents. slice(9007199254740989)'
     
    679676  default: 'Test262Error: Length is 2**53 - 1 Expected SameValue(«4294967295», «9007199254740991») to be true'
    680677  strict mode: 'Test262Error: Length is 2**53 - 1 Expected SameValue(«4294967295», «9007199254740991») to be true'
    681 test/built-ins/Array/prototype/splice/create-proto-from-ctor-realm-non-array.js:
    682   default: 'Test262Error: Expected SameValue(«», «[object Object]») to be true'
    683   strict mode: 'Test262Error: Expected SameValue(«», «[object Object]») to be true'
    684678test/built-ins/Array/prototype/splice/create-proxy.js:
    685679  default: 'TypeError: Attempted to assign to readonly property.'
  • trunk/Source/JavaScriptCore/ChangeLog

    r247166 r247173  
     12019-07-05  Alexey Shvayka  <shvaikalesh@gmail.com>
     2
     3        [JSC] Clean up ArraySpeciesCreate
     4        https://bugs.webkit.org/show_bug.cgi?id=182434
     5
     6        Reviewed by Yusuke Suzuki.
     7
     8        We have duplicate code in arraySpeciesCreate, filter, map, concatSlowPath of ArrayPrototype.js
     9        and speciesConstructArray of ArrayPrototype.cpp. This patch fixes cross-realm Array constructor
     10        detection in native speciesConstructArray, upgrades `length` type to correctly handle large integers,
     11        and exposes it as @arraySpeciesCreate. Also removes now unused @isArrayConstructor private function.
     12        Native speciesConstructArray is preferred because it has fast path via speciesWatchpointIsValid.
     13
     14        Thoroughly benchmarked: this change progresses ARES-6 by 0-1%.
     15
     16        * builtins/ArrayPrototype.js:
     17        (filter):
     18        (map):
     19        (globalPrivate.concatSlowPath):
     20        (globalPrivate.arraySpeciesCreate): Deleted.
     21        * builtins/BuiltinNames.h:
     22        * runtime/ArrayConstructor.cpp:
     23        (JSC::arrayConstructorPrivateFuncIsArrayConstructor): Deleted.
     24        * runtime/ArrayConstructor.h:
     25        * runtime/ArrayPrototype.cpp:
     26        (JSC::arrayProtoFuncSpeciesCreate):
     27        * runtime/ArrayPrototype.h:
     28        * runtime/JSGlobalObject.cpp:
     29        (JSC::JSGlobalObject::init):
     30
    1312019-07-05  Tadeu Zagallo  <tzagallo@apple.com>
    232
  • trunk/Source/JavaScriptCore/builtins/ArrayPrototype.js

    r246567 r247173  
    176176   
    177177    var thisArg = @argument(1);
    178 
    179     // Do 9.4.2.3 ArraySpeciesCreate
    180     var result;
    181     var constructor;
    182     if (@isArray(array)) {
    183         constructor = array.constructor;
    184         // We have this check so that if some array from a different global object
    185         // calls this map they don't get an array with the Array.prototype of the
    186         // other global object.
    187         if (@Array !== constructor && @isArrayConstructor(constructor))
    188             constructor = @undefined;
    189         if (@isObject(constructor)) {
    190             constructor = constructor.@speciesSymbol;
    191             if (constructor === null)
    192                 constructor = @undefined;
    193         }
    194     }
    195     if (constructor === @Array || constructor === @undefined)
    196         result = @newArrayWithSize(0);
    197     else
    198         result = new constructor(0);
     178    var result = @arraySpeciesCreate(array, 0);
    199179
    200180    var nextIndex = 0;
     
    222202   
    223203    var thisArg = @argument(1);
    224 
    225     // Do 9.4.2.3 ArraySpeciesCreate
    226     var result;
    227     var constructor;
    228     if (@isArray(array)) {
    229         constructor = array.constructor;
    230         // We have this check so that if some array from a different global object
    231         // calls this map they don't get an array with the Array.prototype of the
    232         // other global object.
    233         if (@Array !== constructor && @isArrayConstructor(constructor))
    234             constructor = @undefined;
    235         if (@isObject(constructor)) {
    236             constructor = constructor.@speciesSymbol;
    237             if (constructor === null)
    238                 constructor = @undefined;
    239         }
    240     }
    241     if (constructor === @Array || constructor === @undefined)
    242         result = @newArrayWithSize(length);
    243     else
    244         result = new constructor(length);
     204    var result = @arraySpeciesCreate(array, length);
    245205
    246206    for (var i = 0; i < length; i++) {
     
    621581
    622582    var currentElement = @toObject(this, "Array.prototype.concat requires that |this| not be null or undefined");
    623 
    624     var constructor;
    625     if (@isArray(currentElement)) {
    626         constructor = currentElement.constructor;
    627         // We have this check so that if some array from a different global object
    628         // calls this map they don't get an array with the Array.prototype of the
    629         // other global object.
    630         if (@Array !== constructor && @isArrayConstructor(constructor))
    631             constructor = @undefined;
    632         else if (@isObject(constructor)) {
    633             constructor = constructor.@speciesSymbol;
    634             if (constructor === null)
    635                 constructor = @Array;
    636         }
    637     }
    638 
    639583    var argCount = arguments.length;
    640     var result;
    641     if (constructor === @Array || constructor === @undefined)
    642         result = @newArrayWithSize(0);
    643     else
    644         result = new constructor(0);
     584
     585    var result = @arraySpeciesCreate(currentElement, 0);
    645586    var resultIsArray = @isJSArray(result);
    646587
     
    745686
    746687@globalPrivate
    747 function arraySpeciesCreate(array, length)
    748 {
    749     "use strict";
    750 
    751     if (!@isArray(array))
    752         return @newArrayWithSize(length);
    753 
    754     var constructor = array.constructor;
    755     var arrayConstructorInRealm = @Array;
    756     // We have this check so that if some array from a different global object
    757     // calls this map they don't get an array with the Array.prototype of the
    758     // other global object.
    759     if (arrayConstructorInRealm !== constructor && @isArrayConstructor(constructor))
    760         return @newArrayWithSize(length);
    761 
    762     if (@isObject(constructor)) {
    763         constructor = constructor.@speciesSymbol;
    764         if (@isUndefinedOrNull(constructor))
    765             return @newArrayWithSize(length);
    766     }
    767 
    768     if (constructor === arrayConstructorInRealm || constructor === @undefined)
    769         return @newArrayWithSize(length);
    770     return new constructor(length);
    771 }
    772 
    773 @globalPrivate
    774688function flatIntoArray(target, source, sourceLength, targetIndex, depth)
    775689{
  • trunk/Source/JavaScriptCore/builtins/BuiltinNames.h

    r246567 r247173  
    5151    macro(arrayIteratorIsDone) \
    5252    macro(arrayIteratorKind) \
     53    macro(arraySpeciesCreate) \
    5354    macro(assert) \
    5455    macro(charCodeAt) \
     
    133134    macro(instanceOf) \
    134135    macro(isArraySlow) \
    135     macro(isArrayConstructor) \
    136136    macro(isConstructor) \
    137137    macro(concatMemcpy) \
  • trunk/Source/JavaScriptCore/runtime/ArrayConstructor.cpp

    r242650 r247173  
    147147}
    148148
    149 EncodedJSValue JSC_HOST_CALL arrayConstructorPrivateFuncIsArrayConstructor(ExecState* exec)
    150 {
    151     VM& vm = exec->vm();
    152     return JSValue::encode(jsBoolean(jsDynamicCast<ArrayConstructor*>(vm, exec->uncheckedArgument(0))));
    153 }
    154 
    155149} // namespace JSC
  • trunk/Source/JavaScriptCore/runtime/ArrayConstructor.h

    r239191 r247173  
    5959JSArray* constructArrayWithSizeQuirk(ExecState*, ArrayAllocationProfile*, JSGlobalObject*, JSValue length, JSValue prototype = JSValue());
    6060
    61 EncodedJSValue JSC_HOST_CALL arrayConstructorPrivateFuncIsArrayConstructor(ExecState*);
    6261EncodedJSValue JSC_HOST_CALL arrayConstructorPrivateFuncIsArraySlow(ExecState*);
    6362bool isArraySlow(ExecState*, ProxyObject* argument);
  • trunk/Source/JavaScriptCore/runtime/ArrayPrototype.cpp

    r246837 r247173  
    214214};
    215215
    216 static ALWAYS_INLINE std::pair<SpeciesConstructResult, JSObject*> speciesConstructArray(ExecState* exec, JSObject* thisObject, unsigned length)
     216static ALWAYS_INLINE std::pair<SpeciesConstructResult, JSObject*> speciesConstructArray(ExecState* exec, JSObject* thisObject, uint64_t length)
    217217{
    218218    VM& vm = exec->vm();
     
    239239        if (constructor.isConstructor(vm)) {
    240240            JSObject* constructorObject = jsCast<JSObject*>(constructor);
    241             if (exec->lexicalGlobalObject() != constructorObject->globalObject(vm))
    242                 return std::make_pair(SpeciesConstructResult::FastPath, nullptr);;
     241            bool isArrayConstructorFromAnotherRealm = exec->lexicalGlobalObject() != constructorObject->globalObject(vm)
     242                && constructorObject->inherits<ArrayConstructor>(vm);
     243            if (isArrayConstructorFromAnotherRealm)
     244                return std::make_pair(SpeciesConstructResult::FastPath, nullptr);
    243245        }
    244246        if (constructor.isObject()) {
     
    246248            RETURN_IF_EXCEPTION(scope, exceptionResult());
    247249            if (constructor.isNull())
    248                 return std::make_pair(SpeciesConstructResult::FastPath, nullptr);;
     250                return std::make_pair(SpeciesConstructResult::FastPath, nullptr);
    249251        }
    250252    } else {
     
    262264    RETURN_IF_EXCEPTION(scope, exceptionResult());
    263265    return std::make_pair(SpeciesConstructResult::CreatedObject, newObject);
     266}
     267
     268EncodedJSValue JSC_HOST_CALL arrayProtoFuncSpeciesCreate(ExecState* exec)
     269{
     270    VM& vm = exec->vm();
     271    auto scope = DECLARE_THROW_SCOPE(vm);
     272    JSObject* object = asObject(exec->uncheckedArgument(0));
     273    uint64_t length = static_cast<uint64_t>(exec->uncheckedArgument(1).asNumber());
     274
     275    std::pair<SpeciesConstructResult, JSObject*> speciesResult = speciesConstructArray(exec, object, length);
     276    EXCEPTION_ASSERT(!!scope.exception() == (speciesResult.first == SpeciesConstructResult::Exception));
     277    if (UNLIKELY(speciesResult.first == SpeciesConstructResult::Exception))
     278        return { };
     279    if (speciesResult.first == SpeciesConstructResult::CreatedObject)
     280        return JSValue::encode(speciesResult.second);
     281
     282    if (length > std::numeric_limits<unsigned>::max()) {
     283        throwRangeError(exec, scope, "Array size is not a small enough positive integer."_s);
     284        return { };
     285    }
     286
     287    RELEASE_AND_RETURN(scope, JSValue::encode(constructEmptyArray(exec, nullptr, length)));
    264288}
    265289
  • trunk/Source/JavaScriptCore/runtime/ArrayPrototype.h

    r242902 r247173  
    5151};
    5252
     53EncodedJSValue JSC_HOST_CALL arrayProtoFuncSpeciesCreate(ExecState*);
    5354EncodedJSValue JSC_HOST_CALL arrayProtoFuncToString(ExecState*);
    5455EncodedJSValue JSC_HOST_CALL arrayProtoFuncValues(ExecState*);
  • trunk/Source/JavaScriptCore/runtime/JSGlobalObject.cpp

    r246837 r247173  
    909909    JSFunction* privateFuncDateTimeFormat = JSFunction::create(vm, this, 0, String(), globalFuncDateTimeFormat);
    910910#endif
    911     JSFunction* privateFuncIsArrayConstructor = JSFunction::create(vm, this, 0, String(), arrayConstructorPrivateFuncIsArrayConstructor);
    912911    JSFunction* privateFuncIsArraySlow = JSFunction::create(vm, this, 0, String(), arrayConstructorPrivateFuncIsArraySlow);
    913912    JSFunction* privateFuncConcatMemcpy = JSFunction::create(vm, this, 0, String(), arrayProtoPrivateFuncConcatMemcpy);
     
    989988
    990989        GlobalPropertyInfo(vm.propertyNames->builtinNames().repeatCharacterPrivateName(), JSFunction::create(vm, this, 2, String(), stringProtoFuncRepeatCharacter), PropertyAttribute::DontEnum | PropertyAttribute::DontDelete | PropertyAttribute::ReadOnly),
     990        GlobalPropertyInfo(vm.propertyNames->builtinNames().arraySpeciesCreatePrivateName(), JSFunction::create(vm, this, 2, String(), arrayProtoFuncSpeciesCreate), PropertyAttribute::DontEnum | PropertyAttribute::DontDelete | PropertyAttribute::ReadOnly),
    991991        GlobalPropertyInfo(vm.propertyNames->builtinNames().isArrayPrivateName(), arrayConstructor->getDirect(vm, vm.propertyNames->isArray), PropertyAttribute::DontEnum | PropertyAttribute::DontDelete | PropertyAttribute::ReadOnly),
    992992        GlobalPropertyInfo(vm.propertyNames->builtinNames().isArraySlowPrivateName(), privateFuncIsArraySlow, PropertyAttribute::DontEnum | PropertyAttribute::DontDelete | PropertyAttribute::ReadOnly),
    993         GlobalPropertyInfo(vm.propertyNames->builtinNames().isArrayConstructorPrivateName(), privateFuncIsArrayConstructor, PropertyAttribute::DontEnum | PropertyAttribute::DontDelete | PropertyAttribute::ReadOnly),
    994993        GlobalPropertyInfo(vm.propertyNames->builtinNames().concatMemcpyPrivateName(), privateFuncConcatMemcpy, PropertyAttribute::DontEnum | PropertyAttribute::DontDelete | PropertyAttribute::ReadOnly),
    995994        GlobalPropertyInfo(vm.propertyNames->builtinNames().appendMemcpyPrivateName(), privateFuncAppendMemcpy, PropertyAttribute::DontEnum | PropertyAttribute::DontDelete | PropertyAttribute::ReadOnly),
Note: See TracChangeset for help on using the changeset viewer.