Bug 1624717 - Copy release mode bounds checking from nsTArray to Array r=glandium

Differential Revision: https://phabricator.services.mozilla.com/D124833

mfbt/Array.h

@@ -17,6 +17,7 @@
 
 #include "mozilla/Assertions.h"
 #include "mozilla/Attributes.h"
+#include "mozilla/Likely.h"
 
 namespace mozilla {
 
@@ -39,12 +40,16 @@ class Array {
   }
 
   T& operator[](size_t aIndex) {
-    MOZ_ASSERT(aIndex < Length);
+    if (MOZ_UNLIKELY(aIndex >= Length)) {
+      detail::InvalidArrayIndex_CRASH(aIndex, Length);
+    }
     return mArr[aIndex];
   }
 
   const T& operator[](size_t aIndex) const {
-    MOZ_ASSERT(aIndex < Length);
+    if (MOZ_UNLIKELY(aIndex >= Length)) {
+      detail::InvalidArrayIndex_CRASH(aIndex, Length);
+    }
     return mArr[aIndex];
   }
 

mfbt/Assertions.cpp

@@ -44,3 +44,9 @@ MFBT_API MOZ_COLD MOZ_NEVER_INLINE MOZ_FORMAT_PRINTF(1, 2) const
 }
 
 MOZ_END_EXTERN_C
+
+MFBT_API MOZ_NORETURN MOZ_COLD void mozilla::detail::InvalidArrayIndex_CRASH(
+    size_t aIndex, size_t aLength) {
+  MOZ_CRASH_UNSAFE_PRINTF("ElementAt(aIndex = %zu, aLength = %zu)", aIndex,
+                          aLength);
+}

mfbt/Assertions.h

@@ -609,4 +609,15 @@ struct AssertionConditionType {
 #undef MOZ_DUMP_ASSERTION_STACK
 #undef MOZ_CRASH_CRASHREPORT
 
+/*
+ * This is only used by Array and nsTArray classes, therefore it is not
+ * required when included from C code.
+ */
+#ifdef __cplusplus
+namespace mozilla::detail {
+MFBT_API MOZ_NORETURN MOZ_COLD void InvalidArrayIndex_CRASH(size_t aIndex,
+                                                            size_t aLength);
+}  // namespace mozilla::detail
+#endif  // __cplusplus
+
 #endif /* mozilla_Assertions_h */

xpcom/ds/nsTArray-inl.h

@@ -412,7 +412,7 @@ nsTArray_base<Alloc, RelocationStrategy>::InsertSlotsAt(index_type aIndex,
                                                         size_type aElemSize,
                                                         size_t aElemAlign) {
   if (MOZ_UNLIKELY(aIndex > Length())) {
-    InvalidArrayIndex_CRASH(aIndex, Length());
+    mozilla::detail::InvalidArrayIndex_CRASH(aIndex, Length());
   }
 
   if (!ActualAlloc::Successful(

xpcom/ds/nsTArray.cpp

@@ -23,13 +23,6 @@ bool IsTwiceTheRequiredBytesRepresentableAsUint32(size_t aCapacity,
   return ((CheckedUint32(aCapacity) * aElemSize) * 2).isValid();
 }
 
-MOZ_NORETURN MOZ_COLD void InvalidArrayIndex_CRASH(size_t aIndex,
-                                                   size_t aLength) {
-  MOZ_CRASH_UNSAFE_PRINTF(
-      "ElementAt(aIndex = %" PRIu64 ", aLength = %" PRIu64 ")",
-      static_cast<uint64_t>(aIndex), static_cast<uint64_t>(aLength));
-}
-
 void ::detail::SetCycleCollectionArrayFlag(uint32_t& aFlags) {
   aFlags |= CycleCollectionEdgeNameArrayFlag;
 }

xpcom/ds/nsTArray.h

@@ -376,9 +376,6 @@ extern "C" void Gecko_EnsureTArrayCapacity(void* aArray, size_t aCapacity,
 extern "C" void Gecko_ClearPODTArray(void* aArray, size_t aElementSize,
                                      size_t aElementAlign);
 
-MOZ_NORETURN MOZ_COLD void InvalidArrayIndex_CRASH(size_t aIndex,
-                                                   size_t aLength);
-
 //
 // This class serves as a base class for nsTArray.  It shouldn't be used
 // directly.  It holds common implementation code that does not depend on the
@@ -1197,7 +1194,7 @@ class nsTArray_Impl
   // @return A reference to the i'th element of the array.
   [[nodiscard]] elem_type& ElementAt(index_type aIndex) {
     if (MOZ_UNLIKELY(aIndex >= Length())) {
-      InvalidArrayIndex_CRASH(aIndex, Length());
+      mozilla::detail::InvalidArrayIndex_CRASH(aIndex, Length());
     }
     return Elements()[aIndex];
   }
@@ -1208,7 +1205,7 @@ class nsTArray_Impl
   // @return A const reference to the i'th element of the array.
   [[nodiscard]] const elem_type& ElementAt(index_type aIndex) const {
     if (MOZ_UNLIKELY(aIndex >= Length())) {
-      InvalidArrayIndex_CRASH(aIndex, Length());
+      mozilla::detail::InvalidArrayIndex_CRASH(aIndex, Length());
     }
     return Elements()[aIndex];
   }
@@ -1871,7 +1868,7 @@ class nsTArray_Impl
     MOZ_ASSERT(!base_type::IsEmpty());
     const size_type oldLen = Length();
     if (MOZ_UNLIKELY(0 == oldLen)) {
-      InvalidArrayIndex_CRASH(1, 0);
+      mozilla::detail::InvalidArrayIndex_CRASH(1, 0);
     }
     elem_type elem = std::move(Elements()[oldLen - 1]);
     TruncateLengthUnsafe(oldLen - 1);
@@ -2248,7 +2245,7 @@ class nsTArray_Impl
     MOZ_ASSERT(aNewLen <= Length(), "caller should use SetLength instead");
 
     if (MOZ_UNLIKELY(aNewLen > Length())) {
-      InvalidArrayIndex_CRASH(aNewLen, Length());
+      mozilla::detail::InvalidArrayIndex_CRASH(aNewLen, Length());
     }
 
     TruncateLengthUnsafe(aNewLen);
@@ -2459,7 +2456,7 @@ auto nsTArray_Impl<E, Alloc>::ReplaceElementsAtInternal(index_type aStart,
                                                         size_type aArrayLen)
     -> elem_type* {
   if (MOZ_UNLIKELY(aStart > Length())) {
-    InvalidArrayIndex_CRASH(aStart, Length());
+    mozilla::detail::InvalidArrayIndex_CRASH(aStart, Length());
   }
 
   // Adjust memory allocation up-front to catch errors.
@@ -2483,7 +2480,7 @@ void nsTArray_Impl<E, Alloc>::RemoveElementsAt(index_type aStart,
   rangeEnd += aCount;
 
   if (MOZ_UNLIKELY(!rangeEnd.isValid() || rangeEnd.value() > Length())) {
-    InvalidArrayIndex_CRASH(aStart, Length());
+    mozilla::detail::InvalidArrayIndex_CRASH(aStart, Length());
   }
 
   RemoveElementsAtUnsafe(aStart, aCount);
@@ -2506,7 +2503,7 @@ void nsTArray_Impl<E, Alloc>::UnorderedRemoveElementsAt(index_type aStart,
   rangeEnd += aCount;
 
   if (MOZ_UNLIKELY(!rangeEnd.isValid() || rangeEnd.value() > Length())) {
-    InvalidArrayIndex_CRASH(aStart, Length());
+    mozilla::detail::InvalidArrayIndex_CRASH(aStart, Length());
   }
 
   // Destroy the elements which are being removed, and then swap elements in to
@@ -2576,7 +2573,7 @@ template <typename ActualAlloc>
 auto nsTArray_Impl<E, Alloc>::InsertElementAtInternal(index_type aIndex)
     -> elem_type* {
   if (MOZ_UNLIKELY(aIndex > Length())) {
-    InvalidArrayIndex_CRASH(aIndex, Length());
+    mozilla::detail::InvalidArrayIndex_CRASH(aIndex, Length());
   }
 
   // Length() + 1 is guaranteed to not overflow, so EnsureCapacity is OK.
@@ -2597,7 +2594,7 @@ auto nsTArray_Impl<E, Alloc>::InsertElementAtInternal(index_type aIndex,
                                                       Item&& aItem)
     -> elem_type* {
   if (MOZ_UNLIKELY(aIndex > Length())) {
-    InvalidArrayIndex_CRASH(aIndex, Length());
+    mozilla::detail::InvalidArrayIndex_CRASH(aIndex, Length());
   }
 
   // Length() + 1 is guaranteed to not overflow, so EnsureCapacity is OK.