165483 2016-12-06 12:21:14 -0800 REGRESSION(r209399): Causes crashes when dumping JIT disassembly 2016-12-07 13:15:57 -0800 1 1 1 Unclassified WebKit JavaScriptCore Other Unspecified Unspecified RESOLVED FIXED P2 Normal --- 1 msaboff msaboff benjamin cdumez cmarcelo commit-queue darin dbates ryanhaddad oldest_to_newest 1256899 0 msaboff 2016-12-06 12:21:14 -0800 Looks like the RELEASE_ASSERT() at the end of sixCharacterHashStringToInteger() has an inverted check. We want to assert that the string is terminated with a null character. 1256901 1 296306 msaboff 2016-12-06 12:25:18 -0800 Created attachment 296306 Patch 1256903 2 296306 ggaren 2016-12-06 12:27:06 -0800 Comment on attachment 296306 Patch r=me 1256920 3 296306 darin 2016-12-06 12:58:45 -0800 Comment on attachment 296306 Patch Oops! Thanks very much for fixing this. 1256921 4 darin 2016-12-06 12:59:10 -0800 Wonder why EWS missed it. 1256922 5 msaboff 2016-12-06 13:01:23 -0800 (In reply to comment #4) > Wonder why EWS missed it. I don't think the debug tests exercise this path. That would require something to dump the hashed name of a compiled JS function. 1256923 6 296306 commit-queue 2016-12-06 13:04:10 -0800 Comment on attachment 296306 Patch Clearing flags on attachment: 296306 Committed r209413: <http://trac.webkit.org/changeset/209413> 1256924 7 commit-queue 2016-12-06 13:04:15 -0800 All reviewed patches have been landed. Closing bug. 1256958 8 ryanhaddad 2016-12-06 14:08:41 -0800 *** Bug 165490 has been marked as a duplicate of this bug. *** 1256959 9 msaboff 2016-12-06 14:11:25 -0800 (In reply to comment #5) > (In reply to comment #4) > > Wonder why EWS missed it. > > I don't think the debug tests exercise this path. That would require > something to dump the hashed name of a compiled JS function. I was wrong, the debug bots did catch this (https://bugs.webkit.org/show_bug.cgi?id=165490). It just took a little time. 1257122 10 darin 2016-12-06 18:51:00 -0800 Are "the debug bots" part of EWS, or part of some other automated WebKit testing? 1257123 11 darin 2016-12-06 18:51:40 -0800 Also, Michael, do you know why these need to be RELEASE_ASSERT and not just ASSERT? 1257261 12 msaboff 2016-12-07 10:05:25 -0800 (In reply to comment #10) > Are "the debug bots" part of EWS, or part of some other automated WebKit > testing? There is a mac-debug EWS bot. I don't know if this bot would / did catch the issue. It would depend on what tests that bot runs. > Also, Michael, do you know why these need to be RELEASE_ASSERT and not just ASSERT? I don't know why this isn't simply an ASSERT. 1257333 13 ryanhaddad 2016-12-07 13:15:57 -0800 (In reply to comment #12) > (In reply to comment #10) > > Are "the debug bots" part of EWS, or part of some other automated WebKit > > testing? > > There is a mac-debug EWS bot. I don't know if this bot would / did catch > the issue. It would depend on what tests that bot runs. The failure was caught after the patch was landed by the bots that run JSC tests. EWS does not currently run JSC tests. 296306 2016-12-06 12:25:18 -0800 2016-12-06 13:04:10 -0800 Patch 165483.patch text/plain 1250 msaboff SW5kZXg6IFNvdXJjZS9XVEYvQ2hhbmdlTG9nCj09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIFNvdXJjZS9XVEYvQ2hh bmdlTG9nCShyZXZpc2lvbiAyMDk0MTEpCisrKyBTb3VyY2UvV1RGL0NoYW5nZUxvZwkod29ya2lu ZyBjb3B5KQpAQCAtMSwzICsxLDE2IEBACisyMDE2LTEyLTA2ICBNaWNoYWVsIFNhYm9mZiAgPG1z YWJvZmZAYXBwbGUuY29tPgorCisgICAgICAgIFJFR1JFU1NJT04ocjIwOTM5OSk6IENhdXNlcyBj cmFzaGVzIHdoZW4gZHVtcGluZyBKSVQgZGlzYXNzZW1ibHkKKyAgICAgICAgaHR0cHM6Ly9idWdz LndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTE2NTQ4MworCisgICAgICAgIFJldmlld2VkIGJ5 IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAgIEZpeGVkIHRoZSBSRUxFQVNFX0FTU0VSVCgpIHRv IGNoZWNrIHRoYXQgdGhlIDYgY2hhcmFjdGVyIHN0cmluZyBpcyB0ZXJtaW5hdGVkIGJ5IGEgbnVs bAorICAgICAgICBjaGFyYWN0ZXIuCisKKyAgICAgICAgKiB3dGYvU2l4Q2hhcmFjdGVySGFzaC5j cHA6CisgICAgICAgIChXVEY6OnNpeENoYXJhY3Rlckhhc2hTdHJpbmdUb0ludGVnZXIpOgorCiAy MDE2LTEyLTA0ICBEYXJpbiBBZGxlciAgPGRhcmluQGFwcGxlLmNvbT4KIAogICAgICAgICBVc2Ug QVNDSUlDVHlwZSBtb3JlLCBhbmQgaW1wcm92ZSBpdCBhIGxpdHRsZSBiaXQKSW5kZXg6IFNvdXJj ZS9XVEYvd3RmL1NpeENoYXJhY3Rlckhhc2guY3BwCj09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIFNvdXJjZS9XVEYv d3RmL1NpeENoYXJhY3Rlckhhc2guY3BwCShyZXZpc2lvbiAyMDk0MTEpCisrKyBTb3VyY2UvV1RG L3d0Zi9TaXhDaGFyYWN0ZXJIYXNoLmNwcAkod29ya2luZyBjb3B5KQpAQCAtNTAsNyArNTAsNyBA QCB1bnNpZ25lZCBzaXhDaGFyYWN0ZXJIYXNoU3RyaW5nVG9JbnRlZ2VyCiAgICAgICAgIGhhc2gg Kz0gYyAtICcwJyArIDI2ICogMjsKICAgICB9CiAKLSAgICBSRUxFQVNFX0FTU0VSVChzdHJpbmdb Nl0pOyAvLyBGSVhNRTogV2h5IGRvZXMgdGhpcyBuZWVkIHRvIGJlIGEgUkVMRUFTRV9BU1NFUlQ/ CisgICAgUkVMRUFTRV9BU1NFUlQoIXN0cmluZ1s2XSk7IC8vIEZJWE1FOiBXaHkgZG9lcyB0aGlz IG5lZWQgdG8gYmUgYSBSRUxFQVNFX0FTU0VSVD8KIAogICAgIHJldHVybiBoYXNoOwogfQo=