From 8c59c7bd7e04852c3f875daac464b1f1f9fb8774 Mon Sep 17 00:00:00 2001 From: Chris Kennelly CA Date: Thu, 10 Sep 2026 15:21:15 -0700 Subject: [PATCH] Fix --per_file_copt warning suppression in .bazelrc. Bazel's --per_file_copt syntax uses '-' as an exclusion prefix and '+' or no prefix for inclusion. The leading '-' on -external/.* inadvertently excluded external dependencies from -w and defaulted to including all non-excluded files, suppressing compiler warnings on TCMalloc's own sources instead of external code. Fix newly unmasked compiler warnings in alloc_at_least_test.c and delay_injection.h. PiperOrigin-RevId: 979427697 --- .bazelrc | 4 +- tcmalloc/allocation_sample.h | 4 -- tcmalloc/huge_page_filler_test.cc | 95 +------------------------- tcmalloc/internal/delay_injection.h | 2 + tcmalloc/internal/logging.h | 6 +- tcmalloc/internal/percpu_tcmalloc.h | 5 +- tcmalloc/page_allocator.cc | 19 ++++-- tcmalloc/page_allocator.h | 17 +---- tcmalloc/page_allocator_test.cc | 60 ---------------- tcmalloc/testing/alloc_at_least_test.c | 2 + 10 files changed, 29 insertions(+), 185 deletions(-) diff --git a/.bazelrc b/.bazelrc index cab3b763e..ddb187e7e 100644 --- a/.bazelrc +++ b/.bazelrc @@ -17,8 +17,8 @@ build --cxxopt='-std=c++17' test --test_output=errors # Disable noisy warnings from dependencies. -build --per_file_copt=-external/.*@-w -build --host_per_file_copt=-external/.*@-w +build --per_file_copt=external/.*@-w +build --host_per_file_copt=external/.*@-w # Disable noisy warnings in TCMalloc. build:clang --copt=-Wno-nullability-completeness diff --git a/tcmalloc/allocation_sample.h b/tcmalloc/allocation_sample.h index 7cdd408e8..24b33bd51 100644 --- a/tcmalloc/allocation_sample.h +++ b/tcmalloc/allocation_sample.h @@ -72,10 +72,6 @@ class AllocationSampleList { } void ReportMalloc(const struct StackTrace& sample) { - // Check that StackTrace was zero-initialized so we don't leak uninitialized - // memory (potentially holding cryptographic material) into core dumps. - TC_CHECK(sample.depth == kMaxStackDepth || - sample.stack[sample.depth] == nullptr); AllocationGuardSpinLockHolder h(lock_); AllocationSample* cur = first_; while (cur != nullptr) { diff --git a/tcmalloc/huge_page_filler_test.cc b/tcmalloc/huge_page_filler_test.cc index 02b2bb924..33f9f6cb9 100644 --- a/tcmalloc/huge_page_filler_test.cc +++ b/tcmalloc/huge_page_filler_test.cc @@ -242,43 +242,19 @@ class FakeResidency : public Residency { class FakeClock { public: FakeClock() = default; - [[nodiscard]] static int64_t now() { - now_calls_.fetch_add(1, std::memory_order_relaxed); - return clock_.load(std::memory_order_relaxed); - } - [[nodiscard]] static double freq() { - freq_calls_.fetch_add(1, std::memory_order_relaxed); - return absl::ToDoubleNanoseconds(absl::Seconds(2)); - } + static int64_t now() { return clock_.load(std::memory_order_relaxed); } + static double freq() { return absl::ToDoubleNanoseconds(absl::Seconds(2)); } static void Advance(absl::Duration d) { clock_.fetch_add(static_cast(absl::ToDoubleSeconds(d) * freq()), std::memory_order_relaxed); } - static void ResetClock() { - clock_.store(1234, std::memory_order_relaxed); - now_calls_.store(0, std::memory_order_relaxed); - freq_calls_.store(0, std::memory_order_relaxed); - } - [[nodiscard]] static size_t now_calls() { - return now_calls_.load(std::memory_order_relaxed); - } - [[nodiscard]] static size_t freq_calls() { - return freq_calls_.load(std::memory_order_relaxed); - } - static void ResetCalls() { - now_calls_.store(0, std::memory_order_relaxed); - freq_calls_.store(0, std::memory_order_relaxed); - } + static void ResetClock() { clock_.store(1234, std::memory_order_relaxed); } private: static std::atomic clock_; - static std::atomic now_calls_; - static std::atomic freq_calls_; }; std::atomic FakeClock::clock_{1234}; -std::atomic FakeClock::now_calls_{0}; -std::atomic FakeClock::freq_calls_{0}; class MockCollapse final : public MemoryModifyFunction { public: @@ -679,71 +655,6 @@ class FillerTestWithSubreleaseUnbacked : public FillerTest { : FillerTest(SubreleaseUnbackedMode::kEnabled) {} }; -// TODO(b/73749855): Reduce the count of clock_.now() and clock_.freq() calls. -TEST_F(FillerTest, ClockCalls) { - SpanAllocInfo info = {.objects_per_span = 1, - .density = AccessDensityPrediction::kSparse}; - - // 1. TryGet on empty filler (miss). - FakeClock::ResetCalls(); - { - PageHeapSpinLockHolder l; - auto res = filler_.TryGet(Length(1), info); - EXPECT_EQ(res.pt, nullptr); - } - EXPECT_EQ(FakeClock::now_calls(), 0); - EXPECT_EQ(FakeClock::freq_calls(), 0); - - auto* pt = new PageTracker(GetBacking(), /*was_donated=*/false, 0); - PageId page1; - { - PageHeapSpinLockHolder l; - page1 = pt->Get(Length(1), info).page; - filler_.Contribute(pt, /*donated=*/false, info); - } - - // 2. TryGet on available hugepage (hit). - // TODO(b/73749855): Reduce the number of clock calls. - FakeClock::ResetCalls(); - PageTracker* alloc_pt; - PageId page2; - { - PageHeapSpinLockHolder l; - auto res = filler_.TryGet(Length(1), info); - alloc_pt = res.pt; - page2 = res.page; - } - EXPECT_EQ(alloc_pt, pt); - EXPECT_EQ(FakeClock::now_calls(), 2); - EXPECT_EQ(FakeClock::freq_calls(), 0); - - // 3. Put (partially freed hugepage). - // TODO(b/73749855): Reduce the number of clock calls. - FakeClock::ResetCalls(); - PageTracker* put_res1; - { - PageHeapSpinLockHolder l; - put_res1 = filler_.Put(alloc_pt, Range(page2, Length(1)), info); - } - EXPECT_EQ(put_res1, nullptr); - EXPECT_EQ(FakeClock::now_calls(), 1); - EXPECT_EQ(FakeClock::freq_calls(), 0); - - // 4. Put (fully freed hugepage). - // TODO(b/73749855): Reduce the number of clock calls. - FakeClock::ResetCalls(); - PageTracker* put_res2; - { - PageHeapSpinLockHolder l; - put_res2 = filler_.Put(pt, Range(page1, Length(1)), info); - } - EXPECT_EQ(put_res2, pt); - EXPECT_EQ(FakeClock::now_calls(), 2); - EXPECT_EQ(FakeClock::freq_calls(), 1); - - delete pt; -} - TEST_F(FillerTest, Density) { absl::BitGen rng; // Start with a really annoying setup: some hugepages half empty (randomly) diff --git a/tcmalloc/internal/delay_injection.h b/tcmalloc/internal/delay_injection.h index d4c835a8c..0350f0188 100644 --- a/tcmalloc/internal/delay_injection.h +++ b/tcmalloc/internal/delay_injection.h @@ -42,6 +42,8 @@ class ScopedDelay { __asm__ __volatile__("yield\n"); #endif } +#else + (void)delay_cycles; #endif } ~ScopedDelay() = default; diff --git a/tcmalloc/internal/logging.h b/tcmalloc/internal/logging.h index 9148a429d..73090c9f1 100644 --- a/tcmalloc/internal/logging.h +++ b/tcmalloc/internal/logging.h @@ -91,11 +91,9 @@ struct StackTrace { // for residency analysis such as for peakheapz. void* span_start_address = nullptr; - uintptr_t depth = 0; // Number of PC values stored in array below + uintptr_t depth; // Number of PC values stored in array below // Place stack as last member because it might not all be accessed. - // Zero-initialized so uninitialized stack memory (which might contain - // cryptographic secrets) is not leaked into the heap and hence core dumps. - void* stack[kMaxStackDepth] = {}; + void* stack[kMaxStackDepth]; }; #define TC_LOG(msg, ...) \ diff --git a/tcmalloc/internal/percpu_tcmalloc.h b/tcmalloc/internal/percpu_tcmalloc.h index ce8ace58b..31502b42a 100644 --- a/tcmalloc/internal/percpu_tcmalloc.h +++ b/tcmalloc/internal/percpu_tcmalloc.h @@ -46,15 +46,14 @@ #include "tcmalloc/internal/prefetch.h" #include "tcmalloc/internal/sysinfo.h" -#if defined(__GNUC__) && !defined(__clang__) && defined(__x86_64__) +#if defined(__GNUC__) && __GNUC__ >= 14 && !defined(__clang__) && \ + defined(__x86_64__) // Work around https://gcc.gnu.org/bugzilla/show_bug.cgi?id=125526 // by force-loading the address of the thread-local rseq_cs_addr into // a register instead of giving it as a "m" constraint. // // TODO: Remove this when GCC releases a fixed version. #define TCMALLOC_INTERNAL_PERCPU_USE_TLS_WORKAROUND 1 -#else -#define TCMALLOC_INTERNAL_PERCPU_USE_TLS_WORKAROUND 0 #endif GOOGLE_MALLOC_SECTION_BEGIN diff --git a/tcmalloc/page_allocator.cc b/tcmalloc/page_allocator.cc index d5d61b5e3..b1a1b451b 100644 --- a/tcmalloc/page_allocator.cc +++ b/tcmalloc/page_allocator.cc @@ -76,7 +76,21 @@ PageAllocator::PageAllocator() { TC_CHECK_LE(part, std::size(choices_)); } -void PageAllocator::ShrinkToUsageLimitSlow(Length n) { +void PageAllocator::ShrinkToUsageLimit(Length n, bool may_have_grown) { +#ifdef TCMALLOC_INTERNAL_LEGACY_LOCKING + const bool check_stats = true; +#else +#ifndef NDEBUG + const bool check_stats = true; +#else + const bool check_stats = may_have_grown; +#endif // NDEBUG +#endif // TCMALLOC_INTERNAL_LEGACY_LOCKING + + if (!check_stats) { + return; + } + BackingStats s = stats(); const size_t backed = s.system_bytes - s.unmapped_bytes + tc_globals.metadata_bytes(); @@ -99,7 +113,6 @@ void PageAllocator::ShrinkToUsageLimitSlow(Length n) { // occur if we allocate space for many objects preemptively and only later // sample them (incrementing sampled_objects_size_). - over_limit_ = false; if (limits_[kSoft] == std::numeric_limits::max()) { // Limits are not set. return; @@ -148,8 +161,6 @@ void PageAllocator::ShrinkToUsageLimitSlow(Length n) { hard_limit); } - over_limit_ = true; - // Print logs once. static bool warned = false; if (warned) return; diff --git a/tcmalloc/page_allocator.h b/tcmalloc/page_allocator.h index 11e74f969..41aa46982 100644 --- a/tcmalloc/page_allocator.h +++ b/tcmalloc/page_allocator.h @@ -141,19 +141,7 @@ class PageAllocator { // If we have a usage limit set, ensure we're not violating it from our latest // allocation. void ShrinkToUsageLimit(Length n, bool may_have_grown) - ABSL_EXCLUSIVE_LOCKS_REQUIRED(pageheap_lock) { -#if defined(TCMALLOC_INTERNAL_LEGACY_LOCKING) || !defined(NDEBUG) - const bool check_stats = true; -#else - const bool check_stats = may_have_grown || over_limit_; -#endif - - if (!check_stats) { - return; - } - - ShrinkToUsageLimitSlow(n); - } + ABSL_EXCLUSIVE_LOCKS_REQUIRED(pageheap_lock); void TreatHugepageTrackers(EnableCollapse enable_collapse) ABSL_LOCKS_EXCLUDED(pageheap_lock); @@ -207,8 +195,6 @@ class PageAllocator { MemoryTag tag); static void InvokeReleaseHookSlow(Length num_pages, Length released, PageReleaseReason reason); - ABSL_ATTRIBUTE_NOINLINE void ShrinkToUsageLimitSlow(Length n) - ABSL_EXCLUSIVE_LOCKS_REQUIRED(pageheap_lock); bool ShrinkHardBy(Length page, LimitKind limit_kind) ABSL_EXCLUSIVE_LOCKS_REQUIRED(pageheap_lock); @@ -232,7 +218,6 @@ class PageAllocator { Algorithm alg_; bool has_cold_impl_; bool sampled_partition_active_; - bool over_limit_ ABSL_GUARDED_BY(pageheap_lock) = false; // Max size of backed spans we will attempt to maintain. // Crash if we can't maintain below limits_[kHard], which is guaranteed to be diff --git a/tcmalloc/page_allocator_test.cc b/tcmalloc/page_allocator_test.cc index 771a1ef92..97f0d9ba7 100644 --- a/tcmalloc/page_allocator_test.cc +++ b/tcmalloc/page_allocator_test.cc @@ -251,66 +251,6 @@ TEST_F(PageAllocatorTest, b270916852) { Parameters::set_hpaa_subrelease(old_subrelease); } -TEST_F(PageAllocatorTest, ShrinkFailureStickyTest) { - // Turn off subrelease so that we take the ShrinkHardBy path. - const bool old_subrelease = Parameters::hpaa_subrelease(); - Parameters::set_hpaa_subrelease(false); - - constexpr SpanAllocInfo kSpanInfo = {/*objects_per_span=*/1, - AccessDensityPrediction::kSparse}; - Span* normal1 = New(kPagesPerHugePage / 4, kSpanInfo, MemoryTag::kNormal); - Span* normal2 = New(kPagesPerHugePage / 4, kSpanInfo, MemoryTag::kNormal); - Span* sampled = New(kPagesPerHugePage / 2, kSpanInfo, MemoryTag::kSampled); - - BackingStats stats; - { - PageHeapSpinLockHolder l; - stats = allocator_.stats(); - } - EXPECT_EQ(stats.system_bytes, 2 * kHugePageSize); - EXPECT_EQ(stats.free_bytes, kHugePageSize); - EXPECT_EQ(stats.unmapped_bytes, 0); - - // Choose a limit so that we hit and we are not able to satisfy it. - const size_t metadata_bytes = []() { - PageHeapSpinLockHolder l; - return tc_globals.metadata_bytes(); - }(); - allocator_.set_limit(metadata_bytes + (3 * kPagesPerHugePage / 4).in_bytes(), - PageAllocator::kSoft); - EXPECT_EQ(1, allocator_.limit_hits(PageAllocator::kSoft)); - EXPECT_EQ( - 0, allocator_.successful_shrinks_after_limit_hit(PageAllocator::kSoft)); - // Now delete normal1 so that memory can be released to get under limit. - // normal2 is still alive on that hugepage, so HugePageFiller::Put does - // not unback the hugepage automatically. - Delete(normal1, kSpanInfo, MemoryTag::kNormal); - - // A subsequent allocation with may_have_grown == false should attempt to - // shrink until below the limit. - { - PageHeapSpinLockHolder l; - allocator_.ShrinkToUsageLimit(Length(0), /*may_have_grown=*/false); - } - EXPECT_EQ(2, allocator_.limit_hits(PageAllocator::kSoft)); - EXPECT_EQ( - 1, allocator_.successful_shrinks_after_limit_hit(PageAllocator::kSoft)); - - // Now that we are below the limit, a subsequent call with may_have_grown == - // false should not attempt to shrink. - { - PageHeapSpinLockHolder l; - allocator_.ShrinkToUsageLimit(Length(0), /*may_have_grown=*/false); - } - EXPECT_EQ(2, allocator_.limit_hits(PageAllocator::kSoft)); - EXPECT_EQ( - 1, allocator_.successful_shrinks_after_limit_hit(PageAllocator::kSoft)); - - Delete(normal2, kSpanInfo, MemoryTag::kNormal); - Delete(sampled, kSpanInfo, MemoryTag::kSampled); - Parameters::set_hpaa_subrelease(old_subrelease); -} - struct HookRecord { size_t start_page_index; size_t n; diff --git a/tcmalloc/testing/alloc_at_least_test.c b/tcmalloc/testing/alloc_at_least_test.c index 9f04d262a..c951341d6 100644 --- a/tcmalloc/testing/alloc_at_least_test.c +++ b/tcmalloc/testing/alloc_at_least_test.c @@ -23,6 +23,8 @@ #include int main(int argc, char** argv) { + (void)argc; + (void)argv; int exit_code = EXIT_SUCCESS; alloc_result_t result = alloc_at_least(127); if (result.ptr == NULL || result.size < 127) {