Bug 155139 - RegExp.prototype.exec() should call into Yarr at most once
Summary: RegExp.prototype.exec() should call into Yarr at most once
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: WebKit Nightly Build
Hardware: All All
: P2 Normal
Assignee: Filip Pizlo
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2016-03-07 14:57 PST by Filip Pizlo
Modified: 2016-03-07 16:34 PST (History)
8 users (show)

See Also:


Attachments
work in progress (9.65 KB, patch)
2016-03-07 14:57 PST, Filip Pizlo
no flags Details | Formatted Diff | Diff
the patch (15.70 KB, patch)
2016-03-07 15:26 PST, Filip Pizlo
sbarati: review+
Details | Formatted Diff | Diff
patch for landing (16.43 KB, patch)
2016-03-07 15:51 PST, Filip Pizlo
no flags Details | Formatted Diff | Diff
patch for landing (16.43 KB, patch)
2016-03-07 15:56 PST, Filip Pizlo
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Filip Pizlo 2016-03-07 14:57:23 PST
Patch forthcoming.
Comment 1 Filip Pizlo 2016-03-07 14:57:47 PST
Created attachment 273218 [details]
work in progress
Comment 2 Filip Pizlo 2016-03-07 15:26:58 PST
Created attachment 273228 [details]
the patch
Comment 3 Saam Barati 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?
Comment 4 Filip Pizlo 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.
Comment 5 Filip Pizlo 2016-03-07 15:44:28 PST
I have a fix for the debug build.
Comment 6 Filip Pizlo 2016-03-07 15:51:01 PST
Created attachment 273231 [details]
patch for landing
Comment 7 Filip Pizlo 2016-03-07 15:56:57 PST
Created attachment 273233 [details]
patch for landing
Comment 8 Filip Pizlo 2016-03-07 16:34:57 PST
Landed in http://trac.webkit.org/changeset/197715