Closed Bug 1075488 Opened 5 years ago Closed 5 years ago

RInstructionResult: Assertion failure while getting a the registered instance from the activation.

Categories

(Core :: JavaScript Engine: JIT, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla35
Tracking Status
firefox35 --- affected

People

(Reporter: nbp, Assigned: nbp)

References

Details

Attachments

(1 file)

When we do an early recoverery of instruction results, the code used by MaybeRead register the vector on the activation and then fetch it back, but this does not work as the instruction result is not yet initialized by the compute function, and as such the read return a nullptr.

This implies that in optimized builds we would be recovering instructions twice.
Summary: RInstructionResult: Assertion failure while getting an the registered instance from the activation. → RInstructionResult: Assertion failure while getting a the registered instance from the activation.
Comment on attachment 8498139 [details] [diff] [review]
Set the frame pointer on RInstructionResults for lookup.

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

::: js/src/jit/IonFrames.cpp
@@ +1453,1 @@
>      src.fp_ = nullptr;

I think this line can now get removed?
we use the src.initialized now.

::: js/src/jit/JitFrameIterator.h
@@ +274,5 @@
>  
>      // The frame pointer is used as a key to check if the current frame already
>      // bailed out.
>      IonJSFrameLayout *fp_;
> +    bool initialized_;

can you add a newline above?
bonus points for description, but it's quite obvious from name, off course.

::: js/src/vm/Stack.cpp
@@ +1557,2 @@
>      // Check that there is no entry in the vector yet.
> +    MOZ_ASSERT(!maybeIonFrameRecovery(fp));

What about removing the "IonJSFrameLayout *fp" argument and take it from the "RInstructionResults"?
Attachment #8498139 - Flags: review?(hv1989) → review+
(In reply to Hannes Verschore [:h4writer] from comment #2)
> ::: js/src/jit/IonFrames.cpp
> @@ +1453,1 @@
> >      src.fp_ = nullptr;
> 
> I think this line can now get removed?
> we use the src.initialized now.

Removed.

> ::: js/src/jit/JitFrameIterator.h
> @@ +274,5 @@
> >  
> >      // The frame pointer is used as a key to check if the current frame already
> >      // bailed out.
> >      IonJSFrameLayout *fp_;
> > +    bool initialized_;
> 
> can you add a newline above?

Done.

> bonus points for description, but it's quite obvious from name, off course.

And done.
What can I do with the bonus points, is there a reward program?

> ::: js/src/vm/Stack.cpp
> @@ +1557,2 @@
> >      // Check that there is no entry in the vector yet.
> > +    MOZ_ASSERT(!maybeIonFrameRecovery(fp));
> 
> What about removing the "IonJSFrameLayout *fp" argument and take it from the
> "RInstructionResults"?

Removed, and updated the assertion to use the frame pointer from the argument.
https://hg.mozilla.org/mozilla-central/rev/0b063dbaf154
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla35
Duplicate of this bug: 1077349
You need to log in before you can comment on or make changes to this bug.