Bug 155139 – RegExp.prototype.exec() should call into Yarr at most once

• Status: RESOLVED
• Resolution: FIXED
• Component: JavaScriptCore

[Filip Pizlo]

Patch forthcoming.

[Filip Pizlo]

Created attachment 273218 [details]
work in progress

[Filip Pizlo]

Created attachment 273228 [details]
the patch

[Saam Barati]

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]

(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]

I have a fix for the debug build.

[Filip Pizlo]

Created attachment 273231 [details]
patch for landing

[Filip Pizlo]

Created attachment 273233 [details]
patch for landing

[Filip Pizlo]

Landed in http://trac.webkit.org/changeset/197715