201857 2019-09-16 19:20:35 -0700 [JSC] CheckArray+NonArray is not filtering out Array in AI 2019-09-17 12:52:52 -0700 1 1 1 Unclassified WebKit New Bugs WebKit Nightly Build Unspecified Unspecified RESOLVED FIXED InRadar P2 Normal --- 1 ysuzuki ysuzuki ews-watchlist keith_miller mark.lam msaboff saam tzagallo webkit-bug-importer oldest_to_newest 1571386 0 ysuzuki 2019-09-16 19:20:35 -0700 [JSC] CheckArray+NonArray is not filtering out Array in AI 1571387 1 378928 ysuzuki 2019-09-16 19:23:12 -0700 Created attachment 378928 Patch 1571388 2 ysuzuki 2019-09-16 19:23:14 -0700 <rdar://problem/54194820> 1571596 3 378928 keith_miller 2019-09-17 11:22:40 -0700 Comment on attachment 378928 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=378928&action=review r=me with some comments. > Source/JavaScriptCore/ChangeLog:10 > + While we are assuming that CheckArray+NonArray can ensure that it only passes non-array inputs, DFG::ArrayMode::alreadyChecked Nit: While we assume CheckArray+NonArray ensures it only... > Source/JavaScriptCore/dfg/DFGArrayMode.cpp:441 > + // But here, we already filtered TypedArrays. So, just handling it like NonArray. Nit: So, just handle it like a NonArray. > Source/JavaScriptCore/dfg/DFGArrayMode.cpp:478 > + for (unsigned i = value.m_structure.size(); i--;) { > + RegisteredStructure structure = value.m_structure[i]; > + if (structure.get() != originalStructure) Isn't this a set so it should have size 1? How is it possible to have more than one copy of the same structure? 1571626 4 378928 ysuzuki 2019-09-17 12:33:32 -0700 Comment on attachment 378928 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=378928&action=review Thanks! >> Source/JavaScriptCore/ChangeLog:10 >> + While we are assuming that CheckArray+NonArray can ensure that it only passes non-array inputs, DFG::ArrayMode::alreadyChecked > > Nit: While we assume CheckArray+NonArray ensures it only... Fixed. >> Source/JavaScriptCore/dfg/DFGArrayMode.cpp:441 >> + // But here, we already filtered TypedArrays. So, just handling it like NonArray. > > Nit: So, just handle it like a NonArray. Fixed. >> Source/JavaScriptCore/dfg/DFGArrayMode.cpp:478 >> + if (structure.get() != originalStructure) > > Isn't this a set so it should have size 1? How is it possible to have more than one copy of the same structure? Yeah, we can just check the size, getting onlySttructure, and comparing it with this originalStructure. Fixed. 1571639 5 ysuzuki 2019-09-17 12:52:52 -0700 Committed r249976: <https://trac.webkit.org/changeset/249976> 378928 2019-09-16 19:23:12 -0700 2019-09-17 11:22:40 -0700 Patch bug-201857-20190916192312.patch text/plain 7088 ysuzuki U3VidmVyc2lvbiBSZXZpc2lvbjogMjQ5OTI2CmRpZmYgLS1naXQgYS9Tb3VyY2UvSmF2YVNjcmlw dENvcmUvQ2hhbmdlTG9nIGIvU291cmNlL0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZwppbmRleCA0 NWE5OGIxYWJmMThlZDE4MGRjM2Q5MGU3YTk3MGFjMTc4MzIyZDFjLi45MDhkNTdhM2Y3MjJkZWE5 YzZmZTdiMzNmYWFlYjc3NmE3ZDg5OGRmIDEwMDY0NAotLS0gYS9Tb3VyY2UvSmF2YVNjcmlwdENv cmUvQ2hhbmdlTG9nCisrKyBiL1NvdXJjZS9KYXZhU2NyaXB0Q29yZS9DaGFuZ2VMb2cKQEAgLTEs MyArMSwxOSBAQAorMjAxOS0wOS0xNiAgWXVzdWtlIFN1enVraSAgPHlzdXp1a2lAYXBwbGUuY29t PgorCisgICAgICAgIFtKU0NdIENoZWNrQXJyYXkrTm9uQXJyYXkgaXMgbm90IGZpbHRlcmluZyBv dXQgQXJyYXkgaW4gQUkKKyAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcu Y2dpP2lkPTIwMTg1NworICAgICAgICA8cmRhcjovL3Byb2JsZW0vNTQxOTQ4MjA+CisKKyAgICAg ICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAgICAgVGhlIGNvZGUgb2YgREZH OjpBcnJheU1vZGU6OmFscmVhZHlDaGVja2VkIGlzIGRpZmZlcmVudCBmcm9tIFNwZWN1bGF0aXZl SklUJ3MgQ2hlY2tBcnJheSAvIENoZWNrU3RydWN0dXJlLgorICAgICAgICBXaGlsZSB3ZSBhcmUg YXNzdW1pbmcgdGhhdCBDaGVja0FycmF5K05vbkFycmF5IGNhbiBlbnN1cmUgdGhhdCBpdCBvbmx5 IHBhc3NlcyBub24tYXJyYXkgaW5wdXRzLCBERkc6OkFycmF5TW9kZTo6YWxyZWFkeUNoZWNrZWQK KyAgICAgICAgYWNjZXB0cyBhcnJheXMgdG9vLiBTbyBDaGVja0FycmF5K05vbkFycmF5IGlzIHJl bW92ZWQgaW4gQUkgaWYgdGhlIGlucHV0IGlzIHByb3ZlbiB0aGF0IGl0IGlzIGFuIGFycmF5Lgor ICAgICAgICBUaGlzIHBhdGNoIGFsaWducyBERkc6OkFycmF5TW9kZTo6YWxyZWFkeUNoZWNrZWQg dG8gdGhlIGNoZWNrcyBkb25lIGF0IHJ1bnRpbWUuCisKKyAgICAgICAgKiBkZmcvREZHQXJyYXlN b2RlLmNwcDoKKyAgICAgICAgKEpTQzo6REZHOjpBcnJheU1vZGU6OmFscmVhZHlDaGVja2VkIGNv bnN0KToKKwogMjAxOS0wOS0xNiAgTWljaGFlbCBTYWJvZmYgIDxtc2Fib2ZmQGFwcGxlLmNvbT4K IAogICAgICAgICBbSlNDXSBQZXJmb3JtIGNoZWNrIGFnYWluIHdoZW4gd2UgZm91bmQgbm9uLUJN UCBjaGFyYWN0ZXJzCmRpZmYgLS1naXQgYS9Tb3VyY2UvSmF2YVNjcmlwdENvcmUvZGZnL0RGR0Fy cmF5TW9kZS5jcHAgYi9Tb3VyY2UvSmF2YVNjcmlwdENvcmUvZGZnL0RGR0FycmF5TW9kZS5jcHAK aW5kZXggYzBiNDM3N2JjNmVmOTAyYmUxNTMzZjViMjc5ZDFiNTAzNzg1MzQwZS4uOGRjMDQ0MWUz ZmIwYjE4ZDI2ZmZjZDExYTQ1MGRiZDM4NDg5MmJiYSAxMDA2NDQKLS0tIGEvU291cmNlL0phdmFT Y3JpcHRDb3JlL2RmZy9ERkdBcnJheU1vZGUuY3BwCisrKyBiL1NvdXJjZS9KYXZhU2NyaXB0Q29y ZS9kZmcvREZHQXJyYXlNb2RlLmNwcApAQCAtNDE3LDU1ICs0MTcsNzAgQEAgU3RydWN0dXJlKiBB cnJheU1vZGU6Om9yaWdpbmFsQXJyYXlTdHJ1Y3R1cmUoR3JhcGgmIGdyYXBoLCBOb2RlKiBub2Rl KSBjb25zdAogCiBib29sIEFycmF5TW9kZTo6YWxyZWFkeUNoZWNrZWQoR3JhcGgmIGdyYXBoLCBO b2RlKiBub2RlLCBjb25zdCBBYnN0cmFjdFZhbHVlJiB2YWx1ZSwgSW5kZXhpbmdUeXBlIHNoYXBl KSBjb25zdAogeworICAgIEFTU0VSVChpc1NwZWNpZmljKCkpOworCisgICAgSW5kZXhpbmdUeXBl IGluZGV4aW5nTW9kZU1hc2sgPSBJc0FycmF5IHwgSW5kZXhpbmdTaGFwZU1hc2s7CisgICAgaWYg KGFjdGlvbigpID09IEFycmF5OjpXcml0ZSkKKyAgICAgICAgaW5kZXhpbmdNb2RlTWFzayB8PSBD b3B5T25Xcml0ZTsKKwogICAgIHN3aXRjaCAoYXJyYXlDbGFzcygpKSB7Ci0gICAgY2FzZSBBcnJh eTo6T3JpZ2luYWxBcnJheTogewotICAgICAgICBpZiAodmFsdWUubV9zdHJ1Y3R1cmUuaXNUb3Ao KSkKKyAgICBjYXNlIEFycmF5OjpBcnJheTogeworICAgICAgICBpZiAoYXJyYXlNb2Rlc0FscmVh ZHlDaGVja2VkKHZhbHVlLm1fYXJyYXlNb2RlcywgYXNBcnJheU1vZGVzSWdub3JpbmdUeXBlZEFy cmF5cyhzaGFwZSB8IElzQXJyYXkpKSkKKyAgICAgICAgICAgIHJldHVybiB0cnVlOworICAgICAg ICBpZiAoIXZhbHVlLm1fc3RydWN0dXJlLmlzRmluaXRlKCkpCiAgICAgICAgICAgICByZXR1cm4g ZmFsc2U7CiAgICAgICAgIGZvciAodW5zaWduZWQgaSA9IHZhbHVlLm1fc3RydWN0dXJlLnNpemUo KTsgaS0tOykgewogICAgICAgICAgICAgUmVnaXN0ZXJlZFN0cnVjdHVyZSBzdHJ1Y3R1cmUgPSB2 YWx1ZS5tX3N0cnVjdHVyZVtpXTsKLSAgICAgICAgICAgIGlmICgoc3RydWN0dXJlLT5pbmRleGlu Z1R5cGUoKSAmIEluZGV4aW5nU2hhcGVNYXNrKSAhPSBzaGFwZSkKLSAgICAgICAgICAgICAgICBy ZXR1cm4gZmFsc2U7Ci0gICAgICAgICAgICBpZiAoaXNDb3B5T25Xcml0ZShzdHJ1Y3R1cmUtPmlu ZGV4aW5nTW9kZSgpKSAmJiBhY3Rpb24oKSA9PSBBcnJheTo6V3JpdGUpCi0gICAgICAgICAgICAg ICAgcmV0dXJuIGZhbHNlOwotICAgICAgICAgICAgaWYgKCEoc3RydWN0dXJlLT5pbmRleGluZ1R5 cGUoKSAmIElzQXJyYXkpKQotICAgICAgICAgICAgICAgIHJldHVybiBmYWxzZTsKLSAgICAgICAg ICAgIGlmICghZ3JhcGguZ2xvYmFsT2JqZWN0Rm9yKG5vZGUtPm9yaWdpbi5zZW1hbnRpYyktPmlz T3JpZ2luYWxBcnJheVN0cnVjdHVyZShzdHJ1Y3R1cmUuZ2V0KCkpKQorICAgICAgICAgICAgaWYg KChzdHJ1Y3R1cmUtPmluZGV4aW5nTW9kZSgpICYgaW5kZXhpbmdNb2RlTWFzaykgIT0gKHNoYXBl IHwgSXNBcnJheSkpCiAgICAgICAgICAgICAgICAgcmV0dXJuIGZhbHNlOwogICAgICAgICB9CiAg ICAgICAgIHJldHVybiB0cnVlOwogICAgIH0KLSAgICAgICAgCi0gICAgY2FzZSBBcnJheTo6QXJy YXk6IHsKLSAgICAgICAgaWYgKGFycmF5TW9kZXNBbHJlYWR5Q2hlY2tlZCh2YWx1ZS5tX2FycmF5 TW9kZXMsIGFzQXJyYXlNb2Rlc0lnbm9yaW5nVHlwZWRBcnJheXMoc2hhcGUgfCBJc0FycmF5KSkp CisKKyAgICAvLyBBcnJheTo6T3JpZ2luYWxOb25BcnJheSBjYW4gYmUgc2hvd24gd2hlbiB0aGUg dmFsdWUgaXMgYSBUeXBlZEFycmF5IHdpdGggb3JpZ2luYWwgc3RydWN0dXJlLgorICAgIC8vIEJ1 dCBoZXJlLCB3ZSBhbHJlYWR5IGZpbHRlcmVkIFR5cGVkQXJyYXlzLiBTbywganVzdCBoYW5kbGlu ZyBpdCBsaWtlIE5vbkFycmF5LgorICAgIGNhc2UgQXJyYXk6Ok9yaWdpbmFsTm9uQXJyYXk6Cisg ICAgY2FzZSBBcnJheTo6Tm9uQXJyYXk6IHsKKyAgICAgICAgaWYgKGFycmF5TW9kZXNBbHJlYWR5 Q2hlY2tlZCh2YWx1ZS5tX2FycmF5TW9kZXMsIGFzQXJyYXlNb2Rlc0lnbm9yaW5nVHlwZWRBcnJh eXMoc2hhcGUpKSkKICAgICAgICAgICAgIHJldHVybiB0cnVlOwotICAgICAgICBpZiAodmFsdWUu bV9zdHJ1Y3R1cmUuaXNUb3AoKSkKKyAgICAgICAgaWYgKCF2YWx1ZS5tX3N0cnVjdHVyZS5pc0Zp bml0ZSgpKQogICAgICAgICAgICAgcmV0dXJuIGZhbHNlOwogICAgICAgICBmb3IgKHVuc2lnbmVk IGkgPSB2YWx1ZS5tX3N0cnVjdHVyZS5zaXplKCk7IGktLTspIHsKICAgICAgICAgICAgIFJlZ2lz dGVyZWRTdHJ1Y3R1cmUgc3RydWN0dXJlID0gdmFsdWUubV9zdHJ1Y3R1cmVbaV07Ci0gICAgICAg ICAgICBpZiAoKHN0cnVjdHVyZS0+aW5kZXhpbmdNb2RlKCkgJiBJbmRleGluZ1NoYXBlTWFzaykg IT0gc2hhcGUpCi0gICAgICAgICAgICAgICAgcmV0dXJuIGZhbHNlOwotICAgICAgICAgICAgaWYg KGlzQ29weU9uV3JpdGUoc3RydWN0dXJlLT5pbmRleGluZ01vZGUoKSkgJiYgYWN0aW9uKCkgPT0g QXJyYXk6OldyaXRlKQotICAgICAgICAgICAgICAgIHJldHVybiBmYWxzZTsKLSAgICAgICAgICAg IGlmICghKHN0cnVjdHVyZS0+aW5kZXhpbmdUeXBlKCkgJiBJc0FycmF5KSkKKyAgICAgICAgICAg IGlmICgoc3RydWN0dXJlLT5pbmRleGluZ01vZGUoKSAmIGluZGV4aW5nTW9kZU1hc2spICE9IHNo YXBlKQogICAgICAgICAgICAgICAgIHJldHVybiBmYWxzZTsKICAgICAgICAgfQogICAgICAgICBy ZXR1cm4gdHJ1ZTsKICAgICB9Ci0gICAgICAgIAotICAgIGRlZmF1bHQ6IHsKKworICAgIGNhc2Ug QXJyYXk6OlBvc3NpYmx5QXJyYXk6IHsKICAgICAgICAgaWYgKGFycmF5TW9kZXNBbHJlYWR5Q2hl Y2tlZCh2YWx1ZS5tX2FycmF5TW9kZXMsIGFzQXJyYXlNb2Rlc0lnbm9yaW5nVHlwZWRBcnJheXMo c2hhcGUpIHwgYXNBcnJheU1vZGVzSWdub3JpbmdUeXBlZEFycmF5cyhzaGFwZSB8IElzQXJyYXkp KSkKICAgICAgICAgICAgIHJldHVybiB0cnVlOwotICAgICAgICBpZiAodmFsdWUubV9zdHJ1Y3R1 cmUuaXNUb3AoKSkKKyAgICAgICAgaWYgKCF2YWx1ZS5tX3N0cnVjdHVyZS5pc0Zpbml0ZSgpKQog ICAgICAgICAgICAgcmV0dXJuIGZhbHNlOwogICAgICAgICBmb3IgKHVuc2lnbmVkIGkgPSB2YWx1 ZS5tX3N0cnVjdHVyZS5zaXplKCk7IGktLTspIHsKICAgICAgICAgICAgIFJlZ2lzdGVyZWRTdHJ1 Y3R1cmUgc3RydWN0dXJlID0gdmFsdWUubV9zdHJ1Y3R1cmVbaV07Ci0gICAgICAgICAgICBpZiAo KHN0cnVjdHVyZS0+aW5kZXhpbmdNb2RlKCkgJiBJbmRleGluZ1NoYXBlTWFzaykgIT0gc2hhcGUp CisgICAgICAgICAgICBpZiAoKHN0cnVjdHVyZS0+aW5kZXhpbmdNb2RlKCkgJiAoaW5kZXhpbmdN b2RlTWFzayAmIH5Jc0FycmF5KSkgIT0gc2hhcGUpCiAgICAgICAgICAgICAgICAgcmV0dXJuIGZh bHNlOwotICAgICAgICAgICAgaWYgKGlzQ29weU9uV3JpdGUoc3RydWN0dXJlLT5pbmRleGluZ01v ZGUoKSkgJiYgYWN0aW9uKCkgPT0gQXJyYXk6OldyaXRlKQorICAgICAgICB9CisgICAgICAgIHJl dHVybiB0cnVlOworICAgIH0KKworICAgIC8vIElmIEFycmF5TW9kZSBpcyBBcnJheTo6T3JpZ2lu YWxDb3B5T25Xcml0ZUFycmF5IG9yIEFycmF5OjpPcmlnaW5hbEFycmF5LCBDaGVja0FycmF5IGlz IG5ldmVyIGVtaXR0ZWQuIEluc3RlYWQsIHdlIGFsd2F5cyBlbWl0IENoZWNrU3RydWN0dXJlLgor ICAgIC8vIFNvLCB3ZSBzaG91bGQgcGVyZm9ybSB0aGUgc2FtZSBjaGVjayB0byB0aGUgQ2hlY2tT dHJ1Y3R1cmUgaGVyZS4KKyAgICBjYXNlIEFycmF5OjpPcmlnaW5hbEFycmF5OgorICAgIGNhc2Ug QXJyYXk6Ok9yaWdpbmFsQ29weU9uV3JpdGVBcnJheTogeworICAgICAgICBpZiAoIXZhbHVlLm1f c3RydWN0dXJlLmlzRmluaXRlKCkpCisgICAgICAgICAgICByZXR1cm4gZmFsc2U7CisgICAgICAg IFN0cnVjdHVyZSogb3JpZ2luYWxTdHJ1Y3R1cmUgPSBvcmlnaW5hbEFycmF5U3RydWN0dXJlKGdy YXBoLCBub2RlKTsKKyAgICAgICAgZm9yICh1bnNpZ25lZCBpID0gdmFsdWUubV9zdHJ1Y3R1cmUu c2l6ZSgpOyBpLS07KSB7CisgICAgICAgICAgICBSZWdpc3RlcmVkU3RydWN0dXJlIHN0cnVjdHVy ZSA9IHZhbHVlLm1fc3RydWN0dXJlW2ldOworICAgICAgICAgICAgaWYgKHN0cnVjdHVyZS5nZXQo KSAhPSBvcmlnaW5hbFN0cnVjdHVyZSkKICAgICAgICAgICAgICAgICByZXR1cm4gZmFsc2U7CiAg ICAgICAgIH0KICAgICAgICAgcmV0dXJuIHRydWU7Ci0gICAgfSB9CisgICAgfQorICAgIH0KIH0K IAogYm9vbCBBcnJheU1vZGU6OmFscmVhZHlDaGVja2VkKEdyYXBoJiBncmFwaCwgTm9kZSogbm9k ZSwgY29uc3QgQWJzdHJhY3RWYWx1ZSYgdmFsdWUpIGNvbnN0CmRpZmYgLS1naXQgYS9KU1Rlc3Rz L0NoYW5nZUxvZyBiL0pTVGVzdHMvQ2hhbmdlTG9nCmluZGV4IDRhNTA0OTIzZmQxMDFhNTAwYmM4 MzRhMjI0Y2E0ODE4ZTIxZjQ0OTIuLmMwMDM0NWNjYTdmYzA3ZGI0NzM1ZTNjZGFhYmU1Nzg2ZDQ0 NTFjMDQgMTAwNjQ0Ci0tLSBhL0pTVGVzdHMvQ2hhbmdlTG9nCisrKyBiL0pTVGVzdHMvQ2hhbmdl TG9nCkBAIC0xLDMgKzEsMTQgQEAKKzIwMTktMDktMTYgIFl1c3VrZSBTdXp1a2kgIDx5c3V6dWtp QGFwcGxlLmNvbT4KKworICAgICAgICBbSlNDXSBDaGVja0FycmF5K05vbkFycmF5IGlzIG5vdCBm aWx0ZXJpbmcgb3V0IEFycmF5IGluIEFJCisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3Jn L3Nob3dfYnVnLmNnaT9pZD0yMDE4NTcKKyAgICAgICAgPHJkYXI6Ly9wcm9ibGVtLzU0MTk0ODIw PgorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAgICogc3Ry ZXNzL2NoZWNrLWFycmF5LXdpdGgtbm9uLWFycmF5LWRvZXMtbm90LWZpbHRlci1hcnJheXMuanM6 IEFkZGVkLgorICAgICAgICAoZm9vKToKKwogMjAxOS0wOS0xNiAgTWljaGFlbCBTYWJvZmYgIDxt c2Fib2ZmQGFwcGxlLmNvbT4KIAogICAgICAgICBbSlNDXSBQZXJmb3JtIGNoZWNrIGFnYWluIHdo ZW4gd2UgZm91bmQgbm9uLUJNUCBjaGFyYWN0ZXJzCmRpZmYgLS1naXQgYS9KU1Rlc3RzL3N0cmVz cy9jaGVjay1hcnJheS13aXRoLW5vbi1hcnJheS1kb2VzLW5vdC1maWx0ZXItYXJyYXlzLmpzIGIv SlNUZXN0cy9zdHJlc3MvY2hlY2stYXJyYXktd2l0aC1ub24tYXJyYXktZG9lcy1ub3QtZmlsdGVy LWFycmF5cy5qcwpuZXcgZmlsZSBtb2RlIDEwMDY0NAppbmRleCAwMDAwMDAwMDAwMDAwMDAwMDAw MDAwMDAwMDAwMDAwMDAwMDAwMDAwLi5iNjMwOTkzYTRiMzY3MjU0ZWUyNGZlMDZiYTlkNWNhYWFi ZTFhNWFkCi0tLSAvZGV2L251bGwKKysrIGIvSlNUZXN0cy9zdHJlc3MvY2hlY2stYXJyYXktd2l0 aC1ub24tYXJyYXktZG9lcy1ub3QtZmlsdGVyLWFycmF5cy5qcwpAQCAtMCwwICsxLDE2IEBACisv L0AgcnVuRGVmYXVsdCgiLS10aHJlc2hvbGRGb3JGVExPcHRpbWl6ZUFmdGVyV2FybVVwPTAiLCAi LS11c2VDb25jdXJyZW50SklUPTAiKQorCitmdW5jdGlvbiBmb28oLi4uYSkgeworICAgIGZvciAo Y29uc3QgdyBvZiBhKSB7CisgICAgICAgIGZvciAoY29uc3QgcSBvZiBhcmd1bWVudHMpIHsKKyAg ICAgICAgfQorICAgIH0KK30KKworZm9vKDAsIDEsIDIsIDMsIDQsIDUsIDYsIDcsIDgsIDksIDEw LCAxMSwgMTIsIDEzKTsKKworZm9yIChsZXQgaSA9IDA7IGkgPCAxMDAwOyBpKyspIHsKKyAgICBm b28oKTsKK30KKworZm9vKDApOwo=