673763 - TI: Assertion failure: !inline_, at vm/Stack.cpp:865

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Christian Holler (:decoder)]

Attachment: Testcase for shell

The attached testcase asserts on TI revision 9b9fd467eb5f (run with -j -m -n), tested on 64 bit.

Comment 1

[Brian Hackett [Laid off!]]

Attachment: patch

When expanding inlined frames at the top of a VMFrame, we did not update the prevpc/previnline info embedded in the next stack frame, as we do not have any way to access that next frame while recompiling.  This is a recent regression, a change introduced to allow frame expansion to occur during GC (when there is not really any good way of computing the next frame).

Fixing this by just totally disallowing calls from inlined frames that are not to JIT frames in the same VMFrame, so that if the topmost stack frame in a VMFrame has an up frame then that topmost frame is not doing any inlining.

http://hg.mozilla.org/projects/jaegermonkey/rev/752fddcf2d1d

Comment 2

[Luke Wagner [:luke]]

Comment on attachment 548228 [details] [diff] [review]
patch

I like the simplicity of the solution :)  To be clear, though: this means that practically anything that happens from within an inlined frame will  ExpandInlineFrames.

I'm pretty sure the answer is 'yes', but just to be sure: does TI learn when it ExpandInlineFrames to not inline in the future?

Comment 3

[Brian Hackett [Laid off!]]

Things like cross-compartment calls, calls to Invoke that happen within a stub, and direct calls to functions which we couldn't compile (use 'with', etc.) will expand the topmost inline frame and throw it back into the interpreter (actually, everything associated with the VMFrame, until loop heads are reached again).

TI doesn't automatically mark functions uninlineable when expanding inline frames (there are legitimate reasons to continue inlining after some frame was expanded, e.g. another function got recompiled or an exception was thrown).  It would be pretty simple to do that here, and we probably should; this change has the potential to cause some code (though I'm not sure any actually exists) to go from really slow to incredibly slow.