1410683 - Crash [@ JSScript::pcToOffset] involving super

Status: VERIFIED FIXED

(Core :: JavaScript Engine, defect, P1)

Description

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

The following testcase crashes on mozilla-central revision 87fabdfb0915 (build with --enable-debug --enable-more-deterministic, run with --fuzzing-safe --no-threads --ion-eager):

// Adapted from randomly chosen test: js/src/jit-test/tests/ion/typed-arrays-2.js
Object.prototype[0] = 1;
// Adapted from randomly chosen test: js/src/jit-test/tests/class/superElemMegamorphic.js
class C {}
C.prototype.q = "q";
C.prototype.r = "r";
class D extends C {
    foo(p) {
        return super[p];
    }
}
for (let p in C.prototype) {
    (new D).foo(p);
}

Backtrace:

Thread 1 "js-dbg-64-dm-li" received signal SIGSEGV, Segmentation fault.
0x0000000000477bf1 in JSScript::pcToOffset (this=0x2840, pc=pc@entry=0x7ffff5871e3e "}\005\231\236\220\bɘ\220\004") at /home/gkwubu/trees/mozilla-central/js/src/jsscript.h:1240
1240	    size_t pcToOffset(const jsbytecode* pc) const {
(gdb) bt
#0  0x0000000000477bf1 in JSScript::pcToOffset (this=0x2840, pc=pc@entry=0x7ffff5871e3e "}\005\231\236\220\bɘ\220\004") at /home/gkwubu/trees/mozilla-central/js/src/jsscript.h:1240
#1  0x0000000000ed78d3 in js::jit::BaselineFrame::setOverridePc (pc=0x7ffff5871e3e "}\005\231\236\220\bɘ\220\004", this=<optimized out>)
    at /home/gkwubu/trees/mozilla-central/js/src/jit/BaselineFrame.h:366
#2  js::jit::FinishBailoutToBaseline (bailoutInfo=0x7ffff5853400) at /home/gkwubu/trees/mozilla-central/js/src/jit/BaselineBailouts.cpp:1866
#3  0x0000123af739887d in ?? ()
#4  0x0000000100000007 in ?? ()
#5  0x00007fffffffc458 in ?? ()
/snip

For detailed crash information, see attachment.

Opt builds seem to access weird memory address 0x2840 which may or may not be close enough to null, so setting s-s to be safe as a start

Comment 1

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

Attachment: debug stack

Comment 2

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

Attachment: opt crash stack

Comment 3

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

autoBisect shows this is probably related to the following changeset:

The first bad revision is:
changeset:   https://hg.mozilla.org/mozilla-central/rev/82bdb5c8e75d
user:        Tom Schuster
date:        Wed Oct 18 20:47:29 2017 +0200
summary:     Bug 1378186 - Implement super.property in Ion. r=jandem

Tom, is bug 1378186 a likely regressor?

Comment 4

[Tom S]

I am not surprised, baseline bailouts already were a bit of difficult spot of that patch.

Comment 5

[Tom S]

On a hunch I disabled the code, which adds the extra dummy value for GETELEM super: http://searchfox.org/mozilla-central/source/js/src/jit/BaselineBailouts.cpp#1117 and this fixes the crash. My conjecture is that we run in Baseline OSR to Ion (so we actually have that extra stack value already) and then we bailout from Ion add an extra value again?

Seems like we need a "proper" fix for this behavior now. If this is really important we could just disable GETELEM_SUPER in Ion for a bit.

Comment 6

[Christian Holler (:decoder)]

This is an automated crash issue comment:

Summary: Crash [@ JSScript::pcToOffset]
Build version: mozilla-central revision d49501f258b1
Build flags: --enable-posix-nspr-emulation --enable-valgrind --enable-gczeal --disable-tests --enable-stdcxx-compat --disable-profiling --enable-debug --without-intl-api --enable-optimize --target=i686-pc-linux-gnu
Runtime options: --fuzzing-safe --ion-eager

Testcase:

class C {};
C.prototype.a = "a";
C.prototype.q = "q";
C.prototype.r >>= "r";
class D extends C {
    foo(p) {
        return super[p];
    }
}
var d = new D();
for (let p in C.prototype) {
    assertEq(p, d.foo(p));
}

Backtrace:

received signal SIGSEGV, Segmentation fault.
0x080b1bb6 in JSScript::pcToOffset (this=0x24048f5f, pc=0xf53cf3ee) at js/src/jsscript.h:1240
#0  0x080b1bb6 in JSScript::pcToOffset (this=0x24048f5f, pc=0xf53cf3ee) at js/src/jsscript.h:1240
#1  0x08ae6894 in js::jit::BaselineFrame::setOverridePc (pc=0xf53cf3ee, this=<optimized out>) at js/src/jit/BaselineFrame.h:366
#2  js::jit::FinishBailoutToBaseline (bailoutInfo=0xf5299c00) at js/src/jit/BaselineBailouts.cpp:1866
#3  0x269294dd in ?? ()
Backtrace stopped: previous frame inner to this frame (corrupt stack?)
eax	0xf53cf3ee	-180554770
ebx	0x8db6ff4	148598772
ecx	0xf7940800	-141293568
edx	0x24048f5f	604278623
esi	0xf5299c00	-181822464
edi	0x8db6ff4	148598772
ebp	0xffffcbf8	4294953976
esp	0xffffcbf0	4294953968
eip	0x80b1bb6 <JSScript::pcToOffset(unsigned char const*) const+22>
=> 0x80b1bb6 <JSScript::pcToOffset(unsigned char const*) const+22>:	mov    (%edx),%edx
   0x80b1bb8 <JSScript::pcToOffset(unsigned char const*) const+24>:	test   %edx,%edx


Confirms that this is s-s and probably sec-high at least.

Comment 7

[Tom S]

Jan said he would take a look.

Comment 8

[Jan de Mooij [:jandem]]

Attachment: Patch

When we push the extra Value we need to update frameSize too or stack iteration gets confused.

Comment 9

[Tom S]

Comment on attachment 8922850 [details] [diff] [review]
Patch

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

Thanks!

Comment 10

[Jan de Mooij [:jandem]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/5b9bba06cee76ea41dd84f7b3ec1d81687c20a8b

Comment 11

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/5b9bba06cee7

Comment 12

[Fuzzing Team]

JSBugMon: This bug has been automatically verified fixed.