From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 6536 invoked by alias); 19 Mar 2007 21:43:27 -0000 Received: (qmail 6523 invoked by uid 22791); 19 Mar 2007 21:43:24 -0000 X-Spam-Check-By: sourceware.org Received: from elrond.portugalmail.pt (HELO elrond.portugalmail.pt) (195.245.179.181) by sourceware.org (qpsmtpd/0.31) with ESMTP; Mon, 19 Mar 2007 21:43:16 +0000 Received: from localhost (localhost [127.0.0.1]) by elrond.portugalmail.pt (Postfix) with ESMTP id 7E45B3D79B for ; Mon, 19 Mar 2007 21:37:36 +0000 (WET) Received: from elrond.portugalmail.pt ([127.0.0.1]) by localhost (elrond.portugalmail.pt [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id CRxAOx-L1ZTA for ; Mon, 19 Mar 2007 21:37:35 +0000 (WET) Received: from [127.0.0.1] (88.210.72.155.rev.optimus.pt [88.210.72.155]) (Authenticated sender: pedro_alves@portugalmail.pt) by elrond.portugalmail.pt (Postfix) with ESMTP id 80BE93D77D for ; Mon, 19 Mar 2007 20:46:19 +0000 (WET) Message-ID: <45FEF7B0.9070209@portugalmail.pt> Date: Mon, 19 Mar 2007 21:43:00 -0000 From: Pedro Alves User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; pt-BR; rv:1.8.0.10) Gecko/20070221 Thunderbird/1.5.0.10 Mnenhy/0.7.4.0 MIME-Version: 1.0 To: gdb-patches@sourceware.org Subject: Re: TUI + gdbserver broken? References: <45FDECB3.5000002@portugalmail.pt> <20070319021145.GA25872@caradoc.them.org> In-Reply-To: <20070319021145.GA25872@caradoc.them.org> Content-Type: multipart/mixed; boundary="------------060304060300090409000507" X-Antivirus: avast! (VPS 000725-0, 19-03-2007), Outbound message X-Antivirus-Status: Clean 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: 2007-03/txt/msg00172.txt.bz2 This is a multi-part message in MIME format. --------------060304060300090409000507 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Content-length: 5477 Daniel Jacobowitz wrote: > Thanks for all your detective work on this. I'm sorry I apparently > broke TUI so badly - I wish we had test coverage. > No problem. I wonder: Is TUI testable? Since TUI is sitting on top of deprecated hooks, it TUI itself deprecated? > On Mon, Mar 19, 2007 at 01:51:47AM +0000, Pedro Alves wrote: > >> The problem is that we now call deprecated_safe_get_selected_frame >> in tui_selected_frame_level_changed_hook... : >> > > Before I look at your patch, could you check one more thing for me: > what's the backtrace look like when we get here? > > Here it is. I've stopped in tui_selected_frame_level_changed_hook at the point is is obviously going to do something very wrong: /* The selected frame has changed. This is happens after a target stop or when the user explicitly changes the frame (up/down/thread/...). */ static void tui_selected_frame_level_changed_hook (int level) { struct frame_info *fi; fi = deprecated_safe_get_selected_frame (); /* Ensure that symbols for this frame are read in. Also, determine the source language of this frame, and switch to it if desired. */ if (fi) { struct symtab *s; s = find_pc_symtab (get_frame_pc (fi)); <<<<< here. (tui-hooks.c:240) #0 tui_selected_frame_level_changed_hook (level=0) at ../../gdb-server_search/src/gdb/tui/tui-hooks.c:240 #1 0x0047c359 in select_frame (fi=0x1005b6f8) at ../../gdb-server_search/src/gdb/frame.c:995 #2 0x0047c2ab in get_selected_frame (message=0x0) at ../../gdb-server_search/src/gdb/frame.c:965 #3 0x0047c325 in deprecated_safe_get_selected_frame () at ../../gdb-server_search/src/gdb/frame.c:981 #4 0x004a3191 in tui_selected_frame_level_changed_hook (level=-1) at ../../gdb-server_search/src/gdb/tui/tui-hooks.c:233 #5 0x0047c359 in select_frame (fi=0x0) at ../../gdb-server_search/src/gdb/frame.c:995 #6 0x0047c572 in reinit_frame_cache () at ../../gdb-server_search/src/gdb/frame.c:1095 #7 0x004262f7 in handle_inferior_event (ecs=0x22c690) at ../../gdb-server_search/src/gdb/infrun.c:1302 #8 0x00425b87 in wait_for_inferior () at ../../gdb-server_search/src/gdb/infrun.c:1003 #9 0x004259d8 in proceed (addr=4294967295, siggnal=TARGET_SIGNAL_DEFAULT, step=0) at ../../gdb-server_search/src/gdb/infrun.c:825 #10 0x004138ab in continue_command (proc_count_exp=0x0, from_tty=1) at ../../gdb-server_search/src/gdb/infcmd.c:649 #11 0x0042ca1b in do_cfunc (c=0x10050258, args=0x0, from_tty=1) at ../../gdb-server_search/src/gdb/cli/cli-decode.c:62 #12 0x0042ef0d in cmd_func (cmd=0x10050258, args=0x0, from_tty=1) at ../../gdb-server_search/src/gdb/cli/cli-decode.c:1666 #13 0x004025b8 in execute_command (p=0x10033721 "", from_tty=1) at ../../gdb-server_search/src/gdb/top.c:456 #14 0x004243c2 in command_handler (command=0x10033720 "c") at ../../gdb-server_search/src/gdb/event-top.c:519 #15 0x00424b84 in command_line_handler (rl=0x101778c8 "\020hj") at ../../gdb-server_search/src/gdb/event-top.c:804 #16 0x00424cd8 in gdb_readline2 (client_data=0x0) at ../../gdb-server_search/src/gdb/event-top.c:884 #17 0x0042426c in stdin_event_handler (error=0, client_data=0x0) at ../../gdb-server_search/src/gdb/event-top.c:432 #18 0x0043944e in handle_file_event (event_file_desc=0) at ../../gdb-server_search/src/gdb/event-loop.c:730 #19 0x00438cd2 in process_event () at ../../gdb-server_search/src/gdb/event-loop.c:343 #20 0x00438d1b in gdb_do_one_event (data=0x0) at ../../gdb-server_search/src/gdb/event-loop.c:380 #21 0x00412f7b in catch_errors (func=0x438ce7 , func_args=0x0, errstring=0x649101 "", mask=6) at ../../gdb-server_search/src/gdb/exceptions.c:515 #22 0x004a2e1b in tui_command_loop (data=0x0) at ../../gdb-server_search/src/gdb/tui/tui-interp.c:151 #23 0x0041c92a in current_interp_command_loop () at ../../gdb-server_search/src/gdb/interps.c:278 #24 0x004010cb in captured_command_loop (data=0x0) at ../../gdb-server_search/src/gdb/main.c:101 #25 0x00412f7b in catch_errors (func=0x4010c0 , func_args=0x0, errstring=0x61a139 "", mask=6) at ../../gdb-server_search/src/gdb/exceptions.c:515 #26 0x00402110 in captured_main (data=0x22cc90) at ../../gdb-server_search/src/gdb/main.c:867 #27 0x00412f7b in catch_errors (func=0x401102 , func_args=0x22cc90, errstring=0x61a139 "", mask=6) at ../../gdb-server_search/src/gdb/exceptions.c:515 #28 0x00402146 in gdb_main (args=0x22cc90) at ../../gdb-server_search/src/gdb/main.c:876 #29 0x004010ba in main (argc=2, argv=0x10033438) at ../../gdb-server_search/src/gdb/tui/tui-main.c:36 > I think it is more likely that we shouldn't be doing whatever we're > doing until after we've finished cleaning up the target's state. > It's stuck between wait and mourn. > > I should have realized it too 8) Here is an updated patch that also fixes the problem. The comments in the patch should make it obvious, and I think it does what you hinted at. >> tui_registers_changed_hook then has the problem that is is calling >> get_selected_frame, when target_has_registers is false. Fixed by >> using deprecated_safe_get_selected_frame here too. >> > > This bit makes sense; you can commit it separately if you want. > I don't need this anymore with this new patch, so I've dropped it. It is easy to add if needed - error is called in that case. Cheers, Pedro Alves --------------060304060300090409000507 Content-Type: text/plain; name="tui_frame2.diff" Content-Transfer-Encoding: 7bit Content-Disposition: inline; filename="tui_frame2.diff" Content-length: 2532 gdb/ChangeLog * infrun.c (handle_inferior_event): Delay calling reinit_frame_cache to after calling target_mourn_inferior when target has exited or was signalled. --- gdb/infrun.c | 30 +++++++++++++++++++++++------- 1 file changed, 23 insertions(+), 7 deletions(-) Index: src/gdb/infrun.c =================================================================== --- src.orig/gdb/infrun.c 2007-03-18 23:22:38.000000000 +0000 +++ src/gdb/infrun.c 2007-03-19 20:36:40.000000000 +0000 @@ -1299,8 +1299,6 @@ handle_inferior_event (struct execution_ } ecs->infwait_state = infwait_normal_state; - reinit_frame_cache (); - /* If it's a new process, add it to the thread database */ ecs->new_thread_event = (!ptid_equal (ecs->ptid, inferior_ptid) @@ -1308,13 +1306,22 @@ handle_inferior_event (struct execution_ && !in_thread_list (ecs->ptid)); if (ecs->ws.kind != TARGET_WAITKIND_EXITED - && ecs->ws.kind != TARGET_WAITKIND_SIGNALLED && ecs->new_thread_event) + && ecs->ws.kind != TARGET_WAITKIND_SIGNALLED) { - add_thread (ecs->ptid); + /* When the inferior exited, postpone frame related operations + until the new target state is recorded. There may be + hooks installed that may try to fetch data from the + inferior without realizing it is gone. */ + reinit_frame_cache (); + + if (ecs->new_thread_event) + { + add_thread (ecs->ptid); - ui_out_text (uiout, "[New "); - ui_out_text (uiout, target_pid_or_tid_to_str (ecs->ptid)); - ui_out_text (uiout, "]\n"); + ui_out_text (uiout, "[New "); + ui_out_text (uiout, target_pid_or_tid_to_str (ecs->ptid)); + ui_out_text (uiout, "]\n"); + } } switch (ecs->ws.kind) @@ -1387,6 +1394,11 @@ handle_inferior_event (struct execution_ target_mourn_inferior (); singlestep_breakpoints_inserted_p = 0; /*SOFTWARE_SINGLE_STEP_P() */ stop_print_frame = 0; + + /* Now that the target state is recorded we can safelly + do frame related operations. */ + reinit_frame_cache (); + stop_stepping (ecs); return; @@ -1404,6 +1416,10 @@ handle_inferior_event (struct execution_ may be needed. */ target_mourn_inferior (); + /* Now that the target state is recorded we can safelly + do frame related operations. */ + reinit_frame_cache (); + print_stop_reason (SIGNAL_EXITED, stop_signal); singlestep_breakpoints_inserted_p = 0; /*SOFTWARE_SINGLE_STEP_P() */ stop_stepping (ecs); --------------060304060300090409000507--