From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 30509 invoked by alias); 28 Jul 2008 15:52:56 -0000 Received: (qmail 30459 invoked by uid 22791); 28 Jul 2008 15:52:54 -0000 X-Spam-Check-By: sourceware.org Received: from mail.codesourcery.com (HELO mail.codesourcery.com) (65.74.133.4) by sourceware.org (qpsmtpd/0.31) with ESMTP; Mon, 28 Jul 2008 15:52:37 +0000 Received: (qmail 13862 invoked from network); 28 Jul 2008 15:52:33 -0000 Received: from unknown (HELO orlando.local) (pedro@127.0.0.2) by mail.codesourcery.com with ESMTPA; 28 Jul 2008 15:52:33 -0000 From: Pedro Alves To: gdb-patches@sourceware.org Subject: Re: Fix crash on "quit" + TUI + remote + async Date: Mon, 28 Jul 2008 15:52:00 -0000 User-Agent: KMail/1.9.9 References: <200807281633.41104.pedro@codesourcery.com> In-Reply-To: <200807281633.41104.pedro@codesourcery.com> MIME-Version: 1.0 Content-Type: Multipart/Mixed; boundary="Boundary-00=_7sejIHbYY/Uat70" Message-Id: <200807281652.27218.pedro@codesourcery.com> X-IsSubscribed: yes Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2008-07/txt/msg00519.txt.bz2 --Boundary-00=_7sejIHbYY/Uat70 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit Content-Disposition: inline Content-length: 3957 Ahem, EWRONGPATCH. Here is the correct one. OK? On Monday 28 July 2008 16:33:40, Pedro Alves wrote: > I was debugging against a remote stub in async mode, using the TUI, and I > issued "quit" while still connected, in the hopes of bailing out of GDB > without much fuss, but, I ran into this SEGV: > > Program received signal SIGSEGV, Segmentation fault. > [Switching to Thread 0x7fcf597fb6e0 (LWP 13869)] > 0x0000000000552f5e in serial_can_async_p (scb=0x0) at > ../../src/gdb/serial.c:504 504 return (scb->ops->async != NULL); > (top-gdb) bt > #0 0x0000000000552f5e in serial_can_async_p (scb=0x0) at > ../../src/gdb/serial.c:504 During symbol reading, unsupported tag: > 'DW_TAG_const_type'. > #1 0x000000000042ffa3 in remote_can_async_p () at > ../../src/gdb/remote.c:8207 During symbol reading, DW_AT_type missing from > DW_TAG_subrange_type. #2 0x000000000040d740 in execute_command > (p=0x7fff618212a0 "set width 118", from_tty=0) at ../../src/gdb/top.c:382 > #3 0x0000000000457d1c in tui_update_gdb_sizes () at > ../../src/gdb/tui/tui-win.c:468 #4 0x0000000000452002 in tui_disable () at > ../../src/gdb/tui/tui.c:455 #5 0x0000000000452581 in tui_exit () at > ../../src/gdb/tui/tui-interp.c:45 #6 0x00007fcf589e0110 in exit () from > /lib/libc.so.6 > #7 0x000000000040edeb in quit_force (args=0x0, from_tty=1) at > ../../src/gdb/top.c:1292 #8 0x0000000000443f20 in quit_command (args=0x0, > from_tty=1) at ../../src/gdb/cli/cli-cmds.c:315 #9 0x000000000043e660 in > do_cfunc (c=0xa344b0, args=0x0, from_tty=1) at > ../../src/gdb/cli/cli-decode.c:60 #10 0x00000000004412a3 in cmd_func > (cmd=0xa344b0, args=0x0, from_tty=1) at ../../src/gdb/cli/cli-decode.c:1680 > #11 0x000000000040d9df in execute_command (p=0x9e81c1 "", from_tty=1) at > ../../src/gdb/top.c:465 #12 0x00000000004c19ea in command_handler > (command=0x9e81c0 "q") at ../../src/gdb/event-top.c:514 > > As you can see from from #2, disabling the tui ends up > executing a command. At this point, the remote target was still > pushed on the stack, but it had already been closed, hence the remote > connection was closed, and remote_desc was == NULL. > > There are target_can_async_p call around every command invocation, > and since the remote target was still on the stack, remote_can_async_p > was called, but it isn't expecting to be reached with remote_desc == NULL, > as most of remote's code: > > static int > remote_can_async_p (void) > { > if (!remote_async_permitted) > /* We only enable async when the user specifically asks for it. */ > return 0; > > /* We're async whenever the serial device is. */ > return remote_async_mask_value && serial_can_async_p (remote_desc); > } > > The issue is that quit_target closes the current target, but doesn't > pop it: > > static int > quit_target (void *arg) > { > struct qt_args *qt = (struct qt_args *)arg; > > if (! ptid_equal (inferior_ptid, null_ptid) && target_has_execution) > { > if (attach_flag) > target_detach (qt->args, qt->from_tty); > else > target_kill (); > } > > /* UDI wants this, to kill the TIP. */ > target_close (¤t_target, 1); > > /* Save the history information if it is appropriate to do so. */ > if (write_history_p && history_filename) > write_history (history_filename); > > do_final_cleanups (ALL_CLEANUPS); /* Do any final cleanups before > exiting */ > > return 0; > } > > > The UDI target seems to be gone from the sources, but, even if it's still > here and I missed it, this is broken. It should at best be a pop_target. > But, while I'm at it, if the current target is of stratum > > process_stratum, this will still leave the process_stratum target open, > which is much > more likelly to need closing. So, I've come up with a new > pop_all_targets method and used it instead. > > Tested against x86_64-unknown-linux-gnu native and gdbserver. > > OK? -- Pedro Alves --Boundary-00=_7sejIHbYY/Uat70 Content-Type: text/x-diff; charset="utf-8"; name="tui_remote_async_quit_crash.diff" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="tui_remote_async_quit_crash.diff" Content-length: 2634 2008-07-28 Pedro Alves * target.h (pop_all_targets): Declare. * target.c (pop_all_targets): New. * top.c (quit_target): Pop all targets instead of just closing the current. --- gdb/target.c | 18 ++++++++++++++++++ gdb/target.h | 7 +++++++ gdb/top.c | 5 +++-- 3 files changed, 28 insertions(+), 2 deletions(-) Index: src/gdb/target.c =================================================================== --- src.orig/gdb/target.c 2008-07-28 16:36:34.000000000 +0100 +++ src/gdb/target.c 2008-07-28 16:41:53.000000000 +0100 @@ -820,6 +820,24 @@ pop_target (void) internal_error (__FILE__, __LINE__, _("failed internal consistency check")); } +void +pop_all_targets (int quitting) +{ + while ((int) (current_target.to_stratum) > (int) dummy_stratum) + { + target_close (¤t_target, quitting); + if (!unpush_target (target_stack)) + { + fprintf_unfiltered (gdb_stderr, + "pop_all_targets couldn't find target %s\n", + current_target.to_shortname); + internal_error (__FILE__, __LINE__, + _("failed internal consistency check")); + break; + } + } +} + /* Using the objfile specified in OBJFILE, find the address for the current thread's thread-local storage with offset OFFSET. */ CORE_ADDR Index: src/gdb/target.h =================================================================== --- src.orig/gdb/target.h 2008-07-28 16:36:34.000000000 +0100 +++ src/gdb/target.h 2008-07-28 16:39:25.000000000 +0100 @@ -1172,6 +1172,13 @@ extern void target_preopen (int); extern void pop_target (void); +/* Does whatever cleanup is required to get rid of all pushed targets. + QUITTING is propagated to target_close; it indicates that GDB is + exiting and should not get hung on an error (otherwise it is + important to perform clean termination, even if it takes a + while). */ +extern void pop_all_targets (int quitting); + extern CORE_ADDR target_translate_tls_address (struct objfile *objfile, CORE_ADDR offset); Index: src/gdb/top.c =================================================================== --- src.orig/gdb/top.c 2008-07-28 16:36:34.000000000 +0100 +++ src/gdb/top.c 2008-07-28 16:39:25.000000000 +0100 @@ -1229,8 +1229,9 @@ quit_target (void *arg) target_kill (); } - /* UDI wants this, to kill the TIP. */ - target_close (¤t_target, 1); + /* Give all pushed targets a chance to do minimal cleanup, and pop + them all out. */ + pop_all_targets (1); /* Save the history information if it is appropriate to do so. */ if (write_history_p && history_filename) --Boundary-00=_7sejIHbYY/Uat70--