173057 2017-06-07 07:14:28 -0700 [JSC] has_generic_property never accepts non-String 2017-06-07 09:54:35 -0700 1 1 1 Unclassified WebKit New Bugs WebKit Nightly Build Unspecified Unspecified RESOLVED FIXED P2 Normal --- 1 ysuzuki ysuzuki buildbot darin keith_miller mark.lam msaboff saam oldest_to_newest 1316460 0 ysuzuki 2017-06-07 07:14:28 -0700 [JSC] has_generic_property never accepts non-String 1316461 1 312181 ysuzuki 2017-06-07 07:14:56 -0700 Created attachment 312181 Patch 1316462 2 312182 ysuzuki 2017-06-07 07:18:06 -0700 Created attachment 312182 Patch 1316491 3 312182 darin 2017-06-07 09:00:36 -0700 Comment on attachment 312182 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=312182&action=review > Source/JavaScriptCore/runtime/CommonSlowPaths.cpp:717 > + ASSERT(property.isString()); > + RETURN(jsBoolean(base->hasPropertyGeneric(exec, asString(property.asCell())->toIdentifier(exec), PropertySlot::InternalMethodType::GetOwnProperty))); The asString function already does this assertion; separately asserting is unnecessary. 1316493 4 312182 darin 2017-06-07 09:01:07 -0700 Comment on attachment 312182 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=312182&action=review >> Source/JavaScriptCore/runtime/CommonSlowPaths.cpp:717 >> + RETURN(jsBoolean(base->hasPropertyGeneric(exec, asString(property.asCell())->toIdentifier(exec), PropertySlot::InternalMethodType::GetOwnProperty))); > > The asString function already does this assertion; separately asserting is unnecessary. Style-wise, would suggest making the local variable a JSString* and doing the asString on the separate line. 1316494 5 312182 darin 2017-06-07 09:01:54 -0700 Comment on attachment 312182 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=312182&action=review >>> Source/JavaScriptCore/runtime/CommonSlowPaths.cpp:717 >>> + RETURN(jsBoolean(base->hasPropertyGeneric(exec, asString(property.asCell())->toIdentifier(exec), PropertySlot::InternalMethodType::GetOwnProperty))); >> >> The asString function already does this assertion; separately asserting is unnecessary. > > Style-wise, would suggest making the local variable a JSString* and doing the asString on the separate line. But also wondering if we need to check for exception after calling toIdentifier and before calling hasPropertyGeneric, in case toIdentifier runs into a memory issue resolving a rope. 1316514 6 312182 ysuzuki 2017-06-07 09:53:16 -0700 Comment on attachment 312182 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=312182&action=review >>>> Source/JavaScriptCore/runtime/CommonSlowPaths.cpp:717 >>>> + RETURN(jsBoolean(base->hasPropertyGeneric(exec, asString(property.asCell())->toIdentifier(exec), PropertySlot::InternalMethodType::GetOwnProperty))); >>> >>> The asString function already does this assertion; separately asserting is unnecessary. >> >> Style-wise, would suggest making the local variable a JSString* and doing the asString on the separate line. > > But also wondering if we need to check for exception after calling toIdentifier and before calling hasPropertyGeneric, in case toIdentifier runs into a memory issue resolving a rope. Sounds nice. We should insert check here. Fixed. 1316515 7 ysuzuki 2017-06-07 09:54:35 -0700 Committed r217887: <http://trac.webkit.org/changeset/217887> 312181 2017-06-07 07:14:56 -0700 2017-06-07 07:18:04 -0700 Patch bug-173057-20170607231455.patch text/plain 1884 ysuzuki U3VidmVyc2lvbiBSZXZpc2lvbjogMjE3ODgxCmRpZmYgLS1naXQgYS9Tb3VyY2UvSmF2YVNjcmlw dENvcmUvQ2hhbmdlTG9nIGIvU291cmNlL0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZwppbmRleCA2 NGU4M2JkMzVlMDQ3NmRjODNkZWNlYmFjZWIzMWVhYjJhMzdjMjljLi5kZTQ2ZTdhZThiMWZiZGVm NmE4ZDIwZDY2MzgyYTZmYWNhM2Y1NzI1IDEwMDY0NAotLS0gYS9Tb3VyY2UvSmF2YVNjcmlwdENv cmUvQ2hhbmdlTG9nCisrKyBiL1NvdXJjZS9KYXZhU2NyaXB0Q29yZS9DaGFuZ2VMb2cKQEAgLTEs MyArMSwxNSBAQAorMjAxNy0wNi0wNyAgWXVzdWtlIFN1enVraSAgPHV0YXRhbmUudGVhQGdtYWls LmNvbT4KKworICAgICAgICBbSlNDXSBoYXNfZ2VuZXJpY19wcm9wZXJ0eSBuZXZlciBhY2NlcHRz IG5vbi1TdHJpbmcKKyAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dp P2lkPTE3MzA1NworCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAg ICAgIFdlIG5ldmVyIHBhc3Mgbm9uLVN0cmluZyB2YWx1ZSB0byBoYXNfZ2VuZXJpY19wcm9wZXJ0 eSBieXRlY29kZS4KKworICAgICAgICAqIHJ1bnRpbWUvQ29tbW9uU2xvd1BhdGhzLmNwcDoKKyAg ICAgICAgKEpTQzo6U0xPV19QQVRIX0RFQ0wpOgorCiAyMDE3LTA2LTA2ICBGdWppaSBIaXJvbm9y aSAgPEhpcm9ub3JpLkZ1amlpQHNvbnkuY29tPgogCiAgICAgICAgIFtXaW5dW3g4Ni02NF0gU29t ZSBjYWxsZWUgc2F2ZWQgcmVnaXN0ZXJzIGFyZW4ndCBwcmVzZXJ2ZWQKZGlmZiAtLWdpdCBhL1Nv dXJjZS9KYXZhU2NyaXB0Q29yZS9ydW50aW1lL0NvbW1vblNsb3dQYXRocy5jcHAgYi9Tb3VyY2Uv SmF2YVNjcmlwdENvcmUvcnVudGltZS9Db21tb25TbG93UGF0aHMuY3BwCmluZGV4IGRiMDllNjE2 Zjg4ODEzYTliY2I3MWY2NzQyMTBiN2RkZjFjZmQ4N2IuLmJiZDk3NGU3N2Y0N2FmNTQ3M2RhMzYz OTZlY2M5YzhmMTQxMjA5NjQgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9KYXZhU2NyaXB0Q29yZS9ydW50 aW1lL0NvbW1vblNsb3dQYXRocy5jcHAKKysrIGIvU291cmNlL0phdmFTY3JpcHRDb3JlL3J1bnRp bWUvQ29tbW9uU2xvd1BhdGhzLmNwcApAQCAtNzE0LDEzICs3MTQsOCBAQCBTTE9XX1BBVEhfREVD TChzbG93X3BhdGhfaGFzX2dlbmVyaWNfcHJvcGVydHkpCiAgICAgQ0hFQ0tfRVhDRVBUSU9OKCk7 CiAgICAgSlNWYWx1ZSBwcm9wZXJ0eSA9IE9QKDMpLmpzVmFsdWUoKTsKICAgICBib29sIHJlc3Vs dDsKLSAgICBpZiAocHJvcGVydHkuaXNTdHJpbmcoKSkKLSAgICAgICAgcmVzdWx0ID0gYmFzZS0+ aGFzUHJvcGVydHlHZW5lcmljKGV4ZWMsIGFzU3RyaW5nKHByb3BlcnR5LmFzQ2VsbCgpKS0+dG9J ZGVudGlmaWVyKGV4ZWMpLCBQcm9wZXJ0eVNsb3Q6OkludGVybmFsTWV0aG9kVHlwZTo6R2V0T3du UHJvcGVydHkpOwotICAgIGVsc2UgewotICAgICAgICBBU1NFUlQocHJvcGVydHkuaXNVSW50MzIo KSk7Ci0gICAgICAgIHJlc3VsdCA9IGJhc2UtPmhhc1Byb3BlcnR5R2VuZXJpYyhleGVjLCBwcm9w ZXJ0eS5hc1VJbnQzMigpLCBQcm9wZXJ0eVNsb3Q6OkludGVybmFsTWV0aG9kVHlwZTo6R2V0T3du UHJvcGVydHkpOwotICAgIH0KLSAgICBSRVRVUk4oanNCb29sZWFuKHJlc3VsdCkpOworICAgIEFT U0VSVChwcm9wZXJ0eS5pc1N0cmluZygpKTsKKyAgICBSRVRVUk4oanNCb29sZWFuKGJhc2UtPmhh c1Byb3BlcnR5R2VuZXJpYyhleGVjLCBhc1N0cmluZyhwcm9wZXJ0eS5hc0NlbGwoKSktPnRvSWRl bnRpZmllcihleGVjKSwgUHJvcGVydHlTbG90OjpJbnRlcm5hbE1ldGhvZFR5cGU6OkdldE93blBy b3BlcnR5KSkpOwogfQogCiBTTE9XX1BBVEhfREVDTChzbG93X3BhdGhfZ2V0X2RpcmVjdF9wbmFt ZSkK 312182 2017-06-07 07:18:06 -0700 2017-06-07 09:00:36 -0700 Patch bug-173057-20170607231805.patch text/plain 1938 ysuzuki U3VidmVyc2lvbiBSZXZpc2lvbjogMjE3ODgxCmRpZmYgLS1naXQgYS9Tb3VyY2UvSmF2YVNjcmlw dENvcmUvQ2hhbmdlTG9nIGIvU291cmNlL0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZwppbmRleCA2 NGU4M2JkMzVlMDQ3NmRjODNkZWNlYmFjZWIzMWVhYjJhMzdjMjljLi5kZTQ2ZTdhZThiMWZiZGVm NmE4ZDIwZDY2MzgyYTZmYWNhM2Y1NzI1IDEwMDY0NAotLS0gYS9Tb3VyY2UvSmF2YVNjcmlwdENv cmUvQ2hhbmdlTG9nCisrKyBiL1NvdXJjZS9KYXZhU2NyaXB0Q29yZS9DaGFuZ2VMb2cKQEAgLTEs MyArMSwxNSBAQAorMjAxNy0wNi0wNyAgWXVzdWtlIFN1enVraSAgPHV0YXRhbmUudGVhQGdtYWls LmNvbT4KKworICAgICAgICBbSlNDXSBoYXNfZ2VuZXJpY19wcm9wZXJ0eSBuZXZlciBhY2NlcHRz IG5vbi1TdHJpbmcKKyAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dp P2lkPTE3MzA1NworCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAg ICAgIFdlIG5ldmVyIHBhc3Mgbm9uLVN0cmluZyB2YWx1ZSB0byBoYXNfZ2VuZXJpY19wcm9wZXJ0 eSBieXRlY29kZS4KKworICAgICAgICAqIHJ1bnRpbWUvQ29tbW9uU2xvd1BhdGhzLmNwcDoKKyAg ICAgICAgKEpTQzo6U0xPV19QQVRIX0RFQ0wpOgorCiAyMDE3LTA2LTA2ICBGdWppaSBIaXJvbm9y aSAgPEhpcm9ub3JpLkZ1amlpQHNvbnkuY29tPgogCiAgICAgICAgIFtXaW5dW3g4Ni02NF0gU29t ZSBjYWxsZWUgc2F2ZWQgcmVnaXN0ZXJzIGFyZW4ndCBwcmVzZXJ2ZWQKZGlmZiAtLWdpdCBhL1Nv dXJjZS9KYXZhU2NyaXB0Q29yZS9ydW50aW1lL0NvbW1vblNsb3dQYXRocy5jcHAgYi9Tb3VyY2Uv SmF2YVNjcmlwdENvcmUvcnVudGltZS9Db21tb25TbG93UGF0aHMuY3BwCmluZGV4IGRiMDllNjE2 Zjg4ODEzYTliY2I3MWY2NzQyMTBiN2RkZjFjZmQ4N2IuLmZjODkyOTFjNmM0OGRlYzMyYzY4NThk YzFlMTA5NjVmNGJlMGRhODQgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9KYXZhU2NyaXB0Q29yZS9ydW50 aW1lL0NvbW1vblNsb3dQYXRocy5jcHAKKysrIGIvU291cmNlL0phdmFTY3JpcHRDb3JlL3J1bnRp bWUvQ29tbW9uU2xvd1BhdGhzLmNwcApAQCAtNzEzLDE0ICs3MTMsOCBAQCBTTE9XX1BBVEhfREVD TChzbG93X3BhdGhfaGFzX2dlbmVyaWNfcHJvcGVydHkpCiAgICAgSlNPYmplY3QqIGJhc2UgPSBP UCgyKS5qc1ZhbHVlKCkudG9PYmplY3QoZXhlYyk7CiAgICAgQ0hFQ0tfRVhDRVBUSU9OKCk7CiAg ICAgSlNWYWx1ZSBwcm9wZXJ0eSA9IE9QKDMpLmpzVmFsdWUoKTsKLSAgICBib29sIHJlc3VsdDsK LSAgICBpZiAocHJvcGVydHkuaXNTdHJpbmcoKSkKLSAgICAgICAgcmVzdWx0ID0gYmFzZS0+aGFz UHJvcGVydHlHZW5lcmljKGV4ZWMsIGFzU3RyaW5nKHByb3BlcnR5LmFzQ2VsbCgpKS0+dG9JZGVu dGlmaWVyKGV4ZWMpLCBQcm9wZXJ0eVNsb3Q6OkludGVybmFsTWV0aG9kVHlwZTo6R2V0T3duUHJv cGVydHkpOwotICAgIGVsc2UgewotICAgICAgICBBU1NFUlQocHJvcGVydHkuaXNVSW50MzIoKSk7 Ci0gICAgICAgIHJlc3VsdCA9IGJhc2UtPmhhc1Byb3BlcnR5R2VuZXJpYyhleGVjLCBwcm9wZXJ0 eS5hc1VJbnQzMigpLCBQcm9wZXJ0eVNsb3Q6OkludGVybmFsTWV0aG9kVHlwZTo6R2V0T3duUHJv cGVydHkpOwotICAgIH0KLSAgICBSRVRVUk4oanNCb29sZWFuKHJlc3VsdCkpOworICAgIEFTU0VS VChwcm9wZXJ0eS5pc1N0cmluZygpKTsKKyAgICBSRVRVUk4oanNCb29sZWFuKGJhc2UtPmhhc1By b3BlcnR5R2VuZXJpYyhleGVjLCBhc1N0cmluZyhwcm9wZXJ0eS5hc0NlbGwoKSktPnRvSWRlbnRp ZmllcihleGVjKSwgUHJvcGVydHlTbG90OjpJbnRlcm5hbE1ldGhvZFR5cGU6OkdldE93blByb3Bl cnR5KSkpOwogfQogCiBTTE9XX1BBVEhfREVDTChzbG93X3BhdGhfZ2V0X2RpcmVjdF9wbmFtZSkK