public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
From: Andrew Burgess <aburgess@redhat.com>
To: gdb-patches@sourceware.org
Cc: Andrew Burgess <aburgess@redhat.com>
Subject: [PATCHv2 4/6] gdb: error if 'thread' or 'task' keywords are overused
Date: Fri, 20 Jan 2023 09:46:27 +0000	[thread overview]
Message-ID: <d8fdb97bb59b7f74846906995ba0d1f1d9d2385c.1674207665.git.aburgess@redhat.com> (raw)
In-Reply-To: <cover.1674207665.git.aburgess@redhat.com>

When creating a breakpoint or watchpoint, the 'thread' and 'task'
keywords can be used to create a thread or task specific breakpoint or
watchpoint.

Currently, a thread or task specific breakpoint can only apply for a
single thread or task, if multiple threads or tasks are specified when
creating the breakpoint (or watchpoint), then the last specified id
will be used.

The exception to the above is that when the 'thread' keyword is used
during the creation of a watchpoint, GDB will give an error if
'thread' is given more than once.

In this commit I propose making this behaviour consistent, if the
'thread' or 'task' keywords are used more than once when creating
either a breakpoint or watchpoint, then GDB will give an error.

I haven't updated the manual, we don't explicitly say that these
keywords can be repeated, and (to me), given the keyword takes a
single id, I don't think it makes much sense to repeat the keyword.
As such, I see this more as adding a missing error to GDB, rather than
making some big change.  However, I have added an entry to the NEWS
file as I guess it is possible that some people might hit this new
error with an existing (I claim, badly written) GDB script.

I've added some new tests to check for the new error.

Just one test needed updating, gdb.linespec/keywords.exp, this test
did use the 'thread' keyword twice, and expected the breakpoint to be
created.  Looking at what this test was for though, it was checking
the use of '-force-condition', and I don't think that being able to
repeat 'thread' was actually a critical part of this test.

As such, I've updated this test to expect the error when 'thread' is
repeated.
---
 gdb/NEWS                                         |  9 +++++++++
 gdb/breakpoint.c                                 |  9 +++++++++
 gdb/testsuite/gdb.ada/tasks.exp                  |  4 ++++
 gdb/testsuite/gdb.linespec/keywords.exp          | 10 ++++++++--
 gdb/testsuite/gdb.threads/thread-specific-bp.exp |  4 ++++
 gdb/testsuite/gdb.threads/watchthreads2.exp      |  3 +++
 6 files changed, 37 insertions(+), 2 deletions(-)

diff --git a/gdb/NEWS b/gdb/NEWS
index c0aac212e30..fb49f62f7e6 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -9,6 +9,15 @@
   This support requires that GDB be built with Python scripting
   enabled.
 
+* For the break command, multiple uses of the 'thread' or 'task'
+  keywords will now give an error instead of just using the thread or
+  task id from the last instance of the keyword.
+
+* For the watch command, multiple uses of the 'task' keyword will now
+  give an error instead of just using the task id from the last
+  instance of the keyword.  The 'thread' keyword already gave an error
+  when used multiple times with the watch command, this remains unchanged.
+
 * New commands
 
 maintenance print record-instruction [ N ]
diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index b2cd89511fb..1400fd49642 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -8801,6 +8801,9 @@ find_condition_and_thread (const char *tok, CORE_ADDR pc,
 	  const char *tmptok;
 	  struct thread_info *thr;
 
+	  if (*thread != -1)
+	    error(_("You can specify only one thread."));
+
 	  tok = end_tok + 1;
 	  thr = parse_thread_id (tok, &tmptok);
 	  if (tok == tmptok)
@@ -8812,6 +8815,9 @@ find_condition_and_thread (const char *tok, CORE_ADDR pc,
 	{
 	  char *tmptok;
 
+	  if (*task != 0)
+	    error(_("You can specify only one task."));
+
 	  tok = end_tok + 1;
 	  *task = strtol (tok, &tmptok, 0);
 	  if (tok == tmptok)
@@ -10094,6 +10100,9 @@ watch_command_1 (const char *arg, int accessflag, int from_tty,
 	    {
 	      char *tmp;
 
+	      if (task != 0)
+		error(_("You can specify only one task."));
+
 	      task = strtol (value_start, &tmp, 0);
 	      if (tmp == value_start)
 		error (_("Junk after task keyword."));
diff --git a/gdb/testsuite/gdb.ada/tasks.exp b/gdb/testsuite/gdb.ada/tasks.exp
index 23bf3937a20..4441d92719c 100644
--- a/gdb/testsuite/gdb.ada/tasks.exp
+++ b/gdb/testsuite/gdb.ada/tasks.exp
@@ -39,6 +39,10 @@ gdb_test "info tasks" \
                "\r\n"] \
          "info tasks before inserting breakpoint"
 
+# Check that multiple uses of the 'task' keyword will give an error.
+gdb_test "break break_me task 1 task 3" "You can specify only one task\\."
+gdb_test "watch j task 1 task 3" "You can specify only one task\\."
+
 # Insert a breakpoint that should stop only if task 1 stops.  Since
 # task 1 never calls break_me, this shouldn't actually ever trigger.
 # The fact that this breakpoint is created _before_ the next one
diff --git a/gdb/testsuite/gdb.linespec/keywords.exp b/gdb/testsuite/gdb.linespec/keywords.exp
index bff64249542..dc66e32237c 100644
--- a/gdb/testsuite/gdb.linespec/keywords.exp
+++ b/gdb/testsuite/gdb.linespec/keywords.exp
@@ -80,8 +80,14 @@ foreach prefix {"" "thread 1 "} {
     foreach suffix {"" " " " thread 1"} {
 	foreach cond {"" " if 1"} {
 	    with_test_prefix "prefix: '$prefix', suffix: '$suffix', cond: '$cond'" {
-		gdb_breakpoint "main ${prefix}-force-condition${suffix}${cond}"\
-		    "message"
+
+		if { [regexp thread $prefix] && [regexp thread $suffix] } {
+		    gdb_test "break main ${prefix}-force-condition${suffix}${cond}" \
+			"You can specify only one thread\\."
+		} else {
+		    gdb_breakpoint "main ${prefix}-force-condition${suffix}${cond}"\
+			"message"
+		}
 	    }
 	}
     }
diff --git a/gdb/testsuite/gdb.threads/thread-specific-bp.exp b/gdb/testsuite/gdb.threads/thread-specific-bp.exp
index d33b4f47258..008ae5ed05e 100644
--- a/gdb/testsuite/gdb.threads/thread-specific-bp.exp
+++ b/gdb/testsuite/gdb.threads/thread-specific-bp.exp
@@ -63,6 +63,10 @@ proc check_thread_specific_breakpoint {non_stop} {
 	return -1
     }
 
+    # Check that multiple uses of 'thread' keyword give an error.
+    gdb_test "break main thread $start_thre thread $main_thre" \
+	"You can specify only one thread\\."
+
     # Set a thread-specific breakpoint at "main".  This can't ever
     # be hit, but that's OK.
     gdb_breakpoint "main thread $start_thre"
diff --git a/gdb/testsuite/gdb.threads/watchthreads2.exp b/gdb/testsuite/gdb.threads/watchthreads2.exp
index 09858aee486..a1398d668a4 100644
--- a/gdb/testsuite/gdb.threads/watchthreads2.exp
+++ b/gdb/testsuite/gdb.threads/watchthreads2.exp
@@ -71,6 +71,9 @@ if { $nr_started == $NR_THREADS } {
     return -1
 }
 
+# Check that multiple uses of the 'thread' keyword will give an error.
+gdb_test "watch x thread 1 thread 2" "You can specify only one thread\\."
+
 # Watch X, it will be modified by all threads.
 # We want this watchpoint to be set *after* all threads are running.
 gdb_test "watch x" "Hardware watchpoint 3: x"
-- 
2.25.4


  parent reply	other threads:[~2023-01-20  9:46 UTC|newest]

Thread overview: 54+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-11-28 11:25 [PATCH 0/6] Inferior specific breakpoints Andrew Burgess
2022-11-28 11:25 ` [PATCH 1/6] gdb/remote: announce thread exit events for remote targets Andrew Burgess
2022-11-28 11:25 ` [PATCH 2/6] gdb/testsuite: don't try to set non-stop mode on a running target Andrew Burgess
2022-11-28 11:25 ` [PATCH 3/6] gdb: fix display of thread condition for multi-location breakpoints Andrew Burgess
2022-12-23  8:37   ` Aktemur, Tankut Baris
2022-11-28 11:25 ` [PATCH 4/6] gdb: error if 'thread' or 'task' keywords are overused Andrew Burgess
2022-11-28 13:10   ` Eli Zaretskii
2022-11-28 11:25 ` [PATCH 5/6] gdb: add inferior-specific breakpoints and watchpoints Andrew Burgess
2022-11-28 13:18   ` Eli Zaretskii
2022-12-23 10:05   ` Aktemur, Tankut Baris
2023-01-19 19:13     ` Andrew Burgess
2023-01-20 13:12       ` Aktemur, Tankut Baris
2022-11-28 11:25 ` [PATCH 6/6] gdb: convert the 'start' breakpoint to use inferior keyword Andrew Burgess
2022-12-23 10:17   ` Aktemur, Tankut Baris
2022-12-23 10:55 ` [PATCH 0/6] Inferior specific breakpoints Aktemur, Tankut Baris
2023-01-20  9:46 ` [PATCHv2 " Andrew Burgess
2023-01-20  9:46   ` [PATCHv2 1/6] gdb/remote: announce thread exit events for remote targets Andrew Burgess
2023-02-02 17:50     ` Pedro Alves
2023-02-04 15:46       ` Andrew Burgess
2023-01-20  9:46   ` [PATCHv2 2/6] gdb/testsuite: don't try to set non-stop mode on a running target Andrew Burgess
2023-02-04 16:22     ` Andrew Burgess
2023-01-20  9:46   ` [PATCHv2 3/6] gdb: fix display of thread condition for multi-location breakpoints Andrew Burgess
2023-02-02 18:13     ` Pedro Alves
2023-02-06 14:48       ` Andrew Burgess
2023-02-06 17:01         ` Pedro Alves
2023-02-07 14:42           ` Andrew Burgess
2023-01-20  9:46   ` Andrew Burgess [this message]
2023-01-20 13:22     ` [PATCHv2 4/6] gdb: error if 'thread' or 'task' keywords are overused Eli Zaretskii
2023-02-02 14:08       ` Andrew Burgess
2023-02-02 14:31         ` Eli Zaretskii
2023-02-02 18:21     ` Pedro Alves
2023-02-03 16:41       ` Andrew Burgess
2023-02-04  5:52         ` Joel Brobecker
2023-02-04 15:40           ` Andrew Burgess
2023-02-06 11:06       ` Andrew Burgess
2023-01-20  9:46   ` [PATCHv2 5/6] gdb: add inferior-specific breakpoints and watchpoints Andrew Burgess
2023-01-20 13:25     ` Eli Zaretskii
2023-02-02 19:17     ` Pedro Alves
2023-02-03 16:55       ` Andrew Burgess
2023-02-06 17:24         ` Pedro Alves
2023-02-16 12:56     ` Aktemur, Tankut Baris
2023-01-20  9:46   ` [PATCHv2 6/6] gdb: convert the 'start' breakpoint to use inferior keyword Andrew Burgess
2023-02-16 12:59     ` Aktemur, Tankut Baris
2023-03-16 17:03   ` [PATCHv3 0/2] Inferior specific breakpoints Andrew Burgess
2023-03-16 17:03     ` [PATCHv3 1/2] gdb: cleanup around some set_momentary_breakpoint_at_pc calls Andrew Burgess
2023-04-03 14:12       ` Andrew Burgess
2023-03-16 17:03     ` [PATCHv3 2/2] gdb: add inferior-specific breakpoints Andrew Burgess
2023-04-03 14:14     ` [PATCHv4] " Andrew Burgess
2023-05-15 19:15       ` [PATCHv5] " Andrew Burgess
2023-05-30 20:41         ` [PATCHv6] " Andrew Burgess
2023-07-07 10:23           ` [PATCHv7] " Andrew Burgess
2023-08-17 15:53             ` [PUSHEDv8] " Andrew Burgess
2023-08-23  8:06               ` [PUSHED] gdb: add missing notify_breakpoint_modified call Andrew Burgess
2023-08-23  8:19               ` [PUSHED] gdb/testsuite: improve MI support for inferior specific breakpoints Andrew Burgess

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=d8fdb97bb59b7f74846906995ba0d1f1d9d2385c.1674207665.git.aburgess@redhat.com \
    --to=aburgess@redhat.com \
    --cc=gdb-patches@sourceware.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).