diff options
| -rw-r--r-- | gdb/ChangeLog | 15 | ||||
| -rw-r--r-- | gdb/linux-nat.c | 82 | ||||
| -rw-r--r-- | gdb/linux-nat.h | 12 | ||||
| -rw-r--r-- | gdb/testsuite/ChangeLog | 5 | ||||
| -rw-r--r-- | gdb/testsuite/gdb.threads/watchthreads-reorder.c | 369 | ||||
| -rw-r--r-- | gdb/testsuite/gdb.threads/watchthreads-reorder.exp | 103 |
6 files changed, 584 insertions, 2 deletions
diff --git a/gdb/ChangeLog b/gdb/ChangeLog index eee179d..b71505e 100644 --- a/gdb/ChangeLog +++ b/gdb/ChangeLog @@ -1,3 +1,18 @@ +2009-11-20 Jan Kratochvil <jan.kratochvil@redhat.com> + Pedro Alves <pedro@codesourcery.com> + + Fix reordered watchpoints triggered in other threads during all-stop. + + * linux-nat.c (resume_callback, linux_nat_resume): Clear + stopped_by_watchpoint. + (save_sigtrap, linux_nat_stopped_by_watchpoint) + (linux_nat_stopped_data_address): New. + (stop_wait_callback, linux_nat_filter_event): Call save_sigtrap. + (linux_nat_add_target): Install linux_nat_stopped_by_watchpoint + and linux_nat_stopped_data_address. + * linux-nat.h (struct lwp_info): New fields stopped_by_watchpoint, + stopped_data_address_p and stopped_data_address. + 2009-11-20 Michael Snyder <msnyder@vmware.com> * target.h (struct target_ops): New methods to_get_bookmark diff --git a/gdb/linux-nat.c b/gdb/linux-nat.c index 95feca6..c0afecd 100644 --- a/gdb/linux-nat.c +++ b/gdb/linux-nat.c @@ -1882,6 +1882,7 @@ resume_callback (struct lwp_info *lp, void *data) lp->stopped = 0; lp->step = 0; memset (&lp->siginfo, 0, sizeof (lp->siginfo)); + lp->stopped_by_watchpoint = 0; } else if (lp->stopped && debug_linux_nat) fprintf_unfiltered (gdb_stdlog, "RC: Not resuming sibling %s (has pending)\n", @@ -2019,6 +2020,7 @@ linux_nat_resume (struct target_ops *ops, linux_ops->to_resume (linux_ops, ptid, step, signo); memset (&lp->siginfo, 0, sizeof (lp->siginfo)); + lp->stopped_by_watchpoint = 0; if (debug_linux_nat) fprintf_unfiltered (gdb_stdlog, @@ -2570,6 +2572,74 @@ maybe_clear_ignore_sigint (struct lwp_info *lp) } } +/* Fetch the possible triggered data watchpoint info and store it in + LP. + + On some archs, like x86, that use debug registers to set + watchpoints, it's possible that the way to know which watched + address trapped, is to check the register that is used to select + which address to watch. Problem is, between setting the watchpoint + and reading back which data address trapped, the user may change + the set of watchpoints, and, as a consequence, GDB changes the + debug registers in the inferior. To avoid reading back a stale + stopped-data-address when that happens, we cache in LP the fact + that a watchpoint trapped, and the corresponding data address, as + soon as we see LP stop with a SIGTRAP. If GDB changes the debug + registers meanwhile, we have the cached data we can rely on. */ + +static void +save_sigtrap (struct lwp_info *lp) +{ + struct cleanup *old_chain; + + if (linux_ops->to_stopped_by_watchpoint == NULL) + { + lp->stopped_by_watchpoint = 0; + return; + } + + old_chain = save_inferior_ptid (); + inferior_ptid = lp->ptid; + + lp->stopped_by_watchpoint = linux_ops->to_stopped_by_watchpoint (); + + if (lp->stopped_by_watchpoint) + { + if (linux_ops->to_stopped_data_address != NULL) + lp->stopped_data_address_p = + linux_ops->to_stopped_data_address (¤t_target, + &lp->stopped_data_address); + else + lp->stopped_data_address_p = 0; + } + + do_cleanups (old_chain); +} + +/* See save_sigtrap. */ + +static int +linux_nat_stopped_by_watchpoint (void) +{ + struct lwp_info *lp = find_lwp_pid (inferior_ptid); + + gdb_assert (lp != NULL); + + return lp->stopped_by_watchpoint; +} + +static int +linux_nat_stopped_data_address (struct target_ops *ops, CORE_ADDR *addr_p) +{ + struct lwp_info *lp = find_lwp_pid (inferior_ptid); + + gdb_assert (lp != NULL); + + *addr_p = lp->stopped_data_address; + + return lp->stopped_data_address_p; +} + /* Wait until LP is stopped. */ static int @@ -2628,6 +2698,8 @@ stop_wait_callback (struct lwp_info *lp, void *data) /* Save the trap's siginfo in case we need it later. */ save_siginfo (lp); + save_sigtrap (lp); + /* Now resume this LWP and get the SIGSTOP event. */ errno = 0; ptrace (PTRACE_CONT, GET_LWP (lp->ptid), 0, 0); @@ -3027,9 +3099,13 @@ linux_nat_filter_event (int lwpid, int status, int options) return NULL; } - /* Save the trap's siginfo in case we need it later. */ if (WIFSTOPPED (status) && WSTOPSIG (status) == SIGTRAP) - save_siginfo (lp); + { + /* Save the trap's siginfo in case we need it later. */ + save_siginfo (lp); + + save_sigtrap (lp); + } /* Check if the thread has exited. */ if ((WIFEXITED (status) || WIFSIGNALED (status)) @@ -5368,6 +5444,8 @@ linux_nat_add_target (struct target_ops *t) t->to_pid_to_str = linux_nat_pid_to_str; t->to_has_thread_control = tc_schedlock; t->to_thread_address_space = linux_nat_thread_address_space; + t->to_stopped_by_watchpoint = linux_nat_stopped_by_watchpoint; + t->to_stopped_data_address = linux_nat_stopped_data_address; t->to_can_async_p = linux_nat_can_async_p; t->to_is_async_p = linux_nat_is_async_p; diff --git a/gdb/linux-nat.h b/gdb/linux-nat.h index eae74f3..f2a74e8 100644 --- a/gdb/linux-nat.h +++ b/gdb/linux-nat.h @@ -62,6 +62,18 @@ struct lwp_info be the address of a hardware watchpoint. */ struct siginfo siginfo; + /* STOPPED_BY_WATCHPOINT is non-zero if this LWP stopped with a data + watchpoint trap. */ + int stopped_by_watchpoint; + + /* On architectures where it is possible to know the data address of + a triggered watchpoint, STOPPED_DATA_ADDRESS_P is non-zero, and + STOPPED_DATA_ADDRESS contains such data address. Otherwise, + STOPPED_DATA_ADDRESS_P is false, and STOPPED_DATA_ADDRESS is + undefined. Only valid if STOPPED_BY_WATCHPOINT is true. */ + int stopped_data_address_p; + CORE_ADDR stopped_data_address; + /* Non-zero if we expect a duplicated SIGINT. */ int ignore_sigint; diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog index 50c6fb7..83ae052 100644 --- a/gdb/testsuite/ChangeLog +++ b/gdb/testsuite/ChangeLog @@ -1,3 +1,8 @@ +2009-11-20 Jan Kratochvil <jan.kratochvil@redhat.com> + + * gdb.base/watchthreads-reorder.exp, + gdb.base/watchthreads-reorder.c: New. + 2009-11-17 Nathan Sidwell <nathan@codesourcery.com> * gdb.xml/tdesc-regs.exp: Use for m68k. diff --git a/gdb/testsuite/gdb.threads/watchthreads-reorder.c b/gdb/testsuite/gdb.threads/watchthreads-reorder.c new file mode 100644 index 0000000..a67a0d7 --- /dev/null +++ b/gdb/testsuite/gdb.threads/watchthreads-reorder.c @@ -0,0 +1,369 @@ +/* This testcase is part of GDB, the GNU debugger. + + Copyright 2009 Free Software Foundation, Inc. + + 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 3 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, see <http://www.gnu.org/licenses/>. */ + +#define _GNU_SOURCE +#include <pthread.h> +#include <stdio.h> +#include <limits.h> +#include <errno.h> +#include <stdlib.h> +#include <string.h> +#include <assert.h> +#include <sys/types.h> +#include <signal.h> +#include <unistd.h> +#include <asm/unistd.h> + +#define gettid() syscall (__NR_gettid) + +/* Terminate always in the main task, it can lock up with SIGSTOPped GDB + otherwise. */ +#define TIMEOUT (gettid () == getpid() ? 10 : 15) + +static pthread_mutex_t gdbstop_mutex = PTHREAD_ERRORCHECK_MUTEX_INITIALIZER_NP; + +static pid_t thread1_tid; +static pthread_cond_t thread1_tid_cond = PTHREAD_COND_INITIALIZER; +static pthread_mutex_t thread1_tid_mutex = PTHREAD_ERRORCHECK_MUTEX_INITIALIZER_NP; + +static pid_t thread2_tid; +static pthread_cond_t thread2_tid_cond = PTHREAD_COND_INITIALIZER; +static pthread_mutex_t thread2_tid_mutex = PTHREAD_ERRORCHECK_MUTEX_INITIALIZER_NP; + +static pthread_mutex_t terminate_mutex = PTHREAD_ERRORCHECK_MUTEX_INITIALIZER_NP; + +/* These variables must have lower in-memory addresses than thread1_rwatch and + thread2_rwatch so that they take their watchpoint slots. */ + +static int unused1_rwatch; +static int unused2_rwatch; + +static volatile int thread1_rwatch; +static volatile int thread2_rwatch; + +/* Do not use alarm as it would create a ptrace event which would hang up us if + we are being traced by GDB which we stopped ourselves. */ + +static void timed_mutex_lock (pthread_mutex_t *mutex) +{ + int i; + struct timespec start, now; + + i = clock_gettime (CLOCK_MONOTONIC, &start); + assert (i == 0); + + do + { + i = pthread_mutex_trylock (mutex); + if (i == 0) + return; + assert (i == EBUSY); + + i = clock_gettime (CLOCK_MONOTONIC, &now); + assert (i == 0); + assert (now.tv_sec >= start.tv_sec); + } + while (now.tv_sec - start.tv_sec < TIMEOUT); + + fprintf (stderr, "Timed out waiting for internal lock!\n"); + exit (EXIT_FAILURE); +} + +static void * +thread1_func (void *unused) +{ + int i; + volatile int rwatch_store; + + thread1_tid = gettid (); + i = pthread_cond_signal (&thread1_tid_cond); + assert (i == 0); + + /* Be sure GDB is already stopped before continuing. */ + timed_mutex_lock (&gdbstop_mutex); + i = pthread_mutex_unlock (&gdbstop_mutex); + assert (i == 0); + + rwatch_store = thread1_rwatch; + + /* Be sure the "T (tracing stop)" test can proceed for both threads. */ + timed_mutex_lock (&terminate_mutex); + i = pthread_mutex_unlock (&terminate_mutex); + assert (i == 0); + + return NULL; +} + +static void * +thread2_func (void *unused) +{ + int i; + volatile int rwatch_store; + + thread2_tid = gettid (); + i = pthread_cond_signal (&thread2_tid_cond); + assert (i == 0); + + /* Be sure GDB is already stopped before continuing. */ + timed_mutex_lock (&gdbstop_mutex); + i = pthread_mutex_unlock (&gdbstop_mutex); + assert (i == 0); + + rwatch_store = thread2_rwatch; + + /* Be sure the "T (tracing stop)" test can proceed for both threads. */ + timed_mutex_lock (&terminate_mutex); + i = pthread_mutex_unlock (&terminate_mutex); + assert (i == 0); + + return NULL; +} + +static const char * +proc_string (const char *filename, const char *line) +{ + FILE *f; + static char buf[LINE_MAX]; + size_t line_len = strlen (line); + + f = fopen (filename, "r"); + if (f == NULL) + { + fprintf (stderr, "fopen (\"%s\") for \"%s\": %s\n", filename, line, + strerror (errno)); + exit (EXIT_FAILURE); + } + while (errno = 0, fgets (buf, sizeof (buf), f)) + { + char *s; + + s = strchr (buf, '\n'); + assert (s != NULL); + *s = 0; + + if (strncmp (buf, line, line_len) != 0) + continue; + + if (fclose (f)) + { + fprintf (stderr, "fclose (\"%s\") for \"%s\": %s\n", filename, line, + strerror (errno)); + exit (EXIT_FAILURE); + } + + return &buf[line_len]; + } + if (errno != 0) + { + fprintf (stderr, "fgets (\"%s\": %s\n", filename, strerror (errno)); + exit (EXIT_FAILURE); + } + fprintf (stderr, "\"%s\": No line \"%s\" found.\n", filename, line); + exit (EXIT_FAILURE); +} + +static unsigned long +proc_ulong (const char *filename, const char *line) +{ + const char *s = proc_string (filename, line); + long retval; + char *end; + + errno = 0; + retval = strtol (s, &end, 10); + if (retval < 0 || retval >= LONG_MAX || (end && *end)) + { + fprintf (stderr, "\"%s\":\"%s\": %ld, %s\n", filename, line, retval, + strerror (errno)); + exit (EXIT_FAILURE); + } + return retval; +} + +static void +state_wait (pid_t process, const char *wanted) +{ + char *filename; + int i; + struct timespec start, now; + const char *state; + + i = asprintf (&filename, "/proc/%lu/status", (unsigned long) process); + assert (i > 0); + + i = clock_gettime (CLOCK_MONOTONIC, &start); + assert (i == 0); + + do + { + state = proc_string (filename, "State:\t"); + if (strcmp (state, wanted) == 0) + { + free (filename); + return; + } + + if (sched_yield ()) + { + perror ("sched_yield()"); + exit (EXIT_FAILURE); + } + + i = clock_gettime (CLOCK_MONOTONIC, &now); + assert (i == 0); + assert (now.tv_sec >= start.tv_sec); + } + while (now.tv_sec - start.tv_sec < TIMEOUT); + + fprintf (stderr, "Timed out waiting for PID %lu \"%s\" (now it is \"%s\")!\n", + (unsigned long) process, wanted, state); + exit (EXIT_FAILURE); +} + +static volatile pid_t tracer = 0; +static pthread_t thread1, thread2; + +static void +cleanup (void) +{ + printf ("Resuming GDB PID %lu.\n", (unsigned long) tracer); + + if (tracer) + { + int i; + int tracer_save = tracer; + + tracer = 0; + + i = kill (tracer_save, SIGCONT); + assert (i == 0); + } +} + +int +main (int argc, char **argv) +{ + int i; + int standalone = 0; + + if (argc == 2 && strcmp (argv[1], "-s") == 0) + standalone = 1; + else + assert (argc == 1); + + setbuf (stdout, NULL); + + timed_mutex_lock (&gdbstop_mutex); + + timed_mutex_lock (&terminate_mutex); + + i = pthread_create (&thread1, NULL, thread1_func, NULL); + assert (i == 0); + + i = pthread_create (&thread2, NULL, thread2_func, NULL); + assert (i == 0); + + if (!standalone) + { + tracer = proc_ulong ("/proc/self/status", "TracerPid:\t"); + if (tracer == 0) + { + fprintf (stderr, "The testcase must be run by GDB!\n"); + exit (EXIT_FAILURE); + } + if (tracer != getppid ()) + { + fprintf (stderr, "The testcase parent must be our GDB tracer!\n"); + exit (EXIT_FAILURE); + } + } + + /* SIGCONT our debugger in the case of our crash as we would deadlock + otherwise. */ + + atexit (cleanup); + + printf ("Stopping GDB PID %lu.\n", (unsigned long) tracer); + + if (tracer) + { + i = kill (tracer, SIGSTOP); + assert (i == 0); + state_wait (tracer, "T (stopped)"); + } + + timed_mutex_lock (&thread1_tid_mutex); + timed_mutex_lock (&thread2_tid_mutex); + + /* Let the threads start. */ + i = pthread_mutex_unlock (&gdbstop_mutex); + assert (i == 0); + + printf ("Waiting till the threads initialize their TIDs.\n"); + + if (thread1_tid == 0) + { + i = pthread_cond_wait (&thread1_tid_cond, &thread1_tid_mutex); + assert (i == 0); + + assert (thread1_tid > 0); + } + + if (thread2_tid == 0) + { + i = pthread_cond_wait (&thread2_tid_cond, &thread2_tid_mutex); + assert (i == 0); + + assert (thread2_tid > 0); + } + + printf ("Thread 1 TID = %lu, thread 2 TID = %lu, PID = %lu.\n", + (unsigned long) thread1_tid, (unsigned long) thread2_tid, + (unsigned long) getpid ()); + + printf ("Waiting till the threads get trapped by the watchpoints.\n"); + + if (tracer) + { + /* s390x-unknown-linux-gnu will fail with "R (running)". */ + + state_wait (thread1_tid, "T (tracing stop)"); + + state_wait (thread2_tid, "T (tracing stop)"); + } + + cleanup (); + + printf ("Joining the threads.\n"); + + i = pthread_mutex_unlock (&terminate_mutex); + assert (i == 0); + + i = pthread_join (thread1, NULL); + assert (i == 0); + + i = pthread_join (thread2, NULL); + assert (i == 0); + + printf ("Exiting.\n"); /* break-at-exit */ + + /* Just prevent compiler `warning: unusedX_rwatch defined but not used'. */ + unused1_rwatch = 1; + unused2_rwatch = 2; + + return EXIT_SUCCESS; +} diff --git a/gdb/testsuite/gdb.threads/watchthreads-reorder.exp b/gdb/testsuite/gdb.threads/watchthreads-reorder.exp new file mode 100644 index 0000000..bf9b044 --- /dev/null +++ b/gdb/testsuite/gdb.threads/watchthreads-reorder.exp @@ -0,0 +1,103 @@ +# This testcase is part of GDB, the GNU debugger. + +# Copyright 2009 Free Software Foundation, Inc. + +# 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 3 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, see <http://www.gnu.org/licenses/>. + +# Test GDB can cope with two watchpoints being hit by different threads at the +# same time, GDB reports one of them and after "continue" to report the other +# one GDB should not be confused by differently set watchpoints that time. +# This is the goal of "reorder1". "reorder0" tests the basic functionality of +# two watchpoints being hit at the same time, without reordering them during the +# stop. The formerly broken functionality is due to the all-stop mode default +# "show breakpoint always-inserted" being "off". Formerly the remembered hit +# could be assigned during continuation of a thread with pending SIGTRAP to the +# different/new watchpoint, just based on the watchpoint/debug register number. + +if {(![istarget "i?86-*-*"] && ![istarget "x86_64-*-*"] + && ![istarget "ia64-*-*"] && ![istarget "s390*-*-*"]) + || [target_info exists gdb,no_hardware_watchpoints] + || ![istarget *-*-linux*]} { + return 0 +} + +set testfile "watchthreads-reorder" +set srcfile ${testfile}.c +set binfile ${objdir}/${subdir}/${testfile} +if {[gdb_compile_pthreads "${srcdir}/${subdir}/${srcfile}" ${binfile} executable [list debug additional_flags=-lrt]] != "" } { + return -1 +} + +foreach reorder {0 1} { + + global pf_prefix + set prefix_test $pf_prefix + lappend pf_prefix "reorder$reorder:" + + clean_restart $testfile + + gdb_test "set can-use-hw-watchpoints 1" + + if ![runto_main] { + return -1 + } + + # Use "rwatch" as "watch" would report the watchpoint changed just based on its + # read memory value during a stop by unrelated event. We are interested in not + # losing the hardware watchpoint trigger. + + gdb_test "rwatch thread1_rwatch" "Hardware read watchpoint \[0-9\]+: thread1_rwatch" + set test "rwatch thread2_rwatch" + gdb_test_multiple $test $test { + -re "Target does not support this type of hardware watchpoint\\.\r\n$gdb_prompt $" { + # ppc64 supports at most 1 hw watchpoints. + unsupported $test + return + } + -re "Hardware read watchpoint \[0-9\]+: thread2_rwatch\r\n$gdb_prompt $" { + pass $test + } + } + gdb_breakpoint [gdb_get_line_number "break-at-exit"] + + # The watchpoints can happen in arbitrary order depending on random: + # SEL: Found 2 SIGTRAP events, selecting #[01] + # As GDB contains no srand() on the specific host/OS it will behave always the + # same. Such order cannot be guaranteed for GDB in general. + + gdb_test "continue" \ + "Hardware read watchpoint \[0-9\]+: thread\[12\]_rwatch\r\n\r\nValue = 0\r\n0x\[0-9a-f\]+ in thread\[12\]_func .*" \ + "continue a" + + if $reorder { + # GDB orders watchpoints by their addresses so inserting new variables + # with lower addresses will shift the former watchpoints to higher + # debug registers. + + gdb_test "rwatch unused1_rwatch" "Hardware read watchpoint \[0-9\]+: unused1_rwatch" + gdb_test "rwatch unused2_rwatch" "Hardware read watchpoint \[0-9\]+: unused2_rwatch" + } + + gdb_test "continue" \ + "Hardware read watchpoint \[0-9\]+: thread\[12\]_rwatch\r\n\r\nValue = 0\r\n0x\[0-9a-f\]+ in thread\[12\]_func .*" \ + "continue b" + + # While the debug output itself is not checked in this testcase one bug was + # found in the DEBUG_INFRUN code path. + gdb_test "set debug infrun 1" + + gdb_continue_to_breakpoint "break-at-exit" ".*break-at-exit.*" + + set pf_prefix $prefix_test +} |