IonMonkey: CloseLiveIterator read a wrong slot: Assertion failure: isObject(), at ./jsapi.h:507

RESOLVED FIXED

Status

()

Core
JavaScript Engine
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: nbp, Assigned: nbp)

Tracking

Other Branch
x86_64
Linux
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Assignee)

Description

5 years ago
The following test case fails  ./js --ion-eager ./jit-test/tests/for-of/throw.js  with
Assertion failure: isObject(), at ./jsapi.h:507

The problem is that stackDepth is inverted with stackSlot.
This test case implies use a throw and iterators.
(Assignee)

Comment 1

5 years ago
Created attachment 618874 [details] [diff] [review]
Fix iterator index in snapshots.

My first assumption about the inverted order appeared to be wrong when looking at another test failure.

I got the it working by looking at the interpreter, which is doing "fp()->base() + tn->stackDepth" which can be translated to "args + nfixed + stackDepth" and "-1" to know the number which have to be skipped before reading.

I added a case which check for the creation of the iterator because the slot may not exists yet in the snapshot or may not be readable yet, such as the case where the failure happens during the VM call to GetIteratorObject (CallIteratorStart) in which case the output register is not yet available, and should not be read out of the snapshot because the function reported a failure.
Attachment #618874 - Flags: review?(dvander)
(Assignee)

Comment 2

5 years ago
Comment on attachment 618874 [details] [diff] [review]
Fix iterator index in snapshots.

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

Fixed locally. (wish we had some-kind of pull requests …)

::: js/src/ion/IonFrames.cpp
@@ +277,2 @@
>      // Skip stack slots until we reach the iterator object.
> +    uint32 base = CountArgSlots(frame.maybeCallee()) + script->nfixed;

script ---> frame.script()
Comment on attachment 618874 [details] [diff] [review]
Fix iterator index in snapshots.

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

::: js/src/ion/IonFrames.cpp
@@ +277,4 @@
>      // Skip stack slots until we reach the iterator object.
> +    uint32 base = CountArgSlots(frame.maybeCallee()) + script->nfixed;
> +    uint32 skipSlots = base + localSlot - 1;
> +    JS_ASSERT(skipSlots + 1 < si.slots());

This assert isn't needed since si.skip() will assert if it reads out-of-bounds.

@@ +302,5 @@
>      JSTryNote *tn = script->trynotes()->vector;
>      JSTryNote *tnEnd = tn + script->trynotes()->length;
>  
>      for (; tn != tnEnd; ++tn) {
> +        if (uint32(pc - script->code) <  tn->start)

Nit: extraneous space after <
Attachment #618874 - Flags: review?(dvander) → review+
(Assignee)

Comment 4

5 years ago
https://hg.mozilla.org/projects/ionmonkey/rev/5fcc03122569
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
(Assignee)

Updated

5 years ago
Duplicate of this bug: 744266
Duplicate of this bug: 746395
You need to log in before you can comment on or make changes to this bug.