[binutils-gdb] gdb: fix libstdc++ assert caused by invalid use of std::clamp

[binutils-gdb] gdb: fix libstdc++ assert caused by invalid use of std::clamp

[Andrew Burgess]

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=5a22e042e41db962cd6a79cd59cab46cbbe58a98

commit 5a22e042e41db962cd6a79cd59cab46cbbe58a98
Author: Andrew Burgess <aburgess@redhat.com>
Date:   Wed Dec 6 11:16:49 2023 +0000

    gdb: fix libstdc++ assert caused by invalid use of std::clamp
    
    After this commit:
    
      commit 33ae45434d0ab1f7de365b9140ad4e4ffc34b8a2
      Date:   Mon Dec 4 14:23:17 2023 +0000
    
          gdb: Enable early init of thread pool size
    
    I am now seeing this assert from libstdc++:
    
      /usr/include/c++/9/bits/stl_algo.h:3715: constexpr const _Tp& std::clamp(const _Tp&, const _Tp&, const _Tp&) [with _Tp = int]: Assertion '!(__hi < __lo)' failed.
    
    This may only be visible because I compile with:
    
      -D_GLIBCXX_DEBUG=1 -D_GLIBCXX_DEBUG_PEDANTIC=1
    
    but I haven't checked.  The issue the assert is highlighting is real,
    and is caused by this block of code:
    
      if (n_threads < 0)
        {
          const int hardware_threads = std::thread::hardware_concurrency ();
          /* Testing in #29959 indicates that parallel efficiency drops between
             n_threads=5 to 8.  Therefore, clamp the default value to 8 to avoid an
             excessive number of threads in the pool on many-core systems.  */
          const int throttle = 8;
          n_threads = std::clamp (hardware_threads, hardware_threads, throttle);
        }
    
    The arguments to std::clamp are VALUE, LOW, HIGH, but in the above, if
    we have more than 8 hardware threads available the LOW will be greater
    than the HIGH, which is triggering the assert I see above.
    
    I believe std::clamp is the wrong tool to use here.  Instead std::min
    would be a better choice; we want the smaller value of
    HARDWARE_THREADS or THROTTLE.  If h/w threads is 2, then we want 2,
    but if h/w threads is 16 we want 8, this is what std::min gives us.
    
    After this commit, I no longer see the assert.

Diff:
---
 gdb/maint.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/gdb/maint.c b/gdb/maint.c
index 2e6754c9558..68b70bf403d 100644
--- a/gdb/maint.c
+++ b/gdb/maint.c
@@ -855,11 +855,12 @@ update_thread_pool_size ()
   if (n_threads < 0)
     {
       const int hardware_threads = std::thread::hardware_concurrency ();
-      /* Testing in #29959 indicates that parallel efficiency drops between
-	 n_threads=5 to 8.  Therefore, clamp the default value to 8 to avoid an
-	 excessive number of threads in the pool on many-core systems.  */
-      const int throttle = 8;
-      n_threads = std::clamp (hardware_threads, hardware_threads, throttle);
+      /* Testing in PR gdb/29959 indicates that parallel efficiency drops
+	 between n_threads=5 to 8.  Therefore, use no more than 8 threads
+	 to avoid an excessive number of threads in the pool on many-core
+	 systems.  */
+      const int max_thread_count = 8;
+      n_threads = std::min (hardware_threads, max_thread_count);
     }
 
   gdb::thread_pool::g_thread_pool->set_thread_count (n_threads);