[interpreter, debugger] implement bytecode break location iterator.

R=rmcilroy@chromium.org, vogelheim@chromium.org
BUG=v8:4690
LOG=N

Review URL: https://codereview.chromium.org/1682853004

Cr-Commit-Position: refs/heads/master@{#33904}

src/debug/debug.cc

@@ -16,6 +16,7 @@
 #include "src/frames-inl.h"
 #include "src/full-codegen/full-codegen.h"
 #include "src/global-handles.h"
+#include "src/interpreter/bytecodes.h"
 #include "src/isolate-inl.h"
 #include "src/list.h"
 #include "src/log.h"
@@ -69,24 +70,22 @@ BreakLocation::BreakLocation(Handle<DebugInfo> debug_info, DebugBreakType type,
 
 BreakLocation::Iterator* BreakLocation::GetIterator(
     Handle<DebugInfo> debug_info, BreakLocatorType type) {
-  if (debug_info->shared()->HasBytecodeArray()) {
-    UNIMPLEMENTED();
-    return nullptr;
+  if (debug_info->abstract_code()->IsBytecodeArray()) {
+    return new BytecodeArrayIterator(debug_info, type);
+  } else {
+    return new CodeIterator(debug_info, type);
   }
-  return new CodeIterator(debug_info, type);
 }
 
-BreakLocation::Iterator::Iterator(Handle<DebugInfo> debug_info,
-                                  BreakLocatorType type)
+BreakLocation::Iterator::Iterator(Handle<DebugInfo> debug_info)
     : debug_info_(debug_info),
       break_index_(-1),
       position_(1),
-      statement_position_(1) {
-}
+      statement_position_(1) {}
 
 BreakLocation::CodeIterator::CodeIterator(Handle<DebugInfo> debug_info,
                                           BreakLocatorType type)
-    : Iterator(debug_info, type),
+    : Iterator(debug_info),
       reloc_iterator_(debug_info->abstract_code()->GetCode(),
                       GetModeMask(type)) {
   if (!Done()) Next();
@@ -109,8 +108,7 @@ void BreakLocation::CodeIterator::Next() {
   DisallowHeapAllocation no_gc;
   DCHECK(!Done());
 
-  // Iterate through reloc info for code and original code stopping at each
-  // breakable code target.
+  // Iterate through reloc info stopping at each breakable code target.
   bool first = break_index_ == -1;
   while (!Done()) {
     if (!first) reloc_iterator_.next();
@@ -169,6 +167,68 @@ BreakLocation BreakLocation::CodeIterator::GetBreakLocation() {
                        statement_position());
 }
 
+BreakLocation::BytecodeArrayIterator::BytecodeArrayIterator(
+    Handle<DebugInfo> debug_info, BreakLocatorType type)
+    : Iterator(debug_info),
+      source_position_iterator_(
+          debug_info->abstract_code()->GetBytecodeArray()),
+      break_locator_type_(type),
+      start_position_(debug_info->shared()->start_position()) {
+  if (!Done()) Next();
+}
+
+void BreakLocation::BytecodeArrayIterator::Next() {
+  DisallowHeapAllocation no_gc;
+  DCHECK(!Done());
+  bool first = break_index_ == -1;
+  while (!Done()) {
+    if (!first) source_position_iterator_.Advance();
+    first = false;
+    if (Done()) return;
+    position_ = source_position_iterator_.source_position() - start_position_;
+    if (source_position_iterator_.is_statement()) {
+      statement_position_ = position_;
+    }
+    DCHECK(position_ >= 0);
+    DCHECK(statement_position_ >= 0);
+    break_index_++;
+
+    if (break_locator_type_ == ALL_BREAK_LOCATIONS) break;
+
+    DCHECK_EQ(CALLS_AND_RETURNS, break_locator_type_);
+    enum DebugBreakType type = GetDebugBreakType();
+    if (type == DEBUG_BREAK_SLOT_AT_CALL ||
+        type == DEBUG_BREAK_SLOT_AT_RETURN) {
+      break;
+    }
+  }
+}
+
+BreakLocation::DebugBreakType
+BreakLocation::BytecodeArrayIterator::GetDebugBreakType() {
+  BytecodeArray* bytecode_array =
+      debug_info_->abstract_code()->GetBytecodeArray();
+  interpreter::Bytecode bytecode =
+      static_cast<interpreter::Bytecode>(bytecode_array->get(code_offset()));
+
+  if (bytecode == interpreter::Bytecode::kDebugger) {
+    return DEBUGGER_STATEMENT;
+  } else if (bytecode == interpreter::Bytecode::kReturn) {
+    return DEBUG_BREAK_SLOT_AT_RETURN;
+  } else if (interpreter::Bytecodes::IsCallOrNew(bytecode)) {
+    return DEBUG_BREAK_SLOT_AT_CALL;
+  } else if (source_position_iterator_.is_statement()) {
+    return DEBUG_BREAK_SLOT;
+  } else {
+    return NOT_DEBUG_BREAK;
+  }
+}
+
+BreakLocation BreakLocation::BytecodeArrayIterator::GetBreakLocation() {
+  return BreakLocation(debug_info_, GetDebugBreakType(), code_offset(),
+                       position(), statement_position());
+}
+
 // Find the break point at the supplied address, or the closest one before
 // the address.
 BreakLocation BreakLocation::FromCodeOffset(Handle<DebugInfo> debug_info,
@@ -342,7 +402,7 @@ void BreakLocation::SetDebugBreak() {
     DebugCodegen::PatchDebugBreakSlot(isolate(), pc, target);
     DCHECK(IsDebugBreak());
   } else {
-    UNIMPLEMENTED();
+    // TODO(yangguo): implement this once we have a way to record break points.
   }
 }
 
@@ -359,7 +419,7 @@ void BreakLocation::ClearDebugBreak() {
     DebugCodegen::ClearDebugBreakSlot(isolate(), pc);
     DCHECK(!IsDebugBreak());
   } else {
-    UNIMPLEMENTED();
+    // TODO(yangguo): implement this once we have a way to record break points.
   }
 }
 
@@ -373,7 +433,7 @@ bool BreakLocation::IsDebugBreak() const {
     Address pc = code->instruction_start() + code_offset();
     return DebugCodegen::DebugBreakSlotIsPatched(pc);
   } else {
-    UNIMPLEMENTED();
+    // TODO(yangguo): implement this once we have a way to record break points.
     return false;
   }
 }
@@ -1163,6 +1223,10 @@ class RedirectActiveFunctions : public ThreadVisitor {
     for (JavaScriptFrameIterator it(isolate, top); !it.done(); it.Advance()) {
       JavaScriptFrame* frame = it.frame();
       JSFunction* function = frame->function();
+      if (frame->is_interpreted()) {
+        // TODO(yangguo): replace dispatch table for activated frames.
+        continue;
+      }
       if (frame->is_optimized()) continue;
       if (!function->Inlines(shared_)) continue;
 

src/debug/debug.h

@@ -16,6 +16,7 @@
 #include "src/flags.h"
 #include "src/frames.h"
 #include "src/hashmap.h"
+#include "src/interpreter/source-position-table.h"
 #include "src/runtime/runtime.h"
 #include "src/string-stream.h"
 #include "src/v8threads.h"
@@ -84,6 +85,10 @@ class BreakLocation {
 
   inline bool IsReturn() const { return type_ == DEBUG_BREAK_SLOT_AT_RETURN; }
   inline bool IsCall() const { return type_ == DEBUG_BREAK_SLOT_AT_CALL; }
+  inline bool IsDebugBreakSlot() const { return type_ >= DEBUG_BREAK_SLOT; }
+  inline bool IsDebuggerStatement() const {
+    return type_ == DEBUGGER_STATEMENT;
+  }
   inline bool HasBreakPoint() const {
     return debug_info_->HasBreakPoint(code_offset_);
   }
@@ -106,7 +111,7 @@ class BreakLocation {
     return debug_info_->abstract_code();
   }
 
- private:
+ protected:
   enum DebugBreakType {
     NOT_DEBUG_BREAK,
     DEBUGGER_STATEMENT,
@@ -136,7 +141,7 @@ class BreakLocation {
     inline int statement_position() const { return statement_position_; }
 
    protected:
-    Iterator(Handle<DebugInfo> debug_info, BreakLocatorType type);
+    explicit Iterator(Handle<DebugInfo> debug_info);
 
     Handle<DebugInfo> debug_info_;
     int break_index_;
@@ -151,7 +156,7 @@ class BreakLocation {
   class CodeIterator : public Iterator {
    public:
     CodeIterator(Handle<DebugInfo> debug_info, BreakLocatorType type);
-    ~CodeIterator() override{};
+    ~CodeIterator() override {}
 
     BreakLocation GetBreakLocation() override;
     bool Done() const override { return reloc_iterator_.done(); }
@@ -172,9 +177,32 @@ class BreakLocation {
     DISALLOW_COPY_AND_ASSIGN(CodeIterator);
   };
 
+  class BytecodeArrayIterator : public Iterator {
+   public:
+    BytecodeArrayIterator(Handle<DebugInfo> debug_info, BreakLocatorType type);
+    ~BytecodeArrayIterator() override {}
+
+    BreakLocation GetBreakLocation() override;
+    bool Done() const override { return source_position_iterator_.done(); }
+    void Next() override;
+
+    int code_offset() override {
+      return source_position_iterator_.bytecode_offset();
+    }
+
+   private:
+    DebugBreakType GetDebugBreakType();
+
+    interpreter::SourcePositionTableIterator source_position_iterator_;
+    BreakLocatorType break_locator_type_;
+    int start_position_;
+    DISALLOW_COPY_AND_ASSIGN(BytecodeArrayIterator);
+  };
+
   static Iterator* GetIterator(Handle<DebugInfo> debug_info,
                                BreakLocatorType type = ALL_BREAK_LOCATIONS);
 
+ private:
   friend class Debug;
 
   static int BreakIndexFromCodeOffset(Handle<DebugInfo> debug_info, int offset);
@@ -182,11 +210,6 @@ class BreakLocation {
   void SetDebugBreak();
   void ClearDebugBreak();
 
-  inline bool IsDebuggerStatement() const {
-    return type_ == DEBUGGER_STATEMENT;
-  }
-  inline bool IsDebugBreakSlot() const { return type_ >= DEBUG_BREAK_SLOT; }
-
   Handle<DebugInfo> debug_info_;
   int code_offset_;
   DebugBreakType type_;

src/interpreter/bytecode-array-builder.cc

@@ -3,6 +3,7 @@
 // found in the LICENSE file.
 
 #include "src/interpreter/bytecode-array-builder.h"
+#include "src/compiler.h"
 
 namespace v8 {
 namespace internal {
@@ -115,7 +116,7 @@ bool BytecodeArrayBuilder::RegisterIsParameterOrLocal(Register reg) const {
 
 Handle<BytecodeArray> BytecodeArrayBuilder::ToBytecodeArray() {
   DCHECK_EQ(bytecode_generated_, false);
-  EnsureReturn();
+  DCHECK(exit_seen_in_block_);
 
   int bytecode_size = static_cast<int>(bytecodes_.size());
   int register_count =
@@ -1074,10 +1075,10 @@ void BytecodeArrayBuilder::LeaveBasicBlock() {
   exit_seen_in_block_ = false;
 }
 
-
-void BytecodeArrayBuilder::EnsureReturn() {
+void BytecodeArrayBuilder::EnsureReturn(FunctionLiteral* literal) {
   if (!exit_seen_in_block_) {
     LoadUndefined();
+    SetReturnPosition(literal);
     Return();
   }
 }
@@ -1213,6 +1214,11 @@ size_t BytecodeArrayBuilder::GetConstantPoolEntry(Handle<Object> object) {
   return constant_array_builder()->Insert(object);
 }
 
+void BytecodeArrayBuilder::SetReturnPosition(FunctionLiteral* fun) {
+  int pos = std::max(fun->start_position(), fun->end_position() - 1);
+  source_position_table_builder_.AddStatementPosition(bytecodes_.size(), pos);
+}
+
 void BytecodeArrayBuilder::SetStatementPosition(Statement* stmt) {
   if (stmt->position() == RelocInfo::kNoPosition) return;
   source_position_table_builder_.AddStatementPosition(bytecodes_.size(),

src/interpreter/bytecode-array-builder.h

@@ -256,6 +256,7 @@ class BytecodeArrayBuilder final : public ZoneObject, private RegisterMover {
   // entry, so that it can be referenced by above exception handling support.
   int NewHandlerEntry() { return handler_table_builder()->NewHandlerEntry(); }
 
+  void SetReturnPosition(FunctionLiteral* fun);
   void SetStatementPosition(Statement* stmt);
   void SetExpressionPosition(Expression* expr);
 
@@ -268,6 +269,8 @@ class BytecodeArrayBuilder final : public ZoneObject, private RegisterMover {
     return &temporary_allocator_;
   }
 
+  void EnsureReturn(FunctionLiteral* literal);
+
  private:
   class PreviousBytecodeHelper;
   friend class BytecodeRegisterAllocator;
@@ -324,7 +327,6 @@ class BytecodeArrayBuilder final : public ZoneObject, private RegisterMover {
       const ZoneVector<uint8_t>::iterator& jump_location, int delta);
 
   void LeaveBasicBlock();
-  void EnsureReturn();
 
   bool OperandIsValid(Bytecode bytecode, int operand_index,
                       uint32_t operand_value) const;

src/interpreter/bytecode-generator.cc

@@ -574,6 +574,7 @@ Handle<BytecodeArray> BytecodeGenerator::MakeBytecode(CompilationInfo* info) {
     MakeBytecodeBody();
   }
 
+  builder()->EnsureReturn(info->literal());
   set_scope(nullptr);
   set_info(nullptr);
   return builder()->ToBytecodeArray();
@@ -805,6 +806,7 @@ void BytecodeGenerator::VisitStatements(ZoneList<Statement*>* statements) {
 
 
 void BytecodeGenerator::VisitExpressionStatement(ExpressionStatement* stmt) {
+  builder()->SetStatementPosition(stmt);
   VisitForEffect(stmt->expression());
 }
 
@@ -860,6 +862,7 @@ void BytecodeGenerator::VisitBreakStatement(BreakStatement* stmt) {
 
 void BytecodeGenerator::VisitReturnStatement(ReturnStatement* stmt) {
   VisitForAccumulatorValue(stmt->expression());
+  builder()->SetReturnPosition(info_->literal());
   execution_control()->ReturnAccumulator();
 }
 
@@ -1215,6 +1218,7 @@ void BytecodeGenerator::VisitTryFinallyStatement(TryFinallyStatement* stmt) {
 
 
 void BytecodeGenerator::VisitDebuggerStatement(DebuggerStatement* stmt) {
+  builder()->SetStatementPosition(stmt);
   builder()->Debugger();
 }
 

src/interpreter/bytecodes.cc

@@ -259,6 +259,12 @@ bool Bytecodes::IsJump(Bytecode bytecode) {
 }
 
 
+// static
+bool Bytecodes::IsCallOrNew(Bytecode bytecode) {
+  return bytecode == Bytecode::kCall || bytecode == Bytecode::kNew ||
+         bytecode == Bytecode::kCallWide || bytecode == Bytecode::kNewWide;
+}
+
 // static
 bool Bytecodes::IsJumpOrReturn(Bytecode bytecode) {
   return bytecode == Bytecode::kReturn || IsJump(bytecode);

src/interpreter/bytecodes.h

@@ -463,6 +463,9 @@ class Bytecodes {
   // Returns true if the bytecode is a conditional jump, a jump, or a return.
   static bool IsJumpOrReturn(Bytecode bytecode);
 
+  // Returns true if the bytecode is a call or a constructor call.
+  static bool IsCallOrNew(Bytecode bytecode);
+
   // Returns true if |operand_type| is a register index operand (kIdx8/kIdx16).
   static bool IsIndexOperandType(OperandType operand_type);
 

src/interpreter/source-position-table.cc

@@ -18,7 +18,9 @@ class SourcePositionField : public BitField<int, 1, 30> {};
 void SourcePositionTableBuilder::AddStatementPosition(size_t bytecode_offset,
                                                       int source_position) {
   int offset = static_cast<int>(bytecode_offset);
-  AssertMonotonic(offset);
+  // If a position has already been assigned to this bytecode offset,
+  // do not reassign a new statement position.
+  if (CodeOffsetHasPosition(offset)) return;
   uint32_t encoded = IsStatementField::encode(true) |
                      SourcePositionField::encode(source_position);
   entries_.push_back({offset, encoded});
@@ -27,7 +29,9 @@ void SourcePositionTableBuilder::AddStatementPosition(size_t bytecode_offset,
 void SourcePositionTableBuilder::AddExpressionPosition(size_t bytecode_offset,
                                                        int source_position) {
   int offset = static_cast<int>(bytecode_offset);
-  AssertMonotonic(offset);
+  // If a position has already been assigned to this bytecode offset,
+  // do not reassign a new statement position.
+  if (CodeOffsetHasPosition(offset)) return;
   uint32_t encoded = IsStatementField::encode(false) |
                      SourcePositionField::encode(source_position);
   entries_.push_back({offset, encoded});
@@ -38,9 +42,7 @@ void SourcePositionTableBuilder::RevertPosition(size_t bytecode_offset) {
   // If we already added a source position table entry, but the bytecode array
   // builder ended up not outputting a bytecode for the corresponding bytecode
   // offset, we have to remove that entry.
-  if (entries_.size() > 0 && entries_.back().bytecode_offset == offset) {
-    entries_.pop_back();
-  }
+  if (CodeOffsetHasPosition(offset)) entries_.pop_back();
 }
 
 Handle<FixedArray> SourcePositionTableBuilder::ToFixedArray() {

src/interpreter/source-position-table.h

@@ -35,9 +35,10 @@ class SourcePositionTableBuilder {
     uint32_t source_position_and_type;
   };
 
-  void AssertMonotonic(int bytecode_offset) {
-    DCHECK(entries_.size() == 0 ||
-           entries_.back().bytecode_offset < bytecode_offset);
+  bool CodeOffsetHasPosition(int bytecode_offset) {
+    // Return whether bytecode offset already has a position assigned.
+    return entries_.size() > 0 &&
+           entries_.back().bytecode_offset == bytecode_offset;
   }
 
   Isolate* isolate_;

src/objects-inl.h

@@ -5947,7 +5947,8 @@ DebugInfo* SharedFunctionInfo::GetDebugInfo() {
 
 
 bool SharedFunctionInfo::HasDebugCode() {
-  return code()->kind() == Code::FUNCTION && code()->has_debug_break_slots();
+  return HasBytecodeArray() ||
+         (code()->kind() == Code::FUNCTION && code()->has_debug_break_slots());
 }
 
 

test/cctest/cctest.status

@@ -578,8 +578,6 @@
   'test-compiler/OptimizedCodeSharing3': [FAIL],
 
   # TODO(rmcilroy,4689): Stack trace line number failures.
-  'test-api/CaptureStackTraceForUncaughtException': [FAIL],
-  'test-api/GetStackTraceContainsFunctionsWithFunctionName': [FAIL],
   'test-run-jsexceptions/ThrowMessagePosition': [FAIL],
   'test-api/TryCatchMixedNesting': [PASS, FAIL],
 
@@ -617,11 +615,7 @@
   'test-profile-generator/RecordStackTraceAtStartProfiling': [FAIL],
   'test-feedback-vector/VectorCallICStates': [FAIL],
   'test-compiler/FeedbackVectorPreservedAcrossRecompiles': [FAIL],
-  'test-api/DynamicWithSourceURLInStackTraceString': [FAIL],
   'test-api/PromiseRejectCallback': [FAIL],
-  'test-api/RethrowBogusErrorStackTrace': [FAIL],
-  'test-api/RethrowPrimitiveStackTrace': [FAIL],
-  'test-api/TryCatchSourceInfo': [FAIL],
   'test-api/SetJitCodeEventHandler': [FAIL],
   'test-heap/WeakFunctionInConstructor': [FAIL],
   'test-heap/Regress169209': [FAIL],

test/cctest/test-debug.cc

@@ -7978,3 +7978,81 @@ TEST(NoInterruptsInDebugListener) {
   v8::Debug::SetDebugEventListener(env->GetIsolate(), NoInterruptsOnDebugEvent);
   CompileRun("void(0);");
 }
+
+class TestBreakLocation : public i::BreakLocation {
+ public:
+  using i::BreakLocation::GetIterator;
+  using i::BreakLocation::Iterator;
+};
+
+TEST(BreakLocationIterator) {
+  DebugLocalContext env;
+  v8::Isolate* isolate = env->GetIsolate();
+  i::Isolate* i_isolate = reinterpret_cast<i::Isolate*>(isolate);
+  v8::HandleScope scope(isolate);
+
+  v8::Local<v8::Value> result = CompileRun(
+      "function f() {\n"
+      "  debugger;   \n"
+      "  f();        \n"
+      "  debugger;   \n"
+      "}             \n"
+      "f");
+  Handle<i::Object> function_obj = v8::Utils::OpenHandle(*result);
+  Handle<i::JSFunction> function = Handle<i::JSFunction>::cast(function_obj);
+  Handle<i::SharedFunctionInfo> shared(function->shared());
+
+  EnableDebugger(isolate);
+  CHECK(i_isolate->debug()->EnsureDebugInfo(shared, function));
+
+  Handle<i::DebugInfo> debug_info(shared->GetDebugInfo());
+  int code_size = debug_info->abstract_code()->Size();
+
+  bool found_return = false;
+  bool found_call = false;
+  bool found_debugger = false;
+
+  // Test public interface.
+  for (int i = 0; i < code_size; i++) {
+    i::BreakLocation location = i::BreakLocation::FromCodeOffset(debug_info, i);
+    if (location.IsCall()) found_call = true;
+    if (location.IsReturn()) found_return = true;
+    if (location.IsDebuggerStatement()) found_debugger = true;
+  }
+  CHECK(found_call);
+  CHECK(found_return);
+  CHECK(found_debugger);
+
+  // Test underlying implementation.
+  TestBreakLocation::Iterator* iterator =
+      TestBreakLocation::GetIterator(debug_info, i::ALL_BREAK_LOCATIONS);
+  CHECK(iterator->GetBreakLocation().IsDebuggerStatement());
+  CHECK_EQ(7, iterator->GetBreakLocation().position());
+  iterator->Next();
+  CHECK(iterator->GetBreakLocation().IsDebugBreakSlot());
+  CHECK_EQ(22, iterator->GetBreakLocation().position());
+  iterator->Next();
+  CHECK(iterator->GetBreakLocation().IsCall());
+  CHECK_EQ(22, iterator->GetBreakLocation().position());
+  iterator->Next();
+  CHECK(iterator->GetBreakLocation().IsDebuggerStatement());
+  CHECK_EQ(37, iterator->GetBreakLocation().position());
+  iterator->Next();
+  CHECK(iterator->GetBreakLocation().IsReturn());
+  CHECK_EQ(50, iterator->GetBreakLocation().position());
+  iterator->Next();
+  CHECK(iterator->Done());
+  delete iterator;
+
+  iterator = TestBreakLocation::GetIterator(debug_info, i::CALLS_AND_RETURNS);
+  CHECK(iterator->GetBreakLocation().IsCall());
+  CHECK_EQ(22, iterator->GetBreakLocation().position());
+  iterator->Next();
+  CHECK(iterator->GetBreakLocation().IsReturn());
+  CHECK_EQ(50, iterator->GetBreakLocation().position());
+  iterator->Next();
+  CHECK(iterator->Done());
+  delete iterator;
+
+  DisableDebugger(isolate);
+}

test/mjsunit/mjsunit.status

@@ -910,7 +910,6 @@
   # TODO(rmcilroy,4680): Test assert failures.
   'array-literal-feedback': [FAIL],
   'undetectable-compare': [FAIL],
-  'stack-traces': [FAIL],
   'debug-liveedit-2': [FAIL],
   'compiler/deopt-tonumber-compare': [FAIL],
   'es6/string-search': [FAIL],

test/unittests/interpreter/bytecode-array-builder-unittest.cc

@@ -394,7 +394,8 @@ TEST_F(BytecodeArrayBuilderTest, Constants) {
       .LoadLiteral(large_smi)
       .LoadLiteral(heap_num_1)
       .LoadLiteral(heap_num_1)
-      .LoadLiteral(heap_num_2_copy);
+      .LoadLiteral(heap_num_2_copy)
+      .Return();
 
   Handle<BytecodeArray> array = builder.ToBytecodeArray();
   // Should only have one entry for each identical constant.