Avoid potential race between realloc and clear_hdr_marks/reclaim_generic

GC_realloc might be changing the block size while GC_reclaim_block
or GC_clear_hdr_marks is examining it.  The change to the size field
is benign, in that GC_reclaim (and GC_clear_hdr_marks) would work
correctly with either value, since we are not changing the number
of objects in the block.  But seeing a half-updated value (though
unlikely to occur in practice) could be probably bad.
Using unordered atomic accesses on the size and hb_descr fields would
solve the issue.

* mallocx.c [AO_HAVE_store] (GC_realloc): Use AO_store() to update
hhdr->hb_sz and hhdr->hb_descr; add static assert that size of
hhdr->hb_sz matches that of AO_t; add comment.
* mallocx.c [!AO_HAVE_store] (GC_realloc): Add LOCK/UNLOCK around
hb_sz and hb_descr fields assignment.
* mark.c [AO_HAVE_load] (GC_clear_hdr_marks): Use AO_load() to get
hhdr->hb_sz value; add comment.
* reclaim.c (IS_PTRFREE_SAFE): New macro (uses AO_load() if available).
* reclaim.c (GC_reclaim_generic, GC_reclaim_block): Replace
(hhdr)->hb_descr==0 with IS_PTRFREE_SAFE(hhdr).

1 files changed, 10 insertions, 1 deletions

diff --git a/mark.c b/mark.c
index 5a23366f..63ffd19d 100644
--- a/mark.c
+++ b/mark.c
@@ -145,7 +145,16 @@ GC_INNER GC_bool GC_collection_in_progress(void)
 /* clear all mark bits in the header */
 GC_INNER void GC_clear_hdr_marks(hdr *hhdr)
 {
-    size_t last_bit = FINAL_MARK_BIT((size_t)hhdr->hb_sz);
+  size_t last_bit;
+
+# ifdef AO_HAVE_load
+    /* Atomic access is used to avoid racing with GC_realloc.   */
+    last_bit = FINAL_MARK_BIT((size_t)AO_load((volatile AO_t *)&hhdr->hb_sz));
+# else
+    /* No race as GC_realloc holds the lock while updating hb_sz.   */
+    last_bit = FINAL_MARK_BIT((size_t)hhdr->hb_sz);
+# endif
+
     BZERO(hhdr -> hb_marks, sizeof(hhdr->hb_marks));
     set_mark_bit_from_hdr(hhdr, last_bit);
     hhdr -> hb_n_marks = 0;