[Git][ghc/ghc][wip/tsan/all] More races in freeGroup

[Ben Gamari]

Ben Gamari pushed to branch wip/tsan/all at Glasgow Haskell Compiler / GHC


Commits:
e3a7e8bd by Ben Gamari at 2020-10-31T11:42:53-04:00
More races in freeGroup

- - - - -


1 changed file:

- rts/sm/BlockAlloc.c


Changes:

=====================================
rts/sm/BlockAlloc.c
=====================================
@@ -787,6 +787,26 @@ free_mega_group (bdescr *mg)
 }
 
 
+/* Note [Data races in freeGroup]
+ * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+ * freeGroup commits a rather serious concurrency sin in its block coalescence
+ * logic: When freeing a block it looks at bd->free of the previous/next block
+ * to see whether it is allocated. However, the free'ing thread likely does not
+ * own the previous/next block, nor do we make any attempt to synchronize with
+ * the thread that *does* own it; this makes this access a data race.
+ *
+ * The original design argued that this was correct because `bd->free` will
+ * only take a value of -1 when the block is free and thereby owned by the
+ * storage manager. However, this is nevertheless unsafe under the C11 data
+ * model, which guarantees no particular semantics for data races.
+ *
+ * We currently assume (and hope) we won't see torn values and consequently
+ * we will never see `bd->free == -1` for an allocated block which we do not
+ * own. However, this is all extremely dodgy.
+ *
+ * This is tracked as #18913.
+ */
+
 void
 freeGroup(bdescr *p)
 {
@@ -834,6 +854,9 @@ freeGroup(bdescr *p)
   {
       bdescr *next;
       next = p + p->blocks;
+
+      // See Note [Data races in freeGroup].
+      TSAN_ANNOTATE_BENIGN_RACE(&next->free, "freeGroup");
       if (next <= LAST_BDESCR(MBLOCK_ROUND_DOWN(p))
           && RELAXED_LOAD(&next->free) == (P_)-1)
       {
@@ -856,11 +879,7 @@ freeGroup(bdescr *p)
       prev = p - 1;
       if (prev->blocks == 0) prev = prev->link; // find the head
 
-      // This is a bit hairy... we are looking at bd->free of a block that we
-      // do not own and yet have not synchronized with the thread that *does*
-      // own the block. We currently assume we won't see torn values and
-      // consequently this will work, although this is strictly speaking not
-      // guaranteed under the memory model.
+      // See Note [Data races in freeGroup].
       TSAN_ANNOTATE_BENIGN_RACE(&prev->free, "freeGroup");
       if (RELAXED_LOAD(&prev->free) == (P_)-1)
       {



View it on GitLab: https://gitlab.haskell.org/ghc/ghc/-/commit/e3a7e8bd76a817d0a5cab10f5b098153624b7585

-- 
View it on GitLab: https://gitlab.haskell.org/ghc/ghc/-/commit/e3a7e8bd76a817d0a5cab10f5b098153624b7585
You're receiving this email because of your account on gitlab.haskell.org.


-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.haskell.org/pipermail/ghc-commits/attachments/20201031/7185e207/attachment-0001.html>