Last Comment Bug 749048 - IonMonkey: CloseLiveIterator read a wrong slot: Assertion failure: isObject(), at ./jsapi.h:507
: IonMonkey: CloseLiveIterator read a wrong slot: Assertion failure: isObject()...
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: Other Branch
: x86_64 Linux
: -- normal (vote)
: ---
Assigned To: Nicolas B. Pierron [:nbp]
:
:
Mentors:
: 744266 746395 (view as bug list)
Depends on:
Blocks: IonEager
  Show dependency treegraph
 
Reported: 2012-04-25 18:30 PDT by Nicolas B. Pierron [:nbp]
Modified: 2012-05-02 10:04 PDT (History)
3 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Fix iterator index in snapshots. (2.45 KB, patch)
2012-04-26 17:24 PDT, Nicolas B. Pierron [:nbp]
dvander: review+
Details | Diff | Splinter Review

Description Nicolas B. Pierron [:nbp] 2012-04-25 18:30:05 PDT
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.
Comment 1 Nicolas B. Pierron [:nbp] 2012-04-26 17:24:43 PDT
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.
Comment 2 Nicolas B. Pierron [:nbp] 2012-04-26 18:04:47 PDT
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 3 David Anderson [:dvander] 2012-04-27 08:31:27 PDT
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 <
Comment 4 Nicolas B. Pierron [:nbp] 2012-04-30 19:46:59 PDT
https://hg.mozilla.org/projects/ionmonkey/rev/5fcc03122569
Comment 5 Nicolas B. Pierron [:nbp] 2012-05-02 09:50:23 PDT
*** Bug 744266 has been marked as a duplicate of this bug. ***
Comment 6 Christian Holler (:decoder) 2012-05-02 10:04:18 PDT
*** Bug 746395 has been marked as a duplicate of this bug. ***

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