216121 2020-09-03 04:58:55 -0700 Array.prototype.push should always perform [[Set]] in strict mode 2020-09-04 15:37:18 -0700 1 1 1 Unclassified WebKit JavaScriptCore WebKit Nightly Build All All RESOLVED FIXED https://bugs.webkit.org/show_bug.cgi?id=211205 InRadar, WebExposed P2 Trivial --- 1 ashvayka ashvayka darin ews-watchlist keith_miller mark.lam msaboff ross.kirsling saam tzagallo webkit-bug-importer oldest_to_newest 1685230 0 ashvayka 2020-09-03 04:58:55 -0700 Array.prototype.push should always perform [[Set]] in strict mode 1685235 1 407877 ashvayka 2020-09-03 06:05:41 -0700 Created attachment 407877 Patch 1685237 2 ashvayka 2020-09-03 06:10:41 -0700 (In reply to Alexey Shvayka from comment #1) > Created attachment 407877 [details] > Patch Warmed-up runs, --outer 50: r266507 patch new-array-push 6.5605+-0.4641 6.2634+-0.4522 might be 1.0474x faster array-push-0 350.3329+-2.1429 348.3786+-1.2194 array-push-1 733.3222+-13.1726 ? 739.4494+-13.0772 ? might be 1.0084x slower array-push-2 822.0986+-4.4731 ? 822.2864+-3.1685 ? array-push-3 836.1861+-24.9409 832.5322+-23.3867 might be 1.0044x faster --- I've vetted all puts via PutPropertySlot in JSC: other usages set strict mode correctly. 1685300 3 darin 2020-09-03 09:51:15 -0700 Do you understand why the JSC tests are failing on EWS? 1685301 4 ashvayka 2020-09-03 09:53:05 -0700 (In reply to Darin Adler from comment #3) > Do you understand why the JSC tests are failing on EWS? Igalia's servers seem to be down. I don't think it is related. 1685578 5 ashvayka 2020-09-04 00:45:52 -0700 EWS is all-green now. Thank you for review! 1685580 6 ews-feeder 2020-09-04 01:05:40 -0700 Committed r266581: <https://trac.webkit.org/changeset/266581> All reviewed patches have been landed. Closing bug and clearing flags on attachment 407877. 1685581 7 webkit-bug-importer 2020-09-04 01:06:16 -0700 <rdar://problem/68330312> 1685817 8 407877 darin 2020-09-04 12:30:18 -0700 Comment on attachment 407877 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=407877&action=review > Source/JavaScriptCore/runtime/ArrayPrototype.cpp:919 > + thisObj->putByIndexInline(globalObject, static_cast<uint64_t>(length + n), callFrame->uncheckedArgument(n), true); Sorry I didn’t notice this when reviewing. Can we remove this unnecessary static_cast? It caused me concern just now: I was worried that the addition was incorrectly performed in 32 bits with possible overflow before promoting the result to 64 bits. But it’s not, because length is already uint64_t, and for the same reason the static_cast is not needed or helpful. 1685884 9 ashvayka 2020-09-04 15:34:00 -0700 Committed r266641: <https://trac.webkit.org/changeset/266641> 1685887 10 ashvayka 2020-09-04 15:37:18 -0700 (In reply to Darin Adler from comment #8) > Can we remove this unnecessary static_cast? It caused me concern just now: I > was worried that the addition was incorrectly performed in 32 bits with > possible overflow before promoting the result to 64 bits. But it’s not, > because length is already uint64_t, and for the same reason the static_cast > is not needed or helpful. Thank you for the follow-up! I've also checked another casts in ArrayPrototype.cpp, all seem necessary. 407877 2020-09-03 06:05:41 -0700 2020-09-04 01:05:40 -0700 Patch bug-216121-20200903160540.patch text/plain 4117 ashvayka SW5kZXg6IEpTVGVzdHMvQ2hhbmdlTG9nCj09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIEpTVGVzdHMvQ2hhbmdlTG9n CShyZXZpc2lvbiAyNjY1MTApCisrKyBKU1Rlc3RzL0NoYW5nZUxvZwkod29ya2luZyBjb3B5KQpA QCAtMSwzICsxLDEyIEBACisyMDIwLTA5LTAzICBBbGV4ZXkgU2h2YXlrYSAgPHNodmFpa2FsZXNo QGdtYWlsLmNvbT4KKworICAgICAgICBBcnJheS5wcm90b3R5cGUucHVzaCBzaG91bGQgYWx3YXlz IHBlcmZvcm0gW1tTZXRdXSBpbiBzdHJpY3QgbW9kZQorICAgICAgICBodHRwczovL2J1Z3Mud2Vi a2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9MjE2MTIxCisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9C T0RZIChPT1BTISkuCisKKyAgICAgICAgKiB0ZXN0MjYyL2V4cGVjdGF0aW9ucy55YW1sOiBNYXJr IDIgdGVzdCBjYXNlcyBhcyBwYXNzaW5nLgorCiAyMDIwLTA5LTAyICBZdXN1a2UgU3V6dWtpICA8 eXN1enVraUBhcHBsZS5jb20+CiAKICAgICAgICAgW0pTQ10gVXBkYXRlIHRlc3QyNjIKSW5kZXg6 IEpTVGVzdHMvdGVzdDI2Mi9leHBlY3RhdGlvbnMueWFtbAo9PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBKU1Rlc3Rz L3Rlc3QyNjIvZXhwZWN0YXRpb25zLnlhbWwJKHJldmlzaW9uIDI2NjUwNykKKysrIEpTVGVzdHMv dGVzdDI2Mi9leHBlY3RhdGlvbnMueWFtbAkod29ya2luZyBjb3B5KQpAQCAtNjAzLDkgKzYwMyw2 IEBAIHRlc3QvYW5uZXhCL2xhbmd1YWdlL2dsb2JhbC1jb2RlL3N3aXRjaC0KICAgZGVmYXVsdDog J1Rlc3QyNjJFcnJvcjogQW4gaW5pdGlhbGl6ZWQgYmluZGluZyBpcyBub3QgY3JlYXRlZCBwcmlv ciB0byBldmFsdWF0aW9uIEV4cGVjdGVkIGEgUmVmZXJlbmNlRXJyb3IgdG8gYmUgdGhyb3duIGJ1 dCBubyBleGNlcHRpb24gd2FzIHRocm93biBhdCBhbGwnCiB0ZXN0L2FubmV4Qi9sYW5ndWFnZS9n bG9iYWwtY29kZS9zd2l0Y2gtZGZsdC1nbG9iYWwtc2tpcC1lYXJseS1lcnIuanM6CiAgIGRlZmF1 bHQ6ICJTeW50YXhFcnJvcjogQ2Fubm90IGRlY2xhcmUgYSBmdW5jdGlvbiB0aGF0IHNoYWRvd3Mg YSBsZXQvY29uc3QvY2xhc3MvZnVuY3Rpb24gdmFyaWFibGUgJ2YnIGluIHN0cmljdCBtb2RlLiIK LXRlc3QvYnVpbHQtaW5zL0FycmF5L3Byb3RvdHlwZS9wdXNoL2xlbmd0aC1uZWFyLWludGVnZXIt bGltaXQtc2V0LWZhaWx1cmUuanM6Ci0gIGRlZmF1bHQ6ICdUZXN0MjYyRXJyb3I6IEV4cGVjdGVk IGEgVHlwZUVycm9yIHRvIGJlIHRocm93biBidXQgbm8gZXhjZXB0aW9uIHdhcyB0aHJvd24gYXQg YWxsJwotICBzdHJpY3QgbW9kZTogJ1Rlc3QyNjJFcnJvcjogRXhwZWN0ZWQgYSBUeXBlRXJyb3Ig dG8gYmUgdGhyb3duIGJ1dCBubyBleGNlcHRpb24gd2FzIHRocm93biBhdCBhbGwnCiB0ZXN0L2J1 aWx0LWlucy9BcnJheUJ1ZmZlci9wcm90b3R5cGUvYnl0ZUxlbmd0aC9kZXRhY2hlZC1idWZmZXIu anM6CiAgIGRlZmF1bHQ6ICdUZXN0MjYyRXJyb3I6IEV4cGVjdGVkIGEgVHlwZUVycm9yIHRvIGJl IHRocm93biBidXQgbm8gZXhjZXB0aW9uIHdhcyB0aHJvd24gYXQgYWxsJwogICBzdHJpY3QgbW9k ZTogJ1Rlc3QyNjJFcnJvcjogRXhwZWN0ZWQgYSBUeXBlRXJyb3IgdG8gYmUgdGhyb3duIGJ1dCBu byBleGNlcHRpb24gd2FzIHRocm93biBhdCBhbGwnCkluZGV4OiBTb3VyY2UvSmF2YVNjcmlwdENv cmUvQ2hhbmdlTG9nCj09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIFNvdXJjZS9KYXZhU2NyaXB0Q29yZS9DaGFuZ2VM b2cJKHJldmlzaW9uIDI2NjUxMCkKKysrIFNvdXJjZS9KYXZhU2NyaXB0Q29yZS9DaGFuZ2VMb2cJ KHdvcmtpbmcgY29weSkKQEAgLTEsMyArMSwyMSBAQAorMjAyMC0wOS0wMyAgQWxleGV5IFNodmF5 a2EgIDxzaHZhaWthbGVzaEBnbWFpbC5jb20+CisKKyAgICAgICAgQXJyYXkucHJvdG90eXBlLnB1 c2ggc2hvdWxkIGFsd2F5cyBwZXJmb3JtIFtbU2V0XV0gaW4gc3RyaWN0IG1vZGUKKyAgICAgICAg aHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTIxNjEyMQorCisgICAgICAg IFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAgIFRoaXMgcGF0Y2ggZml4ZXMg YXJyYXlQcm90b0Z1bmNQdXNoKCkgdG8gdGhyb3cgYSBUeXBlRXJyb3IgaWYgcHV0dGluZyBhbgor ICAgICAgICBpbmRleCBiZXlvbmQgVUlOVDMyX01BWCBoYXMgZmFpbGVkLCBhbGlnbmluZyBKU0Mg d2l0aCB0aGUgc3BlYyBbMV0sIFY4LAorICAgICAgICBhbmQgU3BpZGVyTW9ua2V5LiBBbHNvLCBy ZWZhY3RvcnMgdGhlIG1ldGhvZCBsZXZlcmFnaW5nIHB1dEJ5SW5kZXhJbmxpbmUoKS4KKworICAg ICAgICBBcnJheS5wcm90b3R5cGUucHVzaCBtaWNyb2JlbmNobWFya3MsIGluY2x1ZGluZyB2YXJh cmdzIHRlc3RzLCBhcmUgbmV1dHJhbC4KKworICAgICAgICBbMV06IGh0dHBzOi8vdGMzOS5lcy9l Y21hMjYyLyNzZWMtYXJyYXkucHJvdG90eXBlLnB1c2ggKHN0ZXAgNS5iKQorCisgICAgICAgICog cnVudGltZS9BcnJheVByb3RvdHlwZS5jcHA6CisgICAgICAgIChKU0M6OmFycmF5UHJvdG9GdW5j UHVzaCk6CisKIDIwMjAtMDktMDIgIE1pY2hhZWwgU2Fib2ZmICA8bXNhYm9mZkBhcHBsZS5jb20+ CiAKICAgICAgICAgQVNTRVJUSU9OIEZBSUxFRDogdmFsdWUuaXNDZWxsKCkgJiYgdmFsdWUuYXND ZWxsKCktPnR5cGUoKSA9PSBDdXN0b21HZXR0ZXJTZXR0ZXJUeXBlIC4vYnl0ZWNvZGUvT2JqZWN0 UHJvcGVydHlDb25kaXRpb25TZXQuY3BwCkluZGV4OiBTb3VyY2UvSmF2YVNjcmlwdENvcmUvcnVu dGltZS9BcnJheVByb3RvdHlwZS5jcHAKPT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0gU291cmNlL0phdmFTY3JpcHRD b3JlL3J1bnRpbWUvQXJyYXlQcm90b3R5cGUuY3BwCShyZXZpc2lvbiAyNjY1MDcpCisrKyBTb3Vy Y2UvSmF2YVNjcmlwdENvcmUvcnVudGltZS9BcnJheVByb3RvdHlwZS5jcHAJKHdvcmtpbmcgY29w eSkKQEAgLTkxNiwxNCArOTE2LDggQEAgRW5jb2RlZEpTVmFsdWUgSlNDX0hPU1RfQ0FMTCBhcnJh eVByb3RvRgogICAgICAgICByZXR1cm4gdGhyb3dWTVR5cGVFcnJvcihnbG9iYWxPYmplY3QsIHNj b3BlLCAicHVzaCBjYW5ub3QgcHJvZHVjZSBhbiBhcnJheSBvZiBsZW5ndGggbGFyZ2VyIHRoYW4g KDIgKiogNTMpIC0gMSJfcyk7CiAKICAgICBmb3IgKHVuc2lnbmVkIG4gPSAwOyBuIDwgYXJnQ291 bnQ7IG4rKykgewotICAgICAgICBpZiAoTElLRUxZKGxlbmd0aCArIG4gPD0gTUFYX0FSUkFZX0lO REVYKSkKLSAgICAgICAgICAgIHRoaXNPYmotPm1ldGhvZFRhYmxlKHZtKS0+cHV0QnlJbmRleCh0 aGlzT2JqLCBnbG9iYWxPYmplY3QsIHN0YXRpY19jYXN0PHVpbnQzMl90PihsZW5ndGggKyBuKSwg Y2FsbEZyYW1lLT51bmNoZWNrZWRBcmd1bWVudChuKSwgdHJ1ZSk7Ci0gICAgICAgIGVsc2Ugewot ICAgICAgICAgICAgUHV0UHJvcGVydHlTbG90IHNsb3QodGhpc09iaik7Ci0gICAgICAgICAgICBJ ZGVudGlmaWVyIHByb3BlcnR5TmFtZSA9IElkZW50aWZpZXI6OmZyb20odm0sIGxlbmd0aCArIG4p OwotICAgICAgICAgICAgdGhpc09iai0+bWV0aG9kVGFibGUodm0pLT5wdXQodGhpc09iaiwgZ2xv YmFsT2JqZWN0LCBwcm9wZXJ0eU5hbWUsIGNhbGxGcmFtZS0+dW5jaGVja2VkQXJndW1lbnQobiks IHNsb3QpOwotICAgICAgICB9Ci0gICAgICAgIFJFVFVSTl9JRl9FWENFUFRJT04oc2NvcGUsIGVu Y29kZWRKU1ZhbHVlKCkpOworICAgICAgICB0aGlzT2JqLT5wdXRCeUluZGV4SW5saW5lKGdsb2Jh bE9iamVjdCwgc3RhdGljX2Nhc3Q8dWludDY0X3Q+KGxlbmd0aCArIG4pLCBjYWxsRnJhbWUtPnVu Y2hlY2tlZEFyZ3VtZW50KG4pLCB0cnVlKTsKKyAgICAgICAgUkVUVVJOX0lGX0VYQ0VQVElPTihz Y29wZSwgeyB9KTsKICAgICB9CiAgICAgCiAgICAgdWludDY0X3QgbmV3TGVuZ3RoID0gbGVuZ3Ro ICsgYXJnQ291bnQ7Cg==