Last Comment Bug 729886 - IonMonkey: Assertion failure: !usedRval_, at ../../vm/Stack.h:207
: IonMonkey: Assertion failure: !usedRval_, at ../../vm/Stack.h:207
Status: RESOLVED FIXED
: 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]
:
: Jason Orendorff [:jorendorff]
Mentors:
Depends on: 732546
Blocks: langfuzz IonFuzz
  Show dependency treegraph
 
Reported: 2012-02-23 04:33 PST by Christian Holler (:decoder)
Modified: 2013-02-07 05:14 PST (History)
6 users (show)
choller: in‑testsuite+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Make dumpStack compatible with IonFrames. (8.12 KB, patch)
2012-03-16 11:18 PDT, Nicolas B. Pierron [:nbp]
dvander: review+
Details | Diff | Splinter Review
Make dumpStack compatible with IonFrames. (12.77 KB, patch)
2012-03-20 15:07 PDT, Nicolas B. Pierron [:nbp]
dvander: review+
Details | Diff | Splinter Review

Description Christian Holler (:decoder) 2012-02-23 04:33:13 PST
The following testcase asserts on ionmonkey revision 5a04fd69aa09 (run with --ion -n -m --ion-eager), tested on 64 bit:


function deep1(x) {
    if (0) {  } 
    else i : dumpStack();
}
for (var i = 0; 1; ++i)
    deep1(i);
Comment 1 David Anderson [:dvander] 2012-02-24 15:44:41 PST
This is a StackIter integration bug. This particular one is shell-only but we should fix it for debugger integration.
Comment 2 Nicolas B. Pierron [:nbp] 2012-03-15 14:58:06 PDT
I can reproduce this error without the interpreter, by using:

for (var i = 0; 1; ++i)
    dumpStack();

(gdb) bt
#0  0x00007ffff7bd046e in raise () from /nix/store/vxycd107wjbhcj720hzkw2px7s7kr724-glibc-2.12.2/lib/libpthread.so.0
#1  0x0000000000825902 in MOZ_Crash () at /home/nicolas/mozilla/ionmonkey/mfbt/Assertions.cpp:79
#2  0x000000000082595b in MOZ_Assert (s=0x86a4ce "!usedRval_", 
    file=0x86a490 "/home/nicolas/mozilla/git/ionmonkey-hgclone/js/src/vm/Stack.h", ln=207)
    at /home/nicolas/mozilla/ionmonkey/mfbt/Assertions.cpp:88
#3  0x0000000000406b13 in js::CallReceiver::callee (this=0x7fffffffa6d0)
    at /home/nicolas/mozilla/git/ionmonkey-hgclone/js/src/vm/Stack.h:207
#4  0x000000000040e499 in DumpStack (cx=0xce8cf0, argc=0, vp=0x7fffffffa770)
    at /home/nicolas/mozilla/git/ionmonkey-hgclone/js/src/shell/js.cpp:2181
#5  0x00007ffff7f49bd5 in ?? ()

(gdb) call js_DumpBacktrace(cx)
#1          (nil)   _build/bug729886.js:2 (0x7ffff0a071f0 @ 22)


Otherwise I got a segmentation fault:

(gdb) bt
#0  0x00000000004044b6 in JS::Value::isObject (this=0xd3004044ae) at /home/nicolas/mozilla/git/ionmonkey-hgclone/js/src/jsapi.h:430
#1  0x00000000004045c4 in JS::Value::toObject (this=0xd3004044ae) at /home/nicolas/mozilla/git/ionmonkey-hgclone/js/src/jsapi.h:543
#2  0x0000000000406b27 in js::CallReceiver::callee (this=0x7fffffffa6c0)
    at /home/nicolas/mozilla/git/ionmonkey-hgclone/js/src/vm/Stack.h:207
#3  0x000000000040e499 in DumpStack (cx=0xce8cf0, argc=0, vp=0x7fffffffa768)
    at /home/nicolas/mozilla/git/ionmonkey-hgclone/js/src/shell/js.cpp:2181
#4  0x00007ffff7f49e0d in ?? ()

(gdb) call js_DumpBacktrace(cx)
#1          (nil)   _build/bug729886.js:4 (0x7ffff0a071f0 @ 7)
#2          (nil)   _build/bug729886.js:7 (0x7ffff0a072e0 @ 29)
Comment 3 Nicolas B. Pierron [:nbp] 2012-03-16 11:18:38 PDT
Created attachment 606648 [details] [diff] [review]
Make dumpStack compatible with IonFrames.
Comment 4 David Anderson [:dvander] 2012-03-19 13:14:09 PDT
Comment on attachment 606648 [details] [diff] [review]
Make dumpStack compatible with IonFrames.

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

r=me with some small changes:

::: js/src/ion/IonFrameIterator.h
@@ +71,5 @@
>      // dependencies, so we need to keep the IonFrameIterator to build a new one
>      // each time and save the pc & script.
>      const IonFrameIterator *bottom_;
>      size_t frameCount_;
> +    void *calleeToken_;

I think we should only use "calleeToken" when we are specifically referring to the word present on the stack. Here, we're forced to create a fake callee token as we iterate over inline frames.

Instead, we should use a JSFunction * here.

::: js/src/ion/IonFrames.cpp
@@ +267,5 @@
> +JSFunction *
> +InlineFrameIterator::callee() const
> +{
> +    if (!js::ion::CalleeTokenIsFunction(calleeToken()))
> +        return NULL;

And here, assert that there is a JSFunction.

::: js/src/shell/js.cpp
@@ +2195,5 @@
>                  v = StringValue(globalStr);
>              }
>          } else {
> +            if (!iter.calleev(&v))
> +                v = StringValue(globalStr);

At this point we should be guaranteed a callee.

::: js/src/vm/Stack.cpp
@@ +1345,5 @@
>  
> +bool
> +StackIter::calleev(Value *v) const
> +{
> +    if (state_ == ION) {

Here, we should assert that it's a function frame.

::: js/src/vm/Stack.h
@@ +1880,5 @@
>      StackFrame *fp() const { JS_ASSERT(!done() && isScript()); return fp_; }
>      Value      *sp() const { JS_ASSERT(!done() && isScript()); return sp_; }
>      jsbytecode *pc() const { JS_ASSERT(!done() && isScripted()); return pc_; }
>      JSScript *script() const { JS_ASSERT(!done() && isScripted()); return script_; }
> +    bool calleev(Value *v) const;

We should only be asking for a callee on function frames. So this should be:
   Value callee() const;
Comment 5 Nicolas B. Pierron [:nbp] 2012-03-20 15:07:29 PDT
Created attachment 607736 [details] [diff] [review]
Make dumpStack compatible with IonFrames.

Follow nits, and make a new patch which add needed is*Frame functions to StackIter.  Replace calleeToken by callee and make it unfallible.
Comment 6 David Anderson [:dvander] 2012-03-20 18:00:57 PDT
Comment on attachment 607736 [details] [diff] [review]
Make dumpStack compatible with IonFrames.

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

Nice - bit by bit, StackFrame snooping will die.

::: js/src/vm/Stack.cpp
@@ +1396,5 @@
> +        return ObjectValue(*ionInlineFrames_.callee());
> +      case SCRIPTED:
> +      {
> +        Value v;
> +        if (!fp()->getValidCalleeObject(cx_, &v))

Hrmm... I didn't realize this was fallible. Whoops. Maybe this function should take in a Value * after all, and bool return should indicate failure.

(Or, maybe it doesn't matter - I hear we want to remove the method barrier and that would make this infallible.)
Comment 7 Nicolas B. Pierron [:nbp] 2012-03-23 09:49:26 PDT
https://hg.mozilla.org/projects/ionmonkey/rev/0ff7a7bd318f
Comment 8 Christian Holler (:decoder) 2013-02-07 05:14:34 PST
Automatically extracted testcase for this bug was committed:

https://hg.mozilla.org/mozilla-central/rev/2e891e0db397

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