1. Enable large object pointer offset check in release build.

Following code will now cause a check error:
char* p = reinterpret_cast<char*>(malloc(kMaxSize + 1));
free(p + 1);

2. Remove a duplicated error reporting function "DieFromBadFreePointer", can
use "InvalidGetAllocatedSize".

Review URL: https://chromiumcodereview.appspot.com/10391178

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

5 files changed, 124 insertions, 9 deletions

diff --git a/base/allocator/allocator.gyp b/base/allocator/allocator.gyp
index bfd9ec0..41bb96b 100644
--- a/base/allocator/allocator.gyp
+++ b/base/allocator/allocator.gyp
@@ -201,7 +201,7 @@
         'allocator_shim.cc',
         'allocator_shim.h',
         'generic_allocators.cc',
-        'win_allocator.cc',        
+        'win_allocator.cc',
       ],
       # sources! means that these are not compiled directly.
       'sources!': [
@@ -433,9 +433,9 @@
       ],
     },
     {
-      # This library is linked in to libbase and allocator_unittests.
-      # It can't depend on either and nothing else should depend on it -
-      # all other code should use the interfaced provided by libbase.
+      # This library is linked in to src/base.gypi:base and allocator_unittests
+      # It can't depend on either and nothing else should depend on it - all
+      # other code should use the interfaced provided by base.
       'target_name': 'allocator_extension_thunks',
       'type': 'static_library',
       'sources': [
@@ -510,6 +510,23 @@
             },
           },
         },
+      {
+        'target_name': 'tcmalloc_unittest',
+        'type': 'executable',
+        'sources': [
+          'tcmalloc_unittest.cc',
+        ],
+        'include_dirs': [
+          '../..',
+          # For constants of TCMalloc.
+          '<(tcmalloc_dir)/src',
+        ],
+        'dependencies': [
+          '../../testing/gtest.gyp:gtest',
+          '../base.gyp:base',
+          'allocator',
+        ],
+      },
       ],
     }],
   ],
diff --git a/base/allocator/allocator_extension_thunks.cc b/base/allocator/allocator_extension_thunks.cc
index f8985a2..0e97f6a 100644
--- a/base/allocator/allocator_extension_thunks.cc
+++ b/base/allocator/allocator_extension_thunks.cc
@@ -11,11 +11,11 @@ namespace allocator {
 namespace thunks {
 
 // This slightly odd translation unit exists because of the peculularity of how
-// allocator_unittests works on windows.  That target has to perform
+// allocator_unittests work on windows.  That target has to perform
 // tcmalloc-specific initialization on windows, but it cannot depend on base
-// otherwise.  This target sits in the middle - both libbase and
-// allocator_unittests can depend on it.
-// This file can't depend on anything else in base, including logging.
+// otherwise. This target sits in the middle - base and allocator_unittests
+// can depend on it. This file can't depend on anything else in base, including
+// logging.
 
 static GetStatsFunction* g_get_stats_function = NULL;
 static ReleaseFreeMemoryFunction* g_release_free_memory_function = NULL;
diff --git a/base/allocator/allocator_shim.cc b/base/allocator/allocator_shim.cc
index 8728097..fd6d741 100644
--- a/base/allocator/allocator_shim.cc
+++ b/base/allocator/allocator_shim.cc
@@ -328,5 +328,17 @@ void SetupSubprocessAllocator() {
 #endif  // ENABLE_DYNAMIC_ALLOCATOR_SWITCHING
 }
 
+void* TCMallocDoMallocForTest(size_t size) {
+  return do_malloc(size);
+}
+
+void TCMallocDoFreeForTest(void* ptr) {
+  do_free(ptr);
+}
+
+size_t ExcludeSpaceForMarkForTest(size_t size) {
+  return ExcludeSpaceForMark(size);
+}
+
 }  // namespace allocator.
 }  // namespace base.
diff --git a/base/allocator/allocator_shim.h b/base/allocator/allocator_shim.h
index b16f6ce..e10fa8d 100644
--- a/base/allocator/allocator_shim.h
+++ b/base/allocator/allocator_shim.h
@@ -1,4 +1,4 @@
-// Copyright (c) 2011 The Chromium Authors. All rights reserved.
+// Copyright (c) 2012 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.
 
@@ -14,6 +14,11 @@ namespace allocator {
 // then a default value (typically set to TCMALLOC).
 void SetupSubprocessAllocator();
 
+// Expose some of tcmalloc functions for test.
+void* TCMallocDoMallocForTest(size_t size);
+void TCMallocDoFreeForTest(void* ptr);
+size_t ExcludeSpaceForMarkForTest(size_t size);
+
 }  // namespace allocator.
 }  // namespace base.
 
diff --git a/base/allocator/tcmalloc_unittest.cc b/base/allocator/tcmalloc_unittest.cc
new file mode 100644
index 0000000..053a9d5
--- /dev/null
+++ b/base/allocator/tcmalloc_unittest.cc
@@ -0,0 +1,81 @@
+// Copyright (c) 2012 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 <stdio.h>
+#include "base/allocator/allocator_shim.h"
+
+#include "testing/gtest/include/gtest/gtest.h"
+
+// TCMalloc header files
+#include "common.h"  // For TCMalloc constants like page size, etc.
+
+using base::allocator::TCMallocDoMallocForTest;
+using base::allocator::TCMallocDoFreeForTest;
+using base::allocator::ExcludeSpaceForMarkForTest;
+
+TEST(TCMallocFreeCheck, BadPointerInFirstPageOfTheLargeObject) {
+  char* p = reinterpret_cast<char*>(
+      TCMallocDoMallocForTest(ExcludeSpaceForMarkForTest(kMaxSize + 1)));
+  for (int offset = 1; offset < kPageSize ; offset <<= 1) {
+    ASSERT_DEATH(TCMallocDoFreeForTest(p + offset),
+                 "Pointer is not pointing to the start of a span");
+  }
+}
+
+TEST(TCMallocFreeCheck, BadPageAlignedPointerInsideLargeObject) {
+  char* p = reinterpret_cast<char*>(
+      TCMallocDoMallocForTest(ExcludeSpaceForMarkForTest(kMaxSize + 1)));
+
+  for (int offset = kPageSize; offset < kMaxSize; offset += kPageSize) {
+    // Only the first and last page of a span are in heap map. So for others
+    // tcmalloc will give a general error of invalid pointer.
+    ASSERT_DEATH(TCMallocDoFreeForTest(p + offset),
+                 "Attempt to free invalid pointer");
+  }
+  ASSERT_DEATH(TCMallocDoFreeForTest(p + kMaxSize),
+               "Pointer is not pointing to the start of a span");
+}
+
+TEST(TCMallocFreeCheck, DoubleFreeLargeObject) {
+  char* p = reinterpret_cast<char*>(
+      TCMallocDoMallocForTest(ExcludeSpaceForMarkForTest(kMaxSize + 1)));
+  ASSERT_DEATH(TCMallocDoFreeForTest(p); TCMallocDoFreeForTest(p),
+               "Object was not in-use");
+}
+
+
+#ifdef NDEBUG
+TEST(TCMallocFreeCheck, DoubleFreeSmallObject) {
+  for (size_t size = 1;
+       size <= ExcludeSpaceForMarkForTest(kMaxSize);
+       size <<= 1) {
+    char* p = reinterpret_cast<char*>(TCMallocDoMallocForTest(size));
+    ASSERT_DEATH(TCMallocDoFreeForTest(p); TCMallocDoFreeForTest(p),
+                 "Circular loop in list detected");
+  }
+}
+#else
+TEST(TCMallocFreeCheck, DoubleFreeSmallObject) {
+  size_t size = 1;
+
+  // When the object is small, tcmalloc validation can not distinguish normal
+  // memory corruption or double free, because there's not enough space in
+  // freed objects to keep the mark.
+  for (; size <= ExcludeSpaceForMarkForTest(kMinClassSize); size <<= 1) {
+    char* p = reinterpret_cast<char*>(TCMallocDoMallocForTest(size));
+    ASSERT_DEATH(TCMallocDoFreeForTest(p); TCMallocDoFreeForTest(p),
+                 "Memory corrupted");
+  }
+
+  for (; size <= ExcludeSpaceForMarkForTest(kMaxSize); size <<= 1) {
+    char* p = reinterpret_cast<char*>(TCMallocDoMallocForTest(size));
+    ASSERT_DEATH(TCMallocDoFreeForTest(p); TCMallocDoFreeForTest(p),
+                 "Attempt to double free");
+  }
+}
+#endif
+
+int main(int argc, char **argv) {
+  testing::InitGoogleTest(&argc, argv);
+  return RUN_ALL_TESTS();
+}