From: Ivan Maidanski <ivmai@mail.ru>
Date: Wed, 15 Nov 2017 22:15:14 +0000 (+0300)
Subject: Workaround TSan false positives in extend_size_map
X-Git-Tag: v8.0.0~512
X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=e522d6d791680b55825b5f11a4c082eb4770ecf5;p=gc

Workaround TSan false positives in extend_size_map

Thread Sanitizer reports data races between fill_size_map (called from
GC_extend_size_map) and GC_gcj_malloc[_ignore_off_page],
GC_malloc_kind_global, GC_generic_malloc_uncollectable,
GC_malloc_explicitly_typed_ignore_off_page which read a value from
GC_size_map before acquiring the allocation lock.  These races could
be ignored as the value is verified after acquiring the lock.

* alloc.c: Refine comment about GC_allocobj (and GC_size_map) usage
for the case of a multi-threaded environment.
* malloc.c (fill_size_map): New static function (with
GC_ATTR_NO_SANITIZE_THREAD attribute).
* malloc.c (GC_extend_size_map): Use fill_size_map() to fill in
a region of GC_size_map.
---

diff --git a/alloc.c b/alloc.c
index e004717d..818e21f8 100644
--- a/alloc.c
+++ b/alloc.c
@@ -31,12 +31,13 @@
  * The call GC_allocobj(i,k) ensures that the freelist for
  * kind k objects of size i points to a non-empty
  * free list. It returns a pointer to the first entry on the free list.
- * In a single-threaded world, GC_allocobj may be called to allocate
- * an object of small size lb (and NORMAL kind) as follows
+ * If not using thread-local allocation, GC_allocobj may be called to
+ * allocate an object of small size lb (and NORMAL kind) as follows
  * (GC_generic_malloc_inner is a wrapper over GC_allocobj which also
  * fills in GC_size_map if needed):
  *
  *   lg = GC_size_map[lb];
+ *   LOCK();
  *   op = GC_objfreelist[lg];
  *   if (NULL == op) {
  *     op = GC_generic_malloc_inner(lb, NORMAL);
@@ -44,6 +45,7 @@
  *     GC_objfreelist[lg] = obj_link(op);
  *     GC_bytes_allocd += GRANULES_TO_BYTES((word)lg);
  *   }
+ *   UNLOCK();
  *
  * Note that this is very fast if the free list is non-empty; it should
  * only involve the execution of 4 or 5 simple instructions.
diff --git a/malloc.c b/malloc.c
index e1594e83..d2b2aeb7 100644
--- a/malloc.c
+++ b/malloc.c
@@ -102,6 +102,18 @@ STATIC ptr_t GC_alloc_large_and_clear(size_t lb, int k, unsigned flags)
     return result;
 }
 
+/* This function should be called with the allocation lock held.        */
+/* At the same time, it is safe to get a value from GC_size_map not     */
+/* acquiring the allocation lock provided the obtained value is used    */
+/* according to the pattern given in alloc.c file (see the comment      */
+/* about GC_allocobj usage and, e.g., GC_malloc_kind_global code).      */
+static void fill_size_map(size_t low_limit, size_t byte_sz, size_t granule_sz)
+                                                GC_ATTR_NO_SANITIZE_THREAD
+{
+  for (; low_limit <= byte_sz; low_limit++)
+    GC_size_map[low_limit] = granule_sz;
+}
+
 /* Fill in additional entries in GC_size_map, including the i-th one.   */
 /* Note that a filled in section of the array ending at n always        */
 /* has the length of at least n/4.                                      */
@@ -151,8 +163,7 @@ STATIC void GC_extend_size_map(size_t i)
                         /* We may need one extra byte; do not always    */
                         /* fill in GC_size_map[byte_sz].                */
 
-  for (; low_limit <= byte_sz; low_limit++)
-    GC_size_map[low_limit] = granule_sz;
+  fill_size_map(low_limit, byte_sz, granule_sz);
 }
 
 /* Allocate lb bytes for an object of kind k.   */