Bug 22148 – nytime.com page crashes browser during load

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

[Chris Dent]

Version 3.1.2 (5525.20.1) and the nightly build just prior.

When loading the above url into a freshly started browser, the application crashes towards the end of the load (most of the layout is done, just getting ready for a good read, and the crash happens).

Consistently repeatable.

[Chris Dent]

If  I disable javascript from the develop menu, the crash does not happen.

[Mark Rowe (bdash)]

Please attach a crash log: <http://webkit.org/quality/crashlogs.html>.

[Matt Lilek]

Confirmed with r38239:

Exception Type:  EXC_BAD_ACCESS (SIGBUS)
Exception Codes: KERN_PROTECTION_FAILURE at 0x0000000000000114
Crashed Thread:  0

Thread 0 Crashed:
0   com.apple.JavaScriptCore      	0x0053dc2f bool WTF::HashTable<void*, std::pair<void*, unsigned int>, WTF::PairFirstExtractor<std::pair<void*, unsigned int> >, WTF::PtrHash<void*>, WTF::PairHashTraits<WTF::HashTraits<void*>, WTF::HashTraits<unsigned int> >, WTF::HashTraits<void*> >::contains<void*, WTF::IdentityHashTranslator<void*, std::pair<void*, unsigned int>, WTF::PtrHash<void*> > >(void* const&) const + 9 (HashTable.h:779)
1   com.apple.JavaScriptCore      	0x0053dc78 WTF::HashTable<void*, std::pair<void*, unsigned int>, WTF::PairFirstExtractor<std::pair<void*, unsigned int> >, WTF::PtrHash<void*>, WTF::PairHashTraits<WTF::HashTraits<void*>, WTF::HashTraits<unsigned int> >, WTF::HashTraits<void*> >::contains(void* const&) const + 24 (HashTable.h:315)
2   com.apple.JavaScriptCore      	0x0053dc96 WTF::HashMap<void*, unsigned int, WTF::PtrHash<void*>, WTF::HashTraits<void*>, WTF::HashTraits<unsigned int> >::contains(void* const&) const + 24 (HashMap.h:173)
3   com.apple.JavaScriptCore      	0x0052fa69 JSC::Machine::cti_vm_throw(void*, ...) + 83 (Machine.cpp:5960)
4   com.apple.JavaScriptCore      	0x00524020 jscGeneratedNativeCode + 0 (Machine.cpp:4296)
5   com.apple.JavaScriptCore      	0x0052afb2 JSC::Machine::execute(JSC::ProgramNode*, JSC::ExecState*, JSC::ScopeChainNode*, JSC::JSObject*, JSC::JSValue**) + 674 (Machine.cpp:934)
6   com.apple.JavaScriptCore      	0x004b358a JSC::Interpreter::evaluate(JSC::ExecState*, JSC::ScopeChain&, JSC::SourceCode const&, JSC::JSValue*) + 438 (Interpreter.cpp:68)
7   com.apple.WebCore             	0x0398b896 WebCore::ScriptController::evaluate(WebCore::String const&, int, WebCore::String const&) + 250 (ScriptController.cpp:111)
8   com.apple.WebCore             	0x0351368c WebCore::FrameLoader::executeScript(WebCore::String const&, int, WebCore::String const&) + 154 (FrameLoader.cpp:792)
9   com.apple.WebCore             	0x035a2054 WebCore::HTMLTokenizer::scriptExecution(WebCore::String const&, WebCore::HTMLTokenizer::State, WebCore::String const&, int) + 300 (HTMLTokenizer.cpp:563)
10  com.apple.WebCore             	0x035a24c3 WebCore::HTMLTokenizer::notifyFinished(WebCore::CachedResource*) + 623 (HTMLTokenizer.cpp:2011)
11  com.apple.WebCore             	0x0335c6ce WebCore::CachedScript::checkNotify() + 86 (CachedScript.cpp:92)
12  com.apple.WebCore             	0x0335c8cb WebCore::CachedScript::data(WTF::PassRefPtr<WebCore::SharedBuffer>, bool) + 279 (CachedScript.cpp:84)
13  com.apple.WebCore             	0x03991351 WebCore::Loader::Host::didFinishLoading(WebCore::SubresourceLoader*) + 413 (loader.cpp:300)
14  com.apple.WebCore             	0x0390c257 WebCore::SubresourceLoader::didFinishLoading() + 169 (SubresourceLoader.cpp:196)
15  com.apple.WebCore             	0x03833332 WebCore::ResourceLoader::didFinishLoading(WebCore::ResourceHandle*) + 24 (ResourceLoader.cpp:399)

[Chris Dent]

Created attachment 25001 [details]
crash log

[Cameron Zwarich (cpst)]

I'll assign this to myself.

[Cameron Zwarich (cpst)]

I can't reproduce this. Can you still reproduce it, Chris?

[Chris Dent]

(In reply to comment #6)
> I can't reproduce this. Can you still reproduce it, Chris?

Yes. I'll attach a new crash report, just to be sure.

I also tried disabling caches, to see if that made any difference. It did not.

[Chris Dent]

Created attachment 25007 [details]
second crash log, for confirmation

[Cameron Zwarich (cpst)]

What is the precise revision of the nightly you are using? I have tried all recent ones, but none seem to crash.

Can you make a web archive (File / Save As...) and see whether loading the web archive crashes? If it does, then upload it to this bug, so that even if nytimes.com changes I can still try it on another machine at work.

[Simon Fraser (smfr)]

It crashes for me; latest nightly (11/9). Mac OS X 10.5.5, intel. Crash at:
Thread 0 Crashed:
0   com.apple.JavaScriptCore      	0x003cae39 JSC::Machine::cti_vm_throw(void*, ...) + 57

[Simon Fraser (smfr)]

In Machine::cti_vm_throw(), codeBlock is 0, so a debug build crashes in:
    ASSERT(codeBlock->ctiReturnAddressVPCMap.contains(ARG_globalData->throwReturnAddress));

[Cameron Zwarich (cpst)]

(In reply to comment #11)
> In Machine::cti_vm_throw(), codeBlock is 0, so a debug build crashes in:
>    
> ASSERT(codeBlock->ctiReturnAddressVPCMap.contains(ARG_globalData->throwReturnAddress));

That's not good. Does it crash with a local web archive copy?

[Simon Fraser (smfr)]

Created attachment 25030 [details]
Webarchive of crashy page

Yes, it crashes when loading from this webarchive.

[Cameron Zwarich (cpst)]

Now that my machine died and I have a loaner, I can reproduce this with the 11/9 nightly. I will try a ToT debug build and see if that works.

[Cameron Zwarich (cpst)]

I have a one line reduction:

function f(a) { f(); } f();

It dies throwing the recursion limit exception. Yes, this was actually happening on a NY Times article, because they load two copies of prototype.js in a weird way.

This was fixed in r38322 by Geoff:

http://trac.webkit.org/changeset/38322

Geoff, do you immediately see why your patch would have fixed this?

I'll make tests for this and similar cases and put them up for review.

[Geoffrey Garen]

> I have a one line reduction:
> 
> function f(a) { f(); } f();

Nice reduction.

> This was fixed in r38322 by Geoff:
> 
> http://trac.webkit.org/changeset/38322
> 
> Geoff, do you immediately see why your patch would have fixed this?

Yes.

The old "throwStackOverflowPreviousFrame" was buggy, but untested. My patch tickled fast/js/global-recursion-on-full-stack.html in a way that illustrated the problem. Since I had to rewrite the logic of throwStackOverflowPreviousFrame anyway, I fixed the bug along the way. Here's what I said in the ChangeLog:

"Fixed a latent bug in stack overflow checking that is now hit because
the register layout has changed a bit -- namely: when throwing a stack
overflow exception inside an op_call helper, we need to account for the
fact that the current call frame is only half-constructed, and use the
parent call frame instead."

[Cameron Zwarich (cpst)]

(In reply to comment #16)
> > Geoff, do you immediately see why your patch would have fixed this?
> 
> Yes.
> 
> The old "throwStackOverflowPreviousFrame" was buggy, but untested. My patch
> tickled fast/js/global-recursion-on-full-stack.html in a way that illustrated
> the problem. Since I had to rewrite the logic of
> throwStackOverflowPreviousFrame anyway, I fixed the bug along the way. Here's
> what I said in the ChangeLog:
> 
> "Fixed a latent bug in stack overflow checking that is now hit because
> the register layout has changed a bit -- namely: when throwing a stack
> overflow exception inside an op_call helper, we need to account for the
> fact that the current call frame is only half-constructed, and use the
> parent call frame instead."

Thanks. I saw that, but I wasn't quite sure at the moment if it applied in this case.

[Cameron Zwarich (cpst)]

Created attachment 25318 [details]
Patch to add tests

I confirmed that the tests added here cause DRT to crash in revisions where this bug existed.

[Geoffrey Garen]

Comment on attachment 25318 [details]
Patch to add tests

r=me

[Cameron Zwarich (cpst)]

Landed in r38624.