157968 2016-05-20 21:30:37 -0700 REGRESSION(r199075): String.prototype.replace fails after being used many times with different replace values 2016-05-22 12:12:25 -0700 1 1 1 Unclassified WebKit JavaScriptCore WebKit Nightly Build Unspecified Unspecified RESOLVED FIXED InRadar P2 Normal --- 1 joepeck saam ap benjamin commit-queue fpizlo ggaren joepeck keith_miller mark.lam msaboff rniwa saam webkit-bug-importer oldest_to_newest 1195329 0 joepeck 2016-05-20 21:30:37 -0700 * SUMMARY String.prototype.replace fails used many times with different replace values. * TEST <script> (function() { var result = "foo".replace(/f/g, ""); if (result != "oo") throw "Error: bad result: "+ result; for (var i = 0; i < 100000; ++i) result = "foo".replace(/f/g, 42); if (result != "42oo") throw "Error: bad result: "+ result; })(); </script> * STEPS TO REPRODUCE 1. Run test => Unexpected Exception, test should not have an exception => The Error is: "Error: bad result: oo" 1195330 1 webkit-bug-importer 2016-05-20 21:31:04 -0700 <rdar://problem/26404735> 1195332 2 joepeck 2016-05-20 21:42:20 -0700 Bisection results: <http://trac.webkit.org/log/trunk/?rev=199084&stop_rev=199073> Most likely culprit is: DFG and FTL should constant-fold RegExpExec, RegExpTest, and StringReplace <​https://bugs.webkit.org/show_bug.cgi?id=155270> <http://trac.webkit.org/changeset/199075> 1195340 3 joepeck 2016-05-20 22:20:05 -0700 Based on the order of tests run on the bots "js/regress/string-replace-generic.html" can fail if "js/regress/string-replace-empty.html" runs before it. So, in the mean time I am marking "js/regress/string-replace-generic.html" as [ Pass Fail ] in LayoutTests/TestExpectations. Please remove that entry when fixing! 1195343 4 joepeck 2016-05-20 22:22:08 -0700 > Please remove that entry when fixing! Test marked flakey in: <http://trac.webkit.org/changeset/201240> 1195345 5 saam 2016-05-20 22:25:40 -0700 Sounds related to Fil's constant folding patch from a month ago. 1195346 6 saam 2016-05-20 23:57:42 -0700 (In reply to comment #5) > Sounds related to Fil's constant folding patch from a month ago. Oops. Looks like you already bisected to that patch 1195347 7 279539 saam 2016-05-21 00:33:37 -0700 Created attachment 279539 patch 1195348 8 saam 2016-05-21 00:34:54 -0700 Fil, we currently won't constant fold the test case now. It's because 42 is a number and not a string. Should we extend LazyJSValue::tryGetStringImpl to handle numbers too? This seems like something we really should constant fold. 1195352 9 279539 joepeck 2016-05-21 02:22:39 -0700 Comment on attachment 279539 patch View in context: https://bugs.webkit.org/attachment.cgi?id=279539&action=review > LayoutTests/TestExpectations:-984 > -webkit.org/b/157968 js/regress/string-replace-generic.html [ Pass Failure ] We should add a new test for this. We got lucky that a bot ran 2 tests in sequence and triggered this. It seems worthwhile to have a single test that covers this on its own. 1195363 10 saam 2016-05-21 10:42:14 -0700 (In reply to comment #9) > Comment on attachment 279539 [details] > patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=279539&action=review > > > LayoutTests/TestExpectations:-984 > > -webkit.org/b/157968 js/regress/string-replace-generic.html [ Pass Failure ] > > We should add a new test for this. We got lucky that a bot ran 2 tests in > sequence and triggered this. It seems worthwhile to have a single test that > covers this on its own. I'll add this to JSC's tests. It reproduces every time when run in JSC without concurrent JIT 1195365 11 279539 rniwa 2016-05-21 11:38:26 -0700 Comment on attachment 279539 patch View in context: https://bugs.webkit.org/attachment.cgi?id=279539&action=review > Source/JavaScriptCore/ChangeLog:10 > + There was a bug in the DFG where we were checking a condition > + on the wrong variable. LOL, that won't do any good! 1195366 12 rniwa 2016-05-21 11:39:03 -0700 Please add a test as Joe pointed out. 1195367 13 279539 fpizlo 2016-05-21 11:40:26 -0700 Comment on attachment 279539 patch Wow!!! R=me too 1195376 14 ap 2016-05-21 13:26:40 -0700 > Based on the order of tests run on the bots "js/regress/string-replace-generic.html" can fail if "js/regress/string-replace-empty.html" runs before it. This should never happen. Tests in one directory are supposed to run sequentially, and alphabetically. Do you have a link to a test run where this happened? 1195431 15 279551 saam 2016-05-22 11:42:29 -0700 Created attachment 279551 patch for landing 1195432 16 279551 commit-queue 2016-05-22 12:12:18 -0700 Comment on attachment 279551 patch for landing Clearing flags on attachment: 279551 Committed r201254: <http://trac.webkit.org/changeset/201254> 1195433 17 commit-queue 2016-05-22 12:12:25 -0700 All reviewed patches have been landed. Closing bug. 279539 2016-05-21 00:33:37 -0700 2016-05-22 11:42:29 -0700 patch a-backup.diff text/plain 2536 saam SW5kZXg6IFNvdXJjZS9KYXZhU2NyaXB0Q29yZS9DaGFuZ2VMb2cKPT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0gU291 cmNlL0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZwkocmV2aXNpb24gMjAxMjQxKQorKysgU291cmNl L0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZwkod29ya2luZyBjb3B5KQpAQCAtMSwzICsxLDE3IEBA CisyMDE2LTA1LTIxICBTYWFtIGJhcmF0aSAgPHNiYXJhdGlAYXBwbGUuY29tPgorCisgICAgICAg IFJFR1JFU1NJT04ocjE5OTA3NSk6IFN0cmluZy5wcm90b3R5cGUucmVwbGFjZSBmYWlscyBhZnRl ciBiZWluZyB1c2VkIG1hbnkgdGltZXMgd2l0aCBkaWZmZXJlbnQgcmVwbGFjZSB2YWx1ZXMKKyAg ICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTE1Nzk2OAorICAg ICAgICA8cmRhcjovL3Byb2JsZW0vMjY0MDQ3MzU+CisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9C T0RZIChPT1BTISkuCisKKyAgICAgICAgVGhlcmUgd2FzIGEgYnVnIGluIHRoZSBERkcgd2hlcmUg d2Ugd2VyZSBjaGVja2luZyBhIGNvbmRpdGlvbgorICAgICAgICBvbiB0aGUgd3JvbmcgdmFyaWFi bGUuCisKKyAgICAgICAgKiBkZmcvREZHU3RyZW5ndGhSZWR1Y3Rpb25QaGFzZS5jcHA6CisgICAg ICAgIChKU0M6OkRGRzo6U3RyZW5ndGhSZWR1Y3Rpb25QaGFzZTo6aGFuZGxlTm9kZSk6CisKIDIw MTYtMDUtMjAgIEpvc2VwaCBQZWNvcmFybyAgPHBlY29yYXJvQGFwcGxlLmNvbT4KIAogICAgICAg ICBSZW1vdmUgTGVnYWN5UHJvZmlsZXIKSW5kZXg6IFNvdXJjZS9KYXZhU2NyaXB0Q29yZS9kZmcv REZHU3RyZW5ndGhSZWR1Y3Rpb25QaGFzZS5jcHAKPT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0gU291cmNlL0phdmFT Y3JpcHRDb3JlL2RmZy9ERkdTdHJlbmd0aFJlZHVjdGlvblBoYXNlLmNwcAkocmV2aXNpb24gMjAx MjQxKQorKysgU291cmNlL0phdmFTY3JpcHRDb3JlL2RmZy9ERkdTdHJlbmd0aFJlZHVjdGlvblBo YXNlLmNwcAkod29ya2luZyBjb3B5KQpAQCAtNjM5LDcgKzYzOSw3IEBAIHByaXZhdGU6CiAgICAg ICAgICAgICB9CiAKICAgICAgICAgICAgIFN0cmluZyByZXBsYWNlID0gbV9ub2RlLT5jaGlsZDMo KS0+dHJ5R2V0U3RyaW5nKG1fZ3JhcGgpOwotICAgICAgICAgICAgaWYgKCFzdHJpbmcpCisgICAg ICAgICAgICBpZiAoIXJlcGxhY2UpCiAgICAgICAgICAgICAgICAgYnJlYWs7CiAKICAgICAgICAg ICAgIFN0cmluZ0J1aWxkZXIgYnVpbGRlcjsKSW5kZXg6IExheW91dFRlc3RzL0NoYW5nZUxvZwo9 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09Ci0tLSBMYXlvdXRUZXN0cy9DaGFuZ2VMb2cJKHJldmlzaW9uIDIwMTI0MSkKKysr IExheW91dFRlc3RzL0NoYW5nZUxvZwkod29ya2luZyBjb3B5KQpAQCAtMSwzICsxLDEzIEBACisy MDE2LTA1LTIxICBTYWFtIGJhcmF0aSAgPHNiYXJhdGlAYXBwbGUuY29tPgorCisgICAgICAgIFJF R1JFU1NJT04ocjE5OTA3NSk6IFN0cmluZy5wcm90b3R5cGUucmVwbGFjZSBmYWlscyBhZnRlciBi ZWluZyB1c2VkIG1hbnkgdGltZXMgd2l0aCBkaWZmZXJlbnQgcmVwbGFjZSB2YWx1ZXMKKyAgICAg ICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTE1Nzk2OAorICAgICAg ICA8cmRhcjovL3Byb2JsZW0vMjY0MDQ3MzU+CisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZ IChPT1BTISkuCisKKyAgICAgICAgKiBUZXN0RXhwZWN0YXRpb25zOgorCiAyMDE2LTA1LTIwICBK b3NlcGggUGVjb3Jhcm8gIDxwZWNvcmFyb0BhcHBsZS5jb20+CiAKICAgICAgICAgVW5yZXZpZXdl ZCB0ZXN0IGdhcmRlbmluZyBhZnRlciByMjAxMjM3IGV4cG9zZWQgYW4gaXNzdWUuCkluZGV4OiBM YXlvdXRUZXN0cy9UZXN0RXhwZWN0YXRpb25zCj09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIExheW91dFRlc3RzL1Rl c3RFeHBlY3RhdGlvbnMJKHJldmlzaW9uIDIwMTI0MSkKKysrIExheW91dFRlc3RzL1Rlc3RFeHBl Y3RhdGlvbnMJKHdvcmtpbmcgY29weSkKQEAgLTk4MCw1ICs5ODAsMyBAQCB3ZWJraXQub3JnL2Iv MTU2NjMxIGZhc3QvdGV4dC9mb250LWZhY2UtCiB3ZWJraXQub3JnL2IvMTU3NTg5IGZhc3QvdGV4 dC1hdXRvc2l6aW5nL2lvcy90ZXh0LWF1dG9zaXppbmctYWZ0ZXItYmFjay5odG1sIFsgUGFzcyBU aW1lb3V0IF0KIAogd2Via2l0Lm9yZy9iLzE1NzgxNiBodHRwL3Rlc3RzL3BlcmZvcm1hbmNlL3Bl cmZvcm1hbmNlLXJlc291cmNlLXRpbWluZy1jYWNoZWQtZW50cmllcy5odG1sIFsgUGFzcyBGYWls dXJlIF0KLQotd2Via2l0Lm9yZy9iLzE1Nzk2OCBqcy9yZWdyZXNzL3N0cmluZy1yZXBsYWNlLWdl bmVyaWMuaHRtbCBbIFBhc3MgRmFpbHVyZSBdCg== 279551 2016-05-22 11:42:29 -0700 2016-05-22 12:12:18 -0700 patch for landing a-backup.diff text/plain 3228 saam SW5kZXg6IFNvdXJjZS9KYXZhU2NyaXB0Q29yZS9DaGFuZ2VMb2cKPT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0gU291 cmNlL0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZwkocmV2aXNpb24gMjAxMjQxKQorKysgU291cmNl L0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZwkod29ya2luZyBjb3B5KQpAQCAtMSwzICsxLDE3IEBA CisyMDE2LTA1LTIxICBTYWFtIGJhcmF0aSAgPHNiYXJhdGlAYXBwbGUuY29tPgorCisgICAgICAg IFJFR1JFU1NJT04ocjE5OTA3NSk6IFN0cmluZy5wcm90b3R5cGUucmVwbGFjZSBmYWlscyBhZnRl ciBiZWluZyB1c2VkIG1hbnkgdGltZXMgd2l0aCBkaWZmZXJlbnQgcmVwbGFjZSB2YWx1ZXMKKyAg ICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTE1Nzk2OAorICAg ICAgICA8cmRhcjovL3Byb2JsZW0vMjY0MDQ3MzU+CisKKyAgICAgICAgUmV2aWV3ZWQgYnkgUnlv c3VrZSBOaXdhIGFuZCBGaWxpcCBQaXpsby4KKworICAgICAgICBUaGVyZSB3YXMgYSBidWcgaW4g dGhlIERGRyB3aGVyZSB3ZSB3ZXJlIGNoZWNraW5nIGEgY29uZGl0aW9uCisgICAgICAgIG9uIHRo ZSB3cm9uZyB2YXJpYWJsZS4KKworICAgICAgICAqIGRmZy9ERkdTdHJlbmd0aFJlZHVjdGlvblBo YXNlLmNwcDoKKyAgICAgICAgKEpTQzo6REZHOjpTdHJlbmd0aFJlZHVjdGlvblBoYXNlOjpoYW5k bGVOb2RlKToKKwogMjAxNi0wNS0yMCAgSm9zZXBoIFBlY29yYXJvICA8cGVjb3Jhcm9AYXBwbGUu Y29tPgogCiAgICAgICAgIFJlbW92ZSBMZWdhY3lQcm9maWxlcgpJbmRleDogU291cmNlL0phdmFT Y3JpcHRDb3JlL2RmZy9ERkdTdHJlbmd0aFJlZHVjdGlvblBoYXNlLmNwcAo9PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0t LSBTb3VyY2UvSmF2YVNjcmlwdENvcmUvZGZnL0RGR1N0cmVuZ3RoUmVkdWN0aW9uUGhhc2UuY3Bw CShyZXZpc2lvbiAyMDEyNDEpCisrKyBTb3VyY2UvSmF2YVNjcmlwdENvcmUvZGZnL0RGR1N0cmVu Z3RoUmVkdWN0aW9uUGhhc2UuY3BwCSh3b3JraW5nIGNvcHkpCkBAIC02MzksNyArNjM5LDcgQEAg cHJpdmF0ZToKICAgICAgICAgICAgIH0KIAogICAgICAgICAgICAgU3RyaW5nIHJlcGxhY2UgPSBt X25vZGUtPmNoaWxkMygpLT50cnlHZXRTdHJpbmcobV9ncmFwaCk7Ci0gICAgICAgICAgICBpZiAo IXN0cmluZykKKyAgICAgICAgICAgIGlmICghcmVwbGFjZSkKICAgICAgICAgICAgICAgICBicmVh azsKIAogICAgICAgICAgICAgU3RyaW5nQnVpbGRlciBidWlsZGVyOwpJbmRleDogU291cmNlL0ph dmFTY3JpcHRDb3JlL3Rlc3RzL3N0cmVzcy9zdHJpbmctcmVwbGFjZS1jb25zdGFudC1mb2xkaW5n LXJlcGxhY2VyLW5vdC1zdHJpbmcuanMKPT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0gU291cmNlL0phdmFTY3JpcHRD b3JlL3Rlc3RzL3N0cmVzcy9zdHJpbmctcmVwbGFjZS1jb25zdGFudC1mb2xkaW5nLXJlcGxhY2Vy LW5vdC1zdHJpbmcuanMJKHJldmlzaW9uIDApCisrKyBTb3VyY2UvSmF2YVNjcmlwdENvcmUvdGVz dHMvc3RyZXNzL3N0cmluZy1yZXBsYWNlLWNvbnN0YW50LWZvbGRpbmctcmVwbGFjZXItbm90LXN0 cmluZy5qcwkod29ya2luZyBjb3B5KQpAQCAtMCwwICsxLDEzIEBACitmdW5jdGlvbiBmb28oKSB7 CisgICAgImZvbyIucmVwbGFjZSgvZi9nLCAiIik7CisgICAgcmV0dXJuICJmb28iLnJlcGxhY2Uo L2YvZywgNDIpOworfQorbm9JbmxpbmUoZm9vKTsKKworbGV0IHJlc3VsdDsKK2ZvciAobGV0IGkg PSAwOyBpIDwgMTAwMDA7IGkrKykgeworICAgIHJlc3VsdCA9IGZvbygpOworICAgIGlmIChyZXN1 bHQgIT09ICI0Mm9vIikKKyAgICAgICAgdGhyb3cgbmV3IEVycm9yKCJFcnJvcjogYmFkIHJlc3Vs dDogIiArIHJlc3VsdCk7Cit9CisKSW5kZXg6IExheW91dFRlc3RzL0NoYW5nZUxvZwo9PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09Ci0tLSBMYXlvdXRUZXN0cy9DaGFuZ2VMb2cJKHJldmlzaW9uIDIwMTI0MSkKKysrIExheW91 dFRlc3RzL0NoYW5nZUxvZwkod29ya2luZyBjb3B5KQpAQCAtMSwzICsxLDEzIEBACisyMDE2LTA1 LTIxICBTYWFtIGJhcmF0aSAgPHNiYXJhdGlAYXBwbGUuY29tPgorCisgICAgICAgIFJFR1JFU1NJ T04ocjE5OTA3NSk6IFN0cmluZy5wcm90b3R5cGUucmVwbGFjZSBmYWlscyBhZnRlciBiZWluZyB1 c2VkIG1hbnkgdGltZXMgd2l0aCBkaWZmZXJlbnQgcmVwbGFjZSB2YWx1ZXMKKyAgICAgICAgaHR0 cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTE1Nzk2OAorICAgICAgICA8cmRh cjovL3Byb2JsZW0vMjY0MDQ3MzU+CisKKyAgICAgICAgUmV2aWV3ZWQgYnkgUnlvc3VrZSBOaXdh IGFuZCBGaWxpcCBQaXpsby4KKworICAgICAgICAqIFRlc3RFeHBlY3RhdGlvbnM6CisKIDIwMTYt MDUtMjAgIEpvc2VwaCBQZWNvcmFybyAgPHBlY29yYXJvQGFwcGxlLmNvbT4KIAogICAgICAgICBV bnJldmlld2VkIHRlc3QgZ2FyZGVuaW5nIGFmdGVyIHIyMDEyMzcgZXhwb3NlZCBhbiBpc3N1ZS4K SW5kZXg6IExheW91dFRlc3RzL1Rlc3RFeHBlY3RhdGlvbnMKPT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0gTGF5b3V0 VGVzdHMvVGVzdEV4cGVjdGF0aW9ucwkocmV2aXNpb24gMjAxMjQxKQorKysgTGF5b3V0VGVzdHMv VGVzdEV4cGVjdGF0aW9ucwkod29ya2luZyBjb3B5KQpAQCAtOTgwLDUgKzk4MCwzIEBAIHdlYmtp dC5vcmcvYi8xNTY2MzEgZmFzdC90ZXh0L2ZvbnQtZmFjZS0KIHdlYmtpdC5vcmcvYi8xNTc1ODkg ZmFzdC90ZXh0LWF1dG9zaXppbmcvaW9zL3RleHQtYXV0b3NpemluZy1hZnRlci1iYWNrLmh0bWwg WyBQYXNzIFRpbWVvdXQgXQogCiB3ZWJraXQub3JnL2IvMTU3ODE2IGh0dHAvdGVzdHMvcGVyZm9y bWFuY2UvcGVyZm9ybWFuY2UtcmVzb3VyY2UtdGltaW5nLWNhY2hlZC1lbnRyaWVzLmh0bWwgWyBQ YXNzIEZhaWx1cmUgXQotCi13ZWJraXQub3JnL2IvMTU3OTY4IGpzL3JlZ3Jlc3Mvc3RyaW5nLXJl cGxhY2UtZ2VuZXJpYy5odG1sIFsgUGFzcyBGYWlsdXJlIF0K