Bug 155139

Summary: RegExp.prototype.exec() should call into Yarr at most once
Product: WebKit Reporter: Filip Pizlo <fpizlo>
Component: JavaScriptCoreAssignee: Filip Pizlo <fpizlo>
Status: RESOLVED FIXED    
Severity: Normal CC: barraclough, benjamin, commit-queue, ggaren, keith_miller, mark.lam, msaboff, saam
Priority: P2    
Version: WebKit Nightly Build   
Hardware: All   
OS: All   
Attachments:
Description Flags
work in progress
none
the patch
saam: review+
patch for landing
none
patch for landing none

Filip Pizlo
Reported 2016-03-07 14:57:23 PST
Patch forthcoming.
Attachments
work in progress (9.65 KB, patch)
2016-03-07 14:57 PST, Filip Pizlo 		no flags
DetailsFormatted DiffDiff
the patch (15.70 KB, patch)
2016-03-07 15:26 PST, Filip Pizlo 		saam: review+
DetailsFormatted DiffDiff
patch for landing (16.43 KB, patch)
2016-03-07 15:51 PST, Filip Pizlo 		no flags
DetailsFormatted DiffDiff
patch for landing (16.43 KB, patch)
2016-03-07 15:56 PST, Filip Pizlo 		no flags
DetailsFormatted DiffDiff
Show Obsolete (3) View All Add attachment proposed patch, testcase, etc.
Filip Pizlo
Comment 1 2016-03-07 14:57:47 PST
Created attachment 273218 [details] work in progress
Filip Pizlo
Comment 2 2016-03-07 15:26:58 PST
Created attachment 273228 [details] the patch
Saam Barati
Comment 3 2016-03-07 15:37:42 PST
Comment on attachment 273228 [details] the patch View in context: https://bugs.webkit.org/attachment.cgi?id=273228&action=review r=me w/ comments > Source/JavaScriptCore/runtime/RegExpMatchesArray.cpp:115 > + array = JSArray::tryCreateUninitialized(vm, globalObject->regExpMatchesArrayStructure(), regExp->numSubpatterns() + 1); I think this can fail. > Source/JavaScriptCore/runtime/RegExpMatchesArray.cpp:125 > + RELEASE_ASSERT(array); Is there ever a valid reason why this would fail? If so, I think we should throw an exception instead. > Source/JavaScriptCore/runtime/RegExpObject.cpp:188 > + String input = string->value(exec); Can't this throw OOM?
Filip Pizlo
Comment 4 2016-03-07 15:43:31 PST
(In reply to comment #3) > Comment on attachment 273228 [details] > the patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=273228&action=review > > r=me w/ comments > > > Source/JavaScriptCore/runtime/RegExpMatchesArray.cpp:115 > > + array = JSArray::tryCreateUninitialized(vm, globalObject->regExpMatchesArrayStructure(), regExp->numSubpatterns() + 1); > > I think this can fail. > > > Source/JavaScriptCore/runtime/RegExpMatchesArray.cpp:125 > > + RELEASE_ASSERT(array); > > Is there ever a valid reason why this would fail? If so, I think we should > throw an exception instead. > > > Source/JavaScriptCore/runtime/RegExpObject.cpp:188 > > + String input = string->value(exec); > > Can't this throw OOM? I think you're right about all of these things. For now, I'm just mirroring the behavior that the code previously had.
Filip Pizlo
Comment 5 2016-03-07 15:44:28 PST
I have a fix for the debug build.
Filip Pizlo
Comment 6 2016-03-07 15:51:01 PST
Created attachment 273231 [details] patch for landing
Filip Pizlo
Comment 7 2016-03-07 15:56:57 PST
Created attachment 273233 [details] patch for landing
Filip Pizlo
Comment 8 2016-03-07 16:34:57 PST
Landed in http://trac.webkit.org/changeset/197715
Note You need to log in before you can comment on or make changes to this bug.