Patchwork [review] infrun: mark an exited thread non-executing when attempting to stop

login
register
mail settings
Submitter Simon Marchi (Code Review)
Date Oct. 18, 2019, 1:27 p.m.
Message ID <gerrit.1571405222000.I7cec98f40283773b79255d998511da434e9cd408@gnutoolchain-gerrit.osci.io>
Download mbox | patch
Permalink /patch/35134/
State New
Headers show

Comments

Simon Marchi (Code Review) - Oct. 18, 2019, 1:27 p.m.
Tankut Baris Aktemur has uploaded a new change for review.

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

infrun: mark an exited thread non-executing 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.  In that case, mark the
thread as not-executing and set its state to THREAD_EXITED.

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 = 42
infrun: handle_inferior_event status->kind = exited, status = 42
[Inferior 2 (process 23703) exited with code 052]
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 = 42
infrun: stop_all_threads status->kind = exited, status = 42 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.

gdb/ChangeLog:
2019-10-18  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; mark the corresponding
	thread as THREAD_EXITED and not-executing.

Change-Id: I7cec98f40283773b79255d998511da434e9cd408
---
M gdb/infrun.c
1 file changed, 9 insertions(+), 0 deletions(-)
Simon Marchi (Code Review) - Nov. 4, 2019, 8:35 a.m.
Tankut Baris Aktemur has posted comments on this change.

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


Patch Set 1:

> Patch Set 1: Code-Review+1
> 
> This looks like an old oversight when handling the case of exited threads when we're attempting to stop all of them. Looking at older code, we used to have a message saying a thread had exited while we were stopping it, but it was removed by a cleanup.
> 
> The change looks good for me.

Thank you.  I noticed that the patch has a problem.  It leaves the exited inferior in an alive state with no threads.  It becomes not possible to re-run the program.  I will send a revision, together with updated tests.

Patch

diff --git a/gdb/infrun.c b/gdb/infrun.c
index 66a066f..01fcbf6 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -4383,6 +4383,15 @@ 
 	    {
 	      /* All resumed threads exited
 		 or one thread/process exited/signalled.  */
+	      thread_info *t = find_thread_ptid (event_ptid);
+	      if (t != nullptr)
+		{
+		  t->stop_requested = 0;
+		  t->executing = 0;
+		  t->resumed = 0;
+		  t->control.may_range_step = 0;
+		  t->state = THREAD_EXITED;
+		}
 	    }
 	  else
 	    {