Mojo: Convert MemoryTest.Invalid (-> InvalidDeath) to new user pointer handling.

R=darin@chromium.org

Review URL: https://codereview.chromium.org/420273003

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@286130 0039d316-1c4b-4281-b951-d872f2087c98

2 files changed, 134 insertions, 67 deletions

diff --git a/mojo/system/memory.h b/mojo/system/memory.h
index bbbbbf4..8721a30 100644
--- a/mojo/system/memory.h
+++ b/mojo/system/memory.h
@@ -99,6 +99,15 @@ class UserPointer {
     return UserPointer<ToType>(reinterpret_cast<ToType*>(pointer_));
   }
 
+  // Checks that this pointer points to a valid |Type| in the same way as
+  // |Get()| and |Put()|.
+  // TODO(vtl): Logically, there should be separate read checks and write
+  // checks.
+  void Check() const {
+    internal::CheckUserPointer<sizeof(NonVoidType),
+                               MOJO_ALIGNOF(NonVoidType)>(pointer_);
+  }
+
   // Checks that this pointer points to a valid array (of type |Type|, or just a
   // buffer if |Type| is |void| or |const void|) of |count| elements (or bytes
   // if |Type| is |void| or |const void|) in the same way as |GetArray()| and
@@ -111,13 +120,16 @@ class UserPointer {
         sizeof(NonVoidType), MOJO_ALIGNOF(NonVoidType)>(pointer_, count);
   }
 
-  // Gets the value (of type |Type|) pointed to by this user pointer. Use this
-  // when you'd use the rvalue |*user_pointer|, but be aware that this may be
-  // costly -- so if the value will be used multiple times, you should save it.
+  // Gets the value (of type |Type|, or a |char| if |Type| is |void|) pointed to
+  // by this user pointer. Use this when you'd use the rvalue |*user_pointer|,
+  // but be aware that this may be costly -- so if the value will be used
+  // multiple times, you should save it.
   //
   // (We want to force a copy here, so return |Type| not |const Type&|.)
-  Type Get() const {
-    internal::CheckUserPointer<sizeof(Type), MOJO_ALIGNOF(Type)>(pointer_);
+  NonVoidType Get() const {
+    Check();
+    internal::CheckUserPointer<sizeof(NonVoidType),
+                               MOJO_ALIGNOF(NonVoidType)>(pointer_);
     return *pointer_;
   }
 
@@ -128,8 +140,7 @@ class UserPointer {
   // sizeof(Type)|.
   void GetArray(typename internal::remove_const<Type>::type* destination,
                 size_t count) const {
-    internal::CheckUserPointerWithCount<
-        sizeof(NonVoidType), MOJO_ALIGNOF(NonVoidType)>(pointer_, count);
+    CheckArray(count);
     memcpy(destination, pointer_, count * sizeof(NonVoidType));
   }
 
@@ -144,9 +155,6 @@ class UserPointer {
   // explicitly instantiated. (On implicit instantiation, only the declarations
   // need be valid, not the definitions.)
   //
-  // If |Type| is |void|, we "convert" it to |char|, so that it makes sense.
-  // (Otherwise, we'd need a suitable specialization to exclude |Put()|.)
-  //
   // In C++11, we could do something like:
   //   template <typename _Type = Type>
   //   typename enable_if<!is_const<_Type>::value &&
@@ -155,8 +163,7 @@ class UserPointer {
   // (which obviously be correct), but C++03 doesn't allow default function
   // template arguments.
   void Put(const NonVoidType& value) {
-    internal::CheckUserPointer<sizeof(NonVoidType), MOJO_ALIGNOF(NonVoidType)>(
-        pointer_);
+    Check();
     *pointer_ = value;
   }
 
@@ -168,8 +175,7 @@ class UserPointer {
   // Note: The same comments about the validity of |Put()| (except for the part
   // about |void|) apply here.
   void PutArray(const Type* source, size_t count) {
-    internal::CheckUserPointerWithCount<
-        sizeof(NonVoidType), MOJO_ALIGNOF(NonVoidType)>(pointer_, count);
+    CheckArray(count);
     memcpy(pointer_, source, count * sizeof(NonVoidType));
   }
 
diff --git a/mojo/system/memory_unittest.cc b/mojo/system/memory_unittest.cc
index e5b2c1c..5cfcb32 100644
--- a/mojo/system/memory_unittest.cc
+++ b/mojo/system/memory_unittest.cc
@@ -161,12 +161,12 @@ TEST(MemoryTest, Valid) {
   }
 }
 
-// TODO(vtl): Convert this test.
-#if 0
-TEST(MemoryTest, Invalid) {
-  // Note: |VerifyUserPointer...()| are defined to be "best effort" checks (and
-  // may always return true). Thus these tests of invalid cases only reflect the
-  // current implementation.
+TEST(MemoryTest, InvalidDeath) {
+  const char kMemoryCheckFailedRegex[] = "Check failed";
+
+  // Note: |Check...()| are defined to be "best effort" checks (and may always
+  // return true). Thus these tests of invalid cases only reflect the current
+  // implementation.
 
   // These tests depend on |int32_t| and |int64_t| having nontrivial alignment.
   MOJO_COMPILE_ASSERT(MOJO_ALIGNOF(int32_t) != 1,
@@ -174,59 +174,120 @@ TEST(MemoryTest, Invalid) {
   MOJO_COMPILE_ASSERT(MOJO_ALIGNOF(int64_t) != 1,
                       int64_t_does_not_have_to_be_aligned);
 
-  int32_t my_int32;
-  int64_t my_int64;
-
-  // |VerifyUserPointer|:
-
-  EXPECT_FALSE(VerifyUserPointer<char>(NULL));
-  EXPECT_FALSE(VerifyUserPointer<int32_t>(NULL));
-  EXPECT_FALSE(VerifyUserPointer<int64_t>(NULL));
-
-  // Unaligned:
-  EXPECT_FALSE(VerifyUserPointer<int32_t>(reinterpret_cast<const int32_t*>(1)));
-  EXPECT_FALSE(VerifyUserPointer<int64_t>(reinterpret_cast<const int64_t*>(1)));
-
-  // |VerifyUserPointerWithCount|:
-
-  EXPECT_FALSE(VerifyUserPointerWithCount<char>(NULL, 1));
-  EXPECT_FALSE(VerifyUserPointerWithCount<int32_t>(NULL, 1));
-  EXPECT_FALSE(VerifyUserPointerWithCount<int64_t>(NULL, 1));
+  // Null:
+  {
+    UserPointer<char> ptr(NULL);
+    char array[5] = {};
+    EXPECT_DEATH_IF_SUPPORTED(ptr.Check(), kMemoryCheckFailedRegex);
+    EXPECT_DEATH_IF_SUPPORTED(ptr.Get(), kMemoryCheckFailedRegex);
+    EXPECT_DEATH_IF_SUPPORTED(ptr.Put('x'), kMemoryCheckFailedRegex);
+    EXPECT_DEATH_IF_SUPPORTED(ptr.CheckArray(5), kMemoryCheckFailedRegex);
+    EXPECT_DEATH_IF_SUPPORTED(ptr.GetArray(array, 5), kMemoryCheckFailedRegex);
+    EXPECT_DEATH_IF_SUPPORTED(ptr.PutArray(array, 5), kMemoryCheckFailedRegex);
+  }
+  {
+    UserPointer<int32_t> ptr(NULL);
+    int32_t array[5] = {};
+    EXPECT_DEATH_IF_SUPPORTED(ptr.Check(), kMemoryCheckFailedRegex);
+    EXPECT_DEATH_IF_SUPPORTED(ptr.Get(), kMemoryCheckFailedRegex);
+    EXPECT_DEATH_IF_SUPPORTED(ptr.Put(123), kMemoryCheckFailedRegex);
+    EXPECT_DEATH_IF_SUPPORTED(ptr.CheckArray(5), kMemoryCheckFailedRegex);
+    EXPECT_DEATH_IF_SUPPORTED(ptr.GetArray(array, 5), kMemoryCheckFailedRegex);
+    EXPECT_DEATH_IF_SUPPORTED(ptr.PutArray(array, 5), kMemoryCheckFailedRegex);
+  }
+  {
+    UserPointer<int64_t> ptr(NULL);
+    int64_t array[5] = {};
+    EXPECT_DEATH_IF_SUPPORTED(ptr.Check(), kMemoryCheckFailedRegex);
+    EXPECT_DEATH_IF_SUPPORTED(ptr.Get(), kMemoryCheckFailedRegex);
+    EXPECT_DEATH_IF_SUPPORTED(ptr.Put(123), kMemoryCheckFailedRegex);
+    EXPECT_DEATH_IF_SUPPORTED(ptr.CheckArray(5), kMemoryCheckFailedRegex);
+    EXPECT_DEATH_IF_SUPPORTED(ptr.GetArray(array, 5), kMemoryCheckFailedRegex);
+    EXPECT_DEATH_IF_SUPPORTED(ptr.PutArray(array, 5), kMemoryCheckFailedRegex);
+  }
+  // Also check a const pointer:
+  {
+    UserPointer<const int32_t> ptr(NULL);
+    int32_t array[5] = {};
+    EXPECT_DEATH_IF_SUPPORTED(ptr.Check(), kMemoryCheckFailedRegex);
+    EXPECT_DEATH_IF_SUPPORTED(ptr.Get(), kMemoryCheckFailedRegex);
+    EXPECT_DEATH_IF_SUPPORTED(ptr.CheckArray(5), kMemoryCheckFailedRegex);
+    EXPECT_DEATH_IF_SUPPORTED(ptr.GetArray(array, 5), kMemoryCheckFailedRegex);
+  }
 
   // Unaligned:
-  EXPECT_FALSE(VerifyUserPointerWithCount<int32_t>(
-                   reinterpret_cast<const int32_t*>(1), 1));
-  EXPECT_FALSE(VerifyUserPointerWithCount<int64_t>(
-                   reinterpret_cast<const int64_t*>(1), 1));
+  {
+    int32_t x[10];
+    UserPointer<int32_t> ptr(
+        reinterpret_cast<int32_t*>(reinterpret_cast<uintptr_t>(x) + 1));
+    int32_t array[5] = {};
+    EXPECT_DEATH_IF_SUPPORTED(ptr.Check(), kMemoryCheckFailedRegex);
+    EXPECT_DEATH_IF_SUPPORTED(ptr.Get(), kMemoryCheckFailedRegex);
+    EXPECT_DEATH_IF_SUPPORTED(ptr.Put(123), kMemoryCheckFailedRegex);
+    EXPECT_DEATH_IF_SUPPORTED(ptr.CheckArray(5), kMemoryCheckFailedRegex);
+    EXPECT_DEATH_IF_SUPPORTED(ptr.GetArray(array, 5), kMemoryCheckFailedRegex);
+    EXPECT_DEATH_IF_SUPPORTED(ptr.PutArray(array, 5), kMemoryCheckFailedRegex);
+  }
+  {
+    int64_t x[10];
+    UserPointer<int64_t> ptr(
+        reinterpret_cast<int64_t*>(reinterpret_cast<uintptr_t>(x) + 1));
+    int64_t array[5] = {};
+    EXPECT_DEATH_IF_SUPPORTED(ptr.Check(), kMemoryCheckFailedRegex);
+    EXPECT_DEATH_IF_SUPPORTED(ptr.Get(), kMemoryCheckFailedRegex);
+    EXPECT_DEATH_IF_SUPPORTED(ptr.Put(123), kMemoryCheckFailedRegex);
+    EXPECT_DEATH_IF_SUPPORTED(ptr.CheckArray(5), kMemoryCheckFailedRegex);
+    EXPECT_DEATH_IF_SUPPORTED(ptr.GetArray(array, 5), kMemoryCheckFailedRegex);
+    EXPECT_DEATH_IF_SUPPORTED(ptr.PutArray(array, 5), kMemoryCheckFailedRegex);
+  }
+  // Also check a const pointer:
+  {
+    int32_t x[10];
+    UserPointer<const int32_t> ptr(
+        reinterpret_cast<const int32_t*>(reinterpret_cast<uintptr_t>(x) + 1));
+    int32_t array[5] = {};
+    EXPECT_DEATH_IF_SUPPORTED(ptr.Check(), kMemoryCheckFailedRegex);
+    EXPECT_DEATH_IF_SUPPORTED(ptr.Get(), kMemoryCheckFailedRegex);
+    EXPECT_DEATH_IF_SUPPORTED(ptr.CheckArray(5), kMemoryCheckFailedRegex);
+    EXPECT_DEATH_IF_SUPPORTED(ptr.GetArray(array, 5), kMemoryCheckFailedRegex);
+  }
 
   // Count too big:
-  EXPECT_FALSE(VerifyUserPointerWithCount<int32_t>(
-                   &my_int32, std::numeric_limits<size_t>::max()));
-  EXPECT_FALSE(VerifyUserPointerWithCount<int64_t>(
-                   &my_int64, std::numeric_limits<size_t>::max()));
-
-  // |VerifyUserPointerWithSize|:
-
-  EXPECT_FALSE(VerifyUserPointerWithSize<1>(NULL, 1));
-  EXPECT_FALSE(VerifyUserPointerWithSize<4>(NULL, 1));
-  EXPECT_FALSE(VerifyUserPointerWithSize<4>(NULL, 4));
-  EXPECT_FALSE(VerifyUserPointerWithSize<8>(NULL, 1));
-  EXPECT_FALSE(VerifyUserPointerWithSize<8>(NULL, 4));
-  EXPECT_FALSE(VerifyUserPointerWithSize<8>(NULL, 8));
+  {
+    const size_t kTooBig =
+        std::numeric_limits<size_t>::max() / sizeof(int32_t) + 1;
+    int32_t x = 0;
+    UserPointer<int32_t> ptr(&x);
+    EXPECT_DEATH_IF_SUPPORTED(ptr.CheckArray(kTooBig), kMemoryCheckFailedRegex);
+    EXPECT_DEATH_IF_SUPPORTED(ptr.GetArray(&x, kTooBig),
+                              kMemoryCheckFailedRegex);
+    EXPECT_DEATH_IF_SUPPORTED(ptr.PutArray(&x, kTooBig),
+                              kMemoryCheckFailedRegex);
+  }
+  {
+    const size_t kTooBig =
+        std::numeric_limits<size_t>::max() / sizeof(int64_t) + 1;
+    int64_t x = 0;
+    UserPointer<int64_t> ptr(&x);
+    EXPECT_DEATH_IF_SUPPORTED(ptr.CheckArray(kTooBig), kMemoryCheckFailedRegex);
+    EXPECT_DEATH_IF_SUPPORTED(ptr.GetArray(&x, kTooBig),
+                              kMemoryCheckFailedRegex);
+    EXPECT_DEATH_IF_SUPPORTED(ptr.PutArray(&x, kTooBig),
+                              kMemoryCheckFailedRegex);
+  }
+  // Also check a const pointer:
+  {
+    const size_t kTooBig =
+        std::numeric_limits<size_t>::max() / sizeof(int32_t) + 1;
+    int32_t x = 0;
+    UserPointer<const int32_t> ptr(&x);
+    EXPECT_DEATH_IF_SUPPORTED(ptr.CheckArray(kTooBig), kMemoryCheckFailedRegex);
+    EXPECT_DEATH_IF_SUPPORTED(ptr.GetArray(&x, kTooBig),
+                              kMemoryCheckFailedRegex);
+  }
 
-  // Unaligned:
-  EXPECT_FALSE(VerifyUserPointerWithSize<4>(reinterpret_cast<const int32_t*>(1),
-                                            1));
-  EXPECT_FALSE(VerifyUserPointerWithSize<4>(reinterpret_cast<const int32_t*>(1),
-                                            4));
-  EXPECT_FALSE(VerifyUserPointerWithSize<8>(reinterpret_cast<const int32_t*>(1),
-                                            1));
-  EXPECT_FALSE(VerifyUserPointerWithSize<8>(reinterpret_cast<const int32_t*>(1),
-                                            4));
-  EXPECT_FALSE(VerifyUserPointerWithSize<8>(reinterpret_cast<const int32_t*>(1),
-                                            8));
+  // TODO(vtl): Tests for |UserPointer{Reader,Writer,ReaderWriter}|.
 }
-#endif
 
 }  // namespace
 }  // namespace system