chakrashim: fix index/toInteger int overflow

Fix JsIntToNumber calls which may overflow when casting to int.

Reviewed-by: @kunalspathak

deps/chakrashim/src/jsrtutils.cc

@@ -24,6 +24,15 @@
 
 namespace jsrt {
 
+JsErrorCode UintToValue(uint32_t value, JsValueRef* result) {
+  if (static_cast<int>(value) >= 0) {
+    return JsIntToNumber(static_cast<int>(value), result);
+  }
+
+  // Otherwise doesn't fit int, use double
+  return JsDoubleToNumber(value, result);
+}
+
 JsErrorCode GetProperty(JsValueRef ref,
                         JsValueRef propName,
                         JsValueRef *result) {
@@ -274,10 +283,10 @@ JsErrorCode HasOwnProperty(JsValueRef object,
 }
 
 JsErrorCode IsValueInArray(
-  JsValueRef arrayRef,
-  JsValueRef valueRef,
-  std::function<JsErrorCode(JsValueRef, JsValueRef, bool*)> comperator,
-  bool* result) {
+    JsValueRef arrayRef,
+    JsValueRef valueRef,
+    std::function<JsErrorCode(JsValueRef, JsValueRef, bool*)> comparator,
+    bool* result) {
   JsErrorCode error;
   unsigned int length;
   *result = false;
@@ -299,8 +308,8 @@ JsErrorCode IsValueInArray(
       return error;
     }
 
-    if (comperator != nullptr) {
-      error = comperator(valueRef, itemRef, result);
+    if (comparator != nullptr) {
+      error = comparator(valueRef, itemRef, result);
     } else {
       error = JsEquals(itemRef, valueRef, result);
     }
@@ -750,11 +759,11 @@ JsErrorCode GetObjectConstructor(JsValueRef objectRef,
 }
 
 JsErrorCode SetIndexedProperty(JsValueRef object,
-                               int index,
+                               unsigned int index,
                                JsValueRef value) {
   JsErrorCode error;
   JsValueRef indexRef;
-  error = JsIntToNumber(index, &indexRef);
+  error = UintToValue(index, &indexRef);
   if (error != JsNoError) {
     return error;
   }
@@ -764,11 +773,11 @@ JsErrorCode SetIndexedProperty(JsValueRef object,
 }
 
 JsErrorCode GetIndexedProperty(JsValueRef object,
-                               int index,
+                               unsigned int index,
                                JsValueRef *value) {
   JsErrorCode error;
   JsValueRef indexRef;
-  error = JsIntToNumber(index, &indexRef);
+  error = UintToValue(index, &indexRef);
   if (error != JsNoError) {
     return error;
   }
@@ -778,10 +787,10 @@ JsErrorCode GetIndexedProperty(JsValueRef object,
 }
 
 JsErrorCode DeleteIndexedProperty(JsValueRef object,
-                                  int index) {
+                                  unsigned int index) {
   JsErrorCode error;
   JsValueRef indexRef;
-  error = JsIntToNumber(index, &indexRef);
+  error = UintToValue(index, &indexRef);
   if (error != JsNoError) {
     return error;
   }
@@ -807,18 +816,19 @@ JsErrorCode HasProperty(JsValueRef object,
 }
 
 JsErrorCode HasIndexedProperty(JsValueRef object,
-                               int index,
+                               unsigned int index,
                                bool *result) {
   JsErrorCode error;
   JsValueRef indexRef;
-  error = JsIntToNumber(index, &indexRef);
+  error = UintToValue(index, &indexRef);
   if (error != JsNoError) {
     return error;
   }
 
   error = JsHasIndexedProperty(object, indexRef, result);
   return error;
 }
+
 JsErrorCode ParseScript(const wchar_t *script,
                         JsSourceContext sourceContext,
                         const wchar_t *sourceUrl,

deps/chakrashim/src/jsrtutils.h

@@ -55,6 +55,8 @@ namespace jsrt {
 
 JsErrorCode InitializePromise();
 
+JsErrorCode UintToValue(uint32_t value, JsValueRef* result);
+
 JsErrorCode GetProperty(JsValueRef ref,
                         JsValueRef propName,
                         JsValueRef *result);
@@ -80,7 +82,7 @@ JsErrorCode SetProperty(JsValueRef ref,
                         JsValueRef propValue);
 
 JsErrorCode DeleteIndexedProperty(JsValueRef object,
-                                  int index);
+                                  unsigned int index);
 
 JsErrorCode DeleteProperty(JsValueRef ref,
                            JsValueRef propName,
@@ -119,7 +121,7 @@ JsErrorCode HasProperty(JsValueRef object,
                         bool *result);
 
 JsErrorCode HasIndexedProperty(JsValueRef object,
-                               int index,
+                               unsigned int index,
                                bool *result);
 
 JsErrorCode GetEnumerableNamedProperties(JsValueRef object,
@@ -267,11 +269,11 @@ JsErrorCode GetObjectConstructor(JsValueRef objectRef,
                                  JsValueRef *constructorRef);
 
 JsErrorCode SetIndexedProperty(JsValueRef object,
-                               int index,
+                               unsigned int index,
                                JsValueRef value);
 
 JsErrorCode GetIndexedProperty(JsValueRef object,
-                               int index,
+                               unsigned int index,
                                JsValueRef *value);
 
 // CHAKRA-TODO : Currently Chakra's ParseScript doesn't support strictMode

deps/chakrashim/src/v8integer.cc

@@ -44,12 +44,14 @@ Local<Integer> Integer::NewFromUnsigned(Isolate* isolate, uint32_t value) {
 }
 
 Local<Integer> Integer::From(uint32_t value) {
-  if (static_cast<int32_t>(value) >= 0) {
-    return From(static_cast<int32_t>(value));
+  JsValueRef ref;
+
+  if (jsrt::UintToValue(value, &ref) != JsNoError) {
+    return Local<Integer>();
   }
 
-  // Otherwise doesn't fit in int32, use double
-  return Number::From(value).As<Integer>();
+  // For perf reason, this doesn't allocate a real Handle
+  return Local<Integer>(ref);
 }
 
 Integer* Integer::Cast(v8::Value* obj) {

deps/chakrashim/src/v8object.cc

@@ -145,14 +145,8 @@ bool Object::ForceSet(Handle<Value> key, Handle<Value> value,
 }
 
 MaybeLocal<Value> Object::Get(Local<Context> context, Local<Value> key) {
-  JsPropertyIdRef idRef;
-
-  if (GetPropertyIdFromValue((JsValueRef)*key, &idRef) != JsNoError) {
-    return Local<Value>();
-  }
-
   JsValueRef valueRef;
-  if (JsGetProperty((JsValueRef)this, idRef, &valueRef) != JsNoError) {
+  if (jsrt::GetProperty((JsValueRef)this, *key, &valueRef) != JsNoError) {
     return Local<Value>();
   }
 
@@ -253,19 +247,11 @@ bool Object::Has(Handle<Value> key) {
 }
 
 Maybe<bool> Object::Delete(Local<Context> context, Local<Value> key) {
-  JsPropertyIdRef idRef;
-
-  if (GetPropertyIdFromName((JsValueRef)*key, &idRef) != JsNoError) {
-    return Nothing<bool>();
-  }
-
   JsValueRef resultRef;
-  if (JsDeleteProperty((JsValueRef)this,
-                       idRef, false, &resultRef) != JsNoError) {
+  if (jsrt::DeleteProperty(this, *key, &resultRef) != JsNoError) {
     return Nothing<bool>();
   }
 
-  // get result
   return Just(Local<Value>(resultRef)->BooleanValue());
 }
 

deps/chakrashim/src/v8value.cc

@@ -233,13 +233,15 @@ MaybeLocal<Integer> Value::ToInteger(Local<Context> context) const {
     return Local<Integer>();
   }
 
-  JsValueRef integerRef;
-  if (JsIntToNumber(static_cast<int>(maybeValue.FromJust()),
-                    &integerRef) != JsNoError) {
-    return Local<Integer>();
+  int64_t value = maybeValue.FromJust();
+  int intValue = static_cast<int>(value);
+
+  if (value == static_cast<int64_t>(intValue)) {
+    return Integer::New(nullptr, intValue);
   }
 
-  return Local<Integer>::New(integerRef);
+  // does not fit int, use double
+  return Number::New(nullptr, value).As<Integer>();
 }
 
 MaybeLocal<Uint32> Value::ToUint32(Local<Context> context) const {