[Interpreter] Initial generate-bytecode-expectations implementation.

test/cctest/bytecodechecker.cc

+// 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 <fstream>
+#include <iostream>
+
+#include "include/libplatform/libplatform.h"
+#include "include/v8.h"
+
+#include "src/base/logging.h"
+#include "src/base/smart-pointers.h"
+#include "src/compiler.h"
+
+#include "src/interpreter/bytecode-array-iterator.h"
+#include "src/interpreter/bytecode-generator.h"
+#include "src/interpreter/bytecodes.h"
+#include "src/interpreter/interpreter.h"
+
+using namespace i::interpreter;
+
+namespace {
+
+const char *kIndent = "       ";
+
+class ArrayBufferAllocator final : public v8::ArrayBuffer::Allocator {
+ public:
+  void *Allocate(size_t length) override {
+    void *data = AllocateUninitialized(length);
+    if (data != nullptr) memset(data, 0, length);
+    return data;
+  }
+  void *AllocateUninitialized(size_t length) override { return malloc(length); }
+  void Free(void *data, size_t) override { free(data); }
+};
+
+class V8InitializationScope final {
+ public:
+  explicit V8InitializationScope(const char *program_name);

[rmcilroy, 2016/02/08 13:55:32]

/s/program_name/exec_path

[Stefano Sanfilippo, 2016/02/09 10:09:58]

Done.

+  ~V8InitializationScope();
+
+  v8::Platform *platform() const { return platform_.get(); }
+  v8::Isolate *isolate() const { return isolate_; }
+  i::Isolate *i_isolate() const {
+    return reinterpret_cast<i::Isolate *>(isolate_);
+  }
+
+ private:
+  v8::base::SmartPointer<v8::Platform> platform_;
+  v8::Isolate *isolate_;
+
+  DISALLOW_COPY_AND_ASSIGN(V8InitializationScope);
+};
+
+V8InitializationScope::V8InitializationScope(const char *exec_path)
+    : platform_(v8::platform::CreateDefaultPlatform()) {
+  i::FLAG_ignition = true;
+  i::FLAG_ignition_filter = i::StrDup("*");

[rmcilroy, 2016/02/08 13:55:32]

just drop this (the default is "*")

[Stefano Sanfilippo, 2016/02/09 10:09:58]

Done.

+  i::FLAG_always_opt = false;
+  i::FLAG_allow_natives_syntax = true;
+
+  v8::V8::InitializeICU();
+  v8::V8::InitializeExternalStartupData(exec_path);
+  v8::V8::InitializePlatform(platform_.get());
+  v8::V8::Initialize();
+
+  ArrayBufferAllocator allocator;
+  v8::Isolate::CreateParams create_params;
+  create_params.array_buffer_allocator = &allocator;
+
+  isolate_ = v8::Isolate::New(create_params);
+  i_isolate()->interpreter()->Initialize();
+}
+
+V8InitializationScope::~V8InitializationScope() {
+  isolate_->Dispose();
+  v8::V8::Dispose();
+  v8::V8::ShutdownPlatform();
+}
+
+v8::Local<v8::String> MakeV8LocalStringFromUTF8(
+    const V8InitializationScope &platform, const char *data) {

[rmcilroy, 2016/02/08 13:55:32]

Just pass in the isolate for these functions (instead of V8InitializationScope)

[Stefano Sanfilippo, 2016/02/09 10:09:58]

Done.

+  return v8::String::NewFromUtf8(platform.isolate(), data,
+                                 v8::NewStringType::kNormal)
+      .ToLocalChecked();
+}
+
+static std::string WrapCodeInFunction(const char *function_name,
+                                      const std::string &source_file) {
+  std::ostringstream program_stream;
+  program_stream << "function " << function_name << "() {" << source_file
+                 << "}\n"
+                 << function_name << "();";
+
+  return program_stream.str();
+}
+
+v8::Local<v8::Value> CompileAndRun(const V8InitializationScope &platform,

[rmcilroy, 2016/02/08 13:55:32]

ditto

[Stefano Sanfilippo, 2016/02/09 10:09:58]

Done.

+                                   const v8::Local<v8::Context> &context,
+                                   const char *program) {
+  v8::Local<v8::String> source = MakeV8LocalStringFromUTF8(platform, program);
+  v8::Local<v8::Script> script =
+      v8::Script::Compile(context, source).ToLocalChecked();
+
+  v8::Local<v8::Value> result;
+  bool success = script->Run(context).ToLocal(&result);

[rmcilroy, 2016/02/08 13:55:31]

Just: CHECK(script->Run(context).ToLocal(&result)); 

[Stefano Sanfilippo, 2016/02/09 10:09:58]

Done.

+  CHECK(success);
+
+  return result;
+}
+
+i::Handle<v8::internal::BytecodeArray> GetBytecodeArrayForGlobal(
+    const V8InitializationScope &platform,

[rmcilroy, 2016/02/08 13:55:32]

ditto

[Stefano Sanfilippo, 2016/02/09 10:09:58]

Done.

+    const v8::Local<v8::Context> &context, const char *global_name) {
+  v8::Local<v8::String> v8_global_name =
+      MakeV8LocalStringFromUTF8(platform, global_name);
+  v8::Local<v8::Function> function = v8::Local<v8::Function>::Cast(
+      context->Global()->Get(context, v8_global_name).ToLocalChecked());
+  i::Handle<i::JSFunction> js_function =
+      i::Handle<i::JSFunction>::cast(v8::Utils::OpenHandle(*function));
+
+  i::Handle<i::BytecodeArray> bytecodes =
+      i::handle(js_function->shared()->bytecode_array(), platform.i_isolate());
+
+  return bytecodes;
+}
+
+void PrintBytecodeOperand(std::ostream &stream,
+                          const BytecodeArrayIterator &bytecode_iter,
+                          const Bytecode &bytecode, int op_index) {
+  OperandType op_type = Bytecodes::GetOperandType(bytecode, op_index);
+  OperandSize op_size = Bytecodes::GetOperandSize(bytecode, op_index);
+
+  const char *size_tag;
+  bool size_tag_is_byte = false;
+  switch (op_size) {
+    case OperandSize::kByte:
+      size_tag = "8";
+      size_tag_is_byte = true;
+      break;
+    case OperandSize::kShort:
+      size_tag = "16";
+      break;
+    default:
+      UNREACHABLE();
+      return;
+  }
+
+  if (Bytecodes::IsRegisterOperandType(op_type)) {
+    Register register_value = bytecode_iter.GetRegisterOperand(op_index);
+    stream << 'R';
+    if (!size_tag_is_byte) stream << size_tag;

[rmcilroy, 2016/02/08 13:55:32]

no need for the size_tag_is_byte, just: if (op_size != OperandSize::kByte).

Eventually we can just make this consistent and replace the R() macro in
test-bytecode-generator with an R8().

[Stefano Sanfilippo, 2016/02/09 10:09:58]

Done.
While this changes goes in the direction of greater uniformity, I can see why
R() was chosen in the first place. Both forms are OK for me, and amending the
code is very easy. So, whatever you think is best.

+    stream << '(' << register_value.index() << ')';
+  } else {
+    uint32_t raw_value = bytecode_iter.GetRawOperand(op_index, op_type);
+    stream << 'U' << size_tag << '(' << raw_value << ')';
+  }
+}
+
+void PrintBytecode(std::ostream &stream,
+                   const BytecodeArrayIterator &bytecode_iter) {
+  Bytecode bytecode = bytecode_iter.current_bytecode();
+
+  stream << "B(" << Bytecodes::ToString(bytecode) << ')';
+
+  int operands_count = Bytecodes::NumberOfOperands(bytecode);
+  for (int op_index = 0; op_index < operands_count; ++op_index) {
+    stream << ", ";
+    PrintBytecodeOperand(stream, bytecode_iter, bytecode, op_index);
+  }
+}
+
+std::string RemoveTrailingSpaces(const std::string &source) {

[rmcilroy, 2016/02/08 13:55:32]

is this necessary? The tests shouldn't have any trailing spaces in the js
snippets, should they (and if so we could just fix them)

[Stefano Sanfilippo, 2016/02/09 10:09:58]

This was done to avoid the ".... \n" thing when reading snippet from files,
since most text editors add a trailing new line to the file being edited. WDYT?

[rmcilroy, 2016/02/09 11:58:24]

Most editors have an option to remove trailing spaces on save, and since the
normal use-case of this tool would be reading from test-bytecode-generator
(which already has the trailing spaces trimmed) I'd rather just keep things
simple and remove this for now.

+  std::string trimmed = source;
+
+  auto tail = trimmed.end() - 1;
+  auto begin = trimmed.begin();
+  while (tail >= begin && isspace(*tail)) {
+    --tail;
+  }
+  trimmed.erase(tail + 1, trimmed.end());
+  return trimmed;
+}
+
+std::string QuoteCString(const std::string &source) {
+  std::stringstream quoted_buffer;
+  for (char c : source) {
+    switch (c) {
+      case '"':
+        quoted_buffer << "\\\"";
+        break;
+      case '\n':
+        quoted_buffer << "\\n";
+        break;
+      case '\t':
+        quoted_buffer << "\\t";
+        break;
+      case '\\':
+        quoted_buffer << "\\\\";
+        break;
+      default:
+        quoted_buffer << c;
+        break;
+    }
+  }
+  return quoted_buffer.str();
+}
+
+void PrintBytecodeArray(std::ostream &stream,
+                        i::Handle<i::BytecodeArray> bytecode_array,
+                        const std::string &body, bool print_banner = true) {
+  const int kPointerSize = sizeof(void *);
+
+  if (print_banner) {
+    stream << "// ExpectedSnippet generated by bytecodechecker.\n"

[rmcilroy, 2016/02/08 13:55:32]

Add indent before the comment.

[Stefano Sanfilippo, 2016/02/09 10:09:58]

Done.

+              "// It is intended for diffing and as a help in writing tests.\n"
+              "// DO NOT blindly copy and paste into the test suite.\n"

[rmcilroy, 2016/02/08 13:55:32]

Not sure these two lines are necessary (you could put them in PrintUsage if you
want though).

[Stefano Sanfilippo, 2016/02/09 10:09:58]

Done.

+              "// \n\n";
+  }
+
+  stream << kIndent << "{" << '"' << QuoteCString(RemoveTrailingSpaces(body))

[rmcilroy, 2016/02/08 13:55:32]

Some comments above each block that you print would be good (e.g., "// Print
Javascript snippet.", "Print frame sizes.", etc.)

[Stefano Sanfilippo, 2016/02/09 10:09:58]

Done.

+         << "\",\n"
+         << kIndent;
+
+  int frame_size = bytecode_array->frame_size();
+  if (frame_size > kPointerSize && frame_size % kPointerSize == 0) {

[rmcilroy, 2016/02/08 13:55:32]

frame_size % kPointerSize must always be '0', so no need for that check (you
could add a DCHECK below if you want).

[Stefano Sanfilippo, 2016/02/09 10:09:58]

Done.

+    stream << ' ' << frame_size / kPointerSize << " * kPointerSize,\n"
+           << kIndent;
+  } else if (frame_size == kPointerSize) {
+    stream << " kPointerSize,\n" << kIndent;

[rmcilroy, 2016/02/08 13:55:32]

No need for this special case, just printing '1 * kPointerSize" is fine

[Stefano Sanfilippo, 2016/02/09 10:09:58]

This was done for consistency with the test cases in test-bytecode-generator.cc.
To tell the truth, both forms are used (cfr. test-bytecode-generator.cc line 141
v. line 542), so again it's the same for me.

+  } else if (frame_size == 0) {
+    stream << " 0,\n" << kIndent;
+  } else {
+    stream << frame_size << ",\n" << kIndent;

[rmcilroy, 2016/02/08 13:55:31]

given the comment above, this shouldn't be necessary

[Stefano Sanfilippo, 2016/02/09 10:09:58]

Done.

+  }
+
+  stream << ' ' << bytecode_array->parameter_count() << ",\n"
+         << kIndent << ' ' << bytecode_array->length() << ",\n"
+         << kIndent << " {\n"
+         << kIndent << "     ";
+
+  BytecodeArrayIterator bytecode_iter{bytecode_array};
+
+  for (; !bytecode_iter.done(); bytecode_iter.Advance()) {
+    // Print separator before each instruction, except the first one

[rmcilroy, 2016/02/08 13:55:32]

full-stop after comments (throughout file).

[Stefano Sanfilippo, 2016/02/09 10:09:58]

Done.

+    if (bytecode_iter.current_offset() > 0) {
+      stream << ",\n" << kIndent << "     ";
+    }
+    PrintBytecode(stream, bytecode_iter);
+  }
+
+  stream << "\n" << kIndent << " },\n";
+  stream << kIndent << " // constant pool and handlers here\n";  // placeholder

[rmcilroy, 2016/02/08 13:55:32]

add a // TODO(ssanfillipo): ... above this line instead of the "// placeholder"

[Stefano Sanfilippo, 2016/02/09 10:09:58]

Done.

+  stream << kIndent << "}\n";
+}
+
+void PrintUsage(const char *exec_path) {

[rmcilroy, 2016/02/08 13:55:32]

nit  - PrintUsage just below ReadFromFileOrStdin (above main())

[Stefano Sanfilippo, 2016/02/09 10:09:58]

Done.

+  std::cerr << "Usage: " << exec_path
+            << " [filename.js|-]\n\n"
+               "No arguments or - reads from standard input.\n"
+               "Anything else is interpreted as a filename.\n";
+}
+
+bool ReadFromFileOrStdin(std::string *body, const char *body_filename) {
+  std::stringstream body_buffer;
+  if (strcmp(body_filename, "-") == 0) {
+    body_buffer << std::cin.rdbuf();
+  } else {
+    std::ifstream body_file{body_filename};
+    if (!body_file) return false;
+    body_buffer << body_file.rdbuf();
+  }
+  *body = body_buffer.str();
+  return true;
+}
+
+}  // namespace
+
+int main(int argc, char **argv) {
+  if (argc > 1 && strcmp(argv[1], "--help") == 0) {
+    PrintUsage(argv[0]);
+    return 0;
+  }
+
+  const char *body_filename = (argc > 1 ? argv[1] : "-");
+  const char *wrapper_function_name = "__bytecodechecker_wrapper__";
+
+  std::string body;
+  if (!ReadFromFileOrStdin(&body, body_filename)) {
+    std::cerr << "Could not open '" << body_filename << "'.\n\n";
+    PrintUsage(argv[0]);
+    return 1;
+  }
+
+  V8InitializationScope platform(argv[0]);
+  {
+    v8::Isolate::Scope isolate_scope(platform.isolate());
+    v8::HandleScope handle_scope(platform.isolate());
+    v8::Local<v8::Context> context = v8::Context::New(platform.isolate());
+    v8::Context::Scope context_scope(context);
+
+    std::string source_code = WrapCodeInFunction(wrapper_function_name, body);
+    CompileAndRun(platform, context, source_code.c_str());
+
+    i::Handle<i::BytecodeArray> bytecode_array =
+        GetBytecodeArrayForGlobal(platform, context, wrapper_function_name);
+
+    PrintBytecodeArray(std::cout, bytecode_array, body);
+  }
+}