[PATCHv2 4/6] gdb: error if 'thread' or 'task' keywords are overused
Andrew Burgess
aburgess@redhat.com
Fri Feb 3 16:41:49 GMT 2023
Pedro Alves <pedro@palves.net> writes:
> On 2023-01-20 9:46 a.m., Andrew Burgess via Gdb-patches wrote:
>> 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.
>
> The patch looks fine, but does it make sense to allow both thread and task
> at the same time?
I don't know enough about Ada tasks to comment here. If someone who
knows can say categorically that threads and tasks can't coexist than
I'd be happy to add a patch to prevent them being used together.
Thanks,
Andrew
>
> For instance, with gdb.ada/tasks.exp :
>
> (gdb) b foo thread 1 task 2
> Breakpoint 1 at 0x555555557bd6: file /home/pedro/gdb/rocm/gdb/src/gdb/testsuite/gdb.ada/tasks/foo.adb, line 16.
> (gdb) info breakpoints
> Num Type Disp Enb Address What
> 1 breakpoint keep y 0x0000555555557bd6 in foo at /home/pedro/gdb/rocm/gdb/src/gdb/testsuite/gdb.ada/tasks/foo.adb:16 thread 1
> stop only in thread 1
>
> Pedro Alves
>
>>
>> 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"
>>
More information about the Gdb-patches
mailing list