[RFC] Add task-specific breakpoint capability...

[RFC] Add task-specific breakpoint capability...

[Joel Brobecker]

[-- Attachment #1: Type: text/plain, Size: 2960 bytes --]

Hello,

This patch is to complete the addition of (Ada) tasking support
in GDB which AdaCore contributed a few months ago.  The idea is
to allow a user to stop on a breakpoint only if the task that
triggered the breakpoint corresponds to a specific task, identified
by its task ID (from the "info tasks" command).  This is very
similar to the thread-specific breakpoints both in implementation
and user interface.

Here is what it looks like from the CLI:

    (gdb) info tasks
      ID       TID P-ID Pri State                  Name
    *  1    640010    0  48 Running                main_task
       2    640d00    1  48 Accept or Select Term  keyboard1
       3    644320    1  48 Accept or Select Term  keyboard2

Task IDs are on the left hand side. So insert a breakpoint that stops
only when task 2 hits it, the user would use the "task" keyword
after the breakpoint location:

    (gdb) break tasks.adb:13 task 2
    Breakpoint 3 at 0x403195: file tasks.adb, line 13. (2 locations)

Resuming the execution should stop on our breakpoint iff task 2
hits it. This is verified by doing another "info tasks" after
hitting the breakpoint:

    (gdb) continue
    [...]
    (gdb) info tasks
      ID       TID P-ID Pri State                  Name
       1    640010    0  48 Waiting on RV with 2   main_task
    *  2    640d00    1  48 Accepting RV with 1    keyboard1
       3    644320    1  48 Accept or Select Term  keyboard2

The "*" marker shows that the task on which we stopped is indeed
task 2 :).

The reason why I'm posting this as an RFC is that, although
the implementation is pretty straightforward, for some reason,
I find it a bit inelegant. Having two integers which are pretty
much mutually exclusive sounds pretty bad to me. But, on the other
hand, I didn't really find anything that is that much better.

One possibility that I envisioned was to avoid storing the thread/
task ID, but store the associated ptid instead. Then, the part
of the code that checks whether the right thread/task can be merged
together. But then we need an extra enum that allows us to remember
whether the breakpoint was task-specific, or thread-specific. We need
that piece of information when displaying the list of breakopints:

    (gdb) info breakpoints 
    Num     Type           Disp Enb Address            What
    2       breakpoint     keep y   0x0000000000403472 in task_switch.break_me
                                                   at task_switch.adb:43 task 3

(we print "task TASK_NO" in the "what" column).

I'm not completely sure this is worth it or not. For now, here is
the code as we've implemented many many moons ago at AdaCore. It
gets the job done, and I don't feel like it's too invasive. If
you guys think the idea above, or any other suggestion, is worth
a shot, let me know.

Once we converge on a solution, I'll provide doc updates, a testcase,
and we'll also probably need to look at extending MI.

Thoughts?

Thanks,
-- 
Joel

[-- Attachment #2: task-specific-breakpoint.diff --]
[-- Type: text/x-diff, Size: 7678 bytes --]

commit df79a13713ea3a6946b2cf0659cc9cbe22beb445
Author: Joel Brobecker <brobecker@adacore.com>
Date:   Tue Mar 24 11:24:49 2009 -0700

        Provide support for (Ada) task-specific breakpoints.
    
        * ada-lang.h (ada_get_task_number): Add declaration.
        * ada-tasks.c (ada_get_task_number): Make non-static.
        * breakpoint.h (struct breakpoint): Add field "task".
        * breakpoint.c (breakpoint_ada_task_match): New function.
        (print_one_breakpoint_location): Add handling of task-specific
        breakpoints.
        (create_breakpoint, create_breakpoints, find_condition_and_thread):
        New parameter "task".
        (break_command_really): Update calls to find_condition_and_thread
        and create_breakpoints.
        (breakpoint_re_set_one): Update call to find_condition_and_thread.
        Set b->task.

diff --git a/gdb/ada-lang.h b/gdb/ada-lang.h
index 88b6c16..4e70b7d 100644
--- a/gdb/ada-lang.h
+++ b/gdb/ada-lang.h
@@ -461,6 +461,8 @@ extern char *ada_main_name (void);
 
 extern int valid_task_id (int);
 
+extern int ada_get_task_number (ptid_t);
+
 extern void ada_adjust_exception_stop (bpstat bs);
 
 extern void ada_print_exception_stop (bpstat bs);
diff --git a/gdb/ada-tasks.c b/gdb/ada-tasks.c
index d0ce5ab..4f0aaf5 100644
--- a/gdb/ada-tasks.c
+++ b/gdb/ada-tasks.c
@@ -160,7 +160,7 @@ static int stale_task_list_p = 1;
 /* Return the task number of the task whose ptid is PTID, or zero
    if the task could not be found.  */
 
-static int
+int
 ada_get_task_number (ptid_t ptid)
 {
   int i;
diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index 7e50342..5114ae7 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -1919,6 +1919,32 @@ breakpoint_thread_match (CORE_ADDR pc, ptid_t ptid)
 
   return 0;
 }
+
+/* Return true if the breakpoint at PC is valid for the Ada task identified
+   by its PTID.  */
+
+int
+breakpoint_ada_task_match (CORE_ADDR pc, ptid_t ptid)
+{
+  struct bp_location *bpt;
+  int taskid = ada_get_task_number (ptid);
+
+  ALL_BP_LOCATIONS (bpt)
+    {
+      if ((bpt->loc_type == bp_loc_software_breakpoint
+	   || bpt->loc_type == bp_loc_hardware_breakpoint)
+	  && (bpt->owner->enable_state == bp_enabled
+	      || bpt->owner->enable_state == bp_permanent)
+	  && bpt->address == pc
+	  && (bpt->owner->task == 0 || bpt->owner->task == taskid))
+      {
+	return 1;
+      }
+    }
+
+  return 0;
+}
+
 \f
 
 /* bpstat stuff.  External routines' interfaces are documented
@@ -3556,12 +3582,20 @@ print_one_breakpoint_location (struct breakpoint *b,
 	break;
       }
 
-  if (!part_of_multiple && b->thread != -1)
+  if (!part_of_multiple)
     {
-      /* FIXME: This seems to be redundant and lost here; see the
-	 "stop only in" line a little further down. */
-      ui_out_text (uiout, " thread ");
-      ui_out_field_int (uiout, "thread", b->thread);
+      if (b->thread != -1)
+	{
+	  /* FIXME: This seems to be redundant and lost here; see the
+	     "stop only in" line a little further down. */
+	  ui_out_text (uiout, " thread ");
+	  ui_out_field_int (uiout, "thread", b->thread);
+	}
+      else if (b->task != 0)
+	{
+	  ui_out_text (uiout, " task ");
+	  ui_out_field_int (uiout, "task", b->task);
+	}
     }
   
   ui_out_text (uiout, "\n");
@@ -5113,7 +5147,7 @@ static void
 create_breakpoint (struct symtabs_and_lines sals, char *addr_string,
 		   char *cond_string,
 		   enum bptype type, enum bpdisp disposition,
-		   int thread, int ignore_count, 
+		   int thread, int task, int ignore_count, 
 		   struct breakpoint_ops *ops, int from_tty, int enabled)
 {
   struct breakpoint *b = NULL;
@@ -5145,6 +5179,7 @@ create_breakpoint (struct symtabs_and_lines sals, char *addr_string,
 	  set_breakpoint_count (breakpoint_count + 1);
 	  b->number = breakpoint_count;
 	  b->thread = thread;
+	  b->task = task;
   
 	  b->cond_string = cond_string;
 	  b->ignore_count = ignore_count;
@@ -5323,7 +5358,7 @@ static void
 create_breakpoints (struct symtabs_and_lines sals, char **addr_string,
 		    char *cond_string,
 		    enum bptype type, enum bpdisp disposition,
-		    int thread, int ignore_count, 
+		    int thread, int task, int ignore_count, 
 		    struct breakpoint_ops *ops, int from_tty,
 		    int enabled)
 {
@@ -5335,7 +5370,7 @@ create_breakpoints (struct symtabs_and_lines sals, char **addr_string,
 
       create_breakpoint (expanded, addr_string[i],
 			 cond_string, type, disposition,
-			 thread, ignore_count, ops, from_tty, enabled);
+			 thread, task, ignore_count, ops, from_tty, enabled);
     }
 
   update_global_location_list (1);
@@ -5442,7 +5477,7 @@ do_captured_parse_breakpoint (struct ui_out *ui, void *data)
    If no thread is found, *THREAD is set to -1.  */
 static void 
 find_condition_and_thread (char *tok, CORE_ADDR pc, 
-			   char **cond_string, int *thread)
+			   char **cond_string, int *thread, int *task)
 {
   *cond_string = NULL;
   *thread = -1;
@@ -5485,6 +5520,18 @@ find_condition_and_thread (char *tok, CORE_ADDR pc,
 	  if (!valid_thread_id (*thread))
 	    error (_("Unknown thread %d."), *thread);
 	}
+      else if (toklen >= 1 && strncmp (tok, "task", toklen) == 0)
+	{
+	  char *tmptok;
+
+	  tok = end_tok + 1;
+	  tmptok = tok;
+	  *task = strtol (tok, &tok, 0);
+	  if (tok == tmptok)
+	    error (_("Junk after task keyword."));
+	  if (!valid_task_id (*task))
+	    error (_("Unknown task %d\n"), *task);
+	}
       else
 	error (_("Junk at end of arguments."));
     }
@@ -5523,6 +5570,7 @@ break_command_really (char *arg, char *cond_string, int thread,
   int i;
   int pending = 0;
   int not_found = 0;
+  int task = 0;
 
   sals.sals = NULL;
   sals.nelts = 0;
@@ -5624,7 +5672,8 @@ break_command_really (char *arg, char *cond_string, int thread,
                re-parse it in context of each sal.  */
             cond_string = NULL;
             thread = -1;
-            find_condition_and_thread (arg, sals.sals[0].pc, &cond_string, &thread);
+            find_condition_and_thread (arg, sals.sals[0].pc, &cond_string,
+                                       &thread, &task);
             if (cond_string)
                 make_cleanup (xfree, cond_string);
         }
@@ -5641,7 +5690,7 @@ break_command_really (char *arg, char *cond_string, int thread,
 			  hardwareflag ? bp_hardware_breakpoint 
 			  : bp_breakpoint,
 			  tempflag ? disp_del : disp_donttouch,
-			  thread, ignore_count, ops, from_tty, enabled);
+			  thread, task, ignore_count, ops, from_tty, enabled);
     }
   else
     {
@@ -7480,11 +7529,14 @@ breakpoint_re_set_one (void *bint)
 	{
 	  char *cond_string = 0;
 	  int thread = -1;
+	  int task = 0;
+
 	  find_condition_and_thread (s, sals.sals[0].pc, 
-				     &cond_string, &thread);
+				     &cond_string, &thread, &task);
 	  if (cond_string)
 	    b->cond_string = cond_string;
 	  b->thread = thread;
+	  b->task = task;
 	  b->condition_not_parsed = 0;
 	}
       expanded = expand_line_sal_maybe (sals.sals[0]);
diff --git a/gdb/breakpoint.h b/gdb/breakpoint.h
index 94287de..e18717d 100644
--- a/gdb/breakpoint.h
+++ b/gdb/breakpoint.h
@@ -423,9 +423,12 @@ struct breakpoint
        hardware.  */
     enum watchpoint_triggered watchpoint_triggered;
 
-    /* Thread number for thread-specific breakpoint, or -1 if don't care */
+    /* Thread number for thread-specific breakpoint, or -1 if don't care.  */
     int thread;
 
+    /* Ada task number for task-specific breakpoint, or 0 if don't care.  */
+    int task;
+
     /* Count of the number of times this breakpoint was taken, dumped
        with the info, but not used for anything else.  Useful for
        seeing how many times you hit a break prior to the program

Re: [RFC] Add task-specific breakpoint capability...

[Tom Tromey]

>>>>> "Joel" == Joel Brobecker <brobecker@adacore.com> writes:

Joel> The reason why I'm posting this as an RFC is that, although
Joel> the implementation is pretty straightforward, for some reason,
Joel> I find it a bit inelegant. Having two integers which are pretty
Joel> much mutually exclusive sounds pretty bad to me. But, on the other
Joel> hand, I didn't really find anything that is that much better.

FWIW this approach seems reasonable enough to me.

Joel> +int
Joel> +breakpoint_ada_task_match (CORE_ADDR pc, ptid_t ptid)

Nothing calls this.  Maybe the patch is missing changes from
infrun.c, and a declaration in breakpoint.h?

Tom

Re: [RFC] Add task-specific breakpoint capability...

[Joel Brobecker]

> FWIW this approach seems reasonable enough to me.

Cool :)

> Joel> +int
> Joel> +breakpoint_ada_task_match (CORE_ADDR pc, ptid_t ptid)
> 
> Nothing calls this.  Maybe the patch is missing changes from
> infrun.c, and a declaration in breakpoint.h?

That's a really good catch - Indeed, I forgot the change in infrun.c.
And I moved the function declaration from ada-lang.h to breakpoint.h,
as suggested. Not sure why we put that declaration in ada-lang.h in
the first place - probably force of habits...

-- 
Joel

Re: [RFC] Add task-specific breakpoint capability...

[Joel Brobecker]

[-- Attachment #1: Type: text/plain, Size: 453 bytes --]

> That's a really good catch - Indeed, I forgot the change in infrun.c.
> And I moved the function declaration from ada-lang.h to breakpoint.h,
> as suggested. Not sure why we put that declaration in ada-lang.h in
> the first place - probably force of habits...

Humpf. Forgot the new patch. Here it is. I'll work on testing (in
particular making sure that the testcase does verify the infrun bit)
and documentation before I check this is in.

-- 
Joel

[-- Attachment #2: task-specific-bp.diff --]
[-- Type: text/x-diff, Size: 9054 bytes --]

commit aa1a55c66239c2ec7b2d90c16e1bf1229b0cf541
Author: Joel Brobecker <brobecker@adacore.com>
Date:   Tue Mar 24 11:24:49 2009 -0700

        Provide support for (Ada) task-specific breakpoints.
    
        * ada-lang.h (ada_get_task_number): Add declaration.
        (breakpoint_ada_task_match): Move declaration to breakpoint.h.
        * ada-tasks.c (ada_get_task_number): Make non-static.
        * breakpoint.h (struct breakpoint): Add field "task".
        (breakpoint_ada_task_match): Add declaration.
        * breakpoint.c (breakpoint_ada_task_match): New function.
        (print_one_breakpoint_location): Add handling of task-specific
        breakpoints.
        (create_breakpoint, create_breakpoints, find_condition_and_thread):
        New parameter "task".
        (break_command_really): Update calls to find_condition_and_thread
        and create_breakpoints.
        (breakpoint_re_set_one): Update call to find_condition_and_thread.
        Set b->task.
        * infrun.c (handle_inferior_event): Handle the case where GDB
        hits a task-specific breakpoint that was meant for another task.

diff --git a/gdb/ada-lang.h b/gdb/ada-lang.h
index 88b6c16..c9554a4 100644
--- a/gdb/ada-lang.h
+++ b/gdb/ada-lang.h
@@ -461,14 +461,14 @@ extern char *ada_main_name (void);
 
 extern int valid_task_id (int);
 
+extern int ada_get_task_number (ptid_t);
+
 extern void ada_adjust_exception_stop (bpstat bs);
 
 extern void ada_print_exception_stop (bpstat bs);
 
 extern int ada_get_current_task (ptid_t);
 
-extern int breakpoint_ada_task_match (CORE_ADDR, ptid_t);
-
 extern int ada_print_exception_breakpoint_nontask (struct breakpoint *);
 
 extern void ada_print_exception_breakpoint_task (struct breakpoint *);
diff --git a/gdb/ada-tasks.c b/gdb/ada-tasks.c
index d0ce5ab..4f0aaf5 100644
--- a/gdb/ada-tasks.c
+++ b/gdb/ada-tasks.c
@@ -160,7 +160,7 @@ static int stale_task_list_p = 1;
 /* Return the task number of the task whose ptid is PTID, or zero
    if the task could not be found.  */
 
-static int
+int
 ada_get_task_number (ptid_t ptid)
 {
   int i;
diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index 7e50342..5114ae7 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -1919,6 +1919,32 @@ breakpoint_thread_match (CORE_ADDR pc, ptid_t ptid)
 
   return 0;
 }
+
+/* Return true if the breakpoint at PC is valid for the Ada task identified
+   by its PTID.  */
+
+int
+breakpoint_ada_task_match (CORE_ADDR pc, ptid_t ptid)
+{
+  struct bp_location *bpt;
+  int taskid = ada_get_task_number (ptid);
+
+  ALL_BP_LOCATIONS (bpt)
+    {
+      if ((bpt->loc_type == bp_loc_software_breakpoint
+	   || bpt->loc_type == bp_loc_hardware_breakpoint)
+	  && (bpt->owner->enable_state == bp_enabled
+	      || bpt->owner->enable_state == bp_permanent)
+	  && bpt->address == pc
+	  && (bpt->owner->task == 0 || bpt->owner->task == taskid))
+      {
+	return 1;
+      }
+    }
+
+  return 0;
+}
+
 \f
 
 /* bpstat stuff.  External routines' interfaces are documented
@@ -3556,12 +3582,20 @@ print_one_breakpoint_location (struct breakpoint *b,
 	break;
       }
 
-  if (!part_of_multiple && b->thread != -1)
+  if (!part_of_multiple)
     {
-      /* FIXME: This seems to be redundant and lost here; see the
-	 "stop only in" line a little further down. */
-      ui_out_text (uiout, " thread ");
-      ui_out_field_int (uiout, "thread", b->thread);
+      if (b->thread != -1)
+	{
+	  /* FIXME: This seems to be redundant and lost here; see the
+	     "stop only in" line a little further down. */
+	  ui_out_text (uiout, " thread ");
+	  ui_out_field_int (uiout, "thread", b->thread);
+	}
+      else if (b->task != 0)
+	{
+	  ui_out_text (uiout, " task ");
+	  ui_out_field_int (uiout, "task", b->task);
+	}
     }
   
   ui_out_text (uiout, "\n");
@@ -5113,7 +5147,7 @@ static void
 create_breakpoint (struct symtabs_and_lines sals, char *addr_string,
 		   char *cond_string,
 		   enum bptype type, enum bpdisp disposition,
-		   int thread, int ignore_count, 
+		   int thread, int task, int ignore_count, 
 		   struct breakpoint_ops *ops, int from_tty, int enabled)
 {
   struct breakpoint *b = NULL;
@@ -5145,6 +5179,7 @@ create_breakpoint (struct symtabs_and_lines sals, char *addr_string,
 	  set_breakpoint_count (breakpoint_count + 1);
 	  b->number = breakpoint_count;
 	  b->thread = thread;
+	  b->task = task;
   
 	  b->cond_string = cond_string;
 	  b->ignore_count = ignore_count;
@@ -5323,7 +5358,7 @@ static void
 create_breakpoints (struct symtabs_and_lines sals, char **addr_string,
 		    char *cond_string,
 		    enum bptype type, enum bpdisp disposition,
-		    int thread, int ignore_count, 
+		    int thread, int task, int ignore_count, 
 		    struct breakpoint_ops *ops, int from_tty,
 		    int enabled)
 {
@@ -5335,7 +5370,7 @@ create_breakpoints (struct symtabs_and_lines sals, char **addr_string,
 
       create_breakpoint (expanded, addr_string[i],
 			 cond_string, type, disposition,
-			 thread, ignore_count, ops, from_tty, enabled);
+			 thread, task, ignore_count, ops, from_tty, enabled);
     }
 
   update_global_location_list (1);
@@ -5442,7 +5477,7 @@ do_captured_parse_breakpoint (struct ui_out *ui, void *data)
    If no thread is found, *THREAD is set to -1.  */
 static void 
 find_condition_and_thread (char *tok, CORE_ADDR pc, 
-			   char **cond_string, int *thread)
+			   char **cond_string, int *thread, int *task)
 {
   *cond_string = NULL;
   *thread = -1;
@@ -5485,6 +5520,18 @@ find_condition_and_thread (char *tok, CORE_ADDR pc,
 	  if (!valid_thread_id (*thread))
 	    error (_("Unknown thread %d."), *thread);
 	}
+      else if (toklen >= 1 && strncmp (tok, "task", toklen) == 0)
+	{
+	  char *tmptok;
+
+	  tok = end_tok + 1;
+	  tmptok = tok;
+	  *task = strtol (tok, &tok, 0);
+	  if (tok == tmptok)
+	    error (_("Junk after task keyword."));
+	  if (!valid_task_id (*task))
+	    error (_("Unknown task %d\n"), *task);
+	}
       else
 	error (_("Junk at end of arguments."));
     }
@@ -5523,6 +5570,7 @@ break_command_really (char *arg, char *cond_string, int thread,
   int i;
   int pending = 0;
   int not_found = 0;
+  int task = 0;
 
   sals.sals = NULL;
   sals.nelts = 0;
@@ -5624,7 +5672,8 @@ break_command_really (char *arg, char *cond_string, int thread,
                re-parse it in context of each sal.  */
             cond_string = NULL;
             thread = -1;
-            find_condition_and_thread (arg, sals.sals[0].pc, &cond_string, &thread);
+            find_condition_and_thread (arg, sals.sals[0].pc, &cond_string,
+                                       &thread, &task);
             if (cond_string)
                 make_cleanup (xfree, cond_string);
         }
@@ -5641,7 +5690,7 @@ break_command_really (char *arg, char *cond_string, int thread,
 			  hardwareflag ? bp_hardware_breakpoint 
 			  : bp_breakpoint,
 			  tempflag ? disp_del : disp_donttouch,
-			  thread, ignore_count, ops, from_tty, enabled);
+			  thread, task, ignore_count, ops, from_tty, enabled);
     }
   else
     {
@@ -7480,11 +7529,14 @@ breakpoint_re_set_one (void *bint)
 	{
 	  char *cond_string = 0;
 	  int thread = -1;
+	  int task = 0;
+
 	  find_condition_and_thread (s, sals.sals[0].pc, 
-				     &cond_string, &thread);
+				     &cond_string, &thread, &task);
 	  if (cond_string)
 	    b->cond_string = cond_string;
 	  b->thread = thread;
+	  b->task = task;
 	  b->condition_not_parsed = 0;
 	}
       expanded = expand_line_sal_maybe (sals.sals[0]);
diff --git a/gdb/breakpoint.h b/gdb/breakpoint.h
index 94287de..3e8e69e 100644
--- a/gdb/breakpoint.h
+++ b/gdb/breakpoint.h
@@ -423,9 +423,12 @@ struct breakpoint
        hardware.  */
     enum watchpoint_triggered watchpoint_triggered;
 
-    /* Thread number for thread-specific breakpoint, or -1 if don't care */
+    /* Thread number for thread-specific breakpoint, or -1 if don't care.  */
     int thread;
 
+    /* Ada task number for task-specific breakpoint, or 0 if don't care.  */
+    int task;
+
     /* Count of the number of times this breakpoint was taken, dumped
        with the info, but not used for anything else.  Useful for
        seeing how many times you hit a break prior to the program
@@ -668,6 +671,8 @@ extern int software_breakpoint_inserted_here_p (CORE_ADDR);
 
 extern int breakpoint_thread_match (CORE_ADDR, ptid_t);
 
+extern int breakpoint_ada_task_match (CORE_ADDR, ptid_t);
+
 extern void until_break_command (char *, int, int);
 
 extern void breakpoint_re_set (void);
diff --git a/gdb/infrun.c b/gdb/infrun.c
index 6600bbb..43d0404 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -2633,7 +2633,8 @@ targets should add new threads to the thread list themselves in non-stop mode.")
       if (regular_breakpoint_inserted_here_p (stop_pc))
 	{
 	  ecs->random_signal = 0;
-	  if (!breakpoint_thread_match (stop_pc, ecs->ptid))
+	  if (!breakpoint_thread_match (stop_pc, ecs->ptid)
+	      || !breakpoint_ada_task_match (stop_pc, ecs->ptid))
 	    thread_hop_needed = 1;
 	}
       else if (singlestep_breakpoints_inserted_p)

Re: [RFC] Add task-specific breakpoint capability...

[Tom Tromey]

>>>>> "Joel" == Joel Brobecker <brobecker@adacore.com> writes:

Joel> Humpf. Forgot the new patch.

I blame your MUA :)

Joel> +int
Joel> +breakpoint_ada_task_match (CORE_ADDR pc, ptid_t ptid)
Joel> +{
Joel> +  struct bp_location *bpt;
Joel> +  int taskid = ada_get_task_number (ptid);
Joel> +
Joel> +  ALL_BP_LOCATIONS (bpt)
[...]

Joel> -	  if (!breakpoint_thread_match (stop_pc, ecs->ptid))
Joel> +	  if (!breakpoint_thread_match (stop_pc, ecs->ptid)
Joel> +	      || !breakpoint_ada_task_match (stop_pc, ecs->ptid))
Joel>  	    thread_hop_needed = 1;

It seems like it would be more efficient to combine
breakpoint_thread_match and breakpoint_ada_task_match, so we only
have a single loop over all breakpoint locations.
This is probably not very important.

Tom

Re: [RFC] Add task-specific breakpoint capability...

[Joel Brobecker]

> It seems like it would be more efficient to combine
> breakpoint_thread_match and breakpoint_ada_task_match, so we only
> have a single loop over all breakpoint locations.
> This is probably not very important.

That's a good idea. The condition inside breakpoint_thread_match gets
a little too big and nested for my own taste, so I'll reorganize
the condition a little bit first, and then post a new patch on top
of that.

<synthesized voice>
Please wait, testing in progress...
</synthesized voice>

-- 
Joel

Re: [RFC] Add task-specific breakpoint capability...

[Joel Brobecker]

[-- Attachment #1: Type: text/plain, Size: 1163 bytes --]

OK, here is a new version of the patch. Hopefully, everything that we
discussed earlier should be incorporated, and I also tried to fix
a few tab/spaces issues (my favorite :). I'm also adding a testcase.

gdb/:
2009-03-25  Joel Brobecker  <brobecker@adacore.com>

        Provide support for (Ada) task-specific breakpoints.

        * ada-lang.h (ada_get_task_number): Add declaration.
        (breakpoint_ada_task_match): Delete declaration.
        * ada-tasks.c (ada_get_task_number): Make non-static.
        * breakpoint.h (struct breakpoint): Add field "task".
        * breakpoint.c (print_one_breakpoint_location): Add handling of
        task-specific breakpoints.
        (create_breakpoint, create_breakpoints, find_condition_and_thread):
        New parameter "task".
        (break_command_really): Update calls to find_condition_and_thread
        and create_breakpoints.
        (breakpoint_re_set_one): Update call to find_condition_and_thread.
        Set b->task.

gdb/testsuite/:
2009-03-25  Joel Brobecker  <brobecker@adacore.com>

        * gdb.ada/tasks: New testcase.

All tested on x86_64-linux. Will commit in a day or two...

-- 
Joel

[-- Attachment #2: task.diff --]
[-- Type: text/x-diff, Size: 7153 bytes --]

diff --git a/gdb/ada-lang.h b/gdb/ada-lang.h
index 88b6c16..c9554a4 100644
--- a/gdb/ada-lang.h
+++ b/gdb/ada-lang.h
@@ -461,14 +461,14 @@ extern char *ada_main_name (void);
 
 extern int valid_task_id (int);
 
+extern int ada_get_task_number (ptid_t);
+
 extern void ada_adjust_exception_stop (bpstat bs);
 
 extern void ada_print_exception_stop (bpstat bs);
 
 extern int ada_get_current_task (ptid_t);
 
-extern int breakpoint_ada_task_match (CORE_ADDR, ptid_t);
-
 extern int ada_print_exception_breakpoint_nontask (struct breakpoint *);
 
 extern void ada_print_exception_breakpoint_task (struct breakpoint *);
diff --git a/gdb/ada-tasks.c b/gdb/ada-tasks.c
index d0ce5ab..4f0aaf5 100644
--- a/gdb/ada-tasks.c
+++ b/gdb/ada-tasks.c
@@ -160,7 +160,7 @@ static int stale_task_list_p = 1;
 /* Return the task number of the task whose ptid is PTID, or zero
    if the task could not be found.  */
 
-static int
+int
 ada_get_task_number (ptid_t ptid)
 {
   int i;
diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index 7ffdf77..c742c7b 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -1893,8 +1893,9 @@ int
 breakpoint_thread_match (CORE_ADDR pc, ptid_t ptid)
 {
   const struct bp_location *bpt;
-  /* The thread ID associated to PTID, computed lazily.  */
+  /* The thread and task IDs associated to PTID, computed lazily.  */
   int thread = -1;
+  int task = 0;
   
   ALL_BP_LOCATIONS (bpt)
     {
@@ -1920,6 +1921,17 @@ breakpoint_thread_match (CORE_ADDR pc, ptid_t ptid)
 	    continue;
 	}
 
+      if (bpt->owner->task != 0)
+        {
+	  /* This is a task-specific breakpoint.  Check that ptid
+	     matches that task.  If task hasn't been computed yet,
+	     it is now time to do so.  */
+	  if (task == 0)
+	    task = ada_get_task_number (ptid);
+	  if (bpt->owner->task != task)
+	    continue;
+        }
+
       if (overlay_debugging 
 	  && section_is_overlay (bpt->section) 
 	  && !section_is_mapped (bpt->section))
@@ -3567,12 +3579,20 @@ print_one_breakpoint_location (struct breakpoint *b,
 	break;
       }
 
-  if (!part_of_multiple && b->thread != -1)
+  if (!part_of_multiple)
     {
-      /* FIXME: This seems to be redundant and lost here; see the
-	 "stop only in" line a little further down. */
-      ui_out_text (uiout, " thread ");
-      ui_out_field_int (uiout, "thread", b->thread);
+      if (b->thread != -1)
+	{
+	  /* FIXME: This seems to be redundant and lost here; see the
+	     "stop only in" line a little further down. */
+	  ui_out_text (uiout, " thread ");
+	  ui_out_field_int (uiout, "thread", b->thread);
+	}
+      else if (b->task != 0)
+	{
+	  ui_out_text (uiout, " task ");
+	  ui_out_field_int (uiout, "task", b->task);
+	}
     }
   
   ui_out_text (uiout, "\n");
@@ -5124,7 +5144,7 @@ static void
 create_breakpoint (struct symtabs_and_lines sals, char *addr_string,
 		   char *cond_string,
 		   enum bptype type, enum bpdisp disposition,
-		   int thread, int ignore_count, 
+		   int thread, int task, int ignore_count, 
 		   struct breakpoint_ops *ops, int from_tty, int enabled)
 {
   struct breakpoint *b = NULL;
@@ -5156,6 +5176,7 @@ create_breakpoint (struct symtabs_and_lines sals, char *addr_string,
 	  set_breakpoint_count (breakpoint_count + 1);
 	  b->number = breakpoint_count;
 	  b->thread = thread;
+	  b->task = task;
   
 	  b->cond_string = cond_string;
 	  b->ignore_count = ignore_count;
@@ -5334,7 +5355,7 @@ static void
 create_breakpoints (struct symtabs_and_lines sals, char **addr_string,
 		    char *cond_string,
 		    enum bptype type, enum bpdisp disposition,
-		    int thread, int ignore_count, 
+		    int thread, int task, int ignore_count, 
 		    struct breakpoint_ops *ops, int from_tty,
 		    int enabled)
 {
@@ -5346,7 +5367,7 @@ create_breakpoints (struct symtabs_and_lines sals, char **addr_string,
 
       create_breakpoint (expanded, addr_string[i],
 			 cond_string, type, disposition,
-			 thread, ignore_count, ops, from_tty, enabled);
+			 thread, task, ignore_count, ops, from_tty, enabled);
     }
 
   update_global_location_list (1);
@@ -5453,7 +5474,7 @@ do_captured_parse_breakpoint (struct ui_out *ui, void *data)
    If no thread is found, *THREAD is set to -1.  */
 static void 
 find_condition_and_thread (char *tok, CORE_ADDR pc, 
-			   char **cond_string, int *thread)
+			   char **cond_string, int *thread, int *task)
 {
   *cond_string = NULL;
   *thread = -1;
@@ -5496,6 +5517,18 @@ find_condition_and_thread (char *tok, CORE_ADDR pc,
 	  if (!valid_thread_id (*thread))
 	    error (_("Unknown thread %d."), *thread);
 	}
+      else if (toklen >= 1 && strncmp (tok, "task", toklen) == 0)
+	{
+	  char *tmptok;
+
+	  tok = end_tok + 1;
+	  tmptok = tok;
+	  *task = strtol (tok, &tok, 0);
+	  if (tok == tmptok)
+	    error (_("Junk after task keyword."));
+	  if (!valid_task_id (*task))
+	    error (_("Unknown task %d\n"), *task);
+	}
       else
 	error (_("Junk at end of arguments."));
     }
@@ -5534,6 +5567,7 @@ break_command_really (char *arg, char *cond_string, int thread,
   int i;
   int pending = 0;
   int not_found = 0;
+  int task = 0;
 
   sals.sals = NULL;
   sals.nelts = 0;
@@ -5635,7 +5669,8 @@ break_command_really (char *arg, char *cond_string, int thread,
                re-parse it in context of each sal.  */
             cond_string = NULL;
             thread = -1;
-            find_condition_and_thread (arg, sals.sals[0].pc, &cond_string, &thread);
+            find_condition_and_thread (arg, sals.sals[0].pc, &cond_string,
+                                       &thread, &task);
             if (cond_string)
                 make_cleanup (xfree, cond_string);
         }
@@ -5652,7 +5687,7 @@ break_command_really (char *arg, char *cond_string, int thread,
 			  hardwareflag ? bp_hardware_breakpoint 
 			  : bp_breakpoint,
 			  tempflag ? disp_del : disp_donttouch,
-			  thread, ignore_count, ops, from_tty, enabled);
+			  thread, task, ignore_count, ops, from_tty, enabled);
     }
   else
     {
@@ -7491,11 +7526,14 @@ breakpoint_re_set_one (void *bint)
 	{
 	  char *cond_string = 0;
 	  int thread = -1;
+	  int task = 0;
+
 	  find_condition_and_thread (s, sals.sals[0].pc, 
-				     &cond_string, &thread);
+				     &cond_string, &thread, &task);
 	  if (cond_string)
 	    b->cond_string = cond_string;
 	  b->thread = thread;
+	  b->task = task;
 	  b->condition_not_parsed = 0;
 	}
       expanded = expand_line_sal_maybe (sals.sals[0]);
diff --git a/gdb/breakpoint.h b/gdb/breakpoint.h
index 94287de..e18717d 100644
--- a/gdb/breakpoint.h
+++ b/gdb/breakpoint.h
@@ -423,9 +423,12 @@ struct breakpoint
        hardware.  */
     enum watchpoint_triggered watchpoint_triggered;
 
-    /* Thread number for thread-specific breakpoint, or -1 if don't care */
+    /* Thread number for thread-specific breakpoint, or -1 if don't care.  */
     int thread;
 
+    /* Ada task number for task-specific breakpoint, or 0 if don't care.  */
+    int task;
+
     /* Count of the number of times this breakpoint was taken, dumped
        with the info, but not used for anything else.  Useful for
        seeing how many times you hit a break prior to the program

[-- Attachment #3: task-tc.diff --]
[-- Type: text/x-diff, Size: 5480 bytes --]

diff --git a/gdb/testsuite/gdb.ada/tasks.exp b/gdb/testsuite/gdb.ada/tasks.exp
new file mode 100644
index 0000000..e5d9f92
--- /dev/null
+++ b/gdb/testsuite/gdb.ada/tasks.exp
@@ -0,0 +1,79 @@
+# Copyright 2009 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/>.
+
+if $tracelevel then {
+    strace $tracelevel
+}
+
+load_lib "ada.exp"
+
+set testdir "tasks"
+set testfile "${testdir}/foo"
+set srcfile ${srcdir}/${subdir}/${testfile}.adb
+set binfile ${objdir}/${subdir}/${testfile}
+
+file mkdir ${objdir}/${subdir}/${testdir}
+if {[gdb_compile_ada "${srcfile}" "${binfile}" executable [list debug ]] != "" } {
+  return -1
+}
+
+gdb_exit
+gdb_start
+gdb_reinitialize_dir $srcdir/$subdir
+gdb_load ${binfile}
+
+set bp_location [gdb_get_line_number "STOP_HERE" ${testdir}/foo.adb]
+runto "foo.adb:$bp_location"
+
+# Make sure that all tasks appear in the "info tasks" listing, and
+# that the active task is the environment task.
+gdb_test "info tasks" \
+         [join {"  ID       TID P-ID Pri State                  Name" \
+                "\\*  1 .* main_task" \
+                "   2 .* task_list\\(1\\)" \
+                "   3 .* task_list\\(2\\)" \
+                "   4 .* task_list\\(3\\)"} \
+               "\r\n"] \
+         "info tasks before inserting breakpoint"
+
+# Now, insert a breakpoint that should stop only if task 3 stops.
+gdb_test "break break_me task 3" "Breakpoint .* at .*"
+
+# Continue to that breakpoint.  Task 2 should hit it first, and GDB
+# is expected to ignore that hit and resume the execution.  Only then
+# task 3 will hit our breakpoint, and GDB is expected to stop at that
+# point.
+gdb_test "continue" \
+         ".*Breakpoint.*, foo.break_me \\(\\).*" \
+         "continue to breakpoint"
+
+# Check that it is indeed task 3 that hit the breakpoint by checking
+# which is the active task.
+gdb_test "info tasks" \
+         [join {"  ID       TID P-ID Pri State                  Name" \
+                "   1 .* main_task" \
+                "   2 .* task_list\\(1\\)" \
+                "\\*  3 .* task_list\\(2\\)" \
+                "   4 .* task_list\\(3\\)"} \
+               "\r\n"] \
+         "info tasks after hitting breakpoint"
+
+# Now, resume the execution and make sure that GDB does not stop when
+# task 4 hits the breakpoint. Continuing thus results in our program
+# running to completion.
+gdb_test "continue" \
+         ".*Program exited normally\..*" \
+         "continue until end of program"
+
diff --git a/gdb/testsuite/gdb.ada/tasks/foo.adb b/gdb/testsuite/gdb.ada/tasks/foo.adb
new file mode 100644
index 0000000..edf66be
--- /dev/null
+++ b/gdb/testsuite/gdb.ada/tasks/foo.adb
@@ -0,0 +1,68 @@
+--  Copyright 2009 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/>.
+
+procedure Foo is
+
+   task type Caller is
+      entry Initialize;
+      entry Call_Break_Me;
+      entry Finalize;
+   end Caller;
+   type Caller_Ptr is access Caller;
+
+   procedure Break_Me is
+   begin
+      null;
+   end Break_Me;
+
+   task body Caller is
+   begin
+      accept Initialize do
+         null;
+      end Initialize;
+      accept Call_Break_Me do
+         Break_Me;
+      end Call_Break_Me;
+      accept Finalize do
+         null;
+      end Finalize;
+   end Caller;
+
+   Task_List : array (1 .. 3) of Caller_Ptr;
+
+begin
+
+   --  Start all our tasks, and call the "Initialize" entry to make
+   --  sure all of them have now been started.  We call that entry
+   --  immediately after having created the task in order to make sure
+   --  that we wait for that task to be created before we try to create
+   --  another one.  That way, we know that the order in our Task_List
+   --  corresponds to the order in the GNAT runtime.
+   for J in Task_List'Range loop
+      Task_List (J) := new Caller;
+      Task_List (J).Initialize;
+   end loop;
+
+   --  Next, call their Call_Break_Me entry of each task, using the same
+   --  order as the order used to create them.
+   for J in Task_List'Range loop  -- STOP_HERE
+      Task_List (J).Call_Break_Me;
+   end loop;
+
+   --  And finally, let all the tasks die...
+   for J in Task_List'Range loop
+      Task_List (J).Finalize;
+   end loop;
+end Foo;

Re: [RFC] Add task-specific breakpoint capability...

[Eli Zaretskii]

> Date: Wed, 25 Mar 2009 12:56:33 -0700
> From: Joel Brobecker <brobecker@adacore.com>
> Cc: gdb-patches@sourceware.org
> 
> OK, here is a new version of the patch. Hopefully, everything that we
> discussed earlier should be incorporated, and I also tried to fix
> a few tab/spaces issues (my favorite :). I'm also adding a testcase.

Thanks.

> gdb/:
> 2009-03-25  Joel Brobecker  <brobecker@adacore.com>
> 
>         Provide support for (Ada) task-specific breakpoints.
> 
>         * ada-lang.h (ada_get_task_number): Add declaration.
>         (breakpoint_ada_task_match): Delete declaration.
>         * ada-tasks.c (ada_get_task_number): Make non-static.
>         * breakpoint.h (struct breakpoint): Add field "task".
>         * breakpoint.c (print_one_breakpoint_location): Add handling of
>         task-specific breakpoints.
>         (create_breakpoint, create_breakpoints, find_condition_and_thread):
>         New parameter "task".
>         (break_command_really): Update calls to find_condition_and_thread
>         and create_breakpoints.
>         (breakpoint_re_set_one): Update call to find_condition_and_thread.
>         Set b->task.
> 
> gdb/testsuite/:
> 2009-03-25  Joel Brobecker  <brobecker@adacore.com>
> 
>         * gdb.ada/tasks: New testcase.

Hmm?... no documentation?

Re: [RFC] Add task-specific breakpoint capability...

[Joel Brobecker]

> gdb/:
> 2009-03-25  Joel Brobecker  <brobecker@adacore.com>
> 
>         Provide support for (Ada) task-specific breakpoints.
> 
>         * ada-lang.h (ada_get_task_number): Add declaration.
>         (breakpoint_ada_task_match): Delete declaration.
>         * ada-tasks.c (ada_get_task_number): Make non-static.
>         * breakpoint.h (struct breakpoint): Add field "task".
>         * breakpoint.c (print_one_breakpoint_location): Add handling of
>         task-specific breakpoints.
>         (create_breakpoint, create_breakpoints, find_condition_and_thread):
>         New parameter "task".
>         (break_command_really): Update calls to find_condition_and_thread
>         and create_breakpoints.
>         (breakpoint_re_set_one): Update call to find_condition_and_thread.
>         Set b->task.
> 
> gdb/testsuite/:
> 2009-03-25  Joel Brobecker  <brobecker@adacore.com>
> 
>         * gdb.ada/tasks: New testcase.
> 
> All tested on x86_64-linux. Will commit in a day or two...

This two patches are now in. Retested on x86_64 linux, JIC.

-- 
Joel