Patch CEntry stub to swallow exception if liveedit/restart frame needs it.

Patch CEntry stub to swallow exception if liveedit/restart frame needs it.
BUG=v8:2203

[Peter.Rybin, 2013-08-14 20:18:52]

Hi Yang

This should be the patch for a long annoying bug.
I definitely could use some help with assembler clean-up, because currently it's
very sketchy.

Peter

[Peter.Rybin, 2013-09-03 14:51:32]

ping

[Yang, 2013-09-13 09:47:56]

I'm not entirely familiar with how frame restart works, so let me try to
understand:

When we restart frames, we find the ones we want to restart, clear the
corresponding range on the stack, and replace it with the frame dropper builtin,
which calls the function to the bottom frame again for restart.

The problem we want to solve is that when an exception is thrown, the debugger
becomes active through OnException. Live edit may restart frames in that time.
When we return to throwing exception after the OnException call, the exception
is set as pending exception, so that when we return to javascript through the C
entry stub, the exception triggers the registered try-catch handler. You want to
circumvent this by adding a check in the C entry stub. Right?

My question here is: by skipping the try-catch handler, where is that pending
exception handled? I don't see it being cleared anywhere. So even though we
restarted the frame, the old exception is still pending?

https://codereview.chromium.org/22801003/diff/2001/src/ia32/code-stubs-ia32.cc
File src/ia32/code-stubs-ia32.cc (right):

https://codereview.chromium.org/22801003/diff/2001/src/ia32/code-stubs-ia32.c...
src/ia32/code-stubs-ia32.cc:4877: __ cmp(ebp, edx);
Instead of these 4 instructions, you could just

__ cmp(ebp, Operand::StaticVariable(c_entry_frame_to_ignore_exception));
__ mov(Operand::StaticVariable(c_entry_frame_to_ignore_exception),
Immediate(0));

I don't think mov sets any flags, so you can do the conditional jump afterwards
based on the results of cmp.

https://codereview.chromium.org/22801003/diff/2001/src/x64/code-stubs-x64.cc
File src/x64/code-stubs-x64.cc (right):

https://codereview.chromium.org/22801003/diff/2001/src/x64/code-stubs-x64.cc#...
src/x64/code-stubs-x64.cc:4022: __ cmpq(rbp, rdx);
Similar to ia32.

[Peter.Rybin, 2013-09-16 14:26:38]

On 2013/09/13 09:47:56, Yang wrote:
> I'm not entirely familiar with how frame restart works, so let me try to
> understand:
> 
> When we restart frames, we find the ones we want to restart, clear the
> corresponding range on the stack, and replace it with the frame dropper
builtin,
> which calls the function to the bottom frame again for restart.
> 
> The problem we want to solve is that when an exception is thrown, the debugger
> becomes active through OnException. Live edit may restart frames in that time.
> When we return to throwing exception after the OnException call, the exception
> is set as pending exception, so that when we return to javascript through the
C
> entry stub, the exception triggers the registered try-catch handler. You want
to
> circumvent this by adding a check in the C entry stub. Right?
> 
> My question here is: by skipping the try-catch handler, where is that pending
> exception handled? I don't see it being cleared anywhere. So even though we
> restarted the frame, the old exception is still pending?

Your description is very accurate, no corrections.

Maybe it's better is I reproduce my rationale for this design, for Andrey and
for the records: I don't have tooling to fix the return address the C++ has, so
C++ is determined to always return to CEntry frame. CEntry frame initiates stack
unwind routine, so I cannot let CEntry frame keep doing this.

As to your question, I don't have an answer right now. I remember that I was
under impression that someone must have already cleared pending exception
variable to that point, so exception is only being kept by a register. If this
not a case, I guess there we should add an instruction that does so in the new
code.

[Peter.Rybin, 2013-09-19 14:44:18]

> My question here is: by skipping the try-catch handler, where is that pending
> exception handled? I don't see it being cleared anywhere. So even though we
> restarted the frame, the old exception is still pending?

I guess the following code always takes care of pending exception variable:
  // Clear the pending exception.
  __ mov(edx, Immediate(masm->isolate()->factory()->the_hole_value()));
  __ mov(Operand::StaticVariable(pending_exception_address), edx);


It is from CEntryStub::GenerateCore and it gets executed whenever exception is
being thrown.
So I guess we shouldn't do anything else.

[Peter.Rybin, 2013-09-19 15:35:31]

https://codereview.chromium.org/22801003/diff/2001/src/ia32/code-stubs-ia32.cc
File src/ia32/code-stubs-ia32.cc (right):

https://codereview.chromium.org/22801003/diff/2001/src/ia32/code-stubs-ia32.c...
src/ia32/code-stubs-ia32.cc:4877: __ cmp(ebp, edx);
On 2013/09/13 09:47:56, Yang wrote:
> Instead of these 4 instructions, you could just
> 
> __ cmp(ebp, Operand::StaticVariable(c_entry_frame_to_ignore_exception));
> __ mov(Operand::StaticVariable(c_entry_frame_to_ignore_exception),
> Immediate(0));
> 
> I don't think mov sets any flags, so you can do the conditional jump
afterwards
> based on the results of cmp.

Thanks a lot.
I also moved "clear" statement below the jump, because not matching frames
obviously mustn't clear this pointer.

https://codereview.chromium.org/22801003/diff/2001/src/x64/code-stubs-x64.cc
File src/x64/code-stubs-x64.cc (right):

https://codereview.chromium.org/22801003/diff/2001/src/x64/code-stubs-x64.cc#...
src/x64/code-stubs-x64.cc:4022: __ cmpq(rbp, rdx);
On 2013/09/13 09:47:56, Yang wrote:
> Similar to ia32.

Done.

[aandrey, 2013-09-25 09:19:00]

Yang, could you plz take a look?