From 99c3fe52d237eae546d7de484d0cfbd615ac192c Mon Sep 17 00:00:00 2001 From: Vladimir Mezentsev Date: Sat, 23 Mar 2024 18:31:03 -0700 Subject: [PATCH] gprofng: fix infinite recursion on calloc with multi-threaded applications libcollector uses pthread_getspecific() and pthread_setspecific() to access thread local memory. libcollector uses this memory to check that interposed functions (like malloc, calloc or free) don't have recursion. The first time we call calloc(), we call pthread_setspecific() to create a thread-specific value. On Ubuntu machine, pthread_setspecific() calls calloc(), and we cannot intercept such recursion. gcc supports thread-local storage. For example, static __thread int reentrance = 0; I rewrote code using this instead of pthread_setspecific(). gprofng/ChangeLog 2024-03-23 Vladimir Mezentsev PR gprofng/31460 * libcollector/heaptrace.c: Use the __thread variable to check for * reentry. Clean up code. --- gprofng/libcollector/heaptrace.c | 160 ++++++++++++------------------- 1 file changed, 62 insertions(+), 98 deletions(-) diff --git a/gprofng/libcollector/heaptrace.c b/gprofng/libcollector/heaptrace.c index 481a69bcb6b..cf39d12cc69 100644 --- a/gprofng/libcollector/heaptrace.c +++ b/gprofng/libcollector/heaptrace.c @@ -62,11 +62,12 @@ static ModuleInterface module_interface = { static CollectorInterface *collector_interface = NULL; static int heap_mode = 0; static CollectorModule heap_hndl = COLLECTOR_MODULE_ERR; -static unsigned heap_key = COLLECTOR_TSD_INVALID_KEY; +static const Heap_packet heap_packet0 = { .comm.tsize = sizeof ( Heap_packet) }; +static __thread int reentrance = 0; -#define CHCK_REENTRANCE(x) ( !heap_mode || ((x) = collector_interface->getKey( heap_key )) == NULL || (*(x) != 0) ) -#define PUSH_REENTRANCE(x) ((*(x))++) -#define POP_REENTRANCE(x) ((*(x))--) +#define CHCK_REENTRANCE ( !heap_mode || reentrance != 0 ) +#define PUSH_REENTRANCE (reentrance++) +#define POP_REENTRANCE (reentrance--) #define gethrtime collector_interface->getHiResTime static void *(*__real_malloc)(size_t) = NULL; @@ -81,14 +82,6 @@ void *__libc_malloc (size_t); void __libc_free (void *); void *__libc_realloc (void *, size_t); -static void -collector_memset (void *s, int c, size_t n) -{ - unsigned char *s1 = s; - while (n--) - *s1++ = (unsigned char) c; -} - void __collector_module_init (CollectorInterface *_collector_interface) { @@ -139,14 +132,6 @@ open_experiment (const char *exp) if (params == NULL) /* Heap data collection not specified */ return COL_ERROR_HEAPINIT; - heap_key = collector_interface->createKey (sizeof ( int), NULL, NULL); - if (heap_key == (unsigned) - 1) - { - Tprintf (0, "heaptrace: TSD key create failed.\n"); - collector_interface->writeLog ("TSD key not created\n", - SP_JCMD_CERROR, COL_ERROR_HEAPINIT); - return COL_ERROR_HEAPINIT; - } collector_interface->writeLog ("\n", SP_JCMD_HEAPTRACE); collector_interface->writeLog (" \n", module_interface.description); @@ -205,7 +190,6 @@ static int close_experiment (void) { heap_mode = 0; - heap_key = COLLECTOR_TSD_INVALID_KEY; Tprintf (0, "heaptrace: close_experiment\n"); return 0; } @@ -215,7 +199,6 @@ detach_experiment (void) /* fork child. Clean up state but don't write to experiment */ { heap_mode = 0; - heap_key = COLLECTOR_TSD_INVALID_KEY; Tprintf (0, "heaptrace: detach_experiment\n"); return 0; } @@ -265,36 +248,33 @@ void * malloc (size_t size) { void *ret; - int *guard; - Heap_packet hpacket; /* Linux startup workaround */ if (!heap_mode) { - void *ppp = (void *) __libc_malloc (size); - Tprintf (DBG_LT4, "heaptrace: LINUX malloc(%ld, %p)...\n", (long) size, ppp); - return ppp; + ret = (void *) __libc_malloc (size); + Tprintf (DBG_LT4, "heaptrace: LINUX malloc(%ld, %p)\n", (long) size, ret); + return ret; } if (NULL_PTR (malloc)) init_heap_intf (); - if (CHCK_REENTRANCE (guard)) + if (CHCK_REENTRANCE) { ret = (void *) CALL_REAL (malloc)(size); Tprintf (DBG_LT4, "heaptrace: real malloc(%ld) = %p\n", (long) size, ret); return ret; } - PUSH_REENTRANCE (guard); - - ret = (void *) CALL_REAL (malloc)(size); - collector_memset (&hpacket, 0, sizeof ( Heap_packet)); - hpacket.comm.tsize = sizeof ( Heap_packet); + PUSH_REENTRANCE; + Heap_packet hpacket = heap_packet0; hpacket.comm.tstamp = gethrtime (); + ret = (void *) CALL_REAL (malloc)(size); hpacket.mtype = MALLOC_TRACE; hpacket.size = (Size_type) size; hpacket.vaddr = (intptr_t) ret; - hpacket.comm.frinfo = collector_interface->getFrameInfo (heap_hndl, hpacket.comm.tstamp, FRINFO_FROM_STACK, &hpacket); + hpacket.comm.frinfo = collector_interface->getFrameInfo (heap_hndl, + hpacket.comm.tstamp, FRINFO_FROM_STACK, &hpacket); collector_interface->writeDataRecord (heap_hndl, (Common_packet*) & hpacket); - POP_REENTRANCE (guard); - return (void *) ret; + POP_REENTRANCE; + return ret; } /*------------------------------------------------------------- free */ @@ -302,8 +282,6 @@ malloc (size_t size) void free (void *ptr) { - int *guard; - Heap_packet hpacket; /* Linux startup workaround */ if (!heap_mode) { @@ -311,28 +289,26 @@ free (void *ptr) __libc_free (ptr); return; } - if (NULL_PTR (malloc)) + if (NULL_PTR (free)) init_heap_intf (); - if (CHCK_REENTRANCE (guard)) + if (ptr == NULL) + return; + if (CHCK_REENTRANCE) { CALL_REAL (free)(ptr); return; } - if (ptr == NULL) - return; - PUSH_REENTRANCE (guard); - + PUSH_REENTRANCE; /* Get a timestamp before 'free' to enforce consistency */ - hrtime_t ts = gethrtime (); + Heap_packet hpacket = heap_packet0; + hpacket.comm.tstamp = gethrtime (); CALL_REAL (free)(ptr); - collector_memset (&hpacket, 0, sizeof ( Heap_packet)); - hpacket.comm.tsize = sizeof ( Heap_packet); - hpacket.comm.tstamp = ts; hpacket.mtype = FREE_TRACE; hpacket.vaddr = (intptr_t) ptr; - hpacket.comm.frinfo = collector_interface->getFrameInfo (heap_hndl, hpacket.comm.tstamp, FRINFO_FROM_STACK, &hpacket); + hpacket.comm.frinfo = collector_interface->getFrameInfo (heap_hndl, + hpacket.comm.tstamp, FRINFO_FROM_STACK, &hpacket); collector_interface->writeDataRecord (heap_hndl, (Common_packet*) & hpacket); - POP_REENTRANCE (guard); + POP_REENTRANCE; return; } @@ -341,9 +317,6 @@ void * realloc (void *ptr, size_t size) { void *ret; - int *guard; - Heap_packet hpacket; - /* Linux startup workaround */ if (!heap_mode) { @@ -354,24 +327,23 @@ realloc (void *ptr, size_t size) } if (NULL_PTR (realloc)) init_heap_intf (); - if (CHCK_REENTRANCE (guard)) + if (CHCK_REENTRANCE) { ret = (void *) CALL_REAL (realloc)(ptr, size); return ret; } - PUSH_REENTRANCE (guard); - hrtime_t ts = gethrtime (); + PUSH_REENTRANCE; + Heap_packet hpacket = heap_packet0; + hpacket.comm.tstamp = gethrtime (); ret = (void *) CALL_REAL (realloc)(ptr, size); - collector_memset (&hpacket, 0, sizeof ( Heap_packet)); - hpacket.comm.tsize = sizeof ( Heap_packet); - hpacket.comm.tstamp = ts; hpacket.mtype = REALLOC_TRACE; hpacket.size = (Size_type) size; hpacket.vaddr = (intptr_t) ret; - hpacket.comm.frinfo = collector_interface->getFrameInfo (heap_hndl, hpacket.comm.tstamp, FRINFO_FROM_STACK, &hpacket); + hpacket.comm.frinfo = collector_interface->getFrameInfo (heap_hndl, + hpacket.comm.tstamp, FRINFO_FROM_STACK, &hpacket); collector_interface->writeDataRecord (heap_hndl, (Common_packet*) & hpacket); - POP_REENTRANCE (guard); - return (void *) ret; + POP_REENTRANCE; + return ret; } /*------------------------------------------------------------- memalign */ @@ -379,26 +351,24 @@ void * memalign (size_t align, size_t size) { void *ret; - int *guard; - Heap_packet hpacket; if (NULL_PTR (memalign)) init_heap_intf (); - if (CHCK_REENTRANCE (guard)) + if (CHCK_REENTRANCE) { ret = (void *) CALL_REAL (memalign)(align, size); return ret; } - PUSH_REENTRANCE (guard); - ret = (void *) CALL_REAL (memalign)(align, size); - collector_memset (&hpacket, 0, sizeof ( Heap_packet)); - hpacket.comm.tsize = sizeof ( Heap_packet); + PUSH_REENTRANCE; + Heap_packet hpacket = heap_packet0; hpacket.comm.tstamp = gethrtime (); + ret = (void *) CALL_REAL (memalign)(align, size); hpacket.mtype = MALLOC_TRACE; hpacket.size = (Size_type) size; hpacket.vaddr = (intptr_t) ret; - hpacket.comm.frinfo = collector_interface->getFrameInfo (heap_hndl, hpacket.comm.tstamp, FRINFO_FROM_STACK, &hpacket); + hpacket.comm.frinfo = collector_interface->getFrameInfo (heap_hndl, + hpacket.comm.tstamp, FRINFO_FROM_STACK, &hpacket); collector_interface->writeDataRecord (heap_hndl, (Common_packet*) & hpacket); - POP_REENTRANCE (guard); + POP_REENTRANCE; return ret; } @@ -408,26 +378,24 @@ void * valloc (size_t size) { void *ret; - int *guard; - Heap_packet hpacket; - if (NULL_PTR (memalign)) + if (NULL_PTR (valloc)) init_heap_intf (); - if (CHCK_REENTRANCE (guard)) + if (CHCK_REENTRANCE) { ret = (void *) CALL_REAL (valloc)(size); return ret; } - PUSH_REENTRANCE (guard); - ret = (void *) CALL_REAL (valloc)(size); - collector_memset (&hpacket, 0, sizeof ( Heap_packet)); - hpacket.comm.tsize = sizeof ( Heap_packet); + PUSH_REENTRANCE; + Heap_packet hpacket = heap_packet0; hpacket.comm.tstamp = gethrtime (); + ret = (void *) CALL_REAL (valloc)(size); hpacket.mtype = MALLOC_TRACE; hpacket.size = (Size_type) size; hpacket.vaddr = (intptr_t) ret; - hpacket.comm.frinfo = collector_interface->getFrameInfo (heap_hndl, hpacket.comm.tstamp, FRINFO_FROM_STACK, &hpacket); + hpacket.comm.frinfo = collector_interface->getFrameInfo (heap_hndl, + hpacket.comm.tstamp, FRINFO_FROM_STACK, &hpacket); collector_interface->writeDataRecord (heap_hndl, (Common_packet*) & hpacket); - POP_REENTRANCE (guard); + POP_REENTRANCE; return ret; } @@ -436,30 +404,28 @@ void * calloc (size_t size, size_t esize) { void *ret; - int *guard; - Heap_packet hpacket; if (NULL_PTR (calloc)) { if (in_init_heap_intf != 0) return NULL; // Terminate infinite loop init_heap_intf (); } - if (CHCK_REENTRANCE (guard)) + if (CHCK_REENTRANCE) { ret = (void *) CALL_REAL (calloc)(size, esize); return ret; } - PUSH_REENTRANCE (guard); - ret = (void *) CALL_REAL (calloc)(size, esize); - collector_memset (&hpacket, 0, sizeof ( Heap_packet)); - hpacket.comm.tsize = sizeof ( Heap_packet); + PUSH_REENTRANCE; + Heap_packet hpacket = heap_packet0; hpacket.comm.tstamp = gethrtime (); + ret = (void *) CALL_REAL (calloc)(size, esize); hpacket.mtype = MALLOC_TRACE; hpacket.size = (Size_type) (size * esize); hpacket.vaddr = (intptr_t) ret; - hpacket.comm.frinfo = collector_interface->getFrameInfo (heap_hndl, hpacket.comm.tstamp, FRINFO_FROM_STACK, &hpacket); + hpacket.comm.frinfo = collector_interface->getFrameInfo (heap_hndl, + hpacket.comm.tstamp, FRINFO_FROM_STACK, &hpacket); collector_interface->writeDataRecord (heap_hndl, (Common_packet*) & hpacket); - POP_REENTRANCE (guard); + POP_REENTRANCE; return ret; } @@ -469,19 +435,17 @@ calloc (size_t size, size_t esize) void __collector_heap_record (int mtype, size_t size, void *vaddr) { - int *guard; - Heap_packet hpacket; - if (CHCK_REENTRANCE (guard)) + if (CHCK_REENTRANCE) return; - PUSH_REENTRANCE (guard); - collector_memset (&hpacket, 0, sizeof ( Heap_packet)); - hpacket.comm.tsize = sizeof ( Heap_packet); + PUSH_REENTRANCE; + Heap_packet hpacket = heap_packet0; hpacket.comm.tstamp = gethrtime (); hpacket.mtype = mtype; hpacket.size = (Size_type) size; hpacket.vaddr = (intptr_t) vaddr; - hpacket.comm.frinfo = collector_interface->getFrameInfo (heap_hndl, hpacket.comm.tstamp, FRINFO_FROM_STACK, &hpacket); + hpacket.comm.frinfo = collector_interface->getFrameInfo (heap_hndl, + hpacket.comm.tstamp, FRINFO_FROM_STACK, &hpacket); collector_interface->writeDataRecord (heap_hndl, (Common_packet*) & hpacket); - POP_REENTRANCE (guard); + POP_REENTRANCE; return; } -- 2.39.5