[debugger] fix break locations for assignments and return.

src/interpreter/bytecode-array-builder.cc

 // Copyright 2015 the V8 project authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "src/interpreter/bytecode-array-builder.h"
 #include "src/compiler.h"
 
 namespace v8 {
 namespace internal {
 namespace interpreter {
@@ skipping 64 matching lines @@
       constant_array_builder_(isolate, zone),
       handler_table_builder_(isolate, zone),
       source_position_table_builder_(isolate, zone),
       last_block_end_(0),
       last_bytecode_start_(~0),
       exit_seen_in_block_(false),
       unbound_jumps_(0),
       parameter_count_(parameter_count),
       local_register_count_(locals_count),
       context_register_count_(context_count),
+      return_position_(RelocInfo::kNoPosition),
       temporary_allocator_(zone, fixed_register_count()),
       register_translator_(this) {
   DCHECK_GE(parameter_count_, 0);
   DCHECK_GE(context_register_count_, 0);
   DCHECK_GE(local_register_count_, 0);
 }
 
 BytecodeArrayBuilder::~BytecodeArrayBuilder() { DCHECK_EQ(0, unbound_jumps_); }
 
 Register BytecodeArrayBuilder::first_context_register() const {
@@ skipping 859 matching lines @@
 
 
 BytecodeArrayBuilder& BytecodeArrayBuilder::ReThrow() {
   Output(Bytecode::kReThrow);
   exit_seen_in_block_ = true;
   return *this;
 }
 
 
 BytecodeArrayBuilder& BytecodeArrayBuilder::Return() {
+  SetReturnPosition();
   Output(Bytecode::kReturn);
   exit_seen_in_block_ = true;
   return *this;
 }
 
 BytecodeArrayBuilder& BytecodeArrayBuilder::Debugger() {
   Output(Bytecode::kDebugger);
   return *this;
 }
 
@@ skipping 61 matching lines @@
   handler_table_builder()->SetTryRegionEnd(handler_id, bytecodes()->size());
   return *this;
 }
 
 
 void BytecodeArrayBuilder::LeaveBasicBlock() {
   last_block_end_ = bytecodes()->size();
   exit_seen_in_block_ = false;
 }
 
 void BytecodeArrayBuilder::EnsureReturn(FunctionLiteral* literal) {

[vogelheim, 2016/02/29 14:52:32]

Remove literal parameter?

[Yang, 2016/03/01 08:09:18]

Done.

   if (!exit_seen_in_block_) {
     LoadUndefined();
-    SetReturnPosition(literal);
     Return();
   }
   DCHECK(exit_seen_in_block_);
 }
 
 BytecodeArrayBuilder& BytecodeArrayBuilder::Call(Register callable,
                                                  Register receiver_args,
                                                  size_t receiver_args_count,
                                                  int feedback_slot,
                                                  TailCallMode tail_call_mode) {
@@ skipping 110 matching lines @@
                                                    LanguageMode language_mode) {
   Output(BytecodeForDelete(language_mode), object.ToRawOperand());
   return *this;
 }
 
 
 size_t BytecodeArrayBuilder::GetConstantPoolEntry(Handle<Object> object) {
   return constant_array_builder()->Insert(object);
 }
 
-void BytecodeArrayBuilder::SetReturnPosition(FunctionLiteral* fun) {
-  // Don't emit dead code.
+void BytecodeArrayBuilder::InitializeReturnPosition(FunctionLiteral* literal) {
+  DCHECK_EQ(RelocInfo::kNoPosition, return_position_);
+  return_position_ =
+      std::max(literal->start_position(), literal->end_position() - 1);
+}
+
+void BytecodeArrayBuilder::SetReturnPosition() {
+  if (return_position_ == RelocInfo::kNoPosition) return;

[vogelheim, 2016/02/29 14:52:32]

Is this ever supposed to happen?

   if (exit_seen_in_block_) return;
-
-  int pos = std::max(fun->start_position(), fun->end_position() - 1);
-  source_position_table_builder_.AddStatementPosition(bytecodes_.size(), pos);
+  source_position_table_builder_.AddStatementPosition(bytecodes_.size(),
+                                                      return_position_);

[vogelheim, 2016/02/29 14:52:32]

I'm confused at this logic. (Not specifically this part, but the general
idea...)

Is the idea to always set a return position at the end of the function, or is
the plan to set a return position to wherever returns are, but make sure there's
at least one (w/ end of the function being the default)?

If the former, we could just set if once & unconditionally when generating the
table.

If the latter, then I don't think this code does what it's meant to. This will
always set a statement position at the end (return_position_), regardless of
whether it's set via the first Return() call or is the default via
EnsureReturnPosition(). Also, I'd suspect in this case we'd want to set a
statement for each actual Return(), while this would set it only for the first
(since exit_seen_in_block_ will be true after the first Return().

 }
 
 void BytecodeArrayBuilder::SetStatementPosition(Statement* stmt) {
   if (stmt->position() == RelocInfo::kNoPosition) return;
   if (exit_seen_in_block_) return;
   source_position_table_builder_.AddStatementPosition(bytecodes_.size(),
                                                       stmt->position());
 }
 
 void BytecodeArrayBuilder::SetExpressionPosition(Expression* expr) {
@@ skipping 429 matching lines @@
 }
 
 // static
 bool BytecodeArrayBuilder::FitsInReg16OperandUntranslated(Register value) {
   return value.is_short_operand();
 }
 
 }  // namespace interpreter
 }  // namespace internal
 }  // namespace v8