0

Add Read/WriteSizeT() functions to the pickle layer, plus one consumer.

This eliminates the need for callers to do explicit conversions, and also
ensures callers don't try to implement pickling of a size_t using a 32-bit type,
leading to truncation on 64-bit targets.  The pickle layer will ensure 64-bit
types are always used.

I'll be changing other callsites to use this in future patches.

BUG=none
TEST=none

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

Cr-Commit-Position: refs/heads/master@{#297774}
This commit is contained in:
pkasting
2014-10-01 20:01:04 -07:00
committed by Commit bot
parent 413cdda971
commit 89a19f1430
4 changed files with 106 additions and 27 deletions

@ -37,13 +37,13 @@ bool ReadHistogramArguments(PickleIterator* iter,
int* flags,
int* declared_min,
int* declared_max,
uint64* bucket_count,
size_t* bucket_count,
uint32* range_checksum) {
if (!iter->ReadString(histogram_name) ||
!iter->ReadInt(flags) ||
!iter->ReadInt(declared_min) ||
!iter->ReadInt(declared_max) ||
!iter->ReadUInt64(bucket_count) ||
!iter->ReadSizeT(bucket_count) ||
!iter->ReadUInt32(range_checksum)) {
DLOG(ERROR) << "Pickle error decoding Histogram: " << *histogram_name;
return false;
@ -297,7 +297,7 @@ bool Histogram::SerializeInfoImpl(Pickle* pickle) const {
pickle->WriteInt(flags()) &&
pickle->WriteInt(declared_min()) &&
pickle->WriteInt(declared_max()) &&
pickle->WriteUInt64(bucket_count()) &&
pickle->WriteSizeT(bucket_count()) &&
pickle->WriteUInt32(bucket_ranges()->checksum());
}
@ -347,7 +347,7 @@ HistogramBase* Histogram::DeserializeInfoImpl(PickleIterator* iter) {
int flags;
int declared_min;
int declared_max;
uint64 bucket_count;
size_t bucket_count;
uint32 range_checksum;
if (!ReadHistogramArguments(iter, &histogram_name, &flags, &declared_min,
@ -636,7 +636,7 @@ HistogramBase* LinearHistogram::DeserializeInfoImpl(PickleIterator* iter) {
int flags;
int declared_min;
int declared_max;
uint64 bucket_count;
size_t bucket_count;
uint32 range_checksum;
if (!ReadHistogramArguments(iter, &histogram_name, &flags, &declared_min,
@ -691,7 +691,7 @@ HistogramBase* BooleanHistogram::DeserializeInfoImpl(PickleIterator* iter) {
int flags;
int declared_min;
int declared_max;
uint64 bucket_count;
size_t bucket_count;
uint32 range_checksum;
if (!ReadHistogramArguments(iter, &histogram_name, &flags, &declared_min,
@ -786,7 +786,7 @@ HistogramBase* CustomHistogram::DeserializeInfoImpl(PickleIterator* iter) {
int flags;
int declared_min;
int declared_max;
uint64 bucket_count;
size_t bucket_count;
uint32 range_checksum;
if (!ReadHistogramArguments(iter, &histogram_name, &flags, &declared_min,

@ -106,6 +106,16 @@ bool PickleIterator::ReadUInt64(uint64* 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 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

@ -35,6 +35,7 @@ class BASE_EXPORT PickleIterator {
bool ReadUInt32(uint32* result) WARN_UNUSED_RESULT;
bool ReadInt64(int64* result) WARN_UNUSED_RESULT;
bool ReadUInt64(uint64* 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;
@ -172,6 +173,10 @@ class BASE_EXPORT Pickle {
uint64* result) const WARN_UNUSED_RESULT {
return iter->ReadUInt64(result);
}
bool ReadSizeT(PickleIterator* iter,
size_t* result) const WARN_UNUSED_RESULT {
return iter->ReadSizeT(result);
}
bool ReadFloat(PickleIterator* iter,
float* result) const WARN_UNUSED_RESULT {
return iter->ReadFloat(result);
@ -248,6 +253,11 @@ class BASE_EXPORT Pickle {
bool WriteUInt64(uint64 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>(value));
}
bool WriteFloat(float value) {
return WritePOD(value);
}

@ -8,6 +8,7 @@
#include "base/memory/scoped_ptr.h"
#include "base/pickle.h"
#include "base/strings/string16.h"
#include "base/strings/utf_string_conversions.h"
#include "testing/gtest/include/gtest/gtest.h"
// Remove when this file is in the base namespace.
@ -15,43 +16,61 @@ using base::string16;
namespace {
const int testint = 2093847192;
const std::string teststr("Hello world"); // note non-aligned string length
const std::wstring testwstr(L"Hello, world");
const char testdata[] = "AAA\0BBB\0";
const int testdatalen = arraysize(testdata) - 1;
const bool testbool1 = false;
const bool testbool2 = true;
const int testint = 2093847192;
const long testlong = 1093847192;
const uint16 testuint16 = 32123;
const uint32 testuint32 = 1593847192;
const int64 testint64 = -0x7E8CA9253104BDFCLL;
const uint64 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
const std::wstring testwstring(L"Hello, world");
const base::string16 teststring16(base::ASCIIToUTF16("Hello, world"));
const char testdata[] = "AAA\0BBB\0";
const int testdatalen = arraysize(testdata) - 1;
// checks that the result
void VerifyResult(const Pickle& pickle) {
PickleIterator iter(pickle);
int outint;
EXPECT_TRUE(pickle.ReadInt(&iter, &outint));
EXPECT_EQ(testint, outint);
std::string outstr;
EXPECT_TRUE(pickle.ReadString(&iter, &outstr));
EXPECT_EQ(teststr, outstr);
std::wstring outwstr;
EXPECT_TRUE(pickle.ReadWString(&iter, &outwstr));
EXPECT_EQ(testwstr, outwstr);
bool outbool;
EXPECT_TRUE(pickle.ReadBool(&iter, &outbool));
EXPECT_FALSE(outbool);
EXPECT_TRUE(pickle.ReadBool(&iter, &outbool));
EXPECT_TRUE(outbool);
int outint;
EXPECT_TRUE(pickle.ReadInt(&iter, &outint));
EXPECT_EQ(testint, outint);
long outlong;
EXPECT_TRUE(pickle.ReadLong(&iter, &outlong));
EXPECT_EQ(testlong, outlong);
uint16 outuint16;
EXPECT_TRUE(pickle.ReadUInt16(&iter, &outuint16));
EXPECT_EQ(testuint16, outuint16);
uint32 outuint32;
EXPECT_TRUE(pickle.ReadUInt32(&iter, &outuint32));
EXPECT_EQ(testuint32, outuint32);
int64 outint64;
EXPECT_TRUE(pickle.ReadInt64(&iter, &outint64));
EXPECT_EQ(testint64, outint64);
uint64 outuint64;
EXPECT_TRUE(pickle.ReadUInt64(&iter, &outuint64));
EXPECT_EQ(testuint64, outuint64);
size_t outsizet;
EXPECT_TRUE(pickle.ReadSizeT(&iter, &outsizet));
EXPECT_EQ(testsizet, outsizet);
float outfloat;
EXPECT_TRUE(pickle.ReadFloat(&iter, &outfloat));
EXPECT_EQ(testfloat, outfloat);
@ -60,6 +79,18 @@ void VerifyResult(const Pickle& pickle) {
EXPECT_TRUE(pickle.ReadDouble(&iter, &outdouble));
EXPECT_EQ(testdouble, outdouble);
std::string outstring;
EXPECT_TRUE(pickle.ReadString(&iter, &outstring));
EXPECT_EQ(teststring, outstring);
std::wstring outwstring;
EXPECT_TRUE(pickle.ReadWString(&iter, &outwstring));
EXPECT_EQ(testwstring, outwstring);
base::string16 outstring16;
EXPECT_TRUE(pickle.ReadString16(&iter, &outstring16));
EXPECT_EQ(teststring16, outstring16);
const char* outdata;
int outdatalen;
EXPECT_TRUE(pickle.ReadData(&iter, &outdata, &outdatalen));
@ -75,14 +106,21 @@ void VerifyResult(const Pickle& pickle) {
TEST(PickleTest, EncodeDecode) {
Pickle pickle;
EXPECT_TRUE(pickle.WriteInt(testint));
EXPECT_TRUE(pickle.WriteString(teststr));
EXPECT_TRUE(pickle.WriteWString(testwstr));
EXPECT_TRUE(pickle.WriteBool(testbool1));
EXPECT_TRUE(pickle.WriteBool(testbool2));
EXPECT_TRUE(pickle.WriteInt(testint));
EXPECT_TRUE(
pickle.WriteLongUsingDangerousNonPortableLessPersistableForm(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));
EXPECT_TRUE(pickle.WriteWString(testwstring));
EXPECT_TRUE(pickle.WriteString16(teststring16));
EXPECT_TRUE(pickle.WriteData(testdata, testdatalen));
VerifyResult(pickle);
@ -96,6 +134,27 @@ TEST(PickleTest, EncodeDecode) {
VerifyResult(pickle3);
}
// Tests that reading/writing a size_t 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) {
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.
EXPECT_TRUE(pickle.WriteUInt64(testuint64));
PickleIterator iter(pickle);
size_t outsizet;
if (sizeof(size_t) < sizeof(uint64)) {
// ReadSizeT() should return false when the original written value can't be
// represented as a size_t.
EXPECT_FALSE(pickle.ReadSizeT(&iter, &outsizet));
} else {
EXPECT_TRUE(pickle.ReadSizeT(&iter, &outsizet));
EXPECT_EQ(testuint64, outsizet);
}
}
// Tests that we can handle really small buffers.
TEST(PickleTest, SmallBuffer) {
scoped_ptr<char[]> buffer(new char[1]);