Summary: | Structure transitions involving many (> 64) properties sometimes cause structure corruption | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Filip Pizlo <fpizlo> | ||||
Component: | JavaScriptCore | Assignee: | Nobody <webkit-unassigned> | ||||
Status: | RESOLVED FIXED | ||||||
Severity: | Normal | CC: | fpizlo, ggaren | ||||
Priority: | P2 | ||||||
Version: | 528+ (Nightly build) | ||||||
Hardware: | All | ||||||
OS: | All | ||||||
Bug Depends on: | |||||||
Bug Blocks: | 68990 | ||||||
Attachments: |
|
Description
Filip Pizlo
2011-09-29 13:24:55 PDT
Created attachment 109196 [details]
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.
Comment on attachment 109196 [details]
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.
If you really want to support an arbitrary size, shouldn't m_offset be size_t? (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). (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. |