Beginning on October 25th, 2016, Persona will no longer be an option for authentication on BMO. For more details see Persona Deprecated.
Last Comment Bug 751635 - IonMonkey: Handle f.arguments: Assertion failure: isFunctionFrame(), at ./vm/Stack.h:807
: IonMonkey: Handle f.arguments: Assertion failure: isFunctionFrame(), at ./vm/...
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: Other Branch
: All Linux
: -- normal (vote)
: ---
Assigned To: Nicolas B. Pierron [:nbp]
: Jason Orendorff [:jorendorff]
Depends on:
Blocks: IonEager
  Show dependency treegraph
Reported: 2012-05-03 10:48 PDT by Nicolas B. Pierron [:nbp]
Modified: 2012-05-07 20:35 PDT (History)
2 users (show)
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

Create unexpected argument object from StackIter. (20.15 KB, patch)
2012-05-03 20:00 PDT, Nicolas B. Pierron [:nbp]
dvander: review+
luke: feedback+
Details | Diff | Splinter Review

Description Nicolas B. Pierron [:nbp] 2012-05-03 10:48:59 PDT
This assertion can be reproduce with js --ion-eager ./jit-test/tests/basic/bug632901.js.  

The relevant backtrace is:

#0  0x00007ffff7bd046e in raise ()
#1  0x0000000000425767 in js::StackFrame::callee (this=0x7ffff0c910b0)
#2  0x000000000062a9d3 in js::ArgumentsObject::createUnexpected (cx=0xd7ce10, fp=0x7ffff0c910b0)
#3  0x00000000004b1b2e in fun_getProperty (cx=0xd7ce10, obj=0x7ffff0a10940, id=..., vp=0x7fffffff7d80)
Comment 1 Nicolas B. Pierron [:nbp] 2012-05-03 20:00:24 PDT
Created attachment 620937 [details] [diff] [review]
Create unexpected argument object from StackIter.

This patch add the minimal support the reported bug.  It provides a similar abstraction to iterate over the arguments of the inlined ion frames.

This patch assert when the number of actual arguments is higher than the number of formal arguments, because this is not handled yet by IonMonkey frames.  Also it lack support of unreadable arguments which may cause a SEGV when we try to access the register referenced by the snapshot.  This deserve another test case and patch following this one, as a second part of this bug.
Comment 2 Luke Wagner [:luke] 2012-05-03 20:03:30 PDT
Comment on attachment 620937 [details] [diff] [review]
Create unexpected argument object from StackIter.

Comment 3 David Anderson [:dvander] 2012-05-07 14:51:13 PDT
Comment on attachment 620937 [details] [diff] [review]
Create unexpected argument object from StackIter.

Review of attachment 620937 [details] [diff] [review]:

Nice work. I'm hoping the edge case doesn't bite us. Hitting it would need to involve an Ion-compiled function |f| that calls a getter or setter that tries to access |f.arguments|. That seems pretty unlikely. Worst case, if this becomes behavior we can't break - we can make sure arguments are stored at getter/setter safepoints.

::: js/src/ion/IonFrameIterator-inl.h
@@ +65,5 @@
> +        s.skip();
> +    while (count--) {
> +        // :TODO: We are not always able to read values from the snapshots,
> +        // some values such as non-gc things may still be live in registers
> +        // and cause a segfault while reading the machine state.

We can't segfault here - instead, populate with UndefinedValue (and maybe an assert so we can catch this easily when debugging).

::: js/src/vm/Stack.cpp
@@ +1467,5 @@
> +      case SCRIPTED:
> +        JS_ASSERT(isFunctionFrame());
> +        return fp()->numActualArgs();
> +      case ION:
> +        // :TODO: We need to handle actual arguments in IonMonkey.  (part of Bug 735406)

Is this :TODO: an action-item, or something that will need to be fixed when bug 735406 lands? If the latter, please clarify and remove the :TODO:.

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