Summary: | ASSERTION FAILED: _hash in KJS::UString::Rep::computedHash() | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Matt Lilek <dev+webkit> | ||||||
Component: | JavaScriptCore | Assignee: | Maciej Stachowiak <mjs> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | ggaren, mitz, mjs, mrowe, rniwa | ||||||
Priority: | P1 | Keywords: | NeedsReduction, Regression | ||||||
Version: | 523.x (Safari 3) | ||||||||
Hardware: | Mac | ||||||||
OS: | OS X 10.5 | ||||||||
Attachments: |
|
Description
Matt Lilek
2007-10-26 18:48:05 PDT
Actually, Google ads trips this so it's reproducible on any site that uses them (ie: <http://digg.com/>). I can reproduce this easily at http://webkit.org/blog/wp-admin/. I'll see if I can't cook up a reduction later this evening. Created attachment 16900 [details]
fix
Comment on attachment 16900 [details]
fix
r=me
The buildbot is still hitting this on one test after Maciej's fix: http://build.webkit.org/results/trunk-mac-intel-debug/1868/plugins/root-object-premature-delete-crash-stderr.txt Crash log is at http://build.webkit.org/results/trunk-mac-intel-debug/1868/DumpRenderTree.crash.log. Created attachment 16902 [details]
fix 2
I'm in the middle of testing, but I'm pretty sure this is right.
Comment on attachment 16902 [details]
fix 2
+ if (!strlen(c)) {
Need the O(1) check of c[0] instead of the O(string-length) check of strlen.
I think it would be even better to do this in UString -- guarantee that null and empty both have a precomputed hash. It's annoying to have extra overhead in Identifier::add for 1-time setup.
Darin suggested on IRC that we could write the hash value directly into Rep::null and Rep::empty, and ASSERT in the UString constructor that it's correct. To save time, I haven't done that, but it seems like a prudent change to make later. Committed revision 27153, with the O(n) -> O(1) change. This may have caused platform/mac/fast/AppleScript/date.html to fail on Leopard: http://build.webkit.org/results/Leopard%20Intel%20Release%20(Tests)/r76652%20(27152)/results.html http://build.webkit.org/results/Leopard%20Intel%20Release%20(Tests)/r76653%20(27153)/results.html |