IonMonkey: Assert that no Ion frame is miss-aligned.

RESOLVED FIXED in Firefox 38

Status

()

RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: nbp, Assigned: nbp)

Tracking

Trunk
mozilla38
Points:
---

Firefox Tracking Flags

(firefox38 fixed)

Details

Attachments

(1 attachment)

(Assignee)

Description

4 years ago
While investigating the remaining alignment issues, I noticed that it was quite useful to add an assertion at the entry point of the Ion frame.  Doing so simplifies the debugging as the caller just pushed its return address.

Note: Unfortunately, it is hard to identify the caller without doing a similar trick as [1], but this is something I intend to address later.

[1] https://developer.mozilla.org/en-US/docs/Mozilla/Projects/SpiderMonkey/Hacking_Tips#Finding_the_script_of_Ion_generated_assembly_%28from_gdb%29
(Assignee)

Comment 1

4 years ago
Created attachment 8560577 [details] [diff] [review]
IonMonkey: Assert that Ion frames are properly aligned.
Attachment #8560577 - Flags: review?(benj)
Comment on attachment 8560577 [details] [diff] [review]
IonMonkey: Assert that Ion frames are properly aligned.

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

Asserts, asserts everywhere.

::: js/src/jit/CodeGenerator.cpp
@@ +1996,5 @@
>      // to 0, before reserving the stack.
>      MOZ_ASSERT(masm.framePushed() == frameSize());
>      masm.setFramePushed(0);
> +
> +    // Ensure that the Ion frames is properly aligned.

nit: either s/is/are, or s/frames/frame :) (comment applies to this file and platform specific codegen functions)

@@ +1997,5 @@
>      MOZ_ASSERT(masm.framePushed() == frameSize());
>      masm.setFramePushed(0);
> +
> +    // Ensure that the Ion frames is properly aligned.
> +    masm.assertStackAlignment(JitStackAlignment, 0);

nit: offset is 0 by default (comment applies to this file and platform specific codegen functions)

::: js/src/jit/JitFrames.cpp
@@ +2761,5 @@
>                  // everything can properly be aligned before adding complexity.
>                  MOZ_RELEASE_ASSERT(frames.ionScript()->frameSize() % JitStackAlignment == 0,
>                    "Ensure that if the Ion frame is aligned, then the spill base is also aligned");
>  
> +                if (isScriptedCallee) {

I found the definition of isScriptedCallee in bug 1112160.  However, it seems that isScriptedCallee, unless I missed a patch, is set for the next loop turn.  So, in the next loop turn, this doesn't apply to the callee anymore, but to the caller.  How about renaming it to wasScriptedCallee, or isScriptedCaller?
Attachment #8560577 - Flags: review?(benj) → review+
https://hg.mozilla.org/mozilla-central/rev/e7e6e82492cb
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
status-firefox38: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla38
You need to log in before you can comment on or make changes to this bug.