Re: [PATCH][gdb] Fix hang after ext sigkill

[Simon Marchi <simark@simark.ca>]

[-- Attachment #1: Type: text/plain, Size: 3811 bytes --]

Hi Tom,

The test fails for me more often than not.  I've attached a gdb.log showing one such
failure.  There seems to be a problem matching the output of the last "continue".

This is what I see when I reproduce the case by hand:

(gdb) c
Continuing.
Couldn't get registers: No such process.
Couldn't get registers: No such process.
(gdb) [Thread 0x7ffff7d99700 (LWP 514079) exited]

Program terminated with signal SIGKILL, Killed.
The program no longer exists.

I didn't really understand why we saw a prompt coming back before the other messages,
so I looked into it a bit and this is what I think happens:

1. While we are stopped at the prompt, the linux-nat event handler is unregistered from the event loop
2. Inferior gets SIGKILL'ed, so GDB gets SIGCHLD'ed, that posts an event to the event pipe, but since it's
   not registered in the event loop, nothing happens
3. User does continue, the linux-nat event handler gets registered with the event loop.  Then, the continue
   fails (No such process), which brings us back at the prompt.  However, the linux-nat event handler stays
   registered with the event loop.
4. When we come back to the event loop, we process the event for the SIGKILL, which makes GDB print the
   thread exit message and "Program terminated" message.

Normally, after the "continue" fails, I don't think we would want to leave the linux-nat
handler registered with the event loop: if it was not registered before, why should it be
registered after?  However, if it wasn't left there, we wouldn't see the messages saying
the program has terminated, so that wouldn't be good either.

Maybe there's a better way to handle it?

However, I still think it would be a good idea to merge a patch like yours, it's already a
step forward (especially since it fixes a regression).

> diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
> index 81518b9646..745694df2d 100644
> --- a/gdb/testsuite/lib/gdb.exp
> +++ b/gdb/testsuite/lib/gdb.exp
> @@ -571,7 +571,7 @@ proc runto { function args } {
>  	    }
>  	    return 1
>  	}
> -	-re "Breakpoint \[0-9\]*, \[0-9xa-f\]* in .*$gdb_prompt $" { 
> +	-re "\[Bb\]reakpoint \[0-9\]*, \[0-9xa-f\]* in .*$gdb_prompt $" {
>  	    if { $print_pass } {
>  		pass $test_name
>  	    }
> diff --git a/gdb/thread.c b/gdb/thread.c
> index 54b59e2244..9876ca3c76 100644
> --- a/gdb/thread.c
> +++ b/gdb/thread.c
> @@ -1444,6 +1444,8 @@ scoped_restore_current_thread::restore ()
>  
>  scoped_restore_current_thread::~scoped_restore_current_thread ()
>  {
> +  if (m_inf == NULL)
> +    return;
>    if (!m_dont_restore)
>      {
>        try
> @@ -1488,7 +1490,17 @@ scoped_restore_current_thread::scoped_restore_current_thread ()
>        else
>  	frame = NULL;
>  
> -      m_selected_frame_id = get_frame_id (frame);
> +      try
> +       {
> +	 m_selected_frame_id = get_frame_id (frame);
> +       }
> +      catch (const gdb_exception &ex)
> +       {
> +	 m_inf = NULL;
> +	 m_selected_frame_id = null_frame_id;
> +	 m_selected_frame_level = -1;
> +	 return;
> +       }

The indentation is a bit off here.

Instead of clearing everything, I think we should just set m_selected_frame_id to
null_frame_id and m_selected_frame_level to -1.  m_inf and m_thread can still be
set.  This way, the right inferior will be restored, at least, I think this is
desirable.  scoped_restore_current_thread::restore handles well the case where the
thread to restore has exited.  The thread_info object is refcounted for this exact
use case, where the thread would get deleted while

In other words:

      try
	{
	  m_selected_frame_id = get_frame_id (frame);
	  m_selected_frame_level = frame_relative_level (frame);
	}
      catch (const gdb_exception &ex)
	{
	  m_selected_frame_id = null_frame_id;
	  m_selected_frame_level = -1;
	}

Simon

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: gdb.log --]
[-- Type: text/x-log; name="gdb.log", Size: 7571 bytes --]

Test run by simark on Tue Mar 24 10:46:56 2020
Native configuration is x86_64-pc-linux-gnu

		=== gdb tests ===

Schedule of variations:
    unix

Running target unix
Using /usr/share/dejagnu/baseboards/unix.exp as board description file for target.
Using /usr/share/dejagnu/config/unix.exp as generic interface file for target.
Using /home/simark/src/binutils-gdb/gdb/testsuite/config/unix.exp as tool-and-target-specific interface file.
Running /home/simark/src/binutils-gdb/gdb/testsuite/gdb.threads/hang-after-ext-sigkill.exp ...
get_compiler_info: gcc-9-3-0
Executing on host: gcc  -fno-stack-protector  -fdiagnostics-color=never -c  -o /home/simark/build/binutils-gdb/gdb/testsuite/outputs/gdb.threads/hang-after-ext-sigkill/hang-after-ext-sigkill0.o /home/simark/src/binutils-gdb/gdb/testsuite/gdb.threads/hang-after-ext-sigkill.c    (timeout = 300)
spawn -ignore SIGHUP gcc -fno-stack-protector -fdiagnostics-color=never -c -o /home/simark/build/binutils-gdb/gdb/testsuite/outputs/gdb.threads/hang-after-ext-sigkill/hang-after-ext-sigkill0.o /home/simark/src/binutils-gdb/gdb/testsuite/gdb.threads/hang-after-ext-sigkill.c
Executing on host: gcc  -fno-stack-protector /home/simark/build/binutils-gdb/gdb/testsuite/outputs/gdb.threads/hang-after-ext-sigkill/hang-after-ext-sigkill0.o  -fdiagnostics-color=never  -lpthreads -lm   -o /home/simark/build/binutils-gdb/gdb/testsuite/outputs/gdb.threads/hang-after-ext-sigkill/hang-after-ext-sigkill    (timeout = 300)
spawn -ignore SIGHUP gcc -fno-stack-protector /home/simark/build/binutils-gdb/gdb/testsuite/outputs/gdb.threads/hang-after-ext-sigkill/hang-after-ext-sigkill0.o -fdiagnostics-color=never -lpthreads -lm -o /home/simark/build/binutils-gdb/gdb/testsuite/outputs/gdb.threads/hang-after-ext-sigkill/hang-after-ext-sigkill
/usr/bin/ld: cannot find -lpthreads
collect2: error: ld returned 1 exit status
compiler exited with status 1
output is:
/usr/bin/ld: cannot find -lpthreads
collect2: error: ld returned 1 exit status

Executing on host: gcc  -fno-stack-protector /home/simark/build/binutils-gdb/gdb/testsuite/outputs/gdb.threads/hang-after-ext-sigkill/hang-after-ext-sigkill0.o  -fdiagnostics-color=never  -lpthread -lm   -o /home/simark/build/binutils-gdb/gdb/testsuite/outputs/gdb.threads/hang-after-ext-sigkill/hang-after-ext-sigkill    (timeout = 300)
spawn -ignore SIGHUP gcc -fno-stack-protector /home/simark/build/binutils-gdb/gdb/testsuite/outputs/gdb.threads/hang-after-ext-sigkill/hang-after-ext-sigkill0.o -fdiagnostics-color=never -lpthread -lm -o /home/simark/build/binutils-gdb/gdb/testsuite/outputs/gdb.threads/hang-after-ext-sigkill/hang-after-ext-sigkill
PASS: gdb.threads/hang-after-ext-sigkill.exp: successfully compiled posix threads test case
spawn /home/simark/build/binutils-gdb/gdb/testsuite/../../gdb/gdb -nw -nx -data-directory /home/simark/build/binutils-gdb/gdb/testsuite/../data-directory
warning: Found custom handler for signal 7 (Bus error) preinstalled.
warning: Found custom handler for signal 8 (Floating point exception) preinstalled.
warning: Found custom handler for signal 11 (Segmentation fault) preinstalled.
Some signal dispositions inherited from the environment (SIG_DFL/SIG_IGN)
won't be propagated to spawned programs.
GNU gdb (GDB) 10.0.50.20200324-git
Copyright (C) 2020 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.
Type "show copying" and "show warranty" for details.
This GDB was configured as "x86_64-pc-linux-gnu".
Type "show configuration" for configuration details.
For bug reporting instructions, please see:
<http://www.gnu.org/software/gdb/bugs/>.
Find the GDB manual and other documentation resources online at:
    <http://www.gnu.org/software/gdb/documentation/>.

For help, type "help".
Type "apropos word" to search for commands related to "word".
(gdb) set height 0
(gdb) set width 0
(gdb) dir
Reinitialize source path to empty? (y or n) y
Source directories searched: $cdir:$cwd
(gdb) dir /home/simark/src/binutils-gdb/gdb/testsuite/gdb.threads
Source directories searched: /home/simark/src/binutils-gdb/gdb/testsuite/gdb.threads:$cdir:$cwd
(gdb) kill
The program is not being run.
(gdb) file /home/simark/build/binutils-gdb/gdb/testsuite/outputs/gdb.threads/hang-after-ext-sigkill/hang-after-ext-sigkill
Reading symbols from /home/simark/build/binutils-gdb/gdb/testsuite/outputs/gdb.threads/hang-after-ext-sigkill/hang-after-ext-sigkill...
(No debugging symbols found in /home/simark/build/binutils-gdb/gdb/testsuite/outputs/gdb.threads/hang-after-ext-sigkill/hang-after-ext-sigkill)
(gdb) delete breakpoints
(gdb) info breakpoints
No breakpoints or watchpoints.
(gdb) tbreak main
Temporary breakpoint 1 at 0x1165
(gdb) run 
Starting program: /home/simark/build/binutils-gdb/gdb/testsuite/outputs/gdb.threads/hang-after-ext-sigkill/hang-after-ext-sigkill 
[Thread debugging using libthread_db enabled]
Using host libthread_db library "/usr/lib/../lib/libthread_db.so.1".

Temporary breakpoint 1, 0x0000555555555165 in main ()
(gdb) info inferior 1
  Num  Description       Connection           Executable        
* 1    process 511301    1 (native)           /home/simark/build/binutils-gdb/gdb/testsuite/outputs/gdb.threads/hang-after-ext-sigkill/hang-after-ext-sigkill 
(gdb) PASS: gdb.threads/hang-after-ext-sigkill.exp: get inferior pid
continue
Continuing.
PASS: gdb.threads/hang-after-ext-sigkill.exp: continue
[New Thread 0x7ffff7c53700 (LWP 511305)]
^C
Thread 2 "hang-after-ext-" received signal SIGINT, Interrupt.
[Switching to Thread 0x7ffff7c53700 (LWP 511305)]
0x00007ffff7d563c5 in clone () from /usr/lib/libc.so.6
(gdb) PASS: gdb.threads/hang-after-ext-sigkill.exp: get sigint
shell kill -s SIGKILL 511301
(gdb) PASS: gdb.threads/hang-after-ext-sigkill.exp: shell kill -s SIGKILL pid
continue
Continuing.
Couldn't get registers: No such process.
(gdb) continue
Continuing.
Couldn't get registers: No such process.
(gdb) FAIL: gdb.threads/hang-after-ext-sigkill.exp: continue
testcase /home/simark/src/binutils-gdb/gdb/testsuite/gdb.threads/hang-after-ext-sigkill.exp completed in 1 seconds

		=== gdb Summary ===

# of expected passes		5
# of unexpected failures	1
Executing on host: /home/simark/build/binutils-gdb/gdb/testsuite/../../gdb/gdb -nw -nx -data-directory /home/simark/build/binutils-gdb/gdb/testsuite/../data-directory --version    (timeout = 300)
spawn -ignore SIGHUP /home/simark/build/binutils-gdb/gdb/testsuite/../../gdb/gdb -nw -nx -data-directory /home/simark/build/binutils-gdb/gdb/testsuite/../data-directory --version
warning: Found custom handler for signal 7 (Bus error) preinstalled.
warning: Found custom handler for signal 8 (Floating point exception) preinstalled.
warning: Found custom handler for signal 11 (Segmentation fault) preinstalled.
Some signal dispositions inherited from the environment (SIG_DFL/SIG_IGN)
won't be propagated to spawned programs.
GNU gdb (GDB) 10.0.50.20200324-git
Copyright (C) 2020 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.
/home/simark/build/binutils-gdb/gdb/gdb version  11 -nw -nx -data-directory /home/simark/build/binutils-gdb/gdb/testsuite/../data-directory 

runtest completed at Tue Mar 24 10:46:58 2020