Re: [patch] fix for PR2424

[Vladimir Prus <vladimir at codesourcery dot com>]

On Monday 10 March 2008 20:18:06 Aleksandar Ristovski wrote:
> Vladimir Prus wrote:
> > 
> > I think this is good. You did not attach your patches, so I cannot comment on
> > the code itself.
> Patches attached.
> > 
> > Also, since this code touches infrun.c and affects CLI, I think some of the
> > global maintainers will have to approve it.
> 
> The proposed patch (attached) will produce the following outputs:
> 
> CLI:
> (gdb) tbreak main
> Temporary breakpoint 1 at 0x80483a0: file ./main.c, line 15.
> (gdb) r
> Starting program: /space/src/testcases/sigsegv/main
> 
> Temporary breakpoint 1, main () at ./main.c:15
> 15 ? ? ? ?foo (p);
> 
> MI:
> (gdb)
> -break-insert -t main
> ^done,bkpt={number="1",type="breakpoint",disp="del",enabled="y",addr="0x080483a0",func="main",file="./main.c",fullname="/space/src/testcases/sigsegv/main.c",line="15",times="0"}
> (gdb)
> -exec-run
> ^running
> (gdb)
> *stopped,reason="breakpoint-hit",disp="del",bkptno="1",thread-id="0",frame={addr="0x080483a0",func="main",args=[],file="./main.c",fullname="/space/src/testcases/sigsegv/main.c",line="15"}
> 
> 
> 
> 
> ----- ChangeLog
> 2008-03-10 ?Aleksandar Ristovski <aristovski@qnx.com>
> 
> ????????* infrun.c (normal_stop) Move breakpoint_auto_delete further down
> ????????to allow printing to 'see' real reason of stop. This fixes PR 2424.
> ????????* breakpoint.c (print_it_typical): Print "Temporary breakpoint" instead
> ????????of just "Breakpoint" when breakpoint is, well, temporary.
> ????????(mention): Likewise.
> 
> ----- testsuite ChangeLog
> 2008-03-10 ?Aleksandar Ristovski ?<aristovski@qnx.com>
> 
> ????????* gdb.base/attach.exp (proc_do_attach_tests): Matching pattern for
> ????????temporary breakpoint to match "Temporary breakpoint".
> ????????* gdb.base/break.exp (delete_breakpoints): Likewise.
> ????????* gdb.base/call-ar-st.exp (get_debug_format): Likewise.
> ????????* gdb.base/commands.exp (temporary_breakpoint_commands): Likewise.
> ????????* gdb.base/display.exp: Likewise.
> ????????* gdb.base/foll-exec.exp (do_exec_tests): Likewise.
> ????????* gdb.base/restore.exp (restore_tests): Likewise.
> ????????* gdb.base/sepdebug.exp: Likewise.
> ????????* gdb.base/watchpoint.exp: Likewise.
> ????????* gdb.mi/mi-pending.exp: Matching pattern for new "disp" field.
> ????????* gdb.mi/mi-simplerun.exp (test_running_the_program): Likewise.
> ????????* gdb.mi/mi-until.exp (test_runnint_to_foo): Likewise.
> ????????* gdb.mi/mi-var-display.exp: Likewise.
> ????????* gdb.mi/mi2-simplerun.exp (test_running_the_program): Likewise.
> ????????* gdb.mi/mi2-until.exp (test_running_to_foo): Likewise.
> ????????* gdb.mi/mi2-var-display.exp: Likewise.
> ????????* lib/gdb.exp (gdb_breakpoint): Likewise.
> ????????* lib/mi-support.exp: Add "reason" and "disp" fields to the matching
> ????????pattern. This is to support fix for PR2424.
> 
> 
> PR2424.diff
>   Index: gdb/infrun.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/infrun.c,v
> retrieving revision 1.266
> diff -u -p -r1.266 infrun.c
> --- gdb/infrun.c????????29 Jan 2008 22:47:19 -0000??????1.266
> +++ gdb/infrun.c????????10 Mar 2008 15:24:07 -0000
> @@ -3151,11 +3151,6 @@ Further execution is probably impossible
> ????????}
> ? ? ?}
> ?
> - ?/* Delete the breakpoint we stopped at, if it wants to be deleted.
> - ? ? Delete any breakpoint that is to be deleted at the next stop. ?*/
> -
> - ?breakpoint_auto_delete (stop_bpstat);
> -
> ? ?/* If an auto-display called a function and that got a signal,
> ? ? ? delete that auto-display to avoid an infinite recursion. ?*/
> ?
> @@ -3294,6 +3289,9 @@ Further execution is probably impossible
> ?done:
> ? ?annotate_stopped ();
> ? ?observer_notify_normal_stop (stop_bpstat);
> + ?/* Delete the breakpoint we stopped at, if it wants to be deleted.
> + ? ? Delete any breakpoint that is to be deleted at the next stop. ?*/
> + ?breakpoint_auto_delete (stop_bpstat);
> ?}
> ?
> ?static int
> Index: gdb/breakpoint.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/breakpoint.c,v
> retrieving revision 1.304
> diff -u -p -r1.304 breakpoint.c
> --- gdb/breakpoint.c????27 Feb 2008 20:27:49 -0000??????1.304
> +++ gdb/breakpoint.c????10 Mar 2008 15:24:10 -0000
> @@ -2131,6 +2131,7 @@ print_it_typical (bpstat bs)
> ? ?struct breakpoint *b;
> ? ?const struct bp_location *bl;
> ? ?struct ui_stream *stb;
> + ?int bp_temp = 0; ?
> ? ?stb = ui_out_stream_new (uiout);
> ? ?old_chain = make_cleanup_ui_out_stream_delete (stb);
> ? ?/* bs->breakpoint_at can be NULL if it was a momentary breakpoint
> @@ -2144,15 +2145,22 @@ print_it_typical (bpstat bs)
> ? ? ?{
> ? ? ?case bp_breakpoint:
> ? ? ?case bp_hardware_breakpoint:
> + ? ? ?bp_temp = bs->breakpoint_at->owner->disposition == disp_del;
> ? ? ? ?if (bl->address != bl->requested_address)
> ????????breakpoint_adjustment_warning (bl->requested_address,
> ???????? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? bl->address,
> ???????????????????????????????? ? ? ? b->number, 1);
> ? ? ? ?annotate_breakpoint (b->number);
> - ? ? ?ui_out_text (uiout, "\nBreakpoint ");
> + ? ? ?if (bp_temp) 
> +???????ui_out_text (uiout, "\nTemporary breakpoint ");
> + ? ? ?else
> +???????ui_out_text (uiout, "\nBreakpoint ");
> ? ? ? ?if (ui_out_is_mi_like_p (uiout))
> -???????ui_out_field_string (uiout, "reason", 
> -??????????????????????? ? ? async_reason_lookup (EXEC_ASYNC_BREAKPOINT_HIT));
> +???????{
> +??????? ?ui_out_field_string (uiout, "reason", 
> +??????????????????????? ?async_reason_lookup (EXEC_ASYNC_BREAKPOINT_HIT));
> +??????? ?ui_out_field_string (uiout, "disp", bp_temp ? "del" : "keep");
> +???????}

For consistency, it's better to use same printing of 'disp' as done
in print_one_breakpoint_location:

   ui_out_field_string (uiout, "disp", bpdisps[(int) b->disposition]);

bpdisps is actually a table local to print_one_breakpoint_location,
but there's no reason why it cannot be made a global table.

- Volodya