Bug 1159244 - Add release mode bounds checking with custom annotations to nsTArray, r=froydnj
authorMichael Layzell <michael@thelayzells.com>
Wed, 10 Aug 2016 14:40:00 -0400
changeset 309310 0c17b64aac661b09afadecbb7dfbd44575f42868
parent 309309 5150bc3f6d211ff0b7ba9c52c9bb5a97b793fec4
child 309311 1c8384a1cfe69e2990e8797e14f15b5ef175759d
push id80574
push usermichael@thelayzells.com
push dateMon, 15 Aug 2016 17:29:54 +0000
treeherdermozilla-inbound@0c17b64aac66 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersfroydnj
bugs1159244
milestone51.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 1159244 - Add release mode bounds checking with custom annotations to nsTArray, r=froydnj MozReview-Commit-ID: Ljx9PwBCyTT
mfbt/Assertions.h
xpcom/glue/nsTArray.cpp
xpcom/glue/nsTArray.h
--- a/mfbt/Assertions.h
+++ b/mfbt/Assertions.h
@@ -29,17 +29,17 @@ namespace CrashReporter {
 // nsExceptionHandler.h is not directly included in this file as it includes
 // windows.h, which can cause problems when it is imported into some files due
 // to the number of macros defined.
 // XXX If you change this definition - also change the definition in
 // nsExceptionHandler.h
 void AnnotateMozCrashReason(const char* aReason);
 } // namespace CrashReporter
 
-#  define MOZ_CRASH_ANNOTATE(...) CrashReporter::AnnotateMozCrashReason("" __VA_ARGS__)
+#  define MOZ_CRASH_ANNOTATE(...) CrashReporter::AnnotateMozCrashReason(__VA_ARGS__)
 #else
 #  define MOZ_CRASH_ANNOTATE(...) do { /* nothing */ } while (0)
 #endif
 
 #include <stddef.h>
 #include <stdio.h>
 #include <stdlib.h>
 #ifdef WIN32
--- a/xpcom/glue/nsTArray.cpp
+++ b/xpcom/glue/nsTArray.cpp
@@ -13,8 +13,24 @@
 nsTArrayHeader nsTArrayHeader::sEmptyHdr = { 0, 0, 0 };
 
 bool
 IsTwiceTheRequiredBytesRepresentableAsUint32(size_t aCapacity, size_t aElemSize)
 {
   using mozilla::CheckedUint32;
   return ((CheckedUint32(aCapacity) * aElemSize) * 2).isValid();
 }
+
+MOZ_NORETURN MOZ_COLD void
+InvalidArrayIndex_CRASH(size_t aIndex, size_t aLength)
+{
+  const size_t CAPACITY = 512;
+  // Leak the buffer on the heap to make sure that it lives long enough, as
+  // MOZ_CRASH_ANNOTATE expects the pointer passed to it to live to the end of
+  // the program.
+  char* buffer = new char[CAPACITY];
+  snprintf(buffer, CAPACITY,
+           "ElementAt(aIndex = %llu, aLength = %llu)",
+           (long long unsigned) aIndex,
+           (long long unsigned) aLength);
+  MOZ_CRASH_ANNOTATE(buffer);
+  MOZ_REALLY_CRASH();
+}
--- a/xpcom/glue/nsTArray.h
+++ b/xpcom/glue/nsTArray.h
@@ -325,16 +325,19 @@ struct nsTArray_SafeElementAtHelper<mozi
       return static_cast<const Derived*>(this)->ElementAt(aIndex);
     }
     return nullptr;
   }
 };
 
 extern "C" void Gecko_EnsureTArrayCapacity(void* aArray, size_t aCapacity, size_t aElemSize);
 
+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
 // element type of the nsTArray.
 //
 template<class Alloc, class Copy>
 class nsTArray_base
 {
@@ -984,27 +987,31 @@ public:
   }
 
   // This method provides direct access to an element of the array. The given
   // index must be within the array bounds.
   // @param aIndex The index of an element in the array.
   // @return A reference to the i'th element of the array.
   elem_type& ElementAt(index_type aIndex)
   {
-    MOZ_ASSERT(aIndex < Length(), "invalid array index");
+    if (MOZ_UNLIKELY(aIndex >= Length())) {
+      InvalidArrayIndex_CRASH(aIndex, Length());
+    }
     return Elements()[aIndex];
   }
 
   // This method provides direct, readonly access to an element of the array
   // The given index must be within the array bounds.
   // @param aIndex The index of an element in the array.
   // @return A const reference to the i'th element of the array.
   const elem_type& ElementAt(index_type aIndex) const
   {
-    MOZ_ASSERT(aIndex < Length(), "invalid array index");
+    if (MOZ_UNLIKELY(aIndex >= Length())) {
+      InvalidArrayIndex_CRASH(aIndex, Length());
+    }
     return Elements()[aIndex];
   }
 
   // This method provides direct access to an element of the array in a bounds
   // safe manner. If the requested index is out of bounds the provided default
   // value is returned.
   // @param aIndex The index of an element in the array.
   // @param aDef   The value to return if the index is out of bounds.