[wasm] Use ErrorThrower more uniformly in wasm-js.cc

src/wasm/wasm-js.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/api-natives.h"
 #include "src/api.h"
 #include "src/asmjs/asm-js.h"
 #include "src/asmjs/asm-typer.h"
 #include "src/asmjs/asm-wasm-builder.h"
 #include "src/assert-scope.h"
@@ skipping 90 matching lines @@
   if (buffer.start == nullptr) return false;
 
   DCHECK(source->IsArrayBuffer() || source->IsTypedArray());
   return i::wasm::ValidateModuleBytes(i_isolate, buffer.start, buffer.end,
                                       thrower,
                                       i::wasm::ModuleOrigin::kWasmOrigin);
 }
 
 // TODO(wasm): move brand check to the respective types, and don't throw
 // in it, rather, use a provided ErrorThrower, or let caller handle it.
-static bool BrandCheck(Isolate* isolate, i::Handle<i::Object> value,
-                       i::Handle<i::Symbol> sym) {
+static bool HasBrand(Isolate* isolate, i::Handle<i::Object> value,
+                     i::Handle<i::Symbol> sym) {
   if (!value->IsJSObject()) return false;
   i::Handle<i::JSObject> object = i::Handle<i::JSObject>::cast(value);
   Maybe<bool> has_brand = i::JSObject::HasOwnProperty(object, sym);
   if (has_brand.IsNothing()) return false;

[rossberg, 2017/01/25 09:47:34]

Nit: could similarly combine this into

return !has_brand.IsNothing() && has_brand.ToChecked();

[titzer, 2017/01/25 09:54:33]

Done.

   return has_brand.ToChecked();
 }
 
-static bool BrandCheck(Isolate* isolate, i::Handle<i::Object> value,
+static bool BrandCheck(ErrorThrower* thrower, i::Handle<i::Object> value,

[rossberg, 2017/01/25 09:47:34]

Why not avoid repetition and simplify this function to

bool result = HasBrand(...);
if (!result) thrower->TypeError(...);
return result;

[titzer, 2017/01/25 09:54:33]

Done.

                        i::Handle<i::Symbol> sym, const char* msg) {
   if (value->IsJSObject()) {
     i::Handle<i::JSObject> object = i::Handle<i::JSObject>::cast(value);
     Maybe<bool> has_brand = i::JSObject::HasOwnProperty(object, sym);
-    if (has_brand.IsNothing()) return false;
-    if (has_brand.ToChecked()) return true;
+    if (!has_brand.IsNothing() && has_brand.ToChecked()) return true;
   }
-  v8::Local<v8::Value> e = v8::Exception::TypeError(v8_str(isolate, msg));
-  isolate->ThrowException(e);
+  thrower->TypeError("%s", msg);
   return false;
 }
 
 void WebAssemblyCompile(const v8::FunctionCallbackInfo<v8::Value>& args) {
   v8::Isolate* isolate = args.GetIsolate();
   HandleScope scope(isolate);
   ErrorThrower thrower(reinterpret_cast<i::Isolate*>(isolate),
                        "WebAssembly.compile()");
 
   Local<Context> context = isolate->GetCurrentContext();
@@ skipping 86 matching lines @@
     if (!thrower->error())
       thrower->RuntimeError("Could not instantiate module");
     return nothing;
   }
   DCHECK(!i_isolate->has_pending_exception());
   return Utils::ToLocal(instance.ToHandleChecked());
 }
 
 namespace {
 i::MaybeHandle<i::WasmModuleObject> GetFirstArgumentAsModule(
-    const v8::FunctionCallbackInfo<v8::Value>& args, ErrorThrower& thrower) {
+    const v8::FunctionCallbackInfo<v8::Value>& args, ErrorThrower* thrower) {
   v8::Isolate* isolate = args.GetIsolate();
   i::MaybeHandle<i::WasmModuleObject> nothing;
   if (args.Length() < 1) {
-    thrower.TypeError("Argument 0 must be a WebAssembly.Module");
+    thrower->TypeError("Argument 0 must be a WebAssembly.Module");
     return nothing;
   }
 
   Local<Context> context = isolate->GetCurrentContext();
   i::Handle<i::Context> i_context = Utils::OpenHandle(*context);
-  if (!BrandCheck(isolate, Utils::OpenHandle(*args[0]),
+  if (!BrandCheck(thrower, Utils::OpenHandle(*args[0]),
                   i::Handle<i::Symbol>(i_context->wasm_module_sym()),
                   "Argument 0 must be a WebAssembly.Module")) {
     return nothing;
   }
 
   Local<Object> module_obj = Local<Object>::Cast(args[0]);
   return i::Handle<i::WasmModuleObject>::cast(
       v8::Utils::OpenHandle(*module_obj));
 }
 }  // namespace
 
 void WebAssemblyModuleImports(const v8::FunctionCallbackInfo<v8::Value>& args) {
   HandleScope scope(args.GetIsolate());
   v8::Isolate* isolate = args.GetIsolate();
   i::Isolate* i_isolate = reinterpret_cast<i::Isolate*>(isolate);
   ErrorThrower thrower(i_isolate, "WebAssembly.Module.imports()");
 
-  auto maybe_module = GetFirstArgumentAsModule(args, thrower);
+  auto maybe_module = GetFirstArgumentAsModule(args, &thrower);
 
   if (!maybe_module.is_null()) {
     auto imports =
         i::wasm::GetImports(i_isolate, maybe_module.ToHandleChecked());
     args.GetReturnValue().Set(Utils::ToLocal(imports));
   }
 }
 
 void WebAssemblyModuleExports(const v8::FunctionCallbackInfo<v8::Value>& args) {
   HandleScope scope(args.GetIsolate());
   v8::Isolate* isolate = args.GetIsolate();
   i::Isolate* i_isolate = reinterpret_cast<i::Isolate*>(isolate);
-
   ErrorThrower thrower(i_isolate, "WebAssembly.Module.exports()");
 
-  auto maybe_module = GetFirstArgumentAsModule(args, thrower);
+  auto maybe_module = GetFirstArgumentAsModule(args, &thrower);
 
   if (!maybe_module.is_null()) {
     auto exports =
         i::wasm::GetExports(i_isolate, maybe_module.ToHandleChecked());
     args.GetReturnValue().Set(Utils::ToLocal(exports));
   }
 }
 
 void WebAssemblyModuleCustomSections(
     const v8::FunctionCallbackInfo<v8::Value>& args) {
   HandleScope scope(args.GetIsolate());
   v8::Isolate* isolate = args.GetIsolate();
   i::Isolate* i_isolate = reinterpret_cast<i::Isolate*>(isolate);
-
   ErrorThrower thrower(i_isolate, "WebAssembly.Module.customSections()");
 
-  auto maybe_module = GetFirstArgumentAsModule(args, thrower);
+  auto maybe_module = GetFirstArgumentAsModule(args, &thrower);
 
   if (args.Length() < 2) {
     thrower.TypeError("Argument 1 must be a string");
     return;
   }
 
   i::Handle<i::Object> name = Utils::OpenHandle(*args[1]);
   if (!name->IsString()) {
     thrower.TypeError("Argument 1 must be a string");
     return;
   }
 
   if (!maybe_module.is_null()) {
     auto custom_sections =
         i::wasm::GetCustomSections(i_isolate, maybe_module.ToHandleChecked(),
                                    i::Handle<i::String>::cast(name), &thrower);
     if (!thrower.error()) {
       args.GetReturnValue().Set(Utils::ToLocal(custom_sections));
     }
   }
 }
 
 void WebAssemblyInstance(const v8::FunctionCallbackInfo<v8::Value>& args) {
   HandleScope scope(args.GetIsolate());
   v8::Isolate* isolate = args.GetIsolate();
   i::Isolate* i_isolate = reinterpret_cast<i::Isolate*>(isolate);
-
   ErrorThrower thrower(i_isolate, "WebAssembly.Instance()");
 
-  auto maybe_module = GetFirstArgumentAsModule(args, thrower);
+  auto maybe_module = GetFirstArgumentAsModule(args, &thrower);
 
   if (!maybe_module.is_null()) {
     MaybeLocal<Value> instance = InstantiateModuleImpl(
         i_isolate, maybe_module.ToHandleChecked(), args, &thrower);
 
     if (instance.IsEmpty()) {
       DCHECK(thrower.error());
       return;
     }
     args.GetReturnValue().Set(instance.ToLocalChecked());
   }
 }
 
 void WebAssemblyInstantiate(const v8::FunctionCallbackInfo<v8::Value>& args) {
   v8::Isolate* isolate = args.GetIsolate();
   i::Isolate* i_isolate = reinterpret_cast<i::Isolate*>(isolate);
+  ErrorThrower thrower(i_isolate, "WebAssembly.instantiate()");
 
   HandleScope scope(isolate);
-  ErrorThrower thrower(i_isolate, "WebAssembly.instantiate()");
 
   Local<Context> context = isolate->GetCurrentContext();
   i::Handle<i::Context> i_context = Utils::OpenHandle(*context);
 
   v8::Local<v8::Promise::Resolver> resolver;
   if (!v8::Promise::Resolver::New(context).ToLocal(&resolver)) return;
   v8::ReturnValue<v8::Value> return_value = args.GetReturnValue();
   return_value.Set(resolver->GetPromise());
 
   if (args.Length() < 1) {
     thrower.TypeError(
         "Argument 0 must be provided and must be either a buffer source or a "
         "WebAssembly.Module object");
     resolver->Reject(context, Utils::ToLocal(thrower.Reify()));
     return;
   }
 
   i::Handle<i::Object> first_arg = Utils::OpenHandle(*args[0]);
   if (!first_arg->IsJSObject()) {
     thrower.TypeError(
         "Argument 0 must be a buffer source or a WebAssembly.Module object");
     resolver->Reject(context, Utils::ToLocal(thrower.Reify()));
     return;
   }
-  bool want_pair = !BrandCheck(
+  bool want_pair = !HasBrand(
       isolate, first_arg, i::Handle<i::Symbol>(i_context->wasm_module_sym()));
   i::Handle<i::WasmModuleObject> module_obj;
   if (want_pair) {
     i::MaybeHandle<i::WasmModuleObject> maybe_module_obj =
         CreateModuleObject(isolate, args[0], &thrower);
     if (!maybe_module_obj.ToHandle(&module_obj)) {
       DCHECK(thrower.error());
       resolver->Reject(context, Utils::ToLocal(thrower.Reify()));
       return;
     }
@@ skipping 154 matching lines @@
     return;
   }
   i::Handle<i::JSObject> memory_obj =
       i::WasmMemoryObject::New(i_isolate, buffer, maximum);
   args.GetReturnValue().Set(Utils::ToLocal(memory_obj));
 }
 
 void WebAssemblyTableGetLength(
     const v8::FunctionCallbackInfo<v8::Value>& args) {
   v8::Isolate* isolate = args.GetIsolate();
+  ErrorThrower thrower(reinterpret_cast<i::Isolate*>(isolate),
+                       "WebAssembly.Table.length()");
   Local<Context> context = isolate->GetCurrentContext();
   i::Handle<i::Context> i_context = Utils::OpenHandle(*context);
-  if (!BrandCheck(isolate, Utils::OpenHandle(*args.This()),
+  if (!BrandCheck(&thrower, Utils::OpenHandle(*args.This()),
                   i::Handle<i::Symbol>(i_context->wasm_table_sym()),
                   "Receiver is not a WebAssembly.Table")) {
     return;
   }
   auto receiver =
       i::Handle<i::WasmTableObject>::cast(Utils::OpenHandle(*args.This()));
   args.GetReturnValue().Set(
       v8::Number::New(isolate, receiver->current_length()));
 }
 
 void WebAssemblyTableGrow(const v8::FunctionCallbackInfo<v8::Value>& args) {
   v8::Isolate* isolate = args.GetIsolate();
+  ErrorThrower thrower(reinterpret_cast<i::Isolate*>(isolate),
+                       "WebAssembly.Table.grow()");
   Local<Context> context = isolate->GetCurrentContext();
   i::Handle<i::Context> i_context = Utils::OpenHandle(*context);
-  if (!BrandCheck(isolate, Utils::OpenHandle(*args.This()),
+  if (!BrandCheck(&thrower, Utils::OpenHandle(*args.This()),
                   i::Handle<i::Symbol>(i_context->wasm_table_sym()),
                   "Receiver is not a WebAssembly.Table")) {
     return;
   }
 
   i::Isolate* i_isolate = reinterpret_cast<i::Isolate*>(isolate);
   auto receiver =
       i::Handle<i::WasmTableObject>::cast(Utils::OpenHandle(*args.This()));
   i::Handle<i::FixedArray> old_array(receiver->functions(), i_isolate);
   int old_size = old_array->length();
   int64_t new_size64 = 0;
   if (args.Length() > 0 && !args[0]->IntegerValue(context).To(&new_size64)) {
     return;
   }
   new_size64 += old_size;
 
   int64_t max_size64 = receiver->maximum_length();
   if (max_size64 < 0 ||
       max_size64 > static_cast<int64_t>(i::FLAG_wasm_max_table_size)) {
     max_size64 = i::FLAG_wasm_max_table_size;
   }
 
   if (new_size64 < old_size || new_size64 > max_size64) {
-    v8::Local<v8::Value> e = v8::Exception::RangeError(
-        v8_str(isolate, new_size64 < old_size ? "trying to shrink table"
-                                              : "maximum table size exceeded"));
-    isolate->ThrowException(e);
+    thrower.RangeError(new_size64 < old_size ? "trying to shrink table"
+                                             : "maximum table size exceeded");
     return;
   }
 
   int new_size = static_cast<int>(new_size64);
   i::WasmTableObject::Grow(i_isolate, receiver,
                            static_cast<uint32_t>(new_size - old_size));
 
   if (new_size != old_size) {
     i::Handle<i::FixedArray> new_array =
         i_isolate->factory()->NewFixedArray(new_size);
     for (int i = 0; i < old_size; ++i) new_array->set(i, old_array->get(i));
     i::Object* null = i_isolate->heap()->null_value();
     for (int i = old_size; i < new_size; ++i) new_array->set(i, null);
     receiver->set_functions(*new_array);
   }
 
   // TODO(gdeepti): use weak links for instances
   v8::ReturnValue<v8::Value> return_value = args.GetReturnValue();
   return_value.Set(old_size);
 }
 
 void WebAssemblyTableGet(const v8::FunctionCallbackInfo<v8::Value>& args) {
   v8::Isolate* isolate = args.GetIsolate();
+  i::Isolate* i_isolate = reinterpret_cast<i::Isolate*>(isolate);
+  ErrorThrower thrower(i_isolate, "WebAssembly.Table.get()");
   Local<Context> context = isolate->GetCurrentContext();
   i::Handle<i::Context> i_context = Utils::OpenHandle(*context);
-  if (!BrandCheck(isolate, Utils::OpenHandle(*args.This()),
+  if (!BrandCheck(&thrower, Utils::OpenHandle(*args.This()),
                   i::Handle<i::Symbol>(i_context->wasm_table_sym()),
                   "Receiver is not a WebAssembly.Table")) {
     return;
   }
 
-  i::Isolate* i_isolate = reinterpret_cast<i::Isolate*>(isolate);
   auto receiver =
       i::Handle<i::WasmTableObject>::cast(Utils::OpenHandle(*args.This()));
   i::Handle<i::FixedArray> array(receiver->functions(), i_isolate);
   int i = 0;
   if (args.Length() > 0 && !args[0]->Int32Value(context).To(&i)) return;
   v8::ReturnValue<v8::Value> return_value = args.GetReturnValue();
   if (i < 0 || i >= array->length()) {
-    v8::Local<v8::Value> e =
-        v8::Exception::RangeError(v8_str(isolate, "index out of bounds"));
-    isolate->ThrowException(e);
+    thrower.RangeError("index out of bounds");
     return;
   }
 
   i::Handle<i::Object> value(array->get(i), i_isolate);
   return_value.Set(Utils::ToLocal(value));
 }
 
 void WebAssemblyTableSet(const v8::FunctionCallbackInfo<v8::Value>& args) {
   v8::Isolate* isolate = args.GetIsolate();
   i::Isolate* i_isolate = reinterpret_cast<i::Isolate*>(isolate);
+  ErrorThrower thrower(i_isolate, "WebAssembly.Table.set()");
   Local<Context> context = isolate->GetCurrentContext();
   i::Handle<i::Context> i_context = Utils::OpenHandle(*context);
-  if (!BrandCheck(isolate, Utils::OpenHandle(*args.This()),
+  if (!BrandCheck(&thrower, Utils::OpenHandle(*args.This()),
                   i::Handle<i::Symbol>(i_context->wasm_table_sym()),
                   "Receiver is not a WebAssembly.Table")) {
     return;
   }
   if (args.Length() < 2) {
-    v8::Local<v8::Value> e = v8::Exception::TypeError(
-        v8_str(isolate, "Argument 1 must be null or a function"));
-    isolate->ThrowException(e);
+    thrower.TypeError("Argument 1 must be null or a function");
     return;
   }
   i::Handle<i::Object> value = Utils::OpenHandle(*args[1]);
   if (!value->IsNull(i_isolate) &&
       (!value->IsJSFunction() ||
        i::Handle<i::JSFunction>::cast(value)->code()->kind() !=
            i::Code::JS_TO_WASM_FUNCTION)) {
-    v8::Local<v8::Value> e = v8::Exception::TypeError(
-        v8_str(isolate, "Argument 1 must be null or a WebAssembly function"));
-    isolate->ThrowException(e);
+    thrower.TypeError("Argument 1 must be null or a WebAssembly function");
     return;
   }
 
   auto receiver =
       i::Handle<i::WasmTableObject>::cast(Utils::OpenHandle(*args.This()));
   i::Handle<i::FixedArray> array(receiver->functions(), i_isolate);
   int i;
   if (!args[0]->Int32Value(context).To(&i)) return;
   if (i < 0 || i >= array->length()) {
-    v8::Local<v8::Value> e =
-        v8::Exception::RangeError(v8_str(isolate, "index out of bounds"));
-    isolate->ThrowException(e);
+    thrower.RangeError("index out of bounds");
     return;
   }
 
   i::Handle<i::FixedArray> dispatch_tables(receiver->dispatch_tables(),
                                            i_isolate);
   if (value->IsNull(i_isolate)) {
     i::wasm::UpdateDispatchTables(i_isolate, dispatch_tables, i,
                                   i::Handle<i::JSFunction>::null());
   } else {
     i::wasm::UpdateDispatchTables(i_isolate, dispatch_tables, i,
                                   i::Handle<i::JSFunction>::cast(value));
   }
 
   i::Handle<i::FixedArray>::cast(array)->set(i, *value);
 }
 
 void WebAssemblyMemoryGrow(const v8::FunctionCallbackInfo<v8::Value>& args) {
   v8::Isolate* isolate = args.GetIsolate();
+  i::Isolate* i_isolate = reinterpret_cast<i::Isolate*>(isolate);
+  ErrorThrower thrower(i_isolate, "WebAssembly.Memory.grow()");
   Local<Context> context = isolate->GetCurrentContext();
   i::Handle<i::Context> i_context = Utils::OpenHandle(*context);
-  if (!BrandCheck(isolate, Utils::OpenHandle(*args.This()),
+  if (!BrandCheck(&thrower, Utils::OpenHandle(*args.This()),
                   i::Handle<i::Symbol>(i_context->wasm_memory_sym()),
                   "Receiver is not a WebAssembly.Memory")) {
     return;
   }
   int64_t delta_size = 0;
   if (args.Length() < 1 || !args[0]->IntegerValue(context).To(&delta_size)) {
-    v8::Local<v8::Value> e = v8::Exception::TypeError(
-        v8_str(isolate, "Argument 0 required, must be numeric value of pages"));
-    isolate->ThrowException(e);
+    thrower.TypeError("Argument 0 required, must be numeric value of pages");
     return;
   }
   i::Handle<i::WasmMemoryObject> receiver =
       i::Handle<i::WasmMemoryObject>::cast(Utils::OpenHandle(*args.This()));
   int64_t max_size64 = receiver->maximum_pages();
   if (max_size64 < 0 ||
       max_size64 > static_cast<int64_t>(i::FLAG_wasm_max_mem_pages)) {
     max_size64 = i::FLAG_wasm_max_mem_pages;
   }
   i::Handle<i::JSArrayBuffer> old_buffer(receiver->buffer());
   uint32_t old_size =
       old_buffer->byte_length()->Number() / i::wasm::kSpecMaxWasmMemoryPages;
   int64_t new_size64 = old_size + delta_size;
   if (delta_size < 0 || max_size64 < new_size64 || new_size64 < old_size) {
-    v8::Local<v8::Value> e = v8::Exception::RangeError(v8_str(
-        isolate, new_size64 < old_size ? "trying to shrink memory"
-                                       : "maximum memory size exceeded"));
-    isolate->ThrowException(e);
+    thrower.RangeError(new_size64 < old_size ? "trying to shrink memory"
+                                             : "maximum memory size exceeded");
     return;
   }
-  i::Isolate* i_isolate = reinterpret_cast<i::Isolate*>(isolate);
   int32_t ret = i::wasm::GrowWebAssemblyMemory(
       i_isolate, receiver, static_cast<uint32_t>(delta_size));
   if (ret == -1) {
-    v8::Local<v8::Value> e = v8::Exception::RangeError(
-        v8_str(isolate, "Unable to grow instance memory."));
-    isolate->ThrowException(e);
+    thrower.RangeError("Unable to grow instance memory.");
     return;
   }
   v8::ReturnValue<v8::Value> return_value = args.GetReturnValue();
   return_value.Set(ret);
 }
 
 void WebAssemblyMemoryGetBuffer(
     const v8::FunctionCallbackInfo<v8::Value>& args) {
   v8::Isolate* isolate = args.GetIsolate();
+  i::Isolate* i_isolate = reinterpret_cast<i::Isolate*>(isolate);
+  ErrorThrower thrower(i_isolate, "WebAssembly.Memory.buffer");
   Local<Context> context = isolate->GetCurrentContext();
   i::Handle<i::Context> i_context = Utils::OpenHandle(*context);
-  if (!BrandCheck(isolate, Utils::OpenHandle(*args.This()),
+  if (!BrandCheck(&thrower, Utils::OpenHandle(*args.This()),
                   i::Handle<i::Symbol>(i_context->wasm_memory_sym()),
                   "Receiver is not a WebAssembly.Memory")) {
     return;
   }
-  i::Isolate* i_isolate = reinterpret_cast<i::Isolate*>(isolate);
   i::Handle<i::WasmMemoryObject> receiver =
       i::Handle<i::WasmMemoryObject>::cast(Utils::OpenHandle(*args.This()));
   i::Handle<i::Object> buffer(receiver->buffer(), i_isolate);
   DCHECK(buffer->IsJSArrayBuffer());
   v8::ReturnValue<v8::Value> return_value = args.GetReturnValue();
   return_value.Set(Utils::ToLocal(buffer));
 }
 }  // namespace
 
 // TODO(titzer): we use the API to create the function template because the
@@ skipping 184 matching lines @@
   Handle<JSFunction> link_error(
       isolate->native_context()->wasm_link_error_function());
   JSObject::AddProperty(webassembly, isolate->factory()->LinkError_string(),
                         link_error, attributes);
   Handle<JSFunction> runtime_error(
       isolate->native_context()->wasm_runtime_error_function());
   JSObject::AddProperty(webassembly, isolate->factory()->RuntimeError_string(),
                         runtime_error, attributes);
 }
 
 static bool HasBrand(i::Handle<i::Object> value, i::Handle<i::Symbol> symbol) {

[rossberg, 2017/01/25 09:47:34]

Why is there another version of HasBrand here?

[titzer, 2017/01/25 09:54:33]

Removed.

   if (value->IsJSObject()) {
     i::Handle<i::JSObject> object = i::Handle<i::JSObject>::cast(value);
     Maybe<bool> has_brand = i::JSObject::HasOwnProperty(object, symbol);
     if (has_brand.IsNothing()) return false;
     if (has_brand.ToChecked()) return true;
   }
   return false;
 }
 
 bool WasmJs::IsWasmMemoryObject(Isolate* isolate, Handle<Object> value) {
   i::Handle<i::Symbol> symbol(isolate->context()->wasm_memory_sym(), isolate);
   return HasBrand(value, symbol);
 }
 
 bool WasmJs::IsWasmTableObject(Isolate* isolate, Handle<Object> value) {
   i::Handle<i::Symbol> symbol(isolate->context()->wasm_table_sym(), isolate);
   return HasBrand(value, symbol);
 }
 }  // namespace internal
 }  // namespace v8