Bug 20719 - REGRESSION (r36135-36244): Hangs, then crashes after several seconds
Summary: REGRESSION (r36135-36244): Hangs, then crashes after several seconds
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 528+ (Nightly build)
Hardware: Mac OS X 10.5
: P2 Normal
Assignee: Cameron Zwarich (cpst)
URL: http://politiken.dk
Keywords: InRadar, NeedsReduction
Depends on:
Blocks:
 
Reported: 2008-09-08 05:53 PDT by Morten Norby Larsen
Modified: 2008-09-09 01:07 PDT (History)
3 users (show)

See Also:


Attachments
Crash log (28.91 KB, text/plain)
2008-09-08 05:55 PDT, Morten Norby Larsen
no flags Details
Sample output (11.57 KB, text/plain)
2008-09-08 05:55 PDT, Morten Norby Larsen
no flags Details
partial reduction - will crash eventually (6.03 KB, text/html)
2008-09-08 07:22 PDT, Gavin Sherlock
no flags Details
better reduction (336 bytes, text/plain)
2008-09-08 10:25 PDT, Gavin Sherlock
no flags Details
sample (38.16 KB, text/plain)
2008-09-08 10:26 PDT, Gavin Sherlock
no flags Details
Self-contained test case (49.13 KB, text/html)
2008-09-08 11:05 PDT, Morten Norby Larsen
no flags Details
Even more reduced file, that does NOT crash (48.98 KB, text/html)
2008-09-08 11:11 PDT, Morten Norby Larsen
no flags Details
Reduction (77.10 KB, text/html)
2008-09-08 21:22 PDT, Cameron Zwarich (cpst)
no flags Details
Further reduction (84 bytes, text/html)
2008-09-08 22:43 PDT, Cameron Zwarich (cpst)
no flags Details
Proposed patch (5.04 KB, patch)
2008-09-09 00:40 PDT, Cameron Zwarich (cpst)
mrowe: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Morten Norby Larsen 2008-09-08 05:53:30 PDT
Webkit build 36247 hangs, then crashes when visiting http://politiken.dk .

To reproduce:

0. go to http://politiken.dk
1. Wait till the progress bar in the URI field is around 80%
2. The browser now becomes unresponsive and after a while the cursor turns into a beach ball.
3. After a while it crashes in this thread:

Thread 0 Crashed:
0   libSystem.B.dylib             	0x9718eb9e __kill + 10
1   libSystem.B.dylib             	0x97205ec2 raise + 26
2   libSystem.B.dylib             	0x9721547f abort + 73

Crash report and sample output attached.

The crash may happen around the time when ads are document.writ'ten into the doc, so result may vary based on ads served.
Comment 1 Morten Norby Larsen 2008-09-08 05:55:05 PDT
Created attachment 23248 [details]
Crash log
Comment 2 Morten Norby Larsen 2008-09-08 05:55:57 PDT
Created attachment 23249 [details]
Sample output
Comment 3 Gavin Sherlock 2008-09-08 06:16:20 PDT
I can confirm this, with the same crash log.  It doesn't occur in Safari 3.1.2, so is a regression, despite the crash itself being in libSystem.
Comment 4 Gavin Sherlock 2008-09-08 06:32:27 PDT
Regressed between r36135 (good) and r36244 (bad), which is a fairly large range of commits, including (as the final commit in the range) the squirrelfish-extreme merge.  I don't have privileges to change the title of the bug to reflect this.
Comment 5 Gavin Sherlock 2008-09-08 07:22:04 PDT
Created attachment 23256 [details]
partial reduction - will crash eventually

Still relies on external resources.

Key part is:

<div id="tw_toplist_widget">
</div>

removing that removes  the hang and crash.
Comment 6 Gavin Sherlock 2008-09-08 10:25:10 PDT
Created attachment 23264 [details]
better reduction

Still relies on one external (horribly obfuscated) javascript file
Comment 7 Gavin Sherlock 2008-09-08 10:26:40 PDT
Created attachment 23265 [details]
sample

more instructive sample when opening latest reduction.  Lots of invalid frame pointer errors - the problem may be in javascript regexes, but I'm not sure, and not expert enough to further reduce the javascript.
Comment 8 Morten Norby Larsen 2008-09-08 11:05:37 PDT
Created attachment 23267 [details]
Self-contained test case

Great work on the reduction - let me just turn it into a self-contained one, by copying the contents of the external JS file into a <script>-tag and deleting the old tag with the href.

This one crashes for me, after beachballing and rendering the whole OS unresponsive for several seconds.
Comment 9 Morten Norby Larsen 2008-09-08 11:07:21 PDT
For anyone concerned with copyright out there, let me just add that I have informed twingly.com, and asked them to let me know if they can't accept it.
Comment 10 Morten Norby Larsen 2008-09-08 11:11:33 PDT
Created attachment 23269 [details]
Even more reduced file, that does NOT crash

One last interesting tidbit: If you remove the html,head, and body tags in the last test case, you don't get beachballs, nor crashes. See test3_no_body.html
Comment 11 Cameron Zwarich (cpst) 2008-09-08 17:30:11 PDT
I can confirm this, and I will assign it to myself. Given that the bulk of this range of revisions is the squirrelfish-extreme merge, this is probably due to squirrelfish-extreme.
Comment 12 Mark Rowe (bdash) 2008-09-08 18:02:34 PDT
The crash is due to a memory allocation failing, in which case we call abort.  This hang and crash does not occur with WREC disabled, so this is definitely a regression due to SquirrelFish Extreme.
Comment 13 Cameron Zwarich (cpst) 2008-09-08 18:55:46 PDT
With WREC enabled, it keeps on trying to match the regexp /\b\w+\b/ against the same long string of obfuscated JavaScript. I'll try to reduce it.
Comment 14 Mark Rowe (bdash) 2008-09-08 18:56:56 PDT
<rdar://problem/6205787>
Comment 15 Cameron Zwarich (cpst) 2008-09-08 21:22:33 PDT
Created attachment 23287 [details]
Reduction

So, it turns out that the matching of the JS file works fine, it just took so long that with printf debugging I thought that it was hanging there. The problem is with the unpacked code itself. Here's a reduction containing that code. I'll try to reduce it further from here.
Comment 16 Cameron Zwarich (cpst) 2008-09-08 22:43:17 PDT
Created attachment 23288 [details]
Further reduction

Here's the problem:

print("PASS".replace(/\u00E5/ig, "%C3%A5"));

It replaces every character with the string. If I remove the 'i' (for case insensitivity), it works fine. This should be an easy fix.

It was hanging the browser because it does a number of these URL escaping substitutions in a row, and since the escaped version is quite a bit larger than a single character, the string size grows exponentially and causes a crash.
Comment 17 Cameron Zwarich (cpst) 2008-09-09 00:40:13 PDT
Created attachment 23289 [details]
Proposed patch
Comment 18 Mark Rowe (bdash) 2008-09-09 00:43:35 PDT
Comment on attachment 23289 [details]
Proposed patch

r=me
Comment 19 Cameron Zwarich (cpst) 2008-09-09 01:07:27 PDT
Landed in r36288.