Re: [patch] fix for PR2424

[Vladimir Prus <vladimir@codesourcery.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