Last Comment Bug 754720 - IonMonkey: Assertion failure: ionInlineFrames_.numActualArgs() <= ionInlineFrames_.callee()->nargs, at vm/Stack.cpp:1496 or Crash [@ js::StackFrame::isFunctionFrame]
: IonMonkey: Assertion failure: ionInlineFrames_.numActualArgs() <= ionInlineFr...
Status: RESOLVED FIXED
[jsbugmon:update,ignore]
: assertion, testcase
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: Other Branch
: x86_64 Linux
: -- major (vote)
: ---
Assigned To: Nicolas B. Pierron [:nbp]
:
Mentors:
Depends on:
Blocks: langfuzz IonFuzz
  Show dependency treegraph
 
Reported: 2012-05-13 15:21 PDT by Christian Holler (:decoder)
Modified: 2013-01-14 08:10 PST (History)
8 users (show)
choller: in‑testsuite+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
f.arguments: Recover overflow of arguments from the stack. (7.23 KB, patch)
2012-05-16 09:58 PDT, Nicolas B. Pierron [:nbp]
no flags Details | Diff | Splinter Review
f.arguments: Recover overflow of arguments from the stack. (7.84 KB, patch)
2012-05-16 10:28 PDT, Nicolas B. Pierron [:nbp]
dvander: review+
Details | Diff | Splinter Review

Description Christian Holler (:decoder) 2012-05-13 15:21:59 PDT
The following testcase asserts on ionmonkey revision e8de64e7e9fe (run with --ion -n -m --ion-eager):


function f2() {
  return f2.arguments;
}
actual = (f2() == null);
actual = (f2(0) == null);
Comment 1 Christian Holler (:decoder) 2012-05-13 15:23:04 PDT
Crashes only in debug build but I cannot judge if that is coincidence or not. Crash trace when stepping through assertions:

Assertion failure: Crossing fingers: Unable to read snapshot slot., at ../ion/IonFrameIterator.h:224

Program received signal SIGABRT, Aborted.

Program received signal SIGSEGV, Segmentation fault.
0x000000000040681c in js::StackFrame::isFunctionFrame (this=0x6d65) at ../../vm/Stack.h:462
462             return !!(flags_ & FUNCTION);
Missing separate debuginfos, use: debuginfo-install libgcc-4.4.6-3.el6.x86_64 libstdc++-4.4.6-3.el6.x86_64
(gdb) bt 8
#0  0x000000000040681c in js::StackFrame::isFunctionFrame (this=0x6d65) at ../../vm/Stack.h:462
#1  0x0000000000639e7c in js::StackFrame::functionScript (this=0x6d65) at ../vm/Stack.h:622
#2  0x00007f0700000002 in ?? ()
#3  0xffffffff00000001 in ?? ()
#4  0x00007fffffffb600 in ?? ()
#5  0x00007fffffffb620 in ?? ()
#6  0x00007f0700000002 in ?? ()
#7  0xffffffff00000001 in ?? ()
(More stack frames follow...)
(gdb) x /i $pc
=> 0x40681c <js::StackFrame::isFunctionFrame() const+12>:       mov    (%rax),%eax
(gdb) info reg rax
rax            0x6d65   28005
Comment 2 Nicolas B. Pierron [:nbp] 2012-05-15 14:04:19 PDT
I added this code because this case is not handled yet.  Currently we are not mapping all arguments of the function, only the expected number of arguments and add Undefined arguments as padding if not provided.

So if the number of actual argument is larger than the number of formal argument, which explain why we have this assertion.  Eager compilation cause f2 to be compiled and when f2.arguments tries to create an unexpected argument object, it needs to recover them from the Ion stack.

Sadly we have no current way to detect such cases ahead enough to avoid entering the ion code which is shrinking the argument vector.
Comment 3 Nicolas B. Pierron [:nbp] 2012-05-16 09:58:42 PDT
Created attachment 624427 [details] [diff] [review]
f.arguments: Recover overflow of arguments from the stack.

This patch should fix the current bug but it also introduce a problem in case of bailouts with an overflow of arguments which is not yet handled and will be fixed in a follow-up bug/patch.
Comment 4 Nicolas B. Pierron [:nbp] 2012-05-16 10:28:09 PDT
Created attachment 624439 [details] [diff] [review]
f.arguments: Recover overflow of arguments from the stack.

The same except that this fix an additional bug SnapshotIterator::hasLocation which raised when I added one formal argument in the test case.
Comment 5 David Anderson [:dvander] 2012-05-16 11:21:06 PDT
Comment on attachment 624439 [details] [diff] [review]
f.arguments: Recover overflow of arguments from the stack.

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

::: js/src/ion/Ion.cpp
@@ +960,5 @@
>          argv = fp->formalArgs() - 1;
> +        if (fp->hasOverflowArgs()) {
> +            argc = fp->numActualArgs() + 1;
> +            argv = fp->actualArgs() - 1;
> +        }

nit: It might read better to put the initial argc/argv assignment in an else branch.

::: js/src/ion/IonFrameIterator-inl.h
@@ +65,5 @@
> +        // Currently inlining does not support overflow of arguments, we have to
> +        // add this feature in IonBuilder.cpp and in Bailouts.cpp before
> +        // continuing. We need to add it to Bailouts.cpp because we need to know
> +        // how to walk over the oveflow of arguments.
> +        JS_ASSERT(end <= nformal);

nit: Please brace this if/else (comment makes it multi-line)
Comment 6 Nicolas B. Pierron [:nbp] 2012-05-16 18:52:16 PDT
https://hg.mozilla.org/projects/ionmonkey/rev/7223b4307198
Comment 7 Nicolas B. Pierron [:nbp] 2012-05-16 22:09:00 PDT
Previous commits have been reverted, due to errors on tbpl.
Comment 8 Christian Holler (:decoder) 2012-05-18 03:42:06 PDT
JSBugMon: The testcase found in this bug no longer reproduces (tried revision 8c54899dae82).
Comment 9 Nicolas B. Pierron [:nbp] 2012-05-18 12:16:56 PDT
https://hg.mozilla.org/projects/ionmonkey/rev/0f9317cf68d2
Comment 10 Christian Holler (:decoder) 2013-01-14 08:10:26 PST
A testcase for this bug was automatically identified at js/src/jit-test/tests/ion/bug754720.js.

Note You need to log in before you can comment on or make changes to this bug.