This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
[RFA] gdb/13333: remote crash
- From: Keith Seitz <keiths at redhat dot com>
- To: "gdb-patches at sourceware dot org ml" <gdb-patches at sourceware dot org>
- Date: Tue, 08 Nov 2011 14:40:25 -0800
- Subject: [RFA] gdb/13333: remote crash
Hi,
I've been looking at this bug, which causes a SEGV in gdb when the user
steps past an inferior exit (and the native gdbserver exits). Here's a
transcript which demonstrates the problem:
$ ./gdb -nx -q ~/tmp/13333
Reading symbols from /home/keiths/tmp/13333...done.
(gdb) b main
Breakpoint 1 at 0x400478: file 13333.c, line 6.
(gdb) tar remote :1234
Remote debugging using :1234
Reading symbols from /lib64/ld-linux-x86-64.so.2...(no debugging symbols
found)...done.
Loaded symbols for /lib64/ld-linux-x86-64.so.2
0x0000003d962016b0 in _start () from /lib64/ld-linux-x86-64.so.2
(gdb) c
Continuing.
Breakpoint 1, main () at 13333.c:6
6 return 0;
(gdb) n
7 }
(gdb)
0x0000003d9662139d in __libc_start_main () from /lib64/libc.so.6
(gdb)
Single stepping until exit from function __libc_start_main,
which has no line number information.
[Inferior 1 (Remote target) exited normally]
Segmentation fault (core dumped)
The segfault occurs because remote_remove_breakpoint is attempting to
call putpkt when remote_desc is NULL (because target_close has already
closed the serial channel and set remote_desc to NULL).
I've spent a fair amount of time attempting to track exactly when this
was broken, and whether it was a logic error somewhere, and I *think*
I've just about convinced myself that this is simply an oversight.
This bug was inadvertently introduced in commit 625c318c:
2010-12-09 Tom Tromey <tromey@redhat.com>
PR c++/9593:
* thread.c (clear_thread_inferior_resources): Call
delete_longjmp_breakpoint.
* infrun.c (handle_inferior_event): Handle exception breakpoints.
(handle_inferior_event): Likewise.
(insert_exception_resume_breakpoint): New function.
(check_exception_resume): Likewise.
* inferior.h (delete_longjmp_breakpoint_cleanup): Declare.
* infcmd.c (delete_longjmp_breakpoint_cleanup): No longer static.
[snip]
The important part here is that a call to delete_longjmp_breakpoint was
moved from a cleanup in handle_inferior_event to
clear_thread_inferior_resources. Allow me to elaborate.
Previously when the inferior exited, target_mourn_inferior would call
unpush_target, which would call remote_close (setting remote_desc to
NULL), and return (after doing some other stuff). This would then pop
the remote target, leaving the "exec" target at the top of the target stack.
Back in handle_inferior_event, the delete_longjmp_cleanup would be run,
and calling delete_longjmp_breakpoint, which calls
target_remove_breakpoint. Because the exec target is currently the top
of the stack, to_remove_breakpoint is set to "ignore", and this function
does what it needs to do. Everything is good.
After the patch, though, the subtle change is the call to
delete_longjmp_breakpoint was pushed down under unpush_target. As a
result, when target_remove_breakpoint is called (from
delete_longjmp_breakpoint in clear_thread_inferior_resources), it ends
up calling remote_remove_breakpoint, which doesn't check whether
remote_desc is valid or not, and POOF! We have the crash.
I believe that the right thing to do is keep the code paths identical in
both of these cases, and that is achieved by simply checking remote_desc
in remote_remove_breakpoint.
Having done that, the test suite shows no regressions, and the internal
breakpoint list on both the working and patched trees give identical
results.
So, remote.c wizards -- what say ye?
Keith
ChangeLog
2011-11-08 Keith Seitz <keiths@redhat.com>
PR gdb/13333
* remote.c (remote_remove_breakpoint): Do not attempt to
push packets down the wire if REMOTE_DESC is NULL.
testsuite/ChangeLog
2011-11-08 Keith Seitz <keiths@redhat.com>
PR gdb/13333
* gdb.server/server-run.exp: Check if gdb crashes when
the inferior and gdbserver exit while stepping.
diff --git a/gdb/remote.c b/gdb/remote.c
index 5182ef1..63c99cf 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -7703,28 +7703,33 @@ static int
remote_remove_breakpoint (struct gdbarch *gdbarch,
struct bp_target_info *bp_tgt)
{
- CORE_ADDR addr = bp_tgt->placed_address;
- struct remote_state *rs = get_remote_state ();
-
- if (remote_protocol_packets[PACKET_Z0].support != PACKET_DISABLE)
+ if (remote_desc != NULL)
{
- char *p = rs->buf;
+ CORE_ADDR addr = bp_tgt->placed_address;
+ struct remote_state *rs = get_remote_state ();
- *(p++) = 'z';
- *(p++) = '0';
- *(p++) = ',';
+ if (remote_protocol_packets[PACKET_Z0].support != PACKET_DISABLE)
+ {
+ char *p = rs->buf;
- addr = (ULONGEST) remote_address_masked (bp_tgt->placed_address);
- p += hexnumstr (p, addr);
- sprintf (p, ",%d", bp_tgt->placed_size);
+ *(p++) = 'z';
+ *(p++) = '0';
+ *(p++) = ',';
- putpkt (rs->buf);
- getpkt (&rs->buf, &rs->buf_size, 0);
+ addr = (ULONGEST) remote_address_masked (bp_tgt->placed_address);
+ p += hexnumstr (p, addr);
+ sprintf (p, ",%d", bp_tgt->placed_size);
- return (rs->buf[0] == 'E');
+ putpkt (rs->buf);
+ getpkt (&rs->buf, &rs->buf_size, 0);
+
+ return (rs->buf[0] == 'E');
+ }
+
+ return memory_remove_breakpoint (gdbarch, bp_tgt);
}
- return memory_remove_breakpoint (gdbarch, bp_tgt);
+ return 1;
}
static int
diff --git a/gdb/testsuite/gdb.server/server-run.exp b/gdb/testsuite/gdb.server/server-run.exp
index 9567043..6cd38f8 100644
--- a/gdb/testsuite/gdb.server/server-run.exp
+++ b/gdb/testsuite/gdb.server/server-run.exp
@@ -47,3 +47,20 @@ if { [istarget *-*-linux*] } {
gdb_breakpoint main
gdb_test "continue" "Breakpoint.* main .*" "continue to main"
+
+# gdb/13333 - Check if gdb crashes while stepping over an inferior exit.
+send_gdb "next\n"
+gdb_expect {
+ -re "exited normally.*$gdb_prompt $" {
+ # The inferior exited, and gdb did not crash.
+ pass "step until inferior exits"
+ }
+
+ -re "$gdb_prompt $" {
+ # We got a prompt -- keep stepping
+ send_gdb "next\n"
+ exp_continue
+ }
+
+ default { fail "step until inferior exits" }
+}