From 3400ee9f4f21a455b7850ec42a4182a3c0eba310 Mon Sep 17 00:00:00 2001 From: clemensh Date: Mon, 13 Jun 2016 02:31:51 -0700 Subject: [PATCH] [wasm] Refactor function name table and lookup The function name table will now always be set; a CHECK will fail if the length would exceed the integer range. Also, the resolution of undefined function names to "" is moved over to the wasm side. R=titzer@chromium.org Review-Url: https://codereview.chromium.org/2057523002 Cr-Commit-Position: refs/heads/master@{#36918} --- src/isolate.cc | 13 ++------- src/messages.cc | 6 ++-- src/wasm/wasm-function-name-table.cc | 14 ++++----- src/wasm/wasm-function-name-table.h | 5 ++-- src/wasm/wasm-module.cc | 43 ++++++++++++++++++---------- src/wasm/wasm-module.h | 15 ++++++---- 6 files changed, 50 insertions(+), 46 deletions(-) diff --git a/src/isolate.cc b/src/isolate.cc index 035a626acd..872434606f 100644 --- a/src/isolate.cc +++ b/src/isolate.cc @@ -605,16 +605,9 @@ class CaptureStackTraceHelper { factory()->NewJSObject(isolate_->object_function()); if (!function_key_.is_null()) { - Object* wasm_object = frame->wasm_obj(); - Handle name; - if (!wasm_object->IsUndefined(isolate_)) { - Handle wasm = handle(JSObject::cast(wasm_object)); - wasm::GetWasmFunctionName(wasm, frame->function_index()) - .ToHandle(&name); - } - if (name.is_null()) { - name = isolate_->factory()->NewStringFromStaticChars(""); - } + Handle name = wasm::GetWasmFunctionName( + isolate_, handle(frame->wasm_obj(), isolate_), + frame->function_index()); JSObject::AddProperty(stack_frame, function_key_, name, NONE); } // Encode the function index as line number. diff --git a/src/messages.cc b/src/messages.cc index 86d0277f2a..ee364b9bc6 100644 --- a/src/messages.cc +++ b/src/messages.cc @@ -205,10 +205,8 @@ Handle CallSite::GetFileName() { Handle CallSite::GetFunctionName() { if (IsWasm()) { - MaybeHandle name = - wasm::GetWasmFunctionName(wasm_obj_, wasm_func_index_); - if (name.is_null()) return isolate_->factory()->null_value(); - return name.ToHandleChecked(); + return wasm::GetWasmFunctionNameOrNull(isolate_, wasm_obj_, + wasm_func_index_); } Handle result = JSFunction::GetName(fun_); if (result->length() != 0) return result; diff --git a/src/wasm/wasm-function-name-table.cc b/src/wasm/wasm-function-name-table.cc index 60ea977f89..cc52125500 100644 --- a/src/wasm/wasm-function-name-table.cc +++ b/src/wasm/wasm-function-name-table.cc @@ -18,23 +18,19 @@ namespace wasm { // integer entry is the negative offset of the next function name. // After these N+1 integer entries, the second part begins, which holds a // concatenation of all function names. -// -// Returns undefined if the array length would not fit in an integer value. -Handle BuildFunctionNamesTable(Isolate* isolate, - const WasmModule* module) { +Handle BuildFunctionNamesTable(Isolate* isolate, + const WasmModule* module) { uint64_t func_names_length = 0; for (auto& func : module->functions) func_names_length += func.name_length; int num_funcs_int = static_cast(module->functions.size()); int current_offset = (num_funcs_int + 1) * kIntSize; uint64_t total_array_length = current_offset + func_names_length; int total_array_length_int = static_cast(total_array_length); - // Check for overflow. Just skip function names if it happens. - if (total_array_length_int != total_array_length || num_funcs_int < 0 || - num_funcs_int != module->functions.size()) - return isolate->factory()->undefined_value(); + // Check for overflow. + CHECK(total_array_length_int == total_array_length && num_funcs_int >= 0 && + num_funcs_int == module->functions.size()); Handle func_names_array = isolate->factory()->NewByteArray(total_array_length_int, TENURED); - if (func_names_array.is_null()) return isolate->factory()->undefined_value(); func_names_array->set_int(0, num_funcs_int); int func_index = 0; for (const WasmFunction& fun : module->functions) { diff --git a/src/wasm/wasm-function-name-table.h b/src/wasm/wasm-function-name-table.h index 9b6a2c8875..ffee782413 100644 --- a/src/wasm/wasm-function-name-table.h +++ b/src/wasm/wasm-function-name-table.h @@ -16,9 +16,8 @@ namespace wasm { struct WasmModule; // Encode all function names of the WasmModule into one ByteArray. -// Returns undefined if the array length would not fit in an integer value. -Handle BuildFunctionNamesTable(Isolate* isolate, - const WasmModule* module); +Handle BuildFunctionNamesTable(Isolate* isolate, + const WasmModule* module); // Extract the function name for the given func_index from the function name // table. diff --git a/src/wasm/wasm-module.cc b/src/wasm/wasm-module.cc index 07d2ef1113..8ad95c8c1e 100644 --- a/src/wasm/wasm-module.cc +++ b/src/wasm/wasm-module.cc @@ -837,10 +837,9 @@ MaybeHandle WasmModule::Instantiate( // Attach an array with function names and an array with offsets into that // first array. //------------------------------------------------------------------------- - { - Handle arr = BuildFunctionNamesTable(isolate, module_env.module); - instance.js_object->SetInternalField(kWasmFunctionNamesArray, *arr); - } + instance.js_object->SetInternalField( + kWasmFunctionNamesArray, + *BuildFunctionNamesTable(isolate, module_env.module)); code_stats.Report(); @@ -983,14 +982,30 @@ int32_t CompileAndRunWasmModule(Isolate* isolate, const WasmModule* module) { return -1; } -MaybeHandle GetWasmFunctionName(Handle wasm, - uint32_t func_index) { - DCHECK(IsWasmObject(wasm)); - Object* func_names_arr_obj = wasm->GetInternalField(kWasmFunctionNamesArray); - Isolate* isolate = wasm->GetIsolate(); - if (func_names_arr_obj->IsUndefined(isolate)) return Handle::null(); - return GetWasmFunctionNameFromTable( - handle(ByteArray::cast(func_names_arr_obj), isolate), func_index); +Handle GetWasmFunctionNameOrNull(Isolate* isolate, Handle wasm, + uint32_t func_index) { + if (!wasm->IsUndefined()) { + Handle func_names_arr_obj( + ByteArray::cast(Handle::cast(wasm)->GetInternalField( + kWasmFunctionNamesArray)), + isolate); + // TODO(clemens): Extract this from the module bytes; skip whole function + // name table. + Handle name; + if (GetWasmFunctionNameFromTable(func_names_arr_obj, func_index) + .ToHandle(&name)) { + return name; + } + } + return isolate->factory()->null_value(); +} + +Handle GetWasmFunctionName(Isolate* isolate, Handle wasm, + uint32_t func_index) { + Handle name_or_null = + GetWasmFunctionNameOrNull(isolate, wasm, func_index); + if (!name_or_null->IsNull()) return Handle::cast(name_or_null); + return isolate->factory()->NewStringFromStaticChars(""); } bool IsWasmObject(Handle object) { @@ -998,9 +1013,7 @@ bool IsWasmObject(Handle object) { return object->GetInternalFieldCount() == kWasmModuleInternalFieldCount && object->GetInternalField(kWasmModuleCodeTable)->IsFixedArray() && object->GetInternalField(kWasmMemArrayBuffer)->IsJSArrayBuffer() && - (object->GetInternalField(kWasmFunctionNamesArray)->IsByteArray() || - object->GetInternalField(kWasmFunctionNamesArray) - ->IsUndefined(object->GetIsolate())); + object->GetInternalField(kWasmFunctionNamesArray)->IsByteArray(); } } // namespace wasm diff --git a/src/wasm/wasm-module.h b/src/wasm/wasm-module.h index 4a9faf0b00..1da17ba91d 100644 --- a/src/wasm/wasm-module.h +++ b/src/wasm/wasm-module.h @@ -5,11 +5,10 @@ #ifndef V8_WASM_MODULE_H_ #define V8_WASM_MODULE_H_ -#include "src/wasm/wasm-opcodes.h" -#include "src/wasm/wasm-result.h" - #include "src/api.h" #include "src/handles.h" +#include "src/wasm/wasm-opcodes.h" +#include "src/wasm/wasm-result.h" namespace v8 { namespace internal { @@ -318,11 +317,17 @@ int32_t CompileAndRunWasmModule(Isolate* isolate, const byte* module_start, // given decoded module. int32_t CompileAndRunWasmModule(Isolate* isolate, const WasmModule* module); +// Extract a function name from the given wasm object. +// Returns "" if the function is unnamed or the name is not a +// valid UTF-8 string. +Handle GetWasmFunctionName(Isolate* isolate, Handle wasm, + uint32_t func_index); + // Extract a function name from the given wasm object. // Returns a null handle if the function is unnamed or the name is not a valid // UTF-8 string. -MaybeHandle GetWasmFunctionName(Handle wasm, - uint32_t func_index); +Handle GetWasmFunctionNameOrNull(Isolate* isolate, Handle wasm, + uint32_t func_index); // Check whether the given object is a wasm object. // This checks the number and type of internal fields, so it's not 100 percent