Patchwork PR server/18081: gdbserver crashes when providing an unexisting binary

login
register
mail settings
Submitter Pedro Alves
Date April 7, 2015, 6:29 p.m.
Message ID <1428431386-16530-1-git-send-email-palves@redhat.com>
Download mbox | patch
Permalink /patch/6076/
State New
Headers show

Comments

Pedro Alves - April 7, 2015, 6:29 p.m.
$ ./gdbserver :1234 blah
 Process blah created; pid = 16471
 Cannot exec blah: No such file or directory.

 Child exited with status 127
 Killing process(es): 16471
 ../../../../src/binutils-gdb/gdb/gdbserver/linux-low.c:920: A problem internal to GDBserver has been detected.
 kill_wait_lwp: Assertion `res > 0' failed.

GDBserver shouldn't even be trying to kill that process.  GDBserver
kills or detaches from all processes on exit, and due to a missing
mourn_inferior call, GDBserver tries to kill the process that it had
already seen exit.

Tested on x86_64 Fedora 20.  New test included.  I emulated what
Windows outputs by hacking an error call in linux_create_inferior.

gdb/gdbserver/ChangeLog:
2015-04-07  Pedro Alves  <palves@redhat.com>

	PR server/18081
	* server.c (start_inferior): If the process exits, mourn it.

gdb/testsuite/ChangeLog:
2015-04-07  Pedro Alves  <palves@redhat.com>

	PR server/18081
	* gdb.server/non-existing-program.exp: New file.
---
 gdb/gdbserver/server.c                            |  2 +
 gdb/testsuite/gdb.server/non-existing-program.exp | 63 +++++++++++++++++++++++
 2 files changed, 65 insertions(+)
 create mode 100644 gdb/testsuite/gdb.server/non-existing-program.exp
Simon Marchi - April 8, 2015, 7:30 p.m.
On 15-04-07 02:29 PM, Pedro Alves wrote:
>  $ ./gdbserver :1234 blah
>  Process blah created; pid = 16471
>  Cannot exec blah: No such file or directory.
> 
>  Child exited with status 127
>  Killing process(es): 16471
>  ../../../../src/binutils-gdb/gdb/gdbserver/linux-low.c:920: A problem internal to GDBserver has been detected.
>  kill_wait_lwp: Assertion `res > 0' failed.
> 
> GDBserver shouldn't even be trying to kill that process.  GDBserver
> kills or detaches from all processes on exit, and due to a missing
> mourn_inferior call, GDBserver tries to kill the process that it had
> already seen exit.
> 
> Tested on x86_64 Fedora 20.  New test included.  I emulated what
> Windows outputs by hacking an error call in linux_create_inferior.
> 
> gdb/gdbserver/ChangeLog:
> 2015-04-07  Pedro Alves  <palves@redhat.com>
> 
> 	PR server/18081
> 	* server.c (start_inferior): If the process exits, mourn it.
> 
> gdb/testsuite/ChangeLog:
> 2015-04-07  Pedro Alves  <palves@redhat.com>
> 
> 	PR server/18081
> 	* gdb.server/non-existing-program.exp: New file.
> ---
>  gdb/gdbserver/server.c                            |  2 +
>  gdb/testsuite/gdb.server/non-existing-program.exp | 63 +++++++++++++++++++++++
>  2 files changed, 65 insertions(+)
>  create mode 100644 gdb/testsuite/gdb.server/non-existing-program.exp
> 
> diff --git a/gdb/gdbserver/server.c b/gdb/gdbserver/server.c
> index 3408ef7..d57674d 100644
> --- a/gdb/gdbserver/server.c
> +++ b/gdb/gdbserver/server.c
> @@ -284,6 +284,8 @@ start_inferior (char **argv)
>        current_thread->last_resume_kind = resume_stop;
>        current_thread->last_status = last_status;
>      }
> +  else
> +    mourn_inferior (find_process_pid (ptid_get_pid (last_ptid)));
>  
>    return signal_pid;
>  }
> diff --git a/gdb/testsuite/gdb.server/non-existing-program.exp b/gdb/testsuite/gdb.server/non-existing-program.exp
> new file mode 100644
> index 0000000..f842c44
> --- /dev/null
> +++ b/gdb/testsuite/gdb.server/non-existing-program.exp
> @@ -0,0 +1,63 @@
> +# This testcase is part of GDB, the GNU debugger.
> +
> +# Copyright 2015 Free Software Foundation, Inc.
> +
> +# This program is free software; you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as published by
> +# the Free Software Foundation; either version 3 of the License, or
> +# (at your option) any later version.
> +#
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program.  If not, see <http://www.gnu.org/licenses/>.
> +
> +# Test starting gdbserver passing it the name of a non-existing
> +# program.
> +
> +load_lib gdbserver-support.exp
> +
> +standard_testfile
> +
> +if { [skip_gdbserver_tests] } {
> +    return 0
> +}
> +
> +set gdbserver [find_gdbserver]
> +if { $gdbserver == "" } {
> +    fail "could not find gdbserver"
> +    return
> +}
> +
> +# Fire off gdbserver.  The port doesn't really matter, gdbserver tries
> +# to spawn the program before opening the connection.
> +set spawn_id [remote_spawn target "$gdbserver stdio non-existing-program"]
> +
> +set msg "gdbserver exits cleanly"
> +set saw_exiting 0
> +expect {
> +    # This is what we get on ptrace-based targets.
> +    -re "stdin/stdout redirected.*No program to debug\r\nExiting\r\n$" {
> +	set saw_exiting 1
> +	exp_continue
> +    }
> +    # This is what we get on Windows.
> +    -re "Error creating process\r\n\r\nExiting\r\n$" {
> +	set saw_exiting 1
> +	exp_continue
> +    }
> +    -re "A problem internal to GDBserver has been detected" {
> +	fail "$msg (GDBserver internal error)"
> +	wait
> +    }
> +    eof {
> +	gdb_assert $saw_exiting $msg
> +	wait
> +    }
> +    timeout {
> +	fail "$msg (timeout)"
> +    }
> +}

It's much better now, thanks!
Pedro Alves - May 6, 2015, 6:14 p.m.
On 04/08/2015 08:30 PM, Simon Marchi wrote:

> It's much better now, thanks!

FYI, I've pushed this in now.

Thanks,
Pedro Alves
Luis Machado - May 8, 2015, 12:53 p.m.
On 05/06/2015 03:14 PM, Pedro Alves wrote:
> On 04/08/2015 08:30 PM, Simon Marchi wrote:
>
>> It's much better now, thanks!
>
> FYI, I've pushed this in now.

For some reason the next test after that now fails and stops the 
testsuite run.


Running 
../../../gdb-head-ro/gdb/testsuite/gdb.server/non-existing-program.exp ...
Running 
../../../gdb-head-ro/gdb/testsuite/gdb.server/server-exec-info.exp ...
can not find channel named "exp8"
     while executing
"match_max [match_max -d]"
     (procedure "default_gdb_init" line 26)
     invoked from within
"default_gdb_init $test_file_name"
     (procedure "gdb_init" line 83)
     invoked from within
"${tool}_init $test_file_name"
     (procedure "runtest" line 19)
     invoked from within
"runtest $test_name"
     ("foreach" body line 24)
     invoked from within
"foreach test_name $testlist {
		if { ${ignoretests} != "" } {
		    if { 0 <= [lsearch ${ignoretests} [file tail ${test_name}]]} {
			continue
		    }..."
     ("foreach" body line 92)
     invoked from within
"foreach pass $multipass {

	# multipass_name is set for `record_test' to use (see framework.exp).
	if { [lindex $pass 0] != "" } {
	    set multipass_..."
     ("foreach" body line 51)
     invoked from within
"foreach current_target $target_list {
     verbose "target is $current_target"
     set current_target_name $current_target
     set tlist [split $curren..."
     (file "/usr/share/dejagnu/runtest.exp" line 1624)
Pedro Alves - May 8, 2015, 3:08 p.m.
On 05/08/2015 01:53 PM, Luis Machado wrote:
> On 05/06/2015 03:14 PM, Pedro Alves wrote:
>> On 04/08/2015 08:30 PM, Simon Marchi wrote:
>>
>>> It's much better now, thanks!
>>
>> FYI, I've pushed this in now.
> 
> For some reason the next test after that now fails and stops the 
> testsuite run.

It's probably the setting of spawn_id.  If so, it's the sort of
thing that is only visible on a serial run, and I probably only
tested a parallel run.  I'll take a look.

Thanks,
Pedro Alves

Patch

diff --git a/gdb/gdbserver/server.c b/gdb/gdbserver/server.c
index 3408ef7..d57674d 100644
--- a/gdb/gdbserver/server.c
+++ b/gdb/gdbserver/server.c
@@ -284,6 +284,8 @@  start_inferior (char **argv)
       current_thread->last_resume_kind = resume_stop;
       current_thread->last_status = last_status;
     }
+  else
+    mourn_inferior (find_process_pid (ptid_get_pid (last_ptid)));
 
   return signal_pid;
 }
diff --git a/gdb/testsuite/gdb.server/non-existing-program.exp b/gdb/testsuite/gdb.server/non-existing-program.exp
new file mode 100644
index 0000000..f842c44
--- /dev/null
+++ b/gdb/testsuite/gdb.server/non-existing-program.exp
@@ -0,0 +1,63 @@ 
+# This testcase is part of GDB, the GNU debugger.
+
+# Copyright 2015 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# Test starting gdbserver passing it the name of a non-existing
+# program.
+
+load_lib gdbserver-support.exp
+
+standard_testfile
+
+if { [skip_gdbserver_tests] } {
+    return 0
+}
+
+set gdbserver [find_gdbserver]
+if { $gdbserver == "" } {
+    fail "could not find gdbserver"
+    return
+}
+
+# Fire off gdbserver.  The port doesn't really matter, gdbserver tries
+# to spawn the program before opening the connection.
+set spawn_id [remote_spawn target "$gdbserver stdio non-existing-program"]
+
+set msg "gdbserver exits cleanly"
+set saw_exiting 0
+expect {
+    # This is what we get on ptrace-based targets.
+    -re "stdin/stdout redirected.*No program to debug\r\nExiting\r\n$" {
+	set saw_exiting 1
+	exp_continue
+    }
+    # This is what we get on Windows.
+    -re "Error creating process\r\n\r\nExiting\r\n$" {
+	set saw_exiting 1
+	exp_continue
+    }
+    -re "A problem internal to GDBserver has been detected" {
+	fail "$msg (GDBserver internal error)"
+	wait
+    }
+    eof {
+	gdb_assert $saw_exiting $msg
+	wait
+    }
+    timeout {
+	fail "$msg (timeout)"
+    }
+}