[wasm] Split off debug info from wasm object

src/wasm/wasm-module.cc

Index: src/wasm/wasm-module.cc
diff --git a/src/wasm/wasm-module.cc b/src/wasm/wasm-module.cc
index 514184b2f9d50ce87ac5f8747b8e15b2ade5ff3f..285c9cb5cb6e9ec78588b4b031e9c10b8c89fa2e 100644
--- a/src/wasm/wasm-module.cc
+++ b/src/wasm/wasm-module.cc
@@ -12,6 +12,7 @@
 
 #include "src/wasm/ast-decoder.h"
 #include "src/wasm/module-decoder.h"
+#include "src/wasm/wasm-debug.h"
 #include "src/wasm/wasm-function-name-table.h"
 #include "src/wasm/wasm-module.h"
 #include "src/wasm/wasm-result.h"
@@ -205,12 +206,15 @@ class WasmLinker {
 
 namespace {
 // Internal constants for the layout of the module object.
-const int kWasmModuleInternalFieldCount = 5;
 const int kWasmModuleFunctionTable = 0;
 const int kWasmModuleCodeTable = 1;
 const int kWasmMemArrayBuffer = 2;
 const int kWasmGlobalsArrayBuffer = 3;
+// TODO(clemensh): Remove function name array, extract names from module bytes.

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

Will you have time to do this? In not, I think you should replace it with
TODO(wasm).

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

If needed, I will do this at the very end for all the remaining TODOs.

 const int kWasmFunctionNamesArray = 4;
+const int kWasmModuleBytesString = 5;
+const int kWasmDebugInfo = 6;
+const int kWasmModuleInternalFieldCount = 7;
 
 void LoadDataSegments(const WasmModule* module, byte* mem_addr,
                       size_t mem_size) {
@@ -701,6 +705,17 @@ MaybeHandle<JSObject> WasmModule::Instantiate(
   Handle<FixedArray> code_table =
       factory->NewFixedArray(static_cast<int>(functions.size()), TENURED);
   instance.js_object->SetInternalField(kWasmModuleCodeTable, *code_table);
+  int module_bytes_len = static_cast<int>(instance.module->module_end -
+                                          instance.module->module_start);
+  DCHECK_EQ(instance.module->module_end - instance.module->module_start,

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

I don't think that this DCHECK is necessary. I would trust the compiler to do
normal arithmetic correctly.

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

This is to check for int-overflows. I refactored this to make it more readable.

+            module_bytes_len);
+  Vector<const uint8_t> module_bytes_vec(instance.module->module_start,
+                                         module_bytes_len);
+  Handle<String> module_bytes_string =
+      factory->NewStringFromOneByte(module_bytes_vec, TENURED)
+          .ToHandleChecked();
+  instance.js_object->SetInternalField(kWasmModuleBytesString,
+                                       *module_bytes_string);
 
   //-------------------------------------------------------------------------
   // Allocate and initialize the linear memory.
@@ -843,6 +858,12 @@ MaybeHandle<JSObject> WasmModule::Instantiate(
 
   code_stats.Report();
 
+  if (isolate->debug()->is_active()) {
+    // Trigger creation of the debug info, this will inform the debugger about
+    // the new module.
+    wasm::GetDebugInfo(*instance.js_object);

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

In GetDebugInfo you access the internal field kWasmDebugInfo before you
initialize it. Could you please add a comment which says why this is okay, if it
is okay?

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

Internal fields are auto-initialized to undefined.
But I just realized that the early instantiation of the debug info is not
required any more, so I just removed this code completely :)

+  }
+
   // Run the start function if one was specified.
   if (this->start_function_index >= 0) {
     HandleScope scope(isolate);
@@ -1010,12 +1031,36 @@ Handle<String> GetWasmFunctionName(Isolate* isolate, Handle<Object> wasm,
   return isolate->factory()->NewStringFromStaticChars("<WASM UNNAMED>");
 }
 
-bool IsWasmObject(Handle<JSObject> object) {
-  // TODO(clemensh): Check wasm byte header once we store a copy of the bytes.
-  return object->GetInternalFieldCount() == kWasmModuleInternalFieldCount &&
-         object->GetInternalField(kWasmModuleCodeTable)->IsFixedArray() &&
-         object->GetInternalField(kWasmMemArrayBuffer)->IsJSArrayBuffer() &&
-         object->GetInternalField(kWasmFunctionNamesArray)->IsByteArray();
+bool IsWasmObject(Object* object) {
+  if (!object->IsJSObject()) return false;
+  JSObject* obj = JSObject::cast(object);
+  if (obj->GetInternalFieldCount() != kWasmModuleInternalFieldCount ||
+      !obj->GetInternalField(kWasmModuleCodeTable)->IsFixedArray() ||
+      !obj->GetInternalField(kWasmMemArrayBuffer)->IsJSArrayBuffer() ||
+      !obj->GetInternalField(kWasmFunctionNamesArray)->IsByteArray() ||
+      !obj->GetInternalField(kWasmModuleBytesString)->IsSeqOneByteString()) {
+    return false;
+  }
+  DisallowHeapAllocation no_gc;
+  SeqOneByteString* bytes =
+      SeqOneByteString::cast(obj->GetInternalField(kWasmModuleBytesString));
+  if (bytes->length() < 4) return false;
+  if (memcmp(bytes->GetChars(), "\0asm", 4)) return false;
+
+  // All checks passed.
+  return true;
+}
+
+SeqOneByteString* GetWasmBytes(JSObject* wasm) {
+  return SeqOneByteString::cast(wasm->GetInternalField(kWasmModuleBytesString));
+}
+
+WasmDebugInfo* GetDebugInfo(JSObject* wasm) {
+  Object* info = wasm->GetInternalField(kWasmDebugInfo);
+  if (!info->IsUndefined(wasm->GetIsolate())) return WasmDebugInfo::cast(info);
+  Handle<WasmDebugInfo> new_info = WasmDebugInfo::New(handle(wasm));
+  wasm->SetInternalField(kWasmDebugInfo, *new_info);
+  return *new_info;
 }
 
 }  // namespace wasm