This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: TUI + gdbserver broken?
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 <gdb_do_one_event>,
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 <captured_command_loop>,
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 <captured_main>,
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
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);