Summary: | REGRESSION: Assertion failure viewing comments page on digg.com | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | David Kilzer (:ddkilzer) <ddkilzer> | ||||||||||
Component: | JavaScriptCore | Assignee: | Darin Adler <darin> | ||||||||||
Status: | RESOLVED FIXED | ||||||||||||
Severity: | Normal | CC: | bdakin, brkemper, david.barto, mitz | ||||||||||
Priority: | P1 | Keywords: | HasReduction, InRadar, Regression | ||||||||||
Version: | 528+ (Nightly build) | ||||||||||||
Hardware: | Mac | ||||||||||||
OS: | OS X 10.4 | ||||||||||||
URL: | http://digg.com/ | ||||||||||||
Attachments: |
|
Description
David Kilzer (:ddkilzer)
2007-11-05 13:39:34 PST
(In reply to comment #0) > 4. Search for "Leopard Windows bsod". > 5. Click on the "Comments" link for the "Remove the Windows BSOD icon in > Leopard, make OS X a little less smug" story. Direct link: http://digg.com/apple/Remove_the_Windows_BSOD_icon_in_Leopard_make_OS_X_a_little_less_smug (In reply to comment #0) > * SUMMARY > An assertion failure still occurs viewing a comments page on digg.com. See Bug 15747 Comment #5. *** Bug 16155 has been marked as a duplicate of this bug. *** *** Bug 16016 has been marked as a duplicate of this bug. *** *** Bug 16031 has been marked as a duplicate of this bug. *** Created attachment 17661 [details]
Reduction (will ASSERT)
The ASSERT is hit when accessing the 0th element of a sparse array. 0 is the empty value in the sparse array hash map so the assertion fails.
The ASSERT was added in <http://trac.webkit.org/projects/webkit/changeset/27196> where a similar problem was fixed for timer IDs. Created attachment 17662 [details]
Reduction (will crash)
A version that crashes release builds.
I noticed another HashMap in JavaScriptCore that might be prone to the same problem: the intIdentifierMap in npruntime.cpp. There might be more in WebCore. Created attachment 17665 [details]
patch
(In reply to comment #10) > I noticed another HashMap in JavaScriptCore that might be prone to the same > problem: the intIdentifierMap in npruntime.cpp. There might be more in WebCore. The intIdentifierMap will have a worse problem, because you can't use either a 0 or -1. In the array case, it's just 0 -- there's no issue with -1. We should probably track that issue with a separate bug. Created attachment 17666 [details]
Different approach, without an extra branch
I started working on this before I saw Darin took the bug. This approach avoids extra work when accessing the array but initializing the empty values to -2 costs more than initializing to 0.
(In reply to comment #13) > Created an attachment (id=17666) [edit] > Different approach, without an extra branch > > I started working on this before I saw Darin took the bug. This approach avoids > extra work when accessing the array but initializing the empty values to -2 > costs more than initializing to 0. It's the JavaScript language standard that makes FFFFFFFF special, not our implementation. We can't just change maxArrayIndex; FFFFFFFD and FFFFFFFE have to work as array indices, whereas FFFFFFFF can be treated as just another non-array property. So I suspect this won't work. We'd need a test case that sets a property with FFFFFFFE and checks the length of the array to demonstrate what's wrong with it. (In reply to comment #14) > So I suspect this won't work. We'd need a test case that sets a property with > FFFFFFFE and checks the length of the array to demonstrate what's wrong with > it. Yup, to make it work we'd need to add an extra branch somewhere :-) Comment on attachment 17665 [details]
patch
\ No newline at end of file
Please add a newline.
r=me
Committed revision 28346. |