Fix overflow when logging MaxInt32 to a sparse histogram.

base/metrics/persistent_sample_map.cc

 // Copyright 2016 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "base/metrics/persistent_sample_map.h"
 
 #include "base/logging.h"
 #include "base/memory/ptr_util.h"
 #include "base/metrics/histogram_macros.h"
 #include "base/metrics/persistent_histogram_allocator.h"
@@ skipping 23 matching lines @@
   typedef std::map<HistogramBase::Sample, HistogramBase::Count*>
       SampleToCountMap;
 
   explicit PersistentSampleMapIterator(const SampleToCountMap& sample_counts);
   ~PersistentSampleMapIterator() override;
 
   // SampleCountIterator:
   bool Done() const override;
   void Next() override;
   void Get(HistogramBase::Sample* min,
-           HistogramBase::Sample* max,
+           int64_t* max,
            HistogramBase::Count* count) const override;
 
  private:
   void SkipEmptyBuckets();
 
   SampleToCountMap::const_iterator iter_;
   const SampleToCountMap::const_iterator end_;
 };
 
 PersistentSampleMapIterator::PersistentSampleMapIterator(
     const SampleToCountMap& sample_counts)
     : iter_(sample_counts.begin()),
       end_(sample_counts.end()) {
   SkipEmptyBuckets();
 }
 
 PersistentSampleMapIterator::~PersistentSampleMapIterator() {}
 
 bool PersistentSampleMapIterator::Done() const {
   return iter_ == end_;
 }
 
 void PersistentSampleMapIterator::Next() {
   DCHECK(!Done());
   ++iter_;
   SkipEmptyBuckets();
 }
 
 void PersistentSampleMapIterator::Get(Sample* min,
-                                      Sample* max,
+                                      int64_t* max,
                                       Count* count) const {
   DCHECK(!Done());
   if (min)
     *min = iter_->first;
   if (max)
-    *max = iter_->first + 1;
+    *max = static_cast<int64_t>(iter_->first) + 1;

[Ilya Sherman, 2017/05/02 21:37:49]

nit: Here and everywhere else that you add a static_cast, could you please use a
strict_cast from base/numerics/safe_conversions.h instead?

[Alexei Svitkine (slow), 2017/05/03 15:21:53]

Done.

   if (count)
     *count = *iter_->second;
 }
 
 void PersistentSampleMapIterator::SkipEmptyBuckets() {
   while (!Done() && *iter_->second == 0) {
     ++iter_;
   }
 }
 
@@ skipping 88 matching lines @@
   record->count = 0;
 
   PersistentMemoryAllocator::Reference ref = allocator->GetAsReference(record);
   allocator->MakeIterable(ref);
   return ref;
 }
 
 bool PersistentSampleMap::AddSubtractImpl(SampleCountIterator* iter,
                                           Operator op) {
   Sample min;
-  Sample max;
+  int64_t max;
   Count count;
   for (; !iter->Done(); iter->Next()) {
     iter->Get(&min, &max, &count);
     if (count == 0)
       continue;
-    if (min + 1 != max)
+    if (static_cast<int64_t>(min) + 1 != max)
       return false;  // SparseHistogram only supports bucket with size 1.
 
 #if 0  // TODO(bcwhite) Re-enable efficient version after crbug.com/682680.
     *GetOrCreateSampleCountStorage(min) +=
         (op == HistogramSamples::ADD) ? count : -count;
 #else
     NegativeSampleReason reason = MAX_NEGATIVE_SAMPLE_REASONS;
     if (op == HistogramSamples::ADD) {
       // Add should generally be adding only positive values.
       Count* local_count_ptr = GetOrCreateSampleCountStorage(min);
@@ skipping 121 matching lines @@
         found_count = &record->count;
       if (!import_everything)
         break;
     }
   }
 
   return found_count;
 }
 
 }  // namespace base