Closed Bug 759107 Opened 9 years ago Closed 8 years ago

SM: Convert engine to use StackIter and ScriptFrameIter

Categories

(Core :: JavaScript Engine, defect)

defect
Not set
normal

Tracking

()

RESOLVED WONTFIX

People

(Reporter: djvj, Assigned: djvj)

References

Details

(Whiteboard: [js:t])

Attachments

(1 file)

Currently the engine in many places directly grabs the current frame pointer using js_GetTopStackFrame.  These should all be changed to use the proper iterators.

See https://bugzilla.mozilla.org/show_bug.cgi?id=757787 for discussion.
Assignee: general → kvijayan
Depends on: 757787
OS: Mac OS X → All
Hardware: x86 → All
Comment on attachment 627731 [details] [diff] [review]
Patch to assert js_GetTopStackFrame when called from Ion compiled frames

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

Apply nits, and r+.

::: js/src/jscntxt.cpp
@@ +941,5 @@
>  {
>      return cx->hasfp() ? cx->regs().pc : NULL;
>  }
>  
> +#ifdef DEBUG

you might also want to add ifdef JS_ION.
This is not cirtical, because StackIter is totally lacking them for the moment and they are easy fix reported by the compiler.

::: js/src/jscntxtinlines.h
@@ +599,5 @@
>          js::mjit::ExpandInlineFrames(cx->compartment);
>  #endif
>  
> +#ifdef JS_ION
> +#  ifdef DEBUG

nit: only one space indentation, based on other files around.

@@ +600,5 @@
>  #endif
>  
> +#ifdef JS_ION
> +#  ifdef DEBUG
> +    bool js_InIonFrame(JSContext *cx);

nit: Declare this function out side the body of js_GetTopStackFrame, this is not guarantee to work on all compilers.
Attachment #627731 - Flags: review?(nicolas.b.pierron) → review+
Whiteboard: [js:t]
Depends on: 732653
No longer depends on: 757787
Depends on: 757787
No longer relevant as js_GetTopStackFrame was eliminated.
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.