Lazy deopt:

Remove frames skipped by a throw from the lazy deopt table.

Be careful to unmark the frames first so stack walks can still succeed. Likewise update the frame before removing the table entry in DeoptimizeCopyFrame.

R=fschneider@google.com

Committed: https://github.com/dart-lang/sdk/commit/1c23917b098ef3c07330f273ed06c0a571019242

[rmacnak, 2016-10-04 18:20:07]

rmacnak@google.com changed reviewers:
+ fschneider@google.com

[rmacnak, 2016-10-04 20:50:38]

Description was changed from

==========
Lazy deopt:

The pending deopt table does not form a stack because deopts may be scheduled in
no particular order. It also may contain stale entries if a throw skips over a
pending lazy deopt without itself hitting a lazy deopt. Make sure to update any
stale entries for the same fp when scheduling a lazy deopt so future reads from
the table get the new value.

Clear entries from the pending table only after updating the frame so a stack
walk no longer needs them.
==========

to

==========
Lazy deopt:

Remove frames skipped by a throw from the lazy deopt table. Be careful to unmark
the frames first so stack walks can still succeed.

Likewise update the frame before removing the table entry in
DeoptimizeCopyFrame.
==========

[rmacnak, 2016-10-04 21:01:49]

Updated to clean up in throw.

[Florian Schneider, 2016-10-04 21:32:41]

Lgtm

https://chromiumcodereview.appspot.com/2396433003/diff/20001/runtime/vm/stack...
File runtime/vm/stack_frame.cc (right):

https://chromiumcodereview.appspot.com/2396433003/diff/20001/runtime/vm/stack...
runtime/vm/stack_frame.cc:569: void ValidateFrames() {
#ifdef DEBUG

https://chromiumcodereview.appspot.com/2396433003/diff/20001/runtime/vm/stack...
File runtime/vm/stack_frame.h (right):

https://chromiumcodereview.appspot.com/2396433003/diff/20001/runtime/vm/stack...
runtime/vm/stack_frame.h:62: void UnmarkForLazyDeopt() {
This reads a little weird: Maybe comment that pc_ is computed by consulting the
pending deopt table if necessary. Or can you use GetCallerPc directly here?

ASSERT(pc() != StubCode::DeoptimizeLazyFromReturn_entry()->EntryPoint());

https://chromiumcodereview.appspot.com/2396433003/diff/20001/runtime/vm/stack...
runtime/vm/stack_frame.h:377: void ValidateFrames();
#ifdef DEBUG

[Florian Schneider, 2016-10-04 21:33:06]

Maybe update the CL subject line for a better commit message.

[rmacnak, 2016-10-04 22:11:29]

https://codereview.chromium.org/2396433003/diff/20001/runtime/vm/stack_frame.cc
File runtime/vm/stack_frame.cc (right):

https://codereview.chromium.org/2396433003/diff/20001/runtime/vm/stack_frame....
runtime/vm/stack_frame.cc:569: void ValidateFrames() {
On 2016/10/04 21:32:41, Florian Schneider wrote:
> #ifdef DEBUG

Done.

https://codereview.chromium.org/2396433003/diff/20001/runtime/vm/stack_frame.h
File runtime/vm/stack_frame.h (right):

https://codereview.chromium.org/2396433003/diff/20001/runtime/vm/stack_frame....
runtime/vm/stack_frame.h:62: void UnmarkForLazyDeopt() {
On 2016/10/04 21:32:41, Florian Schneider wrote:
> This reads a little weird: Maybe comment that pc_ is computed by consulting
the
> pending deopt table if necessary. Or can you use GetCallerPc directly here?
> 
> ASSERT(pc() != StubCode::DeoptimizeLazyFromReturn_entry()->EntryPoint());

Added comment and assert.  Can't use GetCallerPc here; pc_ in this frame came
from GetCallerPc on the callee frame in the iterator.

https://codereview.chromium.org/2396433003/diff/20001/runtime/vm/stack_frame....
runtime/vm/stack_frame.h:377: void ValidateFrames();
On 2016/10/04 21:32:41, Florian Schneider wrote:
> #ifdef DEBUG

Done.

[rmacnak, 2016-10-04 22:11:58]

Description was changed from

==========
Lazy deopt:

Remove frames skipped by a throw from the lazy deopt table. Be careful to unmark
the frames first so stack walks can still succeed.

Likewise update the frame before removing the table entry in
DeoptimizeCopyFrame.
==========

to

==========
Remove frames skipped by a throw from the lazy deopt table.

Be careful to unmark the frames first so stack walks can still succeed. Likewise
update the frame before removing the table entry in DeoptimizeCopyFrame.
==========

[rmacnak, 2016-10-04 22:12:59]

Description was changed from

==========
Remove frames skipped by a throw from the lazy deopt table.

Be careful to unmark the frames first so stack walks can still succeed. Likewise
update the frame before removing the table entry in DeoptimizeCopyFrame.
==========

to

==========
Remove frames skipped by a throw from the lazy deopt table.

Be careful to unmark the frames first so stack walks can still succeed. Likewise
update the frame before removing the table entry in DeoptimizeCopyFrame.

R=fschneider@google.com

Committed:
https://github.com/dart-lang/sdk/commit/1c23917b098ef3c07330f273ed06c0a571019242
==========

[rmacnak, 2016-10-04 22:13:00]

Committed patchset #3 (id:40001) manually as
1c23917b098ef3c07330f273ed06c0a571019242 (presubmit successful).