Date: Sat, 11 Jun 2011 04:43:26 GMT From: KUROSAWA Takahiro <takahiro.kurosawa@gmail.com> To: freebsd-gnats-submit@FreeBSD.org Subject: threads/157755: gdb hardware watchpoints do not work correctly in multi-threaded programs Message-ID: <201106110443.p5B4hQ9M011452@red.freebsd.org> Resent-Message-ID: <201106110450.p5B4o9UF039806@freefall.freebsd.org>
next in thread | raw e-mail | index | archive | help
>Number: 157755 >Category: threads >Synopsis: gdb hardware watchpoints do not work correctly in multi-threaded programs >Confidential: no >Severity: non-critical >Priority: low >Responsible: freebsd-threads >State: open >Quarter: >Keywords: >Date-Required: >Class: sw-bug >Submitter-Id: current-users >Arrival-Date: Sat Jun 11 04:50:09 UTC 2011 >Closed-Date: >Last-Modified: >Originator: KUROSAWA Takahiro >Release: 9-CURRENT >Organization: >Environment: FreeBSD amdk6 9.0-CURRENT FreeBSD 9.0-CURRENT #4: Fri Jun 3 19:18:20 JST 2011 kurosawa@amdk6:/usr/obj/.world/src/sys/AMDK7 i386 >Description: Hardware watchpoints are implemented on gdb in FreeBSD/i386 and FreeBSD/amd64 by setting debug registers with ptrace(PT_SETDBREGS). But watchpoints are effective only for one thread in the multi-threaded program because debug registers are only set per process. >How-To-Repeat: We can reproduce the problem by the following procedure: % cat watch.c #include <pthread.h> #include <stdio.h> #include <string.h> #include <err.h> static int watchpoint; void * thr_main(void *arg) { watchpoint = 2; return NULL; } int main(void) { pthread_t pt; int ret = 0; if ((ret = pthread_create(&pt, NULL, thr_main, NULL)) != 0) errx(1, "pthread_create: %s.\n", strerror(ret)); ret = 2; pthread_join(pt, NULL); watchpoint = 1; ret = 1; return 0; } % cc -pthread -g watch.c % gdb a.out GNU gdb 6.1.1 [FreeBSD] Copyright 2004 Free Software Foundation, Inc. .... # "break thr_main" sets a breakpoint at "watchpoint = 2" (gdb) x/i thr_main 0x8048540 <thr_main>: push %ebp (gdb) break *0x8048540 Breakpoint 1 at 0x8048540: file watch.c, line 10. (gdb) run Starting program: /home/kurosawa/a.out [New LWP 100260] [New Thread 28404300 (LWP 100260/initial thread)] [New Thread 28404900 (LWP 100693/a.out)] [Switching to Thread 28404900 (LWP 100693/a.out)] Breakpoint 1, thr_main (arg=0x0) at watch.c:10 10 { # set an watchpoint to the "watchpoint" variable (gdb) watch watchpoint Hardware watchpoint 2: watchpoint # watchpoint hit in thr_main (gdb) c Continuing. Hardware watchpoint 2: watchpoint Old value = 0 New value = 2 thr_main (arg=0x0) at watch.c:13 13 return NULL; (gdb) c Continuing. [Thread 28404900 (LWP 100693/a.out) exited] [New Thread 28404900 (LWP 100693/a.out)] Program exited normally. # watchpoint must be hit also in main, but program runs without stopping. >Fix: The attached patch fixes the problem. ---- (gdb) x/i thr_main 0x8048540 <thr_main>: push %ebp (gdb) break *0x8048540 Breakpoint 1 at 0x8048540: file watch.c, line 10. (gdb) run Starting program: /home/kurosawa/a.out [New LWP 100163] [New Thread 28404300 (LWP 100163/initial thread)] [New Thread 28404900 (LWP 100694/a.out)] [Switching to Thread 28404900 (LWP 100694/a.out)] Breakpoint 1, thr_main (arg=0x0) at watch.c:10 10 { (gdb) watch watchpoint Hardware watchpoint 2: watchpoint (gdb) c Continuing. Hardware watchpoint 2: watchpoint Old value = 0 New value = 2 thr_main (arg=0x0) at watch.c:13 13 return NULL; (gdb) c Continuing. [Thread 28404900 (LWP 100694/a.out) exited] [New Thread 28404900 (LWP 100694/a.out)] [Switching to Thread 28404300 (LWP 100163/initial thread)] Hardware watchpoint 2: watchpoint Old value = 2 New value = 1 main () at watch.c:28 28 ret = 1; (gdb) c Continuing. Program exited normally. ---- The patch only fixes FreeBSD/i386 but the same approach could be used for FreeBSD/amd64. Patch attached with submission follows: # HG changeset patch # Parent d95f240e861430967a2cd1ee93e8c1b0d5a3b6ef diff -r d95f240e8614 gnu/usr.bin/gdb/arch/i386/Makefile --- a/gnu/usr.bin/gdb/arch/i386/Makefile Fri Jun 03 15:58:48 2011 +0900 +++ b/gnu/usr.bin/gdb/arch/i386/Makefile Sat Jun 11 12:50:52 2011 +0900 @@ -9,7 +9,26 @@ LIBSRCS+= solib.c solib-svr4.c LIBSRCS+= i386-tdep.c i386bsd-tdep.c i386fbsd-tdep-fixed.c i387-tdep.c nm.h: - echo '#include "i386/nm-fbsd.h"' > ${.TARGET} + echo '#ifndef GENSRCS_NM_H' > ${.TARGET} + echo '#define GENSRCS_NM_H' >> ${.TARGET} + echo '#include "i386/nm-fbsd.h"' >> ${.TARGET} + echo '#undef I386_DR_LOW_SET_CONTROL' >> ${.TARGET} + echo '#undef I386_DR_LOW_SET_ADDR' >> ${.TARGET} + echo '#undef I386_DR_LOW_RESET_ADDR' >> ${.TARGET} + echo '#undef I386_DR_LOW_GET_STATUS' >> ${.TARGET} + echo '#define I386_DR_LOW_SET_CONTROL(c) \' >> ${.TARGET} + echo ' fbsd_thread_dr_set_control (c)' >> ${.TARGET} + echo 'void i386bsd_dr_set_control (unsigned long);' >> ${.TARGET} + echo '#define I386_DR_LOW_SET_ADDR(r, v) \' >> ${.TARGET} + echo ' fbsd_thread_dr_set_addr (r, v)' >> ${.TARGET} + echo 'void fbsd_thread_dr_set_addr (int, CORE_ADDR);' >> ${.TARGET} + echo '#define I386_DR_LOW_RESET_ADDR(r) \' >> ${.TARGET} + echo ' fbsd_thread_dr_reset_addr (r)' >> ${.TARGET} + echo 'void fbsd_thread_dr_reset_addr (int);' >> ${.TARGET} + echo '#define I386_DR_LOW_GET_STATUS() \' >> ${.TARGET} + echo ' fbsd_thread_dr_get_status ()' >> ${.TARGET} + echo 'unsigned long fbsd_thread_dr_get_status (void);' >> ${.TARGET} + echo '#endif /* GENSRCS_NM_H */' >> ${.TARGET} tm.h: echo '#include "i386/tm-fbsd.h"' > ${.TARGET} diff -r d95f240e8614 gnu/usr.bin/gdb/libgdb/fbsd-threads.c --- a/gnu/usr.bin/gdb/libgdb/fbsd-threads.c Fri Jun 03 15:58:48 2011 +0900 +++ b/gnu/usr.bin/gdb/libgdb/fbsd-threads.c Sat Jun 11 12:50:52 2011 +0900 @@ -1797,3 +1797,183 @@ ps_linfo(struct ps_prochandle *ph, lwpid return PS_ERR; return PS_OK; } + + +#if defined(HAVE_PT_GETDBREGS) && defined(I386_DR_LOW_SET_ADDR) +/* Hardware watchpoint support based on i386bsd-nat.c: */ +/* Native-dependent code for modern i386 BSD's. + Copyright 2000, 2001, 2002, 2003 Free Software Foundation, Inc. + + This file is part of GDB. + + This program is free software; you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation; either version 2 of the License, or + (at your option) any later version. + + This program is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with this program; if not, write to the Free Software + Foundation, Inc., 59 Temple Place - Suite 330, + Boston, MA 02111-1307, USA. */ + +/* + * We need to add threads support because debug registers need to be + * set per LWP to implement hardware watchpoints. + * + * We do not need to care user-level threads contexts for debug registers + * since they are not in mcontext. + */ + +/* Not all versions of FreeBSD/i386 that support the debug registers + have this macro. */ +#ifndef DBREG_DRX +#define DBREG_DRX(d, x) ((&d->dr0)[x]) +#endif + +static void +fbsd_lwp_dr_set (lwpid_t lwpid, int regnum, unsigned long value) +{ + struct dbreg dbregs; + + if (ptrace (PT_GETDBREGS, lwpid, + (PTRACE_ARG3_TYPE) &dbregs, 0) == -1) + perror_with_name ("Couldn't get debug registers"); + + /* For some mysterious reason, some of the reserved bits in the + debug control register get set. Mask these off, otherwise the + ptrace call below will fail. */ + DBREG_DRX ((&dbregs), 7) &= ~(0x0000fc00); + + DBREG_DRX ((&dbregs), regnum) = value; + + if (ptrace (PT_SETDBREGS, lwpid, + (PTRACE_ARG3_TYPE) &dbregs, 0) == -1) + perror_with_name ("Couldn't write debug registers"); +} + +struct dr_set_thr_arg { + int regnum; + unsigned long value; +}; + +static int +fbsd_thread_dr_set_callback (const td_thrhandle_t *th_p, void *data) +{ + struct dr_set_thr_arg *arg = data; + td_thrinfo_t ti; + td_err_e err; + + err = td_thr_get_info_p (th_p, &ti); + if (err != TD_OK) + error ("Cannot get thread info: %s", thread_db_err_str (err)); + + /* Ignore zombie */ + if (ti.ti_state == TD_THR_UNKNOWN || ti.ti_state == TD_THR_ZOMBIE) + return 0; + + fbsd_lwp_dr_set (ti.ti_lid, arg->regnum, arg->value); + return 0; +} + +static void +fbsd_thread_dr_set (int regnum, unsigned long value) +{ + struct dr_set_thr_arg arg; + td_err_e err; + + arg.regnum = regnum; + arg.value = value; + + /* Iterating over LWPs is sufficient actually. */ + err = td_ta_thr_iter_p (thread_agent, fbsd_thread_dr_set_callback, &arg, + TD_THR_ANY_STATE, TD_THR_LOWEST_PRIORITY, + TD_SIGNO_MASK, TD_THR_ANY_USER_FLAGS); + if (err != TD_OK) + error ("Cannot set debug registers: %s", thread_db_err_str (err)); +} + +void +fbsd_thread_dr_set_control (unsigned long control) +{ + if (fbsd_thread_active) + fbsd_thread_dr_set (7, control); + else + fbsd_lwp_dr_set (GET_PID (inferior_ptid), 7, control); +} + +void +fbsd_thread_dr_set_addr (int regnum, CORE_ADDR addr) +{ + gdb_assert (regnum >= 0 && regnum <= 4); + + if (fbsd_thread_active) + fbsd_thread_dr_set (regnum, addr); + else + fbsd_lwp_dr_set (GET_PID (inferior_ptid), regnum, addr); +} + +void +fbsd_thread_dr_reset_addr (int regnum) +{ + gdb_assert (regnum >= 0 && regnum <= 4); + + if (fbsd_thread_active) + fbsd_thread_dr_set (regnum, 0); + else + fbsd_lwp_dr_set (GET_PID (inferior_ptid), regnum, 0); +} + +unsigned long +fbsd_thread_dr_get_status (void) +{ + struct dbreg dbregs; + td_thrhandle_t th; + td_thrinfo_t ti; + td_err_e err; + lwpid_t lwpid; + + if (IS_THREAD (inferior_ptid)) + { + err = td_ta_map_id2thr_p (thread_agent, GET_THREAD (inferior_ptid), &th); + if (err != TD_OK) + error ("Cannot find thread %d: Thread ID=%ld, %s", + pid_to_thread_id (inferior_ptid), + GET_THREAD (inferior_ptid), thread_db_err_str (err)); + + err = td_thr_get_info_p (&th, &ti); + if (err != TD_OK) + error ("Cannot get thread info: %s", thread_db_err_str (err)); + + lwpid = ti.ti_lid; + } + else if (IS_LWP (inferior_ptid)) + { + lwpid = GET_LWP (inferior_ptid); + } + else + { + lwpid = GET_PID (inferior_ptid); + } + + /* FIXME: kettenis/2001-03-31: Calling perror_with_name if the + ptrace call fails breaks debugging remote targets. The correct + way to fix this is to add the hardware breakpoint and watchpoint + stuff to the target vector. For now, just return zero if the + ptrace call fails. */ + if (ptrace (PT_GETDBREGS, GET_PID (inferior_ptid), + (PTRACE_ARG3_TYPE) & dbregs, 0) == -1) +#if 0 + perror_with_name ("Couldn't read debug registers"); +#else + return 0; +#endif + + return DBREG_DRX ((&dbregs), 6); +} + +#endif /* PT_GETDBREGS && I386_DR_LOW_SET_ADDR */ >Release-Note: >Audit-Trail: >Unformatted:
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201106110443.p5B4hQ9M011452>