RegExpMatchesArray doesn't know how to have a bad time

[Filip Pizlo]

Patch forthcoming.

[Filip Pizlo]

Created attachment 273119 [details]
work in progress

[Filip Pizlo]

Created attachment 273122 [details]
more

[Filip Pizlo]

Created attachment 273125 [details]
possibly done

[Filip Pizlo]

Created attachment 273126 [details]
seems to work

[Filip Pizlo]

Created attachment 273128 [details]
runs some things

[Filip Pizlo]

Created attachment 273131 [details]
the patch

[WebKit Commit Bot]

Attachment 273131 [details] did not pass style-queue:


ERROR: Source/JavaScriptCore/dfg/DFGAbstractInterpreterInlines.h:1908:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
Total errors found: 1 in 38 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Yusuke Suzuki]

Comment on attachment 273131 [details]
the patch

View in context: https://bugs.webkit.org/attachment.cgi?id=273131&action=review

Looks great.

> Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp:2175
> +        Node* regExpExec = addToGraph(RegExpExec, OpInfo(0), OpInfo(prediction), addToGraph(GetGlobalObject, callee), get(virtualRegisterForArgument(0, registerOffset)), get(virtualRegisterForArgument(1, registerOffset)));

OK, we need to insert having a bad time watchpoint checking for the RegExp.prototype.exec's global object.

> Source/JavaScriptCore/runtime/JSGlobalObject.cpp:767
> +    m_regExpMatchesArrayStructure.set(vm, this, m_regExpMatchesArraySlowPutStructure.get());

OK. Switching the structure to slow put one, that is used when creating a new RegExp matches array.
And existing one is switched by the following path.

> Source/JavaScriptCore/tests/stress/regexp-matches-array-slow-put.js:3
> +    Array.prototype.__defineSetter__("0", function(value) { count += value; });

Yeah, let's have a bad time!

[Filip Pizlo]

(In reply to comment #8)
> Comment on attachment 273131 [details]
> the patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=273131&action=review
> 
> Looks great.
> 
> > Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp:2175
> > +        Node* regExpExec = addToGraph(RegExpExec, OpInfo(0), OpInfo(prediction), addToGraph(GetGlobalObject, callee), get(virtualRegisterForArgument(0, registerOffset)), get(virtualRegisterForArgument(1, registerOffset)));
> 
> OK, we need to insert having a bad time watchpoint checking for the
> RegExp.prototype.exec's global object.

Note that we will only insert the haveABadTime watchpoint if the abstract interpreter discovers what the global object is and observes that it's not having a bad time.  If either of those things don't happen, RegExpExec will simply make no promises about the kind of array it returns - so the CodeBlock doesn't need any watchpoints in that case.

This works partly because createRegExpMatchesArray currently always does a check for isHavingABadTime each time it is executed.

> 
> > Source/JavaScriptCore/runtime/JSGlobalObject.cpp:767
> > +    m_regExpMatchesArrayStructure.set(vm, this, m_regExpMatchesArraySlowPutStructure.get());
> 
> OK. Switching the structure to slow put one, that is used when creating a
> new RegExp matches array.
> And existing one is switched by the following path.
> 
> > Source/JavaScriptCore/tests/stress/regexp-matches-array-slow-put.js:3
> > +    Array.prototype.__defineSetter__("0", function(value) { count += value; });
> 
> Yeah, let's have a bad time!

[Filip Pizlo]

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