Bug 208708

Summary: @putByValDirect does not perform [[DefineOwnProperty]] correctly
Product: WebKit Reporter: Alexey Shvayka <ashvayka>
Component: JavaScriptCoreAssignee: Alexey Shvayka <ashvayka>
Status: RESOLVED FIXED    
Severity: Minor CC: commit-queue, darin, ews-watchlist, keith_miller, mark.lam, msaboff, saam, tzagallo, webkit-bug-importer, ysuzuki
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: All   
OS: All   
Bug Depends on:    
Bug Blocks: 163446    
Attachments:
Description Flags
Patch
none
Patch
none
Patch none

Description Alexey Shvayka 2020-03-06 05:49:13 PST 
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
Comment 1 Alexey Shvayka 2020-03-06 06:01:10 PST 
Created attachment 392713 [details]
Patch
Comment 2 Yusuke Suzuki 2020-03-08 01:49:40 PST 
Can you measure JetStream2 score (by 6 times runs) to see the effect of this patch?
Comment 3 Alexey Shvayka 2020-03-08 17:01:35 PDT 
(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
Comment 4 Alexey Shvayka 2020-03-08 17:05:30 PDT 
Created attachment 393001 [details]
Patch

Add note on JetStream2 results.
Comment 5 Yusuke Suzuki 2020-03-09 00:11:04 PDT 
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?
Comment 6 Alexey Shvayka 2020-03-09 12:03:10 PDT 
Created attachment 393060 [details]
Patch

Set reviewer and make ChangeLog more detailed.
Comment 7 WebKit Commit Bot 2020-03-09 16:17:47 PDT 
Comment on attachment 393060 [details]
Patch

Clearing flags on attachment: 393060

Committed r258170: <https://trac.webkit.org/changeset/258170>
Comment 8 WebKit Commit Bot 2020-03-09 16:17:48 PDT 
All reviewed patches have been landed.  Closing bug.
Comment 9 Radar WebKit Bug Importer 2020-03-09 16:18:18 PDT 
<rdar://problem/60248529>