Summary: | nytime.com page crashes browser during load | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Chris Dent <chris.dent> | ||||||||||
Component: | JavaScriptCore | Assignee: | Cameron Zwarich (cpst) <zwarich> | ||||||||||
Status: | RESOLVED FIXED | ||||||||||||
Severity: | Major | CC: | chris.dent, ggaren, mihnea, simon.fraser, zwarich | ||||||||||
Priority: | P1 | Keywords: | NeedsReduction | ||||||||||
Version: | 528+ (Nightly build) | ||||||||||||
Hardware: | Mac (Intel) | ||||||||||||
OS: | OS X 10.5 | ||||||||||||
URL: | http://www.nytimes.com/2008/11/10/health/10heart.html?_r=1&partner=rssnyt&emc=rss&oref=slogin | ||||||||||||
Attachments: |
|
Description
Chris Dent
2008-11-09 08:17:11 PST
If I disable javascript from the develop menu, the crash does not happen. Please attach a crash log: <http://webkit.org/quality/crashlogs.html>. 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) Created attachment 25001 [details]
crash log
I'll assign this to myself. I can't reproduce this. Can you still reproduce it, Chris? (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. Created attachment 25007 [details]
second crash log, for confirmation
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. 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 In Machine::cti_vm_throw(), codeBlock is 0, so a debug build crashes in: ASSERT(codeBlock->ctiReturnAddressVPCMap.contains(ARG_globalData->throwReturnAddress)); (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? Created attachment 25030 [details]
Webarchive of crashy page
Yes, it crashes when loading from this webarchive.
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. 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. > 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." (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. 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.
Comment on attachment 25318 [details]
Patch to add tests
r=me
Landed in r38624. |