Re: [RFA]: Watchpoints per thread patch

Jeff Johnston
2004-10-27T22:36:24Z

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

Daniel Jacobowitz wrote:
> On Wed, Oct 20, 2004 at 01:27:15PM -0400, Andrew Cagney wrote:
>
>>the underlying target (ia64-linux-nat, ...) can locally override the
>>method and handle the problem. The code's the same, but how it is wired
>>up is different
>>
>>Sound reasonable to all?
>
>
> I think that sounds pretty good. Hopefully the changes involved will
> be pretty small, since I imagine that most GNU/Linux targets with
> hardware watchpoints will want it.
>

The attached patch is the rework of my original attempt. It no longer uses
configuration or magic defines. Per Mark's suggestion, it uses an observer to
handle inserting watchpoints on a new thread and only the low-level code knows
about inserting/removing watchpoints on all threads.

Ok to commit?

-- Jeff J.

2004-10-27 Jeff Johnston <jjohnstn@redhat.com>

* breakpoint.c (insert_watchpoints_for_new_thread): New function.
* breakpoint.h (insert_watchpoints_for_new_thread): New prototype.
* ia64-linux-nat.c (ia64_linux_insert_one_watchpoint): New function.
(ia64_linux_insert_watchpoint_callback): Ditto.
(ia64_linux_insert_watchpoint): Change to iterate through lwps
and insert the specified watchpoint per thread.
(ia64_linux_remove_one_watchpoint): New function.
(ia64_linux_remove_watchpoint_callback): Ditto.
(ia64_linux_remove_watchpoint): Change to iterate through lwps and
remove the specified watchpoint for each thread.
(ia64_linux_new_thread): New thread observer.
(_initialize_ia64_linux_nat): New function. Initialize
new thread observer.
* thread-db.c (attach_thread): Notify any observers of the new
thread event.
* s390-nat.c (s390_tid): New function.
(s390_inferior_tid): Change to call s390_tid.
(s390_remove_one_watchpoint): New function.
(s390_remove_watchpoint_callback): Ditto.
(s390_remove_watchpoint): Change to iterate through lwps and
remove the specified watchpoint for each thread.
(s390_insert_one_watchpoint): New function.
(s390_insert_watchpoint_callback): Ditto.
(s390_insert_watchpoint): Change to iterate through lwps and
insert the specified watchpoint on each thread.
(s390_new_thread): New thread observer.
(_initialize_s390_nat): New function. Initialize
new thread observer.

doc/ChangeLog:

2004-10-27 Jeff Johnston <jjohnstn@redhat.com>

* observer.texi (new_thread): New observer.

[-- Attachment #2: watchthreads3.patch --]
[-- Type: text/plain, Size: 14516 bytes --]

Index: breakpoint.c
===================================================================
RCS file: /cvs/src/src/gdb/breakpoint.c,v
retrieving revision 1.183
diff -u -p -r1.183 breakpoint.c
--- breakpoint.c 8 Oct 2004 17:30:46 -0000 1.183
+++ breakpoint.c 27 Oct 2004 21:43:50 -0000
@@ -748,6 +748,91 @@ insert_catchpoint (struct ui_out *uo, vo
return 0;
}

+/* External function to insert all existing watchpoints on a newly
+ attached thread. IWPFN is a callback function to perform
+ the target insert watchpoint. This function is used to support
+ platforms whereby a watchpoint must be inserted/removed on each
+ individual thread (e.g. ia64-linux and s390-linux). For
+ ia64 and s390 linux, this function is called via a new thread
+ observer. */
+int
+insert_watchpoints_for_new_thread (ptid_t new_thread,
+ insert_watchpoint_ftype *iwpfn)
+{
+ struct bp_location *b;
+ int val = 0;
+ int return_val = 0;
+ struct ui_file *tmp_error_stream = mem_fileopen ();
+ make_cleanup_ui_file_delete (tmp_error_stream);
+
+ /* Explicitly mark the warning -- this will only be printed if
+ there was an error. */
+ fprintf_unfiltered (tmp_error_stream, "Warning:\n");
+
+ ALL_BP_LOCATIONS (b)
+ {
+ /* Skip disabled breakpoints. */
+ if (!breakpoint_enabled (b->owner))
+ continue;
+
+ /* For every active watchpoint, we need to insert the watchpoint on
+ the new thread. */
+ if ((b->loc_type == bp_loc_hardware_watchpoint
+ || b->owner->type == bp_watchpoint))
+ {
+ struct value *v = b->owner->val_chain;
+
+ /* Look at each value on the value chain. */
+ for (; v; v = v->next)
+ {
+ /* If it's a memory location, and GDB actually needed
+ its contents to evaluate the expression, then we
+ must watch it. */
+ if (VALUE_LVAL (v) == lval_memory
+ && ! VALUE_LAZY (v))
+ {
+ struct type *vtype = check_typedef (VALUE_TYPE (v));
+
+ /* We only watch structs and arrays if user asked
+ for it explicitly, never if they just happen to
+ appear in the middle of some value chain. */
+ if (v == b->owner->val_chain
+ || (TYPE_CODE (vtype) != TYPE_CODE_STRUCT
+ && TYPE_CODE (vtype) != TYPE_CODE_ARRAY))
+ {
+ CORE_ADDR addr;
+ int len, type;
+
+ addr = VALUE_ADDRESS (v) + VALUE_OFFSET (v);
+ len = TYPE_LENGTH (VALUE_TYPE (v));
+ type = hw_write;
+ if (b->owner->type == bp_read_watchpoint)
+ type = hw_read;
+ else if (b->owner->type == bp_access_watchpoint)
+ type = hw_access;
+ val = (*iwpfn) (new_thread, addr, len, type);
+ }
+ }
+ }
+ }
+
+ if (val)
+ return_val = val;
+ }
+
+ /* Failure to insert a watchpoint on any memory value in the
+ value chain brings us here. */
+ if (return_val)
+ {
+ fprintf_unfiltered (tmp_error_stream,
+ "%s\n",
+ "Could not insert hardware watchpoints on new thread.");
+ target_terminal_ours_for_output ();
+ error_stream (tmp_error_stream);
+ }
+ return return_val;
+}
+
/* Helper routine: free the value chain for a breakpoint (watchpoint). */

static void free_valchain (struct bp_location *b)
Index: breakpoint.h
===================================================================
RCS file: /cvs/src/src/gdb/breakpoint.h,v
retrieving revision 1.34
diff -u -p -r1.34 breakpoint.h
--- breakpoint.h 13 May 2004 16:39:11 -0000 1.34
+++ breakpoint.h 27 Oct 2004 21:43:50 -0000
@@ -663,6 +663,12 @@ extern void tbreak_command (char *, int)

extern int insert_breakpoints (void);

+/* The following provides a callback mechanism to insert watchpoints
+ for a new thread. This is needed, for example, on ia64 linux. */
+typedef int (insert_watchpoint_ftype) (ptid_t, CORE_ADDR, int, int);
+extern int insert_watchpoints_for_new_thread (ptid_t ptid,
+ insert_watchpoint_ftype *fn);
+
extern int remove_breakpoints (void);

/* This function can be used to physically insert eventpoints from the
Index: ia64-linux-nat.c
===================================================================
RCS file: /cvs/src/src/gdb/ia64-linux-nat.c,v
retrieving revision 1.26
diff -u -p -r1.26 ia64-linux-nat.c
--- ia64-linux-nat.c 13 Oct 2004 21:40:41 -0000 1.26
+++ ia64-linux-nat.c 27 Oct 2004 21:43:50 -0000
@@ -39,6 +39,8 @@

#include <asm/ptrace_offsets.h>
#include <sys/procfs.h>
+#include "observer.h"
+#include "linux-nat.h"

/* Prototypes for supply_gregset etc. */
#include "gregset.h"
@@ -559,8 +561,9 @@ is_power_of_2 (int val)
return onecount <= 1;
}

-int
-ia64_linux_insert_watchpoint (ptid_t ptid, CORE_ADDR addr, int len, int rw)
+/* Internal routine to insert one watchpoint for a specified thread. */
+static int
+ia64_linux_insert_one_watchpoint (ptid_t ptid, CORE_ADDR addr, int len, int rw)
{
int idx;
long dbr_addr, dbr_mask;
@@ -606,8 +609,38 @@ ia64_linux_insert_watchpoint (ptid_t pti
return 0;
}

+/* Internal callback routine which can be used via iterate_over_lwps
+ to insert a specific watchpoint from all active threads. */
+static int
+ia64_linux_insert_watchpoint_callback (struct lwp_info *lwp, void *data)
+{
+ struct target_watchpoint *args = (struct target_watchpoint *)data;
+
+ return ia64_linux_insert_one_watchpoint (lwp->ptid, args->addr,
+ args->len, args->type);
+}
+
+/* Insert a watchpoint for all threads. */
int
-ia64_linux_remove_watchpoint (ptid_t ptid, CORE_ADDR addr, int len)
+ia64_linux_insert_watchpoint (ptid_t ptid, CORE_ADDR addr, int len, int rw)
+{
+ struct target_watchpoint args;
+
+ args.addr = addr;
+ args.len = len;
+ args.type = rw;
+
+ /* For ia64, watchpoints must be inserted/removed on each thread so
+ we iterate over the lwp list. */
+ if (iterate_over_lwps (&ia64_linux_insert_watchpoint_callback, &args))
+ return -1;
+
+ return 0;
+}
+
+/* Internal routine to remove one watchpoint for a specified thread. */
+static int
+ia64_linux_remove_one_watchpoint (ptid_t ptid, CORE_ADDR addr, int len)
{
int idx;
long dbr_addr, dbr_mask;
@@ -630,6 +663,34 @@ ia64_linux_remove_watchpoint (ptid_t pti
return -1;
}

+/* Internal callback routine which can be used via iterate_over_lwps
+ to remove a specific watchpoint from all active threads. */
+static int
+ia64_linux_remove_watchpoint_callback (struct lwp_info *lwp, void *data)
+{
+ struct target_watchpoint *args = (struct target_watchpoint *)data;
+
+ return ia64_linux_remove_one_watchpoint (lwp->ptid, args->addr,
+ args->len);
+}
+
+/* Remove a watchpoint for all threads. */
+int
+ia64_linux_remove_watchpoint (ptid_t ptid, CORE_ADDR addr, int len)
+{
+ struct target_watchpoint args;
+
+ args.addr = addr;
+ args.len = len;
+
+ /* For ia64, watchpoints must be inserted/removed on each thread so
+ we iterate over the lwp list. */
+ if (iterate_over_lwps (&ia64_linux_remove_watchpoint_callback, &args))
+ return -1;
+
+ return 0;
+}
+
int
ia64_linux_stopped_data_address (CORE_ADDR *addr_p)
{
@@ -674,3 +735,18 @@ ia64_linux_xfer_unwind_table (struct tar
{
return syscall (__NR_getunwind, readbuf, len);
}
+
+/* Observer function for a new thread attach. We need to insert
+ existing watchpoints on the new thread. */
+void
+ia64_linux_new_thread (ptid_t ptid)
+{
+ insert_watchpoints_for_new_thread (ptid, &ia64_linux_insert_one_watchpoint);
+}
+
+void
+_initialize_ia64_linux_nat (void)
+{
+ observer_attach_new_thread (ia64_linux_new_thread);
+}
+
Index: s390-nat.c
===================================================================
RCS file: /cvs/src/src/gdb/s390-nat.c,v
retrieving revision 1.12
diff -u -p -r1.12 s390-nat.c
--- s390-nat.c 18 Feb 2004 04:17:35 -0000 1.12
+++ s390-nat.c 27 Oct 2004 21:43:50 -0000
@@ -112,18 +112,25 @@ fill_fpregset (fpregset_t *regp, int reg
((char *)regp) + regmap_fpregset[i]);
}

-/* Find the TID for the current inferior thread to use with ptrace. */
+/* Find the TID for use with ptrace. */
static int
-s390_inferior_tid (void)
+s390_tid (ptid_t ptid)
{
/* GNU/Linux LWP ID's are process ID's. */
- int tid = TIDGET (inferior_ptid);
+ int tid = TIDGET (ptid);
if (tid == 0)
- tid = PIDGET (inferior_ptid); /* Not a threaded program. */
+ tid = PIDGET (ptid); /* Not a threaded program. */

return tid;
}

+/* Find the TID for the current inferior thread to use with ptrace. */
+static int
+s390_inferior_tid (void)
+{
+ return s390_tid (inferior_ptid);
+}
+
/* Fetch all general-purpose registers from process/thread TID and
store their values in GDB's register cache. */
static void
@@ -269,9 +276,9 @@ s390_stopped_by_watchpoint (void)
}

static void
-s390_fix_watch_points (void)
+s390_fix_watch_points (ptid_t ptid)
{
- int tid = s390_inferior_tid ();
+ int tid = s390_tid (ptid);

per_struct per_info;
ptrace_area parea;
@@ -308,8 +315,9 @@ s390_fix_watch_points (void)
perror_with_name ("Couldn't modify watchpoint status");
}

-int
-s390_insert_watchpoint (CORE_ADDR addr, int len)
+/* Insert a specified watchpoint on a specified thread. */
+static int
+s390_insert_one_watchpoint (ptid_t ptid, CORE_ADDR addr, int len, int type)
{
struct watch_area *area = xmalloc (sizeof (struct watch_area));
if (!area)
@@ -321,12 +329,36 @@ s390_insert_watchpoint (CORE_ADDR addr,
area->next = watch_base;
watch_base = area;

- s390_fix_watch_points ();
+ s390_fix_watch_points (ptid);
return 0;
}

+/* Callback routine to use with iterate_over_lwps to insert a specified
+ watchpoint from all threads. */
+static int
+s390_insert_watchpoint_callback (struct lwp_info *lwp, void *data)
+{
+ struct target_watchpoint *args = (struct target_watchpoint *)data;
+
+ return s390_insert_one_watchpoint (lwp->ptid, args->addr, args->len, 0);
+}
+
+/* Insert a specified watchpoint on all threads. */
int
-s390_remove_watchpoint (CORE_ADDR addr, int len)
+s390_insert_watchpoint (CORE_ADDR addr, int len)
+{
+ struct target_watchpoint args;
+
+ args.addr = addr;
+ args.len = len;
+ /* For the S390, a watchpoint must be inserted/removed for each
+ thread so we iterate over the list of existing lwps. */
+ return iterate_over_lwps (&s390_insert_watchpoint_callback, &args);
+}
+
+/* Remove a specified watchpoint from a specified thread. */
+static int
+s390_remove_one_watchpoint (ptid_t ptid, CORE_ADDR addr, int len)
{
struct watch_area *area, **parea;

@@ -346,10 +378,32 @@ s390_remove_watchpoint (CORE_ADDR addr,
*parea = area->next;
xfree (area);

- s390_fix_watch_points ();
+ s390_fix_watch_points (ptid);
return 0;
}

+/* Callback routine to use with iterate_over_lwps to remove a specified
+ watchpoint from all threads. */
+static int
+s390_remove_watchpoint_callback (struct lwp_info *lwp, void *data)
+{
+ struct s390_watchpoint_args *args = (struct s390_watchpoint_args *)data;
+
+ return s390_remove_one_watchpoint (lwp->ptid, args->addr, args->len);
+}
+
+/* Remove a specified watchpoint from all threads. */
+int
+s390_remove_watchpoint (CORE_ADDR addr, int len)
+{
+ struct s390_watchpoint_args args;
+
+ args.addr = addr;
+ args.len = len;
+ /* For the S390, a watchpoint must be inserted/removed for each
+ thread so we iterate over the list of existing lwps. */
+ return iterate_over_lwps (&s390_remove_watchpoint_callback, &args);
+}

int
kernel_u_size (void)
@@ -357,3 +411,18 @@ kernel_u_size (void)
return sizeof (struct user);
}

+/* New thread observer that inserts all existing watchpoints on the
+ new thread. */
+static int
+s390_new_thread (ptid_t ptid)
+{
+ return insert_watchpoints_for_new_thread (ptid, &s390_insert_one_watchpoint);
+}
+
+void
+_initialize_s390_nat (void)
+{
+ observer_attach_new_thread (s390_new_thread);
+}
+
+
Index: target.h
===================================================================
RCS file: /cvs/src/src/gdb/target.h,v
retrieving revision 1.65
diff -u -p -r1.65 target.h
--- target.h 8 Oct 2004 20:29:55 -0000 1.65
+++ target.h 27 Oct 2004 21:43:51 -0000
@@ -178,6 +178,15 @@ extern char *target_signal_to_name (enum
/* Given a name (SIGHUP, etc.), return its signal. */
enum target_signal target_signal_from_name (char *);
\f
+
+/* Watchpoint specification. */
+struct target_watchpoint
+ {
+ CORE_ADDR addr;
+ int len;
+ int type;
+ };
+
/* Request the transfer of up to LEN 8-bit bytes of the target's
OBJECT. The OFFSET, for a seekable object, specifies the starting
point. The ANNEX can be used to provide additional data-specific
Index: thread-db.c
===================================================================
RCS file: /cvs/src/src/gdb/thread-db.c,v
retrieving revision 1.46
diff -u -p -r1.46 thread-db.c
--- thread-db.c 8 Oct 2004 20:29:56 -0000 1.46
+++ thread-db.c 27 Oct 2004 21:43:51 -0000
@@ -34,6 +34,7 @@
#include "target.h"
#include "regcache.h"
#include "solib-svr4.h"
+#include "observer.h"

#ifdef HAVE_GNU_LIBC_VERSION_H
#include <gnu/libc-version.h>
@@ -722,6 +723,7 @@ attach_thread (ptid_t ptid, const td_thr
{
struct thread_info *tp;
td_err_e err;
+ ptid_t new_ptid;

/* If we're being called after a TD_CREATE event, we may already
know about this thread. There are two ways this can happen. We
@@ -757,11 +759,16 @@ attach_thread (ptid_t ptid, const td_thr
if (ti_p->ti_state == TD_THR_UNKNOWN || ti_p->ti_state == TD_THR_ZOMBIE)
return; /* A zombie thread -- do not attach. */

+ new_ptid = BUILD_LWP (ti_p->ti_lid, GET_PID (ptid));
+
/* Under GNU/Linux, we have to attach to each and every thread. */
#ifdef ATTACH_LWP
- ATTACH_LWP (BUILD_LWP (ti_p->ti_lid, GET_PID (ptid)), 0);
+ ATTACH_LWP (new_ptid, 0);
#endif

+ /* Inform any observers of new attached thread. */
+ observer_notify_new_thread (new_ptid);
+
/* Enable thread event reporting for this thread. */
err = td_thr_event_enable_p (th_p, 1);
if (err != TD_OK)
Index: doc/observer.texi
===================================================================
RCS file: /cvs/src/src/gdb/doc/observer.texi,v
retrieving revision 1.8
diff -u -p -r1.8 observer.texi
--- doc/observer.texi 1 Sep 2004 17:59:37 -0000 1.8
+++ doc/observer.texi 27 Oct 2004 21:43:51 -0000
@@ -95,3 +95,7 @@ inferior, and before any information on
The specified shared library has been discovered to be unloaded.
@end deftypefun

+@deftypefun void new_thread (ptid_t @var{ptid})
+A new thread has been attached to.
+@end deftypefun
+