75595 2012-01-04 18:24:17 -0800 Default HashTraits for Opcode don't work for Opcode = 0 2012-01-06 14:15:46 -0800 1 1 1 Unclassified WebKit JavaScriptCore 528+ (Nightly build) All All RESOLVED FIXED InRadar P2 Normal --- 1 msaboff msaboff ggaren harrison webkit.review.bot oldest_to_newest 529934 0 msaboff 2012-01-04 18:24:17 -0800 Opcode is defined as a void* and the value '0' is used in a HaspMap when COMPUTED_GOTO_INTERPRETER is enabled. An Opcode specific HashTraits needs to be created to account for this. 529942 1 121202 msaboff 2012-01-04 18:44:04 -0800 Created attachment 121202 Proposed patch 529943 2 webkit.review.bot 2012-01-04 18:46:25 -0800 Attachment 121202 did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/ChangeLog', u'Source..." exit_code: 1 Source/JavaScriptCore/interpreter/Interpreter.h:45: Code inside a namespace should not be indented. [whitespace/indent] [4] Total errors found: 1 in 2 files If any of these errors are false positives, please file a bug against check-webkit-style. 529945 3 121203 msaboff 2012-01-04 18:51:21 -0800 Created attachment 121203 Patch with indent change for stylebot 530460 4 121203 ggaren 2012-01-05 11:21:49 -0800 Comment on attachment 121203 Patch with indent change for stylebot A few comments: 1. I believe you can just use WTF::UnsignedWithZeroKeyHashTraits. You can do this by putting UnsignedWithZeroKeyHashTraits in the declaration of any hash table using Opcode as key, or by declaring HashTraits<Opcode> to inherit from UnsignedWithZeroKeyHashTraits. This is better than duplicating UnsignedWithZeroKeyHashTraits. 2. This code should not be conditional on #if ENABLE(COMPUTED_GOTO_INTERPRETER). That feature define does not have any bearing on whether the Opcode type has 0 in its domain. 3. The best place to put HashTraits for Opcode is Opcode.h. That way, it's impossible to put Opcode into a hash table inocrrectly. 530495 5 ggaren 2012-01-05 11:50:23 -0800 Even better solution: fix up Interpreter::initialize, as we discussed. 530851 6 121379 msaboff 2012-01-05 18:12:28 -0800 Created attachment 121379 Updated patch as discussed 531135 7 harrison 2012-01-06 10:11:07 -0800 rdar://10649745 531170 8 ggaren 2012-01-06 10:53:57 -0800 I think you can remove the whole "if (canUseJIT)" case from Interpreter::initialize. m_opcodeTable isn't used at all if Opcodes are not pointers. 531396 9 msaboff 2012-01-06 14:15:46 -0800 Committed r104338: <http://trac.webkit.org/changeset/104338> 121202 2012-01-04 18:44:04 -0800 2012-01-04 18:51:21 -0800 Proposed patch 75595.patch text/plain 1997 msaboff SW5kZXg6IFNvdXJjZS9KYXZhU2NyaXB0Q29yZS9DaGFuZ2VMb2cKPT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0gU291 cmNlL0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZwkocmV2aXNpb24gMTA0MTEwKQorKysgU291cmNl L0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZwkod29ya2luZyBjb3B5KQpAQCAtMSwzICsxLDEzIEBA CisyMDEyLTAxLTA0ICBNaWNoYWVsIFNhYm9mZiAgPG1zYWJvZmZAYXBwbGUuY29tPgorCisgICAg ICAgIERlZmF1bHQgSGFzaFRyYWl0cyBmb3IgT3Bjb2RlIGRvbid0IHdvcmsgZm9yIE9wY29kZSA9 IDAKKyAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTc1NTk1 CisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAgICAgKiBpbnRl cnByZXRlci9JbnRlcnByZXRlci5oOiBBZGQgaGFzaHRyYWl0cyBmb3IgT3Bjb2RlIHdpdGggZW1w dHkgCisgICAgICAgIG9mIC0xIGFuZCBkZWxldGVkIHZhbHVlIG9mIC0yLgorCiAyMDEyLTAxLTA0 ICBGaWxpcCBQaXpsbyAgPGZwaXpsb0BhcHBsZS5jb20+CiAKICAgICAgICAgVW5yZXZpZXdlZCwg YWNjaWRlbnRhbGx5IHR1cm5lZCBvZmYgdGhlIEpJVCBpbiBwcmV2aW91cyBjb21taXQuIFR1cm5p bmcKSW5kZXg6IFNvdXJjZS9KYXZhU2NyaXB0Q29yZS9pbnRlcnByZXRlci9JbnRlcnByZXRlci5o Cj09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT0KLS0tIFNvdXJjZS9KYXZhU2NyaXB0Q29yZS9pbnRlcnByZXRlci9JbnRlcnBy ZXRlci5oCShyZXZpc2lvbiAxMDQwOTMpCisrKyBTb3VyY2UvSmF2YVNjcmlwdENvcmUvaW50ZXJw cmV0ZXIvSW50ZXJwcmV0ZXIuaAkod29ya2luZyBjb3B5KQpAQCAtMzgsNiArMzgsMjkgQEAKIAog I2luY2x1ZGUgPHd0Zi9IYXNoTWFwLmg+CiAKKyNpZiBFTkFCTEUoQ09NUFVURURfR09UT19JTlRF UlBSRVRFUikKK3VzaW5nIEpTQzo6T3Bjb2RlOworCituYW1lc3BhY2UgV1RGIHsKKyAgICB0ZW1w bGF0ZTw+IHN0cnVjdCBIYXNoVHJhaXRzPEpTQzo6T3Bjb2RlPiB7CisgICAgICAgIHR5cGVkZWYg T3Bjb2RlIFRyYWl0VHlwZTsKKyAgICAgICAgdHlwZWRlZiBjb25zdCBPcGNvZGUmIFBhc3NJblR5 cGU7CisgICAgICAgIHR5cGVkZWYgT3Bjb2RlIFBhc3NPdXRUeXBlOworICAgICAgICB0eXBlZGVm IE9wY29kZSBQZWVrVHlwZTsKKyAgICAgICAgCisgICAgICAgIHN0YXRpYyBjb25zdCBib29sIGVt cHR5VmFsdWVJc1plcm8gPSBmYWxzZTsKKyAgICAgICAgc3RhdGljIGNvbnN0IGJvb2wgbmVlZHNE ZXN0cnVjdGlvbiA9IGZhbHNlOworICAgICAgICBzdGF0aWMgY29uc3QgaW50IG1pbmltdW1UYWJs ZVNpemUgPSA2NDsKKyAgICAgICAgc3RhdGljIE9wY29kZSBlbXB0eVZhbHVlKCkgeyByZXR1cm4g cmVpbnRlcnByZXRfY2FzdDxPcGNvZGU+KC0xKTsgfQorICAgICAgICBzdGF0aWMgdm9pZCBjb25z dHJ1Y3REZWxldGVkVmFsdWUoT3Bjb2RlJiBzbG90KSB7IHNsb3QgPSByZWludGVycHJldF9jYXN0 PE9wY29kZT4oLTIpOyB9CisgICAgICAgIHN0YXRpYyBib29sIGlzRGVsZXRlZFZhbHVlKE9wY29k ZSB2YWx1ZSkgeyByZXR1cm4gdmFsdWUgPT0gcmVpbnRlcnByZXRfY2FzdDxPcGNvZGU+KC0yKTsg fQorICAgICAgICBzdGF0aWMgdm9pZCBzdG9yZShjb25zdCBPcGNvZGUmIHZhbHVlLCBPcGNvZGUm IHN0b3JhZ2UpIHsgc3RvcmFnZSA9IHZhbHVlOyB9CisgICAgICAgIHN0YXRpYyBPcGNvZGUgcGFz c091dChjb25zdCBPcGNvZGUmIHZhbHVlKSB7IHJldHVybiB2YWx1ZTsgfQorICAgICAgICBzdGF0 aWMgT3Bjb2RlIHBlZWsoY29uc3QgT3Bjb2RlJiB2YWx1ZSkgeyByZXR1cm4gdmFsdWU7IH0KKyAg ICB9OworfQorI2VuZGlmCisKIG5hbWVzcGFjZSBKU0MgewogCiAgICAgY2xhc3MgQ29kZUJsb2Nr Owo= 121203 2012-01-04 18:51:21 -0800 2012-01-05 18:12:28 -0800 Patch with indent change for stylebot 75595-1.patch text/plain 1933 msaboff SW5kZXg6IFNvdXJjZS9KYXZhU2NyaXB0Q29yZS9DaGFuZ2VMb2cKPT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0gU291 cmNlL0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZwkocmV2aXNpb24gMTA0MTEwKQorKysgU291cmNl L0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZwkod29ya2luZyBjb3B5KQpAQCAtMSwzICsxLDEzIEBA CisyMDEyLTAxLTA0ICBNaWNoYWVsIFNhYm9mZiAgPG1zYWJvZmZAYXBwbGUuY29tPgorCisgICAg ICAgIERlZmF1bHQgSGFzaFRyYWl0cyBmb3IgT3Bjb2RlIGRvbid0IHdvcmsgZm9yIE9wY29kZSA9 IDAKKyAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTc1NTk1 CisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAgICAgKiBpbnRl cnByZXRlci9JbnRlcnByZXRlci5oOiBBZGQgaGFzaHRyYWl0cyBmb3IgT3Bjb2RlIHdpdGggZW1w dHkgCisgICAgICAgIG9mIC0xIGFuZCBkZWxldGVkIHZhbHVlIG9mIC0yLgorCiAyMDEyLTAxLTA0 ICBGaWxpcCBQaXpsbyAgPGZwaXpsb0BhcHBsZS5jb20+CiAKICAgICAgICAgVW5yZXZpZXdlZCwg YWNjaWRlbnRhbGx5IHR1cm5lZCBvZmYgdGhlIEpJVCBpbiBwcmV2aW91cyBjb21taXQuIFR1cm5p bmcKSW5kZXg6IFNvdXJjZS9KYXZhU2NyaXB0Q29yZS9pbnRlcnByZXRlci9JbnRlcnByZXRlci5o Cj09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT0KLS0tIFNvdXJjZS9KYXZhU2NyaXB0Q29yZS9pbnRlcnByZXRlci9JbnRlcnBy ZXRlci5oCShyZXZpc2lvbiAxMDQwOTMpCisrKyBTb3VyY2UvSmF2YVNjcmlwdENvcmUvaW50ZXJw cmV0ZXIvSW50ZXJwcmV0ZXIuaAkod29ya2luZyBjb3B5KQpAQCAtMzgsNiArMzgsMjkgQEAKIAog I2luY2x1ZGUgPHd0Zi9IYXNoTWFwLmg+CiAKKyNpZiBFTkFCTEUoQ09NUFVURURfR09UT19JTlRF UlBSRVRFUikKK3VzaW5nIEpTQzo6T3Bjb2RlOworCituYW1lc3BhY2UgV1RGIHsKK3RlbXBsYXRl PD4gc3RydWN0IEhhc2hUcmFpdHM8SlNDOjpPcGNvZGU+IHsKKyAgICB0eXBlZGVmIE9wY29kZSBU cmFpdFR5cGU7CisgICAgdHlwZWRlZiBjb25zdCBPcGNvZGUmIFBhc3NJblR5cGU7CisgICAgdHlw ZWRlZiBPcGNvZGUgUGFzc091dFR5cGU7CisgICAgdHlwZWRlZiBPcGNvZGUgUGVla1R5cGU7Cisg ICAgCisgICAgc3RhdGljIGNvbnN0IGJvb2wgZW1wdHlWYWx1ZUlzWmVybyA9IGZhbHNlOworICAg IHN0YXRpYyBjb25zdCBib29sIG5lZWRzRGVzdHJ1Y3Rpb24gPSBmYWxzZTsKKyAgICBzdGF0aWMg Y29uc3QgaW50IG1pbmltdW1UYWJsZVNpemUgPSA2NDsKKyAgICBzdGF0aWMgT3Bjb2RlIGVtcHR5 VmFsdWUoKSB7IHJldHVybiByZWludGVycHJldF9jYXN0PE9wY29kZT4oLTEpOyB9CisgICAgc3Rh dGljIHZvaWQgY29uc3RydWN0RGVsZXRlZFZhbHVlKE9wY29kZSYgc2xvdCkgeyBzbG90ID0gcmVp bnRlcnByZXRfY2FzdDxPcGNvZGU+KC0yKTsgfQorICAgIHN0YXRpYyBib29sIGlzRGVsZXRlZFZh bHVlKE9wY29kZSB2YWx1ZSkgeyByZXR1cm4gdmFsdWUgPT0gcmVpbnRlcnByZXRfY2FzdDxPcGNv ZGU+KC0yKTsgfQorICAgIHN0YXRpYyB2b2lkIHN0b3JlKGNvbnN0IE9wY29kZSYgdmFsdWUsIE9w Y29kZSYgc3RvcmFnZSkgeyBzdG9yYWdlID0gdmFsdWU7IH0KKyAgICBzdGF0aWMgT3Bjb2RlIHBh c3NPdXQoY29uc3QgT3Bjb2RlJiB2YWx1ZSkgeyByZXR1cm4gdmFsdWU7IH0KKyAgICBzdGF0aWMg T3Bjb2RlIHBlZWsoY29uc3QgT3Bjb2RlJiB2YWx1ZSkgeyByZXR1cm4gdmFsdWU7IH0KK307Cit9 CisjZW5kaWYKKwogbmFtZXNwYWNlIEpTQyB7CiAKICAgICBjbGFzcyBDb2RlQmxvY2s7Cg== 121379 2012-01-05 18:12:28 -0800 2012-01-06 14:03:35 -0800 Updated patch as discussed 75595-2.patch text/plain 2872 msaboff SW5kZXg6IFNvdXJjZS9KYXZhU2NyaXB0Q29yZS9DaGFuZ2VMb2cKPT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0gU291 cmNlL0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZwkocmV2aXNpb24gMTA0MjU5KQorKysgU291cmNl L0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZwkod29ya2luZyBjb3B5KQpAQCAtMSwzICsxLDIwIEBA CisyMDEyLTAxLTA1ICBNaWNoYWVsIFNhYm9mZiAgPG1zYWJvZmZAYXBwbGUuY29tPgorCisgICAg ICAgIERlZmF1bHQgSGFzaFRyYWl0cyBmb3IgT3Bjb2RlIGRvbid0IHdvcmsgZm9yIE9wY29kZSA9 IDAKKyAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTc1NTk1 CisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAgICAgUmVtb3Zl ZCB0aGUgcG9wdWxhdGluZyBvZiB0aGUgbV9vcGNvZGVJRFRhYmxlIHRhYmxlIGluIHRoZQorICAg ICAgICBjYXNlIHdoZXJlIHRoZSBPcGNvZGVJRCBhbmQgT3Bjb2RlIGFyZSB0aGUgc2FtZSAobV9l bmFibGVkIGlzIGZhbHNlKS4KKyAgICAgICAgSW5zdGVhZCB3ZSBqdXN0IGNhc3QgdGhlIG9uZSB0 eXBlIHRvIHRoZSBvdGhlci4KKworICAgICAgICAqIGludGVycHJldGVyL0ludGVycHJldGVyLmNw cDoKKyAgICAgICAgKEpTQzo6SW50ZXJwcmV0ZXI6OmluaXRpYWxpemUpOgorICAgICAgICAoSlND OjpJbnRlcnByZXRlcjo6aXNPcGNvZGUpOgorICAgICAgICAqIGludGVycHJldGVyL0ludGVycHJl dGVyLmg6CisgICAgICAgIChKU0M6OkludGVycHJldGVyOjpnZXRPcGNvZGVJRCk6CisKIDIwMTIt MDEtMDUgIEdhdmluIEJhcnJhY2xvdWdoICA8YmFycmFjbG91Z2hAYXBwbGUuY29tPgogCiAgICAg ICAgIEFycmF5LnByb3RvdHlwZS5sYXN0SW5kZXhPZiBpZ25vcmVzIHVuZGVmaW5lZCBmcm9tSW5k ZXguCkluZGV4OiBTb3VyY2UvSmF2YVNjcmlwdENvcmUvaW50ZXJwcmV0ZXIvSW50ZXJwcmV0ZXIu Y3BwCj09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT0KLS0tIFNvdXJjZS9KYXZhU2NyaXB0Q29yZS9pbnRlcnByZXRlci9JbnRl cnByZXRlci5jcHAJKHJldmlzaW9uIDEwNDI0NikKKysrIFNvdXJjZS9KYXZhU2NyaXB0Q29yZS9p bnRlcnByZXRlci9JbnRlcnByZXRlci5jcHAJKHdvcmtpbmcgY29weSkKQEAgLTU1Niw3ICs1NTYs NiBAQCB2b2lkIEludGVycHJldGVyOjppbml0aWFsaXplKGJvb2wgY2FuVXNlCiAgICAgICAgIGZv ciAoaW50IGkgPSAwOyBpIDwgbnVtT3Bjb2RlSURzOyArK2kpIHsKICAgICAgICAgICAgIE9wY29k ZSBvcGNvZGUgPSBiaXR3aXNlX2Nhc3Q8dm9pZCo+KHN0YXRpY19jYXN0PHVpbnRwdHJfdD4oaSkp OwogICAgICAgICAgICAgbV9vcGNvZGVUYWJsZVtpXSA9IG9wY29kZTsKLSAgICAgICAgICAgIG1f b3Bjb2RlSURUYWJsZS5hZGQob3Bjb2RlLCBzdGF0aWNfY2FzdDxPcGNvZGVJRD4oaSkpOwogICAg ICAgICB9CiAgICAgfSBlbHNlIHsKICAgICAgICAgcHJpdmF0ZUV4ZWN1dGUoSW5pdGlhbGl6ZUFu ZFJldHVybiwgMCwgMCk7CkBAIC02NjksNiArNjY4LDggQEAgdm9pZCBJbnRlcnByZXRlcjo6ZHVt cFJlZ2lzdGVycyhDYWxsRnJhbQogYm9vbCBJbnRlcnByZXRlcjo6aXNPcGNvZGUoT3Bjb2RlIG9w Y29kZSkKIHsKICNpZiBFTkFCTEUoQ09NUFVURURfR09UT19JTlRFUlBSRVRFUikKKyAgICBpZiAo IW1fZW5hYmxlZCkKKyAgICAgICAgcmV0dXJuIG9wY29kZSA+PSAwICYmIHN0YXRpY19jYXN0PE9w Y29kZUlEPihiaXR3aXNlX2Nhc3Q8dWludHB0cl90PihvcGNvZGUpKSA8PSBvcF9lbmQ7CiAgICAg cmV0dXJuIG9wY29kZSAhPSBIYXNoVHJhaXRzPE9wY29kZT46OmVtcHR5VmFsdWUoKQogICAgICAg ICAmJiAhSGFzaFRyYWl0czxPcGNvZGU+Ojppc0RlbGV0ZWRWYWx1ZShvcGNvZGUpCiAgICAgICAg ICYmIG1fb3Bjb2RlSURUYWJsZS5jb250YWlucyhvcGNvZGUpOwpJbmRleDogU291cmNlL0phdmFT Y3JpcHRDb3JlL2ludGVycHJldGVyL0ludGVycHJldGVyLmgKPT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0gU291cmNl L0phdmFTY3JpcHRDb3JlL2ludGVycHJldGVyL0ludGVycHJldGVyLmgJKHJldmlzaW9uIDEwNDI0 NikKKysrIFNvdXJjZS9KYXZhU2NyaXB0Q29yZS9pbnRlcnByZXRlci9JbnRlcnByZXRlci5oCSh3 b3JraW5nIGNvcHkpCkBAIC0xMTQsMTEgKzExNCw5IEBAIG5hbWVzcGFjZSBKU0MgewogICAgICAg ICAgICAgQVNTRVJUKG1faW5pdGlhbGl6ZWQpOwogI2lmIEVOQUJMRShDT01QVVRFRF9HT1RPX0lO VEVSUFJFVEVSKQogICAgICAgICAgICAgQVNTRVJUKGlzT3Bjb2RlKG9wY29kZSkpOwotICAgICAg ICAgICAgaWYgKCFtX2VuYWJsZWQpIHsKLSAgICAgICAgICAgICAgICBPcGNvZGVJRCByZXN1bHQg PSBzdGF0aWNfY2FzdDxPcGNvZGVJRD4oYml0d2lzZV9jYXN0PHVpbnRwdHJfdD4ob3Bjb2RlKSk7 Ci0gICAgICAgICAgICAgICAgQVNTRVJUKHJlc3VsdCA9PSBtX29wY29kZUlEVGFibGUuZ2V0KG9w Y29kZSkpOwotICAgICAgICAgICAgICAgIHJldHVybiByZXN1bHQ7Ci0gICAgICAgICAgICB9Cisg ICAgICAgICAgICBpZiAoIW1fZW5hYmxlZCkKKyAgICAgICAgICAgICAgICByZXR1cm4gc3RhdGlj X2Nhc3Q8T3Bjb2RlSUQ+KGJpdHdpc2VfY2FzdDx1aW50cHRyX3Q+KG9wY29kZSkpOworCiAgICAg ICAgICAgICByZXR1cm4gbV9vcGNvZGVJRFRhYmxlLmdldChvcGNvZGUpOwogI2Vsc2UKICAgICAg ICAgICAgIHJldHVybiBvcGNvZGU7Cg==