[RFC] Wrong hw_watchpoint_used_count? (multiple location watchpoints)

Joel Brobecker
2010-01-08T07:57:18Z

[-- Attachment #1: Type: text/plain, Size: 1420 bytes --]

This is mostly from code inspection, while investigating something else,
but I think that the current implementation of hw_watchpoint_used_count
is inaccurate. It counts the number of breakpoints which match the given
type. But in fact, when I looked at the way the watchpoint is created,
watchpoints may have multiple bp_locations. So, a better approximation
would be to count the number of bp_locations, no?

Even so, this does not appear to be entirely accurate either. For
instance, if the user tries to watch an entity that does not fit
in one watchpoint, I think we can still end up with a situation
where we have one bp_location needing 2 h/w watchpoints.
In the end, it looks like the only way to really clean things up
would be to store for each watchpoint the associated number of
needed hardware watchpoints...

I thought about making the following change. It's not clear that it is
a step in the right direction or not. Pragmatically, I think it gives
more accurate results. In terms of implementation, if we eventually
decide to store the mem_cnt in the breakpoint, then we'll have to
go back to iterating over all breakpoints (not bp_locations)...

When in doubt, do nothing?

* breakpoint.c (hw_watchpoint_used_count): Compute the number
of hardware watchpoints by iterating over all bp_locations
instead of all breakpoints.

Tested on x86_64-linux. No regression...

--
Joel

[-- Attachment #2: 0001-Wrong-hw_watchpoint_used_count-multiple-location-wat.patch --]
[-- Type: text/x-diff, Size: 1050 bytes --]

From 6b81fca8153717a836ecee2d4e7ce891b3d32a6e Mon Sep 17 00:00:00 2001
From: Joel Brobecker <brobecker@adacore.com>
Date: Tue, 29 Dec 2009 19:06:50 +0400
Subject: [PATCH] Wrong hw_watchpoint_used_count? (multiple location watchpoints)

* breakpoint.c (hw_watchpoint_used_count): Compute the number
of hardware watchpoints by iterating over all bp_locations
instead of all breakpoints.

---
gdb/breakpoint.c | 6 ++++--
1 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index 0dc8474..91587cb 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -5820,12 +5820,14 @@ hw_breakpoint_used_count (void)
static int
hw_watchpoint_used_count (enum bptype type, int *other_type_used)
{
- struct breakpoint *b;
+ struct bp_location *loc, **loc_temp;
int i = 0;

*other_type_used = 0;
- ALL_BREAKPOINTS (b)
+ ALL_BP_LOCATIONS (loc, loc_temp)
{
+ struct breakpoint *b = loc->owner;
+
if (breakpoint_enabled (b))
{
if (b->type == type)
--
1.6.3.3