[wasm] Split off debug info from wasm object

src/wasm/wasm-debug.cc

Index: src/wasm/wasm-debug.cc
diff --git a/src/wasm/wasm-debug.cc b/src/wasm/wasm-debug.cc
new file mode 100644
index 0000000000000000000000000000000000000000..bd770de04412f166fae99cea8c8be0e4e06677d0
--- /dev/null
+++ b/src/wasm/wasm-debug.cc
@@ -0,0 +1,184 @@
+// 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/wasm/wasm-debug.h"
+
+#include "src/assert-scope.h"
+#include "src/debug/debug.h"
+#include "src/factory.h"
+#include "src/isolate.h"
+#include "src/wasm/module-decoder.h"
+#include "src/wasm/wasm-module.h"
+
+using namespace v8::internal;
+using namespace v8::internal::wasm;
+
+namespace {
+
+enum {
+  kWasmDebugInfoWasmObj,
+  kWasmDebugInfoWasmBytesHash,
+  kWasmDebugInfoFunctionByteOffsets,
+  kWasmDebugInfoNumEntries
+};
+
+ByteArray *GetOrCreateFunctionOffsetTable(WasmDebugInfo *debug_info) {
+  Object *offset_table = debug_info->get(kWasmDebugInfoFunctionByteOffsets);
+  Isolate *isolate = debug_info->GetIsolate();
+  if (!offset_table->IsUndefined(isolate)) return ByteArray::cast(offset_table);
+
+  FunctionOffsetsResult function_offsets;
+  {
+    DisallowHeapAllocation no_gc;
+    SeqOneByteString *wasm_bytes =
+        wasm::GetWasmBytes(debug_info->wasm_object());
+    const byte *bytes_start = wasm_bytes->GetChars();
+    const byte *bytes_end = bytes_start + wasm_bytes->length();
+    function_offsets = wasm::DecodeWasmFunctionOffsets(bytes_start, bytes_end);
+  }
+  DCHECK(function_offsets.ok());
+  size_t array_size = 2 * sizeof(int) * function_offsets.val.size();
+  CHECK_LE(array_size, static_cast<size_t>(std::numeric_limits<int>::max()));
+  ByteArray *arr =
+      *isolate->factory()->NewByteArray(static_cast<int>(array_size));
+  int idx = 0;
+  for (std::pair<int, int> p : function_offsets.val) {
+    arr->set_int(idx++, p.first);
+    arr->set_int(idx++, p.second);
+  }
+  DCHECK_EQ(arr->length(), idx * sizeof(int));

[ahaas, 2016/06/16 12:55:09]

ByteArray uses kIntSize internally instead of sizeof(int), where kIntSize is
defined as sizeof(int), I think you should also use kIntSize.

[clemensh, 2016/06/16 15:59:50]

Thanks for the hint. I also found kMaxInt in globals.h, and use that instead of
std::numeric_limit<int>::max().

+  debug_info->set(kWasmDebugInfoFunctionByteOffsets, arr);
+
+  return arr;
+}
+
+std::pair<int, int> GetFunctionOffsetAndLength(WasmDebugInfo *debug_info,
+                                               int func_index) {
+  ByteArray *arr = GetOrCreateFunctionOffsetTable(debug_info);
+
+  int funcs_in_arr = arr->length() / sizeof(int) / 2;

[ahaas, 2016/06/16 12:55:09]

same here.

[clemensh, 2016/06/16 15:59:50]

Done.

+  if (func_index < 0 || func_index >= funcs_in_arr) return {0, 0};

[ahaas, 2016/06/16 12:55:09]

Are func_index < 0 and func_index >= funcs_in_array valid inputs? If not,
wouldn't it be better to replace this {if} by a DCHECK? Anyways, this check
should be the first line of the function.

[clemensh, 2016/06/16 15:59:50]

Yeah, that's always the question. func_index somehow comes from "external", but
yes, it should be valid.
Changed it to DCHECK, but can't move it further up because we first have to
compute the number of functions.

+
+  int offset = arr->get_int(2 * func_index);
+  int length = arr->get_int(2 * func_index + 1);
+  // Assert that it's distinguishable from the "illegal function index" return.
+  DCHECK(offset > 0 && length > 0);
+  return {offset, length};
+}
+
+Vector<const uint8_t> GetFunctionBytes(WasmDebugInfo *debug_info,

[ahaas, 2016/06/16 12:55:09]

Why don't you make this a private function of WasmDebugInfo?

[clemensh, 2016/06/16 15:59:50]

Because I would have to add it to the header, and I would like to avoid exposing
such internals in the header, if possible.

[ahaas, 2016/06/16 19:42:19]

Isn't that what private methods are for?

+                                       int func_index) {
+  DCHECK(!AllowHeapAllocation::IsAllowed());
+  SeqOneByteString *module_bytes =
+      wasm::GetWasmBytes(debug_info->wasm_object());
+  std::pair<int, int> offset_and_length =
+      GetFunctionOffsetAndLength(debug_info, func_index);
+  return Vector<const uint8_t>(
+      module_bytes->GetChars() + offset_and_length.first,
+      offset_and_length.second);
+}
+
+}  // namespace
+
+Handle<WasmDebugInfo> WasmDebugInfo::New(Handle<JSObject> wasm) {
+  Isolate *isolate = wasm->GetIsolate();
+  Factory *factory = isolate->factory();
+  Handle<FixedArray> arr =
+      factory->NewFixedArray(kWasmDebugInfoNumEntries, TENURED);
+  arr->set(kWasmDebugInfoWasmObj, *wasm);
+  int hash = 0;
+  Handle<SeqOneByteString> wasm_bytes(GetWasmBytes(*wasm), isolate);
+  {
+    DisallowHeapAllocation no_gc;
+    hash = StringHasher::HashSequentialString(
+        wasm_bytes->GetChars(), wasm_bytes->length(), kZeroHashSeed);
+  }
+  Handle<Object> hash_obj = factory->NewNumberFromInt(hash, TENURED);

[ahaas, 2016/06/16 12:55:09]

Is it guaranteed that {hash} is a SMI? If not, then NewNumberFromInt will
allocate a HeapNumber and therefore crash because of the DisallowHeapAllocation
scope. I think it would be nicer anyways to use handle(Smi::FromInt(...))
instead of NewNumberFromInt because of the DisallowHeapAllocation scope.

[clemensh, 2016/06/16 15:59:50]

If a Smi has less than 32 bit, then it could not fit. The heap allocation is out
of the scope of DisallowHeapAllocation though, so it should not be a problem.

[ahaas, 2016/06/16 19:42:18]

True, I missed the ending scope.

+  arr->set(kWasmDebugInfoWasmBytesHash, *hash_obj);
+
+  return Handle<WasmDebugInfo>::cast(arr);
+}
+
+bool WasmDebugInfo::IsDebugInfo(Object *object) {
+  if (!object->IsFixedArray()) return false;
+  FixedArray *arr = FixedArray::cast(object);
+  Isolate *isolate = arr->GetIsolate();
+  return arr->length() == kWasmDebugInfoNumEntries &&
+         IsWasmObject(arr->get(kWasmDebugInfoWasmObj)) &&
+         arr->get(kWasmDebugInfoWasmBytesHash)->IsNumber() &&
+         (arr->get(kWasmDebugInfoFunctionByteOffsets)->IsUndefined(isolate) ||
+          arr->get(kWasmDebugInfoFunctionByteOffsets)->IsByteArray());
+}
+
+WasmDebugInfo *WasmDebugInfo::cast(Object *object) {
+  DCHECK(IsDebugInfo(object));
+  return static_cast<WasmDebugInfo *>(object);

[ahaas, 2016/06/16 12:55:09]

For consistency, could you use a reinterpret_cast here, as all object types in
objects-inl.h do it?

[clemensh, 2016/06/16 15:59:50]

Done.

+}
+
+JSObject *WasmDebugInfo::wasm_object() {
+  return JSObject::cast(get(kWasmDebugInfoWasmObj));
+}
+
+bool WasmDebugInfo::SetBreakPoint(int byte_offset) { return false; }

[ahaas, 2016/06/16 12:55:09]

Add a to-be-implemented here?

[clemensh, 2016/06/16 15:59:50]

Done.

+
+Handle<String> WasmDebugInfo::DisassembleFunction(int func_index) {
+  std::ostringstream disassembly_os;
+
+  {
+    DisallowHeapAllocation no_gc;
+    Vector<const uint8_t> bytes_vec = GetFunctionBytes(this, func_index);
+
+    base::AccountingAllocator allocator;
+    bool ok = PrintAst(
+        &allocator, FunctionBodyForTesting(bytes_vec.start(), bytes_vec.end()),
+        disassembly_os, nullptr);
+    DCHECK(ok);
+    USE(ok);
+  }
+
+  // Unfortunately, we have to copy the string here.
+  std::string code_str = disassembly_os.str();
+  CHECK_LE(code_str.length(),
+           static_cast<size_t>(std::numeric_limits<int>::max()));
+  return GetIsolate()
+      ->factory()
+      ->NewStringFromAscii(Vector<const char>(
+          code_str.data(), static_cast<int>(code_str.length())))
+      .ToHandleChecked();
+}
+
+Handle<FixedArray> WasmDebugInfo::GetFunctionOffsetTable(int func_index) {
+  class NullBuf : public std::streambuf {};
+  NullBuf null_buf;
+  std::ostream null_stream(&null_buf);
+
+  std::vector<std::tuple<uint32_t, int, int>> offset_table_vec;
+
+  {
+    DisallowHeapAllocation no_gc;
+    Vector<const uint8_t> bytes_vec = GetFunctionBytes(this, func_index);
+
+    v8::base::AccountingAllocator allocator;
+    bool ok = PrintAst(

[ahaas, 2016/06/16 12:55:09]

Is PrintAst supposed to be used like this?

[clemensh, 2016/06/16 15:59:50]

What do you mean by "like this"?
It is the function to compute a text representation of wasm code, and it
computes the offset table. Here, we only need the latter, so I pass a null
stream for the text representation.

[ahaas, 2016/06/16 19:42:19]

I was just surprised that a function called PrintAst provides computations.

+        &allocator, FunctionBodyForTesting(bytes_vec.start(), bytes_vec.end()),
+        null_stream, &offset_table_vec);
+    DCHECK(ok);
+    USE(ok);
+  }
+
+  size_t arr_size = 3 * offset_table_vec.size();
+  CHECK_LE(arr_size, static_cast<size_t>(std::numeric_limits<int>::max()));
+  Handle<FixedArray> offset_table = GetIsolate()->factory()->NewFixedArray(
+      static_cast<int>(arr_size), TENURED);
+
+  int idx = 0;
+  for (std::tuple<uint32_t, int, int> elem : offset_table_vec) {
+    offset_table->set(idx++, Smi::FromInt(std::get<0>(elem)));
+    offset_table->set(idx++, Smi::FromInt(std::get<1>(elem)));
+    offset_table->set(idx++, Smi::FromInt(std::get<2>(elem)));
+  }
+  DCHECK_EQ(idx, offset_table->length());
+
+  return offset_table;
+}