Fix a possible crash in thread_is_available()

When we have more then 2 threads then we do an array
access with index 'Threads[slave].activeSplitPoints - 1'
This should be >= 0 because we tested the variable just
few statements before, but because is a shared variable
it could be that the 'slave' thread set the value to zero
just after we test it, so that when we use the decremented
variable for array access we crash.

Bug spotted by Bruno Causse.

Signed-off-by: Marco Costalba <mcostalba@gmail.com>

diff --git a/src/search.cpp b/src/search.cpp
index d54c7ebd91fbcc5379974d7556bf14a86fff2f8a..7c341f878f7200d321343567c249ec8fcd8964f0 100644 (file)
--- a/src/search.cpp
+++ b/src/search.cpp
@@ -2898,7 +2898,10 @@ namespace {
     if (!Threads[slave].idle || slave == master)
         return false;
 
-    if (Threads[slave].activeSplitPoints == 0)
+    // Make a local copy to be sure doesn't change under our feet
+    int localActiveSplitPoints = Threads[slave].activeSplitPoints;
+
+    if (localActiveSplitPoints == 0)
         // No active split points means that the thread is available as
         // a slave for any other thread.
         return true;
@@ -2906,8 +2909,10 @@ namespace {
     if (ActiveThreads == 2)
         return true;
 
-    // Apply the "helpful master" concept if possible
-    if (SplitPointStack[slave][Threads[slave].activeSplitPoints - 1].slaves[master])
+    // Apply the "helpful master" concept if possible. Use localActiveSplitPoints
+    // that is known to be > 0, instead of Threads[slave].activeSplitPoints that
+    // could have been set to 0 by another thread leading to an out of bound access.
+    if (SplitPointStack[slave][localActiveSplitPoints - 1].slaves[master])
         return true;
 
     return false;

diff --git a/src/thread.h b/src/thread.h
index d12ec845c01f4bc4a288bd6bfe076b8652ca9b5b..d0c1d8e9519edd6d7966f86cbdd9da6f4b5415d2 100644 (file)
--- a/src/thread.h
+++ b/src/thread.h
@@ -64,7 +64,7 @@ struct SplitPoint {
 
 struct Thread {
   SplitPoint *splitPoint;
-  int activeSplitPoints;
+  volatile int activeSplitPoints;
   uint64_t nodes;
   uint64_t betaCutOffs[2];
   bool failHighPly1;