Skip to content

Commit

Permalink
libheaptrace: Fix initialized from TLS to global
Browse files Browse the repository at this point in the history
The current implementation checks 'initialized' as a TLS variable, but
it's actually incorrect because heaptrace_init() is invoked only once
for each process, not for each thread.

More precisely, it's invoked when exec() invoked to replace its address
map.

Due to this bug, thread allocations are missed and thread deallocations
are also missed so it keeps memory allocation status if the target
backtrace is allocated by a different thread.

This patch makes 'initialized' variable from TLS to global static.

Signed-off-by: Honggyu Kim <[email protected]>
  • Loading branch information
honggyukim authored and Bojun-Seo committed Dec 3, 2022
1 parent e443599 commit 18c2b05
Show file tree
Hide file tree
Showing 2 changed files with 14 additions and 14 deletions.
3 changes: 0 additions & 3 deletions heaptrace.h
Original file line number Diff line number Diff line change
Expand Up @@ -31,9 +31,6 @@ extern FILE *outfp;
struct thread_flags_t {
// to protect unexpected recursive malloc calls
bool hook_guard;

// true only when heaptrace_init is done
bool initialized;
};
extern thread_local struct thread_flags_t thread_flags;

Expand Down
25 changes: 14 additions & 11 deletions libheaptrace.cc
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,9 @@ static PosixMemalignFunction real_posix_memalign;

thread_local struct thread_flags_t thread_flags;

// true only when heaptrace_init is done
static bool initialized;

struct opts opts;

FILE *outfp;
Expand Down Expand Up @@ -106,7 +109,7 @@ static void heaptrace_init()
pid, comm.c_str());
}

tfs->initialized = true;
initialized = true;
}

__destructor
Expand Down Expand Up @@ -139,7 +142,7 @@ void* operator new(size_t size)
{
auto* tfs = &thread_flags;

if (unlikely(tfs->hook_guard || !tfs->initialized))
if (unlikely(tfs->hook_guard || !initialized))
return __libc_malloc(size);

tfs->hook_guard = true;
Expand All @@ -158,7 +161,7 @@ void* operator new[](size_t size)
{
auto* tfs = &thread_flags;

if (unlikely(tfs->hook_guard || !tfs->initialized))
if (unlikely(tfs->hook_guard || !initialized))
return __libc_malloc(size);

tfs->hook_guard = true;
Expand All @@ -177,7 +180,7 @@ void operator delete(void *ptr)
{
auto* tfs = &thread_flags;

if (unlikely(tfs->hook_guard || !tfs->initialized)) {
if (unlikely(tfs->hook_guard || !initialized)) {
__libc_free(ptr);
return;
}
Expand All @@ -196,7 +199,7 @@ void operator delete[](void *ptr)
{
auto* tfs = &thread_flags;

if (unlikely(tfs->hook_guard || !tfs->initialized)) {
if (unlikely(tfs->hook_guard || !initialized)) {
__libc_free(ptr);
return;
}
Expand All @@ -215,7 +218,7 @@ void* malloc(size_t size)
{
auto* tfs = &thread_flags;

if (unlikely(tfs->hook_guard || !tfs->initialized))
if (unlikely(tfs->hook_guard || !initialized))
return __libc_malloc(size);

tfs->hook_guard = true;
Expand All @@ -234,7 +237,7 @@ void free(void *ptr)
{
auto* tfs = &thread_flags;

if (unlikely(tfs->hook_guard || !tfs->initialized)) {
if (unlikely(tfs->hook_guard || !initialized)) {
__libc_free(ptr);
return;
}
Expand All @@ -253,7 +256,7 @@ void *calloc(size_t nmemb, size_t size)
{
auto* tfs = &thread_flags;

if (unlikely(tfs->hook_guard || !tfs->initialized))
if (unlikely(tfs->hook_guard || !initialized))
return __libc_calloc(nmemb, size);

tfs->hook_guard = true;
Expand All @@ -272,7 +275,7 @@ void *realloc(void *ptr, size_t size)
{
auto* tfs = &thread_flags;

if (unlikely(tfs->hook_guard || !tfs->initialized))
if (unlikely(tfs->hook_guard || !initialized))
return __libc_realloc(ptr, size);

tfs->hook_guard = true;
Expand All @@ -292,7 +295,7 @@ void *memalign(size_t alignment, size_t size)
{
auto *tfs = &thread_flags;

if (unlikely(tfs->hook_guard || !tfs->initialized))
if (unlikely(tfs->hook_guard || !initialized))
return __libc_memalign(alignment, size);

tfs->hook_guard = true;
Expand All @@ -314,7 +317,7 @@ int posix_memalign(void **memptr, size_t alignment, size_t size)
if (unlikely(!real_posix_memalign))
real_posix_memalign = (PosixMemalignFunction)dlsym(RTLD_NEXT, "posix_memalign");

if (unlikely(tfs->hook_guard || !tfs->initialized))
if (unlikely(tfs->hook_guard || !initialized))
return real_posix_memalign(memptr, alignment, size);

tfs->hook_guard = true;
Expand Down

0 comments on commit 18c2b05

Please sign in to comment.