[commit: ghc] ghc-8.0: Fix a crash in requestSync() (8a0485d)

git at git.haskell.org git at git.haskell.org
Thu Aug 25 16:37:10 UTC 2016


Repository : ssh://git@git.haskell.org/ghc

On branch  : ghc-8.0
Link       : http://ghc.haskell.org/trac/ghc/changeset/8a0485d66161c65dd8eeefd11ce37378bfdba089/ghc

>---------------------------------------------------------------

commit 8a0485d66161c65dd8eeefd11ce37378bfdba089
Author: Simon Marlow <smarlow at fb.com>
Date:   Tue May 10 03:22:57 2016 -0700

    Fix a crash in requestSync()
    
    It was possible for a thread to read invalid memory after a conflict
    when multiple threads were synchronising.
    
    I haven't been successful in constructing a test case that triggers
    this, but we have some internal code that ran into it.
    
    (cherry picked from commit ea3d1efb863ca83b28af9056576d47f1abf98fa9)


>---------------------------------------------------------------

8a0485d66161c65dd8eeefd11ce37378bfdba089
 rts/Schedule.c | 23 ++++++++++++++---------
 1 file changed, 14 insertions(+), 9 deletions(-)

diff --git a/rts/Schedule.c b/rts/Schedule.c
index 772748f..632b9d3 100644
--- a/rts/Schedule.c
+++ b/rts/Schedule.c
@@ -1412,25 +1412,30 @@ static void stopAllCapabilities (Capability **pCap, Task *task)
 
 #if defined(THREADED_RTS)
 static rtsBool requestSync (
-    Capability **pcap, Task *task, PendingSync *sync,
+    Capability **pcap, Task *task, PendingSync *new_sync,
     SyncType *prev_sync_type)
 {
-    PendingSync *prev_sync;
+    PendingSync *sync;
 
-    prev_sync = (PendingSync*)cas((StgVolatilePtr)&pending_sync,
-                                  (StgWord)NULL,
-                                  (StgWord)sync);
+    sync = (PendingSync*)cas((StgVolatilePtr)&pending_sync,
+                             (StgWord)NULL,
+                             (StgWord)new_sync);
 
-    if (prev_sync)
+    if (sync != NULL)
     {
+        // sync is valid until we have called yieldCapability().
+        // After the sync is completed, we cannot read that struct any
+        // more because it has been freed.
+        *prev_sync_type = sync->type;
         do {
             debugTrace(DEBUG_sched, "someone else is trying to sync (%d)...",
-                       prev_sync->type);
+                       sync->type);
             ASSERT(*pcap);
             yieldCapability(pcap,task,rtsTrue);
-        } while (pending_sync);
+            sync = pending_sync;
+        } while (sync != NULL);
+
         // NOTE: task->cap might have changed now
-        *prev_sync_type = prev_sync->type;
         return rtsTrue;
     }
     else



More information about the ghc-commits mailing list