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

[Pedro Alves <pedro at codesourcery dot com>]

On Wednesday 28 September 2011 21:41:37, Joel Brobecker wrote:
> 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.

Ooops.  I think I broke this with the "all breakpoints use breakpoint_ops"
changes.  The reason we need to put print_recreate_thread in all
implementations is that tracepoints need to output the passcount as well,
and that needs to be done after the "thread N" bit.

> 
> I was quite surprised to see that we actually do not have a testcase
> for the "save breakpoint" command??? So I added one.

Thanks.  I wish I had done so, but had no time to when I wrote
the command, and then forgot.

> 
> 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.

typo, save-bp.exp.

> +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).
There are scenarios that we can't, but otherwise, the
anchor sometimes catches cases were we print something
after the prompt when we shouldn't.

> +        set info_break "$expect_out(1,string)"
> +        pass $test_name
> +    }
> +}
> +
> +# Now, save the breakpoints into a file...

Please add:

remote_file host delete bps

so that if this fails, the rest of the tests don't run
with a stale bps from a previous run.

> +gdb_test "save breakpoint bps"

> +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).

-- 
Pedro Alves