Avoid potential race when accessing size_map table

There is again a data race between GC_extend_size_map and GC_size_map[]
readers, though it is again not likely to fail in practice.

It is feasible to just move all of the GC_size_map accesses under the
lock, and this does not look to incur a substantial penalty.

* gcj_mlc.c (GC_gcj_malloc, GC_gcj_malloc_ignore_off_page): Move
lg=GC_size_map[lb] to be right after LOCK() instead of preceding it.
* malloc.c (GC_malloc_kind_global, GC_generic_malloc_uncollectable):
Likewise.
* typd_mlc.c (GC_malloc_explicitly_typed_ignore_off_page): Likewise.
* include/gc.h (GC_get_size_map_at): Update comment to note that the
client should use synchronization when calling the function.
* include/private/gc_priv.h (_GC_arrays._size_map): Add comment about
synchronization.

diff --git a/gcj_mlc.c b/gcj_mlc.c
index 3e450bf..55af7a0 100644 (file)
--- a/gcj_mlc.c
+++ b/gcj_mlc.c
@@ -163,9 +163,10 @@ static void maybe_finalize(void)
 
     GC_DBG_COLLECT_AT_MALLOC(lb);
     if(SMALL_OBJ(lb)) {
-        word lg = GC_size_map[lb];
+        word lg;
 
         LOCK();
+        lg = GC_size_map[lb];
         op = GC_gcjobjfreelist[lg];
         if(EXPECT(0 == op, FALSE)) {
             maybe_finalize();
@@ -236,9 +237,10 @@ GC_API GC_ATTR_MALLOC void * GC_CALL GC_gcj_malloc_ignore_off_page(size_t lb,
 
     GC_DBG_COLLECT_AT_MALLOC(lb);
     if(SMALL_OBJ(lb)) {
-        word lg = GC_size_map[lb];
+        word lg;
 
         LOCK();
+        lg = GC_size_map[lb];
         op = GC_gcjobjfreelist[lg];
         if (EXPECT(0 == op, FALSE)) {
             maybe_finalize();

diff --git a/include/gc.h b/include/gc.h
index cf13c92..c612848 100644 (file)
--- a/include/gc.h
+++ b/include/gc.h
@@ -773,7 +773,9 @@ GC_API size_t GC_CALL GC_get_prof_stats(struct GC_prof_stats_s *,
 /* Get the element value (converted to bytes) at a given index of       */
 /* size_map table which provides requested-to-actual allocation size    */
 /* mapping.  Assumes the collector is initialized.  Returns -1 if the   */
-/* index is out of size_map table bounds. Does not use synchronization. */
+/* index is out of size_map table bounds. Does not use synchronization, */
+/* thus clients should call it using GC_call_with_alloc_lock typically  */
+/* to avoid data races on multiprocessors.                              */
 GC_API size_t GC_CALL GC_get_size_map_at(int i);
 
 /* Count total memory use in bytes by all allocated blocks.  Acquires   */

diff --git a/include/private/gc_priv.h b/include/private/gc_priv.h
index 156bee5..f6a43e2 100644 (file)
--- a/include/private/gc_priv.h
+++ b/include/private/gc_priv.h
@@ -1359,7 +1359,8 @@ struct _GC_arrays {
 # endif
   size_t _size_map[MAXOBJBYTES+1];
         /* Number of granules to allocate when asked for a certain      */
-        /* number of bytes.                                             */
+        /* number of bytes.  Should be accessed with the allocation     */
+        /* lock held.                                                   */
 # ifdef STUBBORN_ALLOC
 #   define GC_sobjfreelist GC_arrays._sobjfreelist
     ptr_t _sobjfreelist[MAXOBJGRANULES+1];

diff --git a/malloc.c b/malloc.c
index 25ca72f..0155fa5 100644 (file)
--- a/malloc.c
+++ b/malloc.c
@@ -300,11 +300,12 @@ GC_API GC_ATTR_MALLOC void * GC_CALL GC_malloc_kind_global(size_t lb, int k)
     if (SMALL_OBJ(lb)) {
         void *op;
         void **opp;
-        size_t lg = GC_size_map[lb];
+        size_t lg;
         DCL_LOCK_STATE;
 
         GC_DBG_COLLECT_AT_MALLOC(lb);
         LOCK();
+        lg = GC_size_map[lb];
         opp = &GC_obj_kinds[k].ok_freelist[lg];
         op = *opp;
         if (EXPECT(op != NULL, TRUE)) {
@@ -365,8 +366,8 @@ GC_API GC_ATTR_MALLOC void * GC_CALL GC_generic_malloc_uncollectable(
         if (EXTRA_BYTES != 0 && lb != 0) lb--;
                   /* We don't need the extra byte, since this won't be  */
                   /* collected anyway.                                  */
-        lg = GC_size_map[lb];
         LOCK();
+        lg = GC_size_map[lb];
         opp = &GC_obj_kinds[k].ok_freelist[lg];
         op = *opp;
         if (EXPECT(op != NULL, TRUE)) {

diff --git a/typd_mlc.c b/typd_mlc.c
index 0092547..e7df05b 100644 (file)
--- a/typd_mlc.c
+++ b/typd_mlc.c
@@ -621,8 +621,8 @@ GC_API GC_ATTR_MALLOC void * GC_CALL
     lb = SIZET_SAT_ADD(lb, TYPD_EXTRA_BYTES);
     if (SMALL_OBJ(lb)) {
         GC_DBG_COLLECT_AT_MALLOC(lb);
-        lg = GC_size_map[lb];
         LOCK();
+        lg = GC_size_map[lb];
         op = GC_eobjfreelist[lg];
         if (EXPECT(0 == op, FALSE)) {
             UNLOCK();