From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 14104 invoked by alias); 17 Aug 2009 19:46:29 -0000 Received: (qmail 14092 invoked by uid 22791); 17 Aug 2009 19:46:27 -0000 X-SWARE-Spam-Status: No, hits=-0.4 required=5.0 tests=AWL,BAYES_50,SPF_HELO_PASS,SPF_PASS,ZMIde_GENERICSPAM1 X-Spam-Check-By: sourceware.org Received: from mx2.redhat.com (HELO mx2.redhat.com) (66.187.237.31) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Mon, 17 Aug 2009 19:46:16 +0000 Received: from int-mx2.corp.redhat.com (int-mx2.corp.redhat.com [172.16.27.26]) by mx2.redhat.com (8.13.8/8.13.8) with ESMTP id n7HJkF4n014050 for ; Mon, 17 Aug 2009 15:46:15 -0400 Received: from ns3.rdu.redhat.com (ns3.rdu.redhat.com [10.11.255.199]) by int-mx2.corp.redhat.com (8.13.1/8.13.1) with ESMTP id n7HJkE26017303 for ; Mon, 17 Aug 2009 15:46:14 -0400 Received: from host0.dyn.jankratochvil.net (ovpn01.gateway.prod.ext.phx2.redhat.com [10.5.9.1]) by ns3.rdu.redhat.com (8.13.8/8.13.8) with ESMTP id n7HJkCjL006371 for ; Mon, 17 Aug 2009 15:46:13 -0400 Received: from host0.dyn.jankratochvil.net (localhost [127.0.0.1]) by host0.dyn.jankratochvil.net (8.14.3/8.14.3) with ESMTP id n7HJkCIS002694 for ; Mon, 17 Aug 2009 21:46:12 +0200 Received: (from jkratoch@localhost) by host0.dyn.jankratochvil.net (8.14.3/8.14.3/Submit) id n7HJkC6X002693 for gdb-patches@sourceware.org; Mon, 17 Aug 2009 21:46:12 +0200 Date: Mon, 17 Aug 2009 19:46:00 -0000 From: Jan Kratochvil To: gdb-patches@sourceware.org Subject: [patch 2/4] Fix hw watchpoints: reordered / simultaneously hit Message-ID: <20090817194612.GC10694@host0.dyn.jankratochvil.net> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-2022-jp Content-Disposition: inline User-Agent: Mutt/1.5.19 (2009-01-05) X-IsSubscribed: yes Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2009-08/txt/msg00257.txt.bz2 Hi, if you hit two watchpoints at the same time in the default all-stop mode GDB does not cope well with the pending watchpoint hit in non-current LWP. Moreover when you change the watchpoints setup during such stop. The is a bit problem the dependency on debug register number is specific to x86* (on other arches hardware may directly report the address of triggered watchpoint). But it is also dependent on Linux as it depends on LWPs (so i386-nat.c is too general). And it would need to be duplicate in both {i386,amd64}-linux-nat.c. Therefore the code is put in linux-nat.c where it does not hurt non-x86* arches (it may not even get installed on those). The testcase reproducer is difficult to make reliable (as is now): GDB must find two threads both with a watchpoint hit at the single stop. The used SIGSTOP of GDB is a bit fragile as the inferior being traced must not trigger a ptrace event otherwise it deadlocks with its own debugger SIGSTOPped. Thanks, Jan gdb/ 2009-08-17 Jan Kratochvil Fix reordered watchpoints triggered in other threads during all-stop. * i386-nat.c (i386_stopped_by_watchpoint): Call i386_stopped_data_address through the target vector. * linux-nat.c (save_sigtrap, linux_nat_stopped_data_address): New. (stop_wait_callback, linux_nat_filter_event): Call save_sigtrap. (linux_nat_add_target): Install linux_nat_stopped_data_address. * linux-nat.h (struct lwp_info): New fields watchpoint_hit_set and watchpoint_hit. gdb/testsuite/ 2009-08-17 Jan Kratochvil * watchthreads-reorder.exp, watchthreads-reorder.c: New. --- a/gdb/i386-nat.c +++ b/gdb/i386-nat.c @@ -587,7 +587,7 @@ static int i386_stopped_by_watchpoint (void) { CORE_ADDR addr = 0; - return i386_stopped_data_address (¤t_target, &addr); + return target_stopped_data_address (¤t_target, &addr); } /* Return non-zero if the inferior has some break/watchpoint that --- a/gdb/linux-nat.c +++ b/gdb/linux-nat.c @@ -2164,6 +2164,45 @@ maybe_clear_ignore_sigint (struct lwp_info *lp) } } +/* Fetch the possible triggered data watchpoint info and store it to LP. + The hardware data watchpoint trigger gets cleared during this fetch. */ + +static void +save_sigtrap (struct lwp_info *lp) +{ + struct cleanup *old_chain; + + /* linux_nat_stopped_data_address is not even installed in this case. */ + if (linux_ops->to_stopped_data_address == NULL) + return; + + old_chain = save_inferior_ptid (); + inferior_ptid = lp->ptid; + + lp->watchpoint_hit_set = + linux_ops->to_stopped_data_address (¤t_target, &lp->watchpoint_hit); + + do_cleanups (old_chain); +} + +/* Wrap target_stopped_data_address where the GNU/Linux native target may be + directed by the watchpoint/debug register number. Base the reported value + on the triggered data address instead. During inferior stop the assignment + of watchpoint/debug registers may change making the register number specific + trigger info stale. */ + +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->watchpoint_hit; + + return lp->watchpoint_hit_set; +} + /* Wait until LP is stopped. */ static int @@ -2215,6 +2254,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); @@ -2579,9 +2620,13 @@ linux_nat_filter_event (int lwpid, int status, int options) add_thread (lp->ptid); } - /* 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); + } /* Handle GNU/Linux's extended waitstatus for trace events. */ if (WIFSTOPPED (status) && WSTOPSIG (status) == SIGTRAP && status >> 16 != 0) @@ -4871,6 +4916,8 @@ linux_nat_add_target (struct target_ops *t) t->to_thread_alive = linux_nat_thread_alive; t->to_pid_to_str = linux_nat_pid_to_str; t->to_has_thread_control = tc_schedlock; + if (linux_ops->to_stopped_data_address) + 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; --- a/gdb/linux-nat.h +++ b/gdb/linux-nat.h @@ -62,6 +62,12 @@ struct lwp_info be the address of a hardware watchpoint. */ struct siginfo siginfo; + /* WATCHPOINT_HIT_SET is non-zero if this LWP stopped with a trap and a data + watchpoint has been found as triggered. In such case WATCHPOINT_HIT + contains data address of the triggered data watchpoint. */ + unsigned watchpoint_hit_set : 1; + CORE_ADDR watchpoint_hit; + /* Non-zero if we expect a duplicated SIGINT. */ int ignore_sigint; --- /dev/null +++ b/gdb/testsuite/gdb.threads/watchthreads-reorder.c @@ -0,0 +1,366 @@ +/* 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 . */ + +#define _GNU_SOURCE +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#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; + +static volatile int thread1_rwatch; +static volatile int thread2_rwatch; + +static int unused1_rwatch; +static int unused2_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; +} --- /dev/null +++ b/gdb/testsuite/gdb.threads/watchthreads-reorder.exp @@ -0,0 +1,101 @@ +# 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 . + +# 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 watchpoint 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 {[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] { + gdb_suppress_tests + } + + # 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 to not + # to lose the hardware watchpoint trigger. + + gdb_test "rwatch thread1_rwatch" "Hardware read watchpoint \[0-9\]+: thread1_rwatch" + gdb_test {set $rwatch1=$bpnum} + 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_test {set $rwatch2=$bpnum} + 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_test {delete $rwatch1} + gdb_test {delete $rwatch2} + + 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 "rwatch thread1_rwatch" "Hardware read watchpoint \[0-9\]+: thread1_rwatch" + gdb_test "rwatch thread2_rwatch" "Hardware read watchpoint \[0-9\]+: thread2_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" + + gdb_continue_to_breakpoint "break-at-exit" ".*break-at-exit.*" + + set pf_prefix $prefix_test +}