1515816 - Crash [@ JSObject::hasLazyGroup] with ReadableStream and OOM

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect, P1)

Description

[Christian Holler (:decoder)]

The following testcase crashes on mozilla-central revision d62e952be812 (build with --enable-posix-nspr-emulation --enable-valgrind --enable-gczeal --disable-tests --disable-profiling --enable-debug --without-intl-api --enable-optimize --target=i686-pc-linux-gnu, run with --fuzzing-safe --cpu-count=2 --ion-offthread-compile=off):

See attachment.


Backtrace:

received signal SIGSEGV, Segmentation fault.
#0  0x567da915 in JSObject::hasLazyGroup (this=0x0) at js/src/vm/JSObject.h:164
#1  JSObject::group (this=0x0) at js/src/vm/JSObject.h:146
#2  0x56b3f4db in js::NativeObject::createWithTemplate (templateObject=..., heap=js::gc::DefaultHeap, cx=0xf6e2e800) at js/src/vm/NativeObject-inl.h:560
#3  ReadableStreamCreateReadResult (cx=<optimized out>, value=..., done=true, forAuthorCode=js::ForAuthorCodeBool::Yes) at js/src/builtin/Stream.cpp:1538
#4  0x56b3fb12 in ReadableStreamCloseInternal (cx=<optimized out>, unwrappedStream=...) at js/src/builtin/Stream.cpp:1482
#5  0x56b407f1 in ReadableStreamCancel (cx=<optimized out>, unwrappedStream=..., reason=...) at js/src/builtin/Stream.cpp:1409
#6  0x56b4241b in ReadableStreamReaderGenericCancel (reason=..., unwrappedReader=..., cx=<optimized out>) at js/src/builtin/Stream.cpp:2001
#7  ReadableStreamDefaultReader_cancel (cx=<optimized out>, argc=0, vp=0xf639d830) at js/src/builtin/Stream.cpp:1876
#8  0x567e3c60 in CallJSNative (cx=0xf6e2e800, native=0x56b422d0 <ReadableStreamDefaultReader_cancel(JSContext*, unsigned int, JS::Value*)>, args=...) at js/src/vm/Interpreter.cpp:443
#9  0x567d632d in js::InternalCallOrConstruct (cx=<optimized out>, args=..., construct=js::NO_CONSTRUCT) at js/src/vm/Interpreter.cpp:535
#10 0x567d6990 in InternalCall (cx=cx@entry=0xf6e2e800, args=...) at js/src/vm/Interpreter.cpp:590
#11 0x567d6b4a in js::Call (cx=0xf6e2e800, fval=..., thisv=..., args=..., rval=...) at js/src/vm/Interpreter.cpp:606
#12 0x569d248e in js::fun_call (cx=0xf6e2e800, argc=1, vp=0xf639dce8) at js/src/vm/JSFunction.cpp:1192
#13 0x567e3c60 in CallJSNative (cx=0xf6e2e800, native=0x569d21c0 <js::fun_call(JSContext*, unsigned int, JS::Value*)>, args=...) at js/src/vm/Interpreter.cpp:443
#14 0x567d632d in js::InternalCallOrConstruct (cx=<optimized out>, args=..., construct=js::NO_CONSTRUCT) at js/src/vm/Interpreter.cpp:535
#15 0x567d6990 in InternalCall (cx=cx@entry=0xf6e2e800, args=...) at js/src/vm/Interpreter.cpp:590
#16 0x567d6b0f in js::CallFromStack (cx=0xf6e2e800, args=...) at js/src/vm/Interpreter.cpp:594
#17 0x56f174c5 in js::jit::DoCallFallback (cx=<optimized out>, frame=0xf639dd28, stub=0xf6757b80, argc=1, vp=0xf639dce8, res=...) at js/src/jit/BaselineIC.cpp:3909
#18 0x35cb93fa in ?? ()
#19 0xf6757b80 in ?? ()
#20 0x35ccb66b in ?? ()
#21 0x35caeb7c in ?? ()
#22 0x5713ac4a in EnterJit (cx=<optimized out>, cx@entry=0xf6e2e800, state=..., code=0x35ccabe0 "\351\033") at js/src/jit/Jit.cpp:105
#23 0x5713b651 in js::jit::MaybeEnterJit (cx=0xf6e2e800, state=...) at js/src/jit/Jit.cpp:168
#24 0x567d5ac8 in js::RunScript (cx=<optimized out>, state=...) at js/src/vm/Interpreter.cpp:408
#25 0x567d659e in js::InternalCallOrConstruct (cx=<optimized out>, args=..., construct=js::NO_CONSTRUCT) at js/src/vm/Interpreter.cpp:563
#26 0x567d6990 in InternalCall (cx=cx@entry=0xf6e2e800, args=...) at js/src/vm/Interpreter.cpp:590
#27 0x567d6b4a in js::Call (cx=0xf6e2e800, fval=..., thisv=..., args=..., rval=...) at js/src/vm/Interpreter.cpp:606
#28 0x56ad63c5 in js::CallSelfHostedFunction (cx=0xf6e2e800, name=..., thisv=..., args=..., rval=...) at js/src/vm/SelfHosting.cpp:1854
#29 0x5689e16b in AsyncFunctionResume (cx=<optimized out>, cx@entry=0xf6e2e800, resultPromise=resultPromise@entry=..., generatorVal=..., generatorVal@entry=..., kind=ResumeKind::Normal, valueOrReason=...) at js/src/vm/AsyncFunction.cpp:202
#30 0x568a2107 in AsyncFunctionStart (generatorVal=..., resultPromise=..., cx=0xf6e2e800) at js/src/vm/AsyncFunction.cpp:218
#31 WrappedAsyncFunction (cx=0xf6e2e800, argc=0, vp=0xf639e5a8) at js/src/vm/AsyncFunction.cpp:94
#32 0x35cbde08 in ?? ()
#33 0xf6756220 in ?? ()
#34 0x35ccb66b in ?? ()
#35 0x35caeb7c in ?? ()
[...] (Entering/Leaving JIT repeatedly)
#106 0x5713ac4a in EnterJit (cx=<optimized out>, cx@entry=0xf6e2e800, state=..., code=0x35ccabe0 "\351\033") at js/src/jit/Jit.cpp:105
#107 0x5713b651 in js::jit::MaybeEnterJit (cx=0xf6e2e800, state=...) at js/src/jit/Jit.cpp:168
#108 0x567d5ac8 in js::RunScript (cx=<optimized out>, state=...) at js/src/vm/Interpreter.cpp:408
#109 0x567d659e in js::InternalCallOrConstruct (cx=<optimized out>, args=..., construct=js::NO_CONSTRUCT) at js/src/vm/Interpreter.cpp:563
#110 0x567d6990 in InternalCall (cx=cx@entry=0xf6e2e800, args=...) at js/src/vm/Interpreter.cpp:590
#111 0x567d6b4a in js::Call (cx=0xf6e2e800, fval=..., thisv=..., args=..., rval=...) at js/src/vm/Interpreter.cpp:606
#112 0x56ad63c5 in js::CallSelfHostedFunction (cx=0xf6e2e800, name=..., thisv=..., args=..., rval=...) at js/src/vm/SelfHosting.cpp:1854
#113 0x5689e16b in AsyncFunctionResume (cx=<optimized out>, cx@entry=0xf6e2e800, resultPromise=resultPromise@entry=..., generatorVal=..., generatorVal@entry=..., kind=ResumeKind::Normal, valueOrReason=...) at js/src/vm/AsyncFunction.cpp:202
#114 0x568a2107 in AsyncFunctionStart (generatorVal=..., resultPromise=..., cx=0xf6e2e800) at js/src/vm/AsyncFunction.cpp:218
#115 WrappedAsyncFunction (cx=0xf6e2e800, argc=0, vp=0xf63a1a58) at js/src/vm/AsyncFunction.cpp:94
#116 0x35cbde08 in ?? ()
#117 0xf6756220 in ?? ()
#118 0x35ccb66b in ?? ()
#119 0x35caeb7c in ?? ()
#120 0x5713ac4a in EnterJit (cx=<optimized out>, cx@entry=0xf6e2e800, state=..., code=0x35ccabe0 "\351\033") at js/src/jit/Jit.cpp:105
#121 0x5713b651 in js::jit::MaybeEnterJit (cx=0xf6e2e800, state=...) at js/src/jit/Jit.cpp:168
#122 0x567d5ac8 in js::RunScript (cx=<optimized out>, state=...) at js/src/vm/Interpreter.cpp:408
#123 0x567d659e in js::InternalCallOrConstruct (cx=<optimized out>, args=..., construct=js::NO_CONSTRUCT) at js/src/vm/Interpreter.cpp:563
#124 0x567d6990 in InternalCall (cx=cx@entry=0xf6e2e800, args=...) at js/src/vm/Interpreter.cpp:590
#125 0x567d6b4a in js::Call (cx=0xf6e2e800, fval=..., thisv=..., args=..., rval=...) at js/src/vm/Interpreter.cpp:606
#126 0x56ad63c5 in js::CallSelfHostedFunction (cx=0xf6e2e800, name=..., thisv=..., args=..., rval=...) at js/src/vm/SelfHosting.cpp:1854
#127 0x5689e16b in AsyncFunctionResume (cx=<optimized out>, resultPromise=..., generatorVal=..., kind=ResumeKind::Normal, valueOrReason=...) at js/src/vm/AsyncFunction.cpp:202
eax	0x0	0
ebx	0x57c1cff4	1472319476
ecx	0xf639d308	-163982584
edx	0xf639d3cc	-163982388
esi	0x57c1cff4	1472319476
edi	0xf639d320	-163982560
ebp	0xf639d2c8	4130984648
esp	0xf639d2c0	4130984640
eip	0x567da915 <JSObject::group() const+21>
=> 0x567da915 <JSObject::group() const+21>:	mov    (%eax),%eax
   0x567da917 <JSObject::group() const+23>:	mov    0xc(%eax),%edx


This crash seems very sensitive, making it hard to reduce the testcase further. It should reproduce reliably though on a 32-bit linux debug build. Marking s-s because of that, until investigated.

Given the stack, it could be some sort of stack space exhaustion causing an early return somewhere.

Comment 1

[Christian Holler (:decoder)]

Attachment: Testcase

Comment 2

[Fuzzing Team]

JSBugMon: Cannot process bug: Unable to automatically reproduce, please track manually.

Comment 3

[Al Billings [:abillings - ex-MoCo]]

This involves streams. Is this something you can investigate?

Comment 4

[Jason Orendorff [:jorendorff]]

I configured and built the engine with these options. Unfortunately the crash doesn't reproduce in my Linux VM.

Asking Jeff for help since he's got Linux installed on bare metal. If this doesn't work, I guess the next step is to get access to a fuzzer instance.

Comment 5

[Jeff Walden [:Waldo]]

Testcase didn't fail for me on trunk or at the reported rev with clang r345952, or at the reported rev with gcc 8.0.1. Sounds like time for that fuzzer instance...

Comment 6

[Jason Orendorff [:jorendorff]]

Christian, we're having trouble reproducing this. Can we get access to a fuzzer instance to investigate further?

Comment 7

[Christian Holler (:decoder)]

I found the problem: The testcase has two remaining load() calls that I overlooked.

Waldo, can you just edit the testcase, search for the definition of "libdir" and change it to point to your copy of mozilla-central/js/src/jit-test/lib/ ?

Comment 8

[Jason Orendorff [:jorendorff]]

I have reproduced this. Interestingly, an abort() in ReportOverRecursed does not hit. I didn't have time today to investigate further.

Comment 9

[Jason Orendorff [:jorendorff]]

Very boring OOM crash. oom_recovery_tco++;

Comment 10

[Jason Orendorff [:jorendorff]]

Not sec-anything. Crashes at NULL. Patch coming.

Comment 11

[Jason Orendorff [:jorendorff]]

Attachment: Bug 1515816 - Fix missing OOM check in ReadableStreamCreateReadResult. r?arai

Comment 12

[Jason Orendorff [:jorendorff]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=aa0d708d789d806186dc372d206ccb739bc4cc82

Comment 13

[Jason Orendorff [:jorendorff]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=85a48c09f699b83095fb59ae33649f92f63764e3

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/847d1877feda
Fix missing OOM check in ReadableStreamCreateReadResult. r=arai

Comment 15

[Oana Pop-Rus]

https://hg.mozilla.org/mozilla-central/rev/847d1877feda

Comment 16

[Ryan VanderMeulen [:RyanVM]]

Is this something we should consider uplifting to Beta for Fx65?

Comment 17

[Jason Orendorff [:jorendorff]]

Yes, the code change is trivial and worth uplifting.

If the new test gives you any trouble, just delete it. It shouldn't, but the test case is more temperamental than I'd like. Or ping me on IRC; it's no trouble to take a second and look at any failures.

Comment 18

[Jason Orendorff [:jorendorff]]

Comment on attachment 9036045 [details]
Bug 1515816 - Fix missing OOM check in ReadableStreamCreateReadResult. r?arai

[Beta/Release Uplift Approval Request]

Feature/Bug causing the regression: Bug 1505122

User impact if declined: possible crash after OOM that we would otherwise handle

Is this code covered by automated tests?: Yes

Has the fix been verified in Nightly?: Yes

Needs manual test from QE?: No

If yes, steps to reproduce:

List of other uplifts needed: None

Risk to taking this patch: Low

Why is the change risky/not risky? (and alternatives if risky): Tiny, well-understood patch adding an error check.

String changes made/needed: none

Comment 19

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9036045 [details]
Bug 1515816 - Fix missing OOM check in ReadableStreamCreateReadResult. r?arai

[Triage Comment]
Fixes an OOM crash. Covered by automated tests. Approved for 65.0b12.

Comment 20

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/c789ccba525f