* [binutils-gdb] gprofng: fix infinite recursion on calloc with multi-threaded applications
@ 2024-03-25 23:42 Vladimir Mezentsev
0 siblings, 0 replies; only message in thread
From: Vladimir Mezentsev @ 2024-03-25 23:42 UTC (permalink / raw)
To: binutils-cvs
https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=99c3fe52d237eae546d7de484d0cfbd615ac192c
commit 99c3fe52d237eae546d7de484d0cfbd615ac192c
Author: Vladimir Mezentsev <vladimir.mezentsev@oracle.com>
Date: Sat Mar 23 18:31:03 2024 -0700
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 <vladimir.mezentsev@oracle.com>
PR gprofng/31460
* libcollector/heaptrace.c: Use the __thread variable to check for
* reentry. Clean up code.
Diff:
---
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 ("<event kind=\"%s\" id=\"%d\">TSD key not created</event>\n",
- SP_JCMD_CERROR, COL_ERROR_HEAPINIT);
- return COL_ERROR_HEAPINIT;
- }
collector_interface->writeLog ("<profile name=\"%s\">\n", SP_JCMD_HEAPTRACE);
collector_interface->writeLog (" <profdata fname=\"%s\"/>\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;
}
^ permalink raw reply [flat|nested] only message in thread
only message in thread, other threads:[~2024-03-25 23:42 UTC | newest]
Thread overview: (only message) (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-03-25 23:42 [binutils-gdb] gprofng: fix infinite recursion on calloc with multi-threaded applications Vladimir Mezentsev
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).