Avoid reading many-object span sampled state in assertions.

Spans with multiple objects are mutated by the CentralFreeList as objects are
allocate/deallocated from the span.  The Span contains a bitfield that shares
its byte with nonempty_index_ (mutated by CentralFreeList) and sampled_,
leading to a data race.

This assertion is superfluous, as we assert that the PageMap has a non-zero
size class for the object consistent with the one provided.  We also confirm
that sampled span do not have non-zero size classes upon
allocation/deallocation.

We add a stress test as well, for GetAllocatedSize, since this exercises a path
that checks sampled(), but only on large allocations.

PiperOrigin-RevId: 573323553
Change-Id: I0ab4ae1ff63130318113d397495f685aa0ec12dc

tcmalloc/allocation_sampling.h

@@ -455,6 +455,7 @@ static sized_ptr_t SampleifyAllocation(State& state, Policy policy,
     ASSERT(size_class != 0);
     FreeProxyObject(state, obj, size_class);
   }
+  ASSERT(state.pagemap().sizeclass(span->first_page()) == 0);
   return {(alloc_with_status.alloc != nullptr) ? alloc_with_status.alloc
                                                : span->start_address(),
           capacity};
@@ -466,6 +467,8 @@ inline void MaybeUnsampleAllocation(State& state, void* ptr, Span* span) {
   // state cleared only once. External synchronization when freeing is required;
   // otherwise, concurrent writes here would likely report a double-free.
   if (SampledAllocation* sampled_allocation = span->Unsample()) {
+    ASSERT(state.pagemap().sizeclass(PageIdContaining(ptr)) == 0);
+
     void* const proxy = sampled_allocation->sampled_stack.proxy;
     const size_t weight = sampled_allocation->sampled_stack.weight;
     const size_t requested_size =

tcmalloc/tcmalloc.cc

@@ -722,8 +722,8 @@ static size_t GetSizeClass(void* ptr) {
 template <bool have_size_class>
 inline ABSL_ATTRIBUTE_ALWAYS_INLINE void do_free_with_size_class(
     void* ptr, size_t size_class) {
-  // !have_size_class -> size_class == 0
-  ASSERT(have_size_class || size_class == 0);
+  // !have_size_class <-> size_class == 0
+  ASSERT(have_size_class != (size_class == 0));
 
   // if we have_size_class, then we've excluded ptr == nullptr case. See
   // comment in do_free_with_size. Thus we only bother testing nullptr
@@ -746,7 +746,6 @@ inline ABSL_ATTRIBUTE_ALWAYS_INLINE void do_free_with_size_class(
   if (have_size_class || ABSL_PREDICT_TRUE(size_class != 0)) {
     ASSERT(size_class == GetSizeClass(ptr));
     ASSERT(ptr != nullptr);
-    ASSERT(!tc_globals.pagemap().GetExistingDescriptor(p)->sampled());
     FreeSmall(ptr, size_class);
   } else {
     InvokeHooksAndFreePages(ptr);

tcmalloc/testing/BUILD

@@ -902,7 +902,9 @@ cc_test(
     copts = TCMALLOC_DEFAULT_COPTS,
     malloc = "//tcmalloc/internal:system_malloc",
     deps = [
+        "//tcmalloc:malloc_extension",
         "//tcmalloc:tcmalloc_internal_methods_only",  # buildcleaner: keep
+        "@com_google_absl//absl/log:absl_check",
         "@com_google_absl//absl/random",
         "@com_google_absl//absl/time",
         "@com_google_googletest//:gtest_main",

tcmalloc/testing/parallel_test.cc

@@ -18,9 +18,11 @@
 #include <vector>
 
 #include "gtest/gtest.h"
+#include "absl/log/absl_check.h"
 #include "absl/random/random.h"
 #include "absl/time/clock.h"
 #include "absl/time/time.h"
+#include "tcmalloc/malloc_extension.h"
 
 extern "C" {
 
@@ -50,6 +52,10 @@ struct Allocator {
         v.push_back(TCMallocInternalNew(size));
       }
 
+      for (void* ptr : v) {
+        ABSL_CHECK_GE(*MallocExtension::GetAllocatedSize(ptr), size);
+      }
+
       for (void* ptr : v) {
         if (do_sized_delete) {
           TCMallocInternalDeleteSized(ptr, size);
@@ -66,11 +72,6 @@ struct Allocator {
 };
 
 TEST(ParallelTest, Stable) {
-#ifdef ABSL_HAVE_THREAD_SANITIZER
-  // TODO(b/274996721):  Enable this when Span::nonempty_index_ does not
-  // conflict with other bitfields.
-  GTEST_SKIP() << "Skipping under tsan.";
-#endif
   std::atomic<bool> stop{false};
   Allocator a1(stop, /*do_sized_delete=*/true),
       a2(stop, /*do_sized_delete=*/true), a3(stop, /*do_sized_delete=*/false);