Re: [RFA] thread specific breakpoints and single stepping

[Emi SUZUKI <emi-suzuki@tjsys.co.jp>]

[-- Attachment #1: Type: Text/Plain, Size: 885 bytes --]

Hello Pedro, 

thank you for your review.  

From: Pedro Alves <pedro at codesourcery.com>
Subject: Re: [RFA] thread specific breakpoints and single stepping
Date: Tue, 08 Jul 2008 21:05:02 +0100

> I don't think that's correct.  You can't be sure inferior_ptid
> is the user stepping thread at this point.  GDB may have context-switched
> already due to another event having happened in another thread after
> the user started the step, and before reaching here.  

I see.  I could also reproduce some cases you suggested.  

> Does it work for you?

It works better than mine.  But while I am concerning about
single-stepping for software watchpoints, I noticed that we should
also check whether a hardware watchpoint is triggered.  

A revised patch is attached, which includes tests and ChangeLog
entries.  

Is that OK?

Thanks again, 
-- 
Emi SUZUKI / emi-suzuki at tjsys.co.jp


[-- Attachment #2: thread_specific_break_and_step2.diff --]
[-- Type: Text/X-Diff, Size: 6337 bytes --]

gdb:

2008-07-09  Pedro Alves  <pedro@codesourcery.com>
	    Emi Suzuki  <emi-suzuki@tjsys.co.jp>

	* infrun.c (handle_inferior_event): Check if the thread is
	neither single-stepping nor trapped by a hardware watchpoint
	trigger before hopping over a thread specific breakpoint.  

gdb/testsuite:

2008-07-09  Emi Suzuki  <emi-suzuki@tjsys.co.jp>

	* gdb.threads/thread-specific2.exp,
	gdb.threads/thread-specific2.c: New tests. 

Index: gdb/infrun.c
===================================================================
RCS file: /cvs/src/src/gdb/infrun.c,v
retrieving revision 1.285
diff -u -r1.285 infrun.c
--- gdb/infrun.c	8 Jul 2008 10:31:16 -0000	1.285
+++ gdb/infrun.c	9 Jul 2008 12:05:05 -0000
@@ -2122,7 +2122,19 @@
 	{
 	  ecs->random_signal = 0;
 	  if (!breakpoint_thread_match (stop_pc, ecs->ptid))
-	    thread_hop_needed = 1;
+	    {
+	      if (!ptid_equal (inferior_ptid, ecs->ptid))
+		context_switch (ecs);
+
+	      /* If the thread is currently single-stepping, whether it 
+		 will step over this breakpoint or not should be determined
+		 later.  Also, it should stop when a hardware watchpoint 
+		 trap is triggered.  */
+	      if (!((step_range_end && step_resume_breakpoint == NULL)
+		    || bpstat_should_step ()
+		    || STOPPED_BY_WATCHPOINT (&ecs->ws)))
+		thread_hop_needed = 1;
+	    }
 	}
       else if (singlestep_breakpoints_inserted_p)
 	{
Index: gdb/testsuite/gdb.threads/thread-specific2.c
===================================================================
RCS file: gdb/testsuite/gdb.threads/thread-specific2.c
diff -N gdb/testsuite/gdb.threads/thread-specific2.c
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ gdb/testsuite/gdb.threads/thread-specific2.c	1 Jan 1970 00:00:00 -0000
@@ -0,0 +1,45 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2008 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/>.  */
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <pthread.h>
+
+void *
+thread_entry (void* args)
+{
+  int i = 0;
+  i += 1;  /* thread-specific2.exp: hop over 1 */
+  i -= 1;  /* thread-specific2.exp: hop over 2 */
+
+  pthread_exit(NULL);
+}
+
+int
+main (void)
+{
+  int res;
+  pthread_t tid;
+  void *thread_status;
+
+  res = pthread_create (&tid, NULL, (void *) thread_entry, NULL);
+
+  /* wait for the thread */
+  pthread_join (tid, &thread_status);
+
+  exit (EXIT_SUCCESS);
+}
Index: gdb/testsuite/gdb.threads/thread-specific2.exp
===================================================================
RCS file: gdb/testsuite/gdb.threads/thread-specific2.exp
diff -N gdb/testsuite/gdb.threads/thread-specific2.exp
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ gdb/testsuite/gdb.threads/thread-specific2.exp	1 Jan 1970 00:00:00 -0000
@@ -0,0 +1,97 @@
+# Copyright 2008 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/>.
+
+# It tests that a thread-specific breakpoint would not be hopped when 
+# the trapped thread is currently single-stepping or a watchpoint 
+# triggered on the breakpoint.  
+
+if $tracelevel then {
+    strace $tracelevel
+}
+
+set prms_id 0
+set bug_id 0
+
+set testfile "thread-specific2"
+set srcfile ${testfile}.c
+set binfile ${objdir}/${subdir}/${testfile}
+
+if {[gdb_compile_pthreads "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable [list debug "incdir=${objdir}"]] != "" } {
+    return -1
+}
+
+# Start with a fresh gdb.
+
+gdb_exit
+gdb_start
+gdb_reinitialize_dir $srcdir/$subdir
+
+gdb_load ${binfile}
+
+gdb_test "set print sevenbit-strings" ""
+gdb_test "set width 0" ""
+
+if { ! [ runto "thread_entry" ] } then {
+    untested thread-specific2.exp
+    return -1
+}
+
+# Test 1: the target will get stopped on a thread specific breakpoint
+# when it is singile-stepping.  
+set line1 [gdb_get_line_number "thread-specific2.exp: hop over 1"]
+
+gdb_test_multiple "break $line1 thread 1" \
+    "breakpoint $line1 main thread" {
+    -re "Breakpoint (\[0-9\]*) at.* file .*$srcfile, line.*$gdb_prompt $" {
+	set thread_break1 $expect_out(1,string)
+	pass "breakpoint $line1 main thread"
+    }
+}
+
+gdb_test_multiple "step" \
+    "step onto thread-specific breakpoint" {
+    -re "$line1 *.*\r\n$gdb_prompt $" {
+	pass "step onto breakpoint $thread_break1"
+    }
+}
+
+# Test 2: the target will get stopped on a thread specific breakpoint
+# when a watchpoint triggered.  
+set line2 [gdb_get_line_number "thread-specific2.exp: hop over 2"]
+
+gdb_test_multiple "break $line2 thread 1" \
+    "breakpoint $line2 main thread" {
+    -re "Breakpoint (\[0-9\]*) at.* file .*$srcfile, line.*$gdb_prompt $" {
+	set thread_break2 $expect_out(1,string)
+	pass "breakpoint $line2 main thread"
+    }
+}
+
+gdb_test_multiple "watch i" \
+    "watch i" {
+      -re "\[Ww\]atchpoint (\[0-9\]*): i\r\n$gdb_prompt $" {
+      set watch_num $expect_out(1,string)
+      pass "watch i"
+    }
+}
+
+gdb_test_multiple "continue" \
+    "watchpoint $watch_num triggered on thread-specific breakpoint" {
+    -re "\[Ww\]atchpoint $watch_num: i\r\n.*\r\n$gdb_prompt $" {
+	pass "watchpoint $watch_num triggered on breakpoint $thread_break2"
+    }
+}
+
+return 0