From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 23985 invoked by alias); 19 Oct 2004 23:57:00 -0000 Mailing-List: contact gdb-patches-help@sources.redhat.com; run by ezmlm Precedence: bulk List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sources.redhat.com Received: (qmail 23974 invoked from network); 19 Oct 2004 23:56:58 -0000 Received: from unknown (HELO mx1.redhat.com) (66.187.233.31) by sourceware.org with SMTP; 19 Oct 2004 23:56:58 -0000 Received: from int-mx1.corp.redhat.com (int-mx1.corp.redhat.com [172.16.52.254]) by mx1.redhat.com (8.12.11/8.12.10) with ESMTP id i9JNuwnm030594 for ; Tue, 19 Oct 2004 19:56:58 -0400 Received: from pobox.toronto.redhat.com (pobox.toronto.redhat.com [172.16.14.4]) by int-mx1.corp.redhat.com (8.11.6/8.11.6) with ESMTP id i9JNuwr16275 for ; Tue, 19 Oct 2004 19:56:58 -0400 Received: from touchme.toronto.redhat.com (IDENT:postfix@touchme.toronto.redhat.com [172.16.14.9]) by pobox.toronto.redhat.com (8.12.8/8.12.8) with ESMTP id i9JNuwbU002638 for ; Tue, 19 Oct 2004 19:56:58 -0400 Received: from redhat.com (toocool.toronto.redhat.com [172.16.14.72]) by touchme.toronto.redhat.com (Postfix) with ESMTP id AF474800044 for ; Tue, 19 Oct 2004 19:56:57 -0400 (EDT) Message-ID: <4175A9C9.8040300@redhat.com> Date: Tue, 19 Oct 2004 23:57:00 -0000 From: Jeff Johnston User-Agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.4) Gecko/20030624 Netscape/7.1 MIME-Version: 1.0 To: gdb-patches@sources.redhat.com Subject: [RFA]: Watchpoints per thread patch Content-Type: multipart/mixed; boundary="------------020605040805000603030400" X-SW-Source: 2004-10/txt/msg00328.txt.bz2 This is a multi-part message in MIME format. --------------020605040805000603030400 Content-Type: text/plain; charset=us-ascii; format=flowed Content-Transfer-Encoding: 7bit Content-length: 2844 The following patch adds needed support for the ia64 and s390 platforms. For these platforms, watchpoints need to be inserted / removed on each thread so as to work across all threads. The patch adds support for detecting this at configuration time and setting a new flag WATCHPOINTS_PER_THREAD. This flag is used when inserting/removing watchpoints and when a new thread event occurs. This patch in itself does not give these platforms threaded watchpoint support to pass the watchthreads.exp test case, but I am breaking up my bigger patch that works for x86, x86_64, and ia64. It still fails on the S390 but gets part marks because threaded watchpoints actually trigger properly however the S390 cannot determine which watchpoint gets triggered when multiple watchpoint events occur. On ia64, the watchthreads.exp test case will fail differently than before because the low-level register and watchpoint code does not properly calculate the LWP for accessing registers so we end up eating through the maximum number of watchpoints quicker than anticipated. I have a subsequent patch for getting the LWP reliably, but this subsequent patch may be made unnecessary depending on what Daniel does with ptids and the thread-db layer. Regardless of Daniel's redesign, the watchpoints still must be inserted/removed on each thread. Ok to commit? -- Jeff J. 2004-10-19 Jeff Johnston * configure.host: For ia64 and s390 platforms, support watchpoints per thread. * configure.in: Set WATCHPOINTS_PER_THREAD flag if platform has watchpoints per thread. * config.in: Regenerated. * configure: Ditto. * thread.c (restore_current_thread): Add new silent flag argument. Do not issue message if switching threads and the silent flag is set. Change all existing callers to call with new argument. Add call to find_thread_pid to safeguard against accessing an incomplete or invalid thread list item. (thread_switch_and_call): New function. * breakpoint.h: Define incomplete struct thread_info type. (insert_watchpoints_for_new_thread): New prototype. * breakpoint.c (insert_watchpoints_for_new_thread): New function. (insert_one_watchpoint): Ditto. (insert_watchpoint): Ditto. (remove_one_watchpoint): Ditto. (remove_watchpoint): Ditto. (remove_breakpoint): Call remove_watchpoint instead of target_remove_watchpoint. (insert_bp_location): Call insert_watchpoint instead of target_insert_watchpoint. (print_it_typical): Support watchpoints occurring at the same time as thread events. * thread-db.c (attach_thread)[WATCHPOINTS_PER_THREAD]: Call insert_watchpoints_for_new_thread. --------------020605040805000603030400 Content-Type: text/plain; name="threadswitch.patch" Content-Transfer-Encoding: 7bit Content-Disposition: inline; filename="threadswitch.patch" Content-length: 12668 Index: configure.host =================================================================== RCS file: /cvs/src/src/gdb/configure.host,v retrieving revision 1.84 diff -u -p -r1.84 configure.host --- configure.host 1 Sep 2004 20:46:41 -0000 1.84 +++ configure.host 19 Oct 2004 22:59:47 -0000 @@ -179,3 +179,17 @@ hppa*-*-linux*) gdb_host_long_double_format=0 ;; esac + +# Set various watchpoint flags based on host/cpu + +case "${host}" in +ia64-*-*) + gdb_host_watchpoints_per_thread=1 + ;; +s390*-*-*) + gdb_host_watchpoints_per_thread=1 + ;; +*) + gdb_host_watchpoints_per_thread= + ;; +esac Index: configure.in =================================================================== RCS file: /cvs/src/src/gdb/configure.in,v retrieving revision 1.176 diff -u -p -r1.176 configure.in --- configure.in 10 Oct 2004 15:55:49 -0000 1.176 +++ configure.in 19 Oct 2004 22:59:47 -0000 @@ -1414,6 +1414,11 @@ AC_DEFINE_UNQUOTED(GDB_HOST_FLOAT_FORMAT AC_DEFINE_UNQUOTED(GDB_HOST_DOUBLE_FORMAT,$gdb_host_double_format,[Host double floatformat]) AC_DEFINE_UNQUOTED(GDB_HOST_LONG_DOUBLE_FORMAT,$gdb_host_long_double_format,[Host long double floatformat]) +# List of host watchpoint flags. +if test "x$gdb_host_watchpoints_per_thread" != "x"; then + AC_DEFINE_UNQUOTED(WATCHPOINTS_PER_THREAD,$gdb_host_watchpoints_per_thread,[Host watchpoints are thread-specific]) +fi + # target_subdir is used by the testsuite to find the target libraries. target_subdir= if test "${host}" != "${target}"; then Index: thread.c =================================================================== RCS file: /cvs/src/src/gdb/thread.c,v retrieving revision 1.38 diff -u -p -r1.38 thread.c --- thread.c 25 Aug 2004 15:18:05 -0000 1.38 +++ thread.c 19 Oct 2004 22:59:47 -0000 @@ -61,7 +61,7 @@ static void thread_apply_all_command (ch static int thread_alive (struct thread_info *); static void info_threads_command (char *, int); static void thread_apply_command (char *, int); -static void restore_current_thread (ptid_t); +static void restore_current_thread (ptid_t, int); static void switch_to_thread (ptid_t ptid); static void prune_threads (void); @@ -468,34 +468,38 @@ switch_to_thread (ptid_t ptid) } static void -restore_current_thread (ptid_t ptid) +restore_current_thread (ptid_t ptid, int silent) { - if (!ptid_equal (ptid, inferior_ptid)) + if (!ptid_equal (ptid, inferior_ptid) && + find_thread_pid (ptid)) { switch_to_thread (ptid); - print_stack_frame (get_current_frame (), 1, SRC_LINE); + if (!silent) + print_stack_frame (get_current_frame (), 1, SRC_LINE); } } struct current_thread_cleanup { ptid_t inferior_ptid; + int silent; }; static void do_restore_current_thread_cleanup (void *arg) { struct current_thread_cleanup *old = arg; - restore_current_thread (old->inferior_ptid); + restore_current_thread (old->inferior_ptid, old->silent); xfree (old); } static struct cleanup * -make_cleanup_restore_current_thread (ptid_t inferior_ptid) +make_cleanup_restore_current_thread (ptid_t inferior_ptid, int silent) { struct current_thread_cleanup *old = xmalloc (sizeof (struct current_thread_cleanup)); old->inferior_ptid = inferior_ptid; + old->silent = silent; return make_cleanup (do_restore_current_thread_cleanup, old); } @@ -519,7 +523,7 @@ thread_apply_all_command (char *cmd, int if (cmd == NULL || *cmd == '\000') error ("Please specify a command following the thread ID list"); - old_chain = make_cleanup_restore_current_thread (inferior_ptid); + old_chain = make_cleanup_restore_current_thread (inferior_ptid, 0); /* It is safe to update the thread list now, before traversing it for "thread apply all". MVS */ @@ -560,7 +564,7 @@ thread_apply_command (char *tidlist, int if (*cmd == '\000') error ("Please specify a command following the thread ID list"); - old_chain = make_cleanup_restore_current_thread (inferior_ptid); + old_chain = make_cleanup_restore_current_thread (inferior_ptid, 0); /* Save a copy of the command in case it is clobbered by execute_command */ @@ -616,6 +620,47 @@ thread_apply_command (char *tidlist, int do_cleanups (old_chain); } +int +thread_switch_and_call (int thread_num, int (*callback) (void *), void *args) +{ + struct thread_info *tp; + struct cleanup *old_chain; + int rc = -1; + + old_chain = make_cleanup_restore_current_thread (inferior_ptid, 1); + + if (thread_num == -1) + { + if (thread_list != NULL) + { + for (tp = thread_list; tp; tp = tp->next) + { + if (thread_alive (tp)) + { + switch_to_thread (tp->ptid); + rc = (*callback) (args); + if (rc) + break; + } + } + } + else + rc = (*callback) (args); + } + else + { + for (tp = thread_list; tp; tp = tp->next) + if (tp->num == thread_num && thread_alive (tp)) + { + switch_to_thread (tp->ptid); + rc = (*callback) (args); + } + } + + do_cleanups (old_chain); + return rc; +} + /* Switch to the specified thread. Will dispatch off to thread_apply_command if prefix of arg is `apply'. */ Index: breakpoint.h =================================================================== RCS file: /cvs/src/src/gdb/breakpoint.h,v retrieving revision 1.34 diff -u -p -r1.34 breakpoint.h --- breakpoint.h 13 May 2004 16:39:11 -0000 1.34 +++ breakpoint.h 19 Oct 2004 22:59:47 -0000 @@ -30,6 +30,7 @@ struct value; struct block; +struct thread_info; /* This is the maximum number of bytes a breakpoint instruction can take. Feel free to increase it. It's just used in a few places to size @@ -662,6 +663,7 @@ extern void rwatch_command_wrapper (char extern void tbreak_command (char *, int); extern int insert_breakpoints (void); +extern int insert_watchpoints_for_new_thread (struct thread_info *); extern int remove_breakpoints (void); Index: breakpoint.c =================================================================== RCS file: /cvs/src/src/gdb/breakpoint.c,v retrieving revision 1.183 diff -u -p -r1.183 breakpoint.c --- breakpoint.c 8 Oct 2004 17:30:46 -0000 1.183 +++ breakpoint.c 19 Oct 2004 22:59:48 -0000 @@ -748,6 +748,124 @@ insert_catchpoint (struct ui_out *uo, vo return 0; } +struct target_watchpoint_args +{ + CORE_ADDR addr; + int len; + int type; +}; + +static int +insert_one_watchpoint (void *data) +{ + struct target_watchpoint_args *args = (struct target_watchpoint_args *)data; + + return target_insert_watchpoint (args->addr, args->len, args->type); +} + +static int +insert_watchpoint (struct bp_location *bpt, CORE_ADDR addr, int len, int type) +{ + int val; + struct target_watchpoint_args args; + + args.addr = addr; + args.len = len; + args.type = type; + + /* If watchpoints do not apply to all threads automatically, we have to insert + and delete them for every thread. Otherwise, we can insert or delete them + once from any thread. */ +#ifdef WATCHPOINTS_PER_THREAD + val = thread_switch_and_call (bpt->owner->thread, &insert_one_watchpoint, &args); +#else + val = insert_one_watchpoint (&args); +#endif + + return val; +} + +int +insert_watchpoints_for_new_thread (struct thread_info *ti) +{ + struct bp_location *b; + int val = 0; + int return_val = 0; + struct ui_file *tmp_error_stream = mem_fileopen (); + make_cleanup_ui_file_delete (tmp_error_stream); + + /* Explicitly mark the warning -- this will only be printed if + there was an error. */ + fprintf_unfiltered (tmp_error_stream, "Warning:\n"); + + ALL_BP_LOCATIONS (b) + { + /* Skip disabled breakpoints. */ + if (!breakpoint_enabled (b->owner)) + continue; + + /* For every active watchpoint, we need to insert the watchpoint on the new + thread. */ + if ((b->loc_type == bp_loc_hardware_watchpoint + || b->owner->type == bp_watchpoint)) + { + struct value *v = b->owner->val_chain; + + /* Look at each value on the value chain. */ + for (; v; v = v->next) + { + /* If it's a memory location, and GDB actually needed + its contents to evaluate the expression, then we + must watch it. */ + if (VALUE_LVAL (v) == lval_memory + && ! VALUE_LAZY (v)) + { + struct type *vtype = check_typedef (VALUE_TYPE (v)); + + /* We only watch structs and arrays if user asked + for it explicitly, never if they just happen to + appear in the middle of some value chain. */ + if (v == b->owner->val_chain + || (TYPE_CODE (vtype) != TYPE_CODE_STRUCT + && TYPE_CODE (vtype) != TYPE_CODE_ARRAY)) + { + CORE_ADDR addr; + int len, type; + struct target_watchpoint_args args; + + addr = VALUE_ADDRESS (v) + VALUE_OFFSET (v); + len = TYPE_LENGTH (VALUE_TYPE (v)); + type = hw_write; + if (b->owner->type == bp_read_watchpoint) + type = hw_read; + else if (b->owner->type == bp_access_watchpoint) + type = hw_access; + args.addr = addr; + args.len = len; + args.type = type; + val = thread_switch_and_call (ti->num, &insert_one_watchpoint, &args); + } + } + } + } + + if (val) + return_val = val; + } + + /* Failure to insert a watchpoint on any memory value in the + value chain brings us here. */ + if (return_val) + { + fprintf_unfiltered (tmp_error_stream, + "%s\n", + "Could not insert hardware watchpoints on new thread."); + target_terminal_ours_for_output (); + error_stream (tmp_error_stream); + } + return return_val; +} + /* Helper routine: free the value chain for a breakpoint (watchpoint). */ static void free_valchain (struct bp_location *b) @@ -982,7 +1100,7 @@ insert_bp_location (struct bp_location * else if (bpt->owner->type == bp_access_watchpoint) type = hw_access; - val = target_insert_watchpoint (addr, len, type); + val = insert_watchpoint (bpt, addr, len, type); if (val == -1) { /* Don't exit the loop, try to insert @@ -1404,6 +1522,36 @@ detach_breakpoints (int pid) } static int +remove_one_watchpoint (void *data) +{ + struct target_watchpoint_args *args = (struct target_watchpoint_args *)data; + + return target_remove_watchpoint (args->addr, args->len, args->type); +} + +static int +remove_watchpoint (struct bp_location *bpt, CORE_ADDR addr, int len, int type) +{ + int val; + struct target_watchpoint_args args; + + args.addr = addr; + args.len = len; + args.type = type; + + /* If watchpoints do not apply to all threads automatically, we have to insert + and delete them for every thread. Otherwise, we can insert or delete them + once from any thread. */ +#ifdef WATCHPOINTS_PER_THREAD + val = thread_switch_and_call (bpt->owner->thread, &remove_one_watchpoint, &args); +#else + val = remove_one_watchpoint (&args); +#endif + + return val; +} + +static int remove_breakpoint (struct bp_location *b, insertion_state_t is) { int val; @@ -1512,7 +1660,7 @@ remove_breakpoint (struct bp_location *b else if (b->owner->type == bp_access_watchpoint) type = hw_access; - val = target_remove_watchpoint (addr, len, type); + val = remove_watchpoint (b, addr, len, type); if (val == -1) b->inserted = 1; val = 0; @@ -2122,8 +2270,13 @@ print_it_typical (bpstat bs) break; case bp_thread_event: - /* Not sure how we will get here. - GDB should not stop for these breakpoints. */ + /* We can only get here legitimately if something further on the bs + * list has caused the stop status to be noisy. A valid example + * of this is a new thread event and a software watchpoint have + * both occurred. */ + if (bs->next) + return PRINT_UNKNOWN; + printf_filtered ("Thread Event Breakpoint: gdb should not stop!\n"); return PRINT_NOTHING; break; Index: thread-db.c =================================================================== RCS file: /cvs/src/src/gdb/thread-db.c,v retrieving revision 1.46 diff -u -p -r1.46 thread-db.c --- thread-db.c 8 Oct 2004 20:29:56 -0000 1.46 +++ thread-db.c 19 Oct 2004 22:59:48 -0000 @@ -762,6 +762,13 @@ attach_thread (ptid_t ptid, const td_thr ATTACH_LWP (BUILD_LWP (ti_p->ti_lid, GET_PID (ptid)), 0); #endif + /* For some platforms, watchpoints must be applied to each and every + thread. When a new thread is created, all current watchpoints need + to be inserted. */ +#ifdef WATCHPOINTS_PER_THREAD + insert_watchpoints_for_new_thread (tp); +#endif + /* Enable thread event reporting for this thread. */ err = td_thr_event_enable_p (th_p, 1); if (err != TD_OK) --------------020605040805000603030400--