Fix a crash when stepping and unwinding fails
Daniel Jacobowitz
drow@false.org
Wed May 17 17:59:00 GMT 2006
[GDB 6.5 note: I have seen this bug reported roughly five times now.
I think we need to fix it before the release.]
On Wed, Feb 22, 2006 at 10:54:24PM +0100, Mark Kettenis wrote:
> > I don't think it would help me though. Perhaps the real problem is the
> > use of null_frame_id for both the outermost frame and completely
> > unknown frames. It would be nice if we could tell here:
> >
> > if (frame_id_eq (frame_unwind_id (get_current_frame ()), step_frame_id))
> >
> > that frame_unwind_id has returned something completely invalid instead
> > of the outermost frame
>
> Indeed.
>
> > One way to do that in our current representation would be to check that
> > the frame ID for the current frame is not null_ptid.
>
> Yes, I think it makes sense to punt trying to insert a step-resume
> breakpoint earlier than you do in your patch.
First, an apology for confusing the issue. The frame_id_eq line posted
there is actually OK for this specific problem: frame_id_eq where
either argument is null_frame_id will always return false. It's
actually the _next_ call to insert_step_resume_breakpoint_at_frame that
blows up.
Of course this means stepping in the next-to-outermost frame doesn't
work quite right. That will be harder to fix, and see my post to gdb@
[no responses yet...] about the underlying issue of the outermost frame
ID representation. This is a smaller problem than the segfault.
A related issue I noticed while working on this; there is an unlikely
potential segfault if the undebuggable function being stepped over is
named "main". frame_unwind_id, called in the line quoted above, will
successfully unwind past main; but get_prev_frame will refuse to. Thus
we will pass NULL to insert_step_resume_breakpoint_at_frame.
Accordingly, I think the best fix for now is a little code duplication.
I've split the "set a breakpoint at the current frame" operation, used
for signals, from the "set a breakpoint at the return address"
operation, used for stepping over functions. I've also clarified and
updated some comments.
What do you think of this version?
If you'd like to reproduce this, here's a recipe that works on AMD64;
you'll need to adjust the 0x10 to the return column if you want to try
i386, IIRC 0x8. I don't want to put this in the testsuite because of
the nasty hack with .rodata1, which serves to make sure stop_func_name
is NULL. Otherwise, GDB will incorrectly associate it with some
earlier symbol. This will become easier to test if we teach GDB about
symbol sizes and to not "extend" previous symbols :-)
a.s
===
.globl main
main:
xorl %ebp, %ebp
call foo
ret
foo:
.cfi_startproc
.cfi_escape 0x7, 0x10
xorl %ebp, %ebp
call bar
ret
.cfi_endproc
b.s
===
.section ".rodata1", "ax"
.globl bar
bar:
.cfi_startproc
.cfi_escape 0x7, 0x10
ret
.cfi_endproc
drow@caradoc:~% gcc -c b.s
b.s: Assembler messages:
b.s:1: Warning: setting incorrect section attributes for .rodata1
drow@caradoc:~% gcc -g -o a a.s b.o
drow@caradoc:~% strip -N bar a
drow@caradoc:~% gdb ./a
GNU gdb 6.4.50.20060511-debian
Copyright (C) 2006 Free Software Foundation, Inc.
GDB is free software, covered by the GNU General Public License, and
you are
welcome to change it and/or distribute copies of it under certain
conditions.
Type "show copying" to see the conditions.
There is absolutely no warranty for GDB. Type "show warranty" for
details.
This GDB was configured as "x86_64-linux-gnu"...Using host libthread_db
library "/lib/libthread_db.so.1".
(gdb) b foo
Breakpoint 1 at 0x400430: file a.s, line 10.
(gdb) r
Starting program: /home/drow/a
Breakpoint 1, foo () at a.s:10
10 xorl %ebp, %ebp
Current language: auto; currently asm
(gdb) s
foo () at a.s:11
11 call bar
(gdb)
zsh: segmentation fault (core dumped) gdb ./a
--
Daniel Jacobowitz
CodeSourcery
2006-05-17 Daniel Jacobowitz <dan@codesourcery.com>
* infrun.c (insert_step_resume_breakpoint_at_caller): New function,
based on insert_step_resume_breakpoint_at_frame.
(handle_inferior_event): Update comments. Use
insert_step_resume_breakpoint_at_caller.
(insert_step_resume_breakpoint_at_frame): Revise comments.
Index: infrun.c
===================================================================
RCS file: /cvs/src/src/gdb/infrun.c,v
retrieving revision 1.210
diff -u -p -r1.210 infrun.c
--- infrun.c 30 Mar 2006 16:37:13 -0000 1.210
+++ infrun.c 17 May 2006 15:57:54 -0000
@@ -943,6 +943,7 @@ void handle_inferior_event (struct execu
static void step_into_function (struct execution_control_state *ecs);
static void insert_step_resume_breakpoint_at_frame (struct frame_info *step_frame);
+static void insert_step_resume_breakpoint_at_caller (struct frame_info *);
static void insert_step_resume_breakpoint_at_sal (struct symtab_and_line sr_sal,
struct frame_id sr_id);
static void stop_stepping (struct execution_control_state *ecs);
@@ -2367,9 +2368,13 @@ process_event_stop_test:
return;
}
+ /* Check for subroutine calls.
+
+ NOTE: frame_id_eq will never report two invalid frame IDs as
+ being equal, so to get into this block, both the current and
+ previous frame must have valid frame IDs. */
if (frame_id_eq (frame_unwind_id (get_current_frame ()), step_frame_id))
{
- /* It's a subroutine call. */
CORE_ADDR real_stop_pc;
if (debug_infrun)
@@ -2396,7 +2401,7 @@ process_event_stop_test:
/* We're doing a "next", set a breakpoint at callee's return
address (the address at which the caller will
resume). */
- insert_step_resume_breakpoint_at_frame (get_prev_frame (get_current_frame ()));
+ insert_step_resume_breakpoint_at_caller (get_current_frame ());
keep_going (ecs);
return;
}
@@ -2459,7 +2464,7 @@ process_event_stop_test:
/* Set a breakpoint at callee's return address (the address at
which the caller will resume). */
- insert_step_resume_breakpoint_at_frame (get_prev_frame (get_current_frame ()));
+ insert_step_resume_breakpoint_at_caller (get_current_frame ());
keep_going (ecs);
return;
}
@@ -2513,8 +2518,11 @@ process_event_stop_test:
and no line number corresponding to the address where the
inferior stopped). Since we want to skip this kind of code,
we keep going until the inferior returns from this
- function. */
- if (step_stop_if_no_debug)
+ function - unless the user has asked us not to (via
+ set step-mode) or we no longer know how to get back
+ to the call site. */
+ if (step_stop_if_no_debug
+ || !frame_id_p (frame_unwind_id (get_current_frame ())))
{
/* If we have no line number and the step-stop-if-no-debug
is set, we stop the step so that the user has a chance to
@@ -2528,7 +2536,7 @@ process_event_stop_test:
{
/* Set a breakpoint at callee's return address (the address
at which the caller will resume). */
- insert_step_resume_breakpoint_at_frame (get_prev_frame (get_current_frame ()));
+ insert_step_resume_breakpoint_at_caller (get_current_frame ());
keep_going (ecs);
return;
}
@@ -2735,20 +2743,13 @@ insert_step_resume_breakpoint_at_sal (st
if (breakpoints_inserted)
insert_breakpoints ();
}
-
+
/* Insert a "step resume breakpoint" at RETURN_FRAME.pc. This is used
- to skip a function (next, skip-no-debug) or signal. It's assumed
- that the function/signal handler being skipped eventually returns
- to the breakpoint inserted at RETURN_FRAME.pc.
-
- For the skip-function case, the function may have been reached by
- either single stepping a call / return / signal-return instruction,
- or by hitting a breakpoint. In all cases, the RETURN_FRAME belongs
- to the skip-function's caller.
-
- For the signals case, this is called with the interrupted
- function's frame. The signal handler, when it returns, will resume
- the interrupted function at RETURN_FRAME.pc. */
+ to skip a potential signal handler.
+
+ This is called with the interrupted function's frame. The signal
+ handler, when it returns, will resume the interrupted function at
+ RETURN_FRAME.pc. */
static void
insert_step_resume_breakpoint_at_frame (struct frame_info *return_frame)
@@ -2763,6 +2764,38 @@ insert_step_resume_breakpoint_at_frame (
insert_step_resume_breakpoint_at_sal (sr_sal, get_frame_id (return_frame));
}
+/* Similar to insert_step_resume_breakpoint_at_frame, except
+ but a breakpoint at the previous frame's PC. This is used to
+ skip a function after stepping into it (for "next" or if the called
+ function has no debugging information).
+
+ The current function has almost always been reached by single
+ stepping a call or return instruction. NEXT_FRAME belongs to the
+ current function, and the breakpoint will be set at the caller's
+ resume address.
+
+ This is a separate function rather than reusing
+ insert_step_resume_breakpoint_at_frame in order to avoid
+ get_prev_frame, which may stop prematurely (see the implementation
+ of frame_unwind_id for an example). */
+
+static void
+insert_step_resume_breakpoint_at_caller (struct frame_info *next_frame)
+{
+ struct symtab_and_line sr_sal;
+
+ /* We shouldn't have gotten here if we don't know where the call site
+ is. */
+ gdb_assert (frame_id_p (frame_unwind_id (next_frame)));
+
+ init_sal (&sr_sal); /* initialize to zeros */
+
+ sr_sal.pc = ADDR_BITS_REMOVE (frame_pc_unwind (next_frame));
+ sr_sal.section = find_pc_overlay (sr_sal.pc);
+
+ insert_step_resume_breakpoint_at_sal (sr_sal, frame_unwind_id (next_frame));
+}
+
static void
stop_stepping (struct execution_control_state *ecs)
{
More information about the Gdb-patches
mailing list