743119 - IonMonkey: Crash [@ js::Interpret] with infinite recursion

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Christian Holler (:decoder)]

The following testcase crashes on ionmonkey revision a9a18824b4c1 (run with --ion -n):


function printStatus (msg) {}
try {
evaluate("\
test();\
function test() {\
  var fThis;\
  function f() {\
    new printStatus( fThis, 'isFinite( new String(\"Infinity\") )', false, isFinite(new f('Infinity')) );\
  }\
  f()();\
}\
");
} catch(exc0) {}
evaluate("test();");

Comment 1

[Christian Holler (:decoder)]

Crash stack:

Program received signal SIGSEGV, Segmentation fault.
0x0813efd1 in js::Interpret (cx=0x870de10, entryFrame=0xf7b10090, interpMode=js::JSINTERP_BAILOUT) at /home/ownhero/homes/mozilla/repos/ionmonkey/js/src/jsinterp.cpp:1297
1297    {
(gdb) bt 8
#0  0x0813efd1 in js::Interpret (cx=0x870de10, entryFrame=0xf7b10090, interpMode=js::JSINTERP_BAILOUT) at /home/ownhero/homes/mozilla/repos/ionmonkey/js/src/jsinterp.cpp:1297
#1  0x0846ea85 in js::ion::ThunkToInterpreter (vp=0xff3fe568) at /home/ownhero/homes/mozilla/repos/ionmonkey/js/src/ion/Bailouts.cpp:476
#2  0x00414885 in ?? ()
#3  0x08390f65 in EnterIon (cx=0x870de10, fp=0xf7b10040, jitcode=0x4148d8) at /home/ownhero/homes/mozilla/repos/ionmonkey/js/src/ion/Ion.cpp:974
#4  0x083911aa in js::ion::Cannon (cx=0x870de10, fp=0xf7b10040) at /home/ownhero/homes/mozilla/repos/ionmonkey/js/src/ion/Ion.cpp:1002
#5  0x08149821 in js::Interpret (cx=0x870de10, entryFrame=0xf7b0ffa0, interpMode=js::JSINTERP_BAILOUT) at /home/ownhero/homes/mozilla/repos/ionmonkey/js/src/jsinterp.cpp:2785
#6  0x0846ea85 in js::ion::ThunkToInterpreter (vp=0xff3fee58) at /home/ownhero/homes/mozilla/repos/ionmonkey/js/src/ion/Bailouts.cpp:476
#7  0x00414885 in ?? ()
(More stack frames follow...)

Repeats always with those 5 frames.

Comment 2

[Christian Holler (:decoder)]

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

Comment 3

[Christian Holler (:decoder)]

Bisection points to this revision as the fix:

The first good revision is:
changeset:   92561:c1aa75c192b8
user:        Kannan Vijayan
date:        Wed Apr 11 17:24:51 2012 -0400
summary:     Bug 744535 - Invalidate/recompile scripts on change to ThisTypeSet

Kannan, is this likely a fix for this bug so we can close it?

Comment 4

[Kannan Vijayan [:djvj]]

Not sure about that.  It seemed like Bug 744535 was a perf bug, not correctness, but I could be wrong.  I would think that failing to freeze ThisTypeSet shouldn't lead to infinite loops, just a whole lot of expensive deoptimizations when guards fail.

bhackett should be able to answer this more definitively.

Comment 5

[Brian Hackett [Laid off!]]

I think there is a JS_CHECK_RECURSION needed somewhere in that call stack, probably in ion::Cannon to avoid blowing the C stack.  The test itself has an infinite recursion.

Comment 6

[David Anderson [:dvander] - inactive, e-mail if emergency]

Attachment: fix

Although this bug no longer reproduces, I encountered this a few days ago and we definitely need a JS_CHECK_RECURSION in EnterIon.

Comment 7

[Sean Stangl [:sstangl]]

Comment on attachment 623902 [details] [diff] [review]
fix

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

::: js/src/ion/Ion.cpp
@@ +945,5 @@
>  
>  static bool
>  EnterIon(JSContext *cx, StackFrame *fp, void *jitcode)
>  {
> +    JS_CHECK_RECURSION(cx, return true);

Needs to return false.

Comment 8

[David Anderson [:dvander] - inactive, e-mail if emergency]

Of course, I managed to leave a bug in a one-line trivial patch ;)

http://hg.mozilla.org/projects/ionmonkey/rev/29b9a3fd01e4