IonMonkey: Assertion failure: !usedRval_, at ../../vm/Stack.h:207

RESOLVED FIXED

Status

()

Core
JavaScript Engine
--
major
RESOLVED FIXED
5 years ago
4 years ago

People

(Reporter: decoder, Assigned: nbp)

Tracking

(Blocks: 2 bugs, {assertion, testcase})

Other Branch
x86_64
Linux
assertion, testcase
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

5 years ago
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);
This is a StackIter integration bug. This particular one is shell-only but we should fix it for debugger integration.
Depends on: 732546
(Assignee)

Comment 2

5 years ago
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)
(Assignee)

Updated

5 years ago
Assignee: general → nicolas.b.pierron
Status: NEW → ASSIGNED
(Assignee)

Comment 3

5 years ago
Created attachment 606648 [details] [diff] [review]
Make dumpStack compatible with IonFrames.
Attachment #606648 - Flags: review?(dvander)
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;
Attachment #606648 - Flags: review?(dvander) → review+
(Assignee)

Comment 5

5 years ago
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.
Attachment #606648 - Attachment is obsolete: true
Attachment #607736 - Flags: review?(dvander)
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.)
Attachment #607736 - Flags: review?(dvander) → review+
(Assignee)

Comment 7

5 years ago
https://hg.mozilla.org/projects/ionmonkey/rev/0ff7a7bd318f
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
(Reporter)

Comment 8

4 years ago
Automatically extracted testcase for this bug was committed:

https://hg.mozilla.org/mozilla-central/rev/2e891e0db397
Flags: in-testsuite+
You need to log in before you can comment on or make changes to this bug.