[RFA]: Modified Watchthreads Patch

Jeff Johnston
2004-12-09T23:36:25Z

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

The following is a modified version of my thread watchpoint patch from
October/November. It removes the code I had used to switch between lwp ptids
and thread ptids now that Daniel's lwp patch is in place. It uses the former
version of my observer that is linux-specific and is activated in attach_thread
in linux-thread-db.c. Eli, I renamed the observer as asked to indicate this.

I also addressed Ulrich's comments regarding simplifying the S390 code and using
the s390_fix_watch_points call to actually put the watchpoints on the new thread.

Ulrich/Daniel can you take a look to verify everything is in place. Daniel, I
realize that this touches files that are currently in patch state for you. I
have no problem waiting for your latest patch to apply and retrofitting my
changes at check-in if necessary.

As I mentioned before, more is required to get ia64 threaded watchpoints to
work. For S390, this change allows it to set and recognize threaded watchpoints.

-- Jeff J.

2004-12-09 Jeff Johnston <jjohnstn@redhat.com>

* breakpoint.c (insert_watchpoints_for_new_thread): New function.
(print_it_typical): Do not issue an error for bp_thread_event
if a subsequent event is on the chain.
* 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_stopped_data_address): Call target_get_lwp to ensure
the ptid has its lwp field filled in.
(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 linux thread observer.
* linux-nat.h (struct linux_watchpoint): New structure.
* linux-thread-db.c (attach_thread): Notify all linux new thread
observers.
* s390-nat.c (s390_tid): 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_watchpoint_callback): New function.
(s390_insert_watchpoint): Change to iterate through lwps and
insert the specified watchpoint on each thread.
(s390_linux_new_thread): New linux thread observer.
(_initialize_s390_nat): New function. Initialize
new linux thread observer.
* Makefile.in (s390-nat.o, ia64-linux-nat.o): Add new header file
dependencies.

doc/ChangeLog

2004-12-09 Jeff Johnston <jjohnstn@redhat.com>

* observer.texi (linux_new_thread): New observer.

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

Index: Makefile.in
===================================================================
RCS file: /cvs/src/src/gdb/Makefile.in,v
retrieving revision 1.676
diff -u -p -r1.676 Makefile.in
--- Makefile.in 7 Dec 2004 22:17:59 -0000 1.676
+++ Makefile.in 9 Dec 2004 23:18:41 -0000
@@ -2059,7 +2059,8 @@ ia64-aix-nat.o: ia64-aix-nat.c $(defs_h)
$(objfiles_h) $(gdb_stat_h)
ia64-aix-tdep.o: ia64-aix-tdep.c $(defs_h)
ia64-linux-nat.o: ia64-linux-nat.c $(defs_h) $(gdb_string_h) $(inferior_h) \
- $(target_h) $(gdbcore_h) $(regcache_h) $(gdb_wait_h) $(gregset_h)
+ $(target_h) $(gdbcore_h) $(regcache_h) $(gdb_wait_h) $(gregset_h) \
+ $(linux_nat_h) $(observer_h)
ia64-linux-tdep.o: ia64-linux-tdep.c $(defs_h) $(ia64_tdep_h) \
$(arch_utils_h) $(gdbcore_h) $(regcache_h)
ia64-tdep.o: ia64-tdep.c $(defs_h) $(inferior_h) $(gdbcore_h) \
@@ -2440,7 +2441,7 @@ rs6000-tdep.o: rs6000-tdep.c $(defs_h) $
$(ppc_tdep_h) $(gdb_assert_h) $(dis_asm_h) $(trad_frame_h) \
$(frame_unwind_h) $(frame_base_h)
s390-nat.o: s390-nat.c $(defs_h) $(tm_h) $(regcache_h) $(inferior_h) \
- $(s390_tdep_h)
+ $(s390_tdep_h) $(observer_h) $(linux_nat_h)
s390-tdep.o: s390-tdep.c $(defs_h) $(arch_utils_h) $(frame_h) $(inferior_h) \
$(symtab_h) $(target_h) $(gdbcore_h) $(gdbcmd_h) $(objfiles_h) \
$(tm_h) $(__bfd_bfd_h) $(floatformat_h) $(regcache_h) \
Index: breakpoint.c
===================================================================
RCS file: /cvs/src/src/gdb/breakpoint.c,v
retrieving revision 1.186
diff -u -p -r1.186 breakpoint.c
--- breakpoint.c 1 Dec 2004 06:54:56 -0000 1.186
+++ breakpoint.c 9 Dec 2004 23:18:42 -0000
@@ -738,6 +738,90 @@ 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 where 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)
+ {
+ 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)
@@ -2112,8 +2196,13 @@ print_it_typical (bpstat bs)
break;

case bp_thread_event:
- /* Not sure how we will get here.
- GDB should not stop for these breakpoints. */
+ /* We can only get here legitimately if something further on the bs
+ list has caused the stop status to be noisy. A valid example
+ of this is a new thread event and a software watchpoint have
+ both occurred. */
+ if (bs->next)
+ return PRINT_UNKNOWN;
+
printf_filtered ("Thread Event Breakpoint: gdb should not stop!\n");
return PRINT_NOTHING;
break;
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 9 Dec 2004 23:18:42 -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 9 Dec 2004 23:18:42 -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 linux_watchpoint *args = (struct linux_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 linux_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 linux_watchpoint *args = (struct linux_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 linux_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,19 @@ 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. */
+static 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_linux_new_thread (ia64_linux_new_thread);
+}
+
Index: linux-nat.h
===================================================================
RCS file: /cvs/src/src/gdb/linux-nat.h,v
retrieving revision 1.6
diff -u -p -r1.6 linux-nat.h
--- linux-nat.h 29 Mar 2004 18:07:14 -0000 1.6
+++ linux-nat.h 9 Dec 2004 23:18:42 -0000
@@ -63,6 +63,14 @@ struct lwp_info
struct lwp_info *next;
};

+/* Watchpoint description. */
+struct linux_watchpoint
+{
+ CORE_ADDR addr;
+ int len;
+ int type;
+};
+
/* Read/write to target memory via the Linux kernel's "proc file
system". */
struct mem_attrib;
Index: linux-thread-db.c
===================================================================
RCS file: /cvs/src/src/gdb/linux-thread-db.c,v
retrieving revision 1.2
diff -u -p -r1.2 linux-thread-db.c
--- linux-thread-db.c 8 Dec 2004 15:10:30 -0000 1.2
+++ linux-thread-db.c 9 Dec 2004 23:18:42 -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>
@@ -713,6 +714,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
@@ -748,11 +750,18 @@ 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

+ /* Notify any observers of a new linux thread. This
+ would include any linux platforms that have to insert hardware
+ watchpoints on every thread. */
+ observer_notify_linux_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: 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 9 Dec 2004 23:18:42 -0000
@@ -27,6 +27,8 @@
#include "inferior.h"

#include "s390-tdep.h"
+#include "linux-nat.h"
+#include "observer.h"

#include <asm/ptrace.h>
#include <sys/ptrace.h>
@@ -112,14 +114,14 @@ 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;
}
@@ -203,7 +205,7 @@ store_fpregs (int tid, int regnum)
void
fetch_inferior_registers (int regnum)
{
- int tid = s390_inferior_tid ();
+ int tid = s390_tid (inferior_ptid);

if (regnum == -1
|| (regnum < S390_NUM_REGS && regmap_gregset[regnum] != -1))
@@ -219,7 +221,7 @@ fetch_inferior_registers (int regnum)
void
store_inferior_registers (int regnum)
{
- int tid = s390_inferior_tid ();
+ int tid = s390_tid (inferior_ptid);

if (regnum == -1
|| (regnum < S390_NUM_REGS && regmap_gregset[regnum] != -1))
@@ -261,7 +263,7 @@ s390_stopped_by_watchpoint (void)
parea.len = sizeof (per_lowcore);
parea.process_addr = (addr_t) & per_lowcore;
parea.kernel_addr = offsetof (struct user_regs_struct, per_info.lowcore);
- if (ptrace (PTRACE_PEEKUSR_AREA, s390_inferior_tid (), &parea) < 0)
+ if (ptrace (PTRACE_PEEKUSR_AREA, s390_tid (inferior_ptid), &parea) < 0)
perror_with_name ("Couldn't retrieve watchpoint status");

return per_lowcore.perc_storage_alteration == 1
@@ -269,9 +271,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,6 +310,16 @@ s390_fix_watch_points (void)
perror_with_name ("Couldn't modify watchpoint status");
}

+/* Callback routine to use with iterate_over_lwps to insert a specified
+ watchpoint on all threads. */
+static int
+s390_insert_watchpoint_callback (struct lwp_info *lwp, void *data)
+{
+ s390_fix_watch_points (lwp->ptid);
+ return 0;
+}
+
+/* Insert a specified watchpoint on all threads. */
int
s390_insert_watchpoint (CORE_ADDR addr, int len)
{
@@ -321,10 +333,24 @@ s390_insert_watchpoint (CORE_ADDR addr,
area->next = watch_base;
watch_base = area;

- s390_fix_watch_points ();
+ /* For the S390, a watchpoint must be inserted/removed for each
+ thread so we iterate over the list of existing lwps. */
+ if (iterate_over_lwps (&s390_insert_watchpoint_callback, NULL))
+ return -1;
+
+ 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)
+{
+ s390_fix_watch_points (lwp->ptid);
return 0;
}

+/* Remove a specified watchpoint from all threads. */
int
s390_remove_watchpoint (CORE_ADDR addr, int len)
{
@@ -346,7 +372,11 @@ s390_remove_watchpoint (CORE_ADDR addr,
*parea = area->next;
xfree (area);

- s390_fix_watch_points ();
+ /* For the S390, a watchpoint must be inserted/removed for each
+ thread so we iterate over the list of existing lwps. */
+ if (iterate_over_lwps (&s390_remove_watchpoint_callback, NULL))
+ return -1;
+
return 0;
}

@@ -357,3 +387,19 @@ kernel_u_size (void)
return sizeof (struct user);
}

+/* New thread observer that inserts all existing watchpoints on the
+ new thread. */
+static void
+s390_linux_new_thread (ptid_t ptid)
+{
+ /* Add existing watchpoints to new thread. */
+ s390_fix_watch_points (ptid);
+}
+
+void
+_initialize_s390_nat (void)
+{
+ observer_attach_linux_new_thread (s390_linux_new_thread);
+}
+
+

[-- Attachment #3: watchthreads3bdoc.patch --]
[-- Type: text/plain, Size: 586 bytes --]

Index: observer.texi
===================================================================
RCS file: /cvs/src/src/gdb/doc/observer.texi,v
retrieving revision 1.8
diff -u -p -r1.8 observer.texi
--- observer.texi 1 Sep 2004 17:59:37 -0000 1.8
+++ observer.texi 9 Dec 2004 23:31:58 -0000
@@ -95,3 +95,8 @@ inferior, and before any information on
The specified shared library has been discovered to be unloaded.
@end deftypefun

+@deftypefun void linux_new_thread (ptid_t @var{ptid})
+A new linux thread described by @var{ptid} has been officially attached
+to by gdb.
+@end deftypefun
+