Bug 118996 - [Windows] Parser asserts because sourceOffset != UINT_MAX
Summary: [Windows] Parser asserts because sourceOffset != UINT_MAX
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC All
: P2 Critical
Assignee: Mark Lam
URL:
Keywords: InRadar
: 119017 (view as bug list)
Depends on:
Blocks:
 
Reported: 2013-07-22 17:02 PDT by Brent Fulgham
Modified: 2013-07-29 16:49 PDT (History)
10 users (show)

See Also:


Attachments
the patch (8.57 KB, patch)
2013-07-23 09:39 PDT, Mark Lam
ggaren: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Brent Fulgham 2013-07-22 17:02:55 PDT
Recent changes in the JSC parser are triggering an assertion in Windows because the sourceOffset is getting set to UINT_MAX.

Source location:
    JSTokenLocation(const JSTokenLocation& location)
    {
        line = location.line;
        lineStartOffset = location.lineStartOffset;
        startOffset = location.startOffset;
        endOffset = location.endOffset;
        sourceOffset = location.sourceOffset;
>>>     ASSERT(sourceOffset != UINT_MAX);
    }


Local state:
-location	{line=3 lineStartOffset=0 startOffset=341 ...}	const JSC::JSTokenLocation &
	line	3	int
	lineStartOffset	0	unsigned int
	startOffset	341	unsigned int
	endOffset	342	unsigned int
	sourceOffset	4294967295	unsigned int
	location.sourceOffset	4294967295	unsigned int
	sourceOffset	4294967295	unsigned int
-this	0x000cca20 {line=3 lineStartOffset=0 startOffset=341 ...}	JSC::JSTokenLocation * const
	line	3	int
	lineStartOffset	0	unsigned int
	startOffset	341	unsigned int
	endOffset	342	unsigned int
	sourceOffset	4294967295	unsigned int

Callstack:
 	WTF.dll!WTFCrash()  Line 339	C++
>	JavaScriptCore.dll!JSC::JSTokenLocation::JSTokenLocation(const JSC::JSTokenLocation & location)  Line 189 + 0x37 bytes	C++
 	JavaScriptCore.dll!JSC::JSToken::JSToken(const JSC::JSToken & __that)  + 0x4c bytes	C++
 	JavaScriptCore.dll!JSC::SourceProviderCacheItem::closeBraceToken()  Line 70 + 0xc bytes	C++
 	JavaScriptCore.dll!JSC::Parser<JSC::Lexer<unsigned char> >::parseFunctionInfo<0,0,JSC::ASTBuilder>(JSC::ASTBuilder & context, const JSC::Identifier * & name, JSC::ParameterNode * & parameters, JSC::FunctionBodyNode * & body, unsigned int & openBraceOffset, unsigned int & closeBraceOffset, int & bodyStartLine, unsigned int & bodyStartColumn)  Line 847 + 0xf bytes	C++
 	JavaScriptCore.dll!JSC::Parser<JSC::Lexer<unsigned char> >::parseMemberExpression<JSC::ASTBuilder>(JSC::ASTBuilder & context)  Line 1603 + 0x2b bytes	C++
 	JavaScriptCore.dll!JSC::Parser<JSC::Lexer<unsigned char> >::parseUnaryExpression<JSC::ASTBuilder>(JSC::ASTBuilder & context)  Line 1698 + 0xc bytes	C++
 	JavaScriptCore.dll!JSC::Parser<JSC::Lexer<unsigned char> >::parseBinaryExpression<JSC::ASTBuilder>(JSC::ASTBuilder & context)  Line 1214 + 0xc bytes	C++
 	JavaScriptCore.dll!JSC::Parser<JSC::Lexer<unsigned char> >::parseConditionalExpression<JSC::ASTBuilder>(JSC::ASTBuilder & context)  Line 1175 + 0xc bytes	C++
 	JavaScriptCore.dll!JSC::Parser<JSC::Lexer<unsigned char> >::parseAssignmentExpression<JSC::ASTBuilder>(JSC::ASTBuilder & context)  Line 1116 + 0xc bytes	C++
 	JavaScriptCore.dll!JSC::Parser<JSC::Lexer<unsigned char> >::parseVarDeclarationList<JSC::ASTBuilder>(JSC::ASTBuilder & context, int & declarations, const JSC::Identifier * & lastIdent, JSC::ExpressionNode * & lastInitializer, JSC::JSTextPosition & identStart, JSC::JSTextPosition & initStart, JSC::JSTextPosition & initEnd)  Line 291 + 0xc bytes	C++
 	JavaScriptCore.dll!JSC::Parser<JSC::Lexer<unsigned char> >::parseVarDeclaration<JSC::ASTBuilder>(JSC::ASTBuilder & context)  Line 205 + 0x24 bytes	C++
 	JavaScriptCore.dll!JSC::Parser<JSC::Lexer<unsigned char> >::parseStatement<JSC::ASTBuilder>(JSC::ASTBuilder & context, const JSC::Identifier * & directive, unsigned int * directiveLiteralLength)  Line 711 + 0xc bytes	C++
 	JavaScriptCore.dll!JSC::Parser<JSC::Lexer<unsigned char> >::parseSourceElements<1,JSC::ASTBuilder>(JSC::ASTBuilder & context)  Line 169 + 0x14 bytes	C++
 	JavaScriptCore.dll!JSC::Parser<JSC::Lexer<unsigned char> >::parseBlockStatement<JSC::ASTBuilder>(JSC::ASTBuilder & context)  Line 692 + 0xc bytes	C++
 	JavaScriptCore.dll!JSC::Parser<JSC::Lexer<unsigned char> >::parseStatement<JSC::ASTBuilder>(JSC::ASTBuilder & context, const JSC::Identifier * & directive, unsigned int * directiveLiteralLength)  Line 709 + 0xc bytes	C++
 	JavaScriptCore.dll!JSC::Parser<JSC::Lexer<unsigned char> >::parseSourceElements<0,JSC::ASTBuilder>(JSC::ASTBuilder & context)  Line 169 + 0x14 bytes	C++
 	JavaScriptCore.dll!JSC::Parser<JSC::Lexer<unsigned char> >::parseInner()  Line 116 + 0xf bytes	C++
 	JavaScriptCore.dll!JSC::Parser<JSC::Lexer<unsigned char> >::parse<JSC::FunctionBodyNode>(JSC::ParserError & error)  Line 1018	C++
 	JavaScriptCore.dll!JSC::parse<JSC::FunctionBodyNode>(JSC::VM * vm, const JSC::SourceCode & source, JSC::FunctionParameters * parameters, const JSC::Identifier & name, JSC::JSParserStrictness strictness, JSC::JSParserMode parserMode, JSC::ParserError & error)  Line 1084 + 0x13 bytes	C++
 	JavaScriptCore.dll!JSC::generateFunctionCodeBlock(JSC::VM & vm, JSC::JSScope * scope, JSC::UnlinkedFunctionExecutable * executable, const JSC::SourceCode & source, JSC::CodeSpecializationKind kind, JSC::DebuggerMode debuggerMode, JSC::ProfilerMode profilerMode, JSC::ParserError & error)  Line 52 + 0x3b bytes	C++
 	JavaScriptCore.dll!JSC::UnlinkedFunctionExecutable::codeBlockFor(JSC::VM & vm, JSC::JSScope * scope, const JSC::SourceCode & source, JSC::CodeSpecializationKind specializationKind, JSC::DebuggerMode debuggerMode, JSC::ProfilerMode profilerMode, JSC::ParserError & error)  Line 161 + 0x25 bytes	C++
 	JavaScriptCore.dll!JSC::FunctionExecutable::produceCodeBlockFor(JSC::JSScope * scope, JSC::CodeSpecializationKind specializationKind, JSC::JSObject * & exception)  Line 503 + 0x31 bytes	C++
 	JavaScriptCore.dll!JSC::FunctionExecutable::compileForCallInternal(JSC::ExecState * exec, JSC::JSScope * scope, JSC::JITCode::JITType jitType, unsigned int bytecodeIndex)  Line 533 + 0x16 bytes	C++
 	JavaScriptCore.dll!JSC::FunctionExecutable::compileForCall(JSC::ExecState * exec, JSC::JSScope * scope)  Line 612 + 0x18 bytes	C++
 	JavaScriptCore.dll!JSC::FunctionExecutable::compileFor(JSC::ExecState * exec, JSC::JSScope * scope, JSC::CodeSpecializationKind kind)  Line 670 + 0x10 bytes	C++
 	JavaScriptCore.dll!JSC::lazyLinkFor(JSC::ExecState * callFrame, JSC::CodeSpecializationKind kind)  Line 2273 + 0x19 bytes	C++
 	JavaScriptCore.dll!cti_vm_lazyLinkCall(void * * args)  Line 2298 + 0xb bytes	C++
 	JavaScriptCore.dll!@cti_op_create_this@4()  + 0x17f bytes	C++
 	JavaScriptCore.dll!JSC::JITCode::execute(JSC::JSStack * stack, JSC::ExecState * callFrame, JSC::VM * vm)  Line 135 + 0x29 bytes	C++
 	JavaScriptCore.dll!JSC::Interpreter::execute(JSC::ProgramExecutable * program, JSC::ExecState * callFrame, JSC::JSObject * thisObj)  Line 951 + 0x28 bytes	C++
 	JavaScriptCore.dll!JSC::evaluate(JSC::ExecState * exec, const JSC::SourceCode & source, JSC::JSValue thisValue, JSC::JSValue * returnedException)  Line 85	C++
 	WebKit.dll!WebCore::JSMainThreadExecState::evaluate(JSC::ExecState * exec, const JSC::SourceCode & source, JSC::JSValue thisValue, JSC::JSValue * exception)  Line 77 + 0x1e bytes	C++
 	WebKit.dll!WebCore::ScriptController::evaluateInWorld(const WebCore::ScriptSourceCode & sourceCode, WebCore::DOMWrapperWorld * world)  Line 142 + 0x23 bytes	C++
 	WebKit.dll!WebCore::ScriptController::evaluate(const WebCore::ScriptSourceCode & sourceCode)  Line 158 + 0x16 bytes	C++
 	WebKit.dll!WebCore::ScriptElement::executeScript(const WebCore::ScriptSourceCode & sourceCode)  Line 316 + 0x17 bytes	C++
 	WebKit.dll!WebCore::HTMLScriptRunner::executePendingScriptAndDispatchEvent(WebCore::PendingScript & pendingScript)  Line 151	C++
 	WebKit.dll!WebCore::HTMLScriptRunner::executeParsingBlockingScript()  Line 123	C++
 	WebKit.dll!WebCore::HTMLScriptRunner::executeParsingBlockingScripts()  Line 201 + 0x8 bytes	C++
 	WebKit.dll!WebCore::HTMLScriptRunner::executeScriptsWaitingForLoad(WebCore::CachedResource * cachedScript)  Line 211	C++
 	WebKit.dll!WebCore::HTMLDocumentParser::notifyFinished(WebCore::CachedResource * cachedResource)  Line 935	C++
 	WebKit.dll!WebCore::CachedResource::checkNotify()  Line 369 + 0x11 bytes	C++
 	WebKit.dll!WebCore::CachedResource::finishLoading(WebCore::ResourceBuffer * __formal)  Line 386	C++
 	WebKit.dll!WebCore::CachedScript::finishLoading(WebCore::ResourceBuffer * data)  Line 90	C++
 	WebKit.dll!WebCore::SubresourceLoader::didFinishLoading(double finishTime)  Line 284	C++
 	WebKit.dll!WebCore::ResourceLoader::didFinishLoading(WebCore::ResourceHandle * __formal, double finishTime)  Line 489	C++
 	WebKit.dll!WebCore::didFinishLoading(_CFURLConnection * conn, const void * clientInfo)  Line 263	C++
 	CFNetwork.dll!URLConnectionClient::_clientDidFinishLoading(URLConnectionClient::ClientConnectionEventQueue * preQ)  Line 1739 + 0x13 bytes	C++
 	CFNetwork.dll!URLConnectionClient::ClientConnectionEventQueue::processAllEventsAndConsumePayload(XConnectionEventInfo<enum XClientEvent,XClientEventParams> * e, long count)  Line 2256	C++
 	CFNetwork.dll!URLConnectionClient::ClientConnectionEventQueue::processAllEventsAndConsumePayload(XConnectionEventInfo<enum XClientEvent,XClientEventParams> * e, long count)  Line 2328 + 0x9 bytes	C++
 	CFNetwork.dll!XConnectionEventQueue<enum XClientEvent,XClientEventParams>::processAllEvents()  Line 231	C++
 	CFNetwork.dll!URLConnectionClient::processEvents()  Line 362	C++
 	CFNetwork.dll!MultiplexerSource::perform()  Line 229	C++
 	CoreFoundation.dll!__CFRunLoopDoSources0(__CFRunLoop * rl, __CFRunLoopMode * rlm, unsigned char stopAfterHandle)  Line 41778 + 0xd bytes	C++
 	CoreFoundation.dll!__CFRunLoopRun(__CFRunLoop * rl, __CFRunLoopMode * rlm, double seconds, unsigned char stopAfterHandle, __CFRunLoopMode * previousMode)  Line 42215 + 0xb bytes	C++
 	CoreFoundation.dll!CFRunLoopRunSpecific(__CFRunLoop * rl, const __CFString * modeName, double seconds, unsigned char returnAfterSourceHandled)  Line 42411 + 0x12 bytes	C++
 	CoreFoundation.dll!CFRunLoopRun()  Line 42438 + 0x1d bytes	C++
 	WinLauncher.dll!100022e4() 	
 	[Frames below may be incorrect and/or missing, no symbols loaded for WinLauncher.dll]	
 	WinLauncher.exe!wWinMain(HINSTANCE__ * hInstance, HINSTANCE__ * hPrevInstance, wchar_t * lpstrCmdLine, int nCmdShow)  Line 211 + 0x16 bytes	C++
 	WinLauncher.exe!__tmainCRTStartup()  Line 547 + 0x1c bytes	C
 	kernel32.dll!@BaseThreadInitThunk@12()  + 0xe bytes	
 	ntdll.dll!___RtlUserThreadStart@8()  + 0x27 bytes	
 	ntdll.dll!__RtlUserThreadStart@8()  + 0x1b bytes
Comment 1 Mark Lam 2013-07-23 09:39:46 PDT
Created attachment 207334 [details]
the patch

This bug reports an assertion regarding the value of the sourceOffset field in JSTokenLocation.  Conveniently, that field is no longer needed.  So, we can simply remove it and clean up the code, thereby also removing the assert (which is now moot).

Some background history regarding the sourceOffset field in JSTokenLocation:

The JSTokenLocation sourceOffset field was added in http://trac.webkit.org/changeset/152494 to support some transitional code while that change was being developed.  When all the dust settled, the sourceOffset field was no longer needed (as evident by the fact that the JSTokenLocation "position" methods are not called from anywhere).  The sourceOffset field and associated artifacts should have been removed.

This patch cleans that up.  The patch builds cleanly against trunk r152583 and passes all the javascriptcore tests and relevant layout tests.  Will commence with running the full layout tests after this.  The patch is ready for review.
Comment 2 Brent Fulgham 2013-07-23 09:43:37 PDT
I'm glad that the assert will be removed by this change, but was there some underlying problem in the parser that was causing the assert to trigger?

I'm applying the patch and will report back once the build and test is finished.
Comment 3 Mark Lam 2013-07-23 09:47:04 PDT
(In reply to comment #2)
> I'm glad that the assert will be removed by this change, but was there some underlying problem in the parser that was causing the assert to trigger?
> 
> I'm applying the patch and will report back once the build and test is finished.

The sourceOffset value was not always available at the time that the JSTokenLocation is constructed.  Proper usage dictates that the user of JSTokenLocation properly initializes the sourceOffset field before copying the JSTokenLocation into another.  That was what the assertion was catching.  This condition of use must not have been satisfied in some code path that manifests only on the Windows port.

However, since the field is actually no longer needed, I didn't bother to chase down where it wasn't getting initialized.
Comment 4 Brent Fulgham 2013-07-23 10:25:37 PDT
(In reply to comment #2)
> I'm applying the patch and will report back once the build and test is finished.

The patch applies cleanly, and resolves the assertion that was preventing the Windows debug bots from running.
Comment 5 Geoffrey Garen 2013-07-23 10:41:00 PDT
Comment on attachment 207334 [details]
the patch

r=me
Comment 6 Mark Lam 2013-07-23 10:50:10 PDT
*** Bug 119017 has been marked as a duplicate of this bug. ***
Comment 7 Mark Lam 2013-07-23 17:41:27 PDT
Thanks for the review.  There are no new layout test failures with this patch.  Landed in r153071: <http://trac.webkit.org/changeset/153071>.
Comment 8 Radar WebKit Bug Importer 2013-07-29 16:49:51 PDT
<rdar://problem/14583789>