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  <[email protected]>

	PR gprofng/31460
	* libcollector/heaptrace.c: Use the __thread variable to check for
	* reentry. Clean up code.

Author: Vladimir Mezentsev

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,74 +248,67 @@ 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 */
 
 void
 free (void *ptr)
 {
-  int *guard;
-  Heap_packet hpacket;
   /* Linux startup workaround  */
   if (!heap_mode)
     {
       // Tprintf(DBG_LT4,"heaptrace: LINUX free(%p)...\n",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,51 +327,48 @@ 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 */
 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;
 }