Bug 69102 – Structure transitions involving many (> 64) properties sometimes cause structure corruption

• Status: RESOLVED
• Resolution: FIXED
• Component: JavaScriptCore

[Filip Pizlo]

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.

[Filip Pizlo]

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.

[Darin Adler]

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.

[Geoffrey Garen]

If you really want to support an arbitrary size, shouldn't m_offset be size_t?

[Filip Pizlo]

(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).

[Filip Pizlo]

(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.

[Filip Pizlo]

Landed in r96354.