Bug 157595 – r199812 broke test262

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

[Saam Barati]

It fails around the 38% mark.

[Radar WebKit Bug Importer]

<rdar://problem/26234295>

[Michael Saboff]

We believe that this is due to String.prototype.match() going into an infinite loop. This can happen when the argument RegExp object has an override for .global that always returns true, but the global flag on the base RegExp is false.

[Michael Saboff]

Created attachment 279288 [details]
Patch

[Michael Saboff]

Committed r201105: <http://trac.webkit.org/changeset/201105>

[Saam Barati]

Comment on attachment 279288 [details]
Patch

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

> Source/JavaScriptCore/builtins/RegExpPrototype.js:113
> +            throw new @Error("Out of memory");

Maybe this could be more descriptive about where we're throwing the OOM from?
Or maybe the error object itself nicely handles this?

[Michael Saboff]

(In reply to comment #5)
> Comment on attachment 279288 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=279288&action=review
> 
> > Source/JavaScriptCore/builtins/RegExpPrototype.js:113
> > +            throw new @Error("Out of memory");
> 
> Maybe this could be more descriptive about where we're throwing the OOM from?
> Or maybe the error object itself nicely handles this?

The Error object has a .stack property.  For the added tests it shows:
match@[native code]
test@Source/JavaScriptCore/tests/stress/regress-157595.js:19:23
global code@Source/JavaScriptCore/tests/stress/regress-157595.js:23:9

[Csaba Osztrogonác]

JSC consumes terribly much memory during executing this test:
This new Source/JavaScriptCore/tests/stress/regress-157595.js

It made my desktop machine useless for long minutes and swapped.
I tried it on a server machine with huge RAM, it passed after 26
seconds long running and consumed 8Gb memory.

It's not so good having a test consumes 8Gb memory. :(

[Michael Saboff]

(In reply to comment #7)
> JSC consumes terribly much memory during executing this test:
> This new Source/JavaScriptCore/tests/stress/regress-157595.js
> 
> It made my desktop machine useless for long minutes and swapped.
> I tried it on a server machine with huge RAM, it passed after 26
> seconds long running and consumed 8Gb memory.
> 
> It's not so good having a test consumes 8Gb memory. :(

It does consume 8GB.  That is why I add the "runOneLargeHeap" option in run-jsc-stress-tests and configured it to run that way.  You can use the --memory-limited option when you run the regression tests.

The test is checking that String.prototype.match() doesn't infinite loop in a corner case.  Just like in the C++ code, we set an upper bound on how many results .match() can return before throwing out of memory.  The test runs till it hits that bound, thus the large memory usage.

[Csaba Osztrogonác]

(In reply to comment #8)
> (In reply to comment #7)
> > JSC consumes terribly much memory during executing this test:
> > This new Source/JavaScriptCore/tests/stress/regress-157595.js
> > 
> > It made my desktop machine useless for long minutes and swapped.
> > I tried it on a server machine with huge RAM, it passed after 26
> > seconds long running and consumed 8Gb memory.
> > 
> > It's not so good having a test consumes 8Gb memory. :(
> 
> It does consume 8GB.  That is why I add the "runOneLargeHeap" option in
> run-jsc-stress-tests and configured it to run that way.  You can use the
> --memory-limited option when you run the regression tests.

run-javascriptcore-tests doesn't have --memory-limited option, only 
run-jsc-stress-tests has. :( And buildbots uses the first script. 

> The test is checking that String.prototype.match() doesn't infinite loop in
> a corner case.  Just like in the C++ code, we set an upper bound on how many
> results .match() can return before throwing out of memory.  The test runs
> till it hits that bound, thus the large memory usage.

[Michael Saboff]

(In reply to comment #9)
> (In reply to comment #8)
> > (In reply to comment #7)
> > > JSC consumes terribly much memory during executing this test:
> > > This new Source/JavaScriptCore/tests/stress/regress-157595.js
> > > 
> > > It made my desktop machine useless for long minutes and swapped.
> > > I tried it on a server machine with huge RAM, it passed after 26
> > > seconds long running and consumed 8Gb memory.
> > > 
> > > It's not so good having a test consumes 8Gb memory. :(
> > 
> > It does consume 8GB.  That is why I add the "runOneLargeHeap" option in
> > run-jsc-stress-tests and configured it to run that way.  You can use the
> > --memory-limited option when you run the regression tests.
> 
> run-javascriptcore-tests doesn't have --memory-limited option, only 
> run-jsc-stress-tests has. :( And buildbots uses the first script. 
> 
> > The test is checking that String.prototype.match() doesn't infinite loop in
> > a corner case.  Just like in the C++ code, we set an upper bound on how many
> > results .match() can return before throwing out of memory.  The test runs
> > till it hits that bound, thus the large memory usage.

Landed change set r201172 that skips the test.  I filed <https://bugs.webkit.org/show_bug.cgi?id=157903> to track fixing the memory usage.