1136490 - js/src/jit-test/tests/gc/bug-1109913.js fails with "uncaught exception: out of memory" on ggc build when adding a self-hosted function.

Status: RESOLVED FIXED

Description

[Tooru Fujisawa [:arai]]

While fixing bug 1079919, I hit "uncaught exception: out of memory" in js/src/jit-test/tests/gc/bug-1109913.js.

https://treeherder.mozilla.org/#/jobs?repo=try&revision=98c2decbe7d2
https://treeherder.mozilla.org/logviewer.html#?job_id=5174073&repo=try
> uncaught exception: out of memory
> Exit code: 3
> FAIL - gc/bug-1109913.js
> TEST-UNEXPECTED-FAIL | js/src/jit-test/tests/gc/bug-1109913.js | uncaught exception: out of memory (code 3, args "--ion-eager --ion-offthread-compile=off")
> INFO exit-status : 3
> INFO timed-out : False
> INFO stderr 2> uncaught exception: out of memory
> uncaught exception: out of memory
> Exit code: 3
> FAIL - gc/bug-1109913.js
> TEST-UNEXPECTED-FAIL | js/src/jit-test/tests/gc/bug-1109913.js | uncaught exception: out of memory (code 3, args "--ion-eager --ion-offthread-compile=off --ion-check-range-analysis --no-sse3 --no-threads")
> INFO exit-status : 3
> INFO timed-out : False
> INFO stderr 2> uncaught exception: out of memory 

Then, same issue happens even if I only add an empty function.

> diff --git a/js/src/builtin/RegExp.js b/js/src/builtin/RegExp.js
> --- a/js/src/builtin/RegExp.js
> +++ b/js/src/builtin/RegExp.js
> @@ -31,8 +31,11 @@ function RegExpFlagsGetter() {
>  
>      // Steps 16-18.
>      if (R.sticky)
>          result += "y";
>  
>      // Step 19.
>      return result;
>  }
> +
> +function foo() {
> +}

Configure options: --enable-optimize --enable-debug --enable-stdcxx-compat --enable-ctypes --enable-trace-malloc --disable-shared-js --enable-exact-rooting --enable-gccompacting --enable-threadsafe --with-ccache --enable-nspr-build
or
Configure options: --disable-optimize --enable-debug --enable-stdcxx-compat --enable-ctypes --enable-trace-malloc --disable-shared-js --enable-exact-rooting --enable-gccompacting --enable-threadsafe --with-ccache --enable-nspr-build

Environment variable: JS_GC_ZEAL=14

Confirmed on Linux 64bit and Mac OS X 64bit.

Comment 1

[Tooru Fujisawa [:arai]]

Created attachment 8568905 [details] [diff] [review]
(testing) add a self-hosted function.

Comment 2

[Tooru Fujisawa [:arai]]

Mysteriously, adding 2, 3, 4, or 5 functions don't cause the issue :/

Comment 3

[Tooru Fujisawa [:arai]]

with attachment 8568905 [details] [diff] [review], "out of memory" exception is thrown twice, 2nd one is thrown from `catch` block, especially js::jit::PushBlockScope, and, of course, it's not caught.

0   js                                  0x00000001009d86df _Z20js_ReportOutOfMemoryPN2js16ExclusiveContextE + 52
1   js                                  0x0000000100b3708c _ZN2js2gc9GCRuntime28refillFreeListFromMainThreadILNS_7AllowGCE1EEEPvP9JSContextNS0_9AllocKindE + 444
2   js                                  0x0000000100b36ea0 _ZN2js2gc9GCRuntime27refillFreeListFromAnyThreadILNS_7AllowGCE1EEEPvPNS_16ExclusiveContextENS0_9AllocKindE + 176
3   js                                  0x0000000100ba5fd7 _ZN2js2gc14AllocateObjectILNS_7AllowGCE1EEEP8JSObjectPNS_16ExclusiveContextENS0_9AllocKindEmNS0_11InitialHeapEPKNS_5ClassE + 567
4   js                                  0x0000000100ba5ccf _ZN2js11NewGCObjectILNS_7AllowGCE1EEEP8JSObjectPNS_16ExclusiveContextENS_2gc9AllocKindEmNS6_11InitialHeapEPKNS_5ClassE + 159
5   js                                  0x0000000100ba5a8e _ZN8JSObject6createEPN2js16ExclusiveContextENS0_2gc9AllocKindENS3_11InitialHeapEN2JS6HandleIPNS0_5ShapeEEENS7_IPNS0_11ObjectGroupEEE + 1678
6   js                                  0x0000000100422beb _ZN2js17ClonedBlockObject6createEP9JSContextN2JS6HandleIPNS_17StaticBlockObjectEEENS4_IP8JSObjectEE + 491
7   js                                  0x0000000100423279 _ZN2js17ClonedBlockObject6createEP9JSContextN2JS6HandleIPNS_17StaticBlockObjectEEENS_16AbstractFramePtrE + 153
8   js                                  0x0000000100961c97 _ZN2js3jit13BaselineFrame9pushBlockEP9JSContextN2JS6HandleIPNS_17StaticBlockObjectEEE + 167
9   js                                  0x00000001009315bd _ZN2js3jit14PushBlockScopeEP9JSContextPNS0_13BaselineFrameEN2JS6HandleIPNS_17StaticBlockObjectEEE + 45
10  ???                                 0x00000001041d1fe1 0x0 + 4363984865

So, this is a kind of test bug?

Comment 4

[Till Schneidereit [:till]]

Curious.

Are the other functions you mentioned in comment 2 also getters? More generally, does it matter at all whether you simply add the function to RegExp.js or also register it in the RegExp function list?

Comment 5

[Tooru Fujisawa [:arai]]

(In reply to Till Schneidereit [:till] from comment #4)
> Curious.
> 
> Are the other functions you mentioned in comment 2 also getters? More
> generally, does it matter at all whether you simply add the function to
> RegExp.js or also register it in the RegExp function list?

Just adding empty functions (foo, foo2, foo3, ...) to any of self-hosted JS file. I didn't touch RegExp.cpp while testing (so, the patch for bug 1079919 is not applied. clean m-c tree + testing patch). The issue happens only if I added exact one function (attachment 8568905 [details] [diff] [review]), and it disappears when I add one more function (foo2), then never happens by adding more functions (foo3, foo4, ...), at least up to 6.

The value of `gcparam("gcBytes")` differs depending on the number of added functions, and runtime options, but same number occurs for problematic case and non-problematic case.


 number of |        |        |        |        |        |
 functions |   A    |   B    |   C    |   D    |   E    |   F
-----------+--------+--------+--------+--------+--------+--------
 0         | 626688 | 557056 | 512000 | 552960 | 552960 | 614400
 1         | 548864 | 548864 | 507904*| 552960 | 552960 | 532480
 2         | 540672 | 544768 | 512000 | 548864 | 548864 | 536576
 3         | 552960 | 536576 | 507904 | 557056 | 544768 | 536576
 4         | 548864 | 626688 | 512000 | 634880 | 634880 | 536576
 5         | 544768 | 557056 | 512000 | 630784 | 626688 | 532480
 6         | 540672 | 552960 | 512000 | 630784 | 630784 | 532480

  A. no options
  B. "--ion-eager --ion-offthread-compile=off"
  C. "--ion-eager --ion-offthread-compile=off --ion-check-range-analysis --no-sse3 --no-threads"
  D. "--baseline-eager"
  E. "--baseline-eager --no-fpu"
  F. "--no-baseline --no-ion"

(* error happens)


I'm trying following patch as fix for it (in the case if this is actually a test bug).
  https://hg.mozilla.org/try/rev/587cf7d6bb9e
  https://treeherder.mozilla.org/#/jobs?repo=try&revision=40a932aaa162&exclusion_profile=false
  (forgot to apply the patch for bug 1135401, so orange happens in windows SM(e), but not related to this)

Comment 6

[Jon Coppeard (:jonco)]

Created attachment 8569328 [details] [diff] [review]
bug1136490-report-oom

This fixes the problem, but I'm a little unsure what the exact policy is for when we report OOM.  jandem is there some guidance on this?

Comment 7

[Tooru Fujisawa [:arai]]

Unfortunately, same error happens after applying attachment 8569328 [details] [diff] [review]. InterpreterFrame::pushBlock is called only on no options and "--no-baseline --no-ion".

Comment 8

[Jon Coppeard (:jonco)]

Oh right, I'm not sure why I thought my previous patch would fix this at all.

Looking at it again, the test code sets up a situation where OOM is expected and with your changes it just so happens that it strikes when we're not inside the body of the try.

I think we should take your proposed test code modification.  The code that this is testing has since been reworked and is not present anymore anyway.

(Reproduced with the self-hosting patch applied - I saw the folowing failures:

$ JS_GC_ZEAL=14 ./jit-test/jit_test.py --tbpl optdebug-build/shell gc/bug-1109913.js
FAILURES:
    --ion-eager --ion-offthread-compile=off /Users/jon/work/dev/js/src/jit-test/tests/gc/bug-1109913.js
    --ion-eager --ion-offthread-compile=off --ion-check-range-analysis --no-sse3 --no-threads /Users/jon/work/dev/js/src/jit-test/tests/gc/bug-1109913.js)

Comment 9

[Jon Coppeard (:jonco)]

Created attachment 8570517 [details] [diff] [review]
bug1136490-fix-test-code

Comment 10

[Terrence Cole [:terrence]]

Comment on attachment 8570517 [details] [diff] [review]
bug1136490-fix-test-code

Review of attachment 8570517 [details] [diff] [review]:
-----------------------------------------------------------------

I think I have a patch in my queue that just blanket adds the allow-oom tag for every test that touches maxBytes.

Comment 11

[Jon Coppeard (:jonco)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/e36fa665ed23

Comment 12

[Wes Kocher (:KWierso)]

https://hg.mozilla.org/mozilla-central/rev/e36fa665ed23