[gdb/breakpoints] Fix sigsegv in info prog at exec catchpoint

Message ID 20180720141359.qycrai5coi5rdyag@delia
State New, archived
Headers

Commit Message

Tom de Vries July 20, 2018, 2:14 p.m. UTC
  Hi,

with the test-case contained in this patch and compiled for debug we run into
a segfault with trunk gdb:
...
$ gdb catch-follow-exec -batch -ex "catch exec" \
  -ex "set follow-exec-mode new" -ex "run" -ex "info prog"
Catchpoint 1 (exec)
process xxx is executing new program: /usr/bin/ls
[New inferior 2 (process 0)]
[New process xxx]

Thread 2.1 "ls" hit Catchpoint 1 (exec'd /usr/bin/ls), in _start () from
  /lib64/ld-linux-x86-64.so.2
Segmentation fault (core dumped)
...

The patch fixes the segfault by returning an error in info_program_command
if get_last_target_status returns minus_one_ptid.

The test-case is non-standard, because the standard approach runs into
PR23020, a problem with gdb going to the background.

Build and reg-tested on x86_64-linux.

OK for trunk?

Thanks,
- Tom

[gdb/breakpoints] Fix sigsegv in info prog at exec catchpoint

2018-07-20  Tom de Vries  <tdevries@suse.de>

	PR breakpoints/23366
	* infcmd.c (info_program_command): Handle ptid == minus_one_ptid.

	* gdb.base/catch-follow-exec.c: New test.
	* gdb.base/catch-follow-exec.exp: New file.

---
 gdb/infcmd.c                                 |  2 +-
 gdb/testsuite/gdb.base/catch-follow-exec.c   | 10 +++++
 gdb/testsuite/gdb.base/catch-follow-exec.exp | 56 ++++++++++++++++++++++++++++
 3 files changed, 67 insertions(+), 1 deletion(-)
  

Comments

Tom Tromey July 25, 2018, 7:17 p.m. UTC | #1
>>>>> "Tom" == Tom de Vries <tdevries@suse.de> writes:

Tom> The patch fixes the segfault by returning an error in info_program_command
Tom> if get_last_target_status returns minus_one_ptid.

Tom> The test-case is non-standard, because the standard approach runs into
Tom> PR23020, a problem with gdb going to the background.

Is it possible to fix this part first?
Otherwise it seems to me that the test case has some issues.

Also, is the bug dependent on -batch?  This wasn't clear to me, but I
guess if it is then the approach to the test case makes more sense.

Tom> 	PR breakpoints/23366
Tom> 	* infcmd.c (info_program_command): Handle ptid == minus_one_ptid.

This part looks ok to me.

Tom> +if { ![file exists /bin/bash] } {
...
Tom> +if { ![file exists /bin/ls] } {
...
Tom> +    catch {exec /bin/bash -c "$GDB $FLAGS"} catchlog

I think this sort of thing can't be done when doing remote host testing,
so some sort of check is needed for that.  I don't remember any more
exactly how this is done but there should be other examples in the tree.

Tom> +if { ![file exists /bin/bash] } {
Tom> +    unsupported "no bash"
Tom> +}

Also there has to be a "return" after the "unsupported"; unsupported
just logs a message and without the return, the rest of the test will
still be run.

Tom
  

Patch

diff --git a/gdb/infcmd.c b/gdb/infcmd.c
index 821bcc6544..74d5956765 100644
--- a/gdb/infcmd.c
+++ b/gdb/infcmd.c
@@ -2091,7 +2091,7 @@  info_program_command (const char *args, int from_tty)
       get_last_target_status (&ptid, &ws);
     }
 
-  if (ptid == null_ptid)
+  if (ptid == null_ptid || ptid == minus_one_ptid)
     error (_("No selected thread."));
 
   thread_info *tp = find_thread_ptid (ptid);
diff --git a/gdb/testsuite/gdb.base/catch-follow-exec.c b/gdb/testsuite/gdb.base/catch-follow-exec.c
new file mode 100644
index 0000000000..fa68a2a34e
--- /dev/null
+++ b/gdb/testsuite/gdb.base/catch-follow-exec.c
@@ -0,0 +1,10 @@ 
+#include <stdio.h>
+#include <string.h>
+#include <unistd.h>
+
+int
+main (void)
+{
+  char *exec_args[] = { "/bin/ls", "ppp", NULL };
+  execve (exec_args[0], exec_args, NULL);
+}
diff --git a/gdb/testsuite/gdb.base/catch-follow-exec.exp b/gdb/testsuite/gdb.base/catch-follow-exec.exp
new file mode 100644
index 0000000000..03611e9517
--- /dev/null
+++ b/gdb/testsuite/gdb.base/catch-follow-exec.exp
@@ -0,0 +1,56 @@ 
+# Copyright 2018 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/>.
+
+# Check whether finish respects the print pretty user setting when printing the
+# function result.
+
+standard_testfile
+
+if {[build_executable "failed to prepare" $testfile $srcfile debug] == -1} {
+    return -1
+}
+
+if { ![file exists /bin/bash] } {
+    unsupported "no bash"
+}
+
+if { ![file exists /bin/ls] } {
+    unsupported "no ls"
+}
+
+proc catch_follow_exec { } {
+    global binfile
+    global GDB
+
+    set test "catch-follow-exec"
+
+    append FLAGS " \"$binfile\""
+    append FLAGS " -batch"
+    append FLAGS " -ex \"catch exec\""
+    append FLAGS " -ex \"set follow-exec-mode new\""
+    append FLAGS " -ex \"run\""
+    append FLAGS " -ex \"info prog\""
+
+    catch {exec /bin/bash -c "$GDB $FLAGS"} catchlog
+    send_log "$catchlog\n"
+
+    if { [regexp {No selected thread} $catchlog] } {
+	pass $test
+    } else {
+	fail $test
+    }
+}
+
+catch_follow_exec