[review v2] infrun: handle already-exited threads when attempting to stop

["Tankut Baris Aktemur (Code Review)" <gerrit@gnutoolchain-gerrit.osci.io>]

Tankut Baris Aktemur has posted comments on this change.

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


Patch Set 2:

(1 comment)

> Patch Set 2:
> 
> (1 comment)

| --- gdb/infrun.c
| +++ gdb/infrun.c
| @@ -4494,7 +4509,19 @@ stop_all_threads (void)
| +		  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);

PS2, Line 4518:

> The fix for this I think must be around leaving the TARGET_WAITKIND_EXITED/TARGET_WAITKIND_SIGNALLED
event pending, so that it is processed later when we're out of the
stop_all_threads loop and back to
dequeuing the next event.


I'd like to ask for your opinion on making the second exit event
pending.  One problem is, because the event has not been reported to
the user yet, the user still thinks that the inferior is alive.  So,
after getting the prompt because of the first exit event, they may be
tempted to do "info threads" or switch to the not-yet-reported-
inferior and inspect its state.  This triggers a query (e.g. of
registers) on the process that is already gone.  I tried the following
scenario with the current master branch (the patch that I proposed was
not applied):

~~~
$ gdb ./a.out
(gdb) maint set target-non-stop off
(gdb) start
...
(gdb) add-inferior -exec ./a.out
[New inferior 2]
Added inferior 2
...
(gdb) inferior 2
[Switching to inferior 2 [<null>] (/tmp/a.out)]
(gdb) start
...
(gdb) set schedule-multiple on
(gdb) c
Continuing.
[Inferior 2 (process 16331) exited normally]
(gdb) i inferiors
  Num  Description       Executable
  1    process 16137     /tmp/a.out
* 2    <null>            /tmp/a.out
(gdb) inferior 1
[Switching to inferior 1 [process 16137] (/tmp/a.out)]
[Switching to thread 1.1 (process 16137)]
Couldn't get registers: No such process.
(gdb) i threads
  Id   Target Id         Frame
Couldn't get registers: No such process.
(gdb) c
Continuing.
Couldn't get registers: No such process.
~~~

If I save the exit event in my patch as a pending event (and omit
'maint set target-non-stop off'), I get essentially the same problem.
What is the expected GDB behavior here?  Would it be alright to
actually print both exit events, followed by the gdb-prompt, where the
user can now query $_exitcode or $_exitsignal by switching between
inferiors, assuming those special variables are set correctly per
inferior?

| +		    }
| +		}
|  	    }
|  	  else
|  	    {
|  	      thread_info *t = find_thread_ptid (event_ptid);
|  	      if (t == NULL)
|  		t = add_thread (event_ptid);
|  

-- 
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: Pedro Alves <palves@redhat.com>
Gerrit-Reviewer: Tankut Baris Aktemur <tankut.baris.aktemur@intel.com>
Gerrit-Comment-Date: Mon, 09 Dec 2019 15:09:06 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Pedro Alves <palves@redhat.com>
Comment-In-Reply-To: Tankut Baris Aktemur <tankut.baris.aktemur@intel.com>
Gerrit-MessageType: comment