Bug 191730 – RegExpObject's collectMatches should not be using JSArray::push to fill in its match results.

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

[Mark Lam]

According to the spec https://www.ecma-international.org/ecma-262/9.0/index.html#sec-regexp.prototype-@@match, the RegExp match results are filling in using the spec's CreateDataProperty() which does not consult the prototype for setters.  JSArray:push() consults the prototype for setters.  We should be using putDirectIndex() instead.

<rdar://problem/46048517>

[Mark Lam]

Created attachment 355006 [details]
proposed patch.

[David Kilzer (:ddkilzer)]

Comment on attachment 355006 [details]
proposed patch.

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

> JSTests/ChangeLog:10
>  2018-11-13  Saam Barati  <[e-mail]>

Nit: Blank line before the new entry

[EWS Watchlist]

Comment on attachment 355006 [details]
proposed patch.

Attachment 355006 [details] did not pass jsc-ews (mac):
Output: https://webkit-queues.webkit.org/results/10011421

New failing tests:
stress/regress-187006.js.ftl-eager-no-cjit-b3o1
stress/regress-187006.js.ftl-no-cjit-small-pool
stress/regress-187006.js.ftl-no-cjit-validate-sampling-profiler
stress/regress-187006.js.default
stress/regress-187006.js.dfg-eager-no-cjit-validate
stress/regress-187006.js.ftl-eager
stress/regress-187006.js.dfg-eager
stress/regress-187006.js.dfg-maximal-flush-validate-no-cjit
stress/regress-187006.js.ftl-no-cjit-no-put-stack-validate
stress/regress-187006.js.no-ftl
stress/regress-187006.js.ftl-no-cjit-b3o1
stress/regress-187006.js.no-llint
stress/regress-187006.js.ftl-eager-no-cjit
stress/regress-187006.js.ftl-no-cjit-no-inline-validate
stress/regress-187006.js.no-cjit-validate-phases
stress/regress-187006.js.no-cjit-collect-continuously
apiTests

[Saam Barati]

Comment on attachment 355006 [details]
proposed patch.

r=me

[Mark Lam]

(In reply to Build Bot from comment #3)
> New failing tests:
> stress/regress-187006.js.ftl-eager-no-cjit-b3o1
...
> apiTests

Turns out, the regress-187006.js test is invalid.  It expects a setter in Array.prototype to be called when RegExp.match creates its match array, which violates the spec.

[Mark Lam]

Created attachment 355017 [details]
patch for landing.

[Mark Lam]

Thanks for the review.  Landed in r238270: <http://trac.webkit.org/r238270>.