Bug 208708 – @putByValDirect does not perform [[DefineOwnProperty]] correctly

• Status: RESOLVED
• Resolution: FIXED
• Component: JavaScriptCore

[Alexey Shvayka]

CreateDataPropertyOrThrow (https://tc39.es/ecma262/#sec-createdatapropertyorthrow), called on property with descriptor:

1. {writable: false, configurable: true}
Expected: results in {value, writable: true, enumerable: true, configurable: true}
Actual: TypeError is thrown (seems like [[Set]] is performed instead)
ECMA262: https://tc39.es/ecma262/#sec-validateandapplypropertydescriptor (step 9.a)
Test262: https://test262.report/browse/built-ins/Array/of/does-not-use-set-for-indices.js

2. {writable: true, configurable: false} or {set: function() {}, configurable: false}
Expected: should throw TypeError
Actual: results in {value, writable: true, enumerable: true, configurable: true} (seems like descriptor validation is skipped)
ECMA262: https://tc39.es/ecma262/#sec-validateandapplypropertydescriptor (step 4.a)
Test262:
  https://test262.report/browse/built-ins/Array/prototype/filter/target-array-with-non-configurable-property.js
  https://test262.report/browse/built-ins/Array/prototype/map/target-array-with-non-configurable-property.js

[Alexey Shvayka]

Created attachment 392713 [details]
Patch

[Yusuke Suzuki]

Can you measure JetStream2 score (by 6 times runs) to see the effect of this patch?

[Alexey Shvayka]

(In reply to Yusuke Suzuki from comment #2)
> Can you measure JetStream2 score (by 6 times runs) to see the effect of this patch?

Geometric mean of 6 hot JetStream2 runs (60°C CPU temp, MacBookAir8,1):

trunk: 109.810
patch: 110.567

[Alexey Shvayka]

Created attachment 393001 [details]
Patch

Add note on JetStream2 results.

[Yusuke Suzuki]

Comment on attachment 393001 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=393001&action=review

r=me with ChangeLog change.

> Source/JavaScriptCore/ChangeLog:10
> +        This change fixes @putByValDirect to perform [[DefineOwnProperty]] per spec [1] while preserving
> +        existing behavior for Arguments exotic objects (thus the checks order in canDoFastPutDirectIndex),
> +        aligning JSC with V8 and SpiderMonkey.

Can you describe what is changed in detail?

[Alexey Shvayka]

Created attachment 393060 [details]
Patch

Set reviewer and make ChangeLog more detailed.

[WebKit Commit Bot]

Comment on attachment 393060 [details]
Patch

Clearing flags on attachment: 393060

Committed r258170: <https://trac.webkit.org/changeset/258170>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/60248529>