From f6bc20088048522ffb945254f59f824f5cbdaebd Mon Sep 17 00:00:00 2001 From: Pedro Alves Date: Sat, 21 Nov 2009 21:17:17 +0000 Subject: [PATCH] gdb/ * breakpoint.h (struct breakpoint) : New field. * breakpoint.c (watchpoint_in_thread_scope): New. (update_watchpoint): Skip if the local watchpoint's thread doesn't match the current thread, or if the current thread is running. (watchpoint_check): Ditto. (watch_command_1): Set the watchpoint's watchpoint_thread field. gdb/testsuite/ * gdb.threads/local-watch-wrong-thread.c, gdb.threads/local-watch-wrong-thread.exp: New files. --- gdb/ChangeLog | 9 ++ gdb/breakpoint.c | 40 ++++++- gdb/breakpoint.h | 5 + gdb/testsuite/ChangeLog | 5 + .../gdb.threads/local-watch-wrong-thread.c | 88 +++++++++++++++ .../gdb.threads/local-watch-wrong-thread.exp | 122 +++++++++++++++++++++ 6 files changed, 267 insertions(+), 2 deletions(-) create mode 100644 gdb/testsuite/gdb.threads/local-watch-wrong-thread.c create mode 100644 gdb/testsuite/gdb.threads/local-watch-wrong-thread.exp diff --git a/gdb/ChangeLog b/gdb/ChangeLog index a74e149..7a98160 100644 --- a/gdb/ChangeLog +++ b/gdb/ChangeLog @@ -1,3 +1,12 @@ +2009-11-21 Pedro Alves + + * breakpoint.h (struct breakpoint) : New field. + * breakpoint.c (watchpoint_in_thread_scope): New. + (update_watchpoint): Skip if the local watchpoint's thread doesn't + match the current thread, or if the current thread is running. + (watchpoint_check): Ditto. + (watch_command_1): Set the watchpoint's watchpoint_thread field. + 2009-11-20 Jan Kratochvil * breakpoint.c (bp_location_compare): Change parameter a to ap and b to diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c index bca923e..510399f 100644 --- a/gdb/breakpoint.c +++ b/gdb/breakpoint.c @@ -985,6 +985,24 @@ fetch_watchpoint_value (struct expression *exp, struct value **valp, } } +/* Assuming that B is a watchpoint: returns true if the current thread + and its running state are safe to evaluate or update watchpoint B. + Watchpoints on local expressions need to be evaluated in the + context of the thread that was current when the watchpoint was + created, and, that thread needs to be stopped to be able to select + the correct frame context. Watchpoints on global expressions can + be evaluated on any thread, and in any state. It is presently left + to the target allowing memory accesses when threads are + running. */ + +static int +watchpoint_in_thread_scope (struct breakpoint *b) +{ + return (ptid_equal (b->watchpoint_thread, null_ptid) + || (ptid_equal (inferior_ptid, b->watchpoint_thread) + && !is_executing (inferior_ptid))); +} + /* Assuming that B is a watchpoint: - Reparse watchpoint expression, if REPARSE is non-zero - Evaluate expression and store the result in B->val @@ -1043,6 +1061,12 @@ update_watchpoint (struct breakpoint *b, int reparse) bpstat bs; struct program_space *frame_pspace; + /* If this is a local watchpoint, we only want to check if the + watchpoint frame is in scope if the current thread is the thread + that was used to create the watchpoint. */ + if (!watchpoint_in_thread_scope (b)) + return; + /* We don't free locations. They are stored in bp_location array and update_global_locations will eventually delete them and remove breakpoints if needed. */ @@ -3124,6 +3148,12 @@ watchpoint_check (void *p) b = bs->breakpoint_at->owner; + /* If this is a local watchpoint, we only want to check if the + watchpoint frame is in scope if the current thread is the thread + that was used to create the watchpoint. */ + if (!watchpoint_in_thread_scope (b)) + return WP_VALUE_NOT_CHANGED; + if (b->exp_valid_block == NULL) within_current_scope = 1; else @@ -7190,9 +7220,15 @@ watch_command_1 (char *arg, int accessflag, int from_tty) b->cond_string = 0; if (frame) - b->watchpoint_frame = get_frame_id (frame); + { + b->watchpoint_frame = get_frame_id (frame); + b->watchpoint_thread = inferior_ptid; + } else - b->watchpoint_frame = null_frame_id; + { + b->watchpoint_frame = null_frame_id; + b->watchpoint_thread = null_ptid; + } if (scope_breakpoint != NULL) { diff --git a/gdb/breakpoint.h b/gdb/breakpoint.h index 91e864c..6587407 100644 --- a/gdb/breakpoint.h +++ b/gdb/breakpoint.h @@ -457,6 +457,11 @@ struct breakpoint should be evaluated on the outermost frame. */ struct frame_id watchpoint_frame; + /* Holds the thread which identifies the frame this watchpoint + should be considered in scope for, or `null_ptid' if the + watchpoint should be evaluated in all threads. */ + ptid_t watchpoint_thread; + /* For hardware watchpoints, the triggered status according to the hardware. */ enum watchpoint_triggered watchpoint_triggered; diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog index 5caae10..9de6777 100644 --- a/gdb/testsuite/ChangeLog +++ b/gdb/testsuite/ChangeLog @@ -1,5 +1,10 @@ 2009-11-21 Pedro Alves + * gdb.threads/local-watch-wrong-thread.c, + gdb.threads/local-watch-wrong-thread.exp: New files. + +2009-11-21 Pedro Alves + * gdb.cp/cplusfuncs.exp (info_func_regexp, print_addr): Don't assume new `regsub' syntax available. diff --git a/gdb/testsuite/gdb.threads/local-watch-wrong-thread.c b/gdb/testsuite/gdb.threads/local-watch-wrong-thread.c new file mode 100644 index 0000000..7aed14c --- /dev/null +++ b/gdb/testsuite/gdb.threads/local-watch-wrong-thread.c @@ -0,0 +1,88 @@ +/* This testcase is part of GDB, the GNU debugger. + + Copyright 2002, 2003, 2004, 2007, 2008, 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 . + +*/ + +#include +#include +#include + +unsigned int args[2]; +int trigger = 0; + +void * +thread_function0 (void *arg) +{ + int my_number = (long) arg; + volatile int *myp = (volatile int *) &args[my_number]; + + while (*myp > 0) + { + (*myp) ++; + usleep (1); /* Loop increment 1. */ + } + + return NULL; +} + +void * +thread_function0_1 (void *arg) +{ + void *ret = thread_function0 (arg); + + return ret; /* set breakpoint here */ +} + +void * +thread_function1 (void *arg) +{ + int my_number = (long) arg; + + volatile int *myp = (volatile int *) &args[my_number]; + + while (*myp > 0) + { + (*myp) ++; + usleep (1); /* Loop increment 2. */ + } + + return NULL; +} + +int +main () +{ + int res; + pthread_t threads[2]; + void *thread_result; + long i = 0; + + args[i] = 1; /* Init value. */ + res = pthread_create (&threads[i], NULL, + thread_function0_1, + (void *) i); + + i++; + args[i] = 1; /* Init value. */ + res = pthread_create(&threads[i], NULL, + thread_function1, + (void *) i); + + pthread_join (threads[0], &thread_result); + pthread_join (threads[1], &thread_result); + exit(EXIT_SUCCESS); +} diff --git a/gdb/testsuite/gdb.threads/local-watch-wrong-thread.exp b/gdb/testsuite/gdb.threads/local-watch-wrong-thread.exp new file mode 100644 index 0000000..ecd7be9 --- /dev/null +++ b/gdb/testsuite/gdb.threads/local-watch-wrong-thread.exp @@ -0,0 +1,122 @@ +# 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 . + +# This test verifies that a local watchpoint isn't deleted when a +# thread other than the thread the local watchpoint was set in stops +# for a breakpoint. + +if [target_info exists gdb,no_hardware_watchpoints] { + return 0; +} + +set testfile "local-watch-wrong-thread" +set srcfile ${testfile}.c +set binfile ${objdir}/${subdir}/${testfile} +if {[gdb_compile_pthreads \ + "${srcdir}/${subdir}/${srcfile}" \ + "${binfile}" executable {debug} ] != "" } { + return -1 +} + +gdb_exit +gdb_start +gdb_reinitialize_dir $srcdir/$subdir +gdb_load ${binfile} + +gdb_test "set can-use-hw-watchpoints 1" "" "" + +if ![runto_main] then { + fail "Can't run to main" + return +} + +set inc_line_1 [gdb_get_line_number "Loop increment 1"] +set inc_line_2 [gdb_get_line_number "Loop increment 2"] +set bkpt_here [gdb_get_line_number "set breakpoint here"] + +# Run to the loop within thread_function0, so we can set our local +# watchpoint. +gdb_test "break ${srcfile}:${inc_line_1}" \ + "Breakpoint 2 at .*: file .*${srcfile}, line .*" \ + "breakpoint on thread_function0" + +gdb_test "continue" \ + ".*Breakpoint 2.*thread_function0.*" \ + "continue to thread_function0" + +delete_breakpoints + +# Set the local watchpoint, and confirm that it traps as expected. +gdb_test "watch *myp" \ + "Hardware watchpoint 3\: \\*myp.*" \ + "set local watchpoint on *myp" + +gdb_test "continue" \ + "Hardware watchpoint.*Old value.*New value.*thread_function0.*" \ + "local watchpoint triggers" + +delete_breakpoints + +# Recreate the watchpoint, but this time with a condition we know +# won't trigger. This is so the watchpoint is inserted, and the +# target reports events, but GDB doesn't stop for them. We want to +# hit the breakpoints on the other thread, and make sure this +# watchpoint isn't deleted then. +gdb_test "watch *myp if trigger != 0" \ + "Hardware watchpoint 4\: \\*myp.*" \ + "set local watchpoint on *myp, with false conditional" + +# Run to a breakpoint on a different thread. The previous local +# watchpoint should not be deleted with the standard 'the program has +# left the block in which its expression is valid', because the +# thread_function0 thread should still be running in the loop. +gdb_test "break ${srcfile}:${inc_line_2}" \ + "Breakpoint 5 at .*: file .*${srcfile}, line .*" \ + "breakpoint on the other thread" + +gdb_test "continue" \ + "Breakpoint 5, thread_function1.*" \ + "the other thread stopped on breakpoint" + +# Delete the new breakpoint, we don't need it anymore. +gdb_test "delete 5" "" "" + +# Check if the local watchpoint hasn't been deleted (is still listed). +# This is simpler to check than expecting 'the program has left ...', +# and, is immune to string changes in that warning. +gdb_test "info breakpoints" \ + ".*4.*hw watchpoint.*keep.*y.*\\*myp.*" \ + "local watchpoint is still in breakpoint list" + +# Make the watchpoint condition eval true. +gdb_test "set trigger=1" "" "let local watchpoint trigger" + +gdb_test "continue" \ + "Hardware watchpoint.*Old value.*New value.*thread_function0.*" \ + "local watchpoint still triggers" + +# Confirm that the local watchpoint is indeed deleted when +# thread_function0 returns. +gdb_test "set *myp=0" "" "let thread_function0 return" + +gdb_test "break ${srcfile}:${bkpt_here}" \ + "Breakpoint 6 at .*: file .*${srcfile}, line .*" \ + "breakpoint on thread_function0's caller" + +gdb_test "continue" \ + ".*Watchpoint.*deleted.*" \ + "local watchpoint automatically deleted" -- 2.7.4