1353356 - Assertion failure: aIndex < mLength, at dist/include/mozilla/Vector.h:533 with Debugger

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Christian Holler (:decoder)]

The following testcase crashes on mozilla-central revision 65b0ac174753 (build with --enable-posix-nspr-emulation --enable-valgrind --enable-gczeal --disable-tests --enable-stdcxx-compat --disable-profiling --enable-debug --enable-optimize, run with --fuzzing-safe --ion-offthread-compile=off):

See attachment.

Backtrace:

 received signal SIGSEGV, Segmentation fault.
0x0000000000bebd50 in mozilla::Vector<js::jit::RematerializedFrame*, 0ul, js::TempAllocPolicy>::operator[] (this=<optimized out>, aIndex=<optimized out>) at /srv/jenkins/jobs/mozilla-central-build-jsshell/workspace/arch/64/compiler/gcc/sanitizer/none/type/debug/dist/include/mozilla/Vector.h:533
#0  0x0000000000bebd50 in mozilla::Vector<js::jit::RematerializedFrame*, 0ul, js::TempAllocPolicy>::operator[] (this=<optimized out>, aIndex=<optimized out>) at /srv/jenkins/jobs/mozilla-central-build-jsshell/workspace/arch/64/compiler/gcc/sanitizer/none/type/debug/dist/include/mozilla/Vector.h:533
#1  0x0000000000bd57a7 in JS::GCVector<js::jit::RematerializedFrame*, 0ul, js::TempAllocPolicy>::operator[] (i=0, this=<optimized out>) at /srv/jenkins/jobs/mozilla-central-build-jsshell/workspace/arch/64/compiler/gcc/sanitizer/none/type/debug/dist/include/js/GCVector.h:64
#2  js::jit::JitActivation::getRematerializedFrame (this=<optimized out>, cx=cx@entry=0x7ffff6948000, iter=..., inlineDepth=inlineDepth@entry=0) at js/src/vm/Stack.cpp:1570
#3  0x0000000000bd5c6c in js::FrameIter::ensureHasRematerializedFrame (this=this@entry=0x7fffffff8ea0, cx=cx@entry=0x7ffff6948000) at js/src/vm/Stack.cpp:944
#4  0x0000000000adf51a in js::DebuggerFrame::getOlder (cx=cx@entry=0x7ffff6948000, frame=..., frame@entry=..., result=..., result@entry=...) at js/src/vm/Debugger.cpp:7847
#5  0x0000000000adf6a1 in js::DebuggerFrame::olderGetter (cx=cx@entry=0x7ffff6948000, argc=argc@entry=0, vp=vp@entry=0x7fffffff9410) at js/src/vm/Debugger.cpp:8539
#6  0x00000000008325a0 in js::jit::CallNativeGetter (cx=0x7ffff6948000, callee=..., obj=..., result=...) at js/src/jit/VMFunctions.cpp:1483
#7  0x000024693eec2d3c in ?? ()
[...]
#44 0x00007fffffff98c0 in ?? ()
#45 0x00000000005e77c2 in EnterBaseline (cx=0xfffb7ffff46ce778, data=...) at js/src/jit/BaselineJIT.cpp:162
Backtrace stopped: previous frame inner to this frame (corrupt stack?)
rax	0x0	0
rbx	0x7fffffff8ef8	140737488326392
rcx	0x7ffff6c28a2d	140737333332525
rdx	0x0	0
rsi	0x7ffff6ef7770	140737336276848
rdi	0x7ffff6ef6540	140737336272192
rbp	0x7fffffff89d0	140737488325072
rsp	0x7fffffff89d0	140737488325072
r8	0x7ffff6ef7770	140737336276848
r9	0x7ffff7fe4740	140737354024768
r10	0x0	0
r11	0x0	0
r12	0x7fffffffbe50	140737488338512
r13	0x7fffffff8a40	140737488325184
r14	0x7ffff6948000	140737330315264
r15	0x7ffff69df700	140737330935552
rip	0xbebd50 <mozilla::Vector<js::jit::RematerializedFrame*, 0ul, js::TempAllocPolicy>::operator[](unsigned long)+96>
=> 0xbebd50 <mozilla::Vector<js::jit::RematerializedFrame*, 0ul, js::TempAllocPolicy>::operator[](unsigned long)+96>:	movl   $0x0,0x0
   0xbebd5b <mozilla::Vector<js::jit::RematerializedFrame*, 0ul, js::TempAllocPolicy>::operator[](unsigned long)+107>:	ud2

Comment 1

[Christian Holler (:decoder)]

Attachment: Testcase

Comment 2

[Daniel Veditz [:dveditz]]

Calling "sec-moderate" because of the use of non-web-exposed debugger APIs. If that's not required for the underlying bug please raise to sec-high.

Comment 3

[Mike Taylor [:miketaylr]]

Naveed, is there someone who can take a look at this?

Comment 4

[Fuzzing Team]

JSBugMon: Bisection requested, result:
=== Treeherder Build Bisection Results by autoBisect ===

The "good" changeset has the timestamp "20160115010341" and the hash "32a8c6a3be186bbc1f39da147eb09b087ed322e3".
The "bad" changeset has the timestamp "20160115014842" and the hash "df444117c7bea0a407387dca31ed54c3598b054a".

Likely regression window: https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=32a8c6a3be186bbc1f39da147eb09b087ed322e3&tochange=df444117c7bea0a407387dca31ed54c3598b054a

Comment 5

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

Jon, is bug 1239369 a likely regressor?

Comment 6

[Jon Coppeard (:jonco)]

No, this is likely a debugger / JIT issue.

Comment 7

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

(In reply to Jon Coppeard (:jonco) from comment #6)
> No, this is likely a debugger / JIT issue.

Setting Jan as a fallback needinfo?.

Comment 8

[Jan de Mooij [:jandem]]

What happens is that in JitActivation::getRematerializedFrame, we do:

(1) Call rematerializedFrames_->add.
(2) Then pass a pointer to the empty array to RematerializedFrame::RematerializeInlineFrames. If this fails due to OOM, the rematerializedFrames_ table contains an empty Vector and later on we crash when we try to use it.

Shu, it seems we should do (1) after (2) or remove the entry from the rematerializedFrames_ if (2) fails. WDYT?

Comment 9

[Shu-yu Guo [:shu]]

(In reply to Jan de Mooij [:jandem] from comment #8)
> What happens is that in JitActivation::getRematerializedFrame, we do:
> 
> (1) Call rematerializedFrames_->add.
> (2) Then pass a pointer to the empty array to
> RematerializedFrame::RematerializeInlineFrames. If this fails due to OOM,
> the rematerializedFrames_ table contains an empty Vector and later on we
> crash when we try to use it.
> 
> Shu, it seems we should do (1) after (2) or remove the entry from the
> rematerializedFrames_ if (2) fails. WDYT?

That sounds like the right fix -- I think that code was written at a pre-Move time, maybe, and I wanted to avoid acrobatics to avoid copying.

Leaving NI to remind myself to write a patch.

Comment 10

[Shu-yu Guo [:shu]]

Attachment: Add rematerialized frames to the table on JitActivation after rematerialization succeeds.

Comment 11

[Jan de Mooij [:jandem]]

Comment on attachment 8872774 [details] [diff] [review]
Add rematerialized frames to the table on JitActivation after rematerialization succeeds.

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

LGTM.

Comment 12

[Ryan VanderMeulen [:RyanVM]]

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

Comment 13

[Ryan VanderMeulen [:RyanVM]]

This grafts cleanly to ESR52. Is it worth backporting?

Comment 14

[Shu-yu Guo [:shu]]

(In reply to Ryan VanderMeulen [:RyanVM] from comment #13)
> This grafts cleanly to ESR52. Is it worth backporting?

I don't think so. I consider OOM to be such a catastrophic event and wouldn't prioritize backporting OOM fixes.