159603 2016-07-09 11:51:23 -0700 REGRESSION(201900): validation failure for GetByOffset/PutByOffset in VALIDATE((node), node->child1().node() == node->child2().node() || node->child1()->result() == NodeResultStorage) 2016-07-09 14:08:31 -0700 1 1 1 Unclassified WebKit JavaScriptCore WebKit Nightly Build All All RESOLVED FIXED https://bugs.webkit.org/show_bug.cgi?id=159602 P2 Normal --- 1 fpizlo fpizlo barraclough benjamin commit-queue ggaren keith_miller mark.lam mhahnenb msaboff oliver saam sam oldest_to_newest 1209459 0 fpizlo 2016-07-09 11:51:23 -0700 This validation rule is broken. It should always be valid to take: Foo(@x, @x) and turn it into: a: ValueRep(@x) b: ValueRep(@x) Foo(@a, @b) or: y: Identity(@x) Foo(@x, @y) That's because it should be possible to rewire any data flow edge something that produces an equivalent value. This validation rule means that such rewirings are invalid on GetByOffset/PutByOffset. 1209460 1 fpizlo 2016-07-09 11:59:40 -0700 This is what the IR actually looks like when we crash: 2641:< 1:-> ValueRep(Check:DoubleRep:@858<Double>, JS|PureInt, Bytecodedouble, bc#9) 889:<!0:-> CheckStructure(Check:Cell:@2641, MustGen, [%Ea:Object], R:JSCell_structureID, Exits, bc#9) 2642:< 1:-> ValueRep(Check:DoubleRep:@858<Double>, JS|PureInt, Bytecodedouble, bc#9) 2643:< 1:-> ValueRep(Check:DoubleRep:@858<Double>, JS|PureInt, Bytecodedouble, bc#9) 890:< 1:-> GetByOffset(Check:KnownCell:@2642, Check:KnownCell:@2643, JS|PureInt|UseAsInt, Nonboolint32, id24{Ca}, 0, inferredType = Int32, R:NamedProperties(24), Exits, bc#9) predicting Nonboolint32 We fail validation at the GetByOffset. But the GetByOffset is dead anyway, and even if it wasn't, it would be OK to ignore the first child of the GetByOffset when doing analysis even if it was different from the second child. Both children produce the same value! 1209461 2 283263 fpizlo 2016-07-09 12:06:15 -0700 Created attachment 283263 the patch 1209465 3 283263 keith_miller 2016-07-09 12:24:48 -0700 Comment on attachment 283263 the patch View in context: https://bugs.webkit.org/attachment.cgi?id=283263&action=review > Source/JavaScriptCore/ChangeLog:9 > + This removes an incorrect validation rule and replaces it with a FIXME about how to make this > + aspect of IR easier to validate soundly. You should say why the validation rule is not valid. It's not really clear what this patch is actually fixing without that information. 1209471 4 fpizlo 2016-07-09 13:33:32 -0700 (In reply to comment #3) > Comment on attachment 283263 [details] > the patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=283263&action=review > > > Source/JavaScriptCore/ChangeLog:9 > > + This removes an incorrect validation rule and replaces it with a FIXME about how to make this > > + aspect of IR easier to validate soundly. > > You should say why the validation rule is not valid. It's not really clear > what this patch is actually fixing without that information. I'm assuming you just want me to copy-paste the bug description into the ChangeLog. 1209472 5 keith_miller 2016-07-09 13:35:26 -0700 (In reply to comment #4) > (In reply to comment #3) > > Comment on attachment 283263 [details] > > the patch > > > > View in context: > > https://bugs.webkit.org/attachment.cgi?id=283263&action=review > > > > > Source/JavaScriptCore/ChangeLog:9 > > > + This removes an incorrect validation rule and replaces it with a FIXME about how to make this > > > + aspect of IR easier to validate soundly. > > > > You should say why the validation rule is not valid. It's not really clear > > what this patch is actually fixing without that information. > > I'm assuming you just want me to copy-paste the bug description into the > ChangeLog. Yeah, that would be fine. 1209473 6 283268 fpizlo 2016-07-09 13:36:05 -0700 Created attachment 283268 the patch 1209474 7 283263 keith_miller 2016-07-09 13:37:00 -0700 Comment on attachment 283263 the patch View in context: https://bugs.webkit.org/attachment.cgi?id=283263&action=review r=me if you add the description. >>>> Source/JavaScriptCore/ChangeLog:9 >>>> + aspect of IR easier to validate soundly. >>> >>> You should say why the validation rule is not valid. It's not really clear what this patch is actually fixing without that information. >> >> I'm assuming you just want me to copy-paste the bug description into the ChangeLog. > > Yeah, that would be fine. I actually missed that there was a description. If you do that I think the patch is g2g. 1209475 8 283268 keith_miller 2016-07-09 13:37:22 -0700 Comment on attachment 283268 the patch r=me 1209478 9 283268 commit-queue 2016-07-09 14:08:26 -0700 Comment on attachment 283268 the patch Clearing flags on attachment: 283268 Committed r203034: <http://trac.webkit.org/changeset/203034> 1209479 10 commit-queue 2016-07-09 14:08:31 -0700 All reviewed patches have been landed. Closing bug. 283263 2016-07-09 12:06:15 -0700 2016-07-09 13:37:00 -0700 the patch blah.patch text/plain 2795 fpizlo SW5kZXg6IFNvdXJjZS9KYXZhU2NyaXB0Q29yZS9DaGFuZ2VMb2cKPT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0gU291 cmNlL0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZwkocmV2aXNpb24gMjAzMDMyKQorKysgU291cmNl L0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZwkod29ya2luZyBjb3B5KQpAQCAtMSwzICsxLDE2IEBA CisyMDE2LTA3LTA5ICBGaWxpcCBQaXpsbyAgPGZwaXpsb0BhcHBsZS5jb20+CisKKyAgICAgICAg UkVHUkVTU0lPTigyMDE5MDApOiB2YWxpZGF0aW9uIGZhaWx1cmUgZm9yIEdldEJ5T2Zmc2V0L1B1 dEJ5T2Zmc2V0IGluIFZBTElEQVRFKChub2RlKSwgbm9kZS0+Y2hpbGQxKCkubm9kZSgpID09IG5v ZGUtPmNoaWxkMigpLm5vZGUoKSB8fCBub2RlLT5jaGlsZDEoKS0+cmVzdWx0KCkgPT0gTm9kZVJl c3VsdFN0b3JhZ2UpCisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNn aT9pZD0xNTk2MDMKKworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKyAgICAg ICAgCisgICAgICAgIFRoaXMgcmVtb3ZlcyBhbiBpbmNvcnJlY3QgdmFsaWRhdGlvbiBydWxlIGFu ZCByZXBsYWNlcyBpdCB3aXRoIGEgRklYTUUgYWJvdXQgaG93IHRvIG1ha2UgdGhpcworICAgICAg ICBhc3BlY3Qgb2YgSVIgZWFzaWVyIHRvIHZhbGlkYXRlIHNvdW5kbHkuCisKKyAgICAgICAgKiBk ZmcvREZHVmFsaWRhdGUuY3BwOgorICAgICAgICAqIHRlc3RzL3N0cmVzcy9nZXQtYnktb2Zmc2V0 LWRvdWJsZS5qczogQWRkZWQuCisKIDIwMTYtMDctMDkgIEJlbmphbWluIFBvdWxhaW4gIDxicG91 bGFpbkBhcHBsZS5jb20+CiAKICAgICAgICAgW0pTQ10gRml4IHRoZSBUZW1wbGF0ZSBSYXcgVmFs dWUgb2YgXCAoZXNjYXBlKSArIExpbmVUZXJtaW5hdG9yU2VxdWVuY2UKSW5kZXg6IFNvdXJjZS9K YXZhU2NyaXB0Q29yZS9kZmcvREZHVmFsaWRhdGUuY3BwCj09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIFNvdXJjZS9K YXZhU2NyaXB0Q29yZS9kZmcvREZHVmFsaWRhdGUuY3BwCShyZXZpc2lvbiAyMDI4MzMpCisrKyBT b3VyY2UvSmF2YVNjcmlwdENvcmUvZGZnL0RGR1ZhbGlkYXRlLmNwcAkod29ya2luZyBjb3B5KQpA QCAtMjk5LDcgKzI5OSwxMiBAQCBwdWJsaWM6CiAgICAgICAgICAgICAgICAgICAgIGJyZWFrOwog ICAgICAgICAgICAgICAgIGNhc2UgR2V0QnlPZmZzZXQ6CiAgICAgICAgICAgICAgICAgY2FzZSBQ dXRCeU9mZnNldDoKLSAgICAgICAgICAgICAgICAgICAgVkFMSURBVEUoKG5vZGUpLCBub2RlLT5j aGlsZDEoKS5ub2RlKCkgPT0gbm9kZS0+Y2hpbGQyKCkubm9kZSgpIHx8IG5vZGUtPmNoaWxkMSgp LT5yZXN1bHQoKSA9PSBOb2RlUmVzdWx0U3RvcmFnZSk7CisgICAgICAgICAgICAgICAgICAgIC8v IEZJWE1FOiBXZSBzaG91bGQgYmUgYWJsZSB0byB2YWxpZGF0ZSB0aGF0IEdldEJ5T2Zmc2V0IGFu ZCBQdXRCeU9mZnNldCBhcmUKKyAgICAgICAgICAgICAgICAgICAgLy8gdXNpbmcgdGhlIHNhbWUg b2JqZWN0IGZvciBzdG9yYWdlIGFuZCBiYXNlLiBJIHRoaW5rIHRoaXMgbWVhbnMgZmluYWxseQor ICAgICAgICAgICAgICAgICAgICAvLyBzcGxpdHRpbmcgdGhlc2Ugbm9kZXMgaW50byB0d28gbm9k ZSB0eXBlcywgb25lIGZvciBpbmxpbmUgYW5kIG9uZSBmb3IKKyAgICAgICAgICAgICAgICAgICAg Ly8gb3V0LW9mLWxpbmUuIFRoZSBvdXQtb2YtbGluZSBvbmUgd2lsbCByZXF1aXJlIHRoYXQgdGhl IGZpcnN0IG5vZGUgaXMgc3RvcmFnZSwKKyAgICAgICAgICAgICAgICAgICAgLy8gd2hpbGUgdGhl IGlubGluZSBvbmUgd2lsbCBub3QgdGFrZSBhIHN0b3JhZ2UgY2hpbGQgYXQgYWxsLgorICAgICAg ICAgICAgICAgICAgICAvLyBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9 MTU5NjAyCiAgICAgICAgICAgICAgICAgICAgIGJyZWFrOwogICAgICAgICAgICAgICAgIGRlZmF1 bHQ6CiAgICAgICAgICAgICAgICAgICAgIGJyZWFrOwpJbmRleDogU291cmNlL0phdmFTY3JpcHRD b3JlL3Rlc3RzL3N0cmVzcy9nZXQtYnktb2Zmc2V0LWRvdWJsZS5qcwo9PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBT b3VyY2UvSmF2YVNjcmlwdENvcmUvdGVzdHMvc3RyZXNzL2dldC1ieS1vZmZzZXQtZG91YmxlLmpz CShyZXZpc2lvbiAwKQorKysgU291cmNlL0phdmFTY3JpcHRDb3JlL3Rlc3RzL3N0cmVzcy9nZXQt Ynktb2Zmc2V0LWRvdWJsZS5qcwkod29ya2luZyBjb3B5KQpAQCAtMCwwICsxLDI1IEBACitmdW5j dGlvbiBmb28obywgcCkgeworICAgIGlmIChwKQorICAgICAgICByZXR1cm4gby5mOworICAgIGVs c2UKKyAgICAgICAgcmV0dXJuIFtvICogMS4xLCBvICogMS4yLCBvICogMS4zXTsKK30KKworZm9y ICh2YXIgaSA9IDA7IGkgPCAxMDA7ICsraSkKKyAgICBmb28oe2Y6NDJ9LCB0cnVlKTsKKworZnVu Y3Rpb24gYmFyKCkgeworICAgIHZhciB4ID0gNC41OworICAgIGZvciAodmFyIGkgPSAwOyBpIDwg MTA7ICsraSkgeworICAgICAgICB4ICo9IDEuMTsKKyAgICAgICAgeCArPSAwLjA1OworICAgICAg ICBmb28oeCwgZmFsc2UpOworICAgIH0KKyAgICByZXR1cm4geCAqIDEuMDM7Cit9CisKK25vSW5s aW5lKGJhcik7CisKK2ZvciAodmFyIGkgPSAwOyBpIDwgMTAwMDA7ICsraSkKKyAgICBiYXIoKTsK Kwo= 283268 2016-07-09 13:36:05 -0700 2016-07-09 14:08:26 -0700 the patch blah.patch text/plain 3493 fpizlo SW5kZXg6IFNvdXJjZS9KYXZhU2NyaXB0Q29yZS9DaGFuZ2VMb2cKPT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0gU291 cmNlL0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZwkocmV2aXNpb24gMjAzMDMyKQorKysgU291cmNl L0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZwkod29ya2luZyBjb3B5KQpAQCAtMSwzICsxLDM4IEBA CisyMDE2LTA3LTA5ICBGaWxpcCBQaXpsbyAgPGZwaXpsb0BhcHBsZS5jb20+CisKKyAgICAgICAg UkVHUkVTU0lPTigyMDE5MDApOiB2YWxpZGF0aW9uIGZhaWx1cmUgZm9yIEdldEJ5T2Zmc2V0L1B1 dEJ5T2Zmc2V0IGluIFZBTElEQVRFKChub2RlKSwgbm9kZS0+Y2hpbGQxKCkubm9kZSgpID09IG5v ZGUtPmNoaWxkMigpLm5vZGUoKSB8fCBub2RlLT5jaGlsZDEoKS0+cmVzdWx0KCkgPT0gTm9kZVJl c3VsdFN0b3JhZ2UpCisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNn aT9pZD0xNTk2MDMKKworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKyAgICAg ICAgCisgICAgICAgIFRoaXMgcmVtb3ZlcyBhbiBpbmNvcnJlY3QgdmFsaWRhdGlvbiBydWxlIGFu ZCByZXBsYWNlcyBpdCB3aXRoIGEgRklYTUUgYWJvdXQgaG93IHRvIG1ha2UgdGhpcworICAgICAg ICBhc3BlY3Qgb2YgSVIgZWFzaWVyIHRvIHZhbGlkYXRlIHNvdW5kbHkuCisgICAgICAgIAorICAg ICAgICBJdCdzIG5vdCB2YWxpZCB0byBhc3NlcnQgdGhhdCB0d28gY2hpbGRyZW4gb2YgYSBub2Rl IGFyZSB0aGUgc2FtZS4gSXQgc2hvdWxkIGFsd2F5cyBiZSB2YWxpZAorICAgICAgICB0byB0YWtl OgorICAgICAgICAKKyAgICAgICAgRm9vKEB4LCBAeCkKKyAgICAgICAgCisgICAgICAgIGFuZCB0 dXJuIGl0IGludG86CisgICAgICAgIAorICAgICAgICBhOiBWYWx1ZVJlcChAeCkKKyAgICAgICAg YjogVmFsdWVSZXAoQHgpCisgICAgICAgIEZvbyhAYSwgQGIpCisgICAgICAgIAorICAgICAgICBv ciBldmVuIHNvbWV0aGluZyBsaWtlOgorICAgICAgICAKKyAgICAgICAgeTogSWRlbnRpdHkoQHkp CisgICAgICAgIEZvbyhAeCwgQHkpCisgICAgICAgIAorICAgICAgICBUaGF0J3MgYmVjYXVzZSBp dCBzaG91bGQgYmUgcG9zc2libGUgdG8gcmV3aXJlIGFueSBkYXRhIGZsb3cgZWRnZSBzb21ldGhp bmcgdGhhdCBwcm9kdWNlcyBhbgorICAgICAgICBlcXVpdmFsZW50IHZhbHVlLgorICAgICAgICAK KyAgICAgICAgVGhlIHZhbGlkYXRpb24gcnVsZSB0aGF0IHRoaXMgcGF0Y2ggcmVtb3ZlcyBtZWFu dCB0aGF0IHN1Y2ggcmV3aXJpbmdzIHdlcmUgaW52YWxpZCBvbgorICAgICAgICBHZXRCeU9mZnNl dC9QdXRCeU9mZnNldC4gRml4dXBQaGFzZSBkaWQgc3VjaCBhIHJld2lyaW5nIHNvbWV0aW1lcy4K KworICAgICAgICAqIGRmZy9ERkdWYWxpZGF0ZS5jcHA6CisgICAgICAgICogdGVzdHMvc3RyZXNz L2dldC1ieS1vZmZzZXQtZG91YmxlLmpzOiBBZGRlZC4KKwogMjAxNi0wNy0wOSAgQmVuamFtaW4g UG91bGFpbiAgPGJwb3VsYWluQGFwcGxlLmNvbT4KIAogICAgICAgICBbSlNDXSBGaXggdGhlIFRl bXBsYXRlIFJhdyBWYWx1ZSBvZiBcIChlc2NhcGUpICsgTGluZVRlcm1pbmF0b3JTZXF1ZW5jZQpJ bmRleDogU291cmNlL0phdmFTY3JpcHRDb3JlL2RmZy9ERkdWYWxpZGF0ZS5jcHAKPT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PQotLS0gU291cmNlL0phdmFTY3JpcHRDb3JlL2RmZy9ERkdWYWxpZGF0ZS5jcHAJKHJldmlzaW9u IDIwMjgzMykKKysrIFNvdXJjZS9KYXZhU2NyaXB0Q29yZS9kZmcvREZHVmFsaWRhdGUuY3BwCSh3 b3JraW5nIGNvcHkpCkBAIC0yOTksNyArMjk5LDEyIEBAIHB1YmxpYzoKICAgICAgICAgICAgICAg ICAgICAgYnJlYWs7CiAgICAgICAgICAgICAgICAgY2FzZSBHZXRCeU9mZnNldDoKICAgICAgICAg ICAgICAgICBjYXNlIFB1dEJ5T2Zmc2V0OgotICAgICAgICAgICAgICAgICAgICBWQUxJREFURSgo bm9kZSksIG5vZGUtPmNoaWxkMSgpLm5vZGUoKSA9PSBub2RlLT5jaGlsZDIoKS5ub2RlKCkgfHwg bm9kZS0+Y2hpbGQxKCktPnJlc3VsdCgpID09IE5vZGVSZXN1bHRTdG9yYWdlKTsKKyAgICAgICAg ICAgICAgICAgICAgLy8gRklYTUU6IFdlIHNob3VsZCBiZSBhYmxlIHRvIHZhbGlkYXRlIHRoYXQg R2V0QnlPZmZzZXQgYW5kIFB1dEJ5T2Zmc2V0IGFyZQorICAgICAgICAgICAgICAgICAgICAvLyB1 c2luZyB0aGUgc2FtZSBvYmplY3QgZm9yIHN0b3JhZ2UgYW5kIGJhc2UuIEkgdGhpbmsgdGhpcyBt ZWFucyBmaW5hbGx5CisgICAgICAgICAgICAgICAgICAgIC8vIHNwbGl0dGluZyB0aGVzZSBub2Rl cyBpbnRvIHR3byBub2RlIHR5cGVzLCBvbmUgZm9yIGlubGluZSBhbmQgb25lIGZvcgorICAgICAg ICAgICAgICAgICAgICAvLyBvdXQtb2YtbGluZS4gVGhlIG91dC1vZi1saW5lIG9uZSB3aWxsIHJl cXVpcmUgdGhhdCB0aGUgZmlyc3Qgbm9kZSBpcyBzdG9yYWdlLAorICAgICAgICAgICAgICAgICAg ICAvLyB3aGlsZSB0aGUgaW5saW5lIG9uZSB3aWxsIG5vdCB0YWtlIGEgc3RvcmFnZSBjaGlsZCBh dCBhbGwuCisgICAgICAgICAgICAgICAgICAgIC8vIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3No b3dfYnVnLmNnaT9pZD0xNTk2MDIKICAgICAgICAgICAgICAgICAgICAgYnJlYWs7CiAgICAgICAg ICAgICAgICAgZGVmYXVsdDoKICAgICAgICAgICAgICAgICAgICAgYnJlYWs7CkluZGV4OiBTb3Vy Y2UvSmF2YVNjcmlwdENvcmUvdGVzdHMvc3RyZXNzL2dldC1ieS1vZmZzZXQtZG91YmxlLmpzCj09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT0KLS0tIFNvdXJjZS9KYXZhU2NyaXB0Q29yZS90ZXN0cy9zdHJlc3MvZ2V0LWJ5LW9m ZnNldC1kb3VibGUuanMJKHJldmlzaW9uIDApCisrKyBTb3VyY2UvSmF2YVNjcmlwdENvcmUvdGVz dHMvc3RyZXNzL2dldC1ieS1vZmZzZXQtZG91YmxlLmpzCSh3b3JraW5nIGNvcHkpCkBAIC0wLDAg KzEsMjUgQEAKK2Z1bmN0aW9uIGZvbyhvLCBwKSB7CisgICAgaWYgKHApCisgICAgICAgIHJldHVy biBvLmY7CisgICAgZWxzZQorICAgICAgICByZXR1cm4gW28gKiAxLjEsIG8gKiAxLjIsIG8gKiAx LjNdOworfQorCitmb3IgKHZhciBpID0gMDsgaSA8IDEwMDsgKytpKQorICAgIGZvbyh7Zjo0Mn0s IHRydWUpOworCitmdW5jdGlvbiBiYXIoKSB7CisgICAgdmFyIHggPSA0LjU7CisgICAgZm9yICh2 YXIgaSA9IDA7IGkgPCAxMDsgKytpKSB7CisgICAgICAgIHggKj0gMS4xOworICAgICAgICB4ICs9 IDAuMDU7CisgICAgICAgIGZvbyh4LCBmYWxzZSk7CisgICAgfQorICAgIHJldHVybiB4ICogMS4w MzsKK30KKworbm9JbmxpbmUoYmFyKTsKKworZm9yICh2YXIgaSA9IDA7IGkgPCAxMDAwMDsgKytp KQorICAgIGJhcigpOworCg==