Bug 1531638 - Make Vector::begin() always return a non-null pointer, to accommodate users that need this. r=froydnj
☠☠ backed out by a7bb6e5afaf2 ☠ ☠
authorJeff Walden <jwalden@mit.edu>
Thu, 28 Feb 2019 21:30:49 -0800
changeset 520194 e5d9f2ee7ac7c67f8498b1f13f1d301a76f1fedc
parent 520193 c12a447c2785d9efc277b484e6c67c1b9d892a8d
child 520195 738cf4b1126a5e2daa22fe3f8e653b35da0306ad
push id10862
push userffxbld-merge
push dateMon, 11 Mar 2019 13:01:11 +0000
treeherdermozilla-beta@a2e7f5c935da [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersfroydnj
bugs1531638
milestone67.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 1531638 - Make Vector::begin() always return a non-null pointer, to accommodate users that need this. r=froydnj Differential Revision: https://phabricator.services.mozilla.com/D21825
mfbt/Vector.h
mfbt/tests/TestVector.cpp
--- a/mfbt/Vector.h
+++ b/mfbt/Vector.h
@@ -398,17 +398,24 @@ class MOZ_NON_PARAM Vector final : priva
   };
 
   template <size_t Dummy>
   struct CRAndStorage<0, Dummy> : CapacityAndReserved {
     explicit CRAndStorage(size_t aCapacity, size_t aReserved)
         : CapacityAndReserved(aCapacity, aReserved) {}
     CRAndStorage() = default;
 
-    T* storage() { return nullptr; }
+    T* storage() {
+      // If this returns |nullptr|, functions like |Vector::begin()| would too,
+      // breaking callers that pass a vector's elements as pointer/length to
+      // code that bounds its operation by length but (even just as a sanity
+      // check) always wants a non-null pointer.  Fake up an aligned, non-null
+      // pointer to support these callers.
+      return reinterpret_cast<T*>(sizeof(T));
+    }
   };
 
   CRAndStorage<kInlineCapacity, 0> mTail;
 
 #ifdef _MSC_VER
 #  pragma warning(pop)
 #endif  // _MSC_VER
 
--- a/mfbt/tests/TestVector.cpp
+++ b/mfbt/tests/TestVector.cpp
@@ -501,19 +501,81 @@ static_assert(sizeof(Vector<S, 0>) == si
 
 static_assert(sizeof(Vector<Incomplete, 0>) ==
                   sizeof(NoInlineStorageLayout<Incomplete>),
               "Vector of an incomplete class without inline storage shouldn't "
               "occupy dead space for that absence of storage");
 
 #endif  // DEBUG
 
+static void TestVectorBeginNonNull() {
+  // Vector::begin() should never return nullptr, to accommodate callers that
+  // (either for hygiene, or for semantic reasons)  need a valid pointer even
+  // for zero elements.
+
+  Vector<bool, 0> bvec0;
+  MOZ_RELEASE_ASSERT(bvec0.length() == 0);
+  MOZ_RELEASE_ASSERT(bvec0.begin() != nullptr);
+
+  Vector<bool, 1> bvec1;
+  MOZ_RELEASE_ASSERT(bvec1.length() == 0);
+  MOZ_RELEASE_ASSERT(bvec1.begin() != nullptr);
+
+  Vector<bool, 64> bvec64;
+  MOZ_RELEASE_ASSERT(bvec64.length() == 0);
+  MOZ_RELEASE_ASSERT(bvec64.begin() != nullptr);
+
+  Vector<int, 0> ivec0;
+  MOZ_RELEASE_ASSERT(ivec0.length() == 0);
+  MOZ_RELEASE_ASSERT(ivec0.begin() != nullptr);
+
+  Vector<int, 1> ivec1;
+  MOZ_RELEASE_ASSERT(ivec1.length() == 0);
+  MOZ_RELEASE_ASSERT(ivec1.begin() != nullptr);
+
+  Vector<int, 64> ivec64;
+  MOZ_RELEASE_ASSERT(ivec64.length() == 0);
+  MOZ_RELEASE_ASSERT(ivec64.begin() != nullptr);
+
+  Vector<long, 0> lvec0;
+  MOZ_RELEASE_ASSERT(lvec0.length() == 0);
+  MOZ_RELEASE_ASSERT(lvec0.begin() != nullptr);
+
+  Vector<long, 1> lvec1;
+  MOZ_RELEASE_ASSERT(lvec1.length() == 0);
+  MOZ_RELEASE_ASSERT(lvec1.begin() != nullptr);
+
+  Vector<long, 64> lvec64;
+  MOZ_RELEASE_ASSERT(lvec64.length() == 0);
+  MOZ_RELEASE_ASSERT(lvec64.begin() != nullptr);
+
+  // Vector<T, N> doesn't guarantee N inline elements -- the actual count is
+  // capped so that any Vector fits in a not-crazy amount of space -- so the
+  // code below won't overflow stacks or anything crazy.
+  struct VeryBig {
+    int array[16 * 1024 * 1024];
+  };
+
+  Vector<VeryBig, 0> vbvec0;
+  MOZ_RELEASE_ASSERT(vbvec0.length() == 0);
+  MOZ_RELEASE_ASSERT(vbvec0.begin() != nullptr);
+
+  Vector<VeryBig, 1> vbvec1;
+  MOZ_RELEASE_ASSERT(vbvec1.length() == 0);
+  MOZ_RELEASE_ASSERT(vbvec1.begin() != nullptr);
+
+  Vector<VeryBig, 64> vbvec64;
+  MOZ_RELEASE_ASSERT(vbvec64.length() == 0);
+  MOZ_RELEASE_ASSERT(vbvec64.begin() != nullptr);
+}
+
 int main() {
   VectorTesting::testReserved();
   VectorTesting::testConstRange();
   VectorTesting::testEmplaceBack();
   VectorTesting::testReverse();
   VectorTesting::testExtractRawBuffer();
   VectorTesting::testExtractOrCopyRawBuffer();
   VectorTesting::testReplaceRawBuffer();
   VectorTesting::testInsert();
   VectorTesting::testPodResizeToFit();
+  TestVectorBeginNonNull();
 }