Pedro Alves <pedro@palves.net> writes:
> On 2023-02-08 3:16 p.m., Andrew Burgess via Gdb-patches wrote:
>> Within the breakpoint struct we have two fields ::thread and ::task
>> which are used for thread or task specific breakpoints. When a
>> breakpoint doesn't have a specific thread or task then these fields
>> have the values -1 and 0 respectively.
>>
>> There's no particular reason (as far as I can tell) why these two
>> "default" values are different, and I find the difference a little
>> confusing. Long term I'd like to potentially fold these two fields
>> into a single field, but that isn't what this commit does.
>>
>> What this commit does is switch to using -1 as the "default" value for
>> both fields, this means that the default for breakpoint::task has
>> changed from 0 to -1. I've updated all the code I can find that
>> relied on the value of 0, and I see no test regressions, especially in
>> gdb.ada/tasks.exp, which still fully passes.
>>
>> There should be no user visible changes after this commit.
>
> This bothered me before as well. Thanks for doing this.
>
> Approved-By: Pedro Alves <pedro@palves.net>
I pushed this. Then realised I'd forgotten to update it to take account
of another patch that added a new comparison to 0, so I then pushed the
patch below as a fix.
Sorry for the temporary breakage.
Thanks,
Andrew
---
commit 8282ad74c302a8e0db7a588e500ae117a1df68c5
Author: Andrew Burgess <aburgess@redhat.com>
Date: Sun Feb 12 07:14:31 2023 +0000
gdb: fix describe_other_breakpoints for default task being -1
Commit:
commit 2ecee236752932672fb3d6cd63c6976927f747d8
CommitDate: Sun Feb 12 05:46:44 2023 +0000
gdb: use -1 for breakpoint::task default value
Failed to take account of an earlier commit:
commit f1f517e81039f6aa673b7d87a66bfbd25a66e3d3
CommitDate: Sat Feb 11 17:36:24 2023 +0000
gdb: show task number in describe_other_breakpoints
That both of these are my own commits is only more embarrassing.
This small fix updates describe_other_breakpoints to take account of
the default task number now being -1. This fixes regressions in
gdb.base/break.exp, gdb.base/break-always.exp, and many other tests.
diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index 4b0a909d60c..97dee5cd0fe 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -7057,7 +7057,7 @@ describe_other_breakpoints (struct gdbarch *gdbarch,
struct thread_info *thr = find_thread_global_id (b->thread);
gdb_printf (" (thread %s)", print_thread_id (thr));
}
- else if (b->task != 0)
+ else if (b->task != -1)
gdb_printf (" (task %d)", b->task);
gdb_printf ("%s%s ",
((b->enable_state == bp_disabled
@@ -1462,7 +1462,7 @@ breakpoint_set_thread (struct breakpoint *b, int thread)
/* It is invalid to set the thread field to anything other than -1 (which
means no thread restriction) if a task restriction is already in
place. */
- gdb_assert (thread == -1 || b->task == 0);
+ gdb_assert (thread == -1 || b->task == -1);
int old_thread = b->thread;
@@ -1476,10 +1476,10 @@ breakpoint_set_thread (struct breakpoint *b, int thread)
void
breakpoint_set_task (struct breakpoint *b, int task)
{
- /* It is invalid to set the task field to anything other than 0 (which
+ /* It is invalid to set the task field to anything other than -1 (which
means no task restriction) if a thread restriction is already in
place. */
- gdb_assert (task == 0 || b->thread == -1);
+ gdb_assert (task == -1 || b->thread == -1);
int old_task = b->task;
@@ -5473,7 +5473,7 @@ bpstat_check_breakpoint_conditions (bpstat *bs, thread_info *thread)
evaluating the condition if this isn't the specified
thread/task. */
if ((b->thread != -1 && b->thread != thread->global_num)
- || (b->task != 0 && b->task != ada_get_task_number (thread)))
+ || (b->task != -1 && b->task != ada_get_task_number (thread)))
{
infrun_debug_printf ("incorrect thread or task, not stopping");
bs->stop = false;
@@ -6487,7 +6487,7 @@ print_one_breakpoint_location (struct breakpoint *b,
{
if (b->thread != -1)
uiout->field_signed ("thread", b->thread);
- else if (b->task != 0)
+ else if (b->task != -1)
uiout->field_signed ("task", b->task);
}
@@ -6544,7 +6544,7 @@ print_one_breakpoint_location (struct breakpoint *b,
uiout->text ("\n");
}
- if (!part_of_multiple && b->task != 0)
+ if (!part_of_multiple && b->task != -1)
{
uiout->text ("\tstop only in task ");
uiout->field_signed ("task", b->task);
@@ -8447,7 +8447,7 @@ code_breakpoint::code_breakpoint (struct gdbarch *gdbarch_,
gdb_assert (!sals.empty ());
/* At most one of thread or task can be set on any breakpoint. */
- gdb_assert (thread == -1 || task == 0);
+ gdb_assert (thread == -1 || task == -1);
thread = thread_;
task = task_;
@@ -8763,7 +8763,7 @@ find_condition_and_thread (const char *tok, CORE_ADDR pc,
{
cond_string->reset ();
*thread = -1;
- *task = 0;
+ *task = -1;
rest->reset ();
bool force = false;
@@ -8816,7 +8816,7 @@ find_condition_and_thread (const char *tok, CORE_ADDR pc,
if (*thread != -1)
error(_("You can specify only one thread."));
- if (*task != 0)
+ if (*task != -1)
error (_("You can specify only one of thread or task."));
tok = end_tok + 1;
@@ -8830,7 +8830,7 @@ find_condition_and_thread (const char *tok, CORE_ADDR pc,
{
char *tmptok;
- if (*task != 0)
+ if (*task != -1)
error(_("You can specify only one task."));
if (*thread != -1)
@@ -8871,7 +8871,7 @@ find_condition_and_thread_for_sals (const std::vector<symtab_and_line> &sals,
{
gdb::unique_xmalloc_ptr<char> cond;
int thread_id = -1;
- int task_id = 0;
+ int task_id = -1;
gdb::unique_xmalloc_ptr<char> remaining;
/* Here we want to parse 'arg' to separate condition from thread
@@ -8886,7 +8886,7 @@ find_condition_and_thread_for_sals (const std::vector<symtab_and_line> &sals,
&task_id, &remaining);
*cond_string = std::move (cond);
/* At most one of thread or task can be set. */
- gdb_assert (thread_id == -1 || task_id == 0);
+ gdb_assert (thread_id == -1 || task_id == -1);
*thread = thread_id;
*task = task_id;
*rest = std::move (remaining);
@@ -8988,7 +8988,7 @@ create_breakpoint (struct gdbarch *gdbarch,
{
struct linespec_result canonical;
bool pending = false;
- int task = 0;
+ int task = -1;
int prev_bkpt_count = breakpoint_count;
gdb_assert (ops != NULL);
@@ -10060,7 +10060,7 @@ watch_command_1 (const char *arg, int accessflag, int from_tty,
the hardware watchpoint. */
bool use_mask = false;
CORE_ADDR mask = 0;
- int task = 0;
+ int task = -1;
/* Make sure that we actually have parameters to parse. */
if (arg != NULL && arg[0] != '\0')
@@ -10107,7 +10107,7 @@ watch_command_1 (const char *arg, int accessflag, int from_tty,
if (thread != -1)
error(_("You can specify only one thread."));
- if (task != 0)
+ if (task != -1)
error (_("You can specify only one of thread or task."));
/* Extract the thread ID from the next token. */
@@ -10123,7 +10123,7 @@ watch_command_1 (const char *arg, int accessflag, int from_tty,
{
char *tmp;
- if (task != 0)
+ if (task != -1)
error(_("You can specify only one task."));
if (thread != -1)
@@ -10305,7 +10305,7 @@ watch_command_1 (const char *arg, int accessflag, int from_tty,
w.reset (new watchpoint (nullptr, bp_type));
/* At most one of thread or task can be set on a watchpoint. */
- gdb_assert (thread == -1 || task == 0);
+ gdb_assert (thread == -1 || task == -1);
w->thread = thread;
w->task = task;
w->disposition = disp_donttouch;
@@ -14143,7 +14143,7 @@ breakpoint::print_recreate_thread (struct ui_file *fp) const
if (thread != -1)
gdb_printf (fp, " thread %d", thread);
- if (task != 0)
+ if (task != -1)
gdb_printf (fp, " task %d", task);
gdb_printf (fp, "\n");
@@ -802,9 +802,9 @@ struct breakpoint
care. */
int thread = -1;
- /* Ada task number for task-specific breakpoint, or 0 if don't
+ /* Ada task number for task-specific breakpoint, or -1 if don't
care. */
- int task = 0;
+ int task = -1;
/* Count of the number of times this breakpoint was taken, dumped
with the info, but not used for anything else. Useful for seeing
@@ -1680,9 +1680,9 @@ extern void breakpoint_set_silent (struct breakpoint *b, int silent);
extern void breakpoint_set_thread (struct breakpoint *b, int thread);
-/* Set the task for this breakpoint. If TASK is 0, make the breakpoint
- work for any task. Passing a value other than 0 for TASK should only be
- done if b->thread is -1; it is not valid to try and set both a thread
+/* Set the task for this breakpoint. If TASK is -1, make the breakpoint
+ work for any task. Passing a value other than -1 for TASK should only
+ be done if b->thread is -1; it is not valid to try and set both a thread
and task restriction on a breakpoint. */
extern void breakpoint_set_task (struct breakpoint *b, int task);
@@ -774,7 +774,7 @@ gdbscm_set_breakpoint_thread_x (SCM self, SCM newvalue)
_("invalid thread id"));
}
- if (bp_smob->bp->task != 0)
+ if (bp_smob->bp->task != -1)
scm_misc_error (FUNC_NAME,
_("cannot set both task and thread attributes"),
SCM_EOL);
@@ -797,7 +797,7 @@ gdbscm_breakpoint_task (SCM self)
breakpoint_smob *bp_smob
= bpscm_get_valid_breakpoint_smob_arg_unsafe (self, SCM_ARG1, FUNC_NAME);
- if (bp_smob->bp->task == 0)
+ if (bp_smob->bp->task == -1)
return SCM_BOOL_F;
return scm_from_long (bp_smob->bp->task);
@@ -840,7 +840,7 @@ gdbscm_set_breakpoint_task_x (SCM self, SCM newvalue)
SCM_EOL);
}
else if (gdbscm_is_false (newvalue))
- id = 0;
+ id = -1;
else
SCM_ASSERT_TYPE (0, newvalue, SCM_ARG2, FUNC_NAME, _("integer or #f"));
@@ -271,7 +271,7 @@ bppy_set_thread (PyObject *self, PyObject *newvalue, void *closure)
return -1;
}
- if (self_bp->bp->task != 0)
+ if (self_bp->bp->task != -1)
{
PyErr_SetString (PyExc_RuntimeError,
_("Cannot set both task and thread attributes."));
@@ -337,7 +337,7 @@ bppy_set_task (PyObject *self, PyObject *newvalue, void *closure)
}
}
else if (newvalue == Py_None)
- id = 0;
+ id = -1;
else
{
PyErr_SetString (PyExc_TypeError,
@@ -711,7 +711,7 @@ bppy_get_task (PyObject *self, void *closure)
BPPY_REQUIRE_VALID (self_bp);
- if (self_bp->bp->task == 0)
+ if (self_bp->bp->task == -1)
Py_RETURN_NONE;
return gdb_py_object_from_longest (self_bp->bp->task).release ();