[RFA 1/3] Change watchpoint's enable state in do_enable_breakpoint

Thiago Jung Bauermann
2011-04-18T21:22:34Z

Hi,

This patch changes watchpoint's enable state in do_enable_breakpoint
before calling update_watchpoint. It fixes a bug in the current code
which makes GDB change disabled hardware watchpoints to software
watchpoints when the inferior is restarted:

(gdb) watch a
Hardware watchpoint 2: a
(gdb) disable 2
(gdb) watch c
Hardware watchpoint 3: c
(gdb) disable 3
(gdb) watch d
Hardware watchpoint 4: d
(gdb) i b
Num Type Disp Enb Address What
2 hw watchpoint keep n a
3 hw watchpoint keep n c
4 hw watchpoint keep y d
(gdb) r
The program being debugged has been started already.
Start it from the beginning? (y or n) y
Starting program: /home/bauermann/builds/examples/test-watch64
a = 1
a = 2
Hardware watchpoint 4: d

Old value = 97 'a'
New value = 120 'x'
main (argc=1, argv=0xfffffd46a78) at ../../src/examples/test-watch.c:54
54 c = 31;
(gdb) i b
Num Type Disp Enb Address What
2 watchpoint keep n a
3 watchpoint keep n c
4 hw watchpoint keep y d

Notice that 2 and 3 are now software watchpoints.
This doesn't matter much today since watchpoints can change back and
forth between software and hardware watchpoints, but for masked
watchpoints it's important because they can't be changed to software
watchpoints. If you have a disabled masked watchpoint and there are not
enough debug registers for it, GDB will report an error when restarting
the inferior even though the watchpoint is disabled.

Tested without regressions on ppc-linux, ppc64-linux and i386-linux. Ok?
--
[]'s
Thiago Jung Bauermann
IBM Linux Technology Center

2011-04-18 Thiago Jung Bauermann <bauerman@br.ibm.com>

* breakpoint.c (update_watchpoint): Move code to change
the enable state of breakpoint from here ...
(do_enable_breakpoint): ... to here.

diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index 8ea6cc9..744057a 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -1416,7 +1416,6 @@ update_watchpoint (struct breakpoint *b, int reparse)
if (reg_cnt)
{
int i, target_resources_ok, other_type_used;
- enum enable_state orig_enable_state;

/* We need to determine how many resources are already
used for all other hardware watchpoints plus this one
@@ -1427,17 +1426,9 @@ update_watchpoint (struct breakpoint *b, int reparse)
watchpoint. */
b->type = bp_hardware_watchpoint;

- /* hw_watchpoint_used_count ignores disabled watchpoints,
- and b might be disabled if we're being called from
- do_enable_breakpoint. */
- orig_enable_state = b->enable_state;
- b->enable_state = bp_enabled;
-
i = hw_watchpoint_used_count (bp_hardware_watchpoint,
&other_type_used);

- b->enable_state = orig_enable_state;
-
target_resources_ok = target_can_use_hardware_watchpoint
(bp_hardware_watchpoint, i, other_type_used);
if (target_resources_ok <= 0)
@@ -11490,14 +11481,18 @@ do_enable_breakpoint (struct breakpoint *bpt, enum bpdisp disposition)

if (is_watchpoint (bpt))
{
+ enum enable_state orig_enable_state;
struct gdb_exception e;

TRY_CATCH (e, RETURN_MASK_ALL)
{
+ orig_enable_state = bpt->enable_state;
+ bpt->enable_state = bp_enabled;
update_watchpoint (bpt, 1 /* reparse */);
}
if (e.reason < 0)
{
+ bpt->enable_state = orig_enable_state;
exception_fprintf (gdb_stderr, e, _("Cannot enable watchpoint %d: "),
bpt->number);
return;