From de17652e4701d4b128606153aae8e74e28a9851f Mon Sep 17 00:00:00 2001 From: Marco Costalba Date: Mon, 25 Jan 2010 22:18:12 +0100 Subject: [PATCH] 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 --- src/search.cpp | 19 ++++++++++++------- src/thread.h | 2 +- 2 files changed, 13 insertions(+), 8 deletions(-) diff --git a/src/search.cpp b/src/search.cpp index b56cf9ae..7b71849b 100644 --- a/src/search.cpp +++ b/src/search.cpp @@ -2847,17 +2847,22 @@ namespace { if(!Threads[slave].idle || slave == master) return false; - if(Threads[slave].activeSplitPoints == 0) - // No active split points means that the thread is available as a slave - // for any other thread. - return true; + // 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; if(ActiveThreads == 2) return true; - // Apply the "helpful master" concept if possible. - if(SplitPointStack[slave][Threads[slave].activeSplitPoints-1].slaves[master]) - return true; + // 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 fd28ddf7..707f8358 100644 --- 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;