[RFC] libgcc: Integrating emutls and D garbage collector

[Johannes Pfau <johannespfau at gmail dot com>]

Hello all,

I wanted to ask for some feedback on an issue which has affected GDC
for a long time now: Letting the D garbage collector scan
emulated TLS memory. Basically, for every variable instance in TLS
memory we need to be able to get the address and size of that instance,
so the GC can scan this memory for pointers.

Now with libgcc emutls there are essentially two problems:

1) Memory is allocated individually for each variable, so it's not
possible to simply get a memory range as in native TLS.
I can't think of a way to get the required information with the
current API, so I've attached proof-of-concept patch which
implements a __emutls_iterate_memory function. It is however rather
intrusive, so maybe there's a better solution. Currently it keeps
the size for all variables in every threads local memory, but an
optimization would be to keep the size only once in a separate array.

A completely different, less intrusive solution would be to allow the
frontend to specify different emutls functions (e.g.
__d_emutls_get_address instead of __emutls_get_address). The main
drawback here is that linking C/D emutls variables would not be
possible due to the incompatible ABI. However, this is also true
for the other D compilers' (DMD) emutls implementation, so this may
be fine. A D emutls implementation would also be much simpler and
probably faster (we can just allocate using the GC).

(Of course it's way too late for such changes in GCC9, so I'm just
asking for some general feedback.)


2) TLS memory is only freed once a thread exits, but not on unloading of
shared libraries. This problem is aggravated for D, as this old TLS
memory may contain references to the GC heap and prevent that memory
from being collected. However, I think this issue is not that important,
especially as I can't think of a portable solution (one destructor
per variable could work, but seems quite wasteful). 

I'm looking forward to your answers, maybe there's some better solution
I'm not aware of.

Best regards,
Johannes

---
 libgcc/emutls.c          | 118 +++++++++++++++++++++++++++++++++++++--
 libgcc/libgcc-std.ver.in |   5 ++
 2 files changed, 118 insertions(+), 5 deletions(-)

diff --git a/libgcc/emutls.c b/libgcc/emutls.c
index c725142e465..66b74d29f52 100644
--- a/libgcc/emutls.c
+++ b/libgcc/emutls.c
@@ -52,6 +52,8 @@ struct __emutls_array
 
 void *__emutls_get_address (struct __emutls_object *);
 void __emutls_register_common (struct __emutls_object *, word, word, void *);
+typedef void (*iterate_callback)(void* mem, pointer size, void *user);
+void __emutls_iterate_memory (iterate_callback cb, void *user);
 
 #ifdef __GTHREADS
 #ifdef __GTHREAD_MUTEX_INIT
@@ -62,10 +64,111 @@ static __gthread_mutex_t emutls_mutex;
 static __gthread_key_t emutls_key;
 static pointer emutls_size;
 
+static struct __emutls_array *emutls_arrays;
+
+static void
+emutls_array_register (struct __emutls_array *arr)
+{
+  __gthread_mutex_lock (&emutls_mutex);
+
+  if (emutls_arrays == NULL)
+    {
+      emutls_arrays = calloc (32 + 1, sizeof (void *));
+      emutls_arrays->size = 32;
+    }
+
+  // Try to write to an empty slot
+  pointer slot_index = 0;
+  for (; slot_index < emutls_arrays->size; slot_index++)
+    {
+      if (emutls_arrays->data[slot_index] == NULL)
+	{
+	  emutls_arrays->data[slot_index] = (void *) arr;
+	  break;
+	}
+    }
+  // No empty slot?
+  if (slot_index == emutls_arrays->size)
+    {
+      emutls_arrays = realloc (emutls_arrays, (slot_index + 2) * sizeof (void *));
+      if (emutls_arrays == NULL)
+	abort ();
+      emutls_arrays->size = slot_index + 1;
+      emutls_arrays->data[slot_index] = (void *) arr;
+    }
+
+  __gthread_mutex_unlock (&emutls_mutex);
+}
+
+static void
+emutls_array_update (struct __emutls_array *old, struct __emutls_array *updated)
+{
+  if (updated == old)
+    return;
+
+  __gthread_mutex_lock (&emutls_mutex);
+
+  for (pointer slot_index = 0; slot_index < emutls_arrays->size; slot_index++)
+    {
+      if (emutls_arrays->data[slot_index] == (void *) old)
+        emutls_arrays->data[slot_index] = (void *) updated;
+    }
+
+  __gthread_mutex_unlock (&emutls_mutex);
+}
+
+static void
+emutls_array_unregister (struct __emutls_array *arr)
+{
+  __gthread_mutex_lock (&emutls_mutex);
+
+  for (pointer slot_index = 0; slot_index < emutls_arrays->size; slot_index++)
+    {
+      if (emutls_arrays->data[slot_index] == (void *) arr)
+        emutls_arrays->data[slot_index] = NULL;
+    }
+
+  __gthread_mutex_unlock (&emutls_mutex);
+}
+
+static void
+emutls_array_iterate (struct __emutls_array *arr, iterate_callback cb, void *user)
+{
+  if (arr == NULL)
+    return;
+
+  for (pointer i = 0; i < arr->size; i++)
+    {
+      void *ptr = arr->data[i];
+      if (ptr)
+	{
+	  pointer size = ((pointer*) ptr)[-2];
+	  cb (ptr, size, user);
+	}
+    }
+}
+
+void
+__emutls_iterate_memory (iterate_callback cb, void *user)
+{
+  __gthread_mutex_lock (&emutls_mutex);
+  if (emutls_arrays == NULL)
+    return;
+
+  for (pointer slot_index = 0; slot_index < emutls_arrays->size; slot_index++)
+    {
+      struct __emutls_array *arr = (struct __emutls_array *) emutls_arrays->data[slot_index];
+      emutls_array_iterate (arr, cb, user);
+    }
+
+  __gthread_mutex_unlock (&emutls_mutex);
+}
+
 static void
 emutls_destroy (void *ptr)
 {
   struct __emutls_array *arr = ptr;
+  emutls_array_unregister (arr);
   pointer size = arr->size;
   pointer i;
 
@@ -99,19 +202,21 @@ emutls_alloc (struct __emutls_object *obj)
      emutls_destroy accordingly.  */
   if (obj->align <= sizeof (void *))
     {
-      ptr = malloc (obj->size + sizeof (void *));
+      ptr = malloc (obj->size + 2 * sizeof (void *));
       if (ptr == NULL)
 	abort ();
-      ((void **) ptr)[0] = ptr;
-      ret = ptr + sizeof (void *);
+      ((pointer *) ptr)[0] = obj->size;
+      ((void **) ptr)[1] = ptr;
+      ret = ptr + 2 * sizeof (void *);
     }
   else
     {
-      ptr = malloc (obj->size + sizeof (void *) + obj->align - 1);
+      ptr = malloc (obj->size + 2 * sizeof (void *) + obj->align - 1);
       if (ptr == NULL)
 	abort ();
-      ret = (void *) (((pointer) (ptr + sizeof (void *) + obj->align - 1))
+      ret = (void *) (((pointer) (ptr + 2 * sizeof (void *) + obj->align - 1))
 		      & ~(pointer)(obj->align - 1));
+      ((pointer *) ret)[-2] = obj->size;
       ((void **) ret)[-1] = ptr;
     }
 
@@ -161,9 +266,11 @@ __emutls_get_address (struct __emutls_object *obj)
 	abort ();
       arr->size = size;
       __gthread_setspecific (emutls_key, (void *) arr);
+      emutls_array_register (arr);
     }
   else if (__builtin_expect (offset > arr->size, 0))
     {
+      struct __emutls_array *orig_arr = arr;
       pointer orig_size = arr->size;
       pointer size = orig_size * 2;
       if (offset > size)
@@ -175,6 +282,7 @@ __emutls_get_address (struct __emutls_object *obj)
       memset (arr->data + orig_size, 0,
 	      (size - orig_size) * sizeof (void *));
       __gthread_setspecific (emutls_key, (void *) arr);
+      emutls_array_update (orig_arr, arr);
     }
 
   void *ret = arr->data[offset - 1];
diff --git a/libgcc/libgcc-std.ver.in b/libgcc/libgcc-std.ver.in
index d5352b6f74e..b7c016f6a85 100644
--- a/libgcc/libgcc-std.ver.in
+++ b/libgcc/libgcc-std.ver.in
@@ -1944,3 +1944,8 @@ GCC_7.0.0 {
   __PFX__divmoddi4
   __PFX__divmodti4
 }
+
+%inherit GCC_9.0.0 GCC_7.0.0
+GCC_9.0.0 {
+  __emutls_iterate_memory
+}
-- 
2.19.2