Fix problem with persistent histograms getting ID of zero.

base/metrics/persistent_histogram_allocator.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_histogram_allocator.h"
 
 #include <memory>
 
 #include "base/files/file_path.h"
 #include "base/files/file_util.h"
@@ skipping 20 matching lines @@
 
 // Type identifiers used when storing in persistent memory so they can be
 // identified during extraction; the first 4 bytes of the SHA1 of the name
 // is used as a unique integer. A "version number" is added to the base
 // so that, if the structure of that object changes, stored older versions
 // will be safely ignored.
 enum : uint32_t {
   kTypeIdHistogram   = 0xF1645910 + 2,  // SHA1(Histogram)   v2
   kTypeIdRangesArray = 0xBCEA225A + 1,  // SHA1(RangesArray) v1
   kTypeIdCountsArray = 0x53215530 + 1,  // SHA1(CountsArray) v1
+
+  kTypeIdHistogramUnderConstruction = ~kTypeIdHistogram,
 };
 
 // The current globally-active persistent allocator for all new histograms.
 // The object held here will obviously not be destructed at process exit
 // but that's best since PersistentMemoryAllocator objects (that underlie
 // GlobalHistogramAllocator objects) are explicitly forbidden from doing
 // anything essential at exit anyway due to the fact that they depend on data
 // managed elsewhere and which could be destructed first.
 GlobalHistogramAllocator* g_allocator = nullptr;
 
@@ skipping 216 matching lines @@
     Reference ref) {
   // Unfortunately, the histogram "pickle" methods cannot be used as part of
   // the persistance because the deserialization methods always create local
   // count data (while these must reference the persistent counts) and always
   // add it to the local list of known histograms (while these may be simple
   // references to histograms in other processes).
   PersistentHistogramData* histogram_data =
       memory_allocator_->GetAsObject<PersistentHistogramData>(
           ref, kTypeIdHistogram);
   size_t length = memory_allocator_->GetAllocSize(ref);
+
+  // Check that metadata is reasonable: name is NUL terminated and non-empty,
+  // ID fields have been loaded with a hash of the name (0 is considered
+  // unset/invalid).
   if (!histogram_data ||
-      reinterpret_cast<char*>(histogram_data)[length - 1] != '\0') {
+      reinterpret_cast<char*>(histogram_data)[length - 1] != '\0' ||
+      histogram_data->name[0] == '\0' ||
+      histogram_data->samples_metadata.id == 0 ||
+      histogram_data->logged_metadata.id == 0) {
     RecordCreateHistogramResult(CREATE_HISTOGRAM_INVALID_METADATA);
     NOTREACHED();
     return nullptr;
   }
   return CreateHistogram(histogram_data);
 }
 
 std::unique_ptr<HistogramBase> PersistentHistogramAllocator::AllocateHistogram(
     HistogramType histogram_type,
     const std::string& name,
     int minimum,
     int maximum,
     const BucketRanges* bucket_ranges,
     int32_t flags,
     Reference* ref_ptr) {
   // If the allocator is corrupt, don't waste time trying anything else.
   // This also allows differentiating on the dashboard between allocations
   // failed due to a corrupt allocator and the number of process instances
   // with one, the latter being idicated by "newly corrupt", below.
   if (memory_allocator_->IsCorrupt()) {
     RecordCreateHistogramResult(CREATE_HISTOGRAM_ALLOCATOR_CORRUPT);
     return nullptr;
   }
 
   // Create the metadata necessary for a persistent sparse histogram. This
   // is done first because it is a small subset of what is required for
-  // other histograms.
+  // other histograms. The type is "under construction" so that a crash
+  // during the datafill doesn't leave a bad record around that could cause
+  // confusion by another process trying to read it. It will be corrected
+  // once histogram construction is complete.
   PersistentMemoryAllocator::Reference histogram_ref =
       memory_allocator_->Allocate(
           offsetof(PersistentHistogramData, name) + name.length() + 1,
-          kTypeIdHistogram);
+          kTypeIdHistogramUnderConstruction);
   PersistentHistogramData* histogram_data =
-      memory_allocator_->GetAsObject<PersistentHistogramData>(histogram_ref,
-                                                              kTypeIdHistogram);
+      memory_allocator_->GetAsObject<PersistentHistogramData>(
+          histogram_ref, kTypeIdHistogramUnderConstruction);
   if (histogram_data) {
     memcpy(histogram_data->name, name.c_str(), name.size() + 1);
     histogram_data->histogram_type = histogram_type;
     histogram_data->flags = flags | HistogramBase::kIsPersistent;
   }
 
   // Create the remaining metadata necessary for regular histograms.
   if (histogram_type != SPARSE_HISTOGRAM) {
     size_t bucket_count = bucket_ranges->bucket_count();
     size_t counts_bytes = CalculateRequiredCountsBytes(bucket_count);
@@ skipping 35 matching lines @@
   }
 
   if (histogram_data) {
     // Create the histogram using resources in persistent memory. This ends up
     // resolving the "ref" values stored in histogram_data instad of just
     // using what is already known above but avoids duplicating the switch
     // statement here and serves as a double-check that everything is
     // correct before commiting the new histogram to persistent space.
     std::unique_ptr<HistogramBase> histogram = CreateHistogram(histogram_data);
     DCHECK(histogram);
+    DCHECK_NE(0U, histogram_data->samples_metadata.id);
+    DCHECK_NE(0U, histogram_data->logged_metadata.id);
+    memory_allocator_->ChangeType(histogram_ref, kTypeIdHistogram,
+                                  kTypeIdHistogramUnderConstruction);
+
     if (ref_ptr != nullptr)
       *ref_ptr = histogram_ref;
 
     // By storing the reference within the allocator to this histogram, the
     // next import (which will happen before the next histogram creation)
     // will know to skip it.
     // See also the comment in ImportHistogramsToStatisticsRecorder().
     subtle::NoBarrier_Store(&last_created_, histogram_ref);
     return histogram;
   }
@@ skipping 536 matching lines @@
   while (true) {
     std::unique_ptr<HistogramBase> histogram =
         import_iterator_.GetNextWithIgnore(record_to_ignore);
     if (!histogram)
       break;
     StatisticsRecorder::RegisterOrDeleteDuplicate(histogram.release());
   }
 }
 
 }  // namespace base