Commit 61b84b28 authored by Brian White's avatar Brian White Committed by Commit Bot

Validate checksum calculations.

Crash reports indicate that checksums are valid even though the bucket
boundary values are tested to be incorrect. Try to determine if it is
memory corruption or bad calculations.

Bug: 836238
Change-Id: Ie89df5fef5093e9dd8c45fe9ff26a0430838769e
Reviewed-on: https://chromium-review.googlesource.com/c/1264036Reviewed-by: default avatarAlexei Svitkine <asvitkine@chromium.org>
Commit-Queue: Brian White <bcwhite@chromium.org>
Cr-Commit-Position: refs/heads/master@{#597194}
parent 90f3c584
...@@ -74,7 +74,8 @@ const uint32_t kCrcTable[256] = { ...@@ -74,7 +74,8 @@ const uint32_t kCrcTable[256] = {
// the CRC correct for big-endian vs little-ending calculations. All we need is // the CRC correct for big-endian vs little-ending calculations. All we need is
// a nice hash, that tends to depend on all the bits of the sample, with very // a nice hash, that tends to depend on all the bits of the sample, with very
// little chance of changes in one place impacting changes in another place. // little chance of changes in one place impacting changes in another place.
static uint32_t Crc32(uint32_t sum, HistogramBase::Sample value) { // Temporary non-static for https://crbug.com/836238
/*static*/ uint32_t Crc32(uint32_t sum, HistogramBase::Sample value) {
union { union {
HistogramBase::Sample range; HistogramBase::Sample range;
unsigned char bytes[sizeof(HistogramBase::Sample)]; unsigned char bytes[sizeof(HistogramBase::Sample)];
......
...@@ -99,6 +99,7 @@ class BASE_EXPORT BucketRanges { ...@@ -99,6 +99,7 @@ class BASE_EXPORT BucketRanges {
////////////////////////////////////////////////////////////////////////////// //////////////////////////////////////////////////////////////////////////////
// Expose only for test. // Expose only for test.
BASE_EXPORT extern const uint32_t kCrcTable[256]; BASE_EXPORT extern const uint32_t kCrcTable[256];
uint32_t Crc32(uint32_t sum, HistogramBase::Sample value);
} // namespace base } // namespace base
......
...@@ -465,6 +465,16 @@ void Histogram::InitializeBucketRanges(Sample minimum, ...@@ -465,6 +465,16 @@ void Histogram::InitializeBucketRanges(Sample minimum,
Sample current = minimum; Sample current = minimum;
ranges->set_range(bucket_index, current); ranges->set_range(bucket_index, current);
size_t bucket_count = ranges->bucket_count(); size_t bucket_count = ranges->bucket_count();
// Temporary for https://crbug.com/836238
uint32_t checksum = static_cast<uint32_t>(bucket_count + 1);
checksum = Crc32(checksum, 0);
checksum = Crc32(checksum, current);
debug::Alias(&minimum);
debug::Alias(&maximum);
debug::Alias(&bucket_count);
debug::Alias(&checksum);
while (bucket_count > ++bucket_index) { while (bucket_count > ++bucket_index) {
double log_current; double log_current;
log_current = log(static_cast<double>(current)); log_current = log(static_cast<double>(current));
...@@ -479,9 +489,12 @@ void Histogram::InitializeBucketRanges(Sample minimum, ...@@ -479,9 +489,12 @@ void Histogram::InitializeBucketRanges(Sample minimum,
else else
++current; // Just do a narrow bucket, and keep trying. ++current; // Just do a narrow bucket, and keep trying.
ranges->set_range(bucket_index, current); ranges->set_range(bucket_index, current);
checksum = Crc32(checksum, current);
} }
ranges->set_range(ranges->bucket_count(), HistogramBase::kSampleType_MAX); ranges->set_range(ranges->bucket_count(), HistogramBase::kSampleType_MAX);
ranges->ResetChecksum(); ranges->ResetChecksum();
checksum = Crc32(checksum, HistogramBase::kSampleType_MAX);
CHECK_EQ(checksum, ranges->checksum());
} }
// static // static
...@@ -1094,7 +1107,7 @@ double LinearHistogram::GetBucketSize(Count current, uint32_t i) const { ...@@ -1094,7 +1107,7 @@ double LinearHistogram::GetBucketSize(Count current, uint32_t i) const {
const std::string LinearHistogram::GetAsciiBucketRange(uint32_t i) const { const std::string LinearHistogram::GetAsciiBucketRange(uint32_t i) const {
int range = ranges(i); int range = ranges(i);
auto it = bucket_description_.find(range); BucketDescriptionMap::const_iterator it = bucket_description_.find(range);
if (it == bucket_description_.end()) if (it == bucket_description_.end())
return Histogram::GetAsciiBucketRange(i); return Histogram::GetAsciiBucketRange(i);
return it->second; return it->second;
...@@ -1111,13 +1124,33 @@ void LinearHistogram::InitializeBucketRanges(Sample minimum, ...@@ -1111,13 +1124,33 @@ void LinearHistogram::InitializeBucketRanges(Sample minimum,
double min = minimum; double min = minimum;
double max = maximum; double max = maximum;
size_t bucket_count = ranges->bucket_count(); size_t bucket_count = ranges->bucket_count();
// Temporary for https://crbug.com/836238
bool is_enum = (minimum == 1 &&
static_cast<Sample>(bucket_count) == maximum - minimum + 2);
uint32_t checksum = static_cast<uint32_t>(bucket_count + 1);
checksum = Crc32(checksum, 0);
debug::Alias(&minimum);
debug::Alias(&maximum);
debug::Alias(&min);
debug::Alias(&max);
debug::Alias(&bucket_count);
debug::Alias(&checksum);
debug::Alias(&is_enum);
for (size_t i = 1; i < bucket_count; ++i) { for (size_t i = 1; i < bucket_count; ++i) {
double linear_range = double linear_range =
(min * (bucket_count - 1 - i) + max * (i - 1)) / (bucket_count - 2); (min * (bucket_count - 1 - i) + max * (i - 1)) / (bucket_count - 2);
ranges->set_range(i, static_cast<Sample>(linear_range + 0.5)); uint32_t range = static_cast<Sample>(linear_range + 0.5);
if (is_enum)
CHECK_EQ(static_cast<uint32_t>(i), range);
ranges->set_range(i, range);
checksum = Crc32(checksum, range);
} }
ranges->set_range(ranges->bucket_count(), HistogramBase::kSampleType_MAX); ranges->set_range(ranges->bucket_count(), HistogramBase::kSampleType_MAX);
ranges->ResetChecksum(); ranges->ResetChecksum();
checksum = Crc32(checksum, HistogramBase::kSampleType_MAX);
CHECK_EQ(checksum, ranges->checksum());
} }
// static // static
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment