129657 2014-03-03 21:18:30 -0800 AbstractMacroAssembler::CachedTempRegister should start out invalid 2014-04-24 16:46:03 -0700 1 1 1 Unclassified WebKit JavaScriptCore 528+ (Nightly build) All All RESOLVED FIXED InRadar P2 Normal --- 1 msaboff msaboff oldest_to_newest 986665 0 msaboff 2014-03-03 21:18:30 -0800 Currently AbstractMacroAssembler::CachedTempRegister() are initialized with a value of 0 and that the contents are valid. Instead they should be mark as invalid as we don't know the prior contents before the first use. 986669 1 msaboff 2014-03-03 21:27:52 -0800 <rdar://problem/16214527> 986670 2 225738 msaboff 2014-03-03 21:30:28 -0800 Created attachment 225738 Patch 986674 3 225738 mark.lam 2014-03-03 21:39:39 -0800 Comment on attachment 225738 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=225738&action=review > Source/JavaScriptCore/assembler/AbstractMacroAssembler.h:881 > + , m_validBit(0 << static_cast<unsigned>(registerID)) Why not just "m_validBit(0)”? 986675 4 msaboff 2014-03-03 21:43:49 -0800 (In reply to comment #3) > (From update of attachment 225738 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=225738&action=review > > > Source/JavaScriptCore/assembler/AbstractMacroAssembler.h:881 > > + , m_validBit(0 << static_cast<unsigned>(registerID)) > > Why not just "m_validBit(0)”? Actually, the is the wrong value to clear. Updated patch coming. 986676 5 225738 ggaren 2014-03-03 21:55:53 -0800 Comment on attachment 225738 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=225738&action=review > Source/JavaScriptCore/ChangeLog:12 > + - Changed the setting of the valid bit to 0 (not valid) in the constructor as > + we don't know the contents of the register at the entry to the block we > + are going to generate code for. Are you assuming that all JITs -- including the baseline JIT, the DFG, and the CSS selector JIT -- instantiate a new MacroAssembler at the head of each basic block? 986678 6 225741 msaboff 2014-03-03 22:01:50 -0800 Created attachment 225741 Updated patch Updated after Mark caught that I was invalidating incorrectly. (In reply to comment #5) > (From update of attachment 225738 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=225738&action=review > > > Source/JavaScriptCore/ChangeLog:12 > > + - Changed the setting of the valid bit to 0 (not valid) in the constructor as > > + we don't know the contents of the register at the entry to the block we > > + are going to generate code for. > > Are you assuming that all JITs -- including the baseline JIT, the DFG, and the CSS selector JIT -- instantiate a new MacroAssembler at the head of each basic block? Poor choice of words. I changed "block" to "code". This fix has nothing to do with basic blocks. Whenever we create a MacroAssembler, we need to start with the cached register values as invalid. In the case that triggered this issue, we were generating small stubs and used the temporary registers to materialize constants that fit in 16 bits. 986681 7 msaboff 2014-03-03 22:32:45 -0800 Committed r165038: <http://trac.webkit.org/changeset/165038> 1003878 8 darin 2014-04-24 16:46:03 -0700 Moving all JavaScriptGlue bugs to JavaScriptCore. The JavaScriptGlue framework itself is long gone. And most of the more recent bugs put in this component were put there by people who thought this was for some other aspect of “JavaScript glue” and have nothing to do with the actual original reason for the existence of this component, which was an OS-X-only framework named JavaScriptGlue. 225738 2014-03-03 21:30:28 -0800 2014-03-03 22:01:50 -0800 Patch 129657.patch text/plain 1543 msaboff SW5kZXg6IFNvdXJjZS9KYXZhU2NyaXB0Q29yZS9DaGFuZ2VMb2cKPT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0gU291 cmNlL0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZwkocmV2aXNpb24gMTY1MDM2KQorKysgU291cmNl L0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZwkod29ya2luZyBjb3B5KQpAQCAtMSwzICsxLDE2IEBA CisyMDE0LTAzLTAzICBNaWNoYWVsIFNhYm9mZiAgPG1zYWJvZmZAYXBwbGUuY29tPgorCisgICAg ICAgIEFic3RyYWN0TWFjcm9Bc3NlbWJsZXI6OkNhY2hlZFRlbXBSZWdpc3RlciBzaG91bGQgc3Rh cnQgb3V0IGludmFsaWQKKyAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcu Y2dpP2lkPTEyOTY1NworCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisg ICAgICAgICogYXNzZW1ibGVyL0Fic3RyYWN0TWFjcm9Bc3NlbWJsZXIuaDoKKyAgICAgICAgKEpT Qzo6QWJzdHJhY3RNYWNyb0Fzc2VtYmxlcjo6Q2FjaGVkVGVtcFJlZ2lzdGVyOjpDYWNoZWRUZW1w UmVnaXN0ZXIpOgorICAgICAgICAtIENoYW5nZWQgdGhlIHNldHRpbmcgb2YgdGhlIHZhbGlkIGJp dCB0byAwIChub3QgdmFsaWQpIGluIHRoZSBjb25zdHJ1Y3RvciBhcworICAgICAgICAgIHdlIGRv bid0IGtub3cgdGhlIGNvbnRlbnRzIG9mIHRoZSByZWdpc3RlciBhdCB0aGUgZW50cnkgdG8gdGhl IGJsb2NrIHdlCisgICAgICAgICAgYXJlIGdvaW5nIHRvIGdlbmVyYXRlIGNvZGUgZm9yLgorCiAy MDE0LTAzLTAzICBBbmRyZWFzIEtsaW5nICA8YWtsaW5nQGFwcGxlLmNvbT4KIAogICAgICAgICBT dHJ1Y3R1cmVPck9mZnNldCBzaG91bGQgYmUgZmFzdG1hbGxvY2VkLgpJbmRleDogU291cmNlL0ph dmFTY3JpcHRDb3JlL2Fzc2VtYmxlci9BYnN0cmFjdE1hY3JvQXNzZW1ibGVyLmgKPT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PQotLS0gU291cmNlL0phdmFTY3JpcHRDb3JlL2Fzc2VtYmxlci9BYnN0cmFjdE1hY3JvQXNzZW1i bGVyLmgJKHJldmlzaW9uIDE2NTAyMSkKKysrIFNvdXJjZS9KYXZhU2NyaXB0Q29yZS9hc3NlbWJs ZXIvQWJzdHJhY3RNYWNyb0Fzc2VtYmxlci5oCSh3b3JraW5nIGNvcHkpCkBAIC04NzgsNyArODc4 LDcgQEAgcHJvdGVjdGVkOgogICAgICAgICAgICAgOiBtX21hc20obWFzbSkKICAgICAgICAgICAg ICwgbV9yZWdpc3RlcklEKHJlZ2lzdGVySUQpCiAgICAgICAgICAgICAsIG1fdmFsdWUoMCkKLSAg ICAgICAgICAgICwgbV92YWxpZEJpdCgxIDw8IHN0YXRpY19jYXN0PHVuc2lnbmVkPihyZWdpc3Rl cklEKSkKKyAgICAgICAgICAgICwgbV92YWxpZEJpdCgwIDw8IHN0YXRpY19jYXN0PHVuc2lnbmVk PihyZWdpc3RlcklEKSkKICAgICAgICAgewogICAgICAgICAgICAgQVNTRVJUKHN0YXRpY19jYXN0 PHVuc2lnbmVkPihyZWdpc3RlcklEKSA8IChzaXplb2YodW5zaWduZWQpICogOCkpOwogICAgICAg ICB9Cg== 225741 2014-03-03 22:01:50 -0800 2014-03-03 22:23:02 -0800 Updated patch 129657-2.patch text/plain 1329 msaboff SW5kZXg6IFNvdXJjZS9KYXZhU2NyaXB0Q29yZS9DaGFuZ2VMb2cKPT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0gU291 cmNlL0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZwkocmV2aXNpb24gMTY1MDM2KQorKysgU291cmNl L0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZwkod29ya2luZyBjb3B5KQpAQCAtMSwzICsxLDE2IEBA CisyMDE0LTAzLTAzICBNaWNoYWVsIFNhYm9mZiAgPG1zYWJvZmZAYXBwbGUuY29tPgorCisgICAg ICAgIEFic3RyYWN0TWFjcm9Bc3NlbWJsZXI6OkNhY2hlZFRlbXBSZWdpc3RlciBzaG91bGQgc3Rh cnQgb3V0IGludmFsaWQKKyAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcu Y2dpP2lkPTEyOTY1NworCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisg ICAgICAgICogYXNzZW1ibGVyL0Fic3RyYWN0TWFjcm9Bc3NlbWJsZXIuaDoKKyAgICAgICAgKEpT Qzo6QWJzdHJhY3RNYWNyb0Fzc2VtYmxlcjo6QWJzdHJhY3RNYWNyb0Fzc2VtYmxlcik6CisgICAg ICAgIC0gSW52YWxpZGF0ZSBhbGwgY2FjaGVkIHJlZ2lzdGVycyBpbiBjb25zdHJ1Y3RvciBhcyB3 ZSBkb24ndCBrbm93IHRoZQorICAgICAgICAgIGNvbnRlbnRzIG9mIGFueSByZWdpc3RlciBhdCB0 aGUgZW50cnkgdG8gdGhlIGNvZGUgd2UgYXJlIGdvaW5nIHRvCisgICAgICAgICAgZ2VuZXJhdGUu CisKIDIwMTQtMDMtMDMgIEFuZHJlYXMgS2xpbmcgIDxha2xpbmdAYXBwbGUuY29tPgogCiAgICAg ICAgIFN0cnVjdHVyZU9yT2Zmc2V0IHNob3VsZCBiZSBmYXN0bWFsbG9jZWQuCkluZGV4OiBTb3Vy Y2UvSmF2YVNjcmlwdENvcmUvYXNzZW1ibGVyL0Fic3RyYWN0TWFjcm9Bc3NlbWJsZXIuaAo9PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09Ci0tLSBTb3VyY2UvSmF2YVNjcmlwdENvcmUvYXNzZW1ibGVyL0Fic3RyYWN0TWFjcm9B c3NlbWJsZXIuaAkocmV2aXNpb24gMTY1MDIxKQorKysgU291cmNlL0phdmFTY3JpcHRDb3JlL2Fz c2VtYmxlci9BYnN0cmFjdE1hY3JvQXNzZW1ibGVyLmgJKHdvcmtpbmcgY29weSkKQEAgLTg0MCw2 ICs4NDAsNyBAQCBwcm90ZWN0ZWQ6CiAgICAgQWJzdHJhY3RNYWNyb0Fzc2VtYmxlcigpCiAgICAg ICAgIDogbV9yYW5kb21Tb3VyY2UoY3J5cHRvZ3JhcGhpY2FsbHlSYW5kb21OdW1iZXIoKSkKICAg ICB7CisgICAgICAgIGludmFsaWRhdGVBbGxUZW1wUmVnaXN0ZXJzKCk7CiAgICAgfQogCiAgICAg dWludDMyX3QgcmFuZG9tKCkK