Introduce the ANNOTATE_LEAKING_OBJECT_PTR annotation that can be used to mark

heap allocated objects as intentionally leaked ones.
Annotate the histograms produced by {Histogram,BooleanHistogram,LinearHistogram,CustomHistogram}::FactoryGet(), as leaked.

Rename StatsHistogram::StatsHistogramFactoryGet to StatsHistogram::FactoryGet, annotate the result as leaky, update the suppressions.

BUG=79322
Review URL: http://codereview.chromium.org/7071036

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@87278 0039d316-1c4b-4281-b951-d872f2087c98

8 files changed, 34 insertions, 10 deletions

diff --git a/base/allocator/allocator.gyp b/base/allocator/allocator.gyp
index 4e613a9..ce1e04e 100644
--- a/base/allocator/allocator.gyp
+++ b/base/allocator/allocator.gyp
@@ -338,6 +338,7 @@
               # Do the same for heap leak checker.
               '-Wl,-u_Z21InitialMallocHook_NewPKvj,-u_Z22InitialMallocHook_MMapPKvS0_jiiix,-u_Z22InitialMallocHook_SbrkPKvi',
               '-Wl,-u_Z21InitialMallocHook_NewPKvm,-u_Z22InitialMallocHook_MMapPKvS0_miiil,-u_Z22InitialMallocHook_SbrkPKvl',
+              '-Wl,-u_ZN15HeapLeakChecker12IgnoreObjectEPKv,-u_ZN15HeapLeakChecker14UnIgnoreObjectEPKv',
           ]},
         }],
         [ 'linux_use_debugallocation==1', {
diff --git a/base/debug/leak_annotations.h b/base/debug/leak_annotations.h
index e1086fe..cb03320 100644
--- a/base/debug/leak_annotations.h
+++ b/base/debug/leak_annotations.h
@@ -1,4 +1,4 @@
-// Copyright (c) 2010 The Chromium Authors. All rights reserved.
+// Copyright (c) 2011 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.
 
@@ -18,10 +18,22 @@
 #define ANNOTATE_SCOPED_MEMORY_LEAK \
     HeapLeakChecker::Disabler heap_leak_checker_disabler
 
+// Annotate an object pointer as referencing a leaky object. This object and all
+// the heap objects referenced by it will be ignored by the heap checker.
+//
+// X should be referencing an active allocated object. If it is not, the
+// annotation will be ignored.
+// No object should be annotated with ANNOTATE_SCOPED_MEMORY_LEAK twice.
+// Once an object is annotated with ANNOTATE_SCOPED_MEMORY_LEAK, it cannot be
+// deleted.
+#define ANNOTATE_LEAKING_OBJECT_PTR(X) \
+    HeapLeakChecker::IgnoreObject(X)
+
 #else
 
 // If tcmalloc is not used, the annotations should be no-ops.
-#define ANNOTATE_SCOPED_MEMORY_LEAK
+#define ANNOTATE_SCOPED_MEMORY_LEAK ((void)0)
+#define ANNOTATE_LEAKING_OBJECT_PTR(X) ((void)0)
 
 #endif
 
diff --git a/base/metrics/histogram.cc b/base/metrics/histogram.cc
index 63e5ae6..2441c51 100644
--- a/base/metrics/histogram.cc
+++ b/base/metrics/histogram.cc
@@ -14,6 +14,7 @@
 #include <algorithm>
 #include <string>
 
+#include "base/debug/leak_annotations.h"
 #include "base/logging.h"
 #include "base/pickle.h"
 #include "base/stringprintf.h"
@@ -1030,17 +1031,27 @@ bool StatisticsRecorder::IsActive() {
 }
 
 Histogram* StatisticsRecorder::RegisterOrDeleteDuplicate(Histogram* histogram) {
+  // As per crbug.com/79322 the histograms are intentionally leaked, so we need
+  // to annotate them. Because ANNOTATE_LEAKING_OBJECT_PTR may be used only once
+  // for an object, the duplicates should not be annotated.
+  // Callers are responsible for not calling RegisterOrDeleteDuplicate(ptr)
+  // twice if (lock_ == NULL) || (!histograms_).
   DCHECK(histogram->HasValidRangeChecksum());
-  if (lock_ == NULL)
+  if (lock_ == NULL) {
+    ANNOTATE_LEAKING_OBJECT_PTR(histogram);  // see crbug.com/79322
     return histogram;
+  }
   base::AutoLock auto_lock(*lock_);
-  if (!histograms_)
+  if (!histograms_) {
+    ANNOTATE_LEAKING_OBJECT_PTR(histogram);  // see crbug.com/79322
     return histogram;
+  }
   const std::string name = histogram->histogram_name();
   HistogramMap::iterator it = histograms_->find(name);
   // Avoid overwriting a previous registration.
   if (histograms_->end() == it) {
     (*histograms_)[name] = histogram;
+    ANNOTATE_LEAKING_OBJECT_PTR(histogram);  // see crbug.com/79322
   } else {
     delete histogram;  // We already have one by this name.
     histogram = it->second;
diff --git a/net/disk_cache/stats.cc b/net/disk_cache/stats.cc
index 6fdd31c..af3b941 100644
--- a/net/disk_cache/stats.cc
+++ b/net/disk_cache/stats.cc
@@ -151,7 +151,7 @@ bool Stats::Init(BackendImpl* backend, uint32* storage_addr) {
       // Stats may be reused when the cache is re-created, but we want only one
       // histogram at any given time.
       size_histogram_ =
-          StatsHistogram::StatsHistogramFactoryGet("DiskCache.SizeStats");
+          StatsHistogram::FactoryGet("DiskCache.SizeStats");
       size_histogram_->Init(this);
     }
   }
diff --git a/net/disk_cache/stats_histogram.cc b/net/disk_cache/stats_histogram.cc
index 6d3097a..943990a3 100644
--- a/net/disk_cache/stats_histogram.cc
+++ b/net/disk_cache/stats_histogram.cc
@@ -4,6 +4,7 @@
 
 #include "net/disk_cache/stats_histogram.h"
 
+#include "base/debug/leak_annotations.h"
 #include "base/logging.h"
 #include "net/disk_cache/stats.h"
 
@@ -21,8 +22,7 @@ StatsHistogram::~StatsHistogram() {
     stats_ = NULL;
 }
 
-StatsHistogram* StatsHistogram::StatsHistogramFactoryGet(
-    const std::string& name) {
+StatsHistogram* StatsHistogram::FactoryGet(const std::string& name) {
   Histogram* histogram(NULL);
 
   Sample minimum = 1;
diff --git a/net/disk_cache/stats_histogram.h b/net/disk_cache/stats_histogram.h
index 7f513ce..f41c384 100644
--- a/net/disk_cache/stats_histogram.h
+++ b/net/disk_cache/stats_histogram.h
@@ -35,7 +35,7 @@ class StatsHistogram : public base::Histogram {
       : Histogram(name, minimum, maximum, bucket_count), init_(false) {}
   virtual ~StatsHistogram();
 
-  static StatsHistogram* StatsHistogramFactoryGet(const std::string& name);
+  static StatsHistogram* FactoryGet(const std::string& name);
 
   // We'll be reporting data from the given set of cache stats.
   bool Init(const Stats* stats);
diff --git a/tools/heapcheck/suppressions.txt b/tools/heapcheck/suppressions.txt
index 3e1d282..916eb97 100644
--- a/tools/heapcheck/suppressions.txt
+++ b/tools/heapcheck/suppressions.txt
@@ -148,7 +148,7 @@
    Intentional leak of stats histogram to avoid shutdown races
    Heapcheck:Leak
    ...
-   fun:disk_cache::StatsHistogram::StatsHistogramFactoryGet
+   fun:disk_cache::StatsHistogram::FactoryGet
 }
 {
    String name pushed into deliberately leaked histograms
diff --git a/tools/valgrind/memcheck/suppressions.txt b/tools/valgrind/memcheck/suppressions.txt
index e6b5de3..d0d36a6 100644
--- a/tools/valgrind/memcheck/suppressions.txt
+++ b/tools/valgrind/memcheck/suppressions.txt
@@ -873,7 +873,7 @@
    Histograms via FactoryGet including Stats for disk_cache
    Memcheck:Leak
    fun:_Znw*
-   fun:_ZN10disk_cache14StatsHistogram24StatsHistogramFactoryGet*
+   fun:_ZN10disk_cache14StatsHistogram10FactoryGet*
 }
 
 #-----------------------------------------------------------------------