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

src/wasm/wasm-js.cc

Index: src/wasm/wasm-js.cc
diff --git a/src/wasm/wasm-js.cc b/src/wasm/wasm-js.cc
index 0b6b25b127f1429be3b0f019cc5e90aa53d16250..26651b8b7f14cc472e3a5a9a3c3eba62012769fc 100644
--- a/src/wasm/wasm-js.cc
+++ b/src/wasm/wasm-js.cc
@@ -312,7 +312,7 @@ bool GetIntegerProperty(v8::Isolate* isolate, ErrorThrower* thrower,
                         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)) {
@@ -321,19 +321,13 @@ bool GetIntegerProperty(v8::Isolate* isolate, ErrorThrower* thrower,
                           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;
@@ -375,16 +369,19 @@ void WebAssemblyTable(const v8::FunctionCallbackInfo<v8::Value>& args) {
   }
   // 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;
     }
   }
 
@@ -420,20 +417,25 @@ void WebAssemblyMemory(const v8::FunctionCallbackInfo<v8::Value>& args) {
   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);