1242798 - Assertion failure: tag <= CalleeToken_Script, at js/src/jit/JitFrames.h:34 or Crash [@ hasUncompiledScript] with Debugger

Status: VERIFIED FIXED

(Core :: JavaScript Engine, defect)

Description

[Christian Holler (:decoder)]

The following testcase crashes on mozilla-central revision c2256ee8ae9a (build with --enable-optimize --enable-posix-nspr-emulation --enable-valgrind --enable-gczeal --target=i686-pc-linux-gnu --disable-tests --enable-debug, run with --ion-eager):

var g = newGlobal();
var dbg = new Debugger(g);
g.eval("" + function f(c) {
  if (c == 0)
    return;
  if (c == 2)
    debugger;
  f(c-1);
  for (var i = 0; i < 100; i++)
    Debugger += newGlobal('#15: myObj.parseFloat !== parseFloat');
});
dbg.onDebuggerStatement = function (frame) {};
g.eval("f(2)");



Backtrace:

Program received signal SIGSEGV, Segmentation fault.
0x0807816e in js::jit::GetCalleeTokenTag (token=<optimized out>) at js/src/jit/JitFrames.h:34
#0  0x0807816e in js::jit::GetCalleeTokenTag (token=<optimized out>) at js/src/jit/JitFrames.h:34
#1  0x082374f8 in GetCalleeTokenTag (token=<optimized out>) at js/src/vm/Stack.h:177
#2  ScriptFromCalleeToken (token=<optimized out>) at js/src/jit/JitFrames.h:81
#3  script (this=<optimized out>) at js/src/jit/BaselineFrame.h:147
#4  js::AbstractFramePtr::script (this=0xffffc200) at js/src/vm/Stack-inl.h:663
#5  0x08679e8a in js::Debugger::removeDebuggeeGlobal (this=0xf7a70000, fop=fop@entry=0xffffc4c0, global=global@entry=0xf456e040, debugEnum=0x0) at js/src/vm/Debugger.cpp:3572
#6  0x0867a1ff in js::Debugger::detachAllDebuggersFromGlobal (fop=fop@entry=0xffffc4c0, global=0xf456e040) at js/src/vm/Debugger.cpp:2696
#7  0x0856927e in JSCompartment::sweepGlobalObject (this=0xf43e7000, fop=fop@entry=0xffffc4c0) at js/src/jscompartment.cpp:683
#8  0x085a1774 in js::gc::GCRuntime::beginSweepingZoneGroup (this=this@entry=0xf7a39224) at js/src/jsgc.cpp:5189
#9  0x085a98dc in js::gc::GCRuntime::beginSweepPhase (this=this@entry=0xf7a39224, destroyingRuntime=destroyingRuntime@entry=false) at js/src/jsgc.cpp:5375
#10 0x085aaef3 in js::gc::GCRuntime::incrementalCollectSlice (this=this@entry=0xf7a39224, budget=..., reason=reason@entry=JS::gcreason::DESTROY_CONTEXT) at js/src/jsgc.cpp:6136
#11 0x085abcc1 in js::gc::GCRuntime::gcCycle (this=this@entry=0xf7a39224, nonincrementalByAPI=nonincrementalByAPI@entry=true, budget=..., reason=reason@entry=JS::gcreason::DESTROY_CONTEXT) at js/src/jsgc.cpp:6344
#12 0x085ac219 in js::gc::GCRuntime::collect (this=this@entry=0xf7a39224, nonincrementalByAPI=nonincrementalByAPI@entry=true, budget=..., reason=reason@entry=JS::gcreason::DESTROY_CONTEXT) at js/src/jsgc.cpp:6450
#13 0x085ac482 in js::gc::GCRuntime::gc (this=this@entry=0xf7a39224, gckind=gckind@entry=GC_NORMAL, reason=reason@entry=JS::gcreason::DESTROY_CONTEXT) at js/src/jsgc.cpp:6508
#14 0x08558d30 in js::DestroyContext (cx=cx@entry=0xf7a87020, mode=mode@entry=js::DCM_FORCE_GC) at js/src/jscntxt.cpp:181
#15 0x08558f78 in JS_DestroyContext (cx=cx@entry=0xf7a87020) at js/src/jsapi.cpp:580
#16 0x080d94d7 in DestroyContext (withGC=true, cx=<optimized out>) at js/src/shell/js.cpp:6103
#17 main (argc=3, argv=0xffffcc54, envp=0xffffcc64) at js/src/shell/js.cpp:7008
eax	0x0	0
ebx	0x98448f0	159664368
ecx	0xf7e3b88c	-136071028
edx	0x0	0
esi	0xffffc210	-15856
edi	0x0	0
ebp	0xffffc188	4294951304
esp	0xffffc170	4294951280
eip	0x807816e <js::jit::GetCalleeTokenTag(js::jit::CalleeToken)+42>
=> 0x807816e <js::jit::GetCalleeTokenTag(js::jit::CalleeToken)+42>:	movl   $0x22,0x0
   0x8078178 <js::jit::GetCalleeTokenTag(js::jit::CalleeToken)+52>:	call   0x80fed10 <abort()>

Comment 1

[Fuzzing Team]

JSBugMon: Bisection requested, result:
autoBisect shows this is probably related to the following changeset:

The first bad revision is:
changeset:   https://hg.mozilla.org/mozilla-central/rev/b7ea61be4cad
user:        Hannes Verschore
date:        Fri Jan 22 08:07:52 2016 -0500
summary:     Bug 1214059: Baseline: Enable switch to debug mode at function entry, r=jandem

This iteration took 419.634 seconds to run.

Comment 2

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

Hannes, is bug 1214059 a likely regressor?

Comment 3

[Hannes Verschore [:h4writer]]

Just looked into the the issue a bit and I assume bug 1214059 isn't the culprit and this issue is present before it.

We seem to add a AbstractFramePtr to the debugger list (/tmp/test9.js line 4 > eval:1), but we don't remove it from the list, even after the Baseline script has been ended. I see the BaselineFrame getting overwritten by the c++ code in EnterBaseline after returning out of Baseline. But the debugger is still keeping the pointer and iterating it. As a result we get Bogus info.

Closing it, since it looks we are reading random data.

Comment 4

[Shu-yu Guo [:shu]]

I get a different crash:


#0  0x00000000007abca0 in js::ReadBarriered<js::GlobalObject*>::operator bool (this=0xe80000018100c750) at /home/shu/moz/central/js/src/gc/Barrier.h:607
#1  0x00000000007a4ed2 in JSCompartment::maybeGlobal (this=0xe80000018100c700) at /home/shu/moz/central/js/src/jscompartmentinlines.h:27
#2  0x00000000007a58a2 in JSScript::global (this=0x67d5fe <mozilla::Maybe<JS::AutoAssertOnGC>::~Maybe()+24>) at /home/shu/moz/central/js/src/jsscriptinlines.h:182
#3  0x0000000000b3b4dc in js::Debugger::removeDebuggeeGlobal (this=0x7ffff695b800, fop=0x7fffffffe030, global=0x7ffff7e8e060, debugEnum=0x0) at /home/shu/moz/central/js/src/vm/Debugger.cpp:3572
#4  0x0000000000b3728e in js::Debugger::detachAllDebuggersFromGlobal (fop=0x7fffffffe030, global=0x7ffff7e8e060) at /home/shu/moz/central/js/src/vm/Debugger.cpp:2696
#5  0x00000000009d34cc in JSCompartment::sweepGlobalObject (this=0x7ffff695a000, fop=0x7fffffffe030) at /home/shu/moz/central/js/src/jscompartment.cpp:683
#6  0x0000000000a190e0 in js::gc::GCRuntime::beginSweepingZoneGroup (this=0x7ffff696a420) at /home/shu/moz/central/js/src/jsgc.cpp:5189
#7  0x0000000000a1a08e in js::gc::GCRuntime::beginSweepPhase (this=0x7ffff696a420, destroyingRuntime=false) at /home/shu/moz/central/js/src/jsgc.cpp:5375
#8  0x0000000000a1ca5c in js::gc::GCRuntime::incrementalCollectSlice (this=0x7ffff696a420, budget=..., reason=JS::gcreason::DESTROY_CONTEXT) at /home/shu/moz/central/js/src/jsgc.cpp:6136
#9  0x0000000000a1d3c0 in js::gc::GCRuntime::gcCycle (this=0x7ffff696a420, nonincrementalByAPI=true, budget=..., reason=JS::gcreason::DESTROY_CONTEXT) at /home/shu/moz/central/js/src/jsgc.cpp:6344
#10 0x0000000000a1d8c1 in js::gc::GCRuntime::collect (this=0x7ffff696a420, nonincrementalByAPI=true, budget=..., reason=JS::gcreason::DESTROY_CONTEXT) at /home/shu/moz/central/js/src/jsgc.cpp:6450
#11 0x0000000000a1dbbb in js::gc::GCRuntime::gc (this=0x7ffff696a420, gckind=GC_NORMAL, reason=JS::gcreason::DESTROY_CONTEXT) at /home/shu/moz/central/js/src/jsgc.cpp:6508
#12 0x00000000009cd708 in js::DestroyContext (cx=0x7ffff6919000, mode=js::DCM_FORCE_GC) at /home/shu/moz/central/js/src/jscntxt.cpp:181
#13 0x00000000009b903d in JS_DestroyContext (cx=0x7ffff6919000) at /home/shu/moz/central/js/src/jsapi.cpp:580
#14 0x000000000044468c in DestroyContext (cx=0x7ffff6919000, withGC=true) at /home/shu/moz/central/js/src/shell/js.cpp:6104
#15 0x00000000004480a2 in main (argc=3, argv=0x7fffffffe7e8, envp=0x7fffffffe808) at /home/shu/moz/central/js/src/shell/js.cpp:7009

Comment 5

[Shu-yu Guo [:shu]]

Attachment: Don't OSR into Ion on debuggee frames.

The bug here is that while we never compile an IonScript if the OSR frame is marked as debuggee, if the script already has an IonScript though (in this case, due to OSRing on a recursive call), we can incorrectly jump into Ion on a debuggee OSR frame.

Comment 6

[Fuzzing Team]

JSBugMon: The testcase found in this bug no longer reproduces (tried revision a1b94785b484).

Comment 7

[Jan de Mooij [:jandem]]

Comment on attachment 8712810 [details] [diff] [review]
Don't OSR into Ion on debuggee frames.

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

Nice find, this could explain some crashes I've seen on crash stats.

Comment 9

[Shu-yu Guo [:shu]]

Comment on attachment 8712810 [details] [diff] [review]
Don't OSR into Ion on debuggee frames.

[Security approval request comment]
How easily could an exploit be constructed based on the patch?

> To hit the crash path, the test is sufficient if the attacker can convince the user to run it with the devtools debugger active. To construct an exploit, ISTM to be very difficult.

Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem?

> On the incorrectness, but not on an obviously exploitable problem.

Which older supported branches are affected by this flaw?

> All supported branches.

If not all supported branches, which bug introduced the flaw?

Do you have backports for the affected branches? If not, how different, hard to create, and risky will they be?

> Should apply cleanly I think.

How likely is this patch to cause regressions; how much testing does it need?

> No regressions. Testing wise I think the attached test in the patch is sufficient.

Comment 10

[Daniel Veditz [:dveditz]]

Let's call this sec-moderate because you'd have to lure a victim into debugging, it can't be turned into a pure drive-by.

Comment 11

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

Comment on attachment 8712810 [details] [diff] [review]
Don't OSR into Ion on debuggee frames.

Clearing sec-approval? as sec-moderates don't need approval to land.

Comment 12

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/52cedeb87301

Comment 13

[Fuzzing Team]

JSBugMon: This bug has been automatically verified fixed.

Comment 14

[Liz Henry (:lizzard Please n-i to RyanVM, jcristau, or pascal)]

Too late to uplift, not sec-high or topcrash, so wontfix for 46.