Re: RFA: breakpoint.c: Minor output fixes for hardware watchpoints

[Michael Snyder <msnyder at cygnus dot com>]

Peter.Schauer wrote:
> 
> During implementation of hardware watchpoints on Solaris, I noticed the
> following inconsistencies in breakpoint.c output between software and
> hardware breakpoints.
> 
> 1) insert_breakpoints and do_enable_breakpoint now use select_and_print_frame
>   to reselect frames after frame selection for watchpoint evaluation.
>   Up to gdb-4.18 GDB used select_frame to reselect frames.
>   I have no idea why this was done (I am unable to find a ChangeLog entry,
>   perhaps this was a side effect of the HP merge), and it causes the following
>   confusing output from the recurse.exp testcase:

This change did indeed come in with the HP merge, which was so 
massive that we did not get justifications for every little
change.  I'm going to approve backing this change out.  If 
the folks at HP really need it, let's get a justification 
and a change that doesn't have unpleasant side effects.

Checking in this patch.

> continue
> Continuing.
> #0  recurse (a=10) at /users/pes/gdbnd/devo/gdb/testsuite/gdb.base/recurse.c:15
> 15        if (a == 1)
> Hardware watchpoint 2: b
> 
> Old value = 0
> New value = 10
> recurse (a=10) at /users/pes/gdbnd/devo/gdb/testsuite/gdb.base/recurse.c:19
> 19        b *= recurse (a - 1);
> (gdb) PASS: gdb.base/recurse.exp: continue to first instance watchpoint, first time
> 
> Note the extra frame output before the watchpoint trigger. This is still
> worse in other cases, where we have even more frame output (see below).
> 
> 2) insert_breakpoints tries to reinsert watchpoints even if they are already
>   marked for deletion at next stop, causing the following redundant output,
>   again from the recurse.exp testcase:
> 
> continue
> Continuing.
> #0  recurse (a=5) at /users/pes/gdbnd/devo/gdb/testsuite/gdb.base/recurse.c:20
> 20        return b;
> #0  recurse (a=5) at /users/pes/gdbnd/devo/gdb/testsuite/gdb.base/recurse.c:20
> 20        return b;
> Watchpoint 4 deleted because the program has left the block in
> which its expression is valid.
> #0  0x8048a71 in recurse (a=6) at /users/pes/gdbnd/devo/gdb/testsuite/gdb.base/recurse.c:19
> 19        b *= recurse (a - 1);
> Hardware watchpoint 4 deletedbecause the program has left the block
> in which its expression is valid.
> 0x8048a70 in recurse (a=6) at /users/pes/gdbnd/devo/gdb/testsuite/gdb.base/recurse.c:19
> 19        b *= recurse (a - 1);
> (gdb) PASS: gdb.base/recurse.exp: second instance watchpoint deleted when leaving scope
> 
> Here is a patch for both problems:
> 
> 2000-03-07  Peter Schauer  <pes@regent.e-technik.tu-muenchen.de>
> 
>         * breakpoint.c (insert_breakpoints, do_enable_breakpoint):  Reselect
>         the saved frame silently after frame selection for watchpoint
>         evaluation.
>         (insert_breakpoints):  Add missing space in `Hardware watchpoint
>         deleted' message.  Do not reinsert hardware watchpoint if it is
>         already marked for deletion at next stop.
> 
> *** gdb/breakpoint.c.orig       Thu Feb 24 13:41:33 2000
> --- gdb/breakpoint.c    Sat Mar  4 11:42:48 2000
> ***************
> *** 918,923 ****
> --- 918,924 ----
>               b->type == bp_read_watchpoint ||
>               b->type == bp_access_watchpoint)
>              && b->enable == enabled
> +            && b->disposition != del_at_next_stop
>              && !b->inserted
>              && !b->duplicate)
>         {
> ***************
> *** 1005,1011 ****
>           }
>         else
>           {
> !           printf_filtered ("Hardware watchpoint %d deleted", b->number);
>             printf_filtered ("because the program has left the block \n");
>             printf_filtered ("in which its expression is valid.\n");
>             if (b->related_breakpoint)
> --- 1006,1012 ----
>           }
>         else
>           {
> !           printf_filtered ("Hardware watchpoint %d deleted ", b->number);
>             printf_filtered ("because the program has left the block \n");
>             printf_filtered ("in which its expression is valid.\n");
>             if (b->related_breakpoint)
> ***************
> *** 1016,1022 ****
>         /* Restore the frame and level.  */
>         if ((saved_frame != selected_frame) ||
>             (saved_level != selected_frame_level))
> !         select_and_print_frame (saved_frame, saved_level);
> 
>         if (val)
>           return_val = val;     /* remember failure */
> --- 1017,1023 ----
>         /* Restore the frame and level.  */
>         if ((saved_frame != selected_frame) ||
>             (saved_level != selected_frame_level))
> !         select_frame (saved_frame, saved_level);
> 
>         if (val)
>           return_val = val;     /* remember failure */
> ***************
> *** 7536,7543 ****
>         }
> 
>         if (save_selected_frame_level >= 0)
> !       select_and_print_frame (save_selected_frame,
> !                               save_selected_frame_level);
>         value_free_to_mark (mark);
>       }
>     if (modify_breakpoint_hook)
> --- 7558,7564 ----
>         }
> 
>         if (save_selected_frame_level >= 0)
> !       select_frame (save_selected_frame, save_selected_frame_level);
>         value_free_to_mark (mark);
>       }
>     if (modify_breakpoint_hook)
> 
> --
> Peter Schauer                   pes@regent.e-technik.tu-muenchen.de