This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [RFA] thread specific breakpoints and single stepping
- From: Emi SUZUKI <emi-suzuki at tjsys dot co dot jp>
- To: drow at false dot org
- Cc: pedro at codesourcery dot com, gdb-patches at sourceware dot org
- Date: Fri, 01 Aug 2008 20:58:49 +0900 (JST)
- Subject: Re: [RFA] thread specific breakpoints and single stepping
- References: <200807082105.02583.pedro@codesourcery.com> <20080709.211038.135588294.emi-suzuki@tjsys.co.jp> <20080726030133.GC1895@caradoc.them.org>
Hi Daniel,
thank you for your comment.
From: Daniel Jacobowitz <drow at false.org>
Subject: Re: [RFA] thread specific breakpoints and single stepping
Date: Fri, 25 Jul 2008 23:01:33 -0400
> On Wed, Jul 09, 2008 at 09:10:38PM +0900, Emi SUZUKI wrote:
> > 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.
>
> As this condition gets more complicated, I'm getting worried about
> keeping it in sync with everything else. Could it be that the logic
> is wrong - we should determine whether a thread hop is necessary later
> in the process? It should be just like a breakpoint with a false
> condition.
Ah, yes. I also feel like that I would eventually implement the same
procedures to two different places...
I investigated the condition again, and found that all the need for
thread hopping over a regular breakpoint can determine by calling
bpstat_stop_status as you suggested: even breakpoint_thread_match is
not necessary here.
So I revised the patch to delete it. handle_inferior_event was the
last caller of breakpoint_thread_match, but I just left it defined.
And I found another problem: when a hardware watchpoint trigger
occured while doing software single-stepping, the trigger can be
ignored by a thread hop.
I think it would be fixed by calling
single_step_breakpoint_inserted_here_p instead of checking
singlestep_breakpoints_inserted_p, to make sure a thread is really
trapped by a software single step breakpoint. But I don't have a
environment where I can reproduce the situation.
It may occur on a PPC box...?
And I doubt if the testcases are reasonable now, but at least the
tests fails without a patch, and succeeds with it, on x86-linux-gnu.
Is that OK?
--
Emi SUZUKI / emi-suzuki at tjsys.co.jp
gdb:
2008-08-01 Emi Suzuki <emi-suzuki@tjsys.co.jp>
Organize the check for the need of thread hopping.
* infrun.c (handle_inferior_event): Remove the call of
breakpoint_thread_match. Call single_step_breakpoint_inserted_here_p
instead of checking singlestep_breakpoint_inserted_p.
Update a comment.
* breakpoint.c (single_step_breakpoint_inserted_here_p): Make global.
Delete prototype.
* breakpoint.h (single_step_breakpoint_inserted_here_p): Declare.
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.301
diff -u -r1.301 infrun.c
--- gdb/infrun.c 22 Jul 2008 02:10:14 -0000 1.301
+++ gdb/infrun.c 1 Aug 2008 11:20:03 -0000
@@ -2241,8 +2241,8 @@
deferred_step_ptid = null_ptid;
}
- /* See if a thread hit a thread-specific breakpoint that was meant for
- another thread. If so, then step that thread past the breakpoint,
+ /* See if a thread hit a software single-step breakpoint that was set
+ on another thread. If so, then step that thread past the breakpoint,
and continue it. */
if (stop_signal == TARGET_SIGNAL_TRAP)
@@ -2252,13 +2252,8 @@
/* Check if a regular breakpoint has been hit before checking
for a potential single step breakpoint. Otherwise, GDB will
not see this breakpoint hit when stepping onto breakpoints. */
- if (regular_breakpoint_inserted_here_p (stop_pc))
- {
- ecs->random_signal = 0;
- if (!breakpoint_thread_match (stop_pc, ecs->ptid))
- thread_hop_needed = 1;
- }
- else if (singlestep_breakpoints_inserted_p)
+ if (!regular_breakpoint_inserted_here_p (stop_pc)
+ && single_step_breakpoint_inserted_here_p (stop_pc))
{
/* We have not context switched yet, so this should be true
no matter which thread hit the singlestep breakpoint. */
Index: gdb/breakpoint.c
===================================================================
RCS file: /cvs/src/src/gdb/breakpoint.c,v
retrieving revision 1.338
diff -u -r1.338 breakpoint.c
--- gdb/breakpoint.c 28 Jul 2008 17:53:52 -0000 1.338
+++ gdb/breakpoint.c 1 Aug 2008 11:19:37 -0000
@@ -180,8 +180,6 @@
static void ep_skip_leading_whitespace (char **s);
-static int single_step_breakpoint_inserted_here_p (CORE_ADDR pc);
-
static void free_bp_location (struct bp_location *loc);
static struct bp_location *
@@ -8185,7 +8183,7 @@
/* Check whether a software single-step breakpoint is inserted at PC. */
-static int
+int
single_step_breakpoint_inserted_here_p (CORE_ADDR pc)
{
int i;
Index: gdb/breakpoint.h
===================================================================
RCS file: /cvs/src/src/gdb/breakpoint.h,v
retrieving revision 1.76
diff -u -r1.76 breakpoint.h
--- gdb/breakpoint.h 25 Jul 2008 16:12:03 -0000 1.76
+++ gdb/breakpoint.h 1 Aug 2008 11:19:40 -0000
@@ -685,6 +685,8 @@
extern int software_breakpoint_inserted_here_p (CORE_ADDR);
+extern int single_step_breakpoint_inserted_here_p (CORE_ADDR pc);
+
extern int breakpoint_thread_match (CORE_ADDR, ptid_t);
extern void until_break_command (char *, int, int);
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
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);
+}