[wasm] Split off debug info from wasm object

src/wasm/wasm-module.cc

 // Copyright 2015 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/base/atomic-utils.h"
 #include "src/macro-assembler.h"
 #include "src/objects.h"
 #include "src/property-descriptor.h"
 #include "src/v8.h"
 
 #include "src/simulator.h"
 
 #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"
 
 #include "src/compiler/wasm-compiler.h"
 
 namespace v8 {
 namespace internal {
 namespace wasm {
 
@@ skipping 173 matching lines @@
     }
     if (modified) {
       Assembler::FlushICache(isolate_, code->instruction_start(),
                              code->instruction_size());
     }
   }
 };
 
 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) {
   for (const WasmDataSegment& segment : module->data_segments) {
     if (!segment.init) continue;
     if (!segment.source_size) continue;
     CHECK_LT(segment.dest_addr, mem_size);
     CHECK_LE(segment.source_size, mem_size);
     CHECK_LE(segment.dest_addr + segment.source_size, mem_size);
     byte* addr = mem_addr + segment.dest_addr;
@@ skipping 470 matching lines @@
   //-------------------------------------------------------------------------
   Handle<Map> map = factory->NewMap(
       JS_OBJECT_TYPE,
       JSObject::kHeaderSize + kWasmModuleInternalFieldCount * kPointerSize);
   WasmModuleInstance instance(this);
   instance.context = isolate->native_context();
   instance.js_object = factory->NewJSObjectFromMap(map, TENURED);
   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.
   //-------------------------------------------------------------------------
   isolate->counters()->wasm_min_mem_pages_count()->AddSample(
       instance.module->min_mem_pages);
   isolate->counters()->wasm_max_mem_pages_count()->AddSample(
       instance.module->max_mem_pages);
   if (memory.is_null()) {
     if (!AllocateMemory(&thrower, isolate, &instance)) {
@@ skipping 122 matching lines @@
   //-------------------------------------------------------------------------
   // Attach the function name table.
   //-------------------------------------------------------------------------
   Handle<ByteArray> function_name_table =
       BuildFunctionNamesTable(isolate, module_env.module);
   instance.js_object->SetInternalField(kWasmFunctionNamesArray,
                                        *function_name_table);
 
   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);
     uint32_t index = static_cast<uint32_t>(this->start_function_index);
     Handle<String> name = isolate->factory()->NewStringFromStaticChars("start");
     Handle<Code> code = instance.function_code[index];
     Handle<JSFunction> jsfunc = compiler::CompileJSToWasmWrapper(
         isolate, &module_env, name, code, instance.js_object, index);
 
     // Call the JS function.
@@ skipping 147 matching lines @@
 Handle<String> GetWasmFunctionName(Isolate* isolate, Handle<Object> wasm,
                                    uint32_t func_index) {
   Handle<Object> name_or_null =
       GetWasmFunctionNameOrNull(isolate, wasm, func_index);
   if (!name_or_null->IsNull(isolate)) {
     return Handle<String>::cast(name_or_null);
   }
   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
 }  // namespace internal
 }  // namespace v8