520498 - Recursion limit differs between jit and nonjit

Status: VERIFIED FIXED

(Core :: JavaScript Engine, defect)

Description

[Jesse Ruderman]

(function f(i) { print(i); f(i+1); })(1)

With -j: 
  269675
  InternalError: script stack space quota is exhausted

Without -j:
  3000
  InternalError: too much recursion

I don't know if this is actually a problem, but it surprised me.  Will bad things happen if we fall off trace while thousands of stack frames deep?

Comment 1

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

Right now recursion in the JIT is only limited by its internal callstack limit (which is 500). It can keep hitting this limit and pushing interp frames until it runs out of stack space.

Brendan: should the JIT be obeying the interpreter recursion limit?

Comment 2

[Brendan Eich [:brendan]]

(In reply to comment #1)
> Brendan: should the JIT be obeying the interpreter recursion limit?

Yes -- maybe one of Igor or me should review that code.

/be

Comment 3

[Bob Clary [:bc] (inactive)]

note also bug 514139

Comment 4

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

Attachment: fix

Comment 5

[Brendan Eich [:brendan]]

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

Fun -- we were just missing the magic value sometimes, and going way beyond it till we hit some other limit, I take it (with a different exception)?

/be

Comment 6

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

(In reply to comment #5)

Yes, exactly. The limit still isn't identical (the test in comment #0 is 3010 JIT versus 3000 non-JIT) but it's no longer completely ignored.

http://hg.mozilla.org/tracemonkey/rev/3a72a9e5ccd7

Comment 7

[Jesse Ruderman]

Hrmm.  jsfunfuzz would be happier if the limits were identical.  What causes them to differ by 10?

Comment 8

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

The JIT can construct as many frames as it wants and then push them onto the interpreter stack. So for example, the interpreter's inlineCallCount could be (MAX-1), and the JIT could push 500 frames (its internal limit). Next time the interpreter tries to make a call, it'll throw an exception, but there are ~500 frames over the internal limit. So the inaccuracy could be anywhere in that range.

One way to make the limits identical would be to have the limit of the JIT callstack be MIN(JIT_LIMIT, INTERP_LIMIT - inlineCallCount).

Maybe that's good idea - Brendan, any thoughts?

Comment 9

[Brendan Eich [:brendan]]

(In reply to comment #8)
> One way to make the limits identical would be to have the limit of the JIT
> callstack be MIN(JIT_LIMIT, INTERP_LIMIT - inlineCallCount).
> 
> Maybe that's good idea - Brendan, any thoughts?

That's smart, it avoids dropping JIT frames when converting to interpreter frames and hitting the exact limit (leaving uncommitted changes unflushed and changing the error model). Try it, if it works I'll review.

/be

Comment 10

[Bob Clary [:bc] (inactive)]

js/src/trace-test/tests/basic/bug520498.js

Comment 11

[Robert Sayre]

http://hg.mozilla.org/mozilla-central/rev/3a72a9e5ccd7

Comment 12

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

See follow-up bug 522136.

Comment 13

[Bob Clary [:bc] (inactive)]

v 1.9.3