127944 - RFE - set next statement

Status: RESOLVED INCOMPLETE

(Other Applications Graveyard :: Venkman JS Debugger, enhancement, P2)

Description

[timeless]

I have code right now that's doing

if (iid.Equals(something))
  return this;
throw Components.results.NS_NOINTERFACE;

venkman caught the exception, and i'd like to change the execution point from the throw to the return.

Comment 1

[Robert Ginda]

Sure this would be killer, but how feasible is this in the current engine?  Is
it possible to determine a range of "safe" pc's we could jump to?  In the 1.0
timeframe?

Comment 2

[Brendan Eich [:brendan]]

I think you have the primitives you need.  You know the line number the debugger
user wants to set as the "next statement".  You have JS_LineNumberToPC.  What am
I missing?

/be

Comment 3

[Robert Ginda]

I'm missing two things, I think.  The first, that there is no public way the set
the current pc of a stack frame, is an easy fix.  The second, which pc's can I
safely jump to without corrupting the stack, may or may not be a figment of my
imagination.  I assume that when we're in something like a switch or a
try/catch, there is cleanup code that must be run in order to cleanly pass
control back to the outer block.  I also assume that this could involve some
stack cleanup, that, if not properly done, could cause the engine to crash, or
at least become confused, when the containing function returns.

Consider:

 1: function f (x, z)
 2: {
 3:   var o = {y: 0};
 4:   try {
 5:     with (o) {
 6:      switch (x) {
 7:        case 1: y++; break;
 8:        case 2: y--; break;
 9:      }
10:    }
11:   } catch (e) {
12:     return "foo";
13:   }
14:   return o;
15: }

Jumping out of the with block, for example, would surely cause /some/ trouble,
right?  Would it be fatal, or just wierd?  How about jumping out, or in, to the
try or catch blocls?  

Comment 4

[timeless]

for now i'd be willing to jump through hoops pseudo-stepping out of things.

suppose i was at line 12, if i could set the next statement to 13, step to 14, 
set to 4, step a bit ...
similarly if i were at 7 and could set to 9, step to 11 and set to 12.

visual studio forces me to be somewhat careful like this for cases where the 
place i'd like to set isn't in the current function.  (otherwise, it just adds 
the new destination on as a new frame -- i think).

i don't mind having to be careful, although certainly allowing me to be 
careless would be nice, but this is a debugger, it's kind of like rocket 
science :)

Comment 5

[Brendan Eich [:brendan]]

rginda: quite right, thanks for telling me what I was missing :-).  To balance
the stack, you could walk through the script's bytecode from its main entry
point to the pc you're trying to set as next statement, computing the stack
depth in the same way that the code generator does.  You'd want to do this in a
new jsdbgapi function.  It would go something like the patch I'm about to attach.

/be

Comment 6

[Brendan Eich [:brendan]]

Attachment: proposed js.msg+jsdbgapi.[ch] extension: JS_SetFramePC

JS_SetFramePC takes care of setting fp->sp for you -- it will fail (return null
after reporting an error) if you try to raise the stack pointer, because it
can't know what values should be pushed (e.g., a With object reference if
you're trying to set-next-statement into the middle of a with statement).

Since this function returns the old fp->pc value for convenience (in the best
case, to save a JS_GetFramePC call), this API must also fail if you're trying
to set the pc before the frame's script has begun execution (because fp->pc
will be null in that case -- note the valuable, unambiguous !fp->pc state
indicator mentioned recently! :-).

/be

Comment 7

[Brendan Eich [:brendan]]

That patch is untested, but it looks good to try, so please test well (all hard
cases: with/for..in/multi-level break/continue/try/catch/finally).

/be

Comment 8

[Brendan Eich [:brendan]]

Attachment: silence is golden -- no new js.msg, just jsdbgapi.[ch]

rginda pointed out the futility and badness of reporting errors when the
debugger UI should do its own spanking.

/be

Comment 9

[Brendan Eich [:brendan]]

Attachment: better loop control to consolidate depth sanity checking (still not bulletproof)

What if you are at line 3 below, and set next statement to line 5?

1:  function f(o) {
2:    for (var i in o)
3:	print(i);
4:    with (o)
5:	print(x);
6:  }
7:  f({x:42});

The stack depth decreases, which seems ok, but what's on the stack at the
bottom of the operand stack is not a With object.  The interpreter will crash
and burn.  More work needed to safen this scenario.

/be

Comment 10

[Robert Ginda]

js_Interpret maintains a local copy of pc (and len), so changing fp->pc doesn't
have any effect.  I've sprinkled |if (pc != fp->pc) { pc = fp->pc; len = 0 }| in
the places where we call out to a debugger hook.  This probably isn't exactly
right, it leaves me with...
Assertion failure: JS_UPTRDIFF(fp->sp, fp->spbase) <= depthdiff, at jsinterp.c:374
...when I try to change the pc from a breakpoint.  I'll investigate further. 
For the simple case I tested, changing the pc from a debugger; keyword works fine.

In the last patch, |depth| needs to be a uintN to avoid signed/unsigned
comparison warnings.  |len| could probably stand to be unsigned as well.

Comment 11

[Robert Ginda]

Attachment: js/jsd patch

Changes to js/jsd, makes the jsdIStackFrame::pc attribute read/write... cool.

To test in venkman, type "loadd chrome://venkman/content/venkman-dev.js",
"treetest", and click "debug".	When you stop for the debugger keyword, type
"evald console.frames[0].pc = 0", and then "step".  If all goes well, you'll be
back at the top of the dbg2() function.

Comment 12

[Robert Ginda]

Attachment: js/src changes

Brendan's last patch plus warning cleanup and jsinterp.c changes.

If I set a breakpoint in line 138 of chrome://venkman/content/tests/tree.js,
hit "debug", type "evald console.frames[0].pc = 0", and "cont", bad things
happen.

Previously I botched the assert at jsinterp.c:374 (noted in comment #10), now
I'm crashing...
Program received signal SIGSEGV, Segmentation fault.
0x400c544a in js_MarkGCThing (cx=0x80d8a30, thing=0x8877f50, arg=0x0)

Setting the pc from a debugger; keyword, however, seems to work fine.

Comment 13

[Robert Ginda]

Brendan, do you want to work on this, or should I future it?

Comment 14

[Brendan Eich [:brendan]]

Sure, I'll take a look -- I may need your help.

/be

Comment 15

[Brendan Eich [:brendan]]

This fits in 1.1beta.

/be

Comment 16

[Brendan Eich [:brendan]]

Argh, missed.  1.2alpha is better than trying to cram this into 1.1.

/be

Comment 17

[Brendan Eich [:brendan]]

Attachment: fresh js/src patch, a few nits picked too

Comment 18

[Robert Ginda]

Attachment: updated js/jsd patch

Comment 19

[Brendan Eich [:brendan]]

Moving out, some of these may move to 1.3alpha shortly.

/be

Comment 20

[Brendan Eich [:brendan]]

Fixing TM.

/be

Comment 22

[John J. Barton]

Thanks to Gijs for pointing this out. I'll try to revive this patch on 1.9.2. If anyone here knows more about the issues than they did in 2002, please speak up.

Comment 23

[Brendan Eich [:brendan]]

This is more doable now thanks to js_ReconstructStackDepth and friends, but it still needs work, and I'm not going to have time to do that work. I'll advise and review though, so a volunteer should take and patch.

/be

Comment 24

[Jim Blandy :jimb]

It seems like the original poster would have been happy with something to force a particular frame to return a particular value, like GDB's 'return' command.

I'm kind of astonished that everyone just jumped into the game of recreating the state needed by some arbitrary expression continuation in arbitrary JavaScript code; you simply don't have enough information to do anything like that in a meaningful way.  A debugger 'return' command is well-defined because it's just using a continuation you already have.

I can imagine a meaning for jumping to an arbitrary statement continuation that is within some lexical scope that encloses the current point of execution.  Those only differ in the PC, and use a scope you already have.

Comment 25

[Brendan Eich [:brendan]]

No arbitrary expression continuations -- statements, rather. SpiderMonkey was simple enough in the old days that you could hope to do this if the current pc was in a post-dominator of the next-statement and all stacked state was from outer statements still active.

The goal was not to support all set-next-statement attempts -- some would fail.

But this is a silly bug to keep around now that we are JITting. Feel free to morph or close.

/be

Comment 26

[John J. Barton]

My primary interest is to halt the execution, change the function, and re-execute it in as many cases as possible. So I want "return immediately but back up the caller just enough to call again" with "I can't do that" being one outcome. (JIT is off for debugging). Java has this support and is it heavily used by eclipse users; it can have a big impact for Web 2.0 app devs.

Comment 27

[Rob Campbell [:rc] (:robcee)]

Change the function or create a function if we're in an exception for an unimplemented function call?

It would be a great feature to be able to sit in a debugger, write the next method we needed and hit continue to jump into the newly-created method.

Debugger-driven development is powerful stuff, though the onus would be on the debugger client implementor to create mechanisms to save these created methods.

I think this feature goes beyond the original scope of this bug though.

Comment 28

[Mike Shaver (:shaver emeritus)]

comment 27 wants to use the __noSuchMethod__ hooks and a bit more trickiness to let the debugger signal that it has defined the method, I think, rather than using a general set-next-statement (since you want it to work as part of expressions).  proxies may make it a bit easier.

Comment 29

[Sylvestre Ledru [:Sylvestre]]

No assignee, updating the status.

Comment 30

[Sylvestre Ledru [:Sylvestre]]

No assignee, updating the status.

Comment 31

[BMO Automation]

Component is obsolete so resolving bugs as INCOMPLETE