Re: [RFA/commit] thread-specific breakpoints not saved properly by save-breakpoint

[Joel Brobecker <brobecker at adacore dot com>]

Hi Pedro,

Thanks for the review.
In particular:

> > +set test_name "info break before saving to file"
> > +gdb_test_multiple "info break" $test_name {
> > +    -re "(.*)$gdb_prompt " {
> 
> Any reason you're not anchoring these? ($ at end).

Mistake on my end... But moot as no longer needed, after
you remarked:

> > +gdb_test_multiple "info break" $test_name {
> > +    -re "${info_break_pattn}$gdb_prompt " {
> 
> What out for platforms where the executable will be loaded
> at different addresses on each run (e.g., gdbserver+uclinux).
> This will incorrectly fail there, as the "info break" output
> includes the addresses.  I think we need to be explicit on what
> we want to see here instead (and ignore the breakpoint's
> addresses).

So I adjusted the testcase to just do one `info break', and I match
the output explicitly (it's not pretty).

> Please add:
> remote_file host delete bps

Ooops, yes, that too.

Attached is what I ended up checking in.

-- 
Joel

commit 9f24bc6ad8f924d13976c5fbde6eb1e83350a3fa
Author: Joel Brobecker <brobecker@adacore.com>
Date:   Wed Sep 28 11:59:01 2011 -0700

    thread-specific breakpoints not saved properly by save-breakpoint
    
    Initially, I noticed that the save command was often missing
    new lines in the file that it generated.  For instance, consider:
    
            % gdb save-bp
            (gdb) b break_me
            (gdb) b save-bp.c:27
            (gdb) save breakpoints bps
    
    The contents of the bps file would be:
    
            % cat bps
            break break_mebreak save-bp.c:27
    
    Looking further into the problem, I realized that the missing newlines
    are just a consequence of a missing call to print_recreate_thread.
    After having generated the breakpoint location in the break command,
    we cannot put a new line until we have looked at whether we need to
    add a 'thread NUM' argument.
    
    gdb/ChangeLog:
    
            * breakpoint.c (bkpt_print_recreate): Add call to
            print_recreate_thread.
    
    gdb/testsuite/ChangeLog:
    
            * gdb.base/save-bp.exp, gdb.base/save-bp.c: New files.

diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index f995b14..08ff69b 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -11047,6 +11047,7 @@ bkpt_print_recreate (struct breakpoint *tp, struct ui_file *fp)
 		    _("unhandled breakpoint type %d"), (int) tp->type);
 
   fprintf_unfiltered (fp, " %s", tp->addr_string);
+  print_recreate_thread (tp, fp);
 }
 
 /* Virtual table for internal breakpoints.  */
diff --git a/gdb/testsuite/gdb.base/save-bp.c b/gdb/testsuite/gdb.base/save-bp.c
new file mode 100644
index 0000000..3469124
--- /dev/null
+++ b/gdb/testsuite/gdb.base/save-bp.c
@@ -0,0 +1,36 @@
+/* Copyright 2011 Free Software Foundation, Inc.
+
+   This file is part of GDB.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+void
+break_me (void)
+{
+}
+
+int
+main (void)
+{
+  int i;
+  break_me (); /* BREAK HERE.  */
+  break_me (); /* Try a thread-specific breakpoint.  */
+
+  for (i = 0; i < 5; i++)
+    break_me (); /* Try a condition-specific breakpoint.  */
+
+  break_me (); /* Finally, try a breakpoint with commands.  */
+  return 0;
+}
+
diff --git a/gdb/testsuite/gdb.base/save-bp.exp b/gdb/testsuite/gdb.base/save-bp.exp
new file mode 100644
index 0000000..1ab8646
--- /dev/null
+++ b/gdb/testsuite/gdb.base/save-bp.exp
@@ -0,0 +1,69 @@
+# Copyright (C) 2011 Free Software Foundation, Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+set testfile save-bp
+set srcfile ${testfile}.c
+
+if { [prepare_for_testing ${testfile}.exp ${testfile}] } {
+    return -1
+}
+
+if ![runto_main] {
+    untested ${testfile}.exp
+    return -1
+}
+# Delete all breakpoints so that the "runto_main" breakpoint above
+# does not interfere with our testing.
+delete_breakpoints
+
+# Insert a bunch of breakpoints... The goal is to create breakpoints
+# that we are going to try to save in a file and then reload.  So
+# try to create a good variety of them.
+gdb_breakpoint "break_me"
+
+set loc_bp2 [gdb_get_line_number "BREAK HERE"]
+gdb_breakpoint ${srcfile}:${loc_bp2}
+
+set loc_bp3 [gdb_get_line_number "thread-specific"]
+gdb_breakpoint "${srcfile}:${loc_bp3} thread 1"
+
+set loc_bp4 [gdb_get_line_number "condition"]
+gdb_breakpoint "${srcfile}:${loc_bp4} if i == 1"
+
+set loc_bp5 [gdb_get_line_number "with commands"]
+gdb_breakpoint ${srcfile}:${loc_bp5}
+gdb_test "commands\nsilent\nend" "End with.*" "add breakpoint commands"
+
+# Now, save the breakpoints into a file...
+remote_file host delete "bps"
+gdb_test "save breakpoint bps"
+
+# Now start a new debugger session...
+clean_restart $testfile
+if ![runto_main] {
+    fail "cannot restart program"
+    return -1
+}
+# Delete all breakpoints so that the "runto_main" breakpoint above
+# does not interfere with our testing.
+delete_breakpoints
+
+# ... and restore the breakpoints.
+gdb_test "source bps"
+
+# Now, verify that all breakpoints have been created correctly...
+set bp_row_start "\[0-9\]+ +breakpoint +keep +y +0x\[0-9a-f\]+ +in"
+gdb_test "info break" \
+  " *Num +Type +Disp +Enb +Address +What\r\n$bp_row_start break_me at .*$srcfile:\[0-9\]+\r\n$bp_row_start main at .*$srcfile:$loc_bp2\r\n$bp_row_start main at .*$srcfile:$loc_bp3 +thread 1\r\n\[ \t]+stop only in thread 1\r\n$bp_row_start main at .*$srcfile:$loc_bp4\r\n\[ \t\]+stop only if i == 1\r\n$bp_row_start main at .*$srcfile:$loc_bp5\r\n\[ \t\]+silent"