Revert of [wasm] Refactor function name table and lookup (patchset #2 id:20001...

Revert of [wasm] Refactor function name table and lookup (patchset #2 id:20001 of https://codereview.chromium.org/2057523002/ )

Reason for revert:
[Sheriff] Speculative revert for flaky gc stress crashes, e.g.:
https://build.chromium.org/p/client.v8/builders/V8%20Mac%20GC%20Stress/builds/6592
https://build.chromium.org/p/client.v8/builders/V8%20Linux64%20GC%20Stress%20-%20custom%20snapshot/builds/6143

Original issue's description:
> [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 "<WASM UNNAMED>" is
> moved over to the wasm side.
>
> R=titzer@chromium.org
>
> Committed: https://crrev.com/3400ee9f4f21a455b7850ec42a4182a3c0eba310
> Cr-Commit-Position: refs/heads/master@{#36918}

TBR=titzer@chromium.org,clemensh@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true

Review-Url: https://codereview.chromium.org/2062793002
Cr-Commit-Position: refs/heads/master@{#36925}

src/isolate.cc

@@ -605,9 +605,16 @@ class CaptureStackTraceHelper {
         factory()->NewJSObject(isolate_->object_function());
 
     if (!function_key_.is_null()) {
-      Handle<String> name = wasm::GetWasmFunctionName(
-          isolate_, handle(frame->wasm_obj(), isolate_),
-          frame->function_index());
+      Object* wasm_object = frame->wasm_obj();
+      Handle<String> name;
+      if (!wasm_object->IsUndefined(isolate_)) {
+        Handle<JSObject> wasm = handle(JSObject::cast(wasm_object));
+        wasm::GetWasmFunctionName(wasm, frame->function_index())
+            .ToHandle(&name);
+      }
+      if (name.is_null()) {
+        name = isolate_->factory()->NewStringFromStaticChars("<WASM UNNAMED>");
+      }
       JSObject::AddProperty(stack_frame, function_key_, name, NONE);
     }
     // Encode the function index as line number.

src/messages.cc

@@ -205,8 +205,10 @@ Handle<Object> CallSite::GetFileName() {
 
 Handle<Object> CallSite::GetFunctionName() {
   if (IsWasm()) {
-    return wasm::GetWasmFunctionNameOrNull(isolate_, wasm_obj_,
-                                           wasm_func_index_);
+    MaybeHandle<String> name =
+        wasm::GetWasmFunctionName(wasm_obj_, wasm_func_index_);
+    if (name.is_null()) return isolate_->factory()->null_value();
+    return name.ToHandleChecked();
   }
   Handle<String> result = JSFunction::GetName(fun_);
   if (result->length() != 0) return result;

src/wasm/wasm-function-name-table.cc

@@ -18,7 +18,9 @@ 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.
-Handle<ByteArray> BuildFunctionNamesTable(Isolate* isolate,
+//
+// Returns undefined if the array length would not fit in an integer value.
+Handle<Object> BuildFunctionNamesTable(Isolate* isolate,
                                        const WasmModule* module) {
   uint64_t func_names_length = 0;
   for (auto& func : module->functions) func_names_length += func.name_length;
@@ -26,11 +28,13 @@ Handle<ByteArray> BuildFunctionNamesTable(Isolate* isolate,
   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<int>(total_array_length);
-  // Check for overflow.
-  CHECK(total_array_length_int == total_array_length && num_funcs_int >= 0 &&
-        num_funcs_int == module->functions.size());
+  // 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();
   Handle<ByteArray> 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) {

src/wasm/wasm-function-name-table.h

@@ -16,7 +16,8 @@ namespace wasm {
 struct WasmModule;
 
 // Encode all function names of the WasmModule into one ByteArray.
-Handle<ByteArray> BuildFunctionNamesTable(Isolate* isolate,
+// Returns undefined if the array length would not fit in an integer value.
+Handle<Object> BuildFunctionNamesTable(Isolate* isolate,
                                        const WasmModule* module);
 
 // Extract the function name for the given func_index from the function name

src/wasm/wasm-module.cc

@@ -837,9 +837,10 @@ MaybeHandle<JSObject> WasmModule::Instantiate(
   // Attach an array with function names and an array with offsets into that
   // first array.
   //-------------------------------------------------------------------------
-  instance.js_object->SetInternalField(
-      kWasmFunctionNamesArray,
-      *BuildFunctionNamesTable(isolate, module_env.module));
+  {
+    Handle<Object> arr = BuildFunctionNamesTable(isolate, module_env.module);
+    instance.js_object->SetInternalField(kWasmFunctionNamesArray, *arr);
+  }
 
   code_stats.Report();
 
@@ -982,30 +983,14 @@ int32_t CompileAndRunWasmModule(Isolate* isolate, const WasmModule* module) {
   return -1;
 }
 
-Handle<Object> GetWasmFunctionNameOrNull(Isolate* isolate, Handle<Object> wasm,
-                                         uint32_t func_index) {
-  if (!wasm->IsUndefined()) {
-    Handle<ByteArray> func_names_arr_obj(
-        ByteArray::cast(Handle<JSObject>::cast(wasm)->GetInternalField(
-            kWasmFunctionNamesArray)),
-        isolate);
-    // TODO(clemens): Extract this from the module bytes; skip whole function
-    // name table.
-    Handle<Object> name;
-    if (GetWasmFunctionNameFromTable(func_names_arr_obj, func_index)
-            .ToHandle(&name)) {
-      return name;
-    }
-  }
-  return isolate->factory()->null_value();
-}
-
-Handle<String> GetWasmFunctionName(Isolate* isolate, Handle<Object> wasm,
+MaybeHandle<String> GetWasmFunctionName(Handle<JSObject> wasm,
                                         uint32_t func_index) {
-  Handle<Object> name_or_null =
-      GetWasmFunctionNameOrNull(isolate, wasm, func_index);
-  if (!name_or_null->IsNull()) return Handle<String>::cast(name_or_null);
-  return isolate->factory()->NewStringFromStaticChars("<WASM UNNAMED>");
+  DCHECK(IsWasmObject(wasm));
+  Object* func_names_arr_obj = wasm->GetInternalField(kWasmFunctionNamesArray);
+  Isolate* isolate = wasm->GetIsolate();
+  if (func_names_arr_obj->IsUndefined(isolate)) return Handle<String>::null();
+  return GetWasmFunctionNameFromTable(
+      handle(ByteArray::cast(func_names_arr_obj), isolate), func_index);
 }
 
 bool IsWasmObject(Handle<JSObject> object) {
@@ -1013,7 +998,9 @@ bool IsWasmObject(Handle<JSObject> object) {
   return object->GetInternalFieldCount() == kWasmModuleInternalFieldCount &&
          object->GetInternalField(kWasmModuleCodeTable)->IsFixedArray() &&
          object->GetInternalField(kWasmMemArrayBuffer)->IsJSArrayBuffer() &&
-         object->GetInternalField(kWasmFunctionNamesArray)->IsByteArray();
+         (object->GetInternalField(kWasmFunctionNamesArray)->IsByteArray() ||
+          object->GetInternalField(kWasmFunctionNamesArray)
+              ->IsUndefined(object->GetIsolate()));
 }
 
 }  // namespace wasm

src/wasm/wasm-module.h

@@ -5,11 +5,12 @@
 #ifndef V8_WASM_MODULE_H_
 #define V8_WASM_MODULE_H_
 
-#include "src/api.h"
-#include "src/handles.h"
 #include "src/wasm/wasm-opcodes.h"
 #include "src/wasm/wasm-result.h"
 
+#include "src/api.h"
+#include "src/handles.h"
+
 namespace v8 {
 namespace internal {
 
@@ -317,16 +318,10 @@ 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 "<WASM UNNAMED>" if the function is unnamed or the name is not a
-// valid UTF-8 string.
-Handle<String> GetWasmFunctionName(Isolate* isolate, Handle<Object> 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.
-Handle<Object> GetWasmFunctionNameOrNull(Isolate* isolate, Handle<Object> wasm,
+MaybeHandle<String> GetWasmFunctionName(Handle<JSObject> wasm,
                                         uint32_t func_index);
 
 // Check whether the given object is a wasm object.