Bug 1091882: Simplify some uses of mozilla::PointerRangeSize, and clarify comment. r=waldo
authorJim Blandy <jimb@mozilla.com>
Mon, 03 Nov 2014 15:55:59 -0800
changeset 238127 6902b39cd26e7bf745de72bc459bdb1a7126c6c2
parent 238126 b310190a62b512b116d85ea23a45383a5d34115b
child 238128 c49189c6c30ac07736e2dde37eab54f74827d01b
push id4311
push userraliiev@mozilla.com
push dateMon, 12 Jan 2015 19:37:41 +0000
treeherdermozilla-beta@150c9fed433b [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerswaldo
bugs1091882
milestone36.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 1091882: Simplify some uses of mozilla::PointerRangeSize, and clarify comment. r=waldo
js/src/jsarray.cpp
mfbt/ArrayUtils.h
mfbt/PodOperations.h
--- a/js/src/jsarray.cpp
+++ b/js/src/jsarray.cpp
@@ -46,17 +46,16 @@ using namespace js;
 using namespace js::gc;
 using namespace js::types;
 
 using mozilla::Abs;
 using mozilla::ArrayLength;
 using mozilla::CeilingLog2;
 using mozilla::DebugOnly;
 using mozilla::IsNaN;
-using mozilla::PointerRangeSize;
 
 using JS::AutoCheckCannotGC;
 
 bool
 js::GetLengthProperty(JSContext *cx, HandleObject obj, uint32_t *lengthp)
 {
     if (obj->is<ArrayObject>()) {
         *lengthp = obj->as<ArrayObject>().length();
--- a/mfbt/ArrayUtils.h
+++ b/mfbt/ArrayUtils.h
@@ -20,20 +20,26 @@
 
 #include "mozilla/Alignment.h"
 #include "mozilla/Array.h"
 #include "mozilla/TypeTraits.h"
 
 namespace mozilla {
 
 /*
- * Safely subtract two pointers when it is known that aEnd >= aBegin.  This
- * avoids the common compiler bug that if (size_t(aEnd) - size_t(aBegin)) has
- * the MSB set, the unsigned subtraction followed by right shift will produce
- * -1, or size_t(-1), instead of the real difference.
+ * Safely subtract two pointers when it is known that aEnd >= aBegin, yielding a
+ * size_t result.
+ *
+ * Ordinary pointer subtraction yields a ptrdiff_t result, which, being signed,
+ * has insufficient range to express the distance between pointers at opposite
+ * ends of the address space. Furthermore, most compilers use ptrdiff_t to
+ * represent the intermediate byte address distance, before dividing by
+ * sizeof(T); if that intermediate result overflows, they'll produce results
+ * with the wrong sign even when the correct scaled distance would fit in a
+ * ptrdiff_t.
  */
 template<class T>
 MOZ_ALWAYS_INLINE size_t
 PointerRangeSize(T* aBegin, T* aEnd)
 {
   MOZ_ASSERT(aEnd >= aBegin);
   return (size_t(aEnd) - size_t(aBegin)) / sizeof(T);
 }
--- a/mfbt/PodOperations.h
+++ b/mfbt/PodOperations.h
@@ -77,39 +77,32 @@ PodArrayZero(Array<T, N>& aArr)
 /**
  * Assign |*aSrc| to |*aDst|.  The locations must not be the same and must not
  * overlap.
  */
 template<typename T>
 static MOZ_ALWAYS_INLINE void
 PodAssign(T* aDst, const T* aSrc)
 {
-  MOZ_ASSERT(aDst != aSrc);
-  MOZ_ASSERT_IF(aSrc < aDst,
-                PointerRangeSize(aSrc, static_cast<const T*>(aDst)) >= 1);
-  MOZ_ASSERT_IF(aDst < aSrc,
-                PointerRangeSize(static_cast<const T*>(aDst), aSrc) >= 1);
+  MOZ_ASSERT(aDst + 1 <= aSrc || aSrc + 1 <= aDst,
+             "destination and source must not overlap");
   memcpy(reinterpret_cast<char*>(aDst), reinterpret_cast<const char*>(aSrc),
          sizeof(T));
 }
 
 /**
  * Copy |aNElem| T elements from |aSrc| to |aDst|.  The two memory ranges must
  * not overlap!
  */
 template<typename T>
 static MOZ_ALWAYS_INLINE void
 PodCopy(T* aDst, const T* aSrc, size_t aNElem)
 {
-  MOZ_ASSERT(aDst != aSrc);
-  MOZ_ASSERT_IF(aSrc < aDst,
-                PointerRangeSize(aSrc, static_cast<const T*>(aDst)) >= aNElem);
-  MOZ_ASSERT_IF(aDst < aSrc,
-                PointerRangeSize(static_cast<const T*>(aDst), aSrc) >= aNElem);
-
+  MOZ_ASSERT(aDst + aNElem <= aSrc || aSrc + aNElem <= aDst,
+             "destination and source must not overlap");
   if (aNElem < 128) {
     /*
      * Avoid using operator= in this loop, as it may have been
      * intentionally deleted by the POD type.
      */
     for (const T* srcend = aSrc + aNElem; aSrc < srcend; aSrc++, aDst++) {
       PodAssign(aDst, aSrc);
     }
@@ -117,21 +110,18 @@ PodCopy(T* aDst, const T* aSrc, size_t a
     memcpy(aDst, aSrc, aNElem * sizeof(T));
   }
 }
 
 template<typename T>
 static MOZ_ALWAYS_INLINE void
 PodCopy(volatile T* aDst, const volatile T* aSrc, size_t aNElem)
 {
-  MOZ_ASSERT(aDst != aSrc);
-  MOZ_ASSERT_IF(aSrc < aDst,
-    PointerRangeSize(aSrc, static_cast<const volatile T*>(aDst)) >= aNElem);
-  MOZ_ASSERT_IF(aDst < aSrc,
-    PointerRangeSize(static_cast<const volatile T*>(aDst), aSrc) >= aNElem);
+  MOZ_ASSERT(aDst + aNElem <= aSrc || aSrc + aNElem <= aDst,
+             "destination and source must not overlap");
 
   /*
    * Volatile |aDst| requires extra work, because it's undefined behavior to
    * modify volatile objects using the mem* functions.  Just write out the
    * loops manually, using operator= rather than memcpy for the same reason,
    * and let the compiler optimize to the extent it can.
    */
   for (const volatile T* srcend = aSrc + aNElem;