Re-enable the debug ReentrancyGuard in TCMalloc's slow paths. - #1008
Draft
copybara-service[bot] wants to merge 1 commit into
Draft
copybara-service[bot] wants to merge 1 commit into
copybara-service[bot] wants to merge 1 commit into
Conversation
In debug builds, directly guard against reentrancy. AllocationGuard installs a
new hook while we hold locks, which catches bugs inside TCMalloc, but cannot
detect misuse by allocation/deallocation hooks or by code interposing
low-level C functions (open(), close(), pthread_setspecific()) that allocates
while TCMalloc is on the stack.
`ReentrancyGuard` and `tcmalloc_reentrancy_count` have been declared in
internal/logging.h since the check was first landed, but with the `TC_CHECK_EQ`
replaced by a TODO and no definition or users. This restores the definition,
the check, the guards in tcmalloc.cc, and testing/reentrancy_test.cc, with two
changes to the original placement:
* Guards live only in the `ABSL_ATTRIBUTE_NOINLINE` slow paths
(`slow_alloc_small`, `alloc_small_sampled_hooks_or_perthread`,
`slow_alloc_large`, `FreeSmallSlow`, `FreeWithHooksOrPerThread`,
`InvokeHooksAndFreePages`) and the in-place branch of `do_realloc`, rather
than at the top of `fast_alloc`/`do_free`/`do_free_with_size`. The fast
paths make no calls that can reenter, so this loses no coverage, and it
keeps `TCMALLOC_MUSTTAIL` unconditional. Where a slow path sibling-calls
another guarded slow path, the guard is constructed after that branch so
the callee's frame replaces the caller's exactly as before.
This matters because an RAII guard pins its frame until the destructor
runs. With the original placement, `__libc_malloc` could no longer
tail-call `slow_alloc_small` (`jmp` became `call; decl %fs:count; ret`),
adding one frame between the allocation entry point and every new hook.
The heap checker only ignores JVM allocations when a libjvm.so frame
appears within the top 4 frames of its raw stack
(`[NewHook, HookList::InvokeSlow, alloc_small_sampled_hooks_or_perthread,
caller]`), so the extra frame pushed libjvm to index 4 and every JVM
allocation in every fastbuild Java test was reported as a leak.
* `tcmalloc_reentrancy_count` is declared `ABSL_ATTRIBUTE_INITIAL_EXEC`,
matching `tcmalloc_slabs`, `__rseq_abi`, `tcmalloc_sampler`, and
`ThreadCache::thread_local_data_`. The guard runs on every slow-path
allocation, and a general-dynamic access through `__tls_get_addr` can
itself allocate when the DTV needs to grow after a `dlopen`, which is
exactly the reentrancy the guard is meant to catch.
Under `NDEBUG`, `ReentrancyGuard` is an empty constexpr type and codegen is
unchanged (fast_path.*.golden are byte-identical).
PiperOrigin-RevId: 983923033
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Re-enable the debug ReentrancyGuard in TCMalloc's slow paths.
In debug builds, directly guard against reentrancy. AllocationGuard installs a
new hook while we hold locks, which catches bugs inside TCMalloc, but cannot
detect misuse by allocation/deallocation hooks or by code interposing
low-level C functions (open(), close(), pthread_setspecific()) that allocates
while TCMalloc is on the stack.
ReentrancyGuardandtcmalloc_reentrancy_counthave been declared ininternal/logging.h since the check was first landed, but with the
TC_CHECK_EQreplaced by a TODO and no definition or users. This restores the definition,
the check, the guards in tcmalloc.cc, and testing/reentrancy_test.cc, with two
changes to the original placement:
Guards live only in the
ABSL_ATTRIBUTE_NOINLINEslow paths(
slow_alloc_small,alloc_small_sampled_hooks_or_perthread,slow_alloc_large,FreeSmallSlow,FreeWithHooksOrPerThread,InvokeHooksAndFreePages) and the in-place branch ofdo_realloc, ratherthan at the top of
fast_alloc/do_free/do_free_with_size. The fastpaths make no calls that can reenter, so this loses no coverage, and it
keeps
TCMALLOC_MUSTTAILunconditional. Where a slow path sibling-callsanother guarded slow path, the guard is constructed after that branch so
the callee's frame replaces the caller's exactly as before.
This matters because an RAII guard pins its frame until the destructor
runs. With the original placement,
__libc_malloccould no longertail-call
slow_alloc_small(jmpbecamecall; decl %fs:count; ret),adding one frame between the allocation entry point and every new hook.
The heap checker only ignores JVM allocations when a libjvm.so frame
appears within the top 4 frames of its raw stack
(
[NewHook, HookList::InvokeSlow, alloc_small_sampled_hooks_or_perthread, caller]), so the extra frame pushed libjvm to index 4 and every JVMallocation in every fastbuild Java test was reported as a leak.
tcmalloc_reentrancy_countis declaredABSL_ATTRIBUTE_INITIAL_EXEC,matching
tcmalloc_slabs,__rseq_abi,tcmalloc_sampler, andThreadCache::thread_local_data_. The guard runs on every slow-pathallocation, and a general-dynamic access through
__tls_get_addrcanitself allocate when the DTV needs to grow after a
dlopen, which isexactly the reentrancy the guard is meant to catch.
Under
NDEBUG,ReentrancyGuardis an empty constexpr type and codegen isunchanged (fast_path.*.golden are byte-identical).