Bug 50816

Summary: REGRESSION: Hang inside Yarr::RegexCodeBlock::execute when visiting bugs.webkit.org
Product: WebKit Reporter: Adam Roben (:aroben) <aroben>
Component: JavaScriptCoreAssignee: Michael Saboff <msaboff>
Status: RESOLVED FIXED    
Severity: Normal CC: barraclough, ggaren, msaboff
Priority: P2 Keywords: InRadar, Regression
Version: 528+ (Nightly build)   
Hardware: PC   
OS: Windows XP   
URL: http://bugs.webkit.org/
Attachments:
Description Flags
Patch to set default backtrack destination for first parens in nested parens
barraclough: review+
Updated Patch to handle the first nested paren, not just when the first nested term is a paren barraclough: review+

Description Adam Roben (:aroben) 2010-12-10 07:16:25 PST
To reproduce:

1. Go to http://bugs.webkit.org/

You'll hang. Here's the backtrace:


 	06e50e26()	
>	JavaScriptCore.dll!JSC::Yarr::RegexCodeBlock::execute(const wchar_t * input=0x0785a944, unsigned int start=0, unsigned int length=11, int * output=0x054f1dd4)  Line 67 + 0x1c bytes	C++
 	JavaScriptCore.dll!JSC::Yarr::executeRegex(JSC::Yarr::RegexCodeBlock & jitObject={...}, const wchar_t * input=0x0785a944, unsigned int start=0, unsigned int length=11, int * output=0x054f1dd4)  Line 84	C++
 	JavaScriptCore.dll!JSC::RegExp::match(const JSC::UString & s={Committer):}, int startOffset=0, WTF::Vector<int,32> * ovector=[10](0,1,0,85,-1,85,0,0,-1,16))  Line 139 + 0x2b bytes	C++
 	JavaScriptCore.dll!JSC::RegExpConstructor::performMatch(JSC::RegExp * r=0x055a4d00, const JSC::UString & s={Committer):}, int startOffset=0, int & position=-858993460, int & length=0, int * * ovector=0x00000000)  Line 114 + 0x23 bytes	C++
 	JavaScriptCore.dll!JSC::RegExpObject::match(JSC::ExecState * exec=0x03e901e8)  Line 168	C++
 	JavaScriptCore.dll!JSC::RegExpObject::exec(JSC::ExecState * exec=0x03e901e8)  Line 126 + 0xc bytes	C++
 	JavaScriptCore.dll!JSC::regExpProtoFuncExec(JSC::ExecState * exec=0x03e901e8)  Line 74 + 0x1f bytes	C++
 	026a0852()	
 	JavaScriptCore.dll!cti_op_put_by_id_generic()  Line 1421 + 0x24 bytes	C++
 	JavaScriptCore.dll!JSC::JITCode::execute(JSC::RegisterFile * registerFile=0x022d6cf4, JSC::ExecState * callFrame=0x03e90048, JSC::JSGlobalData * globalData=0x022da3d0)  Line 77 + 0x22 bytes	C++
 	JavaScriptCore.dll!JSC::Interpreter::executeCall(JSC::ExecState * callFrame=0x054c7470, JSC::JSObject * function=0x042d1b80, JSC::CallType callType=CallTypeJS, const JSC::CallData & callData={...}, JSC::JSValue thisValue={...}, const JSC::ArgList & args={...})  Line 849 + 0x2a bytes	C++
 	JavaScriptCore.dll!JSC::call(JSC::ExecState * exec=0x054c7470, JSC::JSValue functionObject={...}, JSC::CallType callType=CallTypeJS, const JSC::CallData & callData={...}, JSC::JSValue thisValue={...}, const JSC::ArgList & args={...})  Line 38 + 0x3c bytes	C++
 	WebKit.dll!WebCore::JSMainThreadExecState::call(JSC::ExecState * exec=0x054c7470, JSC::JSValue functionObject={...}, JSC::CallType callType=CallTypeJS, const JSC::CallData & callData={...}, JSC::JSValue thisValue={...}, const JSC::ArgList & args={...})  Line 48 + 0x29 bytes	C++
 	WebKit.dll!WebCore::JSEventListener::handleEvent(WebCore::ScriptExecutionContext * scriptExecutionContext=, WebCore::Event * event=)  Line 124 + 0x6a bytes	C++
 	WebKit.dll!WebCore::EventTarget::fireEventListeners(WebCore::Event * event=0x055a4ca8, WebCore::EventTargetData * d=0x02389e14, WTF::Vector<WebCore::RegisteredEventListener,1> & entry=[1]({listener=0x0555cfe0 {m_jsFunction=0x042d1b80 m_wrapper={...} m_isAttribute=true ...} useCapture=false }))  Line 342 + 0x35 bytes	C++
 	WebKit.dll!WebCore::EventTarget::fireEventListeners(WebCore::Event * event=0x055a4ca8)  Line 313	C++
 	WebKit.dll!WebCore::EventTarget::dispatchEvent(WTF::PassRefPtr<WebCore::Event> event={...})  Line 297 + 0x11 bytes	C++
 	WebKit.dll!WebCore::XMLHttpRequestProgressEventThrottle::dispatchEvent(WTF::PassRefPtr<WebCore::Event> event={...}, WebCore::ProgressEventAction progressEventAction=DoNotFlushProgressEvent)  Line 82	C++
 	WebKit.dll!WebCore::XMLHttpRequest::callReadyStateChangeListener()  Line 368 + 0x39 bytes	C++
 	WebKit.dll!WebCore::XMLHttpRequest::changeState(WebCore::XMLHttpRequest::State newState=DONE)  Line 352	C++
 	WebKit.dll!WebCore::XMLHttpRequest::didFinishLoading(unsigned long identifier=11)  Line 1014	C++
 	WebKit.dll!WebCore::DocumentThreadableLoader::didFinishLoading(unsigned long identifier=11)  Line 246 + 0x19 bytes	C++
 	WebKit.dll!WebCore::DocumentThreadableLoader::didFinishLoading(WebCore::SubresourceLoader * loader=0x054c5c20)  Line 237	C++
 	WebKit.dll!WebCore::SubresourceLoader::didFinishLoading(double finishTime=0.00000000000000000)  Line 180 + 0x1f bytes	C++
 	WebKit.dll!WebCore::ResourceLoader::didFinishLoading(WebCore::ResourceHandle * __formal=0x02341ed0, double finishTime=0.00000000000000000)  Line 435 + 0x18 bytes	C++
 	WebKit.dll!WebCore::didFinishLoading(_CFURLConnection * conn=0x055a4450, const void * clientInfo=0x02341ed0)  Line 244 + 0x26 bytes	C++

The XHR that just finished loading is for <http://svn.webkit.org/repository/webkit/trunk/WebKitTools/Scripts/webkitpy/common/config/committers.py>.
Comment 1 Adam Roben (:aroben) 2010-12-10 07:38:48 PST
<rdar://problem/8754742>
Comment 2 Michael Saboff 2010-12-10 17:35:11 PST
Problem understood and a fix is in progress.
Comment 3 Michael Saboff 2010-12-10 18:38:04 PST
Created attachment 76287 [details]
Patch to set default backtrack destination for first parens in nested parens
Comment 4 Gavin Barraclough 2010-12-10 19:17:12 PST
Comment on attachment 76287 [details]
Patch to set default backtrack destination for first parens in nested parens

R+ to get ToT working again.  Does this now mean the prior fix is redundant, can we roll it out?
Comment 5 Michael Saboff 2010-12-10 23:32:24 PST
Created attachment 76294 [details]
Updated Patch to handle the first nested paren, not just when the first nested term is a paren

Thought about this a little more after posting the initial patch.  The initial patch solves the problem where the first paren in a nested paren is the first term.  I tried it with patterns where there is some other non-paren term first in an alternative and it didn't work.  I have modified the fix to work for the first paren term in an alternative and it works for the original RE as well as one with a non-paren term inserted before the first nested paren.

Examples:
   The original bug was for the RE: /^\s*((\[[^\]]+\])|(u?)("[^"]+"))\s*/
   and the problem was the (u?) wasn't backtracking to the first (.

   If the RE is modified by (the inserted X's):/^\s*(X(\[[^\]]+\])|X(u?)("[^"]+"))\s*/
   then the original patch won't work.  The default backtracking for the (u?) is still
   broken.  The updated fix counts nested parens instead of using the term number
   and works for both the original and modified REs.
Comment 6 Michael Saboff 2010-12-10 23:35:43 PST
BTW, the prior fix is (mostly) needed due to handling of backtracking to the "next char".  There is some redundant code from the prior fix that can be removed, but not the whole patch.
Comment 7 Michael Saboff 2010-12-11 19:25:24 PST
Committed r73866: <http://trac.webkit.org/changeset/73866>