[RFA] Add option to skip prologue in "break FILE:LINENO" (take 2)

[RFA] Add option to skip prologue in "break FILE:LINENO" (take 2)

[Joel Brobecker]

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

Hello,

Following a request for comments regarding a proposed change of behavior
when inserting a breakpoint on a specific line number, we received a
variety of comments, and it appears very clearly that the proposal
was at the very least controversial.

So, here is a new proposal where the existing behavior is preserved.
A new set/show command is introduced to allow the user to configure
GDB to activate the new proposed behavior:

    (gdb) set/show breakpoint skip-prologue

The help text for this command explains the purpose of this option:

  Set the debugger behavior when inserting a breakpoint on a specific line.
  When inserting a breakpoint on a specific line number that corresponds
  to the prologue of a function, the debugger will automatically adjust
  its location to the first line past the prologue if this option is set.

By default, this option is OFF, preserving the current behavior.
I think this should take care of all the objections I read so far.

2008-01-10  Joel Brobecker  <brobecker@adacore.com>

        * breakpoint.c (breakpoint_skip_prologue_p): New static global.
        (show_breakpoint_skip_prologue): New function.
        (skip_prologue_sal): New function.
        (resolve_sal_pc): Adjust SAL past prologue if the SAL was computed
        from a line number and breakpoint_skip_prologue_p is set.
        (_initialize_breakpoint): Add new set/show break skip-prologue
        command.

Tested on x86-linux, no regression.
A new testcase, a NEWs entry, and proper documentation will be submitted
later if the patch is approved.

OK to commit?

Thanks,
-- 
Joel

[-- Attachment #2: skip-prologue.diff --]
[-- Type: text/plain, Size: 3049 bytes --]

Index: breakpoint.c
===================================================================
--- breakpoint.c	(revision 117)
+++ breakpoint.c	(revision 124)
@@ -246,6 +246,20 @@ Automatic usage of hardware breakpoints 
 		    value);
 }
 
+/* When a user inserts a breakpoint on a specific line that corresponds
+   to a function prologue, GDB should automatically adjust the location
+   to the first line past the prologue if BREAKPOINT_SKIP_PROLOGUE_P
+   is non-zero.  */
+static int breakpoint_skip_prologue_p = 0;
+static void
+show_breakpoint_skip_prologue (struct ui_file *file, int from_tty,
+			       struct cmd_list_element *c, const char *value)
+{
+  fprintf_filtered (file, _("\
+When breaking on a specific line, automatic adjustment of the breakpoint\n\
+location past the function prologue is %s.\n"),
+		    value);
+}
 
 void _initialize_breakpoint (void);
 
@@ -5446,6 +5460,25 @@ gdb_breakpoint (char *address, char *con
 			       0);
 }
 
+/* Adjust SAL to the first instruction past the function prologue.
+   The end of the prologue is determined using the line table from
+   the debugging information.
+
+   If SAL is already past the prologue, then do nothing.  */
+
+static void
+skip_prologue_sal (struct symtab_and_line *sal)
+{
+  struct symbol *sym = find_pc_function (sal->pc);
+  struct symtab_and_line start_sal;
+
+  if (sym == NULL)
+    return;
+
+  start_sal = find_function_start_sal (sym, 1);
+  if (sal->pc < start_sal.pc)
+    *sal = start_sal;
+}
 
 /* Helper function for break_command_1 and disassemble_command.  */
 
@@ -5460,6 +5493,13 @@ resolve_sal_pc (struct symtab_and_line *
 	error (_("No line %d in file \"%s\"."),
 	       sal->line, sal->symtab->filename);
       sal->pc = pc;
+
+      /* If this SAL corresponds to a breakpoint inserted using
+         a line number, and the debugger is configured to skip
+         function prologues in that case, then adjust the SAL
+         if necessary.  */
+      if (sal->explicit_line && breakpoint_skip_prologue_p)
+        skip_prologue_sal (sal);
     }
 
   if (sal->section == 0 && sal->symtab != NULL)
@@ -8282,6 +8322,18 @@ a warning will be emitted for such break
 			   show_automatic_hardware_breakpoints,
 			   &breakpoint_set_cmdlist,
 			   &breakpoint_show_cmdlist);
-  
+
+  add_setshow_boolean_cmd ("skip-prologue", class_breakpoint,
+                           &breakpoint_skip_prologue_p, _("\
+Set the debugger behavior when inserting a breakpoint on a specific line."),
+                           _("\
+Show the debugger behavior when inserting a breakpoint on a specific line."),
+                           _("\
+When inserting a breakpoint on a specific line number that corresponds\n\
+to the prologue of a function, the debugger will automatically adjust\n\
+its location to the first line past the prologue if this option is set."),
+                           NULL, show_breakpoint_skip_prologue,
+                           &breakpoint_set_cmdlist, &breakpoint_show_cmdlist);
+
   automatic_hardware_breakpoints = 1;
 }

Re: [RFA] Add option to skip prologue in "break FILE:LINENO" (take 2)

[Jim Blandy]

Joel Brobecker <brobecker at adacore.com> writes:
> Following a request for comments regarding a proposed change of behavior
> when inserting a breakpoint on a specific line number, we received a
> variety of comments, and it appears very clearly that the proposal
> was at the very least controversial.
>
> So, here is a new proposal where the existing behavior is preserved.
> A new set/show command is introduced to allow the user to configure
> GDB to activate the new proposed behavior:
>
>     (gdb) set/show breakpoint skip-prologue
>
> The help text for this command explains the purpose of this option:
>
>   Set the debugger behavior when inserting a breakpoint on a specific line.
>   When inserting a breakpoint on a specific line number that corresponds
>   to the prologue of a function, the debugger will automatically adjust
>   its location to the first line past the prologue if this option is set.
>
> By default, this option is OFF, preserving the current behavior.
> I think this should take care of all the objections I read so far.

Some of Michael's objections I didn't understand at all --- he seemed
to believe that the change would affect GDB's behavior in ways I think
it won't.  I don't think your change would affect the usage that was
most important to him.

I'd like to get that (or perhaps just me) straightened out before we
add an option.  Options of this sort are effective at resolving
contentious mailing list threads, but not very effective for the vast
majority of users.  Most users have little or no interest in GDB for
its own sake, and won't look for options like 'set breakpoint
skip-prologue', so subtle improvements that are disabled by default
have almost no impact.

Re: [RFA] Add option to skip prologue in "break FILE:LINENO" (take 2)

[Michael Snyder]

On Thu, 2008-01-10 at 09:24 -0800, Jim Blandy wrote:
> Joel Brobecker <brobecker at adacore.com> writes:
> > Following a request for comments regarding a proposed change of behavior
> > when inserting a breakpoint on a specific line number, we received a
> > variety of comments, and it appears very clearly that the proposal
> > was at the very least controversial.
> >
> > So, here is a new proposal where the existing behavior is preserved.
> > A new set/show command is introduced to allow the user to configure
> > GDB to activate the new proposed behavior:
> >
> >     (gdb) set/show breakpoint skip-prologue
> >
> > The help text for this command explains the purpose of this option:
> >
> >   Set the debugger behavior when inserting a breakpoint on a specific line.
> >   When inserting a breakpoint on a specific line number that corresponds
> >   to the prologue of a function, the debugger will automatically adjust
> >   its location to the first line past the prologue if this option is set.
> >
> > By default, this option is OFF, preserving the current behavior.
> > I think this should take care of all the objections I read so far.
> 
> Some of Michael's objections I didn't understand at all --- he seemed
> to believe that the change would affect GDB's behavior in ways I think
> it won't.  I don't think your change would affect the usage that was
> most important to him.
> 
> I'd like to get that (or perhaps just me) straightened out before we
> add an option.  Options of this sort are effective at resolving
> contentious mailing list threads, but not very effective for the vast
> majority of users.  Most users have little or no interest in GDB for
> its own sake, and won't look for options like 'set breakpoint
> skip-prologue', so subtle improvements that are disabled by default
> have almost no impact.

You're right -- initializations are treated as part of the function
body, not the prologue.  My concerns were largely unfounded.   ;-)

Re: [RFA] Add option to skip prologue in "break FILE:LINENO" (take 2)

[Jim Blandy]

Michael Snyder <msnyder at specifix.com> writes:
>> Some of Michael's objections I didn't understand at all --- he seemed
>> to believe that the change would affect GDB's behavior in ways I think
>> it won't.  I don't think your change would affect the usage that was
>> most important to him.
>
> You're right -- initializations are treated as part of the function
> body, not the prologue.  My concerns were largely unfounded.   ;-)

Yay!!  :)