142936 2015-03-21 05:02:57 -0700 Array.prototype.shift should check exception after thisObj->get(exec, 0) 2022-10-05 15:43:27 -0700 1 1 1 Unclassified WebKit JavaScriptCore 528+ (Nightly build) Unspecified Unspecified NEW P2 Normal --- 1 ysuzuki ysuzuki ahmad.saleem792 oldest_to_newest 1079060 0 ysuzuki 2015-03-21 05:02:57 -0700 Array.prototype.unshift should check exception after thisObj->get(exec, 0) since it could raise exceptions. 1082212 1 249973 ysuzuki 2015-04-02 05:40:02 -0700 Created attachment 249973 Patch 1082214 2 249974 ysuzuki 2015-04-02 05:58:41 -0700 Created attachment 249974 Patch 1082238 3 249974 darin 2015-04-02 09:13:25 -0700 Comment on attachment 249974 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=249974&action=review I’d like us to find an efficient way to write a lot of tests like this one. I’m not so happy with the code change because it doesn’t fix anything at this time! But I’ll say r=me since I assume you are trying to get to the point where we can remove the callGetter check. > Source/JavaScriptCore/ChangeLog:9 > + Currently, it's not observable. I don’t understand why we would make the change if we can’t observe any benefit. I think it means we added more code for no reason. > Source/JavaScriptCore/ChangeLog:11 > + But I think it's not intended to become unobservable > + because this is guarded by callGetter's workaround check noted as FIXME. Seems that we could deal with this later then. The important part is to write and run the test. I don’t want us to add code that has no observable effect. 1082294 4 249974 ggaren 2015-04-02 11:50:17 -0700 Comment on attachment 249974 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=249974&action=review > Source/JavaScriptCore/runtime/ArrayPrototype.cpp:616 > result = thisObj->get(exec, 0); > + if (exec->hadException()) > + return JSValue::encode(jsUndefined()); > shift<JSArray::ShiftCountForShift>(exec, thisObj, 0, 1, 0, length); Doesn't the call to shift have observable side effects? 1082314 5 249974 ysuzuki 2015-04-02 12:54:56 -0700 Comment on attachment 249974 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=249974&action=review >> Source/JavaScriptCore/runtime/ArrayPrototype.cpp:616 >> shift<JSArray::ShiftCountForShift>(exec, thisObj, 0, 1, 0, length); > > Doesn't the call to shift have observable side effects? Before calling observable side effect like calling getters in the shift function, workaround exception checking in callGetter code works and it immediately returns. 1082315 6 ysuzuki 2015-04-02 12:55:48 -0700 In fact, when dropping the workaround check in callGetter, we can see the regression with the attached test cases in this issue. 1903575 7 ahmad.saleem792 2022-10-05 15:43:27 -0700 I checked the bug ID in Webkit GitHub but it seems this r+ patch didn't landed. But this patch was trying to modify following portion of code +/- few lines: https://github.com/WebKit/WebKit/blob/3c9c31d0fd7c9a2c6579790319f90dc88f98f4f5/Source/JavaScriptCore/runtime/ArrayPrototype.cpp#L1024 Do we need this or we can land only test or we don't need it at all. Appreciate if someone can comment and do actions accordingly. Thanks! 249973 2015-04-02 05:40:02 -0700 2015-04-02 05:58:38 -0700 Patch bug-142936-20150402213917.patch text/plain 2861 ysuzuki U3VidmVyc2lvbiBSZXZpc2lvbjogMTgyMjcwCmRpZmYgLS1naXQgYS9Tb3VyY2UvSmF2YVNjcmlw dENvcmUvQ2hhbmdlTG9nIGIvU291cmNlL0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZwppbmRleCA3 NjcyYThiYjExNWJmYmU4MmE1NWRkMjJiNWJmMzZhYzRjMDZhMjc5Li45ODg1MGM0NGYwYjljNGFh MWM4MGY5N2EwNGUzNDU4MGY4ZjUwMjY1IDEwMDY0NAotLS0gYS9Tb3VyY2UvSmF2YVNjcmlwdENv cmUvQ2hhbmdlTG9nCisrKyBiL1NvdXJjZS9KYXZhU2NyaXB0Q29yZS9DaGFuZ2VMb2cKQEAgLTEs MyArMSwyMCBAQAorMjAxNS0wNC0wMiAgWXVzdWtlIFN1enVraSAgPHV0YXRhbmUudGVhQGdtYWls LmNvbT4KKworICAgICAgICBBcnJheS5wcm90b3R5cGUuc2hpZnQgc2hvdWxkIGNoZWNrIGV4Y2Vw dGlvbiBhZnRlciB0aGlzT2JqLT5nZXQoZXhlYywgMCkKKyAgICAgICAgaHR0cHM6Ly9idWdzLndl YmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTE0MjkzNgorCisgICAgICAgIFJldmlld2VkIGJ5IE5P Qk9EWSAoT09QUyEpLgorCisgICAgICAgIFRoZXJlJ3MgbWlzc2luZyBleGNlcHRpb24gY2hlY2sg aW4gQXJyYXkucHJvdG90eXBlLnNoaWZ0LgorICAgICAgICBDdXJyZW50bHksIGl0J3Mgbm90IG9i c2VydmFibGUuCisgICAgICAgIEJ1dCBJIHRoaW5rIGl0J3Mgbm90IGludGVuZGVkIHRvIGJlY29t ZSB1bm9ic2VydmFibGUKKyAgICAgICAgYmVjYXVzZSB0aGlzIGlzIGd1YXJkZWQgYnkgY2FsbEdl dHRlcidzIHdvcmthcm91bmQgY2hlY2sgbm90ZWQgYXMgRklYTUUuCisKKyAgICAgICAgKiBydW50 aW1lL0FycmF5UHJvdG90eXBlLmNwcDoKKyAgICAgICAgKEpTQzo6YXJyYXlQcm90b0Z1bmNTaGlm dCk6CisgICAgICAgICogdGVzdHMvc3RyZXNzL21pc3NpbmctZXJyb3ItZ3VhcmQtaW4tYXJyYXkt c2hpZnQuanM6IEFkZGVkLgorICAgICAgICAob2JqZWN0LmdldCAwKToKKwogMjAxNS0wNC0wMSAg QWxleCBDaHJpc3RlbnNlbiAgPGFjaHJpc3RlbnNlbkB3ZWJraXQub3JnPgogCiAgICAgICAgIFBy b2dyZXNzIHRvd2FyZHMgQ01ha2Ugb24gV2luZG93cyBhbmQgTWFjLgpkaWZmIC0tZ2l0IGEvU291 cmNlL0phdmFTY3JpcHRDb3JlL3J1bnRpbWUvQXJyYXlQcm90b3R5cGUuY3BwIGIvU291cmNlL0ph dmFTY3JpcHRDb3JlL3J1bnRpbWUvQXJyYXlQcm90b3R5cGUuY3BwCmluZGV4IDg5OTc2NDIyODgx OWJjYzVmNGQyMTQwOWViOTMxNTkzMTBmOTliODQuLmJiNDVhOGIyOWUyZWQzNDE3Y2IwYjU4M2U2 ZDVjMjkxYmIzNmQ0Y2EgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9KYXZhU2NyaXB0Q29yZS9ydW50aW1l L0FycmF5UHJvdG90eXBlLmNwcAorKysgYi9Tb3VyY2UvSmF2YVNjcmlwdENvcmUvcnVudGltZS9B cnJheVByb3RvdHlwZS5jcHAKQEAgLTYxMSw2ICs2MTEsOCBAQCBFbmNvZGVkSlNWYWx1ZSBKU0Nf SE9TVF9DQUxMIGFycmF5UHJvdG9GdW5jU2hpZnQoRXhlY1N0YXRlKiBleGVjKQogICAgICAgICBy ZXN1bHQgPSBqc1VuZGVmaW5lZCgpOwogICAgIH0gZWxzZSB7CiAgICAgICAgIHJlc3VsdCA9IHRo aXNPYmotPmdldChleGVjLCAwKTsKKyAgICAgICAgaWYgKGV4ZWMtPmhhZEV4Y2VwdGlvbigpKQor ICAgICAgICAgICAgcmV0dXJuIEpTVmFsdWU6OmVuY29kZShqc1VuZGVmaW5lZCgpKTsKICAgICAg ICAgc2hpZnQ8SlNBcnJheTo6U2hpZnRDb3VudEZvclNoaWZ0PihleGVjLCB0aGlzT2JqLCAwLCAx LCAwLCBsZW5ndGgpOwogICAgICAgICBpZiAoZXhlYy0+aGFkRXhjZXB0aW9uKCkpCiAgICAgICAg ICAgICByZXR1cm4gSlNWYWx1ZTo6ZW5jb2RlKGpzVW5kZWZpbmVkKCkpOwpkaWZmIC0tZ2l0IGEv U291cmNlL0phdmFTY3JpcHRDb3JlL3Rlc3RzL3N0cmVzcy9taXNzaW5nLWVycm9yLWd1YXJkLWlu LWFycmF5LXNoaWZ0LmpzIGIvU291cmNlL0phdmFTY3JpcHRDb3JlL3Rlc3RzL3N0cmVzcy9taXNz aW5nLWVycm9yLWd1YXJkLWluLWFycmF5LXNoaWZ0LmpzCm5ldyBmaWxlIG1vZGUgMTAwNjQ0Cmlu ZGV4IDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAuLmIzNGY3MTQ0YWUy MzU5ZWRjMTcwZGE0NjkzN2Y2MWY5MTkwYzI5MjUKLS0tIC9kZXYvbnVsbAorKysgYi9Tb3VyY2Uv SmF2YVNjcmlwdENvcmUvdGVzdHMvc3RyZXNzL21pc3NpbmctZXJyb3ItZ3VhcmQtaW4tYXJyYXkt c2hpZnQuanMKQEAgLTAsMCArMSwyOCBAQAordmFyIG9iamVjdCA9IHsKKyAgICBsZW5ndGg6IDIs CisgICAgY291bnQ6IDAsCisgICAgZ2V0IDAoKSB7CisgICAgICAgIG9iamVjdC5jb3VudCsrOwor ICAgICAgICAvLyBPdmVycmlkZQorICAgICAgICBPYmplY3QuZGVmaW5lUHJvcGVydHkodGhpcywg MCwgeworICAgICAgICAgICAgdmFsdWU6IDQyCisgICAgICAgIH0pOworICAgICAgICB0aHJvdyBu ZXcgRXJyb3IoJ2dldHRlciAiMCIgaXMgY2FsbGVkLicpOworICAgIH0sCisgICAgMTogMTEKK307 CisKK3ZhciBlcnJvciA9IG51bGw7Cit0cnkgeworICAgIEFycmF5LnByb3RvdHlwZS5zaGlmdC5j YWxsKG9iamVjdCk7Cit9IGNhdGNoIChlKSB7CisgICAgZXJyb3IgPSBlOworfQoraWYgKCFlcnJv cikKKyAgICB0aHJvdyBuZXcgRXJyb3IoJ25vdCB0aHJvd24nKTsKK2lmIChTdHJpbmcoZXJyb3Ip ICE9PSAnRXJyb3I6IGdldHRlciAiMCIgaXMgY2FsbGVkLicpCisgICAgdGhyb3cgbmV3IEVycm9y KCdiYWQgZXJyb3I6ICcgKyBTdHJpbmcoZXJyb3IpKTsKK2lmIChvYmplY3QuY291bnQgIT09IDEp CisgICAgdGhyb3cgbmV3IEVycm9yKCdiYWQgdmFsdWU6ICcgKyBvYmplY3QuY291bnQpOworaWYg KG9iamVjdFsxXSAhPT0gMTEpCisgICAgdGhyb3cgbmV3IEVycm9yKCdiYWQgdmFsdWU6ICcgKyBv YmplY3RbMV0pOwo= 249974 2015-04-02 05:58:41 -0700 2015-04-02 09:13:25 -0700 Patch bug-142936-20150402215756.patch text/plain 4109 ysuzuki U3VidmVyc2lvbiBSZXZpc2lvbjogMTgyMjcwCmRpZmYgLS1naXQgYS9Tb3VyY2UvSmF2YVNjcmlw dENvcmUvQ2hhbmdlTG9nIGIvU291cmNlL0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZwppbmRleCA3 NjcyYThiYjExNWJmYmU4MmE1NWRkMjJiNWJmMzZhYzRjMDZhMjc5Li45YTJmNzgxMDZjYjk1NWEw MWM3MjIyMDc0NzgxNjFjMzE2MmY1N2ZlIDEwMDY0NAotLS0gYS9Tb3VyY2UvSmF2YVNjcmlwdENv cmUvQ2hhbmdlTG9nCisrKyBiL1NvdXJjZS9KYXZhU2NyaXB0Q29yZS9DaGFuZ2VMb2cKQEAgLTEs MyArMSwyMyBAQAorMjAxNS0wNC0wMiAgWXVzdWtlIFN1enVraSAgPHV0YXRhbmUudGVhQGdtYWls LmNvbT4KKworICAgICAgICBBcnJheS5wcm90b3R5cGUuc2hpZnQgc2hvdWxkIGNoZWNrIGV4Y2Vw dGlvbiBhZnRlciB0aGlzT2JqLT5nZXQoZXhlYywgMCkKKyAgICAgICAgaHR0cHM6Ly9idWdzLndl YmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTE0MjkzNgorCisgICAgICAgIFJldmlld2VkIGJ5IE5P Qk9EWSAoT09QUyEpLgorCisgICAgICAgIFRoZXJlJ3MgbWlzc2luZyBleGNlcHRpb24gY2hlY2sg aW4gQXJyYXkucHJvdG90eXBlLnNoaWZ0LgorICAgICAgICBDdXJyZW50bHksIGl0J3Mgbm90IG9i c2VydmFibGUuCisgICAgICAgIEJ1dCBJIHRoaW5rIGl0J3Mgbm90IGludGVuZGVkIHRvIGJlY29t ZSB1bm9ic2VydmFibGUKKyAgICAgICAgYmVjYXVzZSB0aGlzIGlzIGd1YXJkZWQgYnkgY2FsbEdl dHRlcidzIHdvcmthcm91bmQgY2hlY2sgbm90ZWQgYXMgRklYTUUuCisKKyAgICAgICAgKiBydW50 aW1lL0FycmF5UHJvdG90eXBlLmNwcDoKKyAgICAgICAgKEpTQzo6YXJyYXlQcm90b0Z1bmNTaGlm dCk6CisgICAgICAgICogdGVzdHMvc3RyZXNzL21pc3NpbmctZXJyb3ItZ3VhcmQtaW4tYXJyYXkt c2hpZnQuanM6IEFkZGVkLgorICAgICAgICAob2JqZWN0LmdldCAwKToKKyAgICAgICAgKHRlc3Qu cHJvcGVydHlHZXR0ZXIpOgorICAgICAgICAodGVzdC5nZXQgT2JqZWN0KToKKyAgICAgICAgKHRl c3QpOgorCiAyMDE1LTA0LTAxICBBbGV4IENocmlzdGVuc2VuICA8YWNocmlzdGVuc2VuQHdlYmtp dC5vcmc+CiAKICAgICAgICAgUHJvZ3Jlc3MgdG93YXJkcyBDTWFrZSBvbiBXaW5kb3dzIGFuZCBN YWMuCmRpZmYgLS1naXQgYS9Tb3VyY2UvSmF2YVNjcmlwdENvcmUvcnVudGltZS9BcnJheVByb3Rv dHlwZS5jcHAgYi9Tb3VyY2UvSmF2YVNjcmlwdENvcmUvcnVudGltZS9BcnJheVByb3RvdHlwZS5j cHAKaW5kZXggODk5NzY0MjI4ODE5YmNjNWY0ZDIxNDA5ZWI5MzE1OTMxMGY5OWI4NC4uYmI0NWE4 YjI5ZTJlZDM0MTdjYjBiNTgzZTZkNWMyOTFiYjM2ZDRjYSAxMDA2NDQKLS0tIGEvU291cmNlL0ph dmFTY3JpcHRDb3JlL3J1bnRpbWUvQXJyYXlQcm90b3R5cGUuY3BwCisrKyBiL1NvdXJjZS9KYXZh U2NyaXB0Q29yZS9ydW50aW1lL0FycmF5UHJvdG90eXBlLmNwcApAQCAtNjExLDYgKzYxMSw4IEBA IEVuY29kZWRKU1ZhbHVlIEpTQ19IT1NUX0NBTEwgYXJyYXlQcm90b0Z1bmNTaGlmdChFeGVjU3Rh dGUqIGV4ZWMpCiAgICAgICAgIHJlc3VsdCA9IGpzVW5kZWZpbmVkKCk7CiAgICAgfSBlbHNlIHsK ICAgICAgICAgcmVzdWx0ID0gdGhpc09iai0+Z2V0KGV4ZWMsIDApOworICAgICAgICBpZiAoZXhl Yy0+aGFkRXhjZXB0aW9uKCkpCisgICAgICAgICAgICByZXR1cm4gSlNWYWx1ZTo6ZW5jb2RlKGpz VW5kZWZpbmVkKCkpOwogICAgICAgICBzaGlmdDxKU0FycmF5OjpTaGlmdENvdW50Rm9yU2hpZnQ+ KGV4ZWMsIHRoaXNPYmosIDAsIDEsIDAsIGxlbmd0aCk7CiAgICAgICAgIGlmIChleGVjLT5oYWRF eGNlcHRpb24oKSkKICAgICAgICAgICAgIHJldHVybiBKU1ZhbHVlOjplbmNvZGUoanNVbmRlZmlu ZWQoKSk7CmRpZmYgLS1naXQgYS9Tb3VyY2UvSmF2YVNjcmlwdENvcmUvdGVzdHMvc3RyZXNzL21p c3NpbmctZXJyb3ItZ3VhcmQtaW4tYXJyYXktc2hpZnQuanMgYi9Tb3VyY2UvSmF2YVNjcmlwdENv cmUvdGVzdHMvc3RyZXNzL21pc3NpbmctZXJyb3ItZ3VhcmQtaW4tYXJyYXktc2hpZnQuanMKbmV3 IGZpbGUgbW9kZSAxMDA2NDQKaW5kZXggMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAw MDAwMDAwMC4uMGVkY2Y3MjJjMmQ5MGY4OWIzYmRmNTMwMDk2OGU0ODcxMjAyYjgxZAotLS0gL2Rl di9udWxsCisrKyBiL1NvdXJjZS9KYXZhU2NyaXB0Q29yZS90ZXN0cy9zdHJlc3MvbWlzc2luZy1l cnJvci1ndWFyZC1pbi1hcnJheS1zaGlmdC5qcwpAQCAtMCwwICsxLDc2IEBACit2YXIgb2JqZWN0 ID0geworICAgIGxlbmd0aDogMiwKKyAgICBjb3VudDogMCwKKyAgICBnZXQgMCgpIHsKKyAgICAg ICAgb2JqZWN0LmNvdW50Kys7CisgICAgICAgIC8vIE92ZXJyaWRlCisgICAgICAgIE9iamVjdC5k ZWZpbmVQcm9wZXJ0eSh0aGlzLCAwLCB7CisgICAgICAgICAgICB2YWx1ZTogNDIKKyAgICAgICAg fSk7CisgICAgICAgIHRocm93IG5ldyBFcnJvcignZ2V0dGVyICIwIiBpcyBjYWxsZWQuJyk7Cisg ICAgfSwKKyAgICAxOiAxMQorfTsKKwordmFyIGVycm9yID0gbnVsbDsKK3RyeSB7CisgICAgQXJy YXkucHJvdG90eXBlLnNoaWZ0LmNhbGwob2JqZWN0KTsKK30gY2F0Y2ggKGUpIHsKKyAgICBlcnJv ciA9IGU7Cit9CitpZiAoIWVycm9yKQorICAgIHRocm93IG5ldyBFcnJvcignbm90IHRocm93bicp OworaWYgKFN0cmluZyhlcnJvcikgIT09ICdFcnJvcjogZ2V0dGVyICIwIiBpcyBjYWxsZWQuJykK KyAgICB0aHJvdyBuZXcgRXJyb3IoJ2JhZCBlcnJvcjogJyArIFN0cmluZyhlcnJvcikpOworaWYg KG9iamVjdC5jb3VudCAhPT0gMSkKKyAgICB0aHJvdyBuZXcgRXJyb3IoJ2JhZCB2YWx1ZTogJyAr IG9iamVjdC5jb3VudCk7CitpZiAob2JqZWN0WzFdICE9PSAxMSkKKyAgICB0aHJvdyBuZXcgRXJy b3IoJ2JhZCB2YWx1ZTogJyArIG9iamVjdFsxXSk7CisKK3ZhciBvYmplY3QgPSB7CisgICAgbGVu Z3RoOiAyLAorICAgIGNvdW50OiAwLAorICAgIGdldCAwKCkgeworICAgICAgICBvYmplY3QuY291 bnQrKzsKKyAgICAgICAgdGhyb3cgbmV3IEVycm9yKCdnZXR0ZXIgIjAiIGlzIGNhbGxlZC4nKTsK KyAgICB9LAorICAgIDE6IDExCit9OworCit2YXIgZXJyb3IgPSBudWxsOwordHJ5IHsKKyAgICBB cnJheS5wcm90b3R5cGUuc2hpZnQuY2FsbChvYmplY3QpOworfSBjYXRjaCAoZSkgeworICAgIGVy cm9yID0gZTsKK30KK2lmICghZXJyb3IpCisgICAgdGhyb3cgbmV3IEVycm9yKCdub3QgdGhyb3du Jyk7CitpZiAoU3RyaW5nKGVycm9yKSAhPT0gJ0Vycm9yOiBnZXR0ZXIgIjAiIGlzIGNhbGxlZC4n KQorICAgIHRocm93IG5ldyBFcnJvcignYmFkIGVycm9yOiAnICsgU3RyaW5nKGVycm9yKSk7Citp ZiAob2JqZWN0LmNvdW50ICE9PSAxKQorICAgIHRocm93IG5ldyBFcnJvcignYmFkIHZhbHVlOiAn ICsgb2JqZWN0LmNvdW50KTsKK2lmIChvYmplY3RbMV0gIT09IDExKQorICAgIHRocm93IG5ldyBF cnJvcignYmFkIHZhbHVlOiAnICsgb2JqZWN0WzFdKTsKKworZnVuY3Rpb24gdGVzdChmKSB7CisK KyAgICB2YXIgdGVzdE9iaiA9IHsKKyAgICAgICAgbGVuZ3RoOiAzCisgICAgfTsKKyAgICB2YXIg cHJvcGVydHlHZXR0ZXIgPSB7CisgICAgICAgIGdldDogKGZ1bmN0aW9uKCkgeyB0aHJvdyB0cnVl OyB9KQorICAgIH0KKyAgICBPYmplY3QuZGVmaW5lUHJvcGVydHkodGVzdE9iaiwgMCwgcHJvcGVy dHlHZXR0ZXIpOworICAgIE9iamVjdC5kZWZpbmVQcm9wZXJ0eSh0ZXN0T2JqLCAxLCB7IGdldDog TWFwIH0pOyAgLy8gVGhyb3cgVHlwZUVycm9yIGltbWVkaWF0ZWx5IGZyb20gdGhlIGhvc3QgZnVu Y3Rpb24uCisgICAgT2JqZWN0LmRlZmluZVByb3BlcnR5KHRlc3RPYmosIDIsIHByb3BlcnR5R2V0 dGVyKTsKKworICAgIHRyeSB7CisgICAgICAgIGYuY2FsbCh0ZXN0T2JqLCBmdW5jdGlvbigpe30p OworICAgICAgICByZXR1cm4gZmFsc2U7CisgICAgfSBjYXRjaCAoZSkgeworICAgICAgICByZXR1 cm4gZSA9PT0gdHJ1ZTsKKyAgICB9Cit9Cit2YXIgcmV0ID0gdGVzdChBcnJheS5wcm90b3R5cGUu c2hpZnQpOworaWYgKCFyZXQpCisgICAgdGhyb3cgbmV3IEVycm9yKCdiYWQgdmFsdWU6ICcgKyBy ZXQpOwo=