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

RESOLVED FIXED in Firefox 55

Status

()

defect
--
critical
RESOLVED FIXED
2 years ago
a year ago

People

(Reporter: decoder, Assigned: shu)

Tracking

(Blocks 1 bug, 5 keywords)

Trunk
mozilla55
x86_64
Linux
Points:
---
Dependency tree / graph
Bug Flags:
qe-verify -

Firefox Tracking Flags

(firefox-esr52 wontfix, firefox53 wontfix, firefox54 wontfix, firefox55 fixed)

Details

(Whiteboard: [jsbugmon:update][adv-main55+][post-critsmash-triage])

Attachments

(2 attachments)

(Reporter)

Description

2 years ago
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
(Reporter)

Comment 1

2 years ago
Posted file Testcase
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.
Keywords: sec-moderate
Naveed, is there someone who can take a look at this?
Flags: needinfo?(nihsanullah)

Updated

2 years ago
Whiteboard: [jsbugmon:update,bisect] → [jsbugmon:update]

Comment 4

2 years ago
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
Jon, is bug 1239369 a likely regressor?
Flags: needinfo?(nihsanullah) → needinfo?(jcoppeard)
No, this is likely a debugger / JIT issue.
Flags: needinfo?(jcoppeard)
(In reply to Jon Coppeard (:jonco) from comment #6)
> No, this is likely a debugger / JIT issue.

Setting Jan as a fallback needinfo?.
Flags: needinfo?(jdemooij)
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?
Flags: needinfo?(jdemooij) → needinfo?(shu)
(Assignee)

Comment 9

2 years ago
(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.
(Assignee)

Updated

2 years ago
Flags: needinfo?(shu)
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.
Attachment #8872774 - Flags: review?(jdemooij) → review+
https://hg.mozilla.org/mozilla-central/rev/187554939350
Assignee: nobody → shu
Status: NEW → RESOLVED
Last Resolved: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
Group: javascript-core-security → core-security-release
This grafts cleanly to ESR52. Is it worth backporting?
Flags: needinfo?(shu)
(Assignee)

Comment 14

2 years ago
(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.
Flags: needinfo?(shu)
Whiteboard: [jsbugmon:update] → [jsbugmon:update][adv-main55+]
Flags: qe-verify-
Whiteboard: [jsbugmon:update][adv-main55+] → [jsbugmon:update][adv-main55+][post-critsmash-triage]
Group: core-security-release
You need to log in before you can comment on or make changes to this bug.