[1/3] Fix any_thread_of_inferior

Message ID 20210315234339.457551-2-pedro@palves.net
State New
Headers show
Series
  • Fix gdbserver + "maint set target-non-stop" problems
Related show

Commit Message

Pedro Alves March 15, 2021, 11:43 p.m.
Running gdb-term.exp against gdbserver with "maint set target-non-stop
on", runs into this:

  [infrun] fetch_inferior_event: exit
  [infrun] fetch_inferior_event: enter
  /home/pedro/gdb/binutils-gdb/src/gdb/thread.c:72: internal-error: thread_info* inferior_thread(): Assertion `current_thread_ != nullptr' failed.
  A problem internal to GDB has been detected,
  further debugging may prove unreliable.

  This is a bug, please report it.  For instructions, see:
  <https://www.gnu.org/software/gdb/bugs/>.

  FAIL: gdb.base/gdb-sigterm.exp: expect eof #2 (GDB internal error)
  Resyncing due to internal error.
  ERROR: : spawn id exp9 not open
      while executing
  "expect {
  -i exp9 -timeout 10
	      -re "Quit this debugging session\\? \\(y or n\\) $" {
		  send_gdb "n\n" answer
		  incr count
	      }
	      -re "Create ..."
      ("uplevel" body line 1)
      invoked from within
  "uplevel $body" NONE : spawn id exp9 not open
  ERROR: Could not resync from internal error (timeout)
  gdb.base/gdb-sigterm.exp: expect eof #2: stepped 0 times
  UNRESOLVED: gdb.base/gdb-sigterm.exp: 50 SIGTERM passes

The assertion fails here:

  ...
  #5  0x000055af4b4a7164 in internal_error (file=0x55af4b5e5de8 "/home/pedro/gdb/binutils-gdb/src/gdb/thread.c", line=72, fmt=0x55af4b5e5ce9 "%s: Assertion `%s' failed.") at /home/pedro/gdb/binutils-gdb/src/gdbsupport/errors.cc:55
  #6  0x000055af4b25fc43 in inferior_thread () at /home/pedro/gdb/binutils-gdb/src/gdb/thread.c:72
  #7  0x000055af4b26177e in any_thread_of_inferior (inf=0x55af4cf874f0) at /home/pedro/gdb/binutils-gdb/src/gdb/thread.c:638
  #8  0x000055af4b26eec8 in kill_or_detach (inf=0x55af4cf874f0, from_tty=0) at /home/pedro/gdb/binutils-gdb/src/gdb/top.c:1665
  #9  0x000055af4b26f37f in quit_force (exit_arg=0x0, from_tty=0) at /home/pedro/gdb/binutils-gdb/src/gdb/top.c:1767
  #10 0x000055af4b2f72a7 in quit () at /home/pedro/gdb/binutils-gdb/src/gdb/utils.c:633
  #11 0x000055af4b2f730b in maybe_quit () at /home/pedro/gdb/binutils-gdb/src/gdb/utils.c:657
  #12 0x000055af4b1adb74 in ser_base_wait_for (scb=0x55af4d02e460, timeout=0) at /home/pedro/gdb/binutils-gdb/src/gdb/ser-base.c:236
  #13 0x000055af4b1adf0f in do_ser_base_readchar (scb=0x55af4d02e460, timeout=0) at /home/pedro/gdb/binutils-gdb/src/gdb/ser-base.c:365
  #14 0x000055af4b1ae06d in generic_readchar (scb=0x55af4d02e460, timeout=0, do_readchar=0x55af4b1adeb1 <do_ser_base_readchar(serial*, int)>) at /home/pedro/gdb/binutils-gdb/src/gdb/ser-base.c:444
  ...

The bug is that any_thread_of_inferior incorrectly assumes that
there's always a selected thread.  This fixes it.

gdb/ChangeLog:

	* thread.c (any_thread_of_inferior): Check if there's a selected
	thread before calling inferior_thread().

Change-Id: Ica4b9ec746121a7a7c22bef09baea72103b3853d
---
 gdb/thread.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

-- 
2.26.2

Comments

Lancelot SIX via Gdb-patches March 17, 2021, 3:14 p.m. | #1
On 2021-03-15 7:43 p.m., Pedro Alves wrote:
> Running gdb-term.exp against gdbserver with "maint set target-non-stop

> on", runs into this:

> 

>   [infrun] fetch_inferior_event: exit

>   [infrun] fetch_inferior_event: enter

>   /home/pedro/gdb/binutils-gdb/src/gdb/thread.c:72: internal-error: thread_info* inferior_thread(): Assertion `current_thread_ != nullptr' failed.

>   A problem internal to GDB has been detected,

>   further debugging may prove unreliable.

> 

>   This is a bug, please report it.  For instructions, see:

>   <https://www.gnu.org/software/gdb/bugs/>.

> 

>   FAIL: gdb.base/gdb-sigterm.exp: expect eof #2 (GDB internal error)

>   Resyncing due to internal error.

>   ERROR: : spawn id exp9 not open

>       while executing

>   "expect {

>   -i exp9 -timeout 10

> 	      -re "Quit this debugging session\\? \\(y or n\\) $" {

> 		  send_gdb "n\n" answer

> 		  incr count

> 	      }

> 	      -re "Create ..."

>       ("uplevel" body line 1)

>       invoked from within

>   "uplevel $body" NONE : spawn id exp9 not open

>   ERROR: Could not resync from internal error (timeout)

>   gdb.base/gdb-sigterm.exp: expect eof #2: stepped 0 times

>   UNRESOLVED: gdb.base/gdb-sigterm.exp: 50 SIGTERM passes

> 

> The assertion fails here:

> 

>   ...

>   #5  0x000055af4b4a7164 in internal_error (file=0x55af4b5e5de8 "/home/pedro/gdb/binutils-gdb/src/gdb/thread.c", line=72, fmt=0x55af4b5e5ce9 "%s: Assertion `%s' failed.") at /home/pedro/gdb/binutils-gdb/src/gdbsupport/errors.cc:55

>   #6  0x000055af4b25fc43 in inferior_thread () at /home/pedro/gdb/binutils-gdb/src/gdb/thread.c:72

>   #7  0x000055af4b26177e in any_thread_of_inferior (inf=0x55af4cf874f0) at /home/pedro/gdb/binutils-gdb/src/gdb/thread.c:638

>   #8  0x000055af4b26eec8 in kill_or_detach (inf=0x55af4cf874f0, from_tty=0) at /home/pedro/gdb/binutils-gdb/src/gdb/top.c:1665

>   #9  0x000055af4b26f37f in quit_force (exit_arg=0x0, from_tty=0) at /home/pedro/gdb/binutils-gdb/src/gdb/top.c:1767

>   #10 0x000055af4b2f72a7 in quit () at /home/pedro/gdb/binutils-gdb/src/gdb/utils.c:633

>   #11 0x000055af4b2f730b in maybe_quit () at /home/pedro/gdb/binutils-gdb/src/gdb/utils.c:657

>   #12 0x000055af4b1adb74 in ser_base_wait_for (scb=0x55af4d02e460, timeout=0) at /home/pedro/gdb/binutils-gdb/src/gdb/ser-base.c:236

>   #13 0x000055af4b1adf0f in do_ser_base_readchar (scb=0x55af4d02e460, timeout=0) at /home/pedro/gdb/binutils-gdb/src/gdb/ser-base.c:365

>   #14 0x000055af4b1ae06d in generic_readchar (scb=0x55af4d02e460, timeout=0, do_readchar=0x55af4b1adeb1 <do_ser_base_readchar(serial*, int)>) at /home/pedro/gdb/binutils-gdb/src/gdb/ser-base.c:444

>   ...

> 

> The bug is that any_thread_of_inferior incorrectly assumes that

> there's always a selected thread.  This fixes it.

> 

> gdb/ChangeLog:

> 

> 	* thread.c (any_thread_of_inferior): Check if there's a selected

> 	thread before calling inferior_thread().

> 

> Change-Id: Ica4b9ec746121a7a7c22bef09baea72103b3853d

> ---

>  gdb/thread.c | 4 ++--

>  1 file changed, 2 insertions(+), 2 deletions(-)

> 

> diff --git a/gdb/thread.c b/gdb/thread.c

> index 3e7d6e14bf7..fc6db96fbcb 100644

> --- a/gdb/thread.c

> +++ b/gdb/thread.c

> @@ -637,8 +637,8 @@ any_thread_of_inferior (inferior *inf)

>  {

>    gdb_assert (inf->pid != 0);

>  

> -  /* Prefer the current thread.  */

> -  if (inf == current_inferior ())

> +  /* Prefer the current thread, if there's one.  */

> +  if (inf == current_inferior () && inferior_ptid != null_ptid)

>      return inferior_thread ();

>  

>    for (thread_info *tp : inf->non_exited_threads ())

> 


Makes sense.  current_thread_ is always supposed to be in sync with
inferior_ptid because we are always supposed to use switch_to_thread or
similar function to switch thread, right?

Simon
Pedro Alves March 19, 2021, 7:37 p.m. | #2
On 17/03/21 15:14, Simon Marchi wrote:

> On 2021-03-15 7:43 p.m., Pedro Alves wrote:


>> diff --git a/gdb/thread.c b/gdb/thread.c

>> index 3e7d6e14bf7..fc6db96fbcb 100644

>> --- a/gdb/thread.c

>> +++ b/gdb/thread.c

>> @@ -637,8 +637,8 @@ any_thread_of_inferior (inferior *inf)

>>  {

>>    gdb_assert (inf->pid != 0);

>>  

>> -  /* Prefer the current thread.  */

>> -  if (inf == current_inferior ())

>> +  /* Prefer the current thread, if there's one.  */

>> +  if (inf == current_inferior () && inferior_ptid != null_ptid)

>>      return inferior_thread ();

>>  

>>    for (thread_info *tp : inf->non_exited_threads ())

>>

> 

> Makes sense.  current_thread_ is always supposed to be in sync with

> inferior_ptid because we are always supposed to use switch_to_thread or

> similar function to switch thread, right?


Right.  Part of the multi-target work was ensuring that was done throughout
the codebase.

I've merged this patch.

Thanks,
Pedro Alves

Patch

diff --git a/gdb/thread.c b/gdb/thread.c
index 3e7d6e14bf7..fc6db96fbcb 100644
--- a/gdb/thread.c
+++ b/gdb/thread.c
@@ -637,8 +637,8 @@  any_thread_of_inferior (inferior *inf)
 {
   gdb_assert (inf->pid != 0);
 
-  /* Prefer the current thread.  */
-  if (inf == current_inferior ())
+  /* Prefer the current thread, if there's one.  */
+  if (inf == current_inferior () && inferior_ptid != null_ptid)
     return inferior_thread ();
 
   for (thread_info *tp : inf->non_exited_threads ())