Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: "Tankut Baris Aktemur (Code Review)" <gerrit@gnutoolchain-gerrit.osci.io>
To: Luis Machado <luis.machado@linaro.org>, gdb-patches@sourceware.org
Subject: [review v2] infrun: handle already-exited threads when attempting to stop
Date: Mon, 04 Nov 2019 10:23:00 -0000	[thread overview]
Message-ID: <20191104102336.331FD25B28@gnutoolchain-gerrit.osci.io> (raw)
In-Reply-To: <gerrit.1571405222000.I7cec98f40283773b79255d998511da434e9cd408@gnutoolchain-gerrit.osci.io>

Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/133
......................................................................

infrun: handle already-exited threads when attempting to stop

In stop_all_threads, GDB sends signals to other threads in an attempt
to stop them.  While in a typical scenario the expected wait status is
TARGET_WAITKIND_STOPPED, it is possible that the thread GDB attempted
to stop has already terminated.  If so, a waitstatus other than
TARGET_WAITKIND_STOPPED would be received.  Handle this case
appropriately.

If a wait status that denotes thread termination is ignored, GDB goes
into an infinite loop in stop_all_threads.
E.g.:

~~~
$ gdb ./a.out
(gdb) start
...
(gdb) add-inferior -exec ./a.out
...
(gdb) inferior 2
...
(gdb) start
...
(gdb) set schedule-multiple on
(gdb) set debug infrun 2
(gdb) continue
Continuing.
infrun: clear_proceed_status_thread (process 23419)
infrun: clear_proceed_status_thread (process 23703)
infrun: proceed (addr=0xffffffffffffffff, signal=GDB_SIGNAL_DEFAULT)
infrun: proceed: resuming process 23419
infrun: resume (step=0, signal=GDB_SIGNAL_0), trap_expected=0, current thread [process 23419] at 0x55555555514e
infrun: infrun_async(1)
infrun: prepare_to_wait
infrun: proceed: resuming process 23703
infrun: resume (step=0, signal=GDB_SIGNAL_0), trap_expected=0, current thread [process 23703] at 0x55555555514e
infrun: prepare_to_wait
infrun: target_wait (-1.0.0, status) =
infrun:   23703.23703.0 [process 23703],
infrun:   status->kind = exited, status = 0
infrun: handle_inferior_event status->kind = exited, status = 0
[Inferior 2 (process 23703) exited normally]
infrun: stop_waiting
infrun: stop_all_threads
infrun: stop_all_threads, pass=0, iterations=0
infrun:   process 23419 executing, need stop
infrun: target_wait (-1.0.0, status) =
infrun:   23419.23419.0 [process 23419],
infrun:   status->kind = exited, status = 0
infrun: stop_all_threads status->kind = exited, status = 0 process 23419
infrun:   process 23419 executing, already stopping
infrun: target_wait (-1.0.0, status) =
infrun:   -1.0.0 [process -1],
infrun:   status->kind = no-resumed
infrun: stop_all_threads status->kind = no-resumed process -1
infrun:   process 23419 executing, already stopping
infrun: target_wait (-1.0.0, status) =
infrun:   -1.0.0 [process -1],
infrun:   status->kind = no-resumed
infrun: stop_all_threads status->kind = no-resumed process -1
infrun:   process 23419 executing, already stopping
infrun: target_wait (-1.0.0, status) =
infrun:   -1.0.0 [process -1],
infrun:   status->kind = no-resumed
infrun: stop_all_threads status->kind = no-resumed process -1
infrun:   process 23419 executing, already stopping
infrun: target_wait (-1.0.0, status) =
infrun:   -1.0.0 [process -1],
infrun:   status->kind = no-resumed
infrun: stop_all_threads status->kind = no-resumed process -1
infrun:   process 23419 executing, already stopping
~~~

And this polling goes on forever.  This patch prevents the infinite
looping behavior.  For the same scenario above, we obtain the
following behavior:

~~~
...
(gdb) continue
Continuing.
infrun: clear_proceed_status_thread (process 1635)
infrun: clear_proceed_status_thread (process 1763)
infrun: proceed (addr=0xffffffffffffffff, signal=GDB_SIGNAL_DEFAULT)
infrun: proceed: resuming process 1635
infrun: resume (step=0, signal=GDB_SIGNAL_0), trap_expected=0, current thread [process 1635] at 0x55555555514e
infrun: infrun_async(1)
infrun: prepare_to_wait
infrun: proceed: resuming process 1763
infrun: resume (step=0, signal=GDB_SIGNAL_0), trap_expected=0, current thread [process 1763] at 0x55555555514e
infrun: prepare_to_wait
infrun: target_wait (-1.0.0, status) =
infrun:   1635.1635.0 [process 1635],
infrun:   status->kind = exited, status = 0
infrun: handle_inferior_event status->kind = exited, status = 0
[Inferior 1 (process 1635) exited normally]
infrun: stop_waiting
infrun: stop_all_threads
infrun: stop_all_threads, pass=0, iterations=0
infrun:   process 1763 executing, need stop
infrun: target_wait (-1.0.0, status) =
infrun:   1763.1763.0 [process 1763],
infrun:   status->kind = exited, status = 0
infrun: stop_all_threads status->kind = exited, status = 0 process 1763
[Inferior 2 (process 1763) exited normally]
infrun: stop_all_threads, pass=1, iterations=1
infrun: stop_all_threads done
(gdb) info inferiors
  Num  Description       Executable
* 1    <null>            /path/to/a.out
  2    <null>            /path/to/a.out
(gdb) info threads
No threads.
(gdb)
~~~

gdb/ChangeLog:
2019-11-04  Tankut Baris Aktemur  <tankut.baris.aktemur@intel.com>

	* infrun.c (stop_all_threads): Do NOT ignore
	TARGET_WAITKIND_NO_RESUMED, TARGET_WAITKIND_THREAD_EXITED,
	TARGET_WAITKIND_EXITED, TARGET_WAITKIND_SIGNALLED wait statuses
	received from threads we attempt to stop.

Change-Id: I7cec98f40283773b79255d998511da434e9cd408
---
M gdb/infrun.c
1 file changed, 29 insertions(+), 2 deletions(-)



diff --git a/gdb/infrun.c b/gdb/infrun.c
index f628fd1..45aa1d9 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -4489,8 +4489,35 @@
 	      || ws.kind == TARGET_WAITKIND_EXITED
 	      || ws.kind == TARGET_WAITKIND_SIGNALLED)
 	    {
-	      /* All resumed threads exited
-		 or one thread/process exited/signalled.  */
+	      if (event_ptid != minus_one_ptid)
+		{
+		  thread_info *t = find_thread_ptid (event_ptid);
+		  if (t == nullptr)
+		    {
+		      /* This is the first time we see this thread.  */
+		      t = add_thread (event_ptid);
+		    }
+
+		  /* Set the threads as non-executing to avoid infinitely
+		     waiting for them to stop.  */
+		  mark_non_executing_threads (event_ptid, ws);
+
+		  if (ws.kind == TARGET_WAITKIND_NO_RESUMED)
+		    {
+		      /* Do nothing.  Already marked the threads.  */
+		    }
+		  if (ws.kind == TARGET_WAITKIND_THREAD_EXITED)
+		    delete_thread (t);
+		  else
+		    {
+		      /* TARGET_WAITKIND_EXITED or
+			 TARGET_WAITKIND_SIGNALLED.  */
+		      /* Need to restore the context because
+			 handle_inferior_exit switches it.  */
+		      scoped_restore_current_pspace_and_thread restore;
+		      handle_inferior_exit (event_ptid, ws);
+		    }
+		}
 	    }
 	  else
 	    {

-- 
Gerrit-Project: binutils-gdb
Gerrit-Branch: master
Gerrit-Change-Id: I7cec98f40283773b79255d998511da434e9cd408
Gerrit-Change-Number: 133
Gerrit-PatchSet: 2
Gerrit-Owner: Tankut Baris Aktemur <tankut.baris.aktemur@intel.com>
Gerrit-Reviewer: Luis Machado <luis.machado@linaro.org>
Gerrit-Reviewer: Tankut Baris Aktemur <tankut.baris.aktemur@intel.com>
Gerrit-MessageType: newpatchset


  parent reply	other threads:[~2019-11-04 10:23 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-10-18 13:27 [review] infrun: mark an exited thread non-executing " Tankut Baris Aktemur (Code Review)
2019-11-04  8:36 ` Tankut Baris Aktemur (Code Review)
2019-11-04 10:23 ` Tankut Baris Aktemur (Code Review) [this message]
2019-12-04 12:02 ` [review v2] infrun: handle already-exited threads " Tankut Baris Aktemur (Code Review)
2019-12-05 19:26 ` Pedro Alves (Code Review)
2019-12-06 16:19 ` Pedro Alves (Code Review)
2019-12-06 17:41 ` Tankut Baris Aktemur (Code Review)
2019-12-09 15:09 ` Tankut Baris Aktemur (Code Review)
2020-01-08 15:57 ` Tankut Baris Aktemur (Code Review)
2020-01-29 17:54 ` Tom de Vries (Code Review)
2020-01-30  9:10 ` Tom de Vries (Code Review)
2020-01-30 13:58 ` Tom de Vries (Code Review)
2020-01-30 16:41 ` Tankut Baris Aktemur (Code Review)
2020-01-30 21:06 ` Tom de Vries (Code Review)
2020-02-03 15:13 ` Tom de Vries (Code Review)
2020-02-03 16:02 ` Tankut Baris Aktemur (Code Review)
2020-02-03 16:27 ` Tom de Vries (Code Review)
2020-02-04  9:06 ` Tankut Baris Aktemur (Code Review)
2020-02-05 13:19 ` [review v3] " Tankut Baris Aktemur (Code Review)
2020-02-05 13:24 ` Tankut Baris Aktemur (Code Review)
2020-02-05 22:59 ` Tom de Vries (Code Review)
2020-02-07 12:11 ` Tankut Baris Aktemur (Code Review)

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20191104102336.331FD25B28@gnutoolchain-gerrit.osci.io \
    --to=gerrit@gnutoolchain-gerrit.osci.io \
    --cc=gdb-patches@sourceware.org \
    --cc=luis.machado@linaro.org \
    --cc=tankut.baris.aktemur@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox