Re: [patch] Watchpoints: support for thread <thread_num> parameters

[Luis Machado <luisgpm at linux dot vnet dot ibm dot com>]

Hi folks,

Here is the patch together with the testcase and the documentation bits.
Please let me know if there are any improvements/corrections suitable
for this one.

Best regards,
Luis

On Thu, 2007-10-11 at 17:29 -0300, Luis Machado wrote:
> Daniel, Eli,
> 
> Thanks for reviewing.
> 
> I'll provide the necessary bits of documentation and a testcase for this
> one, then i'll attach those to the patch.
> 
> Regards,
> On Thu, 2007-10-11 at 15:35 -0400, Daniel Jacobowitz wrote:
> > On Fri, Aug 17, 2007 at 12:46:56PM -0300, Luis Machado wrote:
> > > 2007-08-16  Luis Machado  <luisgpm@linux.vnet.ibm.com>
> > > 
> > >   * breakpoint.c: (watch_command_1): Parse additional optional 
> > >   "thread" parameter to the watchpoint command and set the 
> > >   "thread" member of the breakpoint struct.
> > 
> > Like Eli, I'm happy enough with this version - but it needs
> > documentation and a test case.
> > 

2007-11-13  Luis Machado  <luisgpm@br.ibm.com>

      * breakpoint.c: (watch_command_1): Parse additional optional 
      "thread" parameter to the watchpoint command and set the 
      "thread" member of the breakpoint struct.
      * doc/gdb.texinfo: Add new parameter's description.
      * testsuite/gdb.base/watch_thread_num.c: New testcase source file.
      * testsuite/gdb.base/watch_thread_num.exp: New testcase expect file.

Index: gdb/breakpoint.c
===================================================================
--- gdb.orig/breakpoint.c	2007-11-12 11:22:15.000000000 -0800
+++ gdb/breakpoint.c	2007-11-12 11:22:18.000000000 -0800
@@ -5882,7 +5882,7 @@
   struct frame_info *prev_frame = NULL;
   char *exp_start = NULL;
   char *exp_end = NULL;
-  char *tok, *end_tok;
+  char *tok, *id_tok_start, *end_tok;
   int toklen;
   char *cond_start = NULL;
   char *cond_end = NULL;
@@ -5890,10 +5890,72 @@
   int i, other_type_used, target_resources_ok = 0;
   enum bptype bp_type;
   int mem_cnt = 0;
+  int thread = -1;
 
   init_sal (&sal);		/* initialize to zeroes */
 
-  /* Parse arguments.  */
+  /* Make sure that we actually have parameters to parse.  */
+  if (arg != NULL && strlen (arg) >= 1)
+    {
+      toklen = strlen (arg); /* Size of argument list.  */
+
+      /* Points tok to the end of the argument list.  */
+      tok = arg + toklen - 1;
+
+      /* Go backwards in the parameters list. Skip the last parameter.
+         If we're expecting a 'thread <thread_num>' parameter, this should
+         be the thread identifier.  */
+      while (tok > arg && (*tok == ' ' || *tok == '\t'))
+        tok--;
+      while (tok > arg && (*tok != ' ' && *tok != '\t'))
+        tok--;
+
+      /* Points end_tok to the beginning of the last token.  */
+      id_tok_start = tok + 1;
+
+      /* Go backwards in the parameters list. Skip one more parameter.
+         If we're expecting a 'thread <thread_num>' parameter, we should
+         reach a "thread" token.  */
+      while (tok > arg && (*tok == ' ' || *tok == '\t'))
+        tok--;
+
+      end_tok = tok;
+
+      while (tok > arg && (*tok != ' ' && *tok != '\t'))
+        tok--;
+
+      /* Move the pointer forward to skip the whitespace and
+         calculate the length of the token.  */
+      tok++;
+      toklen = end_tok - tok;
+
+      if (toklen >= 1 && strncmp (tok, "thread", toklen) == 0)
+        {
+          /* At this point we've found a "thread" token, which means
+             the user is trying to set a watchpoint that triggers
+             only in a specific thread.  */
+          char *endp;
+
+          /* Extract the thread ID from the next token.  */
+          thread = strtol (id_tok_start, &endp, 0);
+
+          /* Check if the user provided a valid numeric value for the
+             thread ID.  */
+          if (*endp != ' ' && *endp != '\t' && *endp != '\0')
+            error (_("Invalid thread ID specification %s."), id_tok_start);
+
+          /* Check if the thread actually exists.  */
+          if (!valid_thread_id (thread))
+            error (_("Unknown thread %d."), thread);
+
+          /* Truncate the string and get rid of the thread <thread_num>
+             parameter before the parameter list is parsed by the
+             evaluate_expression() function.  */
+          *tok = '\0';
+        }
+    }
+
+  /* Parse the rest of the arguments.  */
   innermost_block = NULL;
   exp_start = arg;
   exp = parse_exp_1 (&arg, 0, 0);
@@ -5986,6 +6048,7 @@
   b = set_raw_breakpoint (sal, bp_type);
   set_breakpoint_count (breakpoint_count + 1);
   b->number = breakpoint_count;
+  b->thread = thread;
   b->disposition = disp_donttouch;
   b->exp = exp;
   b->exp_valid_block = exp_valid_block;
Index: gdb/testsuite/gdb.base/watch_thread_num.c
===================================================================
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ gdb/testsuite/gdb.base/watch_thread_num.c	2007-11-13 06:35:30.000000000 -0800
@@ -0,0 +1,63 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2002, 2003, 2004 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 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.
+
+   This file is copied from schedlock.c.  */
+
+#include <stdio.h>
+#include <unistd.h>
+#include <stdlib.h>
+#include <pthread.h>
+
+void *thread_function (void *arg); /* Pointer to function executed by each thread */
+
+#define NUM 5
+
+static unsigned int shared_var = 1;
+
+int main () {
+    int res;
+    pthread_t threads[NUM];
+    void *thread_result;
+    long i;
+
+    for (i = 0; i < NUM; i++)
+      {
+        res = pthread_create (&threads[i],
+                             NULL,
+                             thread_function,
+			     (void *) i);
+      }
+
+    thread_result = thread_function ((void *) i);
+
+    exit (EXIT_SUCCESS);
+}
+
+void *thread_function (void *arg) {
+    int my_number = (long) arg;
+    /* Don't run forever.  Run just short of it :)  */
+    while (shared_var > 0)
+      {
+        shared_var++;
+	usleep (1); /* Loop increment.  */
+      }
+
+    pthread_exit (NULL);
+}
+
Index: gdb/testsuite/gdb.base/watch_thread_num.exp
===================================================================
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ gdb/testsuite/gdb.base/watch_thread_num.exp	2007-11-12 11:26:37.000000000 -0800
@@ -0,0 +1,67 @@
+# This testcase is part of GDB, the GNU debugger.
+
+# Copyright 2007
+# 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/>.  */
+
+# watch-thread_num.exp   Test thread <thread_num> parameter for
+#                        watch commands.
+#
+
+if $tracelevel then {
+	strace $tracelevel
+}
+
+set testfile watch_thread_num
+set srcfile ${testfile}.c
+set binfile ${objdir}/${subdir}/${testfile}
+
+# What compiler are we using?
+#
+if [get_compiler_info ${binfile}] {
+    return -1
+}
+
+if  { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable {debug libs=-lpthread}] != "" } {
+     untested watch_thread_num.exp
+     return -1
+}
+
+# use this to debug:
+#log_user 1
+
+gdb_exit
+gdb_start
+gdb_reinitialize_dir $srcdir/$subdir
+gdb_load ${binfile}
+
+if { ![runto main] } then {
+   fail "run to main"
+   return
+}
+
+gdb_test "watch shared_var thread 0" "Unknown thread 0\." "Watchpoint on invalid thread"
+gdb_test "watch shared_var thread" "A syntax error in expression, near `thread'\." "Invalid watch syntax"
+
+gdb_test "Next 5" ""
+
+gdb_test "watch shared_var thread 2" "Hardware watchpoint 2: shared_var" "Watchpoint on shared variable"
+gdb_test "info breakpoint 2" "stop only in thread 2"
+
+for {set i 0} {$i < 10} {incr i 1} {
+gdb_test "continue" "Hardware watchpoint 2: shared_var.*" "Watchpoint triggered"
+gdb_test "thread" "\\\[Current thread is 2 \\\(Thread $hex \\\(LWP $decimal\\\)\\\)\\\]" "Check thread that triggered"
+}
+
Index: gdb/doc/gdb.texinfo
===================================================================
--- gdb.orig/doc/gdb.texinfo	2007-11-12 11:22:15.000000000 -0800
+++ gdb/doc/gdb.texinfo	2007-11-12 11:22:18.000000000 -0800
@@ -3210,7 +3210,7 @@
 catch errors where you have no clue what part of your program is the
 culprit.)
 
-On some systems, such as HP-UX, @sc{gnu}/Linux and most other
+On some systems, such as HP-UX, PowerPC, @sc{gnu}/Linux and most other
 x86-based targets, @value{GDBN} includes support for hardware
 watchpoints, which do not slow down the running of your program.
 
@@ -3351,6 +3351,13 @@
 In multi-threaded programs, watchpoints will detect changes to the
 watched expression from every thread.
 
+@kindex watch thread thread_num
+@item watch @var{expr} thread thread_num
+Set a watchpoint that will break when @var{expr} is either read from
+or written into by the thread identified by @var{thread_num}. If @var{expr}
+is modified by any other threads not matching @var{thread_num}, @value{GDBN}
+will not break. Note that this will only work with Hardware Watchpoints.
+
 @quotation
 @emph{Warning:} In multi-threaded programs, software watchpoints
 have only limited usefulness.  If @value{GDBN} creates a software