Re: x86 watchpoints bug (Re: ping: Re: PATCH : allow to set length of hw watchpoints (e.g. for Valgrind gdbserver))

Pedro Alves
2011-07-22T16:40:27Z

On Friday 22 July 2011 17:02:42, Philippe Waroquiers wrote:

> During the testing, I however found something else slightly strange.
> With reference to the previous s.c test program, watching a string length 1000
> is ok at the start (handled as a sw breakpoint), but this watchpoint cannot be disabled
> then re-enabled:
> (gdb) watch s1000
> Hardware watchpoint 1: s1000
> (gdb) start <<<<<<<<<<<<<<<<<<<<<<<<<< this runs slowly as s1000 is sw-watched
> Temporary breakpoint 2 at 0x400480: file s.c, line 22.
> Starting program: /home/philippe/gdb/s
> Error in re-setting breakpoint 1: Expression cannot be implemented with read/access watchpoint.
> Error in re-setting breakpoint 1: Expression cannot be implemented with read/access watchpoint.
> Error in re-setting breakpoint 1: Expression cannot be implemented with read/access watchpoint.
>
> Temporary breakpoint 2, main () at s.c:22
> 22 char * p = s1000;
> (gdb) dis 1
> (gdb) ena 1
> Cannot enable watchpoint 1: Expression cannot be implemented with read/access watchpoint.
> (gdb)
> At this point, if the watchpoint is deleted then re-created, then the watchpoint is again 'sw-accepted'.
> Note that this looks to be a regression in 7.3.50.20110722-cvs, as I do not see the same problem on 7.2.

Hmm, I can reproduce this.

#0 error (string=...) at ../../src/gdb/utils.c:776
#1 0x000000000064733d in update_watchpoint (b=0x1f4fde0, reparse=1) at ../../src/gdb/breakpoint.c:1456
#2 0x000000000065a364 in enable_breakpoint_disp (bpt=0x1f4fde0, disposition=disp_donttouch) at ../../src/gdb/breakpoint.c:11917

1453 }
1454 else if (b->ops && b->ops->works_in_software_mode
1455 && !b->ops->works_in_software_mode (b))
1456 error (_("Expression cannot be implemented with "
1457 "read/access watchpoint."));
1458 else
1459 b->type = bp_watchpoint;

(top-gdb) info symbol b->ops
watchpoint_breakpoint_ops in section .data of /home/pedro/gdb/baseline/build/gdb/gdb

(top-gdb) info symbol b->ops->works_in_software_mode
works_in_software_mode_watchpoint in section .text of /home/pedro/gdb/baseline/build/gdb/gdb

int
works_in_software_mode_watchpoint (const struct breakpoint *b)
{
return b->type == bp_hardware_watchpoint;
}

(top-gdb) p b->type
$5 = bp_watchpoint

From the error string, looks like the check should be something like:

else if (b->type == bp_read_watchpoint
|| b->type == bp_access_watchpoint)
error (_("Expression cannot be implemented with "
"read/access watchpoint."));

instead, as those watchpoints can't indeed be implemented
as software watchpoints. Though the intention may have
been to catch something about masked watchpoints.
Maybe better would be to change works_in_software_mode_watchpoint to:

int
works_in_software_mode_watchpoint (const struct breakpoint *b)
{
- return b->type == bp_hardware_watchpoint;
+ return (b->type == bp_watchpoint || b->type == bp_hardware_watchpoint);
}

The error string could also be enhanced to include the real
watchpoint type (so a user of masked watchpoints doesn't get
confused).

Thiago, WDYT?

--
Pedro Alves