[RFC]: x86 threaded watchpoint support [1/3]

Jeff Johnston
2004-06-11T21:14:32Z

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

The following patch gets threaded watchpoint support working for the x86. On
x86 linux, the dr_status register is thread-specific. This means that the
current method which uses the PID to call PTRACE is wrong. I have changed this
to use the current lwp for the inferior_ptid. Corresponding to this, the
i386_stopped_data_address function switches the inferior_ptid to the trap_ptid.
Thus, we can see if we really stopped for a watchpoint or hardware breakpoint.

Because the thread-db.c code changes the trap_ptid into a high-level ptid (pid +
tid), I had to add a new target vector interface which gives back the lwp for a
given ptid. This is used by the low level dr get routine.

With this change plus a change to breakpoint.c, threaded watchpoints work on the
x86. Part 2 will be the breakpoint.c change which is not x86-specific. Part 3
will be a test case when I figure out how to code it properly. If you want to
test it with the two patches applied, just debug gdb.threads/schedlock and issue:

b main
run
watch args[0]
watch args[1]
continue
continue
continue
etc...

Ok?

-- Jeff J.

2004-06-11 Jeff Johnston <jjohnstn@redhat.com>

* i386-linux-nat.c (i386_linux_dr_get): Use target_get_lwp
function to get lwp of current ptid to use in the PTRACE
call.
* i386-nat.c: Add comments regarding the DR status register.
(i386_stopped_data_address): Switch to the trap_ptid before
getting the dr_status register.
(i386_stopped_by_hwbp): Ditto.
* lin-lwp.c (child_wait): Set the trap_ptid when appropriate.
* target.h (struct target ops): Add new function to_get_lwp.
(target_get_lwp): New macro.
* target.c (INHERIT): Inherit to_get_lwp.
(update_current_target): Default to_get_lwp to ptid_get_lwp.
(init_dummy_target): Ditto.
* thread-db.c (thread_db_get_lwp): New function.
(thread_db_init): Expose thread_db_get_lwp in target vector.

[-- Attachment #2: x86-watchpoint.patch1 --]
[-- Type: text/plain, Size: 8214 bytes --]

Index: i386-linux-nat.c
===================================================================
RCS file: /cvs/src/src/gdb/i386-linux-nat.c,v
retrieving revision 1.55
diff -u -p -r1.55 i386-linux-nat.c
--- i386-linux-nat.c 9 Apr 2004 16:31:01 -0000 1.55
+++ i386-linux-nat.c 11 Jun 2004 20:55:28 -0000
@@ -649,10 +649,11 @@ i386_linux_dr_get (int regnum)
int tid;
unsigned long value;

- /* FIXME: kettenis/2001-01-29: It's not clear what we should do with
- multi-threaded processes here. For now, pretend there is just
- one thread. */
- tid = PIDGET (inferior_ptid);
+ /* The debug status register is thread-specific and so we should
+ use the current lwp when fetching the debug registers. */
+ tid = target_get_lwp (inferior_ptid);
+ if (tid == 0)
+ tid = PIDGET (inferior_ptid); /* Not a threaded program. */

/* FIXME: kettenis/2001-03-27: Calling perror_with_name if the
ptrace call fails breaks debugging remote targets. The correct
Index: i386-nat.c
===================================================================
RCS file: /cvs/src/src/gdb/i386-nat.c,v
retrieving revision 1.8
diff -u -p -r1.8 i386-nat.c
--- i386-nat.c 5 Mar 2004 20:58:00 -0000 1.8
+++ i386-nat.c 11 Jun 2004 20:55:28 -0000
@@ -166,7 +166,10 @@
#define ALL_DEBUG_REGISTERS(i) for (i = 0; i < DR_NADDR; i++)

/* Mirror the inferior's DRi registers. We keep the status and
- control registers separated because they don't hold addresses. */
+ control registers separated because they don't hold addresses.
+
+ Note that the DR_STATUS register is thread-specific and the
+ mirror value should be refreshed as necessary. */
static CORE_ADDR dr_mirror[DR_NADDR];
static unsigned dr_status_mirror, dr_control_mirror;

@@ -567,13 +570,22 @@ i386_region_ok_for_watchpoint (CORE_ADDR
/* If the inferior has some watchpoint that triggered, return the
address associated with that watchpoint. Otherwise, return zero. */

+extern ptid_t trap_ptid;
+extern ptid_t inferior_ptid;
+
CORE_ADDR
i386_stopped_data_address (void)
{
CORE_ADDR addr = 0;
int i;
+ ptid_t saved_ptid = inferior_ptid;

+ /* Debug status register is thread-specific. A watchpoint will
+ cause a trap to occur. Switch to trap ptid to get relevant
+ status for that thread. */
+ inferior_ptid = trap_ptid;
dr_status_mirror = I386_DR_LOW_GET_STATUS ();
+ inferior_ptid = saved_ptid;

ALL_DEBUG_REGISTERS(i)
{
@@ -603,8 +615,16 @@ int
i386_stopped_by_hwbp (void)
{
int i;
+ ptid_t saved_ptid = inferior_ptid;
+
+ /* Debug status register is thread-specific. A hardware breakpoint
+ will cause a trap to occur. Switch to trap ptid to get relevant
+ status for that thread. */

+ inferior_ptid = trap_ptid;
dr_status_mirror = I386_DR_LOW_GET_STATUS ();
+ inferior_ptid = saved_ptid;
+
if (maint_show_dr)
i386_show_dr ("stopped_by_hwbp", 0, 0, hw_execute);

Index: lin-lwp.c
===================================================================
RCS file: /cvs/src/src/gdb/lin-lwp.c,v
retrieving revision 1.55
diff -u -p -r1.55 lin-lwp.c
--- lin-lwp.c 25 May 2004 14:58:28 -0000 1.55
+++ lin-lwp.c 11 Jun 2004 20:55:28 -0000
@@ -1200,21 +1200,29 @@ child_wait (ptid_t ptid, struct target_w
}

/* Handle GNU/Linux's extended waitstatus for trace events. */
- if (pid != -1 && WIFSTOPPED (status) && WSTOPSIG (status) == SIGTRAP
- && status >> 16 != 0)
+ if (pid != -1 && WIFSTOPPED (status) && WSTOPSIG (status) == SIGTRAP)
{
- linux_handle_extended_wait (pid, status, ourstatus);
+ /* Set trap_ptid like lin_lwp_wait does. This is needed
+ for watchpoint support. For example, the x86 linux
+ watchpoints need to know what thread an event occurred
+ on so as to read the correct thread-specific status. */
+ trap_ptid = pid_to_ptid (pid);

- /* If we see a clone event, detach the child, and don't
- report the event. It would be nice to offer some way to
- switch into a non-thread-db based threaded mode at this
- point. */
- if (ourstatus->kind == TARGET_WAITKIND_SPURIOUS)
+ if (status >> 16 != 0)
{
- ptrace (PTRACE_DETACH, ourstatus->value.related_pid, 0, 0);
- ourstatus->kind = TARGET_WAITKIND_IGNORE;
- pid = -1;
- save_errno = EINTR;
+ linux_handle_extended_wait (pid, status, ourstatus);
+
+ /* If we see a clone event, detach the child, and don't
+ report the event. It would be nice to offer some way to
+ switch into a non-thread-db based threaded mode at this
+ point. */
+ if (ourstatus->kind == TARGET_WAITKIND_SPURIOUS)
+ {
+ ptrace (PTRACE_DETACH, ourstatus->value.related_pid, 0, 0);
+ ourstatus->kind = TARGET_WAITKIND_IGNORE;
+ pid = -1;
+ save_errno = EINTR;
+ }
}
}

Index: target.c
===================================================================
RCS file: /cvs/src/src/gdb/target.c,v
retrieving revision 1.75
diff -u -p -r1.75 target.c
--- target.c 9 Jun 2004 20:09:42 -0000 1.75
+++ target.c 11 Jun 2004 20:55:28 -0000
@@ -422,6 +422,7 @@ update_current_target (void)
INHERIT (to_notice_signals, t);
INHERIT (to_thread_alive, t);
INHERIT (to_find_new_threads, t);
+ INHERIT (to_get_lwp, t);
INHERIT (to_pid_to_str, t);
INHERIT (to_extra_thread_info, t);
INHERIT (to_stop, t);
@@ -601,6 +602,9 @@ update_current_target (void)
de_fault (to_find_new_threads,
(void (*) (void))
target_ignore);
+ de_fault (to_get_lwp,
+ (long (*) (ptid_t))
+ ptid_get_lwp);
de_fault (to_extra_thread_info,
(char *(*) (struct thread_info *))
return_zero);
@@ -1616,6 +1620,7 @@ init_dummy_target (void)
dummy_target.to_find_memory_regions = dummy_find_memory_regions;
dummy_target.to_make_corefile_notes = dummy_make_corefile_notes;
dummy_target.to_xfer_partial = default_xfer_partial;
+ dummy_target.to_get_lwp = ptid_get_lwp;
dummy_target.to_magic = OPS_MAGIC;
}
\f
Index: target.h
===================================================================
RCS file: /cvs/src/src/gdb/target.h,v
retrieving revision 1.60
diff -u -p -r1.60 target.h
--- target.h 7 Jun 2004 17:58:32 -0000 1.60
+++ target.h 11 Jun 2004 20:55:29 -0000
@@ -370,6 +370,7 @@ struct target_ops
void (*to_notice_signals) (ptid_t ptid);
int (*to_thread_alive) (ptid_t ptid);
void (*to_find_new_threads) (void);
+ long (*to_get_lwp) (ptid_t ptid);
char *(*to_pid_to_str) (ptid_t);
char *(*to_extra_thread_info) (struct thread_info *);
void (*to_stop) (void);
@@ -815,6 +816,10 @@ extern void target_load (char *arg, int
#define target_find_new_threads() \
(*current_target.to_find_new_threads) (); \

+/* Get the lwp for a thread. */
+#define target_get_lwp(ptid) \
+ (*current_target.to_get_lwp) (ptid); \
+
/* Make target stop in a continuable fashion. (For instance, under
Unix, this should act like SIGSTOP). This function is normally
used by GUIs to implement a stop button. */
Index: thread-db.c
===================================================================
RCS file: /cvs/src/src/gdb/thread-db.c,v
retrieving revision 1.42
diff -u -p -r1.42 thread-db.c
--- thread-db.c 7 Jun 2004 22:35:55 -0000 1.42
+++ thread-db.c 11 Jun 2004 20:55:29 -0000
@@ -408,6 +408,22 @@ lwp_from_thread (ptid_t ptid)
}
\f

+static long
+thread_db_get_lwp (ptid_t ptid)
+{
+ if (!is_thread (ptid))
+ return ptid.pid;
+ else
+ {
+ struct thread_info *thread_info = find_thread_pid (ptid);
+ if (!thread_info)
+ return 0;
+
+ thread_db_get_info (thread_info);
+ return thread_info->private->ti.ti_lid;
+ }
+}
+
void
thread_db_init (struct target_ops *target)
{
@@ -1382,6 +1398,7 @@ init_thread_db_ops (void)
thread_db_ops.to_mourn_inferior = thread_db_mourn_inferior;
thread_db_ops.to_thread_alive = thread_db_thread_alive;
thread_db_ops.to_find_new_threads = thread_db_find_new_threads;
+ thread_db_ops.to_get_lwp = thread_db_get_lwp;
thread_db_ops.to_pid_to_str = thread_db_pid_to_str;
thread_db_ops.to_stratum = thread_stratum;
thread_db_ops.to_has_thread_control = tc_schedlock;