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

third_party/tcmalloc/chromium/src/tcmalloc.cc

 // Copyright (c) 2005, Google Inc.
 // All rights reserved.
 //
 // Redistribution and use in source and binary forms, with or without
 // modification, are permitted provided that the following conditions are
 // met:
 //
 //     * Redistributions of source code must retain the above copyright
 // notice, this list of conditions and the following disclaimer.
 //     * Redistributions in binary form must reproduce the above
@@ skipping 160 matching lines @@
 using tcmalloc::kCrashWithStats;
 using tcmalloc::Log;
 using tcmalloc::PageHeap;
 using tcmalloc::PageHeapAllocator;
 using tcmalloc::SizeMap;
 using tcmalloc::Span;
 using tcmalloc::StackTrace;
 using tcmalloc::Static;
 using tcmalloc::ThreadCache;
 
-// ---- Double free debug declarations
+// ---- Functions doing validation with an extra mark.
 static size_t ExcludeSpaceForMark(size_t size);
 static void AddRoomForMark(size_t* size);
 static void ExcludeMarkFromSize(size_t* new_size);
 static void MarkAllocatedRegion(void* ptr);
 static void ValidateAllocatedRegion(void* ptr, size_t cl);
-// ---- End Double free debug declarations
+// ---- End validation functions.
 
 DECLARE_int64(tcmalloc_sample_parameter);
 DECLARE_double(tcmalloc_release_rate);
 
 // For windows, the printf we use to report large allocs is
 // potentially dangerous: it could cause a malloc that would cause an
 // infinite loop.  So by default we set the threshold to a huge number
 // on windows, so this bad situation will never trigger.  You can
 // always set TCMALLOC_LARGE_ALLOC_REPORT_THRESHOLD manually if you
 // want this functionality.
@@ skipping 950 matching lines @@
       // pointer was allocated with some other allocator besides
       // tcmalloc.  The latter can happen if tcmalloc is linked in via
       // a dynamic library, but is not listed last on the link line.
       // In that case, libraries after it on the link line will
       // allocate with libc malloc, but free with tcmalloc's free.
       (*invalid_free_fn)(ptr);  // Decide how to handle the bad free request
       return;
     }
     cl = span->sizeclass;
     Static::pageheap()->CacheSizeClass(p, cl);
+
+    // The span is not in use! A double free?

[gpike, 2012/05/21 19:04:58]

At this point cl could be anything. Don't these checks need to be in the block
of code that is executed when you are sure you are about to call
Static::pageheap()->Delete(span)?

[kaiwang, 2012/05/21 22:21:42]

oops, sorry I accidentally removed a {} pair.

These checks should be before ValidateAllocatedRegion, so error message will be
the same in debug and release builds (while ValidateAllocatedRegion does some
extra check not available in release build).
Also moving chromium modified code together is good for later merging

+    CHECK_CONDITION_PRINT(span->location == Span::IN_USE,
+                          "Freeing a span not in use");
+
+    // Mimic debug assertion below to validate pointer of large objects.
+    CHECK_CONDITION_PRINT(span->start == p,

[gpike, 2012/05/21 19:04:58]

It seems safe to replace the last two checks with a check for span->start <<
kPageShift == ptr.

[kaiwang, 2012/05/21 22:21:42]

Done. Good idea

+                          "Pointer is not inside the first page of a span");
+    CHECK_CONDITION_PRINT(reinterpret_cast<uintptr_t>(ptr) % kPageSize == 0,
+                          "Pointer is not pointing to the start of a span");

[jar (doing other things), 2012/05/21 18:44:10]

I appreciate that the text of these messages is only here to assure that we have
a death-test with a matching log.... but I'm sad that "test code" causes bloat
in the release version (re: strings), beyond the if/else execution cost of the
CHECK.

Is there any way you can propose to not have the text appear in release
versions?  Maybe there should be yet another flavor of macro based on
OFFICIAL_BUILD (or such) that discards the text, and leaves it to us to check
stacks to determine the cause of a CHECK.  

Suggestions?

[kaiwang, 2012/05/21 22:21:42]

Actually for this specific case, the size will not increase, because
CHECK_CONDITION still have a string which is set to the condition parameter.
(You can see from above that the length is about the same..

If size of tcmalloc library (release version) is actually an issue, I think we
can make some code change to internal_logging.h and logging.h. But might be in
another CL :)

   }
-
   ValidateAllocatedRegion(ptr, cl);
 
   if (cl != 0) {
     ASSERT(!Static::pageheap()->GetDescriptor(p)->sample);
     ThreadCache* heap = GetCacheIfPresent();
     if (heap != NULL) {
       heap->Deallocate(ptr, cl);
     } else {
       // Delete directly into central cache
       tcmalloc::FL_Init(ptr);
@@ skipping 99 matching lines @@
 // For use by exported routines below that want specific alignments
 //
 // Note: this code can be slow for alignments > 16, and can
 // significantly fragment memory.  The expectation is that
 // memalign/posix_memalign/valloc/pvalloc will not be invoked very
 // often.  This requirement simplifies our implementation and allows
 // us to tune for expected allocation patterns.
 void* do_memalign(size_t align, size_t size) {
   ASSERT((align & (align - 1)) == 0);
   ASSERT(align > 0);
-  // Marked in CheckMallocResult(), which is also inside SpanToMallocResult(). 
+  // Marked in CheckedMallocResult(), which is also inside SpanToMallocResult().
   AddRoomForMark(&size);
   if (size + align < size) return NULL;         // Overflow
 
   // Fall back to malloc if we would already align this memory access properly.
   if (align <= AlignmentForSize(size)) {
     void* p = do_malloc(size);
     ASSERT((reinterpret_cast<uintptr_t>(p) % align) == 0);
     return p;
   }
 
@@ skipping 401 matching lines @@
   return do_mallinfo();
 }
 #endif
 
 extern "C" PERFTOOLS_DLL_DECL size_t tc_malloc_size(void* ptr) __THROW {
   return MallocExtension::instance()->GetAllocatedSize(ptr);
 }
 
 #endif  // TCMALLOC_USING_DEBUGALLOCATION
 
-// ---Double free() debugging implementation -----------------------------------
+// --- Validation implementation with an extra mark ----------------------------
 // We will put a mark at the extreme end of each allocation block.  We make
 // sure that we always allocate enough "extra memory" that we can fit in the
 // mark, and still provide the requested usable region.  If ever that mark is
 // not as expected, then we know that the user is corrupting memory beyond their
 // request size, or that they have called free a second time without having
 // the memory allocated (again).  This allows us to spot most double free()s,
 // but some can "slip by" or confuse our logic if the caller reallocates memory
 // (for a second use) before performing an evil double-free of a first
 // allocation
 
@@ skipping 22 matching lines @@
 
 static size_t ExcludeSpaceForMark(size_t size) { return size; }
 static void AddRoomForMark(size_t* size) {}
 static void ExcludeMarkFromSize(size_t* new_size) {}
 static void MarkAllocatedRegion(void* ptr) {}
 static void ValidateAllocatedRegion(void* ptr, size_t cl) {}
 
 #else  // TCMALLOC_VALIDATION
 
 static void DieFromDoubleFree() {
-  char* p = NULL;
-  p++;
-  *p += 1;  // Segv.
-}
-
-static size_t DieFromBadFreePointer(const void* unused) {
-  char* p = NULL;
-  p += 2;
-  *p += 2;  // Segv.
-  return 0;
+  Log(kCrash, __FILE__, __LINE__, "Attempt to double free");
 }
 
 static void DieFromMemoryCorruption() {
-  char* p = NULL;
-  p += 3;
-  *p += 3;  // Segv.
+  Log(kCrash, __FILE__, __LINE__, "Memory corrupted");
 }
 
 // We can either do byte marking, or whole word marking based on the following
 // define.  char is as small as we can get, and word marking probably provides
 // more than enough bits that we won't miss a corruption. Any sized integral
 // type can be used, but we just define two examples.
 
 //  #define TCMALLOC_SMALL_VALIDATION
 #if defined (TCMALLOC_SMALL_VALIDATION)
 
 typedef char MarkType;  // char saves memory... int is more complete.
 static const MarkType kAllocationMarkMask = static_cast<MarkType>(0x36);
 
-#else 
+#else
 
 typedef int MarkType;  // char saves memory... int is more complete.
 static const MarkType kAllocationMarkMask = static_cast<MarkType>(0xE1AB9536);
 
 #endif
 
 // TODO(jar): See if use of reference rather than pointer gets better inlining,
 // or if macro is needed.  My fear is that taking address map preclude register
 // allocation :-(.
 inline static void AddRoomForMark(size_t* size) {
   *size += sizeof(kAllocationMarkMask);
 }
 
 inline static void ExcludeMarkFromSize(size_t* new_size) {
   *new_size -= sizeof(kAllocationMarkMask);
 }
 
 inline static size_t ExcludeSpaceForMark(size_t size) {
   return size - sizeof(kAllocationMarkMask);  // Lie about size when asked.
 }
 
 inline static MarkType* GetMarkLocation(void* ptr) {
-  size_t class_size = GetSizeWithCallback(ptr, DieFromBadFreePointer);
-  ASSERT(class_size % sizeof(kAllocationMarkMask) == 0);
-  size_t last_index = (class_size / sizeof(kAllocationMarkMask)) - 1;
+  size_t size = GetSizeWithCallback(ptr, &InvalidGetAllocatedSize);
+  ASSERT(size % sizeof(kAllocationMarkMask) == 0);
+  size_t last_index = (size / sizeof(kAllocationMarkMask)) - 1;
   return static_cast<MarkType*>(ptr) + last_index;
 }
 
 // We hash in the mark location plus the pointer so that we effectively mix in
 // the size of the block.  This means that if a span is used for different sizes
 // that the mark will be different. It would be good to hash in the size (which
 // we effectively get by using both mark location and pointer), but even better
 // would be to also include the class, as it concisely contains the entropy
 // found in the size (when we don't have large allocation), and there is less
 // risk of losing those bits to truncation. It would probably be good to combine
@@ skipping 50 matching lines @@
   *mark = ~allocated_mark;  //  Distinctively not allocated.
 }
 
 static void MarkAllocatedRegion(void* ptr) {
   if (ptr == NULL) return;
   MarkType* mark = GetMarkLocation(ptr);
   *mark = GetMarkValue(ptr, mark);
 }
 
 #endif  // TCMALLOC_VALIDATION