Bug 37661 – [v8] Bail out if fetching of Object.prototype fails

• Component: WebCore Misc.
• Status: RESOLVED FIXED

[anton muhin]

[v8] Bail out if fetching of Object.prototype fails

[anton muhin]

Most probably that's due to empty handle of the value to set---receiver and keys are checked many times on the path so we would die earlier.

I don't know exact reason why fetching Object.prototype fails.  Most probably some exception is thrown (if it were due to IsDeadCheck, we shouldn't get past first line of v8::SetHiddenPrototype).

If we have some logging mechanism, we could try to see if it's indeed the case: we could add TryCatch and store exception somewhere on the stack to have a chance to read it from minidump.

For now adding some more bailout checks, let's see if it could help.

[anton muhin]

Created attachment 53446 [details]
Patch

[Adam Barth]

Comment on attachment 53446 [details]
Patch

The ChangeLog doesn't explain why we're making this change.  You bug comments give some hint, but they don't explain what problem we're trying to solve.  Is there a crash?  Can we make a test case for it?

[anton muhin]

Sorry, I should have been more explicit.

It's a crash in Chromium.  Sample stack trace:

0x025ec87a	 [chrome.dll	 - handles.cc:217]	v8::internal::SetProperty(v8::internal::Handle<v8::internal::JSObject>,v8::internal::Handle<v8::internal::String>,v8::internal::Handle<v8::internal::Object>,PropertyAttributes)
0x0265d733	 [chrome.dll	 - runtime.cc:3799]	v8::internal::Runtime::SetObjectProperty(v8::internal::Handle<v8::internal::Object>,v8::internal::Handle<v8::internal::Object>,v8::internal::Handle<v8::internal::Object>,PropertyAttributes)
0x025e9bb5	 [chrome.dll	 - objects.cc:1416]	v8::internal::JSObject::SetPropertyPostInterceptor(v8::internal::String *,v8::internal::Object *,PropertyAttributes)
0x025ec9fa	 [chrome.dll	 - handles.cc:226]	v8::internal::SetProperty(v8::internal::Handle<v8::internal::Object>,v8::internal::Handle<v8::internal::Object>,v8::internal::Handle<v8::internal::Object>,PropertyAttributes)
0x025c7cd2	 [chrome.dll	 - api.cc:2393]	v8::Object::SetHiddenValue(v8::Handle<v8::String>,v8::Handle<v8::Value>)
0x025cb321	 [chrome.dll	 - api.cc:2083]	v8::Object::Get(v8::Handle<v8::Value>)
0x025ccbff	 [chrome.dll	 - api.cc:3069]	v8::Context::Global()
0x01d8a5f0	 [chrome.dll	 - v8domwindowshell.cpp:518]	WebCore::V8DOMWindowShell::installHiddenObjectPrototype(v8::Handle<v8::Context>)
0x01d89ffe	 [chrome.dll	 - v8domwindowshell.cpp:289]	WebCore::V8DOMWindowShell::initContextIfNeeded()
0x01d8c66e	 [chrome.dll	 - v8proxy.cpp:279]	WebCore::V8Proxy::evaluateInIsolatedWorld(int,WTF::Vector<WebCore::ScriptSourceCode,0> const &,int)
0x01e89d98	 [chrome.dll	 - webframeimpl.cpp:736]	WebKit::WebFrameImpl::executeScriptInIsolatedWorld(int,WebKit::WebScriptSource const *,unsigned int,int)
0x01ffd199	 [chrome.dll	 - user_script_slave.cc:219]	UserScriptSlave::InjectScripts(WebKit::WebFrame *,UserScript::RunLocation)
0x01fee20c	 [chrome.dll	 - render_view.cc:2658]	RenderView::didFinishDocumentLoad(WebKit::WebFrame *)
0x01e943db	 [chrome.dll	 - frameloaderclientimpl.cpp:391]	WebKit::FrameLoaderClientImpl::dispatchDidFinishDocumentLoad()
0x01c6c37e	 [chrome.dll	 - frameloader.cpp:1076]	WebCore::FrameLoader::finishedParsing()
0x01cc243a	 [chrome.dll	 - document.cpp:4203]	WebCore::Document::finishedParsing()
0x01e401a8	 [chrome.dll	 - htmlparser.cpp:1666]	WebCore::HTMLParser::finished()
0x01dc66a7	 [chrome.dll	 - htmltokenizer.cpp:1870]	WebCore::HTMLTokenizer::end()
0x01dc654f	 [chrome.dll	 - htmltokenizer.cpp:1811]	WebCore::HTMLTokenizer::write(WebCore::SegmentedString const &,bool)
0x01dc661d	 [chrome.dll	 - htmltokenizer.cpp:1848]	WebCore::HTMLTokenizer::timerFired(WebCore::Timer<WebCore::HTMLTokenizer> *)
0x01e07a0f	 [chrome.dll	 - timer.h:98]	WebCore::Timer<WebCore::Scrollbar>::fired()
0x01d2334b	 [chrome.dll	 - threadtimers.cpp:112]	WebCore::ThreadTimers::sharedTimerFiredInternal()
0x01d232be	 [chrome.dll	 - threadtimers.cpp:90]	WebCore::ThreadTimers::sharedTimerFired()
0x01fc2b6f	 [chrome.dll	 - message_loop.cc:329]	MessageLoop::RunTask(Task *)
0x01fc2bac	 [chrome.dll	 - message_loop.cc:337]	MessageLoop::DeferOrRunPendingTask(MessageLoop::PendingTask const &)
0x01fc2d42	 [chrome.dll	 - message_loop.cc:444]	MessageLoop::DoWork()
0x01fd321f	 [chrome.dll	 - message_pump_default.cc:50]	base::MessagePumpDefault::Run(base::MessagePump::Delegate *)
0x01fc2a1a	 [chrome.dll	 - message_loop.cc:205]	MessageLoop::RunInternal()
0x01fc299f	 [chrome.dll	 - message_loop.cc:177]	MessageLoop::RunHandler()
0x01fc294d	 [chrome.dll	 - message_loop.cc:155]	MessageLoop::Run()
0x01fdd2fe	 [chrome.dll	 - renderer_main.cc:289]	RendererMain(MainFunctionParams const &)
0x01c33bb9	 [chrome.dll	 - chrome_dll_main.cc:716]	ChromeMain
0x004033ec	 [chrome.exe	 - client_util.cc:195]	MainDllLoader::Launch(HINSTANCE__ *,sandbox::SandboxInterfaceInfo *)
0x00403a72	 [chrome.exe	 - chrome_exe_main.cc:46]	wWinMain
0x00445dce	 [chrome.exe	 - crt0.c:263]	__tmainCRTStartup
0x7c816fd6	 [kernel32.dll	 + 0x00016fd6]	BaseProcessStart

The analysis still apply: it's almost for sure empty handle of value being set.

Alas, I don't have an easy repro case.

[Adam Barth]

Comment on attachment 53446 [details]
Patch

Ok. Please add more details to the change log before landing.

[anton muhin]

Created attachment 53822 [details]
Better ChangeLog

[anton muhin]

(In reply to comment #6)
> Created an attachment (id=53822) [details]
> Better ChangeLog

Adam, is this wording enough?

[Adam Barth]

Comment on attachment 53822 [details]
Better ChangeLog

It's still pretty weak.  I don't want to hold up your patch longer, but someone looking at this change in the future won't understand why we made the change without referring to this bug.

[anton muhin]

Comment on attachment 53822 [details]
Better ChangeLog

Thanks a lot, Adam

[WebKit Commit Bot]

Comment on attachment 53822 [details]
Better ChangeLog

Clearing flags on attachment: 53822

Committed r57991: <http://trac.webkit.org/changeset/57991>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.