* [PATCH 0/2] Cleanup and bug fix related to thread iteration and proceed calls
@ 2026-08-06 20:53 Andrew Burgess
2026-08-06 20:54 ` [PATCH 1/2] gdb: use inferior::threads instead of all_threads and a PID filter Andrew Burgess
2026-08-06 20:54 ` [PATCH 2/2] gdb: share some thread proceed related code between CLI and MI Andrew Burgess
0 siblings, 2 replies; 10+ messages in thread
From: Andrew Burgess @ 2026-08-06 20:53 UTC (permalink / raw)
To: gdb-patches; +Cc: Andrew Burgess
After commit:
commit 2e9aaf9455118cee476ea6c4e6aa78477fa84a8e
Date: Wed May 13 09:57:06 2026 -0600
Remove for_each_thread
I spotted a couple of places where we could simplify the code by not
iterating over all_threads, this is patch #1.
And this led me to spot an actual bug in the MI code. Not a bug
introduced by the above commit, this is something that has been in GDB
for a while. This is patch #2.
Thanks,
Andrew
---
Andrew Burgess (2):
gdb: use inferior::threads instead of all_threads and a PID filter
gdb: share some thread proceed related code between CLI and MI
gdb/infcmd.c | 62 ++++++++----
gdb/inferior.h | 11 +++
gdb/mi/mi-main.c | 49 +++-------
gdb/testsuite/gdb.mi/mi-corefile-and-live.c | 37 +++++++
gdb/testsuite/gdb.mi/mi-corefile-and-live.exp | 97 +++++++++++++++++++
5 files changed, 202 insertions(+), 54 deletions(-)
create mode 100644 gdb/testsuite/gdb.mi/mi-corefile-and-live.c
create mode 100644 gdb/testsuite/gdb.mi/mi-corefile-and-live.exp
base-commit: f94273aeb37908c32aa26530ff56e862f0060daf
--
2.25.4
^ permalink raw reply [flat|nested] 10+ messages in thread* [PATCH 1/2] gdb: use inferior::threads instead of all_threads and a PID filter 2026-08-06 20:53 [PATCH 0/2] Cleanup and bug fix related to thread iteration and proceed calls Andrew Burgess @ 2026-08-06 20:54 ` Andrew Burgess 2026-08-07 7:59 ` Tom de Vries 2026-08-06 20:54 ` [PATCH 2/2] gdb: share some thread proceed related code between CLI and MI Andrew Burgess 1 sibling, 1 reply; 10+ messages in thread From: Andrew Burgess @ 2026-08-06 20:54 UTC (permalink / raw) To: gdb-patches; +Cc: Andrew Burgess I noticed a few places where we iterated over `all_threads ()` but then immediately filtered by PID. It would be better to instead iterate over `inferior::threads ()` and remove the PID filtering. These should be equivalent as a single inferior should contain every thread with a given PID, and should only contain threads with that given PID. While editing the `for (...)` loops I have replaced 'auto' with 'thread_info' for additional type clarity. There should be no user visible changes after this commit. --- gdb/mi/mi-main.c | 18 +++++++----------- 1 file changed, 7 insertions(+), 11 deletions(-) diff --git a/gdb/mi/mi-main.c b/gdb/mi/mi-main.c index 8b6da41ffeb..903c6a5f411 100644 --- a/gdb/mi/mi-main.c +++ b/gdb/mi/mi-main.c @@ -359,14 +359,11 @@ mi_cmd_exec_interrupt (const char *command, const char *const *argv, int argc) scoped_disable_commit_resumed disable_commit_resumed ("interrupting all threads of thread group"); - for (auto &thread : all_threads ()) + for (thread_info &thread : inf->threads ()) { if (thread.state () != THREAD_RUNNING) continue; - if (thread.ptid.pid () != inf->pid) - continue; - target_stop (thread.ptid); } } @@ -603,14 +600,13 @@ print_one_inferior (struct inferior *inferior, bool recurse, if (inferior->pid != 0) { - for (auto &ti : all_threads ()) - if (ti.ptid.pid () == inferior->pid) - { - int core = target_core_of_thread (ti.ptid); + for (thread_info &ti : inferior->threads ()) + { + int core = target_core_of_thread (ti.ptid); - if (core != -1) - cores.insert (core); - } + if (core != -1) + cores.insert (core); + } } if (!cores.empty ()) -- 2.25.4 ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/2] gdb: use inferior::threads instead of all_threads and a PID filter 2026-08-06 20:54 ` [PATCH 1/2] gdb: use inferior::threads instead of all_threads and a PID filter Andrew Burgess @ 2026-08-07 7:59 ` Tom de Vries 0 siblings, 0 replies; 10+ messages in thread From: Tom de Vries @ 2026-08-07 7:59 UTC (permalink / raw) To: Andrew Burgess, gdb-patches On 8/6/26 10:54 PM, Andrew Burgess wrote: > I noticed a few places where we iterated over `all_threads ()` but > then immediately filtered by PID. It would be better to instead > iterate over `inferior::threads ()` and remove the PID filtering. > > These should be equivalent as a single inferior should contain every > thread with a given PID, and should only contain threads with that > given PID. > > While editing the `for (...)` loops I have replaced 'auto' with > 'thread_info' for additional type clarity. > > There should be no user visible changes after this commit. Hi Andrew, this LGTM. FWIW, I had a Claude Code session review this, and I asked it to find other locations in need of the same treatment, and it found sync_threadlists in gdb/aix-thread.c, where we can replace all_threads_safe with inferior::threads_safe (). But that requires testing on AIX, so I suppose it's not something that is easily added to this patch. Approved-By: Tom de Vries <tdevries@suse.de> Thanks, - Tom > --- > gdb/mi/mi-main.c | 18 +++++++----------- > 1 file changed, 7 insertions(+), 11 deletions(-) > > diff --git a/gdb/mi/mi-main.c b/gdb/mi/mi-main.c > index 8b6da41ffeb..903c6a5f411 100644 > --- a/gdb/mi/mi-main.c > +++ b/gdb/mi/mi-main.c > @@ -359,14 +359,11 @@ mi_cmd_exec_interrupt (const char *command, const char *const *argv, int argc) > scoped_disable_commit_resumed disable_commit_resumed > ("interrupting all threads of thread group"); > > - for (auto &thread : all_threads ()) > + for (thread_info &thread : inf->threads ()) > { > if (thread.state () != THREAD_RUNNING) > continue; > > - if (thread.ptid.pid () != inf->pid) > - continue; > - > target_stop (thread.ptid); > } > } > @@ -603,14 +600,13 @@ print_one_inferior (struct inferior *inferior, bool recurse, > > if (inferior->pid != 0) > { > - for (auto &ti : all_threads ()) > - if (ti.ptid.pid () == inferior->pid) > - { > - int core = target_core_of_thread (ti.ptid); > + for (thread_info &ti : inferior->threads ()) > + { > + int core = target_core_of_thread (ti.ptid); > > - if (core != -1) > - cores.insert (core); > - } > + if (core != -1) > + cores.insert (core); > + } > } > > if (!cores.empty ()) ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 2/2] gdb: share some thread proceed related code between CLI and MI 2026-08-06 20:53 [PATCH 0/2] Cleanup and bug fix related to thread iteration and proceed calls Andrew Burgess 2026-08-06 20:54 ` [PATCH 1/2] gdb: use inferior::threads instead of all_threads and a PID filter Andrew Burgess @ 2026-08-06 20:54 ` Andrew Burgess 2026-08-07 9:20 ` Tom de Vries 2026-08-07 10:14 ` Tom de Vries 1 sibling, 2 replies; 10+ messages in thread From: Andrew Burgess @ 2026-08-06 20:54 UTC (permalink / raw) To: gdb-patches; +Cc: Andrew Burgess I noticed that some code related to proceeding threads could be shared between CLI and MI. This fixes a bug as the CLI code contains a fix that the MI code is missing. In continue_1 (in infcmd.c) we have a loop that iterates over all threads looking for threads that are THREAD_STOPPED and are in an inferior that has_execution. For each thread found we then call: switch_to_thread (&thread); clear_proceed_status (0); proceed ((CORE_ADDR) -1, GDB_SIGNAL_DEFAULT); In exec_continue (in mi/mi-main.c) we also have a loop over all threads that calls the proceed_thread helper function which skips threads that are not THREAD_STOPPED, does some PID related filtering (more on this later) and then calls the same three functions: switch_to_thread, clear_proceed_status, proceed. The PID filtering mentioned above allows proceed_thread to do two jobs, if the PID is zero then we proceed all threads. If PID is non-zero then we proceed only the threads in the inferior with that PID. You might also have spotted that in continue_1 we checked if the inferior has execution or not. This check was added in commit: commit 5b6d1e4fa4fc6827c7b3f0e99ff120dfa14d65d2 Date: Fri Jan 10 20:06:08 2020 +0000 Multi-target support A matching check was not added into the MI at this point, nor did the commit message mention why such a check was not added. I'm choosing to believe that this was an oversight in the 5b6d1e4fa4fc6827 commit. And this is the bug I mentioned above. If we call `proceed` with a thread that is part of an inferior that does not "has_execution" then the thread will be marked running even though it will never actually be set running. See the early return at the top of `proceed_resume_thread_checked` and the call to set_state in `proceed`. I do worry that there might be a bigger set of bugs here if proceed can set a thread's state to THREAD_RUNNING, but then never actually sets the underlying thread running. But in this case, just having the MI share code with the CLI means that we pick up the fix for this case basically for free. I propose adding a new global helper function `proceed_one_thread`, this will check if the thread is THREAD_STOPPED and is in an inferior which has_execution. If these conditions are met then the three functions mentioned above will be called to proceed the thread. I will then add a second new function `proceed_all_threads`, this will iterate over all threads and call proceed_one_thread. We can then use proceed_all_threads from continue_1, replacing the existing loop. In exec_continue we can move the PID (or rather inferior) check earlier, outside the loop. If we want to resume all threads (the old PID is zero path) then we call proceed_all_threads. If we only want to proceed threads with one PID then we loop over threads in the matching inferior and call proceed_one_thread on each. As the code I am factoring out is all within non_stop only paths I have added `gdb_assert (non_stop);` to each of the new helper functions, this will prevent these functions accidentally being called in the all_stop code path. While moving the two `for (...)` loops I have replaced 'auto' with 'thread_info' for additional type clarity. With the exception of the new has_execution check in the MI path there should be no other user visible changes with this commit. I've added a new test which exposes the missing has_execution check issue. --- gdb/infcmd.c | 62 ++++++++---- gdb/inferior.h | 11 +++ gdb/mi/mi-main.c | 31 ++---- gdb/testsuite/gdb.mi/mi-corefile-and-live.c | 37 +++++++ gdb/testsuite/gdb.mi/mi-corefile-and-live.exp | 97 +++++++++++++++++++ 5 files changed, 195 insertions(+), 43 deletions(-) create mode 100644 gdb/testsuite/gdb.mi/mi-corefile-and-live.c create mode 100644 gdb/testsuite/gdb.mi/mi-corefile-and-live.exp diff --git a/gdb/infcmd.c b/gdb/infcmd.c index 3e943123519..23e93587874 100644 --- a/gdb/infcmd.c +++ b/gdb/infcmd.c @@ -722,6 +722,46 @@ ensure_not_running (void) error_is_running (); } +/* See inferior.h. */ + +void +proceed_one_thread (thread_info &thread) +{ + gdb_assert (non_stop); + + if (thread.state () != THREAD_STOPPED) + return; + + if (!thread.inf->has_execution ()) + return; + + switch_to_thread (&thread); + clear_proceed_status (0); + proceed ((CORE_ADDR) -1, GDB_SIGNAL_DEFAULT); +} + +/* See inferior.h. */ + +void +proceed_all_threads () +{ + gdb_assert (non_stop); + + for (thread_info &thread : all_threads ()) + { + /* We go through all threads individually instead of compressing + into a single target `resume_all' request, because some threads + may be stopped in internal breakpoints/events, or stopped waiting + for its turn in the displaced stepping queue (that is, they are + running from the user's perspective but internally stopped). The + target side has no idea about why the thread is stopped, so a + `resume_all' command would resume too much. If/when GDB gains a + way to tell the target `hold this thread stopped until I say + otherwise', then we can optimize this. */ + proceed_one_thread (thread); + } +} + void continue_1 (bool all_threads_p) { @@ -739,27 +779,7 @@ continue_1 (bool all_threads_p) scoped_disable_commit_resumed disable_commit_resumed ("continue all threads in non-stop"); - for (auto &thread : all_threads ()) - { - /* We go through all threads individually instead of compressing - into a single target `resume_all' request, because some threads - may be stopped in internal breakpoints/events, or stopped waiting - for its turn in the displaced stepping queue (that is, they are - running from the user's perspective but internally stopped). The - target side has no idea about why the thread is stopped, so a - `resume_all' command would resume too much. If/when GDB gains a - way to tell the target `hold this thread stopped until I say - otherwise', then we can optimize this. */ - if (thread.state () != THREAD_STOPPED) - continue; - - if (!thread.inf->has_execution ()) - continue; - - switch_to_thread (&thread); - clear_proceed_status (0); - proceed ((CORE_ADDR) -1, GDB_SIGNAL_DEFAULT); - } + proceed_all_threads (); if (current_ui->prompt_state == PROMPT_BLOCKED) { diff --git a/gdb/inferior.h b/gdb/inferior.h index 217741adc07..8255654d9b8 100644 --- a/gdb/inferior.h +++ b/gdb/inferior.h @@ -227,6 +227,17 @@ extern void registers_info (const char *, bool); extern void continue_1 (bool all_threads_p); +/* For use only when non_stop is true. Proceed all threads in every + inferior. */ + +extern void proceed_all_threads (); + +/* For use only when non_stop is true. If THREAD is stopped, and is in an + inferior that has_execution then switch to THREAD, clear its proceed + status, and proceed the thread. */ + +extern void proceed_one_thread (thread_info &thread); + extern void interrupt_target_1 (bool all_threads); using delete_longjmp_breakpoint_cleanup diff --git a/gdb/mi/mi-main.c b/gdb/mi/mi-main.c index 903c6a5f411..e110b029e1e 100644 --- a/gdb/mi/mi-main.c +++ b/gdb/mi/mi-main.c @@ -235,20 +235,6 @@ mi_cmd_exec_jump (const char *args, const char *const *argv, int argc) mi_execute_async_cli_command ("jump", argv, argc); } -static void -proceed_thread (struct thread_info *thread, int pid) -{ - if (thread->state () != THREAD_STOPPED) - return; - - if (pid != 0 && thread->ptid.pid () != pid) - return; - - switch_to_thread (thread); - clear_proceed_status (0); - proceed ((CORE_ADDR) -1, GDB_SIGNAL_DEFAULT); -} - static void exec_continue (const char *const *argv, int argc) { @@ -262,24 +248,25 @@ exec_continue (const char *const *argv, int argc) all threads in all inferiors, we need to iterate over threads. - See comment on infcmd.c:proceed_thread_callback for rationale. */ + See comment in infcmd.c:proceed_all_threads for rationale. */ if (current_context->all || current_context->thread_group != -1) { scoped_restore_current_thread restore_thread; scoped_disable_commit_resumed disable_commit_resumed ("MI continue all threads in non-stop"); - int pid = 0; + inferior *inf = nullptr; if (!current_context->all) - { - struct inferior *inf - = find_inferior_id (current_context->thread_group); + inf = find_inferior_id (current_context->thread_group); - pid = inf->pid; + if (inf == nullptr) + proceed_all_threads (); + else + { + for (thread_info &thread : inf->threads ()) + proceed_one_thread (thread); } - for (auto &thread : all_threads ()) - proceed_thread (&thread, pid); disable_commit_resumed.reset_and_commit (); } else diff --git a/gdb/testsuite/gdb.mi/mi-corefile-and-live.c b/gdb/testsuite/gdb.mi/mi-corefile-and-live.c new file mode 100644 index 00000000000..831c7f0de7f --- /dev/null +++ b/gdb/testsuite/gdb.mi/mi-corefile-and-live.c @@ -0,0 +1,37 @@ +/* Copyright 2026 Free Software Foundation, Inc. + + This file is part of GDB. + + This program is free software; you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation; either version 3 of the License, or + (at your option) any later version. + + This program is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with this program. If not, see <http://www.gnu.org/licenses/>. */ + +#include <stdlib.h> + +void +bar (void) +{ + abort (); +} + +void +foo (void) +{ + bar (); +} + +int +main (int argc, char **argv) +{ + foo (); + return 0; +} diff --git a/gdb/testsuite/gdb.mi/mi-corefile-and-live.exp b/gdb/testsuite/gdb.mi/mi-corefile-and-live.exp new file mode 100644 index 00000000000..a3a57d3c81a --- /dev/null +++ b/gdb/testsuite/gdb.mi/mi-corefile-and-live.exp @@ -0,0 +1,97 @@ +# Copyright 2026 Free Software Foundation, Inc. +# +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 3 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see <http://www.gnu.org/licenses/>. + +# In non-stop mode with schedule-multiple turned on, create two +# inferiors, a core target and one other. Then use the command +# '-exec-continue --all'. +# +# The core target threads should not be marked as running by this. + +load_lib mi-support.exp +set MIFLAGS "-i=mi" + +require isnative +require allow_multi_inferior_tests + +standard_testfile + +if {[build_executable "build executable" $testfile $srcfile] == -1} { + return +} + +set corefile [core_find $binfile] +if {$corefile == ""} { + untested "unable to create or find corefile" + return +} + +# Start GDB in non-stop and schedule-multiple mode. +save_vars { GDBFLAGS } { + append GDBFLAGS " -ex \"set non-stop on\"" + append GDBFLAGS " -ex \"set schedule-multiple on\"" + mi_clean_restart $::testfile +} + +if {[mi_runto_main] == -1} { + return +} + +# Arrange for inferior 1, our non core file inferior, to stop before +# it hits the abort call. +mi_create_breakpoint "-g i1 foo" \ + "set breakpoint on foo" \ + -inferior 1 + +# Setup inferior 2, this will load the core file. +mi_gdb_test "-add-inferior" \ + [multi_line "=thread-group-added,id=\"i2\"" \ + "~\"\\\[New inferior 2\\\]\\\\n\"" \ + "\~\"Added inferior 2\[^\r\n\]*\\\\n\"" \ + "\\^done,inferior=\"\[^\"\]+\"(?:,connection={.*})?" ] \ + "add inferior 2" + +# Set the executable for inferior 2. +mi_gdb_test "-file-exec-and-symbols --thread-group i2 $::binfile" \ + "\\^done" \ + "set executable of inferior 2" + +# Load the core file into inferior 2. +mi_gdb_test \ + "-target-select --thread-group i2 core $::corefile" \ + [multi_line \ + "=thread-group-started,id=\"i2\",.*" \ + "=thread-created,id=\"2\",group-id=\"i2\"" \ + ".*\\^connected,frame=.*"] \ + "load core file in inferior 2" + +# Check the core file thread is initially shown as stopped. +mi_gdb_test "-thread-info 2" ".*,state=\"stopped\".*" \ + "core file thread is initially stopped" + +# Resume "all" threads. As the core target doesn't support execution +# this should not try to set the core target threads running. +mi_gdb_test "-exec-continue --all" \ + [multi_line \ + "\\^running" \ + "\\*running,thread-id=\"1\""] \ + "resume all" + +# Wait for the non-core target thread to stop. +mi_expect_stop "breakpoint-hit" \ + "foo" ".*" ".*" ".*" {"" "disp=\"keep\""} "w1,i2 stop" + +# Check that the core target thread is still showing as stopped. +mi_gdb_test "-thread-info 2" ".*,state=\"stopped\".*" \ + "core file thread is still stopped" -- 2.25.4 ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] gdb: share some thread proceed related code between CLI and MI 2026-08-06 20:54 ` [PATCH 2/2] gdb: share some thread proceed related code between CLI and MI Andrew Burgess @ 2026-08-07 9:20 ` Tom de Vries 2026-08-07 15:40 ` Andrew Burgess 2026-08-07 10:14 ` Tom de Vries 1 sibling, 1 reply; 10+ messages in thread From: Tom de Vries @ 2026-08-07 9:20 UTC (permalink / raw) To: Andrew Burgess, gdb-patches [-- Attachment #1: Type: text/plain, Size: 4845 bytes --] On 8/6/26 10:54 PM, Andrew Burgess wrote: > I noticed that some code related to proceeding threads could be shared > between CLI and MI. This fixes a bug as the CLI code contains a fix > that the MI code is missing. > > In continue_1 (in infcmd.c) we have a loop that iterates over all > threads looking for threads that are THREAD_STOPPED and are in an > inferior that has_execution. For each thread found we then call: > > switch_to_thread (&thread); > clear_proceed_status (0); > proceed ((CORE_ADDR) -1, GDB_SIGNAL_DEFAULT); > > In exec_continue (in mi/mi-main.c) we also have a loop over all > threads that calls the proceed_thread helper function which skips > threads that are not THREAD_STOPPED, does some PID related > filtering (more on this later) and then calls the same three > functions: switch_to_thread, clear_proceed_status, proceed. > > The PID filtering mentioned above allows proceed_thread to do two > jobs, if the PID is zero then we proceed all threads. If PID is > non-zero then we proceed only the threads in the inferior with that > PID. > > You might also have spotted that in continue_1 we checked if the > inferior has execution or not. This check was added in commit: > > commit 5b6d1e4fa4fc6827c7b3f0e99ff120dfa14d65d2 > Date: Fri Jan 10 20:06:08 2020 +0000 > > Multi-target support > > A matching check was not added into the MI at this point, nor did the > commit message mention why such a check was not added. I'm choosing > to believe that this was an oversight in the 5b6d1e4fa4fc6827 commit. > And this is the bug I mentioned above. > > If we call `proceed` with a thread that is part of an inferior that > does not "has_execution" then the thread will be marked running even > though it will never actually be set running. See the early return at > the top of `proceed_resume_thread_checked` and the call to set_state > in `proceed`. > > I do worry that there might be a bigger set of bugs here if proceed > can set a thread's state to THREAD_RUNNING, but then never actually > sets the underlying thread running. But in this case, just having the > MI share code with the CLI means that we pick up the fix for this case > basically for free. > > I propose adding a new global helper function `proceed_one_thread`, > this will check if the thread is THREAD_STOPPED and is in an inferior > which has_execution. If these conditions are met then the three > functions mentioned above will be called to proceed the thread. > > I will then add a second new function `proceed_all_threads`, this will > iterate over all threads and call proceed_one_thread. > > We can then use proceed_all_threads from continue_1, replacing the > existing loop. > > In exec_continue we can move the PID (or rather inferior) check > earlier, outside the loop. If we want to resume all threads (the old > PID is zero path) then we call proceed_all_threads. If we only want > to proceed threads with one PID then we loop over threads in the > matching inferior and call proceed_one_thread on each. > > As the code I am factoring out is all within non_stop only paths I > have added `gdb_assert (non_stop);` to each of the new helper > functions, this will prevent these functions accidentally being called > in the all_stop code path. > > While moving the two `for (...)` loops I have replaced 'auto' with > 'thread_info' for additional type clarity. > > With the exception of the new has_execution check in the MI path there > should be no other user visible changes with this commit. I've added > a new test which exposes the missing has_execution check issue. Hi Andrew, thanks for finding and fixing this. The functional change is minimal (add one check), and uses a pattern already used elsewhere in the code, so LGTM. Approved-By: Tom de Vries <tdevries@suse.de> FWIW, while reviewing the patch I came to the hypothesis that there are really three parts: - refactoring in infcmd.c - minimal fix - refactoring in exec_continue To verify this, I split off the first two parts, and confirmed that this minimal fix (not showing the part removing proceed_thread): ... diff --git a/gdb/mi/mi-main.c b/gdb/mi/mi-main.c index 8b6da41ffeb..a7e3845d6e3 100644 --- a/gdb/mi/mi-main.c +++ b/gdb/mi/mi-main.c @@ -279,7 +265,12 @@ exec_continue (const char *const *argv, int argc) } for (auto &thread : all_threads ()) - proceed_thread (&thread, pid); + { + if (pid != 0 && thread.ptid.pid () != pid) + continue; + proceed_one_thread (thread); + } + disable_commit_resumed.reset_and_commit (); } else ... fixes the test-case failure. I didn't like the escaping in the test-case much, so I wrote a patch fixing this (attached). You could merge before committing, or I can do a follow-up commit, as you like. Thanks, - Tom [-- Attachment #2: 0001-gdb-testsuite-Make-gdb.mi-mi-corefile-and-live.exp-r.patch --] [-- Type: text/x-patch, Size: 3293 bytes --] From a88acf26e0346c57fa8275af4d0b26514873aa8a Mon Sep 17 00:00:00 2001 From: Tom de Vries <tdevries@suse.de> Date: Fri, 7 Aug 2026 09:44:12 +0200 Subject: [PATCH] [gdb/testsuite] Make gdb.mi/mi-corefile-and-live.exp regexps more readable --- gdb/testsuite/gdb.mi/mi-corefile-and-live.exp | 38 ++++++++++--------- 1 file changed, 21 insertions(+), 17 deletions(-) diff --git a/gdb/testsuite/gdb.mi/mi-corefile-and-live.exp b/gdb/testsuite/gdb.mi/mi-corefile-and-live.exp index a3a57d3c81a..1a34029d087 100644 --- a/gdb/testsuite/gdb.mi/mi-corefile-and-live.exp +++ b/gdb/testsuite/gdb.mi/mi-corefile-and-live.exp @@ -39,8 +39,8 @@ if {$corefile == ""} { # Start GDB in non-stop and schedule-multiple mode. save_vars { GDBFLAGS } { - append GDBFLAGS " -ex \"set non-stop on\"" - append GDBFLAGS " -ex \"set schedule-multiple on\"" + append GDBFLAGS { -ex "set non-stop on"} + append GDBFLAGS { -ex "set schedule-multiple on"} mi_clean_restart $::testfile } @@ -56,42 +56,46 @@ mi_create_breakpoint "-g i1 foo" \ # Setup inferior 2, this will load the core file. mi_gdb_test "-add-inferior" \ - [multi_line "=thread-group-added,id=\"i2\"" \ - "~\"\\\[New inferior 2\\\]\\\\n\"" \ - "\~\"Added inferior 2\[^\r\n\]*\\\\n\"" \ - "\\^done,inferior=\"\[^\"\]+\"(?:,connection={.*})?" ] \ + [quotemeta \ + [multi_line \ + {=thread-group-added,id="i2"} \ + {~"[New inferior 2]\n"} \ + {~"Added inferior 2@/[^\r\n]*/\n"} \ + {^done,inferior="@/[^"]+/"@/(?:,connection={.*})?/}]] \ "add inferior 2" # Set the executable for inferior 2. mi_gdb_test "-file-exec-and-symbols --thread-group i2 $::binfile" \ - "\\^done" \ + [string_to_regexp "^done"] \ "set executable of inferior 2" # Load the core file into inferior 2. mi_gdb_test \ "-target-select --thread-group i2 core $::corefile" \ - [multi_line \ - "=thread-group-started,id=\"i2\",.*" \ - "=thread-created,id=\"2\",group-id=\"i2\"" \ - ".*\\^connected,frame=.*"] \ + [quotemeta \ + [multi_line \ + {=thread-group-started,id="i2",@...} \ + {=thread-created,id="2",group-id="i2"} \ + {@...^connected,frame=@...}]] \ "load core file in inferior 2" # Check the core file thread is initially shown as stopped. -mi_gdb_test "-thread-info 2" ".*,state=\"stopped\".*" \ +mi_gdb_test "-thread-info 2" {.*,state="stopped".*} \ "core file thread is initially stopped" # Resume "all" threads. As the core target doesn't support execution # this should not try to set the core target threads running. mi_gdb_test "-exec-continue --all" \ - [multi_line \ - "\\^running" \ - "\\*running,thread-id=\"1\""] \ + [string_to_regexp \ + [multi_line \ + "^running" \ + {*running,thread-id="1"}]] \ "resume all" # Wait for the non-core target thread to stop. mi_expect_stop "breakpoint-hit" \ - "foo" ".*" ".*" ".*" {"" "disp=\"keep\""} "w1,i2 stop" + "foo" ".*" ".*" ".*" {"" {disp="keep"}} "w1,i2 stop" # Check that the core target thread is still showing as stopped. -mi_gdb_test "-thread-info 2" ".*,state=\"stopped\".*" \ +mi_gdb_test "-thread-info 2" {.*,state="stopped".*} \ "core file thread is still stopped" base-commit: 4df4712cb94f40463daf6a75c71a64eaf84e7901 -- 2.51.0 ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] gdb: share some thread proceed related code between CLI and MI 2026-08-07 9:20 ` Tom de Vries @ 2026-08-07 15:40 ` Andrew Burgess 0 siblings, 0 replies; 10+ messages in thread From: Andrew Burgess @ 2026-08-07 15:40 UTC (permalink / raw) To: Tom de Vries, gdb-patches Tom de Vries <tdevries@suse.de> writes: > On 8/6/26 10:54 PM, Andrew Burgess wrote: >> I noticed that some code related to proceeding threads could be shared >> between CLI and MI. This fixes a bug as the CLI code contains a fix >> that the MI code is missing. >> >> In continue_1 (in infcmd.c) we have a loop that iterates over all >> threads looking for threads that are THREAD_STOPPED and are in an >> inferior that has_execution. For each thread found we then call: >> >> switch_to_thread (&thread); >> clear_proceed_status (0); >> proceed ((CORE_ADDR) -1, GDB_SIGNAL_DEFAULT); >> >> In exec_continue (in mi/mi-main.c) we also have a loop over all >> threads that calls the proceed_thread helper function which skips >> threads that are not THREAD_STOPPED, does some PID related >> filtering (more on this later) and then calls the same three >> functions: switch_to_thread, clear_proceed_status, proceed. >> >> The PID filtering mentioned above allows proceed_thread to do two >> jobs, if the PID is zero then we proceed all threads. If PID is >> non-zero then we proceed only the threads in the inferior with that >> PID. >> >> You might also have spotted that in continue_1 we checked if the >> inferior has execution or not. This check was added in commit: >> >> commit 5b6d1e4fa4fc6827c7b3f0e99ff120dfa14d65d2 >> Date: Fri Jan 10 20:06:08 2020 +0000 >> >> Multi-target support >> >> A matching check was not added into the MI at this point, nor did the >> commit message mention why such a check was not added. I'm choosing >> to believe that this was an oversight in the 5b6d1e4fa4fc6827 commit. >> And this is the bug I mentioned above. >> >> If we call `proceed` with a thread that is part of an inferior that >> does not "has_execution" then the thread will be marked running even >> though it will never actually be set running. See the early return at >> the top of `proceed_resume_thread_checked` and the call to set_state >> in `proceed`. >> >> I do worry that there might be a bigger set of bugs here if proceed >> can set a thread's state to THREAD_RUNNING, but then never actually >> sets the underlying thread running. But in this case, just having the >> MI share code with the CLI means that we pick up the fix for this case >> basically for free. >> >> I propose adding a new global helper function `proceed_one_thread`, >> this will check if the thread is THREAD_STOPPED and is in an inferior >> which has_execution. If these conditions are met then the three >> functions mentioned above will be called to proceed the thread. >> >> I will then add a second new function `proceed_all_threads`, this will >> iterate over all threads and call proceed_one_thread. >> >> We can then use proceed_all_threads from continue_1, replacing the >> existing loop. >> >> In exec_continue we can move the PID (or rather inferior) check >> earlier, outside the loop. If we want to resume all threads (the old >> PID is zero path) then we call proceed_all_threads. If we only want >> to proceed threads with one PID then we loop over threads in the >> matching inferior and call proceed_one_thread on each. >> >> As the code I am factoring out is all within non_stop only paths I >> have added `gdb_assert (non_stop);` to each of the new helper >> functions, this will prevent these functions accidentally being called >> in the all_stop code path. >> >> While moving the two `for (...)` loops I have replaced 'auto' with >> 'thread_info' for additional type clarity. >> >> With the exception of the new has_execution check in the MI path there >> should be no other user visible changes with this commit. I've added >> a new test which exposes the missing has_execution check issue. > > Hi Andrew, > > thanks for finding and fixing this. > > The functional change is minimal (add one check), and uses a pattern > already used elsewhere in the code, so LGTM. > > Approved-By: Tom de Vries <tdevries@suse.de> > > FWIW, while reviewing the patch I came to the hypothesis that there are > really three parts: > - refactoring in infcmd.c > - minimal fix > - refactoring in exec_continue > > To verify this, I split off the first two parts, and confirmed that this > minimal fix (not showing the part removing proceed_thread): > ... > diff --git a/gdb/mi/mi-main.c b/gdb/mi/mi-main.c > index 8b6da41ffeb..a7e3845d6e3 100644 > --- a/gdb/mi/mi-main.c > +++ b/gdb/mi/mi-main.c > @@ -279,7 +265,12 @@ exec_continue (const char *const *argv, int argc) > } > > for (auto &thread : all_threads ()) > - proceed_thread (&thread, pid); > + { > + if (pid != 0 && thread.ptid.pid () != pid) > + continue; > + proceed_one_thread (thread); > + } > + > disable_commit_resumed.reset_and_commit (); > } > else > ... > fixes the test-case failure. > > I didn't like the escaping in the test-case much, so I wrote a patch > fixing this (attached). You could merge before committing, or I can do > a follow-up commit, as you like. Thanks, I merged everything except the quotemeta related changes. Not a huge fan, and in this case the imbalanced quotes was mucking up the syntax highlighting in my editor. I know, that's a "me" problem, but I really don't want to push a patch that makes my life harder. I'll hold off pushing this until we've discussed your other review email about this change. Thanks, Andrew > > Thanks, > - Tom > From a88acf26e0346c57fa8275af4d0b26514873aa8a Mon Sep 17 00:00:00 2001 > From: Tom de Vries <tdevries@suse.de> > Date: Fri, 7 Aug 2026 09:44:12 +0200 > Subject: [PATCH] [gdb/testsuite] Make gdb.mi/mi-corefile-and-live.exp regexps > more readable > > --- > gdb/testsuite/gdb.mi/mi-corefile-and-live.exp | 38 ++++++++++--------- > 1 file changed, 21 insertions(+), 17 deletions(-) > > diff --git a/gdb/testsuite/gdb.mi/mi-corefile-and-live.exp b/gdb/testsuite/gdb.mi/mi-corefile-and-live.exp > index a3a57d3c81a..1a34029d087 100644 > --- a/gdb/testsuite/gdb.mi/mi-corefile-and-live.exp > +++ b/gdb/testsuite/gdb.mi/mi-corefile-and-live.exp > @@ -39,8 +39,8 @@ if {$corefile == ""} { > > # Start GDB in non-stop and schedule-multiple mode. > save_vars { GDBFLAGS } { > - append GDBFLAGS " -ex \"set non-stop on\"" > - append GDBFLAGS " -ex \"set schedule-multiple on\"" > + append GDBFLAGS { -ex "set non-stop on"} > + append GDBFLAGS { -ex "set schedule-multiple on"} > mi_clean_restart $::testfile > } > > @@ -56,42 +56,46 @@ mi_create_breakpoint "-g i1 foo" \ > > # Setup inferior 2, this will load the core file. > mi_gdb_test "-add-inferior" \ > - [multi_line "=thread-group-added,id=\"i2\"" \ > - "~\"\\\[New inferior 2\\\]\\\\n\"" \ > - "\~\"Added inferior 2\[^\r\n\]*\\\\n\"" \ > - "\\^done,inferior=\"\[^\"\]+\"(?:,connection={.*})?" ] \ > + [quotemeta \ > + [multi_line \ > + {=thread-group-added,id="i2"} \ > + {~"[New inferior 2]\n"} \ > + {~"Added inferior 2@/[^\r\n]*/\n"} \ > + {^done,inferior="@/[^"]+/"@/(?:,connection={.*})?/}]] \ > "add inferior 2" > > # Set the executable for inferior 2. > mi_gdb_test "-file-exec-and-symbols --thread-group i2 $::binfile" \ > - "\\^done" \ > + [string_to_regexp "^done"] \ > "set executable of inferior 2" > > # Load the core file into inferior 2. > mi_gdb_test \ > "-target-select --thread-group i2 core $::corefile" \ > - [multi_line \ > - "=thread-group-started,id=\"i2\",.*" \ > - "=thread-created,id=\"2\",group-id=\"i2\"" \ > - ".*\\^connected,frame=.*"] \ > + [quotemeta \ > + [multi_line \ > + {=thread-group-started,id="i2",@...} \ > + {=thread-created,id="2",group-id="i2"} \ > + {@...^connected,frame=@...}]] \ > "load core file in inferior 2" > > # Check the core file thread is initially shown as stopped. > -mi_gdb_test "-thread-info 2" ".*,state=\"stopped\".*" \ > +mi_gdb_test "-thread-info 2" {.*,state="stopped".*} \ > "core file thread is initially stopped" > > # Resume "all" threads. As the core target doesn't support execution > # this should not try to set the core target threads running. > mi_gdb_test "-exec-continue --all" \ > - [multi_line \ > - "\\^running" \ > - "\\*running,thread-id=\"1\""] \ > + [string_to_regexp \ > + [multi_line \ > + "^running" \ > + {*running,thread-id="1"}]] \ > "resume all" > > # Wait for the non-core target thread to stop. > mi_expect_stop "breakpoint-hit" \ > - "foo" ".*" ".*" ".*" {"" "disp=\"keep\""} "w1,i2 stop" > + "foo" ".*" ".*" ".*" {"" {disp="keep"}} "w1,i2 stop" > > # Check that the core target thread is still showing as stopped. > -mi_gdb_test "-thread-info 2" ".*,state=\"stopped\".*" \ > +mi_gdb_test "-thread-info 2" {.*,state="stopped".*} \ > "core file thread is still stopped" > > base-commit: 4df4712cb94f40463daf6a75c71a64eaf84e7901 > -- > 2.51.0 ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] gdb: share some thread proceed related code between CLI and MI 2026-08-06 20:54 ` [PATCH 2/2] gdb: share some thread proceed related code between CLI and MI Andrew Burgess 2026-08-07 9:20 ` Tom de Vries @ 2026-08-07 10:14 ` Tom de Vries 2026-08-07 15:31 ` Andrew Burgess 1 sibling, 1 reply; 10+ messages in thread From: Tom de Vries @ 2026-08-07 10:14 UTC (permalink / raw) To: Andrew Burgess, gdb-patches On 8/6/26 10:54 PM, Andrew Burgess wrote: > + if (inf == nullptr) > + proceed_all_threads (); > + else > + { > + for (thread_info &thread : inf->threads ()) > + proceed_one_thread (thread); > } FWIW, also I wonder if it makes sense to fold this logic into proceed_all_threads: ... diff --git a/gdb/infcmd.c b/gdb/infcmd.c index 23e93587874..d2301ddf093 100644 --- a/gdb/infcmd.c +++ b/gdb/infcmd.c @@ -743,10 +743,18 @@ proceed_one_thread (thread_info &thread) /* See inferior.h. */ void -proceed_all_threads () +proceed_all_threads (inferior *inf) { gdb_assert (non_stop); + if (inf != nullptr) + { + for (thread_info &thread : inf->threads ()) + proceed_one_thread (thread); + + return; + } + for (thread_info &thread : all_threads ()) { /* We go through all threads individually instead of compressing diff --git a/gdb/inferior.h b/gdb/inferior.h index 8255654d9b8..2067f60ab7d 100644 --- a/gdb/inferior.h +++ b/gdb/inferior.h @@ -230,7 +230,7 @@ extern void continue_1 (bool all_threads_p); /* For use only when non_stop is true. Proceed all threads in every inferior. */ -extern void proceed_all_threads (); +extern void proceed_all_threads (inferior *inf = nullptr); /* For use only when non_stop is true. If THREAD is stopped, and is in an inferior that has_execution then switch to THREAD, clear its proceed diff --git a/gdb/mi/mi-main.c b/gdb/mi/mi-main.c index e110b029e1e..1eb087b85db 100644 --- a/gdb/mi/mi-main.c +++ b/gdb/mi/mi-main.c @@ -259,13 +259,7 @@ exec_continue (const char *const *argv, int argc) if (!current_context->all) inf = find_inferior_id (current_context->thread_group); - if (inf == nullptr) - proceed_all_threads (); - else - { - for (thread_info &thread : inf->threads ()) - proceed_one_thread (thread); - } + proceed_all_threads (inf); disable_commit_resumed.reset_and_commit (); } ... Thanks, - Tom ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] gdb: share some thread proceed related code between CLI and MI 2026-08-07 10:14 ` Tom de Vries @ 2026-08-07 15:31 ` Andrew Burgess 2026-08-07 16:52 ` Tom de Vries 0 siblings, 1 reply; 10+ messages in thread From: Andrew Burgess @ 2026-08-07 15:31 UTC (permalink / raw) To: Tom de Vries, gdb-patches Tom de Vries <tdevries@suse.de> writes: > On 8/6/26 10:54 PM, Andrew Burgess wrote: >> + if (inf == nullptr) >> + proceed_all_threads (); >> + else >> + { >> + for (thread_info &thread : inf->threads ()) >> + proceed_one_thread (thread); >> } > > FWIW, also I wonder if it makes sense to fold this logic into > proceed_all_threads: I took a look through all the other proceed() calls and couldn't see anything else that might want to share this code, so my preference would be to leave things as I initially proposed. I like the simpler each function has one clear goal, rather than overloading proceed_all_threads with multiple meanings ("all threads", or "all threads in inferior"). Especially not a fan of default arguments for cases like this (or in general much). That said, if you consider this a blocker to merging this patch then I don't feel that strongly against it so would make the change to get this fix merged. Just let me know. Thanks, Andrew > ... > diff --git a/gdb/infcmd.c b/gdb/infcmd.c > index 23e93587874..d2301ddf093 100644 > --- a/gdb/infcmd.c > +++ b/gdb/infcmd.c > @@ -743,10 +743,18 @@ proceed_one_thread (thread_info &thread) > /* See inferior.h. */ > > void > -proceed_all_threads () > +proceed_all_threads (inferior *inf) > { > gdb_assert (non_stop); > > + if (inf != nullptr) > + { > + for (thread_info &thread : inf->threads ()) > + proceed_one_thread (thread); > + > + return; > + } > + > for (thread_info &thread : all_threads ()) > { > /* We go through all threads individually instead of compressing > diff --git a/gdb/inferior.h b/gdb/inferior.h > index 8255654d9b8..2067f60ab7d 100644 > --- a/gdb/inferior.h > +++ b/gdb/inferior.h > @@ -230,7 +230,7 @@ extern void continue_1 (bool all_threads_p); > /* For use only when non_stop is true. Proceed all threads in every > inferior. */ > > -extern void proceed_all_threads (); > +extern void proceed_all_threads (inferior *inf = nullptr); > > /* For use only when non_stop is true. If THREAD is stopped, and is in an > inferior that has_execution then switch to THREAD, clear its proceed > diff --git a/gdb/mi/mi-main.c b/gdb/mi/mi-main.c > index e110b029e1e..1eb087b85db 100644 > --- a/gdb/mi/mi-main.c > +++ b/gdb/mi/mi-main.c > @@ -259,13 +259,7 @@ exec_continue (const char *const *argv, int argc) > if (!current_context->all) > inf = find_inferior_id (current_context->thread_group); > > - if (inf == nullptr) > - proceed_all_threads (); > - else > - { > - for (thread_info &thread : inf->threads ()) > - proceed_one_thread (thread); > - } > + proceed_all_threads (inf); > > disable_commit_resumed.reset_and_commit (); > } > ... > > Thanks, > - Tom ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] gdb: share some thread proceed related code between CLI and MI 2026-08-07 15:31 ` Andrew Burgess @ 2026-08-07 16:52 ` Tom de Vries 2026-08-24 18:18 ` Andrew Burgess 0 siblings, 1 reply; 10+ messages in thread From: Tom de Vries @ 2026-08-07 16:52 UTC (permalink / raw) To: Andrew Burgess, gdb-patches On 8/7/26 5:31 PM, Andrew Burgess wrote: > Tom de Vries <tdevries@suse.de> writes: > >> On 8/6/26 10:54 PM, Andrew Burgess wrote: >>> + if (inf == nullptr) >>> + proceed_all_threads (); >>> + else >>> + { >>> + for (thread_info &thread : inf->threads ()) >>> + proceed_one_thread (thread); >>> } >> >> FWIW, also I wonder if it makes sense to fold this logic into >> proceed_all_threads: > > I took a look through all the other proceed() calls and couldn't see > anything else that might want to share this code, so my preference would > be to leave things as I initially proposed. I like the simpler each > function has one clear goal, rather than overloading proceed_all_threads > with multiple meanings ("all threads", or "all threads in inferior"). > Especially not a fan of default arguments for cases like this (or in > general much). > > That said, if you consider this a blocker to merging this patch then I > don't feel that strongly against it so would make the change to get this > fix merged. > Hi Andrew, I don't consider this a blocker. Thanks, - Tom > Just let me know. > > Thanks, > Andrew > > >> ... >> diff --git a/gdb/infcmd.c b/gdb/infcmd.c >> index 23e93587874..d2301ddf093 100644 >> --- a/gdb/infcmd.c >> +++ b/gdb/infcmd.c >> @@ -743,10 +743,18 @@ proceed_one_thread (thread_info &thread) >> /* See inferior.h. */ >> >> void >> -proceed_all_threads () >> +proceed_all_threads (inferior *inf) >> { >> gdb_assert (non_stop); >> >> + if (inf != nullptr) >> + { >> + for (thread_info &thread : inf->threads ()) >> + proceed_one_thread (thread); >> + >> + return; >> + } >> + >> for (thread_info &thread : all_threads ()) >> { >> /* We go through all threads individually instead of compressing >> diff --git a/gdb/inferior.h b/gdb/inferior.h >> index 8255654d9b8..2067f60ab7d 100644 >> --- a/gdb/inferior.h >> +++ b/gdb/inferior.h >> @@ -230,7 +230,7 @@ extern void continue_1 (bool all_threads_p); >> /* For use only when non_stop is true. Proceed all threads in every >> inferior. */ >> >> -extern void proceed_all_threads (); >> +extern void proceed_all_threads (inferior *inf = nullptr); >> >> /* For use only when non_stop is true. If THREAD is stopped, and is in an >> inferior that has_execution then switch to THREAD, clear its proceed >> diff --git a/gdb/mi/mi-main.c b/gdb/mi/mi-main.c >> index e110b029e1e..1eb087b85db 100644 >> --- a/gdb/mi/mi-main.c >> +++ b/gdb/mi/mi-main.c >> @@ -259,13 +259,7 @@ exec_continue (const char *const *argv, int argc) >> if (!current_context->all) >> inf = find_inferior_id (current_context->thread_group); >> >> - if (inf == nullptr) >> - proceed_all_threads (); >> - else >> - { >> - for (thread_info &thread : inf->threads ()) >> - proceed_one_thread (thread); >> - } >> + proceed_all_threads (inf); >> >> disable_commit_resumed.reset_and_commit (); >> } >> ... >> >> Thanks, >> - Tom > ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] gdb: share some thread proceed related code between CLI and MI 2026-08-07 16:52 ` Tom de Vries @ 2026-08-24 18:18 ` Andrew Burgess 0 siblings, 0 replies; 10+ messages in thread From: Andrew Burgess @ 2026-08-24 18:18 UTC (permalink / raw) To: Tom de Vries, gdb-patches Tom de Vries <tdevries@suse.de> writes: > On 8/7/26 5:31 PM, Andrew Burgess wrote: >> Tom de Vries <tdevries@suse.de> writes: >> >>> On 8/6/26 10:54 PM, Andrew Burgess wrote: >>>> + if (inf == nullptr) >>>> + proceed_all_threads (); >>>> + else >>>> + { >>>> + for (thread_info &thread : inf->threads ()) >>>> + proceed_one_thread (thread); >>>> } >>> >>> FWIW, also I wonder if it makes sense to fold this logic into >>> proceed_all_threads: >> >> I took a look through all the other proceed() calls and couldn't see >> anything else that might want to share this code, so my preference would >> be to leave things as I initially proposed. I like the simpler each >> function has one clear goal, rather than overloading proceed_all_threads >> with multiple meanings ("all threads", or "all threads in inferior"). >> Especially not a fan of default arguments for cases like this (or in >> general much). >> >> That said, if you consider this a blocker to merging this patch then I >> don't feel that strongly against it so would make the change to get this >> fix merged. >> > > Hi Andrew, > > I don't consider this a blocker. Great. I've now pushed these patches. Thanks, Andrew ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2026-08-24 18:18 UTC | newest] Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2026-08-06 20:53 [PATCH 0/2] Cleanup and bug fix related to thread iteration and proceed calls Andrew Burgess 2026-08-06 20:54 ` [PATCH 1/2] gdb: use inferior::threads instead of all_threads and a PID filter Andrew Burgess 2026-08-07 7:59 ` Tom de Vries 2026-08-06 20:54 ` [PATCH 2/2] gdb: share some thread proceed related code between CLI and MI Andrew Burgess 2026-08-07 9:20 ` Tom de Vries 2026-08-07 15:40 ` Andrew Burgess 2026-08-07 10:14 ` Tom de Vries 2026-08-07 15:31 ` Andrew Burgess 2026-08-07 16:52 ` Tom de Vries 2026-08-24 18:18 ` Andrew Burgess
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox