Fix null dereference when detecting double-free.

In optimized builds, the non-null annotation may keep us from successfully
checking whether GetExistingDescriptor returned null.  This shouldn't happen in
ordinary execution, but it defeats the hardening check.

PiperOrigin-RevId: 706021893
Change-Id: Ia4af4a46db5ca8367e10905c7a769c975edd8908

tcmalloc/pagemap.h

@@ -440,10 +440,10 @@ class PageMap {
   // Return the descriptor for the specified page.
   // PageId must have been previously allocated.
   // No locks required.  See SYNCHRONIZATION explanation at top of tcmalloc.cc.
-  [[nodiscard]] ABSL_ATTRIBUTE_RETURNS_NONNULL inline Span*
-  GetExistingDescriptor(PageId p) const ABSL_NO_THREAD_SAFETY_ANALYSIS {
+  [[nodiscard]] inline Span* GetExistingDescriptor(PageId p) const
+      ABSL_NO_THREAD_SAFETY_ANALYSIS {
     Span* span = map_.get_existing(p.index());
-    TC_ASSERT_NE(span, nullptr);
+    TC_ASSERT_NE(span, nullptr, "Possible double free detected");
     return span;
   }
 

tcmalloc/tcmalloc.cc

@@ -495,18 +495,21 @@ extern "C" size_t MallocExtension_Internal_ReleaseCpuMemory(int cpu) {
 // Helpers for the exported routines below
 //-------------------------------------------------------------------
 
-inline size_t GetLargeSize(const void* ptr, const PageId p) {
-  const Span* span = tc_globals.pagemap().GetExistingDescriptor(p);
-  if (span->sampled()) {
+inline size_t GetLargeSize(const void* ptr, const Span& span) {
+  if (span.sampled()) {
     if (tc_globals.guardedpage_allocator().PointerIsMine(ptr)) {
       return tc_globals.guardedpage_allocator().GetRequestedSize(ptr);
     }
-    return span->sampled_allocation()->sampled_stack.allocated_size;
+    return span.sampled_allocation()->sampled_stack.allocated_size;
   } else {
-    return span->bytes_in_span();
+    return span.bytes_in_span();
   }
 }
 
+inline size_t GetLargeSize(const void* ptr, const PageId p) {
+  return GetLargeSize(ptr, *tc_globals.pagemap().GetExistingDescriptor(p));
+}
+
 inline size_t GetSize(const void* ptr) {
   if (ptr == nullptr) return 0;
   const PageId p = PageIdContainingTagged(ptr);
@@ -636,6 +639,12 @@ static void InvokeHooksAndFreePages(void* ptr, std::optional<size_t> size) {
   const PageId p = PageIdContaining(ptr);
 
   Span* span = tc_globals.pagemap().GetExistingDescriptor(p);
+  // This check failing most likely means we double-freed the span.  In the
+  // page heap, we clear the descriptor on Delete(span).
+  //
+  // We may also encounter this if we free a pointer that was never allocated
+  // (it's corrupted, it's an interior pointer to another allocation separated
+  // by more than kPageSize from the true pointer, etc.).
   TC_CHECK_NE(span, nullptr, "Possible double free detected");
 
   MaybeUnsampleAllocation(tc_globals, ptr, size, *span);

tcmalloc/testing/memory_errors_test.cc

@@ -357,6 +357,19 @@ TEST_F(TcMallocTest, DoubleFree) {
                "InvokeHooksAndFreePages\\(\\)");
 }
 
+TEST_F(TcMallocTest, LargeDoubleFree) {
+  ScopedGuardedSamplingInterval gs(-1);
+  ScopedProfileSamplingInterval s(1);
+  auto DoubleFree = []() {
+    void* buf = ::operator new(tcmalloc_internal::kMaxSize + 1);
+    benchmark::DoNotOptimize(buf);
+    ::operator delete(buf);
+    benchmark::DoNotOptimize(buf);
+    ::operator delete(buf);
+  };
+  EXPECT_DEATH(DoubleFree(), "span != nullptr.*Possible double free detected");
+}
+
 TEST_F(TcMallocTest, ReallocLarger) {
   // Note: sizes are chosen so that size + 2 access below
   // does not write out of actual allocation bounds.