69102 2011-09-29 13:24:55 -0700 Structure transitions involving many (> 64) properties sometimes cause structure corruption 2011-09-30 05:49:56 -0700 1 1 1 Unclassified WebKit JavaScriptCore 528+ (Nightly build) All All RESOLVED FIXED P2 Normal --- 68990 1 fpizlo webkit-unassigned fpizlo ggaren oldest_to_newest 475637 0 fpizlo 2011-09-29 13:24:55 -0700 This involves two problems: 1) Some transitions, such as function despecify, lead the new structure to "forget" that it should have been a dictionary. 2) Flattening a dictionary with many (> 64) fields overflows Structure::m_offset, making it impossible to rematerialize the property map. 475639 1 109196 fpizlo 2011-09-29 13:27:22 -0700 Created attachment 109196 the patch I tried to reduce the fail to a test case, but failed. The chain of structure transitions that causes this bug to manifest is too chaotic for my small mind. 475647 2 109196 darin 2011-09-29 13:36:18 -0700 Comment on attachment 109196 the patch Unfortunate that we can’t figure out how to test. Given we did signed char before, I’m surprised you went right to int rather than considering short. Not sure how we’re doing on total size of Structure. 475649 3 ggaren 2011-09-29 13:37:23 -0700 If you really want to support an arbitrary size, shouldn't m_offset be size_t? 475651 4 fpizlo 2011-09-29 13:38:26 -0700 (In reply to comment #2) > (From update of attachment 109196 [details]) > Unfortunate that we can’t figure out how to test. Given we did signed char before, I’m surprised you went right to int rather than considering short. Not sure how we’re doing on total size of Structure. I went to int because I didn't want to see this bug ever again. I considered unsigned, but that would make the change bigger (the code uses -1 as a marker). 475654 5 fpizlo 2011-09-29 13:41:32 -0700 (In reply to comment #3) > If you really want to support an arbitrary size, shouldn't m_offset be size_t? I could imagine code that wants > 2^15 properties. I've seen Java code out there that pushes right up to that limit. (Java has a 2^16 hard limit on fields, and I've seen code generators that push that limit by splitting the code into multiple classes.) If someone wanted to set >2^31 fields, then we'd probably fall over and die for other reasons. I didn't want to use an unsigned type because that would require making this a bigger change. We use -1 as a marker. And anyway, the difference between dying at 2^31 and 2^32 is not so great. 475656 6 fpizlo 2011-09-29 13:45:08 -0700 Landed in r96354. 109196 2011-09-29 13:27:22 -0700 2011-09-29 13:36:17 -0700 the patch fixstruct_patch_1.diff text/plain 2536 fpizlo SW5kZXg6IFNvdXJjZS9KYXZhU2NyaXB0Q29yZS9DaGFuZ2VMb2cKPT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0gU291 cmNlL0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZwkocmV2aXNpb24gOTYzNTEpCisrKyBTb3VyY2Uv SmF2YVNjcmlwdENvcmUvQ2hhbmdlTG9nCSh3b3JraW5nIGNvcHkpCkBAIC0xLDMgKzEsMTcgQEAK KzIwMTEtMDktMjkgIEZpbGlwIFBpemxvICA8ZnBpemxvQGFwcGxlLmNvbT4KKworICAgICAgICBT dHJ1Y3R1cmUgdHJhbnNpdGlvbnMgaW52b2x2aW5nIG1hbnkgKD4gNjQpIHByb3BlcnRpZXMgc29t ZXRpbWVzIGNhdXNlIHN0cnVjdHVyZSBjb3JydXB0aW9uCisgICAgICAgIGh0dHBzOi8vYnVncy53 ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD02OTEwMgorCisgICAgICAgIFJldmlld2VkIGJ5IE5P Qk9EWSAoT09QUyEpLgorICAgICAgICAKKyAgICAgICAgTWFkZSBtX29mZnNldCBhbiBpbnQgaW5z dGVhZCBvZiBhIHNpZ25lZCBjaGFyLiBDaGFuZ2VkIHRoZSBjb2RlIHRvIGVuc3VyZSB0aGF0IHRy YW5zaXRpb25zCisgICAgICAgIGRvbid0IGxlYWQgdG8gdGhlIGRpY3Rpb25hcnkga2luZCBiZWlu ZyBmb3Jnb3R0ZW4uCisgICAgICAgIAorICAgICAgICAqIHJ1bnRpbWUvU3RydWN0dXJlLmNwcDoK KyAgICAgICAgKEpTQzo6U3RydWN0dXJlOjpTdHJ1Y3R1cmUpOgorICAgICAgICAqIHJ1bnRpbWUv U3RydWN0dXJlLmg6CisKIDIwMTEtMDktMjkgIFl1cWlhbmcgWGlhbiAgPHl1cWlhbmcueGlhbkBp bnRlbC5jb20+CiAKICAgICAgICAgREZHIG9wZXJhdGlvbiBjYWxscyBzaG91bGQgYmUgc3RkY2Fs bCBpbiBMaW51eCBKU1ZBTFVFMzJfNjQgREZHIEpJVApJbmRleDogU291cmNlL0phdmFTY3JpcHRD b3JlL3J1bnRpbWUvU3RydWN0dXJlLmNwcAo9PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvSmF2YVNjcmlw dENvcmUvcnVudGltZS9TdHJ1Y3R1cmUuY3BwCShyZXZpc2lvbiA5NjMwNSkKKysrIFNvdXJjZS9K YXZhU2NyaXB0Q29yZS9ydW50aW1lL1N0cnVjdHVyZS5jcHAJKHdvcmtpbmcgY29weSkKQEAgLTIw Miw3ICsyMDIsNyBAQCBTdHJ1Y3R1cmU6OlN0cnVjdHVyZShKU0dsb2JhbERhdGEmIGdsb2JhCiAg ICAgLCBtX2NsYXNzSW5mbyhwcmV2aW91cy0+bV9jbGFzc0luZm8pCiAgICAgLCBtX3Byb3BlcnR5 U3RvcmFnZUNhcGFjaXR5KHByZXZpb3VzLT5tX3Byb3BlcnR5U3RvcmFnZUNhcGFjaXR5KQogICAg ICwgbV9vZmZzZXQobm9PZmZzZXQpCi0gICAgLCBtX2RpY3Rpb25hcnlLaW5kKE5vbmVEaWN0aW9u YXJ5S2luZCkKKyAgICAsIG1fZGljdGlvbmFyeUtpbmQocHJldmlvdXMtPm1fZGljdGlvbmFyeUtp bmQpCiAgICAgLCBtX2lzUGlubmVkUHJvcGVydHlUYWJsZShmYWxzZSkKICAgICAsIG1faGFzR2V0 dGVyU2V0dGVyUHJvcGVydGllcyhwcmV2aW91cy0+bV9oYXNHZXR0ZXJTZXR0ZXJQcm9wZXJ0aWVz KQogICAgICwgbV9oYXNOb25FbnVtZXJhYmxlUHJvcGVydGllcyhwcmV2aW91cy0+bV9oYXNOb25F bnVtZXJhYmxlUHJvcGVydGllcykKSW5kZXg6IFNvdXJjZS9KYXZhU2NyaXB0Q29yZS9ydW50aW1l L1N0cnVjdHVyZS5oCj09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIFNvdXJjZS9KYXZhU2NyaXB0Q29yZS9ydW50aW1l L1N0cnVjdHVyZS5oCShyZXZpc2lvbiA5NjMwNSkKKysrIFNvdXJjZS9KYXZhU2NyaXB0Q29yZS9y dW50aW1lL1N0cnVjdHVyZS5oCSh3b3JraW5nIGNvcHkpCkBAIC0yMzgsOSArMjM4LDkgQEAgbmFt ZXNwYWNlIEpTQyB7CiAKICAgICAgICAgYm9vbCBpc1ZhbGlkKEV4ZWNTdGF0ZSosIFN0cnVjdHVy ZUNoYWluKiBjYWNoZWRQcm90b3R5cGVDaGFpbikgY29uc3Q7CiAKLSAgICAgICAgc3RhdGljIGNv bnN0IHNpZ25lZCBjaGFyIHNfbWF4VHJhbnNpdGlvbkxlbmd0aCA9IDY0OworICAgICAgICBzdGF0 aWMgY29uc3QgaW50IHNfbWF4VHJhbnNpdGlvbkxlbmd0aCA9IDY0OwogCi0gICAgICAgIHN0YXRp YyBjb25zdCBzaWduZWQgY2hhciBub09mZnNldCA9IC0xOworICAgICAgICBzdGF0aWMgY29uc3Qg aW50IG5vT2Zmc2V0ID0gLTE7CiAKICAgICAgICAgc3RhdGljIGNvbnN0IHVuc2lnbmVkIG1heFNw ZWNpZmljRnVuY3Rpb25UaHJhc2hDb3VudCA9IDM7CiAKQEAgLTI2NSw3ICsyNjUsNyBAQCBuYW1l c3BhY2UgSlNDIHsKICAgICAgICAgdWludDMyX3QgbV9wcm9wZXJ0eVN0b3JhZ2VDYXBhY2l0eTsK IAogICAgICAgICAvLyBtX29mZnNldCBkb2VzIG5vdCBhY2NvdW50IGZvciBhbm9ueW1vdXMgc2xv dHMKLSAgICAgICAgc2lnbmVkIGNoYXIgbV9vZmZzZXQ7CisgICAgICAgIGludCBtX29mZnNldDsK IAogICAgICAgICB1bnNpZ25lZCBtX2RpY3Rpb25hcnlLaW5kIDogMjsKICAgICAgICAgYm9vbCBt X2lzUGlubmVkUHJvcGVydHlUYWJsZSA6IDE7Cg==