980059 - Expose script-less frames via the stack frame iterator

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Luke Wagner [:luke]]

asm.js functions do not have JSScripts which is a problem when trying to expose asm.js functions via ScriptFrameIter.  This bug removes the restriction since most of the stack iterations that actually want to see asm.js (ComputeStackString et al) only need the (filename, lineno, principal).  To do this, ScriptFrameIter is renamed to FrameIter (which can view frames where !hasScript()) and ScriptFrameIter is re-added as a derived class that auto-skips frames with !hasScript()).  Having audited all the uses of ScriptFrameIter, most either aren't looking for asm.js or will never see it so they can keep using ScriptFrameIter without change.

Comment 1

[Luke Wagner [:luke]]

Attachment: tidying

This patch just does some trivial renaming and rearrangement in preparation for the next patches.

Comment 2

[Luke Wagner [:luke]]

Attachment: change-asmjsactivation

This patch makes AsmJSActivation be a real Activation.  ScriptFrameIter just automatically skips over AsmJSActivations for now.

Comment 3

[Luke Wagner [:luke]]

Attachment: add-frame-iter

This patch just splits out FrameIter from ScriptFrameIter, but it still doesn't actually look at asm.js frames.

Comment 4

[Luke Wagner [:luke]]

Attachment: add-asmjs-frame

As a holdover until actual asm.js stack-walking is done, this patch just makes the first asm.js function called in an AsmJSActivation show up in the stack.  This is enough to test all the places where asm.js can show up and give the fuzzers a target.  In another bug I'll make a real AsmJSFrameIterator that behaves and is used like to IonFrameIterator.

Comment 5

[Jan de Mooij [:jandem]]

Comment on attachment 8386366 [details] [diff] [review]
tidying

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

::: js/src/vm/Stack.h
@@ +961,5 @@
>  }
>  
>  /*****************************************************************************/
>  
> +class InterpreterRegs

Nice, I also want to do s/StackFrame/InterpreterFrame soon.

Comment 6

[Luke Wagner [:luke]]

Attachment: add-asmjs-frame

Fix thinkos in DescribeScriptedCallerForCompilation caught by debugger tests.

Comment 7

[Luke Wagner [:luke]]

(In reply to Jan de Mooij [:jandem] from comment #5)
> Nice, I also want to do s/StackFrame/InterpreterFrame soon.

That'll be good.

Comment 8

[Jan de Mooij [:jandem]]

Comment on attachment 8386370 [details] [diff] [review]
add-frame-iter

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

::: js/src/vm/Stack.h
@@ +1464,5 @@
> +//  - When provided, the optional JSPrincipal argument will cause FrameIter to
> +//    only show frames in globals whose JSPrincipals are subsumed (via
> +//    JSSecurityCallbacks::subsume) by the given JSPrincipal.
> +//
> +// Additionally, there are automatically skip certain types of frames:

Nit: fix this sentence (nice comment btw).

Comment 9

[Jan de Mooij [:jandem]]

Comment on attachment 8386814 [details] [diff] [review]
add-asmjs-frame

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

Nice, these changes turned out to be simpler than I expected.

::: js/src/vm/OldDebugAPI.cpp
@@ +901,5 @@
>  js_CallContextDebugHandler(JSContext *cx)
>  {
> +    FrameIter iter(cx);
> +    for (; !iter.done(); ++iter) {
> +        // If there is no script to debug, then abort execution from he slow

Nit: s/he/the

::: js/src/vm/Stack.h
@@ +1431,5 @@
>      void *errorRejoinSP_;
>      SPSProfiler *profiler_;
>      void *resumePC_;
>  
> +    // This bits are temporary and will be replaced with real stack-walking

Nit: these bits, or something else.

Comment 10

[Luke Wagner [:luke]]

Attachment: add-non-builtin-frame-iter

Thanks for the fast review!  I just realized that the fix I needed in DescribeScriptedCallerForCompilation was needed in *all* the places where I changed a NonBuiltinScriptFrameIter to a FrameIter.  This makes the need clear for adding a NonBuiltinFrameIter which does the obvious thing.  So one more simple patch.

Comment 11

[Jan de Mooij [:jandem]]

Comment on attachment 8386917 [details] [diff] [review]
add-non-builtin-frame-iter

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

::: js/src/vm/OldDebugAPI.cpp
@@ +907,5 @@
>          if (!iter.hasScript())
>              return false;
>  
>          // Only stop once we reach a non-self-hosted script.
>          if (!iter.script()->selfHosted())

This condition is always true now.

Comment 12

[Luke Wagner [:luke]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/797981dc5695
https://hg.mozilla.org/integration/mozilla-inbound/rev/85f02194cd28
https://hg.mozilla.org/integration/mozilla-inbound/rev/88e543e9677c
https://hg.mozilla.org/integration/mozilla-inbound/rev/7f5a942d240f

Comment 13

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/7f5a942d240f
https://hg.mozilla.org/mozilla-central/rev/88e543e9677c
https://hg.mozilla.org/mozilla-central/rev/85f02194cd28
https://hg.mozilla.org/mozilla-central/rev/797981dc5695