Bug 1704042 part 9 - Report an exception for large lengths from JS::GetArrayLength API. r=lth
authorJan de Mooij <jdemooij@mozilla.com>
Fri, 09 Apr 2021 13:20:48 +0000
changeset 575220 79f3f364ed334760f3026af08b9611e74aa62d3c
parent 575219 6da24a2d63347051144344c01fb92d00e2590ba5
child 575221 2ac2582ca447674ce82b31950c0ea07560fac111
push id38359
push usernbeleuzu@mozilla.com
push dateSat, 10 Apr 2021 03:36:58 +0000
treeherdermozilla-central@f84605e6fc8b [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerslth
bugs1704042
milestone89.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1704042 part 9 - Report an exception for large lengths from JS::GetArrayLength API. r=lth There are a number of callers in Gecko, most of them are using this to iterate over an object expected to be an array. This should be okay for now; if there are callers that need the full length we can add a separate API that returns uint64_t. This lets us remove ToLengthClamped and the GetLengthProperty overload returning uint32_t. Differential Revision: https://phabricator.services.mozilla.com/D111389
js/public/Array.h
js/src/builtin/Array.cpp
js/src/builtin/Array.h
--- a/js/public/Array.h
+++ b/js/public/Array.h
@@ -55,16 +55,19 @@ extern JS_PUBLIC_API bool IsArrayObject(
  */
 extern JS_PUBLIC_API bool IsArrayObject(JSContext* cx, Handle<JSObject*> obj,
                                         bool* isArray);
 
 /**
  * Store |*lengthp = ToLength(obj.length)| and return true on success, else
  * return false.
  *
+ * If the length does not fit in |uint32_t|, an exception is reported and false
+ * is returned.
+ *
  * |ToLength| converts its input to an integer usable to index an
  * array-like object.
  *
  * If |obj| is an Array, this overall operation is the same as getting
  * |obj.length|.
  */
 extern JS_PUBLIC_API bool GetArrayLength(JSContext* cx, Handle<JSObject*> obj,
                                          uint32_t* lengthp);
--- a/js/src/builtin/Array.cpp
+++ b/js/src/builtin/Array.cpp
@@ -98,68 +98,16 @@ bool JS::IsArray(JSContext* cx, HandleOb
   *isArray = answer == IsArrayAnswer::Array;
   return true;
 }
 
 bool js::IsArrayFromJit(JSContext* cx, HandleObject obj, bool* isArray) {
   return JS::IsArray(cx, obj, isArray);
 }
 
-// ES2017 7.1.15 ToLength, but clamped to the [0,2^32-2] range.
-static bool ToLengthClamped(JSContext* cx, HandleValue v, uint32_t* out) {
-  if (v.isInt32()) {
-    int32_t i = v.toInt32();
-    *out = i < 0 ? 0 : i;
-    return true;
-  }
-  double d;
-  if (v.isDouble()) {
-    d = v.toDouble();
-  } else {
-    if (!ToNumber(cx, v, &d)) {
-      return false;
-    }
-  }
-  d = JS::ToInteger(d);
-  if (d <= 0.0) {
-    *out = 0;
-  } else if (d < double(UINT32_MAX - 1)) {
-    *out = uint32_t(d);
-  } else {
-    *out = UINT32_MAX;
-  }
-  return true;
-}
-
-bool js::GetLengthProperty(JSContext* cx, HandleObject obj, uint32_t* lengthp) {
-  if (obj->is<ArrayObject>()) {
-    *lengthp = obj->as<ArrayObject>().length();
-    return true;
-  }
-
-  if (obj->is<ArgumentsObject>()) {
-    ArgumentsObject& argsobj = obj->as<ArgumentsObject>();
-    if (!argsobj.hasOverriddenLength()) {
-      *lengthp = argsobj.initialLength();
-      return true;
-    }
-  }
-
-  RootedValue value(cx);
-  if (!GetProperty(cx, obj, obj, cx->names().length, &value)) {
-    return false;
-  }
-
-  if (!ToLengthClamped(cx, value, lengthp)) {
-    return false;
-  }
-
-  return true;
-}
-
 // ES2017 7.1.15 ToLength.
 bool js::ToLength(JSContext* cx, HandleValue v, uint64_t* out) {
   if (v.isInt32()) {
     int32_t i = v.toInt32();
     *out = i < 0 ? 0 : i;
     return true;
   }
 
@@ -4267,17 +4215,29 @@ JS_PUBLIC_API bool JS::IsArrayObject(JSC
 }
 
 JS_PUBLIC_API bool JS::GetArrayLength(JSContext* cx, Handle<JSObject*> obj,
                                       uint32_t* lengthp) {
   AssertHeapIsIdle();
   CHECK_THREAD(cx);
   cx->check(obj);
 
-  return GetLengthProperty(cx, obj, lengthp);
+  uint64_t len = 0;
+  if (!GetLengthProperty(cx, obj, &len)) {
+    return false;
+  }
+
+  if (len > UINT32_MAX) {
+    JS_ReportErrorNumberASCII(cx, GetErrorMessage, nullptr,
+                              JSMSG_BAD_ARRAY_LENGTH);
+    return false;
+  }
+
+  *lengthp = uint32_t(len);
+  return true;
 }
 
 JS_PUBLIC_API bool JS::SetArrayLength(JSContext* cx, Handle<JSObject*> obj,
                                       uint32_t length) {
   AssertHeapIsIdle();
   CHECK_THREAD(cx);
   cx->check(obj);
 
--- a/js/src/builtin/Array.h
+++ b/js/src/builtin/Array.h
@@ -78,19 +78,16 @@ extern ArrayObject* NewDenseFullyAllocat
     JSContext* cx, uint32_t length, ArrayObject* templateObject);
 
 extern ArrayObject* NewArrayWithShape(JSContext* cx, uint32_t length,
                                       HandleShape shape);
 
 extern bool ToLength(JSContext* cx, HandleValue v, uint64_t* out);
 
 extern bool GetLengthProperty(JSContext* cx, HandleObject obj,
-                              uint32_t* lengthp);
-
-extern bool GetLengthProperty(JSContext* cx, HandleObject obj,
                               uint64_t* lengthp);
 
 extern bool SetLengthProperty(JSContext* cx, HandleObject obj, uint32_t length);
 
 /*
  * Copy 'length' elements from aobj to vp.
  *
  * This function assumes 'length' is effectively the result of calling