[interpreter] add source positions for call and call-new.

This change adds the basic infrastructure to record source
positions for bytecode.

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

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

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

BUILD.gn

@@ -1122,6 +1122,8 @@ source_set("v8_base") {
     "src/interpreter/interpreter.h",
     "src/interpreter/register-translator.cc",
     "src/interpreter/register-translator.h",
+    "src/interpreter/source-position-table.cc",
+    "src/interpreter/source-position-table.h",
     "src/isolate-inl.h",
     "src/isolate.cc",
     "src/isolate.h",

src/DEPS

@@ -12,6 +12,7 @@ include_rules = [
   "+src/interpreter/bytecode-array-iterator.h",
   "+src/interpreter/bytecodes.h",
   "+src/interpreter/interpreter.h",
+  "+src/interpreter/source-position-table.h",
   "-src/libplatform",
   "-include/libplatform"
 ]

src/interpreter/bytecode-array-builder.cc

@@ -73,6 +73,7 @@ BytecodeArrayBuilder::BytecodeArrayBuilder(Isolate* isolate, Zone* zone,
       bytecode_generated_(false),
       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),
@@ -122,10 +123,13 @@ Handle<BytecodeArray> BytecodeArrayBuilder::ToBytecodeArray() {
   int frame_size = register_count * kPointerSize;
   Handle<FixedArray> constant_pool = constant_array_builder()->ToFixedArray();
   Handle<FixedArray> handler_table = handler_table_builder()->ToHandlerTable();
+  Handle<FixedArray> source_position_table =
+      source_position_table_builder()->ToFixedArray();
   Handle<BytecodeArray> output = isolate_->factory()->NewBytecodeArray(
       bytecode_size, &bytecodes_.front(), frame_size, parameter_count(),
       constant_pool);
   output->set_handler_table(*handler_table);
+  output->set_source_position_table(*source_position_table);
   bytecode_generated_ = true;
   return output;
 }
@@ -1192,6 +1196,18 @@ size_t BytecodeArrayBuilder::GetConstantPoolEntry(Handle<Object> object) {
   return constant_array_builder()->Insert(object);
 }
 
+void BytecodeArrayBuilder::SetStatementPosition(Statement* stmt) {
+  if (stmt->position() == RelocInfo::kNoPosition) return;
+  source_position_table_builder_.AddStatementPosition(
+      static_cast<int>(bytecodes_.size()), stmt->position());
+}
+
+void BytecodeArrayBuilder::SetExpressionPosition(Expression* expr) {
+  if (expr->position() == RelocInfo::kNoPosition) return;
+  source_position_table_builder_.AddExpressionPosition(
+      static_cast<int>(bytecodes_.size()), expr->position());
+}
+
 bool BytecodeArrayBuilder::TemporaryRegisterIsLive(Register reg) const {
   return temporary_register_allocator()->RegisterIsLive(reg);
 }

src/interpreter/bytecode-array-builder.h

@@ -11,6 +11,7 @@
 #include "src/interpreter/constant-array-builder.h"
 #include "src/interpreter/handler-table-builder.h"
 #include "src/interpreter/register-translator.h"
+#include "src/interpreter/source-position-table.h"
 #include "src/zone-containers.h"
 
 namespace v8 {
@@ -255,6 +256,9 @@ 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 SetStatementPosition(Statement* stmt);
+  void SetExpressionPosition(Expression* expr);
+
   // Accessors
   Zone* zone() const { return zone_; }
   TemporaryRegisterAllocator* temporary_register_allocator() {
@@ -345,6 +349,9 @@ class BytecodeArrayBuilder final : public ZoneObject, private RegisterMover {
   HandlerTableBuilder* handler_table_builder() {
     return &handler_table_builder_;
   }
+  SourcePositionTableBuilder* source_position_table_builder() {
+    return &source_position_table_builder_;
+  }
   RegisterTranslator* register_translator() { return &register_translator_; }
 
   Isolate* isolate_;
@@ -353,6 +360,7 @@ class BytecodeArrayBuilder final : public ZoneObject, private RegisterMover {
   bool bytecode_generated_;
   ConstantArrayBuilder constant_array_builder_;
   HandlerTableBuilder handler_table_builder_;
+  SourcePositionTableBuilder source_position_table_builder_;
   size_t last_block_end_;
   size_t last_bytecode_start_;
   bool exit_seen_in_block_;

src/interpreter/bytecode-generator.cc

@@ -1954,7 +1954,7 @@ void BytecodeGenerator::VisitCall(Call* expr) {
         .StoreAccumulatorInRegister(callee);
   }
 
-  // TODO(rmcilroy): Use CallIC to allow call type feedback.
+  builder()->SetExpressionPosition(expr);
   builder()->Call(callee, receiver, 1 + args->length(),
                   feedback_index(expr->CallFeedbackICSlot()));
   execution_result()->SetResultInAccumulator();
@@ -1968,6 +1968,7 @@ void BytecodeGenerator::VisitCallNew(CallNew* expr) {
 
   ZoneList<Expression*>* args = expr->arguments();
   Register first_arg = VisitArguments(args);
+  builder()->SetExpressionPosition(expr);
   builder()->New(constructor, first_arg, args->length());
   execution_result()->SetResultInAccumulator();
 }

src/interpreter/source-position-table.cc

@@ -0,0 +1,81 @@
+// Copyright 2016 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/source-position-table.h"
+
+#include "src/assembler.h"
+#include "src/objects-inl.h"
+#include "src/objects.h"
+
+namespace v8 {
+namespace internal {
+namespace interpreter {
+
+class IsStatementField : public BitField<bool, 0, 1> {};
+class SourcePositionField : public BitField<int, 1, 30> {};
+
+void SourcePositionTableBuilder::AddStatementPosition(int bytecode_offset,
+                                                      int source_position) {
+  AssertMonotonic(bytecode_offset);
+  uint32_t encoded = IsStatementField::encode(true) |
+                     SourcePositionField::encode(source_position);
+  entries_.push_back({bytecode_offset, encoded});
+}
+
+void SourcePositionTableBuilder::AddExpressionPosition(int bytecode_offset,
+                                                       int source_position) {
+  AssertMonotonic(bytecode_offset);
+  uint32_t encoded = IsStatementField::encode(false) |
+                     SourcePositionField::encode(source_position);
+  entries_.push_back({bytecode_offset, encoded});
+}
+
+Handle<FixedArray> SourcePositionTableBuilder::ToFixedArray() {
+  int length = static_cast<int>(entries_.size());
+  Handle<FixedArray> table =
+      isolate_->factory()->NewFixedArray(length * 2, TENURED);
+  for (int i = 0; i < length; i++) {
+    table->set(i * 2, Smi::FromInt(entries_[i].bytecode_offset));
+    table->set(i * 2 + 1, Smi::FromInt(entries_[i].source_position_and_type));
+  }
+  return table;
+}
+
+SourcePositionTableIterator::SourcePositionTableIterator(
+    BytecodeArray* bytecode_array)
+    : table_(bytecode_array->source_position_table()),
+      index_(0),
+      length_(table_->length()) {
+  DCHECK(table_->length() % 2 == 0);
+  Advance();
+}
+
+void SourcePositionTableIterator::Advance() {
+  if (index_ < length_) {
+    int new_bytecode_offset = Smi::cast(table_->get(index_))->value();
+    // Bytecode offsets are in ascending order.
+    DCHECK(bytecode_offset_ < new_bytecode_offset || index_ == 0);
+    bytecode_offset_ = new_bytecode_offset;
+    uint32_t source_position_and_type =
+        static_cast<uint32_t>(Smi::cast(table_->get(index_ + 1))->value());
+    is_statement_ = IsStatementField::decode(source_position_and_type);
+    source_position_ = SourcePositionField::decode(source_position_and_type);
+  }
+  index_ += 2;
+}
+
+int SourcePositionTableIterator::PositionFromBytecodeOffset(
+    BytecodeArray* bytecode_array, int bytecode_offset) {
+  int last_position = 0;
+  for (SourcePositionTableIterator iterator(bytecode_array);
+       !iterator.done() && iterator.bytecode_offset() <= bytecode_offset;
+       iterator.Advance()) {
+    last_position = iterator.source_position();
+  }
+  return last_position;
+}
+
+}  // namespace interpreter
+}  // namespace internal
+}  // namespace v8

src/interpreter/source-position-table.h

@@ -0,0 +1,82 @@
+// Copyright 2016 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.
+
+#ifndef V8_INTERPRETER_SOURCE_POSITION_TABLE_H_
+#define V8_INTERPRETER_SOURCE_POSITION_TABLE_H_
+
+#include "src/assert-scope.h"
+#include "src/handles.h"
+#include "src/zone.h"
+#include "src/zone-containers.h"
+
+namespace v8 {
+namespace internal {
+
+class BytecodeArray;
+class FixedArray;
+class Isolate;
+
+namespace interpreter {
+
+class SourcePositionTableBuilder {
+ public:
+  explicit SourcePositionTableBuilder(Isolate* isolate, Zone* zone)
+      : isolate_(isolate), entries_(zone) {}
+
+  void AddStatementPosition(int bytecode_offset, int source_position);
+  void AddExpressionPosition(int bytecode_offset, int source_position);
+  Handle<FixedArray> ToFixedArray();
+
+ private:
+  struct Entry {
+    int bytecode_offset;
+    uint32_t source_position_and_type;
+  };
+
+  void AssertMonotonic(int bytecode_offset) {
+    DCHECK(entries_.size() == 0 ||
+           entries_.back().bytecode_offset < bytecode_offset);
+  }
+
+  Isolate* isolate_;
+  ZoneVector<Entry> entries_;
+};
+
+class SourcePositionTableIterator {
+ public:
+  explicit SourcePositionTableIterator(BytecodeArray* bytecode_array);
+
+  static int PositionFromBytecodeOffset(BytecodeArray* bytecode_array,
+                                        int bytecode_offset);
+  void Advance();
+
+  int bytecode_offset() const {
+    DCHECK(!done());
+    return bytecode_offset_;
+  }
+  int source_position() const {
+    DCHECK(!done());
+    return source_position_;
+  }
+  bool is_statement() const {
+    DCHECK(!done());
+    return is_statement_;
+  }
+  bool done() const { return index_ > length_; }
+
+ private:
+  FixedArray* table_;
+  int index_;
+  int length_;
+  bool is_statement_;
+  int bytecode_offset_;
+  int source_position_;
+  DisallowHeapAllocation no_gc;
+};
+
+}  // namespace interpreter
+}  // namespace internal
+}  // namespace v8
+
+#endif  // V8_INTERPRETER_SOURCE_POSITION_TABLE_H_

src/objects.cc

@@ -30,6 +30,7 @@
 #include "src/ic/ic.h"
 #include "src/identity-map.h"
 #include "src/interpreter/bytecodes.h"
+#include "src/interpreter/source-position-table.h"
 #include "src/isolate-inl.h"
 #include "src/key-accumulator.h"
 #include "src/list.h"
@@ -15025,8 +15026,8 @@ void Code::Disassemble(const char* name, std::ostream& os) {  // NOLINT
 #endif  // ENABLE_DISASSEMBLER
 
 int BytecodeArray::SourcePosition(int offset) {
-  // TODO(yangguo): implement this.
-  return 0;
+  return interpreter::SourcePositionTableIterator::PositionFromBytecodeOffset(
+      this, offset);
 }
 
 void BytecodeArray::Disassemble(std::ostream& os) {
@@ -15036,12 +15037,23 @@ void BytecodeArray::Disassemble(std::ostream& os) {
 
   const uint8_t* first_bytecode_address = GetFirstBytecodeAddress();
   int bytecode_size = 0;
+
+  interpreter::SourcePositionTableIterator source_positions(this);
+
   for (int i = 0; i < this->length(); i += bytecode_size) {
     const uint8_t* bytecode_start = &first_bytecode_address[i];
     interpreter::Bytecode bytecode =
         interpreter::Bytecodes::FromByte(bytecode_start[0]);
     bytecode_size = interpreter::Bytecodes::Size(bytecode);
 
+    if (!source_positions.done() && i == source_positions.bytecode_offset()) {
+      os << std::setw(5) << source_positions.source_position();
+      os << (source_positions.is_statement() ? " S> " : " E> ");
+      source_positions.Advance();
+    } else {
+      os << "         ";
+    }
+
     SNPrintF(buf, "%p", bytecode_start);
     os << buf.start() << " : ";
     interpreter::Bytecodes::Decode(os, bytecode_start, parameter_count());
@@ -15064,7 +15076,8 @@ void BytecodeArray::Disassemble(std::ostream& os) {
       SNPrintF(buf, " (%p)", bytecode_start + offset);
       os << buf.start();
     }
-    os << "\n";
+
+    os << std::endl;
   }
 
   if (constant_pool()->length() > 0) {

test/cctest/cctest.status

@@ -599,21 +599,18 @@
   'test-compiler/OptimizedCodeSharing3': [FAIL],
 
   # TODO(rmcilroy,4689): Stack trace line number failures.
-  'test-api/CaptureStackTrace': [FAIL],
-  'test-api/GetStackTraceForUncaughtExceptionFromSimpleStackTrace': [FAIL],
   'test-api/CaptureStackTraceForUncaughtException': [FAIL],
   'test-api/GetStackTraceContainsFunctionsWithFunctionName': [FAIL],
   'test-run-jsexceptions/ThrowMessagePosition': [FAIL],
+  'test-api/TryCatchMixedNesting': [PASS, FAIL],
 
   # TODO(rmcilroy,4689): Stack frame calling frame is_javascript() failure.
   'test-log-stack-tracer/CFromJSStackTrace': [FAIL],
   'test-log-stack-tracer/PureJSStackTrace': [FAIL],
 
   # TODO(rmcilroy,4680): Exception message assert failure.
-  'test-api/CallICFastApi_SimpleSignature_Miss2': [FAIL],
-  'test-api/CallICFastApi_SimpleSignature_Miss2WithProfiler': [FAIL],
-  'test-api/InterceptorCallICFastApi_SimpleSignature_Miss3': [FAIL],
-  'test-api/InterceptorCallICFastApi_SimpleSignature_Miss3WithProfiler': [FAIL],
+  'test-api/CallICFastApi_SimpleSignature_Miss2WithProfiler': [PASS, FAIL],
+  'test-api/InterceptorCallICFastApi_SimpleSignature_Miss3WithProfiler': [PASS, FAIL],
 
   # TODO(rmcilroy,4680): Test assert errors.
   'test-cpu-profiler/CodeEvents': [FAIL],
@@ -632,14 +629,10 @@
   'test-cpu-profiler/NativeAccessorMonomorphicIC': [FAIL],
   'test-cpu-profiler/CollectSampleAPI': [FAIL],
   'test-compiler/FeedbackVectorPreservedAcrossRecompiles': [FAIL],
-  'test-api/ExceptionInNativeScript': [FAIL],
-  'test-api/ExceptionCreateMessage': [FAIL],
-  'test-api/TryFinallyMessage': [FAIL],
   'test-api/DynamicWithSourceURLInStackTraceString': [FAIL],
   'test-api/PromiseRejectCallback': [FAIL],
   'test-api/RethrowBogusErrorStackTrace': [FAIL],
   'test-api/RethrowPrimitiveStackTrace': [FAIL],
-  'test-api/RethrowStackTrace': [FAIL],
   'test-api/TryCatchSourceInfo': [FAIL],
   'test-api/SetJitCodeEventHandler': [FAIL],
   'test-heap/WeakFunctionInConstructor': [FAIL],
@@ -716,7 +709,6 @@
   'test-api/CallAsFunction': [PASS, FAIL],
   'test-api/ConstructorForObject': [PASS, FAIL],
   'test-api/Regress528': [PASS, FAIL],
-  'test-api/TryCatchMixedNesting': [PASS, FAIL],
 }],
 
 ['ignition == True and arch == x64', {

test/mjsunit/ignition/stack-trace-source-position.js

@@ -3,11 +3,19 @@
 // found in the LICENSE file.
 
 // Flags: --ignition --ignition-filter=f
+// Flags: --no-turbo
 
-function f() {
-  return new Error().stack;
+// TODO(yangguo): fix for turbofan
+
+function f(x) {
+  if (x == 0) {
+    return new Error().stack;
+  }
+  return f(x - 1);
 }
 
-// TODO(yangguo): this is just a dummy source position calculated for
-// interpreter bytecode. Update this once something better comes along.
-assertTrue(/at f.*?:\d+:\d+/.test(f()));
+var stack_lines = f(2).split("\n");
+
+assertTrue(/at f \(.*?:12:12\)/.test(stack_lines[1]));
+assertTrue(/at f \(.*?:14:10\)/.test(stack_lines[2]));
+assertTrue(/at f \(.*?:14:10\)/.test(stack_lines[3]));

tools/gyp/v8.gyp

@@ -921,6 +921,8 @@
         '../../src/interpreter/interpreter.h',
         '../../src/interpreter/register-translator.cc',
         '../../src/interpreter/register-translator.h',
+        '../../src/interpreter/source-position-table.cc',
+        '../../src/interpreter/source-position-table.h',
         '../../src/isolate-inl.h',
         '../../src/isolate.cc',
         '../../src/isolate.h',