[RFA/controversial] move bp by line number past the prologue

[RFA/controversial] move bp by line number past the prologue

[Joel Brobecker]

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

This proposal has been discussed on this gdb-patches mailing list, and
there has been no consensus. Some of us are in favor of this change,
others are against. I am submitting this change for approval as a
request to make a decision. I don't know if making a small summary of
the different points for/against this change of behavior would be
useful, all the discussion is archived under

   [RFC] breakpoints and function prologues...

http://sources.redhat.com/ml/gdb-patches/2002-07/msg00448.html and
http://sources.redhat.com/ml/gdb-patches/2002-07/msg00451.html

Let me know if you would like to see such a summary (I'll try not to
distort too much the different points too much in my favor :-).

2002-08-29  Jim Ingham <jingham@apple.com>

        * linespec.c (decode_line_1): Skip the function prologue if
        funfirstline is set. Changes the behavior of the break command
        to skip the function prologue when breaking by line number.

This change introduces 5 FAILs in one testcase which assumes the current
behavior, so the FAIL is expected. Should this patch be approved, I'll
the testcase up.

Ahem... Ok to apply?
-- 
Joel

[-- Attachment #2: linespec.c.diff --]
[-- Type: text/plain, Size: 1312 bytes --]

Index: linespec.c
===================================================================
RCS file: /cvs/src/src/gdb/linespec.c,v
retrieving revision 1.21
diff -c -3 -p -r1.21 linespec.c
*** linespec.c	11 May 2002 22:46:19 -0000	1.21
--- linespec.c	29 Aug 2002 18:03:02 -0000
*************** decode_line_1 (char **argptr, int funfir
*** 1067,1073 ****
--- 1067,1097 ----
        if (val.symtab == 0)
  	val.symtab = s;
  
+       /* If funfirstline is set, we need to look up the function
+        containing the line, and move past the prologue. */
+ 
        val.pc = 0;
+       if (funfirstline)
+       {
+         CORE_ADDR pc = 0;
+ 
+         if (find_line_pc (val.symtab, val.line, &pc))
+           {
+             struct symbol *func_sym;
+             struct symtab_and_line sal;
+             
+             func_sym = find_pc_function (pc);
+             if (func_sym)
+               {
+                 sal = find_function_start_sal (func_sym, 1);
+                 /* Don't move the line, just set the pc
+                    to the right place. */
+                 if (val.line <= sal.line)
+                   val.pc = sal.pc;
+               }
+           }
+       }
+ 
        values.sals = (struct symtab_and_line *)
  	xmalloc (sizeof (struct symtab_and_line));
        values.sals[0] = val;

Re: [RFA/controversial] move bp by line number past the prologue

[Kevin Buettner]

On Aug 29, 11:15am, Joel Brobecker wrote:

> 2002-08-29  Jim Ingham <jingham@apple.com>
> 
>         * linespec.c (decode_line_1): Skip the function prologue if
>         funfirstline is set. Changes the behavior of the break command
>         to skip the function prologue when breaking by line number.

Joel,

My opinion is that this is a reasonable change providing that we
still have a mechanism for setting a breakpoint on the prologue.
When I want to do this I usually do ``b *foo'' instead of relying
on line numbers.  (Usually I don't know the line number anyway.)
So, I have no objection to this patch so long as it doesn't change
the present behavior of ``b *foo''.

I've reread the thread in which this matter was discussed earlier
(i.e, the "[RFC] breakpoints and function prologues" messages).  Given
that there was such disagreement before, I doubt that everyone will
agree with my point of view.  If this turns out to be the case, might
I suggest a user settable option for controlling whether setting a
breakpoint by line number will always cause the prologue to be
skipped?  That way both sides get the behavior they want.  (GUIs can
set the desired behavior at initialization time.)

Kevin

Re: [RFA/controversial] move bp by line number past the prologue

[Andrew Cagney]

> On Aug 29, 11:15am, Joel Brobecker wrote:
> 
> 
>> 2002-08-29  Jim Ingham <jingham@apple.com>
>> 
>>         * linespec.c (decode_line_1): Skip the function prologue if
>>         funfirstline is set. Changes the behavior of the break command
>>         to skip the function prologue when breaking by line number.
> 
> 
> Joel,
> 
> My opinion is that this is a reasonable change providing that we
> still have a mechanism for setting a breakpoint on the prologue.
> When I want to do this I usually do ``b *foo'' instead of relying
> on line numbers.  (Usually I don't know the line number anyway.)
> So, I have no objection to this patch so long as it doesn't change
> the present behavior of ``b *foo''.
> 
> I've reread the thread in which this matter was discussed earlier
> (i.e, the "[RFC] breakpoints and function prologues" messages).  Given
> that there was such disagreement before, I doubt that everyone will
> agree with my point of view.  If this turns out to be the case, might
> I suggest a user settable option for controlling whether setting a
> breakpoint by line number will always cause the prologue to be
> skipped?  That way both sides get the behavior they want.  (GUIs can
> set the desired behavior at initialization time.)

GUI and CLI behavior should be kept independant.  Running GDB under a 
GUI, should not affect the CLI.

Andrew

Re: [RFA/controversial] move bp by line number past the prologue

[Kevin Buettner]

On Aug 29,  4:33pm, Andrew Cagney wrote:

> > I've reread the thread in which this matter was discussed earlier
> > (i.e, the "[RFC] breakpoints and function prologues" messages).  Given
> > that there was such disagreement before, I doubt that everyone will
> > agree with my point of view.  If this turns out to be the case, might
> > I suggest a user settable option for controlling whether setting a
> > breakpoint by line number will always cause the prologue to be
> > skipped?  That way both sides get the behavior they want.  (GUIs can
> > set the desired behavior at initialization time.)
> 
> GUI and CLI behavior should be kept independant.  Running GDB under a 
> GUI, should not affect the CLI.

Perhaps a different interface is needed for the GUI to use?

Kevin

Re: [RFA/controversial] move bp by line number past the prologue

[Michael Snyder]

Kevin Buettner wrote:
> 
> On Aug 29,  4:33pm, Andrew Cagney wrote:
> 
> > > I've reread the thread in which this matter was discussed earlier
> > > (i.e, the "[RFC] breakpoints and function prologues" messages).  Given
> > > that there was such disagreement before, I doubt that everyone will
> > > agree with my point of view.  If this turns out to be the case, might
> > > I suggest a user settable option for controlling whether setting a
> > > breakpoint by line number will always cause the prologue to be
> > > skipped?  That way both sides get the behavior they want.  (GUIs can
> > > set the desired behavior at initialization time.)
> >
> > GUI and CLI behavior should be kept independant.  Running GDB under a
> > GUI, should not affect the CLI.
> 
> Perhaps a different interface is needed for the GUI to use?

I think I would prefer that.  In fact, in order of preference, 
I would prefer:

1) Teach the GUI users that the open curly brace represents the
prologue.
2) see 1.
3) see 1.
4) Provide an interface for GUIs to skip the prologue in this context.
5)....
....
...
...
99) Change the command line behavior.

Re: [RFA/controversial] move bp by line number past the prologue

[Joel Brobecker]

On Thu, Aug 29, 2002 at 01:16:35PM -0700, Kevin Buettner wrote:
> My opinion is that this is a reasonable change providing that we
> still have a mechanism for setting a breakpoint on the prologue.
> When I want to do this I usually do ``b *foo'' instead of relying
> on line numbers.  (Usually I don't know the line number anyway.)

I completely agree.

> I've reread the thread in which this matter was discussed earlier
> (i.e, the "[RFC] breakpoints and function prologues" messages).  Given
> that there was such disagreement before, I doubt that everyone will
> agree with my point of view.  If this turns out to be the case, might
> I suggest a user settable option for controlling whether setting a
> breakpoint by line number will always cause the prologue to be
> skipped?  That way both sides get the behavior they want.  (GUIs can
> set the desired behavior at initialization time.)

How about this change? Any advice on the option name and description
would be appreciated. With this patch, the 5 regressions I was
mentioning in my previous e-mail can be cleared by turning this new
behavior off (there is also one other expected regression in help.exp,
since the output of "help breakpoint" is modified by this patch). Before
sending a patch for these regressions, I prefer to wait for comments
regarding this patch.

2002-08-30  Jim Ingham <jingham@apple.com>

        * linespec.c (decode_line_1): Skip the function prologue if
        funfirstline is set, unless adjust_break_after_prologue is
        unset by the user. Changes the behavior of the break command
        to skip the function prologue when breaking by line number.

2002-08-30  Joel Brobecker  <brobecker@gnat.com>

        * linespec.c (adjust_break_after_prologue): New static variable.
        (_initialize_linespec): New function.

        * Makefile.in: Update the dependencies for linespec.c

-- 
Joel

Re: [RFA/controversial] move bp by line number past the prologue

[Joel Brobecker]

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

> 2002-08-30  Jim Ingham <jingham@apple.com>
> 
>         * linespec.c (decode_line_1): Skip the function prologue if
>         funfirstline is set, unless adjust_break_after_prologue is
>         unset by the user. Changes the behavior of the break command
>         to skip the function prologue when breaking by line number.
> 
> 2002-08-30  Joel Brobecker  <brobecker@gnat.com>
> 
>         * linespec.c (adjust_break_after_prologue): New static variable.
>         (_initialize_linespec): New function.
> 
>         * Makefile.in: Update the dependencies for linespec.c

KevinB rightfully pointed out that the patch was missing...

-- 
Joel

[-- Attachment #2: linespec.c.diff --]
[-- Type: text/plain, Size: 3674 bytes --]

Index: linespec.c
===================================================================
RCS file: /cvs/src/src/gdb/linespec.c,v
retrieving revision 1.21
diff -c -3 -p -r1.21 linespec.c
*** linespec.c	11 May 2002 22:46:19 -0000	1.21
--- linespec.c	30 Aug 2002 18:44:03 -0000
***************
*** 30,35 ****
--- 30,40 ----
  #include "value.h"
  #include "completer.h"
  #include "cp-abi.h"
+ #include "gdbcmd.h"
+ 
+ /* local variables */
+ 
+ static int adjust_break_after_prologue = 1;
  
  /* Prototype for one function in parser-defs.h,
     instead of including that entire file. */
*************** extern char *operator_chars (char *, cha
*** 42,47 ****
--- 47,54 ----
  
  /* Prototypes for local functions */
  
+ void _initialize_linespec (void);
+ 
  static void cplusplus_error (const char *name, const char *fmt, ...) ATTR_FORMAT (printf, 2, 3);
  
  static int total_number_of_methods (struct type *type);
*************** decode_line_1 (char **argptr, int funfir
*** 1067,1073 ****
--- 1074,1105 ----
        if (val.symtab == 0)
  	val.symtab = s;
  
+       /* If funfirstline is set, we need to look up the function
+        containing the line, and move past the prologue. This behavior
+        can be deactivated by unsetting adjust-break-after-prologue.  */
+ 
        val.pc = 0;
+       if (funfirstline && adjust_break_after_prologue)
+       {
+         CORE_ADDR pc = 0;
+ 
+         if (find_line_pc (val.symtab, val.line, &pc))
+           {
+             struct symbol *func_sym;
+             struct symtab_and_line sal;
+             
+             func_sym = find_pc_function (pc);
+             if (func_sym)
+               {
+                 sal = find_function_start_sal (func_sym, 1);
+                 /* Don't move the line, just set the pc
+                    to the right place. */
+                 if (val.line <= sal.line)
+                   val.pc = sal.pc;
+               }
+           }
+       }
+ 
        values.sals = (struct symtab_and_line *)
  	xmalloc (sizeof (struct symtab_and_line));
        values.sals[0] = val;
*************** minimal_symbol_found:		/* We also jump h
*** 1267,1270 ****
--- 1299,1317 ----
  
    error ("Function \"%s\" not defined.", copy);
    return values;		/* for lint */
+ }
+ 
+ void
+ _initialize_linespec (void)
+ {
+   struct cmd_list_element *c;
+ 
+   c = add_set_cmd ("adjust-break-after-prologue", class_breakpoint,
+                    var_boolean, (char *) &adjust_break_after_prologue,
+                    "Set whether GDB should adjust the location of a \
+ breakpoint past function prologues when computing its address from a line \
+ number.", &setlist);
+ 
+   add_show_from_set (c, &showlist);
+ 
  }
Index: Makefile.in
===================================================================
RCS file: /cvs/src/src/gdb/Makefile.in,v
retrieving revision 1.236
diff -c -3 -p -r1.236 Makefile.in
*** Makefile.in	14 Aug 2002 18:13:30 -0000	1.236
--- Makefile.in	30 Aug 2002 18:44:04 -0000
*************** symtab.o: symtab.c $(defs_h) $(symtab_h)
*** 2233,2239 ****
  
  linespec.o: linespec.c $(linespec_h) $(defs_h) $(frame_h) $(value_h) \
  	$(objfiles_h) $(symfile_h) $(completer_h) $(symtab_h) \
! 	$(demangle_h) $(command_h) $(cp_abi_h)
  
  macroexp.o: macroexp.c $(defs_h) $(gdb_obstack_h) $(bcache_h) $(macrotab_h) \
  	$(macroexp_h) $(gdb_assert_h)
--- 2233,2239 ----
  
  linespec.o: linespec.c $(linespec_h) $(defs_h) $(frame_h) $(value_h) \
  	$(objfiles_h) $(symfile_h) $(completer_h) $(symtab_h) \
! 	$(demangle_h) $(command_h) $(cp_abi_h) $(gdbcmd_h)
  
  macroexp.o: macroexp.c $(defs_h) $(gdb_obstack_h) $(bcache_h) $(macrotab_h) \
  	$(macroexp_h) $(gdb_assert_h)