]> granicus.if.org Git - gc/commitdiff
Workaround TSan false positives in extend_size_map
authorIvan Maidanski <ivmai@mail.ru>
Wed, 15 Nov 2017 22:15:14 +0000 (01:15 +0300)
committerIvan Maidanski <ivmai@mail.ru>
Wed, 15 Nov 2017 22:15:14 +0000 (01:15 +0300)
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.

alloc.c
malloc.c

diff --git a/alloc.c b/alloc.c
index e004717dad8bad3ce14db8d21d9e3c45b523bee3..818e21f8422bab29627cf3ff315f312c22317556 100644 (file)
--- a/alloc.c
+++ b/alloc.c
  * 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.
index e1594e835c1c8f41edecf54fa74d2068ab35981e..d2b2aeb7edf2e2d5c7d0eec6a5aa24291b9ee61f 100644 (file)
--- 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.   */