From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 7496 invoked by alias); 24 Mar 2009 20:33:34 -0000 Received: (qmail 7488 invoked by uid 22791); 24 Mar 2009 20:33:32 -0000 X-SWARE-Spam-Status: No, hits=-1.5 required=5.0 tests=AWL,BAYES_00,J_CHICKENPOX_44,J_CHICKENPOX_45,J_CHICKENPOX_53 X-Spam-Check-By: sourceware.org Received: from rock.gnat.com (HELO rock.gnat.com) (205.232.38.15) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Tue, 24 Mar 2009 20:33:27 +0000 Received: from localhost (localhost.localdomain [127.0.0.1]) by filtered-rock.gnat.com (Postfix) with ESMTP id 9EEB42BAB9A for ; Tue, 24 Mar 2009 16:33:25 -0400 (EDT) Received: from rock.gnat.com ([127.0.0.1]) by localhost (rock.gnat.com [127.0.0.1]) (amavisd-new, port 10024) with LMTP id 6-Tj2TZ3lKsM for ; Tue, 24 Mar 2009 16:33:25 -0400 (EDT) Received: from joel.gnat.com (localhost.localdomain [127.0.0.1]) by rock.gnat.com (Postfix) with ESMTP id 337DA2BAB90 for ; Tue, 24 Mar 2009 16:33:25 -0400 (EDT) Received: by joel.gnat.com (Postfix, from userid 1000) id 86CBFF5898; Tue, 24 Mar 2009 13:33:19 -0700 (PDT) Date: Tue, 24 Mar 2009 21:27:00 -0000 From: Joel Brobecker To: gdb-patches@sourceware.org Subject: [RFC] Add task-specific breakpoint capability... Message-ID: <20090324203319.GB24100@adacore.com> MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="ew6BAiZeqk4r7MaW" Content-Disposition: inline User-Agent: Mutt/1.5.18 (2008-05-17) Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2009-03/txt/msg00532.txt.bz2 --ew6BAiZeqk4r7MaW Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-length: 2960 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 --ew6BAiZeqk4r7MaW Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="task-specific-breakpoint.diff" Content-length: 7678 commit df79a13713ea3a6946b2cf0659cc9cbe22beb445 Author: Joel Brobecker 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; +} + /* 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 --ew6BAiZeqk4r7MaW--