After updating sources this morning and rebuilding, a test case for a bug I am working on now asserts in JavaScriptCore: Test site: <http://www.statesman.com/s/sports/> Assertion: ASSERT(m_source->startColumn() > 0); (Line 921 in JavaScriptCore/parser/Parser.h) Call stack: Thread 1Queue : com.apple.main-thread (serial) #0 0x000000010d4aa6da in WTFCrash at /Volumes/Data/Projects/WebKit/OpenSource/Source/WTF/wtf/Assertions.cpp:321 #1 0x000000010cb320b9 in std::__1::unique_ptr<JSC::ProgramNode, std::__1::default_delete<JSC::ProgramNode> > JSC::Parser<JSC::Lexer<unsigned char> >::parse<JSC::ProgramNode>(JSC::ParserError&) at /Volumes/Data/Projects/WebKit/OpenSource/Source/JavaScriptCore/parser/Parser.h:921 #2 0x000000010cb308fb in std::__1::unique_ptr<JSC::ProgramNode, std::__1::default_delete<JSC::ProgramNode> > JSC::parse<JSC::ProgramNode>(JSC::VM*, JSC::SourceCode const&, JSC::FunctionParameters*, JSC::Identifier const&, JSC::JSParserBuiltinMode, JSC::JSParserStrictMode, JSC::JSParserCodeType, JSC::ParserError&, JSC::JSTextPosition*, JSC::ConstructorKind) at /Volumes/Data/Projects/WebKit/OpenSource/Source/JavaScriptCore/parser/Parser.h:998 #3 0x000000010cbdde2f in JSC::UnlinkedProgramCodeBlock* JSC::CodeCache::getGlobalCodeBlock<JSC::UnlinkedProgramCodeBlock, JSC::ProgramExecutable>(JSC::VM&, JSC::ProgramExecutable*, JSC::SourceCode const&, JSC::JSParserBuiltinMode, JSC::JSParserStrictMode, JSC::DebuggerMode, JSC::ProfilerMode, JSC::ParserError&) at /Volumes/Data/Projects/WebKit/OpenSource/Source/JavaScriptCore/runtime/CodeCache.cpp:95 #4 0x000000010cbdd0df in JSC::CodeCache::getProgramCodeBlock(JSC::VM&, JSC::ProgramExecutable*, JSC::SourceCode const&, JSC::JSParserBuiltinMode, JSC::JSParserStrictMode, JSC::DebuggerMode, JSC::ProfilerMode, JSC::ParserError&) at /Volumes/Data/Projects/WebKit/OpenSource/Source/JavaScriptCore/runtime/CodeCache.cpp:125 #5 0x000000010d1477cf in JSC::JSGlobalObject::createProgramCodeBlock(JSC::ExecState*, JSC::ProgramExecutable*, JSC::JSObject**) at /Volumes/Data/Projects/WebKit/OpenSource/Source/JavaScriptCore/runtime/JSGlobalObject.cpp:789 #6 0x000000010cf13c1d in JSC::ProgramExecutable::initializeGlobalProperties(JSC::VM&, JSC::ExecState*, JSC::JSScope*) at /Volumes/Data/Projects/WebKit/OpenSource/Source/JavaScriptCore/runtime/Executable.cpp:496 #7 0x000000010d0a9f73 in JSC::Interpreter::execute(JSC::ProgramExecutable*, JSC::ExecState*, JSC::JSObject*) at /Volumes/Data/Projects/WebKit/OpenSource/Source/JavaScriptCore/interpreter/Interpreter.cpp:830 #8 0x000000010cc07f30 in JSC::evaluate(JSC::ExecState*, JSC::SourceCode const&, JSC::JSValue, JSC::JSValue*) at /Volumes/Data/Projects/WebKit/OpenSource/Source/JavaScriptCore/runtime/Completion.cpp:83 #9 0x000000010fbd99f5 in WebCore::JSMainThreadExecState::evaluate(JSC::ExecState*, JSC::SourceCode const&, JSC::JSValue, JSC::JSValue*) at /Volumes/Data/Projects/WebKit/OpenSource/Source/WebCore/bindings/js/JSMainThreadExecState.h:62 #10 0x000000011061510d in WebCore::ScriptController::evaluateInWorld(WebCore::ScriptSourceCode const&, WebCore::DOMWrapperWorld&) at /Volumes/Data/Projects/WebKit/OpenSource/Source/WebCore/bindings/js/ScriptController.cpp:164 #11 0x0000000110615254 in WebCore::ScriptController::evaluate(WebCore::ScriptSourceCode const&) at /Volumes/Data/Projects/WebKit/OpenSource/Source/WebCore/bindings/js/ScriptController.cpp:180 #12 0x00000001106243d7 in WebCore::ScriptElement::executeScript(WebCore::ScriptSourceCode const&) at /Volumes/Data/Projects/WebKit/OpenSource/Source/WebCore/dom/ScriptElement.cpp:301 #13 0x0000000110623360 in WebCore::ScriptElement::prepareScript(WTF::TextPosition const&, WebCore::ScriptElement::LegacyTypeSupport) at /Volumes/Data/Projects/WebKit/OpenSource/Source/WebCore/dom/ScriptElement.cpp:237 #14 0x000000010f634c06 in WebCore::HTMLScriptRunner::runScript(WebCore::Element*, WTF::TextPosition const&) at /Volumes/Data/Projects/WebKit/OpenSource/Source/WebCore/html/parser/HTMLScriptRunner.cpp:309 #15 0x000000010f634a09 in WebCore::HTMLScriptRunner::execute(WTF::PassRefPtr<WebCore::Element>, WTF::TextPosition const&) at /Volumes/Data/Projects/WebKit/OpenSource/Source/WebCore/html/parser/HTMLScriptRunner.cpp:178 #16 0x000000010f56f5d0 in WebCore::HTMLDocumentParser::runScriptsForPausedTreeBuilder() at /Volumes/Data/Projects/WebKit/OpenSource/Source/WebCore/html/parser/HTMLDocumentParser.cpp:195 #17 0x000000010f56f6d1 in WebCore::HTMLDocumentParser::canTakeNextToken(WebCore::HTMLDocumentParser::SynchronousMode, WebCore::PumpSession&) at /Volumes/Data/Projects/WebKit/OpenSource/Source/WebCore/html/parser/HTMLDocumentParser.cpp:213 #18 0x000000010f56e9e8 in WebCore::HTMLDocumentParser::pumpTokenizer(WebCore::HTMLDocumentParser::SynchronousMode) at /Volumes/Data/Projects/WebKit/OpenSource/Source/WebCore/html/parser/HTMLDocumentParser.cpp:259 #19 0x000000010f56e5c9 in WebCore::HTMLDocumentParser::pumpTokenizerIfPossible(WebCore::HTMLDocumentParser::SynchronousMode) at /Volumes/Data/Projects/WebKit/OpenSource/Source/WebCore/html/parser/HTMLDocumentParser.cpp:166 #20 0x000000010f570719 in WebCore::HTMLDocumentParser::resumeParsingAfterScriptExecution() at /Volumes/Data/Projects/WebKit/OpenSource/Source/WebCore/html/parser/HTMLDocumentParser.cpp:496 #21 0x000000010f570b18 in WebCore::HTMLDocumentParser::notifyFinished(WebCore::CachedResource*) at /Volumes/Data/Projects/WebKit/OpenSource/Source/WebCore/html/parser/HTMLDocumentParser.cpp:536 #22 0x000000010f570b7f in non-virtual thunk to WebCore::HTMLDocumentParser::notifyFinished(WebCore::CachedResource*) at /Volumes/Data/Projects/WebKit/OpenSource/Source/WebCore/html/parser/HTMLDocumentParser.cpp:537 #23 0x000000010ed1dd52 in WebCore::CachedResource::checkNotify() at /Volumes/Data/Projects/WebKit/OpenSource/Source/WebCore/loader/cache/CachedResource.cpp:291 #24 0x000000010ed1de64 in WebCore::CachedResource::finishLoading(WebCore::SharedBuffer*) at /Volumes/Data/Projects/WebKit/OpenSource/Source/WebCore/loader/cache/CachedResource.cpp:307 #25 0x000000010ed3cee1 in WebCore::CachedScript::finishLoading(WebCore::SharedBuffer*) at /Volumes/Data/Projects/WebKit/OpenSource/Source/WebCore/loader/cache/CachedScript.cpp:86 #26 0x00000001108e0495 in WebCore::SubresourceLoader::didFinishLoading(double) at /Volumes/Data/Projects/WebKit/OpenSource/Source/WebCore/loader/SubresourceLoader.cpp:364 #27 0x000000010a25bdac in WebKit::WebResourceLoader::didFinishResourceLoad(double) at /Volumes/Data/Projects/WebKit/OpenSource/Source/WebKit2/WebProcess/Network/WebResourceLoader.cpp:162 #28 0x000000010a261143 in void IPC::callMemberFunctionImpl<WebKit::WebResourceLoader, void (WebKit::WebResourceLoader::*)(double), std::__1::tuple<double>, 0ul>(WebKit::WebResourceLoader*, void (WebKit::WebResourceLoader::*)(double), std::__1::tuple<double>&&, std::index_sequence<0ul>) at /Volumes/Data/Projects/WebKit/OpenSource/Source/WebKit2/Platform/IPC/HandleMessage.h:16 #29 0x000000010a261098 in void IPC::callMemberFunction<WebKit::WebResourceLoader, void (WebKit::WebResourceLoader::*)(double), std::__1::tuple<double>, std::make_index_sequence<1ul> >(std::__1::tuple<double>&&, WebKit::WebResourceLoader*, void (WebKit::WebResourceLoader::*)(double)) at /Volumes/Data/Projects/WebKit/OpenSource/Source/WebKit2/Platform/IPC/HandleMessage.h:22 #30 0x000000010a26054d in void IPC::handleMessage<Messages::WebResourceLoader::DidFinishResourceLoad, WebKit::WebResourceLoader, void (WebKit::WebResourceLoader::*)(double)>(IPC::MessageDecoder&, WebKit::WebResourceLoader*, void (WebKit::WebResourceLoader::*)(double)) at /Volumes/Data/Projects/WebKit/OpenSource/Source/WebKit2/Platform/IPC/HandleMessage.h:92 #31 0x000000010a25fc82 in WebKit::WebResourceLoader::didReceiveWebResourceLoaderMessage(IPC::Connection&, IPC::MessageDecoder&) at /Volumes/Data/Projects/WebKit/OpenSource/WebKitBuild/Debug/DerivedSources/WebKit2/WebResourceLoaderMessageReceiver.cpp:71 #32 0x0000000109b75680 in WebKit::NetworkProcessConnection::didReceiveMessage(IPC::Connection&, IPC::MessageDecoder&) at /Volumes/Data/Projects/WebKit/OpenSource/Source/WebKit2/WebProcess/Network/NetworkProcessConnection.cpp:60 #33 0x0000000109986b33 in IPC::Connection::dispatchMessage(IPC::MessageDecoder&) at /Volumes/Data/Projects/WebKit/OpenSource/Source/WebKit2/Platform/IPC/Connection.cpp:847 #34 0x000000010997ef40 in IPC::Connection::dispatchMessage(std::__1::unique_ptr<IPC::MessageDecoder, std::__1::default_delete<IPC::MessageDecoder> >) at /Volumes/Data/Projects/WebKit/OpenSource/Source/WebKit2/Platform/IPC/Connection.cpp:870 #35 0x000000010998712f in IPC::Connection::dispatchOneMessage() at /Volumes/Data/Projects/WebKit/OpenSource/Source/WebKit2/Platform/IPC/Connection.cpp:898 #36 0x000000010998881d in IPC::Connection::enqueueIncomingMessage(std::__1::unique_ptr<IPC::MessageDecoder, std::__1::default_delete<IPC::MessageDecoder> >)::$_9::operator()() const at /Volumes/Data/Projects/WebKit/OpenSource/Source/WebKit2/Platform/IPC/Connection.cpp:841 #37 0x00000001099887ec in decltype(std::__1::forward<IPC::Connection::enqueueIncomingMessage(std::__1::unique_ptr<IPC::MessageDecoder, std::__1::default_delete<IPC::MessageDecoder> >)::$_9&>(fp)(std::__1::forward<>(fp0))) std::__1::__invoke<IPC::Connection::enqueueIncomingMessage(std::__1::unique_ptr<IPC::MessageDecoder, std::__1::default_delete<IPC::MessageDecoder> >)::$_9&>(IPC::Connection::enqueueIncomingMessage(std::__1::unique_ptr<IPC::MessageDecoder, std::__1::default_delete<IPC::MessageDecoder> >)::$_9&&&) [inlined] at /Applications/Xcode.app/Contents/Developer/Toolchains/OSX10.10.xctoolchain/usr/bin/../include/c++/v1/__functional_base:413 #38 0x00000001099887db in std::__1::__function::__func<IPC::Connection::enqueueIncomingMessage(std::__1::unique_ptr<IPC::MessageDecoder, std::__1::default_delete<IPC::MessageDecoder> >)::$_9, std::__1::allocator<IPC::Connection::enqueueIncomingMessage(std::__1::unique_ptr<IPC::MessageDecoder, std::__1::default_delete<IPC::MessageDecoder> >)::$_9>, void ()>::operator()() at /Applications/Xcode.app/Contents/Developer/Toolchains/OSX10.10.xctoolchain/usr/bin/../include/c++/v1/functional:1370 #39 0x000000010cff345a in std::__1::function<void ()>::operator()() const at /Applications/Xcode.app/Contents/Developer/Toolchains/OSX10.10.xctoolchain/usr/bin/../include/c++/v1/functional:1755 #40 0x000000010d4e5551 in WTF::RunLoop::performWork() at /Volumes/Data/Projects/WebKit/OpenSource/Source/WTF/wtf/RunLoop.cpp:119 #41 0x000000010d4e6724 in WTF::RunLoop::performWork(void*) at /Volumes/Data/Projects/WebKit/OpenSource/Source/WTF/wtf/cf/RunLoopCF.cpp:38 #42 0x00007fff8d0daa01 in __CFRUNLOOP_IS_CALLING_OUT_TO_A_SOURCE0_PERFORM_FUNCTION__ () #43 0x00007fff8d0ccc5c in __CFRunLoopDoSources0 () #44 0x00007fff8d0cc1bf in __CFRunLoopRun () #45 0x00007fff8d0cbbd8 in CFRunLoopRunSpecific () #46 0x00007fff83b6a56f in RunCurrentEventLoopInMode () #47 0x00007fff83b6a2ea in ReceiveNextEventCommon () #48 0x00007fff83b6a12b in _BlockUntilNextEventMatchingListInModeWithFilter () #49 0x00007fff86677a7b in _DPSNextEvent () #50 0x00007fff86677028 in -[NSApplication nextEventMatchingMask:untilDate:inMode:dequeue:] () #51 0x00007fff8666ccb3 in -[NSApplication run] () #52 0x00007fff865e9424 in NSApplicationMain () #53 0x00007fff89ed7958 in _xpc_objc_main () #54 0x00007fff89ed9060 in xpc_main () #55 0x00000001056d8185 in main at /Volumes/Data/Projects/WebKit/OpenSource/Source/WebKit2/Shared/EntryPointUtilities/mac/XPCService/XPCServiceMain.Development.mm:162 #56 0x00007fff8c0345c9 in start ()
*** Bug 142973 has been marked as a duplicate of this bug. ***
I think I caused this in <http://trac.webkit.org/changeset/181810>.
<rdar://problem/20263890>
It looks like the HTML parser sometimes -- including on this site -- produces crazy negative column numbers, and the sanitization that I relaxed in <http://trac.webkit.org/changeset/181810> used to protect us against them.
I don't really like the solution I came up with for attribute event listeners before, so I'll take this opportunity to fix them up a bit.
Also at http://www.theblaze.com/stories/2015/03/25/man-thought-he-had-to-worry-about-people-stealing-his-packages-but-was-surprised-by-what-he-caught-a-mail-carrier-doing-on-camera-instead/ (from bug 143054).
Created attachment 249520 [details] Patch
Attachment 249520 [details] did not pass style-queue: ERROR: Source/WebCore/ChangeLog:8: You should remove the 'No new tests' and either add and list tests, or explain why no new tests were possible. [changelog/nonewtests] [5] Total errors found: 1 in 19 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 249520 [details] Patch Will the line number rebasing require changes to LayoutTests/inspector/debugger/{breakpoint-columns,breakpoint-scope,search-scripts} tests? These use line number in the tests and expected output.
> Will the line number rebasing require changes to > LayoutTests/inspector/debugger/{breakpoint-columns,breakpoint-scope,search- > scripts} tests? These use line number in the tests and expected output. I didn't see any changes in those tests when I ran them. For reasons that are subtle, the old way and the new way are equivalent for most line numbers -- it's just that the new way is more robust and catches some corner cases.
Created attachment 249527 [details] Patch
Added a test specifically covering the behavior change I described in Comment 10.
Created attachment 249528 [details] Patch
Comment on attachment 249528 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=249528&action=review r=me > Source/JavaScriptCore/interpreter/StackVisitor.cpp:337 > + if (codeBlock->ownerExecutable()->hasOverrideLineNo()) > + line = codeBlock->ownerExecutable()->overrideLineNo(); Are the divot details important? If you have a multiline inline attribute, will this provide the expected line for messages/errors for each line? Test: <div onclick="console.log(1); console.log(2); console.log(3);">Click Me</div> > Source/JavaScriptCore/runtime/Executable.h:363 > + void setOverrideLineNo(int overrideLineNo) { m_overrideLineNo = overrideLineNo; } > + bool hasOverrideLineNo() { return m_overrideLineNo != -1; } > + int overrideLineNo() { return m_overrideLineNo;; } Normally we avoid abbreviations. Especially weird ones like "No" instead of "Number", despite the pre-existing case, can we make the new versions "overrideLineNumber"? Nit: The bottom two look like they can be made const. Nit: Double semicolon on line 363. > Source/WebCore/bindings/js/JSLazyEventListener.cpp:109 > + // We want all errors to refer back to the line on which our attribute was > + // declared, regardless of any newlines in our JavaScript source text. > + int overrideLineNo = m_position.m_line.oneBasedInt(); Ahh, it seems your intent is to always be the line of the attribute and not just with an original offset starting from the the line of the attribute. That seems fine.
> Are the divot details important? If you have a multiline inline attribute, > will this provide the expected line for messages/errors for each line? Test: > > <div onclick="console.log(1); > console.log(2); > console.log(3);">Click Me</div> > > Source/WebCore/bindings/js/JSLazyEventListener.cpp:109 > > + // We want all errors to refer back to the line on which our attribute was > > + // declared, regardless of any newlines in our JavaScript source text. > > + int overrideLineNo = m_position.m_line.oneBasedInt(); > > Ahh, it seems your intent is to always be the line of the attribute and not > just with an original offset starting from the the line of the attribute. > That seems fine. Right. It would be nicer to get the exact right line in this multiline case, but that's very tricky because: (1) Newlines in JavaScript source text do not always break a line in the .html file; (2) JavaScript anonymous function rules add their own newlines. So, I think the best we can do for these attributes (and it's pretty good) is just to guarantee that we'll point you back at the location of the attribute.
> > Source/JavaScriptCore/runtime/Executable.h:363 > > + void setOverrideLineNo(int overrideLineNo) { m_overrideLineNo = overrideLineNo; } > > + bool hasOverrideLineNo() { return m_overrideLineNo != -1; } > > + int overrideLineNo() { return m_overrideLineNo;; } > > Normally we avoid abbreviations. Especially weird ones like "No" instead of > "Number", despite the pre-existing case, can we make the new versions > "overrideLineNumber"? Yes. I'll take care of this in a follow-up style patch. > Nit: The bottom two look like they can be made const. > Nit: Double semicolon on line 363. Fixed.
Comment on attachment 249528 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=249528&action=review r=me > Source/WebCore/bindings/js/ScriptController.cpp:281 > + // FIXME: If we are not currently parsing, we should use our current location > + // in JavaScript, to cover cases like "element.setAttribute('click', ...)". > + > + // FIXME: This location maps to the end of the HTML tag, and not to the > + // exact column number belonging to the event handler attribute. Add defects for these FIXME's.
Committed r182034: <http://trac.webkit.org/changeset/182034>