Created attachment 13630 [details]
Patch to add a related issue into the testsuite.

I ran into the same assertion while debugging some other issue.  My steps to reproduce are slightly different, but I thought I'd add the details here as it feels like these two cases must be related:

  $ /tmp/gdb/testsuite/outputs/gdb.threads/detach-step-over/detach-step-over
  $ gdb
  (gdb) attach 608289
  Attaching to program: /tmp/gdb/testsuite/outputs/gdb.threads/detach-step-over/detach-step-over, process 608289
  ... snip ...
  [Thread debugging using libthread_db enabled]
  Using host libthread_db library "/lib64/libthread_db.so.1".
  0x00007f84068f4215 in nanosleep () from /lib64/libc.so.6
  (gdb) break detach-step-over.c:54 if 0
  Breakpoint 1 at 0x40119c: file /tmp/gdb/testsuite/../../../src/gdb/testsuite/gdb.threads/detach-step-over.c, line 54.
  (gdb) continue &
  Continuing.
  (gdb) q
  A debugging session is active.
  
  	Inferior 1 [process 608289] will be detached.
  
  Quit anyway? (y or n) y
  ../../src/gdb/target.c:3778: internal-error: void target_stop(ptid_t): Assertion `!proc_target->commit_resumed_state' failed.

The patch I've attached modifies gdb.threads/detach-step-over.exp to show this bug, this patch isn't ready to upstream in its current state.

I found another procedure to reach the same crash:

- Run a multithreaded GUI app (eg. gdb kwrite).
- Start the application (type run).
- In another terminal, call `killall kwrite` or similar. GDB will say "Thread 1 "kwrite" received signal SIGTERM, Terminated."
- Type continue into gdb. gdb will say [Thread ... (LWP ...) exited], but not [Inferior 1 (process ...) exited ...], even though the PID no longer exists in /proc/NUMBER.
- Type kill. When gdb asks "Kill the program being debugged? (y or n)", say y. gdb will say [Inferior 1 (process ...) killed].
- Type run.

This results in the same error: target.c:2603: internal-error: ptid_t target_wait(ptid_t, target_waitstatus*, target_wait_flags): Assertion `!proc_target->commit_resumed_state' failed.

Something like this should fix the original issue (when re-running):

From 0bc259c5c7640fb9bb655b7b279eb0b2ec87c28b Mon Sep 17 00:00:00 2001
From: Simon Marchi <simon.marchi@polymtl.ca>
Date: Fri, 29 Oct 2021 08:36:00 -0400
Subject: [PATCH] gdb: fix commit resumed failed assertion when re-running

Change-Id: Ic7ed0524751d0e5524936b2890048cd4a3d92765
---
 gdb/infcmd.c |  4 ++--
 gdb/target.c | 46 +++++++++++++++++++++-------------------------
 gdb/target.h |  8 ++++----
 3 files changed, 27 insertions(+), 31 deletions(-)

diff --git a/gdb/infcmd.c b/gdb/infcmd.c
index 302db421a21f..57726650f155 100644
--- a/gdb/infcmd.c
+++ b/gdb/infcmd.c
@@ -371,7 +371,6 @@ run_command_1 (const char *args, int from_tty, enum run_how run_how)
 {
   const char *exec_file;
   struct ui_out *uiout = current_uiout;
-  struct target_ops *run_target;
   int async_exec;
 
   dont_repeat ();
@@ -403,7 +402,7 @@ run_command_1 (const char *args, int from_tty, enum run_how run_how)
   /* Do validation and preparation before possibly changing anything
      in the inferior.  */
 
-  run_target = find_run_target ();
+  process_stratum_target *run_target = find_run_target ();
 
   prepare_execution_command (run_target, async_exec);
 
@@ -445,6 +444,7 @@ run_command_1 (const char *args, int from_tty, enum run_how run_how)
       uiout->flush ();
     }
 
+  run_target->commit_resumed_state = false;
   run_target->create_inferior (exec_file,
 			       current_inferior ()->args (),
 			       current_inferior ()->environment.envp (),
diff --git a/gdb/target.c b/gdb/target.c
index 6cff56474873..967551a6ad8e 100644
--- a/gdb/target.c
+++ b/gdb/target.c
@@ -84,7 +84,7 @@ static int default_verify_memory (struct target_ops *self,
 
 static void tcomplain (void) ATTRIBUTE_NORETURN;
 
-static struct target_ops *find_default_run_target (const char *);
+static process_stratum_target *find_default_run_target (const char *);
 
 static int dummy_find_memory_regions (struct target_ops *self,
 				      find_memory_region_ftype ignore1,
@@ -2880,12 +2880,12 @@ show_auto_connect_native_target (struct ui_file *file, int from_tty,
 /* A pointer to the target that can respond to "run" or "attach".
    Native targets are always singletons and instantiated early at GDB
    startup.  */
-static target_ops *the_native_target;
+static process_stratum_target *the_native_target;
 
 /* See target.h.  */
 
 void
-set_native_target (target_ops *target)
+set_native_target (process_stratum_target *target)
 {
   if (the_native_target != NULL)
     internal_error (__FILE__, __LINE__,
@@ -2897,7 +2897,7 @@ set_native_target (target_ops *target)
 
 /* See target.h.  */
 
-target_ops *
+process_stratum_target *
 get_native_target ()
 {
   return the_native_target;
@@ -2910,7 +2910,7 @@ get_native_target ()
    If DO_MESG is not NULL, the result is always valid (error() is
    called for errors); else, return NULL on error.  */
 
-static struct target_ops *
+static process_stratum_target *
 find_default_run_target (const char *do_mesg)
 {
   if (auto_connect_native_target && the_native_target != NULL)
@@ -2923,17 +2923,15 @@ find_default_run_target (const char *do_mesg)
 
 /* See target.h.  */
 
-struct target_ops *
-find_attach_target (void)
+process_stratum_target *
+find_attach_target ()
 {
-  /* If a target on the current stack can attach, use it.  */
-  for (target_ops *t = current_inferior ()->top_target ();
-       t != NULL;
-       t = t->beneath ())
-    {
-      if (t->can_attach ())
-	return t;
-    }
+  /* If the current process stratum target can attach, use it.  */
+  process_stratum_target *current_proc_target
+    = current_inferior ()->process_target ();
+  if (current_proc_target != nullptr
+      && current_proc_target->can_attach ())
+    return current_proc_target;
 
   /* Otherwise, use the default run target for attaching.  */
   return find_default_run_target ("attach");
@@ -2941,17 +2939,15 @@ find_attach_target (void)
 
 /* See target.h.  */
 
-struct target_ops *
-find_run_target (void)
+process_stratum_target *
+find_run_target ()
 {
-  /* If a target on the current stack can run, use it.  */
-  for (target_ops *t = current_inferior ()->top_target ();
-       t != NULL;
-       t = t->beneath ())
-    {
-      if (t->can_create_inferior ())
-	return t;
-    }
+  /* If the current process stratum target can run, use it.  */
+  process_stratum_target *current_proc_target
+    = current_inferior ()->process_target ();
+  if (current_proc_target != nullptr
+      && current_proc_target->can_run ())
+    return current_proc_target;
 
   /* Otherwise, use the default run target.  */
   return find_default_run_target ("run");
diff --git a/gdb/target.h b/gdb/target.h
index 92fc1381db2b..5750a7655670 100644
--- a/gdb/target.h
+++ b/gdb/target.h
@@ -1364,11 +1364,11 @@ typedef gdb::ref_ptr<target_ops, target_ops_ref_policy> target_ops_ref;
 /* Native target backends call this once at initialization time to
    inform the core about which is the target that can respond to "run"
    or "attach".  Note: native targets are always singletons.  */
-extern void set_native_target (target_ops *target);
+extern void set_native_target (process_stratum_target *target);
 
 /* Get the registered native target, if there's one.  Otherwise return
    NULL.  */
-extern target_ops *get_native_target ();
+extern process_stratum_target *get_native_target ();
 
 /* Type that manages a target stack.  See description of target stacks
    and strata at the top of the file.  */
@@ -1430,13 +1430,13 @@ void target_close (struct target_ops *targ);
    current stack supports attaching, then it is returned.  Otherwise,
    the default run target is returned.  */
 
-extern struct target_ops *find_attach_target (void);
+extern process_stratum_target *find_attach_target (void);
 
 /* Find the correct target to use for "run".  If a target on the
    current stack supports creating a new inferior, then it is
    returned.  Otherwise, the default run target is returned.  */
 
-extern struct target_ops *find_run_target (void);
+extern process_stratum_target *find_run_target ();
 
 /* Some targets don't generate traps when attaching to the inferior,
    or their target_attach implementation takes care of the waiting.
-- 
2.33.1

I ran into this today on an aarch64 machine:
...
tdevries@ampere3:~/gdb> grep -a -i 'internal-error:.*assertion' gdb.log | sed 's/^(gdb) //' | sort | uniq -c
     44 /suse/tdevries/gdb/src/gdb/target.c:2590: internal-error: target_wait: Assertion `!proc_target->commit_resumed_state' failed.
...
but at first glance that looks related to forgetting to install libexpat-devel.

(In reply to Tom de Vries from comment #4)
> I ran into this today on an aarch64 machine:
> ...
> tdevries@ampere3:~/gdb> grep -a -i 'internal-error:.*assertion' gdb.log |
> sed 's/^(gdb) //' | sort | uniq -c
>      44 /suse/tdevries/gdb/src/gdb/target.c:2590: internal-error:
> target_wait: Assertion `!proc_target->commit_resumed_state' failed.
> ...
> but at first glance that looks related to forgetting to install
> libexpat-devel.

Can you give specific test names?  I can try to build a GDB without expat and run those tests.

(In reply to Simon Marchi from comment #5)
> Can you give specific test names?  I can try to build a GDB without expat
> and run those tests.

I've deinstalled libexpat-devel and I'm doing a rerun.  Will let you known.

(In reply to Tom de Vries from comment #6)
> (In reply to Simon Marchi from comment #5)
> > Can you give specific test names?  I can try to build a GDB without expat
> > and run those tests.
> 
> I've deinstalled libexpat-devel and I'm doing a rerun.  Will let you known.


In this particular run I get only 26 hits, all in gdb.server/ext-attach.exp:
...
$ egrep -i -a "^Running /|internal-error.*assertion" gdb.log \
  | sed 's/^(gdb) //' \
  | uniq -c \
  | grep -i -B1 "internal-error.*assertion"
      1 Running /suse/tdevries/gdb/src/gdb/testsuite/gdb.server/ext-attach.exp ...
     26 /suse/tdevries/gdb/src/gdb/target.c:2590: internal-error: target_wait: Assertion `!proc_target->commit_resumed_state' failed.
...

(In reply to Tom de Vries from comment #7)
> (In reply to Tom de Vries from comment #6)
> > (In reply to Simon Marchi from comment #5)
> > > Can you give specific test names?  I can try to build a GDB without expat
> > > and run those tests.
> > 
> > I've deinstalled libexpat-devel and I'm doing a rerun.  Will let you known.
> 
> 
> In this particular run I get only 26 hits, all in gdb.server/ext-attach.exp:
> ...
> $ egrep -i -a "^Running /|internal-error.*assertion" gdb.log \
>   | sed 's/^(gdb) //' \
>   | uniq -c \
>   | grep -i -B1 "internal-error.*assertion"
>       1 Running
> /suse/tdevries/gdb/src/gdb/testsuite/gdb.server/ext-attach.exp ...
>      26 /suse/tdevries/gdb/src/gdb/target.c:2590: internal-error:
> target_wait: Assertion `!proc_target->commit_resumed_state' failed.
> ...

Hmm, and we're running the test-case because aarch64 is not skipped in skip_gdbserver_tests:
...
    # If GDB is lack of XML support, and targets, like arm, have                     
    # multiple target descriptions, GDB doesn't know which target                    
    # description GDBserver uses, and may fail to parse 'g' packet                   
    # after connection.                                                              
    if { [gdb_skip_xml_test]
         && ([istarget "arm*-*-linux*"]
             || [istarget "mips*-*-linux*"]
             || [istarget "powerpc*-*-linux*"]
             || [istarget "s390*-*-linux*"]
             || [istarget "x86_64-*-linux*"]
             || [istarget "i\[34567\]86-*-linux*"]) } {
        return 1
    }
...

(In reply to Tom de Vries from comment #8)
> Hmm, and we're running the test-case because aarch64 is not skipped in
> skip_gdbserver_tests:
> ...
>     # If GDB is lack of XML support, and targets, like arm, have            
> 
>     # multiple target descriptions, GDB doesn't know which target           
> 
>     # description GDBserver uses, and may fail to parse 'g' packet          
> 
>     # after connection.                                                     
> 
>     if { [gdb_skip_xml_test]
>          && ([istarget "arm*-*-linux*"]
>              || [istarget "mips*-*-linux*"]
>              || [istarget "powerpc*-*-linux*"]
>              || [istarget "s390*-*-linux*"]
>              || [istarget "x86_64-*-linux*"]
>              || [istarget "i\[34567\]86-*-linux*"]) } {
>         return 1
>     }
> ...

https://sourceware.org/pipermail/gdb-patches/2022-November/193819.html

Created attachment 14458 [details]
Patch including test and possible fix (for issue in comment #1)

I took a look at the issue I raised in comment #1.  This updated patch includes a test that I think is now ready for upstream, and a possible fix.

I've not posted this to the m/l as I believe Simon is currently looking at wider commit_resumed_state issues, so I don't want to conflict with any work he's doing.

Hopefully the new test at least will be included with anything Simon posts upstream, and the fix I propose might be helpful.

Adding target milestone to GDB 13.1 to mark this as needed for that release (requested by SimonM, and I agree it would be good to have this fixed, since this is a crash).

The master branch has been updated by Simon Marchi <simark@sourceware.org>:

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

commit ddff2a2dea5261789e1f281f3ee1b33dd5fd8892
Author: Andrew Burgess <aburgess@redhat.com>
Date:   Mon Nov 21 12:12:11 2022 -0500

    gdb: fix assert when quitting GDB while a thread is stepping
    
    This commit addresses one of the issues identified in PR gdb/28275.
    
    Bug gdb/28275 identifies a number of situations in which this assert:
    
      Assertion `!proc_target->commit_resumed_state' failed.
    
    could be triggered.  There's actually a number of similar places where
    this assert is found in GDB, the two of interest in gdb/28275 are in
    target_wait and target_stop.
    
    In one of the comments:
    
      https://sourceware.org/bugzilla/show_bug.cgi?id=28275#c1
    
    steps to trigger the assertion within target_stop were identified when
    using a modified version of the gdb.threads/detach-step-over.exp test
    script.
    
    In the gdb.threads/detach-step-over.exp test, we attach to a
    multi-threaded inferior, and continue the inferior in asynchronous
    (background) mode.  Each thread is continuously hitting a conditional
    breakpoint where the condition is always false.  While the inferior is
    running we detach.  The goal is that we detach while GDB is performing a
    step-over for the conditional breakpoint in at least one thread.
    
    While detaching, if a step-over is in progress, then GDB has to
    complete the step over before we can detach.  This involves calling
    target_stop and target_wait (see prepare_for_detach).
    
    As far as gdb/28275 is concerned, the interesting part here, is the
    the process_stratum_target::commit_resumed_state variable must be
    false when target_stop and target_wait are called.
    
    This is currently ensured because, in detach_command (infrun.c), we
    create an instance of scoped_disable_commit_resumed, this ensures that
    when target_detach is called, ::commit_resumed_state will be false.
    
    The modification to the test that I propose here, and which exposed
    the bug, is that, instead of using "detach" to detach from the
    inferior, we instead use "quit".  Quitting GDB after attaching to an
    inferior will cause GDB to first detach, and then exit.
    
    When we quit GDB we end up calling target_detach via a different code
    path, the stack looks like:
    
      #0 target_detach
      #1 kill_or_detach
      #2 quit_force
      #3 quit_command
    
    Along this path there is no scoped_disable_commit_resumed created.
    ::commit_resumed_state can be true when we reach prepare_for_detach,
    which calls target_wait and target_stop, so the assertion will trigger.
    
    In this commit, I propose fixing this by adding the creation of a
    scoped_disable_commit_resumed into target_detach.  This will ensure
    that ::commit_resumed_state is false when calling prepare_for_detach
    from within target_detach.
    
    I did consider placing the scoped_disable_commit_resumed in
    prepare_for_detach, however, placing it in target_detach ensures that
    the target's commit_resumed_state flag is left to false if the detached
    inferior was the last one for that target.  It's the same rationale as
    for patch "gdb: disable commit resumed in target_kill" that comes later
    in this series, but for detach instead of kill.
    
    detach_command still includes a scoped_disable_commit_resumed too, but I
    think it is still relevant to cover the resumption at the end of the
    function.
    
    Co-Authored-By: Simon Marchi <simon.marchi@efficios.com>
    Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=28275
    Change-Id: Ie128f7aba6ef0e018859275eca372e6ea738e96f

The master branch has been updated by Simon Marchi <simark@sourceware.org>:

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

commit ed14d866a31625c6f8c2cb6d4a445a8372b46161
Author: Simon Marchi <simon.marchi@efficios.com>
Date:   Mon Nov 21 12:12:13 2022 -0500

    gdb: disable commit resumed in target_kill
    
    New in this version:
    
     - Better comment in target_kill
     - Uncomment line in test to avoid hanging when exiting, when testing on
       native-extended-gdbserver
    
    PR 28275 shows that doing a sequence of:
    
     - Run inferior in background (run &)
     - kill that inferior
     - Run again
    
    We get into this assertion:
    
        /home/smarchi/src/binutils-gdb/gdb/target.c:2590: internal-error: target_wait: Assertion `!proc_target->commit_resumed_state' failed.
    
        #0  internal_error_loc (file=0x5606b344e740 "/home/smarchi/src/binutils-gdb/gdb/target.c", line=2590, fmt=0x5606b344d6a0 "%s: Assertion `%s' failed.") at /home/smarchi/src/binutils-gdb/gdbsupport/errors.cc:54
        #1  0x00005606b6296475 in target_wait (ptid=..., status=0x7fffb9390630, options=...) at /home/smarchi/src/binutils-gdb/gdb/target.c:2590
        #2  0x00005606b5767a98 in startup_inferior (proc_target=0x5606bfccb2a0 <the_amd64_linux_nat_target>, pid=3884857, ntraps=1, last_waitstatus=0x0, last_ptid=0x0) at /home/smarchi/src/binutils-gdb/gdb/nat/fork-inferior.c:482
        #3  0x00005606b4e6c9c5 in gdb_startup_inferior (pid=3884857, num_traps=1) at /home/smarchi/src/binutils-gdb/gdb/fork-child.c:132
        #4  0x00005606b50f14a5 in inf_ptrace_target::create_inferior (this=0x5606bfccb2a0 <the_amd64_linux_nat_target>, exec_file=0x604000039f50 "/home/smarchi/build/binutils-gdb/gdb/test", allargs="", env=0x61500000a580, from_tty=1)
            at /home/smarchi/src/binutils-gdb/gdb/inf-ptrace.c:105
        #5  0x00005606b53b6d23 in linux_nat_target::create_inferior (this=0x5606bfccb2a0 <the_amd64_linux_nat_target>, exec_file=0x604000039f50 "/home/smarchi/build/binutils-gdb/gdb/test", allargs="", env=0x61500000a580, from_tty=1)
            at /home/smarchi/src/binutils-gdb/gdb/linux-nat.c:978
        #6  0x00005606b512b79b in run_command_1 (args=0x0, from_tty=1, run_how=RUN_NORMAL) at /home/smarchi/src/binutils-gdb/gdb/infcmd.c:468
        #7  0x00005606b512c236 in run_command (args=0x0, from_tty=1) at /home/smarchi/src/binutils-gdb/gdb/infcmd.c:526
    
    When running the kill command, commit_resumed_state for the
    process_stratum_target (linux-nat, here) is true.  After the kill, when
    there are no more threads, commit_resumed_state is still true, as
    nothing touches this flag during the kill operation.  During the
    subsequent run command, run_command_1 does:
    
        scoped_disable_commit_resumed disable_commit_resumed ("running");
    
    We would think that this would clear the commit_resumed_state flag of
    our native target, but that's not the case, because
    scoped_disable_commit_resumed iterates on non-exited inferiors in order
    to find active process targets.  And after the kill, the inferior is
    exited, and the native target was unpushed from it anyway.  So
    scoped_disable_commit_resumed doesn't touch the commit_resumed_state
    flag of the native target, it stays true.  When reaching target_wait, in
    startup_inferior (to consume the initial expect stop events while the
    inferior is starting up and working its way through the shell),
    commit_resumed_state is true, breaking the contract saying that
    commit_resumed_state is always false when calling the targets' wait
    method.
    
    (note: to be correct, I think that startup_inferior should toggle
    commit_resumed between the target_wait and target_resume calls, but I'll
    ignore that for now)
    
    I can see multiple ways to fix this.  In the end, we need
    commit_resumed_state to be cleared by the time we get to that
    target_wait.  It could be done at the end of the kill command, or at the
    beginning of the run command.
    
    To keep things in a coherent state, I'd like to make it so that after
    the kill command, when the target is left with no threads, its
    commit_resumed_state flag is left to false.  This way, we can keep
    working with the assumption that a target with no threads (and therefore
    no running threads) has commit_resumed_state == false.
    
    Do this by adding a scoped_disable_commit_resumed in target_kill.  It
    clears the target's commit_resumed_state on entry, and leaves it false
    if the target does not have any resumed thread on exit.  That means,
    even if the target has another inferior with stopped threads,
    commit_resumed_state will be left to false, which makes sense.
    
    Add a test that tries to cover various combinations of actions done
    while an inferior is running (and therefore while commit_resumed_state
    is true on the process target).
    
    Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=28275
    Change-Id: I8e6fe6dc1f475055921520e58cab68024039a1e9
    Approved-By: Andrew Burgess <aburgess@redhat.com>

Should be fixed by the commits shown above.

The gdb-12-branch branch has been updated by Simon Marchi <simark@sourceware.org>:

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

commit 0fd7bbc90133138d12914608ebb68bab16259b78
Author: Andrew Burgess <aburgess@redhat.com>
Date:   Mon Nov 21 12:12:11 2022 -0500

    gdb: fix assert when quitting GDB while a thread is stepping
    
    This commit addresses one of the issues identified in PR gdb/28275.
    
    Bug gdb/28275 identifies a number of situations in which this assert:
    
      Assertion `!proc_target->commit_resumed_state' failed.
    
    could be triggered.  There's actually a number of similar places where
    this assert is found in GDB, the two of interest in gdb/28275 are in
    target_wait and target_stop.
    
    In one of the comments:
    
      https://sourceware.org/bugzilla/show_bug.cgi?id=28275#c1
    
    steps to trigger the assertion within target_stop were identified when
    using a modified version of the gdb.threads/detach-step-over.exp test
    script.
    
    In the gdb.threads/detach-step-over.exp test, we attach to a
    multi-threaded inferior, and continue the inferior in asynchronous
    (background) mode.  Each thread is continuously hitting a conditional
    breakpoint where the condition is always false.  While the inferior is
    running we detach.  The goal is that we detach while GDB is performing a
    step-over for the conditional breakpoint in at least one thread.
    
    While detaching, if a step-over is in progress, then GDB has to
    complete the step over before we can detach.  This involves calling
    target_stop and target_wait (see prepare_for_detach).
    
    As far as gdb/28275 is concerned, the interesting part here, is the
    the process_stratum_target::commit_resumed_state variable must be
    false when target_stop and target_wait are called.
    
    This is currently ensured because, in detach_command (infrun.c), we
    create an instance of scoped_disable_commit_resumed, this ensures that
    when target_detach is called, ::commit_resumed_state will be false.
    
    The modification to the test that I propose here, and which exposed
    the bug, is that, instead of using "detach" to detach from the
    inferior, we instead use "quit".  Quitting GDB after attaching to an
    inferior will cause GDB to first detach, and then exit.
    
    When we quit GDB we end up calling target_detach via a different code
    path, the stack looks like:
    
      #0 target_detach
      #1 kill_or_detach
      #2 quit_force
      #3 quit_command
    
    Along this path there is no scoped_disable_commit_resumed created.
    ::commit_resumed_state can be true when we reach prepare_for_detach,
    which calls target_wait and target_stop, so the assertion will trigger.
    
    In this commit, I propose fixing this by adding the creation of a
    scoped_disable_commit_resumed into target_detach.  This will ensure
    that ::commit_resumed_state is false when calling prepare_for_detach
    from within target_detach.
    
    I did consider placing the scoped_disable_commit_resumed in
    prepare_for_detach, however, placing it in target_detach ensures that
    the target's commit_resumed_state flag is left to false if the detached
    inferior was the last one for that target.  It's the same rationale as
    for patch "gdb: disable commit resumed in target_kill" that comes later
    in this series, but for detach instead of kill.
    
    detach_command still includes a scoped_disable_commit_resumed too, but I
    think it is still relevant to cover the resumption at the end of the
    function.
    
    Co-Authored-By: Simon Marchi <simon.marchi@efficios.com>
    Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=28275
    Change-Id: Ie128f7aba6ef0e018859275eca372e6ea738e96f

The gdb-12-branch branch has been updated by Simon Marchi <simark@sourceware.org>:

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

commit 1aefce82fdf8834c7440747b80656e0d030aa2fd
Author: Simon Marchi <simon.marchi@efficios.com>
Date:   Mon Nov 21 12:12:13 2022 -0500

    gdb: disable commit resumed in target_kill
    
    New in this version:
    
     - Better comment in target_kill
     - Uncomment line in test to avoid hanging when exiting, when testing on
       native-extended-gdbserver
    
    PR 28275 shows that doing a sequence of:
    
     - Run inferior in background (run &)
     - kill that inferior
     - Run again
    
    We get into this assertion:
    
        /home/smarchi/src/binutils-gdb/gdb/target.c:2590: internal-error: target_wait: Assertion `!proc_target->commit_resumed_state' failed.
    
        #0  internal_error_loc (file=0x5606b344e740 "/home/smarchi/src/binutils-gdb/gdb/target.c", line=2590, fmt=0x5606b344d6a0 "%s: Assertion `%s' failed.") at /home/smarchi/src/binutils-gdb/gdbsupport/errors.cc:54
        #1  0x00005606b6296475 in target_wait (ptid=..., status=0x7fffb9390630, options=...) at /home/smarchi/src/binutils-gdb/gdb/target.c:2590
        #2  0x00005606b5767a98 in startup_inferior (proc_target=0x5606bfccb2a0 <the_amd64_linux_nat_target>, pid=3884857, ntraps=1, last_waitstatus=0x0, last_ptid=0x0) at /home/smarchi/src/binutils-gdb/gdb/nat/fork-inferior.c:482
        #3  0x00005606b4e6c9c5 in gdb_startup_inferior (pid=3884857, num_traps=1) at /home/smarchi/src/binutils-gdb/gdb/fork-child.c:132
        #4  0x00005606b50f14a5 in inf_ptrace_target::create_inferior (this=0x5606bfccb2a0 <the_amd64_linux_nat_target>, exec_file=0x604000039f50 "/home/smarchi/build/binutils-gdb/gdb/test", allargs="", env=0x61500000a580, from_tty=1)
            at /home/smarchi/src/binutils-gdb/gdb/inf-ptrace.c:105
        #5  0x00005606b53b6d23 in linux_nat_target::create_inferior (this=0x5606bfccb2a0 <the_amd64_linux_nat_target>, exec_file=0x604000039f50 "/home/smarchi/build/binutils-gdb/gdb/test", allargs="", env=0x61500000a580, from_tty=1)
            at /home/smarchi/src/binutils-gdb/gdb/linux-nat.c:978
        #6  0x00005606b512b79b in run_command_1 (args=0x0, from_tty=1, run_how=RUN_NORMAL) at /home/smarchi/src/binutils-gdb/gdb/infcmd.c:468
        #7  0x00005606b512c236 in run_command (args=0x0, from_tty=1) at /home/smarchi/src/binutils-gdb/gdb/infcmd.c:526
    
    When running the kill command, commit_resumed_state for the
    process_stratum_target (linux-nat, here) is true.  After the kill, when
    there are no more threads, commit_resumed_state is still true, as
    nothing touches this flag during the kill operation.  During the
    subsequent run command, run_command_1 does:
    
        scoped_disable_commit_resumed disable_commit_resumed ("running");
    
    We would think that this would clear the commit_resumed_state flag of
    our native target, but that's not the case, because
    scoped_disable_commit_resumed iterates on non-exited inferiors in order
    to find active process targets.  And after the kill, the inferior is
    exited, and the native target was unpushed from it anyway.  So
    scoped_disable_commit_resumed doesn't touch the commit_resumed_state
    flag of the native target, it stays true.  When reaching target_wait, in
    startup_inferior (to consume the initial expect stop events while the
    inferior is starting up and working its way through the shell),
    commit_resumed_state is true, breaking the contract saying that
    commit_resumed_state is always false when calling the targets' wait
    method.
    
    (note: to be correct, I think that startup_inferior should toggle
    commit_resumed between the target_wait and target_resume calls, but I'll
    ignore that for now)
    
    I can see multiple ways to fix this.  In the end, we need
    commit_resumed_state to be cleared by the time we get to that
    target_wait.  It could be done at the end of the kill command, or at the
    beginning of the run command.
    
    To keep things in a coherent state, I'd like to make it so that after
    the kill command, when the target is left with no threads, its
    commit_resumed_state flag is left to false.  This way, we can keep
    working with the assumption that a target with no threads (and therefore
    no running threads) has commit_resumed_state == false.
    
    Do this by adding a scoped_disable_commit_resumed in target_kill.  It
    clears the target's commit_resumed_state on entry, and leaves it false
    if the target does not have any resumed thread on exit.  That means,
    even if the target has another inferior with stopped threads,
    commit_resumed_state will be left to false, which makes sense.
    
    Add a test that tries to cover various combinations of actions done
    while an inferior is running (and therefore while commit_resumed_state
    is true on the process target).
    
    Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=28275
    Change-Id: I8e6fe6dc1f475055921520e58cab68024039a1e9
    Approved-By: Andrew Burgess <aburgess@redhat.com>

(In reply to cvs-commit@gcc.gnu.org from comment #15)
> The gdb-12-branch branch has been updated by Simon Marchi
> <simark@sourceware.org>:
> 

At https://sourceware.org/gdb/wiki/GDB_12_Release there's a  'see fixed bugs marked "Target Milestone 12.2"' link, which doesn't show this PR, because the target milestone is set to 13.1, which might be because there's no 12.2 target milestone.  Should we add such a milestone, and update this PR's target milestone?

IMO, I think it would be confusing to create 12.2 release that hasn't been created.