Bug 157595

Summary: r199812 broke test262
Product: WebKit Reporter: Saam Barati <saam>
Component: JavaScriptCoreAssignee: Michael Saboff <msaboff>
Status: RESOLVED FIXED    
Severity: Normal CC: benjamin, fpizlo, ggaren, gskachkov, keith_miller, mark.lam, msaboff, oliver, ossy, saam, sukolsak, webkit-bug-importer, ysuzuki
Priority: P2 Keywords: InRadar
Version: WebKit Local Build   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 156832    
Attachments:
Description Flags
Patch fpizlo: review+

Saam Barati
Reported 2016-05-11 16:45:15 PDT
It fails around the 38% mark.
Attachments
Patch (4.10 KB, patch)
2016-05-18 14:54 PDT, Michael Saboff
fpizlo: review+
Radar WebKit Bug Importer
Comment 1 2016-05-11 16:48:31 PDT
Michael Saboff
Comment 2 2016-05-18 10:36:01 PDT
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
Comment 3 2016-05-18 14:54:58 PDT
Michael Saboff
Comment 4 2016-05-18 15:35:02 PDT
Saam Barati
Comment 5 2016-05-18 16:15:30 PDT
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
Comment 6 2016-05-18 17:08:40 PDT
(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
Comment 7 2016-05-19 09:27:07 PDT
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
Comment 8 2016-05-19 09:33:07 PDT
(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
Comment 9 2016-05-19 10:21:43 PDT
(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
Comment 10 2016-05-19 11:39:12 PDT
(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.
Note You need to log in before you can comment on or make changes to this bug.