From 7879d172dec3300341d3eff15808425e01c15569 Mon Sep 17 00:00:00 2001
From: Boris Kolpackov <boris@codesynthesis.com>
Date: Thu, 22 Jun 2023 07:30:31 +0200
Subject: Fix wraparound in task_count logic

---
 libbuild2/operation.cxx | 18 +++++++++---------
 libbuild2/target.cxx    |  5 +++--
 libbuild2/target.ixx    | 15 ++++++++-------
 3 files changed, 20 insertions(+), 18 deletions(-)

(limited to 'libbuild2')

diff --git a/libbuild2/operation.cxx b/libbuild2/operation.cxx
index bafc263..4af03fe 100644
--- a/libbuild2/operation.cxx
+++ b/libbuild2/operation.cxx
@@ -164,8 +164,9 @@ namespace build2
     //
     map.reserve (ctx.targets.size () / 2);
 
-    bool e (false);
+    size_t count_matched (ctx.count_matched ());
 
+    bool e (false);
     for (size_t pass (1); pass != 3; ++pass)
     {
       for (const auto& pt: ctx.targets)
@@ -180,8 +181,7 @@ namespace build2
         //
         const target::opstate& s (t->state[a]);
 
-        if (s.task_count.load (memory_order_relaxed) - ctx.count_base () <
-            target::offset_matched)
+        if (s.task_count.load (memory_order_relaxed) < count_matched)
           continue;
 
         // Skip if for some reason the path is not assigned.
@@ -828,8 +828,8 @@ namespace build2
 
           // Execute unless already executed.
           //
-          if (s.task_count.load (memory_order_relaxed) - base !=
-              target::offset_executed)
+          if (s.task_count.load (memory_order_relaxed) !=
+              base + target::offset_executed)
             pretend_execute (t, pretend_execute);
         };
 
@@ -891,8 +891,8 @@ namespace build2
         // We are only interested in the targets that have been matched for
         // this operation and are in the applied state.
         //
-        if (s.task_count.load (memory_order_relaxed) - base !=
-            target::offset_applied)
+        if (s.task_count.load (memory_order_relaxed) !=
+            base + target::offset_applied)
           continue;
 
         if (s.resolve_counted)
@@ -924,9 +924,9 @@ namespace build2
         // Only consider targets that have been matched for this operation
         // (since matching is what causes the dependents count reset).
         //
-        size_t c (s.task_count.load (memory_order_relaxed) - base);
+        size_t c (s.task_count.load (memory_order_relaxed));
 
-        return (c >= target::offset_applied
+        return (c >= base + target::offset_applied
                 ? s.dependents.load (memory_order_relaxed)
                 : 0);
       };
diff --git a/libbuild2/target.cxx b/libbuild2/target.cxx
index 19edad7..fb47b6d 100644
--- a/libbuild2/target.cxx
+++ b/libbuild2/target.cxx
@@ -1009,9 +1009,10 @@ namespace build2
         const opstate& s (state[action () /* inner */]);
 
         // Note: already synchronized.
-        size_t o (s.task_count.load (memory_order_relaxed) - ctx.count_base ());
+        size_t c (s.task_count.load (memory_order_relaxed));
+        size_t b (ctx.count_base ()); // Note: cannot do (c - b)!
 
-        if (o != offset_applied && o != offset_executed)
+        if (c != (b + offset_applied) && c != (b + offset_executed))
           break;
       }
       // Fall through.
diff --git a/libbuild2/target.ixx b/libbuild2/target.ixx
index a550acb..f0d5cea 100644
--- a/libbuild2/target.ixx
+++ b/libbuild2/target.ixx
@@ -240,8 +240,8 @@ namespace build2
     assert (ctx.phase == run_phase::match ||
             ctx.phase == run_phase::execute);
 
-    const opstate& s (state[a]);
-    size_t c (s.task_count.load (mo) - ctx.count_base ());
+    size_t c (state[a].task_count.load (mo));
+    size_t b (ctx.count_base ()); // Note: cannot do (c - b)!
 
     if (ctx.phase == run_phase::match)
     {
@@ -250,7 +250,7 @@ namespace build2
       // Note that we can't do >= offset_applied since offset_busy means it is
       // being matched.
       //
-      return c == offset_applied || c == offset_executed;
+      return c == (b + offset_applied) || c == (b + offset_executed);
     }
     else
     {
@@ -258,7 +258,7 @@ namespace build2
       // least offset_matched since it must have been "achieved" before the
       // phase switch.
       //
-      return c >= offset_matched;
+      return c >= (b + offset_matched);
     }
   }
 
@@ -298,9 +298,10 @@ namespace build2
 
     // Note: already synchronized.
     //
-    size_t o (s.task_count.load (memory_order_relaxed) - ctx.count_base ());
+    size_t c (s.task_count.load (memory_order_relaxed));
+    size_t b (ctx.count_base ()); // Note: cannot do (c - b)!
 
-    if (o == offset_tried)
+    if (c == (b + offset_tried))
       return make_pair (false, target_state::unknown);
     else
     {
@@ -308,7 +309,7 @@ namespace build2
       // latter case we are guaranteed to be synchronized since we are in the
       // match phase.
       //
-      assert (o == offset_applied || o == offset_executed);
+      assert (c == (b + offset_applied) || c == (b + offset_executed));
       return make_pair (true, (group_state (a) ? group->state[a] : s).state);
     }
   }
-- 
cgit v1.1