rts: Add missing write barriers in MVar wake-up pathsghc-8.10

Previously PerformPut failed to respect the non-moving collector's
snapshot invariant, hiding references to an MVar and its new value by
overwriting a stack frame without dirtying the stack. Fix this.

PerformTake exhibited a similar bug, failing to dirty (and therefore
mark) the blocked stack before mutating it.

Closes #20399.

(cherry picked from commit 801978bdfbe635a76e474ea32fd3da83b59325d1)

2 files changed, 28 insertions, 0 deletions

diff --git a/rts/PrimOps.cmm b/rts/PrimOps.cmm
index e073b66ae8..41b3d269bf 100644
--- a/rts/PrimOps.cmm
+++ b/rts/PrimOps.cmm
@@ -1545,6 +1545,23 @@ stg_writeTVarzh (P_ tvar,     /* :: TVar a */
  * exception and never perform its take or put, and we'd end up with a
  * deadlock.
  *
+ * Note [Nonmoving write barrier in Perform{Take,Put}]
+ * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+ * As noted in Note [Non-moving garbage collector] in NonMoving.c, the
+ * non-moving GC requires that all overwritten pointers be pushed to the update
+ * remembered set. In the case of stack mutation this typically happens by
+ * "dirtying" the stack, which eagerly traces the entire stack chunk.
+ *
+ * An exception to this rule is PerformPut, which mutates the stack of a
+ * blocked thread (overwriting an stg_block_putmvar frame). To ensure that the
+ * collector sees the MVar and value reachable from the overwritten frame, we
+ * must push them to the update remembered set. Failing to do so was the cause
+ * of #20399.
+ *
+ * Note that unlike PerformPut, the callers of PerformTake first dirty the
+ * stack prior mutating it (since they introduce a *new*, potentially
+ * inter-generational reference to the stack) and therefore the barrier
+ * described above is unnecessary in this case.
  * -------------------------------------------------------------------------- */
 
 stg_isEmptyMVarzh ( P_ mvar /* :: MVar a */ )
@@ -1573,15 +1590,22 @@ stg_newMVarzh ()
 }
 
 
+// See Note [Nonmoving write barrier in Perform{Put,Take}].
+// Precondition: the stack must be dirtied.
 #define PerformTake(stack, value)               \
     W_ sp;                                      \
     sp = StgStack_sp(stack);                    \
     W_[sp + WDS(1)] = value;                    \
     W_[sp + WDS(0)] = stg_ret_p_info;
 
+// See Note [Nonmoving write barrier in Perform{Put,Take}].
 #define PerformPut(stack,lval)                  \
     W_ sp;                                      \
     sp = StgStack_sp(stack) + WDS(3);           \
+    IF_NONMOVING_WRITE_BARRIER_ENABLED {        \
+      ccall updateRemembSetPushClosure_(BaseReg "ptr", W_[sp - WDS(1)] "ptr"); \
+      ccall updateRemembSetPushClosure_(BaseReg "ptr", W_[sp - WDS(2)] "ptr"); \
+    }                                           \
     StgStack_sp(stack) = sp;                    \
     lval = W_[sp - WDS(1)];
 
diff --git a/rts/sm/NonMoving.c b/rts/sm/NonMoving.c
index 99fd9c1ece..5971cbac20 100644
--- a/rts/sm/NonMoving.c
+++ b/rts/sm/NonMoving.c
@@ -229,6 +229,10 @@ Mutex concurrent_coll_finished_lock;
  *  - Note [StgStack dirtiness flags and concurrent marking] (TSO.h) describes
  *    the protocol for concurrent marking of stacks.
  *
+ *  - Note [Nonmoving write barrier in Perform{Take,Put}] (PrimOps.cmm) describes
+ *    a tricky barrier necessary when resuming threads blocked on MVar
+ *    operations.
+ *
  *  - Note [Static objects under the nonmoving collector] (Storage.c) describes
  *    treatment of static objects.
  *