[wasm] Do a proper HasProperty() check in the memory and table setup.

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 292 matching lines @@
     return;
   }
   v8::ReturnValue<v8::Value> return_value = args.GetReturnValue();
   return_value.Set(Utils::ToLocal(instance.ToHandleChecked()));
 }
 
 bool GetIntegerProperty(v8::Isolate* isolate, ErrorThrower* thrower,
                         Local<Context> context, Local<v8::Object> object,
                         Local<String> property, int* result, int lower_bound,
                         int upper_bound) {
   v8::MaybeLocal<v8::Value> maybe = object->Get(context, property);

[Franzi, 2016/09/23 14:06:49]

You're still doing Object::Get(), which I believe can modify the object. Why
don't you use Object::Has()?  

[Franzi, 2016/09/23 17:23:28]

Never mind, I found the Has() :P

   v8::Local<v8::Value> value;
-  if (maybe.ToLocal(&value) && !value->IsUndefined()) {
+  if (maybe.ToLocal(&value)) {
     int64_t number;
     if (!value->IntegerValue(context).To(&number)) return false;
     if (number < static_cast<int64_t>(lower_bound)) {
       thrower->RangeError("Property value %" PRId64
                           " is below the lower bound %d",
                           number, lower_bound);
       return false;
     }
-    if (number > static_cast<int64_t>(std::numeric_limits<int>::max())) {
-      thrower->RangeError("Property value %" PRId64 " is out of integer range",
-                          number);
-      return false;
-    }
-    int num = static_cast<int>(number);
-    if (num > upper_bound) {
+    if (number > static_cast<int64_t>(upper_bound)) {
       thrower->RangeError("Property value %" PRId64
                           " is above the upper bound %d",
                           number, upper_bound);
       return false;
     }
-    *result = num;
+    *result = static_cast<int>(number);
     return true;
   }
   return false;
 }
 
 void WebAssemblyTable(const v8::FunctionCallbackInfo<v8::Value>& args) {
   v8::Isolate* isolate = args.GetIsolate();
   HandleScope scope(isolate);
   ErrorThrower thrower(reinterpret_cast<i::Isolate*>(isolate),
                        "WebAssembly.Module()");
@@ skipping 21 matching lines @@
   const int max_table_size = 1 << 26;
   // The descriptor's 'initial'.
   int initial;
   if (!GetIntegerProperty(isolate, &thrower, context, descriptor,
                           v8_str(isolate, "initial"), &initial, 0,
                           max_table_size)) {
     return;
   }
   // The descriptor's 'maximum'.
   int maximum;
-  bool has_maximum = true;
-  if (!GetIntegerProperty(isolate, &thrower, context, descriptor,
-                          v8_str(isolate, "maximum"), &maximum, initial,
-                          max_table_size)) {
-    if (reinterpret_cast<i::Isolate*>(isolate)->has_pending_exception() ||
-        thrower.error()) {
+  Local<String> maximum_key = v8_str(isolate, "maximum");
+  Maybe<bool> maybe_has_maximum = descriptor->Has(context, maximum_key);
+
+  bool has_maximum;

[Franzi, 2016/09/23 17:23:28]

I'd prefer to inline maybe_has_max and is_just. 
I think you can do

Maybe<bool> has_maximum = desc->Has();
if (has_maximum.IsNothing()) return;
if (has_maximum.FromJust()) {
  // get integer property
}

[ahaas, 2016/09/26 09:19:12]

Thanks, done.

+  bool is_just = maybe_has_maximum.To(&has_maximum);
+  if (!is_just) {
+    // There has been an exception, just return.
+    return;
+  }
+  if (has_maximum) {
+    if (!GetIntegerProperty(isolate, &thrower, context, descriptor, maximum_key,
+                            &maximum, initial, max_table_size)) {
       return;
-    } else {
-      // There was no error, the property just does not exist.
-      has_maximum = false;
     }
   }
 
   i::Isolate* i_isolate = reinterpret_cast<i::Isolate*>(isolate);
   i::Handle<i::JSFunction> table_cons(
       i_isolate->native_context()->wasm_table_constructor());
   i::Handle<i::JSObject> table_obj =
       i_isolate->factory()->NewJSObject(table_cons);
   i::Handle<i::FixedArray> fixed_array =
       i_isolate->factory()->NewFixedArray(initial);
@@ skipping 15 matching lines @@
   ErrorThrower thrower(reinterpret_cast<i::Isolate*>(isolate),
                        "WebAssembly.Module()");
   if (args.Length() < 1 || !args[0]->IsObject()) {
     thrower.TypeError("Argument 0 must be a table descriptor");
     return;
   }
   Local<Context> context = isolate->GetCurrentContext();
   Local<v8::Object> descriptor = args[0]->ToObject(context).ToLocalChecked();
   // The descriptor's 'initial'.
   int initial;
-  GetIntegerProperty(isolate, &thrower, context, descriptor,
-                     v8_str(isolate, "initial"), &initial, 0, 65536);
+  if (!GetIntegerProperty(isolate, &thrower, context, descriptor,
+                          v8_str(isolate, "initial"), &initial, 0, 65536)) {
+    return;
+  }
   // The descriptor's 'maximum'.
   int maximum;
-  bool has_maximum = true;
-  if (!GetIntegerProperty(isolate, &thrower, context, descriptor,
-                          v8_str(isolate, "maximum"), &maximum, initial,
-                          65536)) {
-    if (reinterpret_cast<i::Isolate*>(isolate)->has_pending_exception() ||
-        thrower.error()) {
+  Local<String> maximum_key = v8_str(isolate, "maximum");
+  Maybe<bool> maybe_has_maximum = descriptor->Has(context, maximum_key);
+
+  bool has_maximum;
+  bool is_just = maybe_has_maximum.To(&has_maximum);

[Franzi, 2016/09/23 17:23:28]

Same as above.

[ahaas, 2016/09/26 09:19:12]

Done.

+  if (!is_just) {
+    // There has been an exception, just return.
+    return;
+  }
+  if (has_maximum) {
+    if (!GetIntegerProperty(isolate, &thrower, context, descriptor, maximum_key,
+                            &maximum, initial, 65536)) {
       return;
-    } else {
-      // There was no error, the property just does not exist.
-      has_maximum = false;
     }
   }
   i::Isolate* i_isolate = reinterpret_cast<i::Isolate*>(isolate);
   i::Handle<i::JSFunction> memory_cons(
       i_isolate->native_context()->wasm_memory_constructor());
   i::Handle<i::JSObject> memory_obj =
       i_isolate->factory()->NewJSObject(memory_cons);
   i::Handle<i::JSArrayBuffer> buffer =
       i_isolate->factory()->NewJSArrayBuffer(i::SharedFlag::kNotShared);
   size_t size = static_cast<size_t>(i::wasm::WasmModule::kPageSize) *
@@ skipping 241 matching lines @@
     int unused_property_fields = in_object_properties - pre_allocated;
     Handle<Map> map = Map::CopyInitialMap(
         prev_map, instance_size, in_object_properties, unused_property_fields);
 
     context->set_wasm_function_map(*map);
   }
 }
 
 }  // namespace internal
 }  // namespace v8