759107 - SM: Convert engine to use StackIter and ScriptFrameIter

Status: RESOLVED WONTFIX

(Core :: JavaScript Engine, defect)

Description

[Kannan Vijayan [:djvj]]

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.

Comment 1

[Kannan Vijayan [:djvj]]

Attachment: Patch to assert js_GetTopStackFrame when called from Ion compiled frames

Comment 2

[Nicolas B. Pierron [:nbp] {away}]

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.

Comment 3

[Kannan Vijayan [:djvj]]

No longer relevant as js_GetTopStackFrame was eliminated.