[commit/RFA] Add assert inside insert_step_resume_breakpoint_at_frame

[commit/RFA] Add assert inside insert_step_resume_breakpoint_at_frame

[Joel Brobecker]

[-- Attachment #1: Type: text/plain, Size: 2046 bytes --]

Hello,

I just came across a very particular set of conditions that lead
insert_step_resume_breakpoint_at_frame to be passed a NULL frame,
causing the debugger to crash.

Basically, the debugger was doing a "next". During the course of
this operation, it landed inside a function called by the code
I was stepping over. This function was an internally-genearted
function for which no debugging info is generated. So GDB tried
to insert a breakpoint at the return address. Unfortunately,
trying to get the previous frame failed because of a combination
of factors: - The debugging info was stabs. With stabs, the N_FUN
              entries that define a function only provide the function
              start address. At this time of writing, the compiler
              is not generated the unnamed N_FUN entries used by GCC
              to specify the function end address
            - The funtion we called is placed immediately after
              the main (function).
            - This results in GDB thinking that the function we
              stepped over from goes over a range of instruction
              that is too wide. It ends up including our internally
              generated function.
            - So get_prev_frame accidently determines that we're inside
              the main function, and thus stops the backtracing by
              returning a NULL frame.

This is a pretty unlikely situation, but I think we should add an
assert that "return_frame" is not NULL before dereferencing it.
It's a lot cleaner to report an assertion failure rather than crashing.

As for us, we are working on enhancing the compiler to generate that
empty N_FUN stabs entry.

2007-09-14  Joel Brobecker  <brobecker@adacore.com>

        * infrun.c (insert_step_resume_breakpoint_at_frame): Add assertion
        that return_frame is not null.

Tested on x86-linux, no regression. This also prevents the crash
initially observed.

This seems pretty straightforward and safe, but I'll give it a week
for comments before checking in.

-- 
Joel

[-- Attachment #2: infrun.diff --]
[-- Type: text/plain, Size: 494 bytes --]

Index: infrun.c
===================================================================
RCS file: /cvs/src/src/gdb/infrun.c,v
retrieving revision 1.246
diff -u -p -r1.246 infrun.c
--- infrun.c	10 Sep 2007 18:49:26 -0000	1.246
+++ infrun.c	14 Sep 2007 21:28:04 -0000
@@ -2868,6 +2868,7 @@ insert_step_resume_breakpoint_at_frame (
 {
   struct symtab_and_line sr_sal;
 
+  gdb_assert (return_frame != NULL);
   init_sal (&sr_sal);		/* initialize to zeros */
 
   sr_sal.pc = gdbarch_addr_bits_remove

Re: [commit/RFA] Add assert inside insert_step_resume_breakpoint_at_frame

[Daniel Jacobowitz]

On Fri, Sep 14, 2007 at 02:40:05PM -0700, Joel Brobecker wrote:
> 2007-09-14  Joel Brobecker  <brobecker@adacore.com>
> 
>         * infrun.c (insert_step_resume_breakpoint_at_frame): Add assertion
>         that return_frame is not null.

This is definitely OK.

-- 
Daniel Jacobowitz
CodeSourcery

Re: [commit/RFA] Add assert inside insert_step_resume_breakpoint_at_frame

[Joel Brobecker]

> > 2007-09-14  Joel Brobecker  <brobecker@adacore.com>
> > 
> >         * infrun.c (insert_step_resume_breakpoint_at_frame): Add assertion
> >         that return_frame is not null.
> 
> This is definitely OK.

Great! checked in.

Thank you,
-- 
Joel