Add compile time checks against longs being used in IPC structs on 32 bit Android.

long is 4 bytes in 32 bit builds and 8 bytes in 64 bit builds so we don't want to send it between 32 and 64 bit processes. We can't remove the long IPC traits completely since types like uint64_t also use the long traits on Windows and 64 bit POSIX. So keep the traits for these platforms but remove it for others. This ensures that longs aren't sent over IPC between 32 and 64 bit  configs since the 32 bit build would have a compile error.

Also remove the size_t methods from Pickle. We can't add compile time checks for that since it's a typedef. A clang plugin will catch those cases.

BUG=581409

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

Cr-Commit-Position: refs/heads/master@{#374707}

3 files changed, 35 insertions, 59 deletions

diff --git a/base/pickle.cc b/base/pickle.cc
index b12ec9a..94f3b1c 100644
--- a/base/pickle.cc
+++ b/base/pickle.cc
@@ -11,6 +11,7 @@
 
 #include "base/bits.h"
 #include "base/macros.h"
+#include "base/numerics/safe_conversions.h"
 #include "build/build_config.h"
 
 namespace base {
@@ -89,7 +90,15 @@ bool PickleIterator::ReadInt(int* result) {
 }
 
 bool PickleIterator::ReadLong(long* result) {
-  return ReadBuiltinType(result);
+  // Always read long as a 64-bit value to ensure compatibility between 32-bit
+  // and 64-bit processes.
+  int64_t result_int64 = 0;
+  if (!ReadBuiltinType(&result_int64))
+    return false;
+  // CHECK if the cast truncates the value so that we know to change this IPC
+  // parameter to use int64_t.
+  *result = base::checked_cast<long>(result_int64);
+  return true;
 }
 
 bool PickleIterator::ReadUInt16(uint16_t* result) {
@@ -108,16 +117,6 @@ bool PickleIterator::ReadUInt64(uint64_t* result) {
   return ReadBuiltinType(result);
 }
 
-bool PickleIterator::ReadSizeT(size_t* result) {
-  // Always read size_t as a 64-bit value to ensure compatibility between 32-bit
-  // and 64-bit processes.
-  uint64_t result_uint64 = 0;
-  bool success = ReadBuiltinType(&result_uint64);
-  *result = static_cast<size_t>(result_uint64);
-  // Fail if the cast above truncates the value.
-  return success && (*result == result_uint64);
-}
-
 bool PickleIterator::ReadFloat(float* result) {
   // crbug.com/315213
   // The source data may not be properly aligned, and unaligned float reads
diff --git a/base/pickle.h b/base/pickle.h
index 18d8afe..eb4888a 100644
--- a/base/pickle.h
+++ b/base/pickle.h
@@ -45,7 +45,6 @@ class BASE_EXPORT PickleIterator {
   bool ReadUInt32(uint32_t* result) WARN_UNUSED_RESULT;
   bool ReadInt64(int64_t* result) WARN_UNUSED_RESULT;
   bool ReadUInt64(uint64_t* result) WARN_UNUSED_RESULT;
-  bool ReadSizeT(size_t* result) WARN_UNUSED_RESULT;
   bool ReadFloat(float* result) WARN_UNUSED_RESULT;
   bool ReadDouble(double* result) WARN_UNUSED_RESULT;
   bool ReadString(std::string* result) WARN_UNUSED_RESULT;
@@ -122,12 +121,11 @@ class BASE_EXPORT PickleSizer {
 
   void AddBool() { return AddInt(); }
   void AddInt() { AddPOD<int>(); }
-  void AddLongUsingDangerousNonPortableLessPersistableForm() { AddPOD<long>(); }
+  void AddLong() { AddPOD<uint64_t>(); }
   void AddUInt16() { return AddPOD<uint16_t>(); }
   void AddUInt32() { return AddPOD<uint32_t>(); }
   void AddInt64() { return AddPOD<int64_t>(); }
   void AddUInt64() { return AddPOD<uint64_t>(); }
-  void AddSizeT() { return AddPOD<uint64_t>(); }
   void AddFloat() { return AddPOD<float>(); }
   void AddDouble() { return AddPOD<double>(); }
   void AddString(const StringPiece& value);
@@ -229,23 +227,15 @@ class BASE_EXPORT Pickle {
   bool WriteInt(int value) {
     return WritePOD(value);
   }
-  // WARNING: DO NOT USE THIS METHOD IF PICKLES ARE PERSISTED IN ANY WAY.
-  // It will write whatever a "long" is on this architecture. On 32-bit
-  // platforms, it is 32 bits. On 64-bit platforms, it is 64 bits. If persisted
-  // pickles are still around after upgrading to 64-bit, or if they are copied
-  // between dissimilar systems, YOUR PICKLES WILL HAVE GONE BAD.
-  bool WriteLongUsingDangerousNonPortableLessPersistableForm(long value) {
-    return WritePOD(value);
+  bool WriteLong(long value) {
+    // Always write long as a 64-bit value to ensure compatibility between
+    // 32-bit and 64-bit processes.
+    return WritePOD(static_cast<int64_t>(value));
   }
   bool WriteUInt16(uint16_t value) { return WritePOD(value); }
   bool WriteUInt32(uint32_t value) { return WritePOD(value); }
   bool WriteInt64(int64_t value) { return WritePOD(value); }
   bool WriteUInt64(uint64_t value) { return WritePOD(value); }
-  bool WriteSizeT(size_t value) {
-    // Always write size_t as a 64-bit value to ensure compatibility between
-    // 32-bit and 64-bit processes.
-    return WritePOD(static_cast<uint64_t>(value));
-  }
   bool WriteFloat(float value) {
     return WritePOD(value);
   }
diff --git a/base/pickle_unittest.cc b/base/pickle_unittest.cc
index 6dae177..307cb51 100644
--- a/base/pickle_unittest.cc
+++ b/base/pickle_unittest.cc
@@ -27,7 +27,6 @@ const uint16_t testuint16 = 32123;
 const uint32_t testuint32 = 1593847192;
 const int64_t testint64 = -0x7E8CA9253104BDFCLL;
 const uint64_t testuint64 = 0xCE8CA9253104BDF7ULL;
-const size_t testsizet = 0xFEDC7654;
 const float testfloat = 3.1415926935f;
 const double testdouble = 2.71828182845904523;
 const std::string teststring("Hello world");  // note non-aligned string length
@@ -73,10 +72,6 @@ void VerifyResult(const Pickle& pickle) {
   EXPECT_TRUE(iter.ReadUInt64(&outuint64));
   EXPECT_EQ(testuint64, outuint64);
 
-  size_t outsizet;
-  EXPECT_TRUE(iter.ReadSizeT(&outsizet));
-  EXPECT_EQ(testsizet, outsizet);
-
   float outfloat;
   EXPECT_TRUE(iter.ReadFloat(&outfloat));
   EXPECT_EQ(testfloat, outfloat);
@@ -119,13 +114,11 @@ TEST(PickleTest, EncodeDecode) {
   EXPECT_TRUE(pickle.WriteBool(testbool1));
   EXPECT_TRUE(pickle.WriteBool(testbool2));
   EXPECT_TRUE(pickle.WriteInt(testint));
-  EXPECT_TRUE(
-      pickle.WriteLongUsingDangerousNonPortableLessPersistableForm(testlong));
+  EXPECT_TRUE(pickle.WriteLong(testlong));
   EXPECT_TRUE(pickle.WriteUInt16(testuint16));
   EXPECT_TRUE(pickle.WriteUInt32(testuint32));
   EXPECT_TRUE(pickle.WriteInt64(testint64));
   EXPECT_TRUE(pickle.WriteUInt64(testuint64));
-  EXPECT_TRUE(pickle.WriteSizeT(testsizet));
   EXPECT_TRUE(pickle.WriteFloat(testfloat));
   EXPECT_TRUE(pickle.WriteDouble(testdouble));
   EXPECT_TRUE(pickle.WriteString(teststring));
@@ -145,25 +138,26 @@ TEST(PickleTest, EncodeDecode) {
   VerifyResult(pickle3);
 }
 
-// Tests that reading/writing a size_t works correctly when the source process
+// Tests that reading/writing a long works correctly when the source process
 // is 64-bit.  We rely on having both 32- and 64-bit trybots to validate both
 // arms of the conditional in this test.
-TEST(PickleTest, SizeTFrom64Bit) {
+TEST(PickleTest, LongFrom64Bit) {
   Pickle pickle;
-  // Under the hood size_t is always written as a 64-bit value, so simulate a
-  // 64-bit size_t even on 32-bit architectures by explicitly writing a
-  // uint64_t.
-  EXPECT_TRUE(pickle.WriteUInt64(testuint64));
+  // Under the hood long is always written as a 64-bit value, so simulate a
+  // 64-bit long even on 32-bit architectures by explicitly writing an int64_t.
+  EXPECT_TRUE(pickle.WriteInt64(testint64));
 
   PickleIterator iter(pickle);
-  size_t outsizet;
-  if (sizeof(size_t) < sizeof(uint64_t)) {
-    // ReadSizeT() should return false when the original written value can't be
-    // represented as a size_t.
-    EXPECT_FALSE(iter.ReadSizeT(&outsizet));
+  long outlong;
+  if (sizeof(long) < sizeof(int64_t)) {
+    // ReadLong() should return false when the original written value can't be
+    // represented as a long.
+#if GTEST_HAS_DEATH_TEST
+    EXPECT_DEATH(ignore_result(iter.ReadLong(&outlong)), "");
+#endif
   } else {
-    EXPECT_TRUE(iter.ReadSizeT(&outsizet));
-    EXPECT_EQ(testuint64, outsizet);
+    EXPECT_TRUE(iter.ReadLong(&outlong));
+    EXPECT_EQ(testint64, outlong);
   }
 }
 
@@ -556,14 +550,14 @@ TEST(PickleTest, ClaimBytes) {
   std::string data("Hello, world!");
 
   TestingPickle pickle;
-  pickle.WriteSizeT(data.size());
+  pickle.WriteUInt32(data.size());
   void* bytes = pickle.ClaimBytes(data.size());
   pickle.WriteInt(42);
   memcpy(bytes, data.data(), data.size());
 
   PickleIterator iter(pickle);
-  size_t out_data_length;
-  EXPECT_TRUE(iter.ReadSizeT(&out_data_length));
+  uint32_t out_data_length;
+  EXPECT_TRUE(iter.ReadUInt32(&out_data_length));
   EXPECT_EQ(data.size(), out_data_length);
 
   const char* out_data = nullptr;
@@ -594,8 +588,8 @@ TEST(PickleTest, PickleSizer) {
   {
     TestingPickle pickle;
     base::PickleSizer sizer;
-    pickle.WriteLongUsingDangerousNonPortableLessPersistableForm(42);
-    sizer.AddLongUsingDangerousNonPortableLessPersistableForm();
+    pickle.WriteLong(42);
+    sizer.AddLong();
     EXPECT_EQ(sizer.payload_size(), pickle.payload_size());
   }
   {
@@ -629,13 +623,6 @@ TEST(PickleTest, PickleSizer) {
   {
     TestingPickle pickle;
     base::PickleSizer sizer;
-    pickle.WriteSizeT(42);
-    sizer.AddSizeT();
-    EXPECT_EQ(sizer.payload_size(), pickle.payload_size());
-  }
-  {
-    TestingPickle pickle;
-    base::PickleSizer sizer;
     pickle.WriteFloat(42.0f);
     sizer.AddFloat();
     EXPECT_EQ(sizer.payload_size(), pickle.payload_size());