212001 2020-05-17 13:25:12 -0700 OSR loop entry to iterator_next generic needs to CheckNotEmpty on m_next 2020-05-18 11:59:08 -0700 1 1 1 Unclassified WebKit New Bugs WebKit Nightly Build Unspecified Unspecified RESOLVED FIXED InRadar P2 Normal --- 1 keith_miller keith_miller ews-watchlist mark.lam msaboff saam tzagallo webkit-bug-importer oldest_to_newest 1653438 0 keith_miller 2020-05-17 13:25:12 -0700 OSR loop entry to iterator_next generic needs to CheckNotEmpty on m_next 1653439 1 399603 keith_miller 2020-05-17 13:26:16 -0700 Created attachment 399603 Patch 1653440 2 keith_miller 2020-05-17 13:26:39 -0700 rdar://problem/62844069 1653442 3 399605 keith_miller 2020-05-17 13:57:52 -0700 Created attachment 399605 Patch 1653666 4 399605 saam 2020-05-18 10:49:39 -0700 Comment on attachment 399605 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=399605&action=review > Source/JavaScriptCore/ChangeLog:15 > + If we happen to OSR enter into iterator_next during a for-of loop > + that has only profiled a generic iterator but is actually running > + a fast iterator we will incorrectly exit forward to the next > + getter checkpoint. This is because we don't check the next > + function is non empty when we only emit a generic iterator_next > + bytecode. The fix for this is to simply put a CheckNotEmpty at the > + top of the generic case. 99.9% of the time this check will be > + eliminated so it doesn't really cost anything. the explanation you gave me in Slack is much clearer than this. I'd try to explain it like that in here. > Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp:7000 > + // Sanity check that we don't get here with a fast path during OSR entry. I think this deserves a proper explanation. "Sanity check", to me, implies this should never happen. But it can happen, so let's describe how, and just say the below code can't handle the empty value. 1653695 5 keith_miller 2020-05-18 11:56:54 -0700 Committed r261824: <https://trac.webkit.org/changeset/261824> 1653698 6 399605 keith_miller 2020-05-18 11:59:08 -0700 Comment on attachment 399605 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=399605&action=review >> Source/JavaScriptCore/ChangeLog:15 >> + eliminated so it doesn't really cost anything. > > the explanation you gave me in Slack is much clearer than this. I'd try to explain it like that in here. Done. >> Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp:7000 >> + // Sanity check that we don't get here with a fast path during OSR entry. > > I think this deserves a proper explanation. "Sanity check", to me, implies this should never happen. But it can happen, so let's describe how, and just say the below code can't handle the empty value. Ok, I added the description of how we could fail this check and that Call can't handle it without the check. 399603 2020-05-17 13:26:16 -0700 2020-05-17 13:57:50 -0700 Patch bug-212001-20200517132615.patch text/plain 2892 keith_miller U3VidmVyc2lvbiBSZXZpc2lvbjogMjYxNzgzCmRpZmYgLS1naXQgYS9Tb3VyY2UvSmF2YVNjcmlw dENvcmUvQ2hhbmdlTG9nIGIvU291cmNlL0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZwppbmRleCAx OGNlYzVhMTQ1ZmM1ZTIwMjZlMWYyN2RlOGRkNzk3NTAxN2RiNDIxLi5kNTVlNWQzY2M5NjFmOWQx MzhmMjY3ODg1YTRkM2MxMDQ1NmU1MjIxIDEwMDY0NAotLS0gYS9Tb3VyY2UvSmF2YVNjcmlwdENv cmUvQ2hhbmdlTG9nCisrKyBiL1NvdXJjZS9KYXZhU2NyaXB0Q29yZS9DaGFuZ2VMb2cKQEAgLTEs MyArMSwxMyBAQAorMjAyMC0wNS0xNyAgS2VpdGggTWlsbGVyICA8a2VpdGhfbWlsbGVyQGFwcGxl LmNvbT4KKworICAgICAgICBPU1IgbG9vcCBlbnRyeSB0byBpdGVyYXRvcl9uZXh0IGdlbmVyaWMg bmVlZHMgdG8gQ2hlY2tOb3RFbXB0eSBvbiBtX25leHQKKyAgICAgICAgaHR0cHM6Ly9idWdzLndl YmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTIxMjAwMQorCisgICAgICAgIFJldmlld2VkIGJ5IE5P Qk9EWSAoT09QUyEpLgorCisgICAgICAgICogZGZnL0RGR0J5dGVDb2RlUGFyc2VyLmNwcDoKKyAg ICAgICAgKEpTQzo6REZHOjpCeXRlQ29kZVBhcnNlcjo6cGFyc2VCbG9jayk6CisKIDIwMjAtMDUt MTYgIFl1c3VrZSBTdXp1a2kgIDx5c3V6dWtpQGFwcGxlLmNvbT4KIAogICAgICAgICBbSlNDXSBN YWtlIE91dE9mTWVtb3J5IGVycm9yIGFzIGluc3RhbmNlIG9mIFJhbmdlRXJyb3IKZGlmZiAtLWdp dCBhL1NvdXJjZS9KYXZhU2NyaXB0Q29yZS9kZmcvREZHQnl0ZUNvZGVQYXJzZXIuY3BwIGIvU291 cmNlL0phdmFTY3JpcHRDb3JlL2RmZy9ERkdCeXRlQ29kZVBhcnNlci5jcHAKaW5kZXggYTY5NDZh YTIyZTBkZWYyOGM1ZTIxNDNkZmU2MmQ3ODZhZjc3Yzg4MS4uZDQ2Mjc1NGNlZGI1ZDM1MjA3MzY2 NTA4ODMzOWI0YWRiODNlMjY3ZSAxMDA2NDQKLS0tIGEvU291cmNlL0phdmFTY3JpcHRDb3JlL2Rm Zy9ERkdCeXRlQ29kZVBhcnNlci5jcHAKKysrIGIvU291cmNlL0phdmFTY3JpcHRDb3JlL2RmZy9E RkdCeXRlQ29kZVBhcnNlci5jcHAKQEAgLTY5OTcsNiArNjk5Nyw5IEBAIHZvaWQgQnl0ZUNvZGVQ YXJzZXI6OnBhcnNlQmxvY2sodW5zaWduZWQgbGltaXQpCiAgICAgICAgICAgICAgICAgfSBlbHNl CiAgICAgICAgICAgICAgICAgICAgIEFTU0VSVCghZ2VuZXJhdGVkQ2FzZSk7CiAKKyAgICAgICAg ICAgICAgICAvLyBTYW5pdHkgY2hlY2sgdGhhdCB3ZSBkb24ndCBnZXQgaGVyZSB3aXRoIGEgZmFz dCBwYXRoIGR1cmluZyBPU1IgZW50cnkuCisgICAgICAgICAgICAgICAgYWRkVG9HcmFwaChDaGVj a05vdEVtcHR5LCBnZXQoYnl0ZWNvZGUubV9uZXh0KSk7CisKICAgICAgICAgICAgICAgICBUZXJt aW5hbGl0eSB0ZXJtaW5hbGl0eSA9IGhhbmRsZUNhbGw8T3BJdGVyYXRvck5leHQ+KGN1cnJlbnRJ bnN0cnVjdGlvbiwgQ2FsbCwgQ2FsbE1vZGU6OlJlZ3VsYXIsIG5leHRDaGVja3BvaW50KCkpOwog ICAgICAgICAgICAgICAgIEFTU0VSVF9VTlVTRUQodGVybWluYWxpdHksIHRlcm1pbmFsaXR5ID09 IE5vblRlcm1pbmFsKTsKICAgICAgICAgICAgICAgICBwcm9ncmVzc1RvTmV4dENoZWNrcG9pbnQo KTsKZGlmZiAtLWdpdCBhL0pTVGVzdHMvQ2hhbmdlTG9nIGIvSlNUZXN0cy9DaGFuZ2VMb2cKaW5k ZXggY2M2ZTRmZjlmNjNhNzhkYmVhNDQwMTAwYzQ4NTk2NWUwMTAwYjFhMC4uN2UyMWY2NDIyOWM0 OGQ3NzA3NWFlY2EyNjlhZDhlYzJmMjIzMzcwMCAxMDA2NDQKLS0tIGEvSlNUZXN0cy9DaGFuZ2VM b2cKKysrIGIvSlNUZXN0cy9DaGFuZ2VMb2cKQEAgLTEsMyArMSwxMyBAQAorMjAyMC0wNS0xNyAg S2VpdGggTWlsbGVyICA8a2VpdGhfbWlsbGVyQGFwcGxlLmNvbT4KKworICAgICAgICBPU1IgbG9v cCBlbnRyeSB0byBpdGVyYXRvcl9uZXh0IGdlbmVyaWMgbmVlZHMgdG8gQ2hlY2tOb3RFbXB0eSBv biBtX25leHQKKyAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lk PTIxMjAwMQorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAg ICogc3RyZXNzL2Zvci1vZi1vc3ItbG9vcC1lbnRlci1hY3RpdmUtbmV4dC1kaWZmZXJzLWZyb20t c2Vlbi1tb2Rlcy5qczogQWRkZWQuCisgICAgICAgIChmb28pOgorCiAyMDIwLTA1LTE2ICBZdXN1 a2UgU3V6dWtpICA8eXN1enVraUBhcHBsZS5jb20+CiAKICAgICAgICAgW0pTQ10gTWFrZSBPdXRP Zk1lbW9yeSBlcnJvciBhcyBpbnN0YW5jZSBvZiBSYW5nZUVycm9yCmRpZmYgLS1naXQgYS9KU1Rl c3RzL3N0cmVzcy9mb3Itb2Ytb3NyLWxvb3AtZW50ZXItYWN0aXZlLW5leHQtZGlmZmVycy1mcm9t LXNlZW4tbW9kZXMuanMgYi9KU1Rlc3RzL3N0cmVzcy9mb3Itb2Ytb3NyLWxvb3AtZW50ZXItYWN0 aXZlLW5leHQtZGlmZmVycy1mcm9tLXNlZW4tbW9kZXMuanMKbmV3IGZpbGUgbW9kZSAxMDA2NDQK aW5kZXggMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMC4uMzc0NThmNjA1 NzEzYzBkNjYwYWQ0YmVkMTgzNTJlNTg5ZjQ1NDFjNAotLS0gL2Rldi9udWxsCisrKyBiL0pTVGVz dHMvc3RyZXNzL2Zvci1vZi1vc3ItbG9vcC1lbnRlci1hY3RpdmUtbmV4dC1kaWZmZXJzLWZyb20t c2Vlbi1tb2Rlcy5qcwpAQCAtMCwwICsxLDcgQEAKKy8vQCByZXF1aXJlT3B0aW9ucygiLS1qaXRQ b2xpY3lTY2FsZT0wIiwgIi0tZm9yY2VEZWJ1Z2dlckJ5dGVjb2RlR2VuZXJhdGlvbj0xIiwgIi0t dXNlQ29uY3VycmVudEpJVD0wIiwgIi0tbWluaW11bU9wdGltaXphdGlvbkRlbGF5PTYiKQorZnVu Y3Rpb24gZm9vKCkgeworICBPYmplY3QuZnJvbUVudHJpZXMoYXJndW1lbnRzKTsKKyAgT2JqZWN0 LmZyb21FbnRyaWVzKFtdKTsKK30KKworbmV3IFByb21pc2UoZm9vKTsK 399605 2020-05-17 13:57:52 -0700 2020-05-18 10:49:39 -0700 Patch bug-212001-20200517135751.patch text/plain 3458 keith_miller U3VidmVyc2lvbiBSZXZpc2lvbjogMjYxNzgzCmRpZmYgLS1naXQgYS9Tb3VyY2UvSmF2YVNjcmlw dENvcmUvQ2hhbmdlTG9nIGIvU291cmNlL0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZwppbmRleCAx OGNlYzVhMTQ1ZmM1ZTIwMjZlMWYyN2RlOGRkNzk3NTAxN2RiNDIxLi45ZGUwZDRkYmQ3OGRlNDBk MzM5NmRmNDdiMjM3Y2MxZGI3Y2MwZjg0IDEwMDY0NAotLS0gYS9Tb3VyY2UvSmF2YVNjcmlwdENv cmUvQ2hhbmdlTG9nCisrKyBiL1NvdXJjZS9KYXZhU2NyaXB0Q29yZS9DaGFuZ2VMb2cKQEAgLTEs MyArMSwyMiBAQAorMjAyMC0wNS0xNyAgS2VpdGggTWlsbGVyICA8a2VpdGhfbWlsbGVyQGFwcGxl LmNvbT4KKworICAgICAgICBPU1IgbG9vcCBlbnRyeSB0byBpdGVyYXRvcl9uZXh0IGdlbmVyaWMg bmVlZHMgdG8gQ2hlY2tOb3RFbXB0eSBvbiBtX25leHQKKyAgICAgICAgaHR0cHM6Ly9idWdzLndl YmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTIxMjAwMQorCisgICAgICAgIFJldmlld2VkIGJ5IE5P Qk9EWSAoT09QUyEpLgorCisgICAgICAgIElmIHdlIGhhcHBlbiB0byBPU1IgZW50ZXIgaW50byBp dGVyYXRvcl9uZXh0IGR1cmluZyBhIGZvci1vZiBsb29wCisgICAgICAgIHRoYXQgaGFzIG9ubHkg cHJvZmlsZWQgYSBnZW5lcmljIGl0ZXJhdG9yIGJ1dCBpcyBhY3R1YWxseSBydW5uaW5nCisgICAg ICAgIGEgZmFzdCBpdGVyYXRvciB3ZSB3aWxsIGluY29ycmVjdGx5IGV4aXQgZm9yd2FyZCB0byB0 aGUgbmV4dAorICAgICAgICBnZXR0ZXIgY2hlY2twb2ludC4gVGhpcyBpcyBiZWNhdXNlIHdlIGRv bid0IGNoZWNrIHRoZSBuZXh0CisgICAgICAgIGZ1bmN0aW9uIGlzIG5vbiBlbXB0eSB3aGVuIHdl IG9ubHkgZW1pdCBhIGdlbmVyaWMgaXRlcmF0b3JfbmV4dAorICAgICAgICBieXRlY29kZS4gVGhl IGZpeCBmb3IgdGhpcyBpcyB0byBzaW1wbHkgcHV0IGEgQ2hlY2tOb3RFbXB0eSBhdCB0aGUKKyAg ICAgICAgdG9wIG9mIHRoZSBnZW5lcmljIGNhc2UuIDk5LjklIG9mIHRoZSB0aW1lIHRoaXMgY2hl Y2sgd2lsbCBiZQorICAgICAgICBlbGltaW5hdGVkIHNvIGl0IGRvZXNuJ3QgcmVhbGx5IGNvc3Qg YW55dGhpbmcuCisKKyAgICAgICAgKiBkZmcvREZHQnl0ZUNvZGVQYXJzZXIuY3BwOgorICAgICAg ICAoSlNDOjpERkc6OkJ5dGVDb2RlUGFyc2VyOjpwYXJzZUJsb2NrKToKKwogMjAyMC0wNS0xNiAg WXVzdWtlIFN1enVraSAgPHlzdXp1a2lAYXBwbGUuY29tPgogCiAgICAgICAgIFtKU0NdIE1ha2Ug T3V0T2ZNZW1vcnkgZXJyb3IgYXMgaW5zdGFuY2Ugb2YgUmFuZ2VFcnJvcgpkaWZmIC0tZ2l0IGEv U291cmNlL0phdmFTY3JpcHRDb3JlL2RmZy9ERkdCeXRlQ29kZVBhcnNlci5jcHAgYi9Tb3VyY2Uv SmF2YVNjcmlwdENvcmUvZGZnL0RGR0J5dGVDb2RlUGFyc2VyLmNwcAppbmRleCBhNjk0NmFhMjJl MGRlZjI4YzVlMjE0M2RmZTYyZDc4NmFmNzdjODgxLi5kNDYyNzU0Y2VkYjVkMzUyMDczNjY1MDg4 MzM5YjRhZGI4M2UyNjdlIDEwMDY0NAotLS0gYS9Tb3VyY2UvSmF2YVNjcmlwdENvcmUvZGZnL0RG R0J5dGVDb2RlUGFyc2VyLmNwcAorKysgYi9Tb3VyY2UvSmF2YVNjcmlwdENvcmUvZGZnL0RGR0J5 dGVDb2RlUGFyc2VyLmNwcApAQCAtNjk5Nyw2ICs2OTk3LDkgQEAgdm9pZCBCeXRlQ29kZVBhcnNl cjo6cGFyc2VCbG9jayh1bnNpZ25lZCBsaW1pdCkKICAgICAgICAgICAgICAgICB9IGVsc2UKICAg ICAgICAgICAgICAgICAgICAgQVNTRVJUKCFnZW5lcmF0ZWRDYXNlKTsKIAorICAgICAgICAgICAg ICAgIC8vIFNhbml0eSBjaGVjayB0aGF0IHdlIGRvbid0IGdldCBoZXJlIHdpdGggYSBmYXN0IHBh dGggZHVyaW5nIE9TUiBlbnRyeS4KKyAgICAgICAgICAgICAgICBhZGRUb0dyYXBoKENoZWNrTm90 RW1wdHksIGdldChieXRlY29kZS5tX25leHQpKTsKKwogICAgICAgICAgICAgICAgIFRlcm1pbmFs aXR5IHRlcm1pbmFsaXR5ID0gaGFuZGxlQ2FsbDxPcEl0ZXJhdG9yTmV4dD4oY3VycmVudEluc3Ry dWN0aW9uLCBDYWxsLCBDYWxsTW9kZTo6UmVndWxhciwgbmV4dENoZWNrcG9pbnQoKSk7CiAgICAg ICAgICAgICAgICAgQVNTRVJUX1VOVVNFRCh0ZXJtaW5hbGl0eSwgdGVybWluYWxpdHkgPT0gTm9u VGVybWluYWwpOwogICAgICAgICAgICAgICAgIHByb2dyZXNzVG9OZXh0Q2hlY2twb2ludCgpOwpk aWZmIC0tZ2l0IGEvSlNUZXN0cy9DaGFuZ2VMb2cgYi9KU1Rlc3RzL0NoYW5nZUxvZwppbmRleCBj YzZlNGZmOWY2M2E3OGRiZWE0NDAxMDBjNDg1OTY1ZTAxMDBiMWEwLi43ZTIxZjY0MjI5YzQ4ZDc3 MDc1YWVjYTI2OWFkOGVjMmYyMjMzNzAwIDEwMDY0NAotLS0gYS9KU1Rlc3RzL0NoYW5nZUxvZwor KysgYi9KU1Rlc3RzL0NoYW5nZUxvZwpAQCAtMSwzICsxLDEzIEBACisyMDIwLTA1LTE3ICBLZWl0 aCBNaWxsZXIgIDxrZWl0aF9taWxsZXJAYXBwbGUuY29tPgorCisgICAgICAgIE9TUiBsb29wIGVu dHJ5IHRvIGl0ZXJhdG9yX25leHQgZ2VuZXJpYyBuZWVkcyB0byBDaGVja05vdEVtcHR5IG9uIG1f bmV4dAorICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9MjEy MDAxCisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAgICAgKiBz dHJlc3MvZm9yLW9mLW9zci1sb29wLWVudGVyLWFjdGl2ZS1uZXh0LWRpZmZlcnMtZnJvbS1zZWVu LW1vZGVzLmpzOiBBZGRlZC4KKyAgICAgICAgKGZvbyk6CisKIDIwMjAtMDUtMTYgIFl1c3VrZSBT dXp1a2kgIDx5c3V6dWtpQGFwcGxlLmNvbT4KIAogICAgICAgICBbSlNDXSBNYWtlIE91dE9mTWVt b3J5IGVycm9yIGFzIGluc3RhbmNlIG9mIFJhbmdlRXJyb3IKZGlmZiAtLWdpdCBhL0pTVGVzdHMv c3RyZXNzL2Zvci1vZi1vc3ItbG9vcC1lbnRlci1hY3RpdmUtbmV4dC1kaWZmZXJzLWZyb20tc2Vl bi1tb2Rlcy5qcyBiL0pTVGVzdHMvc3RyZXNzL2Zvci1vZi1vc3ItbG9vcC1lbnRlci1hY3RpdmUt bmV4dC1kaWZmZXJzLWZyb20tc2Vlbi1tb2Rlcy5qcwpuZXcgZmlsZSBtb2RlIDEwMDY0NAppbmRl eCAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwLi4zNzQ1OGY2MDU3MTNj MGQ2NjBhZDRiZWQxODM1MmU1ODlmNDU0MWM0Ci0tLSAvZGV2L251bGwKKysrIGIvSlNUZXN0cy9z dHJlc3MvZm9yLW9mLW9zci1sb29wLWVudGVyLWFjdGl2ZS1uZXh0LWRpZmZlcnMtZnJvbS1zZWVu LW1vZGVzLmpzCkBAIC0wLDAgKzEsNyBAQAorLy9AIHJlcXVpcmVPcHRpb25zKCItLWppdFBvbGlj eVNjYWxlPTAiLCAiLS1mb3JjZURlYnVnZ2VyQnl0ZWNvZGVHZW5lcmF0aW9uPTEiLCAiLS11c2VD b25jdXJyZW50SklUPTAiLCAiLS1taW5pbXVtT3B0aW1pemF0aW9uRGVsYXk9NiIpCitmdW5jdGlv biBmb28oKSB7CisgIE9iamVjdC5mcm9tRW50cmllcyhhcmd1bWVudHMpOworICBPYmplY3QuZnJv bUVudHJpZXMoW10pOworfQorCituZXcgUHJvbWlzZShmb28pOwo=