From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 13221 invoked by alias); 10 Sep 2007 00:21:30 -0000 Received: (qmail 13210 invoked by uid 22791); 10 Sep 2007 00:21:22 -0000 X-Spam-Check-By: sourceware.org Received: from NaN.false.org (HELO nan.false.org) (208.75.86.248) by sourceware.org (qpsmtpd/0.31) with ESMTP; Mon, 10 Sep 2007 00:21:06 +0000 Received: from nan.false.org (localhost [127.0.0.1]) by nan.false.org (Postfix) with ESMTP id EA19498308; Mon, 10 Sep 2007 00:21:06 +0000 (GMT) Received: from caradoc.them.org (22.svnf5.xdsl.nauticom.net [209.195.183.55]) by nan.false.org (Postfix) with ESMTP id 4597798100; Mon, 10 Sep 2007 00:21:06 +0000 (GMT) Received: from drow by caradoc.them.org with local (Exim 4.67) (envelope-from ) id 1IUX1T-0007wg-3D; Sun, 09 Sep 2007 20:21:03 -0400 Date: Mon, 10 Sep 2007 00:21:00 -0000 From: Daniel Jacobowitz To: Luis Machado Cc: gdb-patches@sourceware.org Subject: Re: [patch 0/1] Threaded Watchpoints Message-ID: <20070910002103.GA25048@caradoc.them.org> Mail-Followup-To: Luis Machado , gdb-patches@sourceware.org References: <1187013078.4346.9.camel@localhost> <1187631217.11176.8.camel@localhost> <1187631568.11176.11.camel@localhost> <20070905020350.GA10025@caradoc.them.org> <1188995481.4879.5.camel@localhost> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1188995481.4879.5.camel@localhost> User-Agent: Mutt/1.5.15 (2007-04-09) X-IsSubscribed: yes 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: 2007-09/txt/msg00132.txt.bz2 On Wed, Sep 05, 2007 at 09:31:21AM -0300, Luis Machado wrote: > I added the ppc piece together with the s390's, ia64's and amd64's code > in the first patch. I believe the second one is a bit more complicated > because it works with bits and pieces of the i386 code, which i'm not > really familiar with, so it was a plain refresh of Jeff's patch. If i > can be of any help on those, feel free to contact me in the available > ways, i'm on IRC as well. > > Looking forward to your comments. Thanks for reviewing it. Here is my current patch. The PowerPC bits are totally untested, not even compiled - could you test it for me, please? I don't have a suitable system. Most of this is liberally borrowed from Jeff's (and your's and Jan's) work. There are some bits I'm not happy with yet, but I hope I'll have time to finish it up this week or next weekend, and then I will submit each logical change separately. Some of them deserve more explanation (and comments and gdbint documentation). I've regression tested i386, amd64, and ia64. I tested S/390 by hand and it works, but the extra logic in watchthreads.exp for that platform hasn't been tested (no DejaGNU or expect on my test system). -- Daniel Jacobowitz CodeSourcery Index: amd64-linux-nat.c =================================================================== RCS file: /cvs/src/src/gdb/amd64-linux-nat.c,v retrieving revision 1.17 diff -u -p -r1.17 amd64-linux-nat.c --- amd64-linux-nat.c 23 Aug 2007 18:08:26 -0000 1.17 +++ amd64-linux-nat.c 10 Sep 2007 00:20:24 -0000 @@ -233,25 +233,33 @@ amd64_linux_store_inferior_registers (st } } +/* Support for debug registers. */ + +static unsigned long amd64_linux_dr[DR_CONTROL + 1]; + +struct amd64_linux_dr_set_args +{ + int regnum; + unsigned long value; +}; static unsigned long -amd64_linux_dr_get (int regnum) +amd64_linux_dr_get (ptid_t ptid, int regnum) { int tid; unsigned long value; - /* FIXME: kettenis/2001-01-29: It's not clear what we should do with - multi-threaded processes here. For now, pretend there is just - one thread. */ - tid = PIDGET (inferior_ptid); + tid = TIDGET (ptid); + if (tid == 0) + tid = PIDGET (ptid); /* FIXME: kettenis/2001-03-27: Calling perror_with_name if the ptrace call fails breaks debugging remote targets. The correct way to fix this is to add the hardware breakpoint and watchpoint - stuff to the target vectore. For now, just return zero if the + stuff to the target vector. For now, just return zero if the ptrace call fails. */ errno = 0; - value = ptrace (PT_READ_U, tid, + value = ptrace (PTRACE_PEEKUSER, tid, offsetof (struct user, u_debugreg[regnum]), 0); if (errno != 0) #if 0 @@ -264,25 +272,49 @@ amd64_linux_dr_get (int regnum) } static void -amd64_linux_dr_set (int regnum, unsigned long value) +amd64_linux_dr_set (ptid_t ptid, int regnum, unsigned long value) { int tid; - /* FIXME: kettenis/2001-01-29: It's not clear what we should do with - multi-threaded processes here. For now, pretend there is just - one thread. */ - tid = PIDGET (inferior_ptid); + tid = TIDGET (ptid); + if (tid == 0) + tid = PIDGET (ptid); errno = 0; - ptrace (PT_WRITE_U, tid, offsetof (struct user, u_debugreg[regnum]), value); + ptrace (PTRACE_POKEUSER, tid, + offsetof (struct user, u_debugreg[regnum]), value); if (errno != 0) perror_with_name (_("Couldn't write debug register")); } +static int +amd64_linux_dr_set_lwp (struct lwp_info *lp, void *args_p) +{ + struct amd64_linux_dr_set_args *args = args_p; + + /* We do not need to set the debug registers for new threads now; + they'll be done at resume anyway. */ + if (linux_nat_lwp_is_new (lp->ptid)) + return 0; + + amd64_linux_dr[args->regnum] = args->value; + amd64_linux_dr_set (lp->ptid, args->regnum, args->value); + + return 0; +} + void amd64_linux_dr_set_control (unsigned long control) { - amd64_linux_dr_set (DR_CONTROL, control); + if (amd64_linux_dr[DR_CONTROL] != control) + { + struct amd64_linux_dr_set_args args; + + amd64_linux_dr[DR_CONTROL] = control; + args.regnum = DR_CONTROL; + args.value = control; + iterate_over_lwps (amd64_linux_dr_set_lwp, &args); + } } void @@ -290,21 +322,41 @@ amd64_linux_dr_set_addr (int regnum, COR { gdb_assert (regnum >= 0 && regnum <= DR_LASTADDR - DR_FIRSTADDR); - amd64_linux_dr_set (DR_FIRSTADDR + regnum, addr); + if (amd64_linux_dr[DR_FIRSTADDR + regnum] != addr) + { + struct amd64_linux_dr_set_args args; + + amd64_linux_dr[DR_FIRSTADDR + regnum] = addr; + args.regnum = DR_FIRSTADDR + regnum; + args.value = addr; + iterate_over_lwps (amd64_linux_dr_set_lwp, &args); + } } void amd64_linux_dr_reset_addr (int regnum) { - gdb_assert (regnum >= 0 && regnum <= DR_LASTADDR - DR_FIRSTADDR); - - amd64_linux_dr_set (DR_FIRSTADDR + regnum, 0L); + amd64_linux_dr_set_addr (regnum, 0); } unsigned long amd64_linux_dr_get_status (void) { - return amd64_linux_dr_get (DR_STATUS); + return amd64_linux_dr_get (inferior_ptid, DR_STATUS); +} + +static void +amd64_linux_dr_update (ptid_t ptid) +{ + int i; + + if (!linux_nat_lwp_is_new (ptid)) + return; + + for (i = DR_FIRSTADDR; i <= DR_LASTADDR; i++) + amd64_linux_dr_set (ptid, i, amd64_linux_dr[i]); + + amd64_linux_dr_set (ptid, DR_CONTROL, amd64_linux_dr[DR_CONTROL]); } @@ -368,6 +420,7 @@ ps_get_thread_area (const struct ps_proc static void (*super_post_startup_inferior) (ptid_t ptid); +static void (*super_resume) (ptid_t ptid, int step, enum target_signal signal); static void amd64_linux_child_post_startup_inferior (ptid_t ptid) @@ -375,6 +428,13 @@ amd64_linux_child_post_startup_inferior i386_cleanup_dregs (); super_post_startup_inferior (ptid); } + +static void +amd64_linux_resume (ptid_t ptid, int step, enum target_signal signal) +{ + amd64_linux_dr_update (ptid); + super_resume (ptid, step, signal); +} /* Provide a prototype to silence -Wmissing-prototypes. */ @@ -398,6 +458,10 @@ _initialize_amd64_linux_nat (void) /* Fill in the generic GNU/Linux methods. */ t = linux_target (); + /* Override the default ptrace resume method. */ + super_resume = t->to_resume; + t->to_resume = amd64_linux_resume; + /* Override the GNU/Linux inferior startup hook. */ super_post_startup_inferior = t->to_post_startup_inferior; t->to_post_startup_inferior = amd64_linux_child_post_startup_inferior; Index: breakpoint.c =================================================================== RCS file: /cvs/src/src/gdb/breakpoint.c,v retrieving revision 1.263 diff -u -p -r1.263 breakpoint.c --- breakpoint.c 29 Aug 2007 22:07:47 -0000 1.263 +++ breakpoint.c 10 Sep 2007 00:20:24 -0000 @@ -2519,6 +2519,83 @@ bpstat_alloc (struct breakpoint *b, bpst return bs; } +/* The target has stopped with waitstatus WS. Check if any hardware + watchpoints have triggered, according to the target. */ + +int +watchpoints_triggered (struct target_waitstatus *ws) +{ + int stopped_by_watchpoint = STOPPED_BY_WATCHPOINT (*ws); + CORE_ADDR addr; + struct breakpoint *b; + + if (!stopped_by_watchpoint) + { + /* We were not stopped by a watchpoint. Mark all watchpoints + as not triggered. */ + ALL_BREAKPOINTS (b) + if (b->type == bp_hardware_watchpoint + || b->type == bp_read_watchpoint + || b->type == bp_access_watchpoint) + b->watchpoint_triggered = watch_triggered_no; + + return 0; + } + + if (!target_stopped_data_address (¤t_target, &addr)) + { + /* We were stopped by a watchpoint, but we don't know where. + Mark all watchpoints as unknown. */ + ALL_BREAKPOINTS (b) + if (b->type == bp_hardware_watchpoint + || b->type == bp_read_watchpoint + || b->type == bp_access_watchpoint) + b->watchpoint_triggered = watch_triggered_unknown; + + return stopped_by_watchpoint; + } + + /* The target could report the data address. Mark watchpoints + affected by this data address as triggered, and all others as not + triggered. */ + + ALL_BREAKPOINTS (b) + if (b->type == bp_hardware_watchpoint + || b->type == bp_read_watchpoint + || b->type == bp_access_watchpoint) + { + struct value *v; + + b->watchpoint_triggered = watch_triggered_no; + for (v = b->val_chain; v; v = value_next (v)) + { + if (VALUE_LVAL (v) == lval_memory && ! value_lazy (v)) + { + struct type *vtype = check_typedef (value_type (v)); + + if (v == b->val_chain + || (TYPE_CODE (vtype) != TYPE_CODE_STRUCT + && TYPE_CODE (vtype) != TYPE_CODE_ARRAY)) + { + CORE_ADDR vaddr; + + vaddr = VALUE_ADDRESS (v) + value_offset (v); + /* Exact match not required. Within range is + sufficient. */ + if (addr >= vaddr + && addr < vaddr + TYPE_LENGTH (value_type (v))) + { + b->watchpoint_triggered = watch_triggered_yes; + break; + } + } + } + } + } + + return 1; +} + /* Possible return values for watchpoint_check (this can't be an enum because of check_errors). */ /* The watchpoint has been deleted. */ @@ -2637,11 +2714,9 @@ which its expression is valid.\n"); } /* Get a bpstat associated with having just stopped at address - BP_ADDR in thread PTID. STOPPED_BY_WATCHPOINT is 1 if the - target thinks we stopped due to a hardware watchpoint, 0 if we - know we did not trigger a hardware watchpoint, and -1 if we do not know. */ + BP_ADDR in thread PTID. -/* Determine whether we stopped at a breakpoint, etc, or whether we + Determine whether we stopped at a breakpoint, etc, or whether we don't understand this stop. Result is a chain of bpstat's such that: if we don't understand the stop, the result is a null pointer. @@ -2656,7 +2731,7 @@ which its expression is valid.\n"); commands, FIXME??? fields. */ bpstat -bpstat_stop_status (CORE_ADDR bp_addr, ptid_t ptid, int stopped_by_watchpoint) +bpstat_stop_status (CORE_ADDR bp_addr, ptid_t ptid) { struct breakpoint *b, *temp; /* True if we've hit a breakpoint (as opposed to a watchpoint). */ @@ -2691,16 +2766,30 @@ bpstat_stop_status (CORE_ADDR bp_addr, p continue; } - /* Continuable hardware watchpoints are treated as non-existent if the - reason we stopped wasn't a hardware watchpoint (we didn't stop on - some data address). Otherwise gdb won't stop on a break instruction - in the code (not from a breakpoint) when a hardware watchpoint has - been defined. */ + /* Pretend that watchpoints are triggered if we have hit their associated + scope breakpoint. */ + if ((b->type == bp_watchpoint + || b->type == bp_hardware_watchpoint + || b->type == bp_read_watchpoint + || b->type == bp_access_watchpoint) + && b->related_breakpoint + && b->related_breakpoint->loc->address == bp_addr + && (!overlay_debugging + || !section_is_overlay (b->related_breakpoint->loc->section) + || section_is_mapped (b->related_breakpoint->loc->section))) + b->watchpoint_triggered = watch_triggered_yes; + + /* Continuable hardware watchpoints are treated as non-existent if the + reason we stopped wasn't a hardware watchpoint (we didn't stop on + some data address). Otherwise gdb won't stop on a break instruction + in the code (not from a breakpoint) when a hardware watchpoint has + been defined. Also skip watchpoints which we know did not trigger + (did not match the data address). */ if ((b->type == bp_hardware_watchpoint || b->type == bp_read_watchpoint || b->type == bp_access_watchpoint) - && !stopped_by_watchpoint) + && b->watchpoint_triggered == watch_triggered_no) continue; if (b->type == bp_hardware_breakpoint) @@ -2766,82 +2855,33 @@ bpstat_stop_status (CORE_ADDR bp_addr, p bs->stop = 1; bs->print = 1; - if (b->type == bp_watchpoint || - b->type == bp_hardware_watchpoint) - { - char *message = xstrprintf ("Error evaluating expression for watchpoint %d\n", - b->number); - struct cleanup *cleanups = make_cleanup (xfree, message); - int e = catch_errors (watchpoint_check, bs, message, - RETURN_MASK_ALL); - do_cleanups (cleanups); - switch (e) - { - case WP_DELETED: - /* We've already printed what needs to be printed. */ - /* Actually this is superfluous, because by the time we - call print_it_typical() the wp will be already deleted, - and the function will return immediately. */ - bs->print_it = print_it_done; - /* Stop. */ - break; - case WP_VALUE_CHANGED: - /* Stop. */ - ++(b->hit_count); - break; - case WP_VALUE_NOT_CHANGED: - /* Don't stop. */ - bs->print_it = print_it_noop; - bs->stop = 0; - continue; - default: - /* Can't happen. */ - /* FALLTHROUGH */ - case 0: - /* Error from catch_errors. */ - printf_filtered (_("Watchpoint %d deleted.\n"), b->number); - if (b->related_breakpoint) - b->related_breakpoint->disposition = disp_del_at_next_stop; - b->disposition = disp_del_at_next_stop; - /* We've already printed what needs to be printed. */ - bs->print_it = print_it_done; - - /* Stop. */ - break; - } - } - else if (b->type == bp_read_watchpoint || - b->type == bp_access_watchpoint) + if (b->type == bp_watchpoint + || b->type == bp_read_watchpoint + || b->type == bp_access_watchpoint + || b->type == bp_hardware_watchpoint) { CORE_ADDR addr; struct value *v; - int found = 0; - - if (!target_stopped_data_address (¤t_target, &addr)) - continue; - for (v = b->val_chain; v; v = value_next (v)) - { - if (VALUE_LVAL (v) == lval_memory - && ! value_lazy (v)) - { - struct type *vtype = check_typedef (value_type (v)); + int must_check_value = 0; - if (v == b->val_chain - || (TYPE_CODE (vtype) != TYPE_CODE_STRUCT - && TYPE_CODE (vtype) != TYPE_CODE_ARRAY)) - { - CORE_ADDR vaddr; + if (b->type == bp_watchpoint) + /* For a software watchpoint, we must always check the + watched value. */ + must_check_value = 1; + else if (b->watchpoint_triggered == watch_triggered_yes) + /* We have a hardware watchpoint (read, write, or access) + and the target earlier reported an address watched by + this watchpoint. */ + must_check_value = 1; + else if (b->watchpoint_triggered == watch_triggered_unknown + && b->type == bp_hardware_watchpoint) + /* We were stopped by a hardware watchpoint, but the target could + not report the data address. We must check the watchpoint's + value. Access and read watchpoints are out of luck; without + a data address, we can't figure it out. */ + must_check_value = 1; - vaddr = VALUE_ADDRESS (v) + value_offset (v); - /* Exact match not required. Within range is - sufficient. */ - if (addr >= vaddr && - addr < vaddr + TYPE_LENGTH (value_type (v))) - found = 1; - } - } - } - if (found) + if (must_check_value) { char *message = xstrprintf ("Error evaluating expression for watchpoint %d\n", b->number); @@ -2869,6 +2909,15 @@ bpstat_stop_status (CORE_ADDR bp_addr, p ++(b->hit_count); break; case WP_VALUE_NOT_CHANGED: + if (b->type == bp_hardware_watchpoint + || b->type == bp_watchpoint) + { + /* Don't stop: write watchpoints shouldn't fire if + the value hasn't changed. */ + bs->print_it = print_it_noop; + bs->stop = 0; + continue; + } /* Stop. */ ++(b->hit_count); break; @@ -2885,12 +2934,12 @@ bpstat_stop_status (CORE_ADDR bp_addr, p break; } } - else /* found == 0 */ + else /* must_check_value == 0 */ { - /* This is a case where some watchpoint(s) triggered, - but not at the address of this watchpoint (FOUND - was left zero). So don't print anything for this - watchpoint. */ + /* This is a case where some watchpoint(s) triggered, but + not at the address of this watchpoint, or else no + watchpoint triggered after all. So don't print + anything for this watchpoint. */ bs->print_it = print_it_noop; bs->stop = 0; continue; @@ -2942,6 +2991,9 @@ bpstat_stop_status (CORE_ADDR bp_addr, p annotate_ignore_count_change (); bs->stop = 0; } + else if (b->type == bp_thread_event || b->type == bp_overlay_event) + /* We do not stop for these. */ + bs->stop = 0; else { /* We will stop here */ @@ -2968,17 +3020,27 @@ bpstat_stop_status (CORE_ADDR bp_addr, p bs->next = NULL; /* Terminate the chain */ bs = root_bs->next; /* Re-grab the head of the chain */ - /* The value of a hardware watchpoint hasn't changed, but the - intermediate memory locations we are watching may have. */ - if (bs && !bs->stop && - (bs->breakpoint_at->type == bp_hardware_watchpoint || - bs->breakpoint_at->type == bp_read_watchpoint || - bs->breakpoint_at->type == bp_access_watchpoint)) - { - remove_breakpoints (); - insert_breakpoints (); - } - return bs; + /* If we aren't stopping, the value of some hardware watchpoint may + not have changed, but the intermediate memory locations we are + watching may have. Don't bother if we're stopping; this will get + done later. */ + for (bs = root_bs->next; bs != NULL; bs = bs->next) + if (bs->stop) + break; + + if (bs == NULL) + for (bs = root_bs->next; bs != NULL; bs = bs->next) + if (!bs->stop + && (bs->breakpoint_at->type == bp_hardware_watchpoint + || bs->breakpoint_at->type == bp_read_watchpoint + || bs->breakpoint_at->type == bp_access_watchpoint)) + { + remove_breakpoints (); + insert_breakpoints (); + break; + } + + return root_bs->next; } /* Tell what to do about this bpstat. */ @@ -5827,6 +5889,7 @@ watch_command_1 (char *arg, int accessfl /* The scope breakpoint is related to the watchpoint. We will need to act on them together. */ b->related_breakpoint = scope_breakpoint; + scope_breakpoint->related_breakpoint = b; } } value_free_to_mark (mark); Index: breakpoint.h =================================================================== RCS file: /cvs/src/src/gdb/breakpoint.h,v retrieving revision 1.47 diff -u -p -r1.47 breakpoint.h --- breakpoint.h 23 Aug 2007 18:08:26 -0000 1.47 +++ breakpoint.h 10 Sep 2007 00:20:24 -0000 @@ -299,6 +299,19 @@ struct breakpoint_ops void (*print_mention) (struct breakpoint *); }; +enum watchpoint_triggered +{ + /* This watchpoint definitely did not trigger. */ + watch_triggered_no = 0, + + /* Some hardware watchpoint triggered, and it might have been this + one, but we do not know which it was. */ + watch_triggered_unknown, + + /* This hardware watchpoint definitely did trigger. */ + watch_triggered_yes +}; + /* Note that the ->silent field is not currently used by any commands (though the code is in there if it was to be, and set_raw_breakpoint does set it to 0). I implemented it because I thought it would be @@ -378,6 +391,10 @@ struct breakpoint should be evaluated on the outermost frame. */ struct frame_id watchpoint_frame; + /* For hardware watchpoints, the triggered status according to the + hardware. */ + enum watchpoint_triggered watchpoint_triggered; + /* Thread number for thread-specific breakpoint, or -1 if don't care */ int thread; @@ -440,8 +457,7 @@ extern void bpstat_clear (bpstat *); is part of the bpstat is copied as well. */ extern bpstat bpstat_copy (bpstat); -extern bpstat bpstat_stop_status (CORE_ADDR pc, ptid_t ptid, - int stopped_by_watchpoint); +extern bpstat bpstat_stop_status (CORE_ADDR pc, ptid_t ptid); /* This bpstat_what stuff tells wait_for_inferior what to do with a breakpoint (a challenging task). */ @@ -836,4 +852,8 @@ extern void remove_single_step_breakpoin extern void *deprecated_insert_raw_breakpoint (CORE_ADDR); extern int deprecated_remove_raw_breakpoint (void *); +/* Check if any hardware watchpoints have triggered, according to the + target. */ +int watchpoints_triggered (struct target_waitstatus *); + #endif /* !defined (BREAKPOINT_H) */ Index: i386-linux-nat.c =================================================================== RCS file: /cvs/src/src/gdb/i386-linux-nat.c,v retrieving revision 1.82 diff -u -p -r1.82 i386-linux-nat.c --- i386-linux-nat.c 23 Aug 2007 18:08:34 -0000 1.82 +++ i386-linux-nat.c 10 Sep 2007 00:20:24 -0000 @@ -579,16 +579,23 @@ i386_linux_store_inferior_registers (str /* Support for debug registers. */ +static unsigned long i386_linux_dr[DR_CONTROL + 1]; + +struct i386_linux_dr_set_args +{ + int regnum; + unsigned long value; +}; + static unsigned long -i386_linux_dr_get (int regnum) +i386_linux_dr_get (ptid_t ptid, int regnum) { int tid; unsigned long value; - /* FIXME: kettenis/2001-01-29: It's not clear what we should do with - multi-threaded processes here. For now, pretend there is just - one thread. */ - tid = PIDGET (inferior_ptid); + tid = TIDGET (ptid); + if (tid == 0) + tid = PIDGET (ptid); /* FIXME: kettenis/2001-03-27: Calling perror_with_name if the ptrace call fails breaks debugging remote targets. The correct @@ -609,14 +616,13 @@ i386_linux_dr_get (int regnum) } static void -i386_linux_dr_set (int regnum, unsigned long value) +i386_linux_dr_set (ptid_t ptid, int regnum, unsigned long value) { int tid; - /* FIXME: kettenis/2001-01-29: It's not clear what we should do with - multi-threaded processes here. For now, pretend there is just - one thread. */ - tid = PIDGET (inferior_ptid); + tid = TIDGET (ptid); + if (tid == 0) + tid = PIDGET (ptid); errno = 0; ptrace (PTRACE_POKEUSER, tid, @@ -625,10 +631,34 @@ i386_linux_dr_set (int regnum, unsigned perror_with_name (_("Couldn't write debug register")); } +static int +i386_linux_dr_set_lwp (struct lwp_info *lp, void *args_p) +{ + struct i386_linux_dr_set_args *args = args_p; + + /* We do not need to set the debug registers for new threads now; + they'll be done at resume anyway. */ + if (linux_nat_lwp_is_new (lp->ptid)) + return 0; + + i386_linux_dr[args->regnum] = args->value; + i386_linux_dr_set (lp->ptid, args->regnum, args->value); + + return 0; +} + void i386_linux_dr_set_control (unsigned long control) { - i386_linux_dr_set (DR_CONTROL, control); + if (i386_linux_dr[DR_CONTROL] != control) + { + struct i386_linux_dr_set_args args; + + i386_linux_dr[DR_CONTROL] = control; + args.regnum = DR_CONTROL; + args.value = control; + iterate_over_lwps (i386_linux_dr_set_lwp, &args); + } } void @@ -636,21 +666,41 @@ i386_linux_dr_set_addr (int regnum, CORE { gdb_assert (regnum >= 0 && regnum <= DR_LASTADDR - DR_FIRSTADDR); - i386_linux_dr_set (DR_FIRSTADDR + regnum, addr); + if (i386_linux_dr[DR_FIRSTADDR + regnum] != addr) + { + struct i386_linux_dr_set_args args; + + i386_linux_dr[DR_FIRSTADDR + regnum] = addr; + args.regnum = DR_FIRSTADDR + regnum; + args.value = addr; + iterate_over_lwps (i386_linux_dr_set_lwp, &args); + } } void i386_linux_dr_reset_addr (int regnum) { - gdb_assert (regnum >= 0 && regnum <= DR_LASTADDR - DR_FIRSTADDR); - - i386_linux_dr_set (DR_FIRSTADDR + regnum, 0L); + i386_linux_dr_set_addr (regnum, 0); } unsigned long i386_linux_dr_get_status (void) { - return i386_linux_dr_get (DR_STATUS); + return i386_linux_dr_get (inferior_ptid, DR_STATUS); +} + +static void +i386_linux_dr_update (ptid_t ptid) +{ + int i; + + if (!linux_nat_lwp_is_new (ptid)) + return; + + for (i = DR_FIRSTADDR; i <= DR_LASTADDR; i++) + i386_linux_dr_set (ptid, i, i386_linux_dr[i]); + + i386_linux_dr_set (ptid, DR_CONTROL, i386_linux_dr[DR_CONTROL]); } @@ -729,12 +779,6 @@ i386_linux_resume (ptid_t ptid, int step int request = PTRACE_CONT; - if (pid == -1) - /* Resume all threads. */ - /* I think this only gets used in the non-threaded case, where "resume - all threads" and "resume inferior_ptid" are the same. */ - pid = PIDGET (inferior_ptid); - if (step) { struct regcache *regcache = get_thread_regcache (pid_to_ptid (pid)); @@ -784,6 +828,8 @@ i386_linux_resume (ptid_t ptid, int step } } + i386_linux_dr_update (ptid); + if (ptrace (request, pid, 0, target_signal_to_host (signal)) == -1) perror_with_name (("ptrace")); } Index: ia64-linux-nat.c =================================================================== RCS file: /cvs/src/src/gdb/ia64-linux-nat.c,v retrieving revision 1.40 diff -u -p -r1.40 ia64-linux-nat.c --- ia64-linux-nat.c 23 Aug 2007 18:08:35 -0000 1.40 +++ ia64-linux-nat.c 10 Sep 2007 00:20:24 -0000 @@ -479,8 +479,9 @@ fill_fpregset (const struct regcache *re #define IA64_PSR_DD (1UL << 39) static void -enable_watchpoints_in_psr (struct regcache *regcache) +enable_watchpoints_in_psr (ptid_t ptid) { + struct regcache *regcache = get_thread_regcache (ptid); ULONGEST psr; regcache_cooked_read_unsigned (regcache, IA64_PSR_REGNUM, &psr); @@ -492,20 +493,21 @@ enable_watchpoints_in_psr (struct regcac } } -static long -fetch_debug_register (ptid_t ptid, int idx) +static int +enable_watchpoints_in_psr_lwp (struct lwp_info *lp, void *unused) { - long val; - int tid; - - tid = TIDGET (ptid); - if (tid == 0) - tid = PIDGET (ptid); + enable_watchpoints_in_psr (lp->ptid); + return 0; +} - val = ptrace (PT_READ_U, tid, (PTRACE_TYPE_ARG3) (PT_DBR + 8 * idx), 0); +static long debug_registers[8]; - return val; -} +struct store_debug_register_pair_args +{ + int idx; + long dbr_addr; + long dbr_mask; +}; static void store_debug_register (ptid_t ptid, int idx, long val) @@ -520,15 +522,6 @@ store_debug_register (ptid_t ptid, int i } static void -fetch_debug_register_pair (ptid_t ptid, int idx, long *dbr_addr, long *dbr_mask) -{ - if (dbr_addr) - *dbr_addr = fetch_debug_register (ptid, 2 * idx); - if (dbr_mask) - *dbr_mask = fetch_debug_register (ptid, 2 * idx + 1); -} - -static void store_debug_register_pair (ptid_t ptid, int idx, long *dbr_addr, long *dbr_mask) { if (dbr_addr) @@ -538,6 +531,22 @@ store_debug_register_pair (ptid_t ptid, } static int +store_debug_register_pair_lwp (struct lwp_info *lp, void *args_p) +{ + struct store_debug_register_pair_args *args = args_p; + + /* We do not need to set the debug registers for new threads now; + they'll be done at resume anyway. */ + if (linux_nat_lwp_is_new (lp->ptid)) + return 0; + + store_debug_register_pair (lp->ptid, args->idx, &args->dbr_addr, + &args->dbr_mask); + + return 0; +} + +static int is_power_of_2 (int val) { int i, onecount; @@ -557,13 +566,14 @@ ia64_linux_insert_watchpoint (CORE_ADDR int idx; long dbr_addr, dbr_mask; int max_watchpoints = 4; + struct store_debug_register_pair_args args; if (len <= 0 || !is_power_of_2 (len)) return -1; for (idx = 0; idx < max_watchpoints; idx++) { - fetch_debug_register_pair (ptid, idx, NULL, &dbr_mask); + dbr_mask = debug_registers[idx * 2 + 1]; if ((dbr_mask & (0x3UL << 62)) == 0) { /* Exit loop if both r and w bits clear */ @@ -592,8 +602,15 @@ ia64_linux_insert_watchpoint (CORE_ADDR return -1; } + debug_registers[2 * idx] = dbr_addr; + debug_registers[2 * idx + 1] = dbr_mask; + args.idx = idx; + args.dbr_addr = dbr_addr; + args.dbr_mask = dbr_mask; + iterate_over_lwps (store_debug_register_pair_lwp, &args); + store_debug_register_pair (ptid, idx, &dbr_addr, &dbr_mask); - enable_watchpoints_in_psr (get_current_regcache ()); + iterate_over_lwps (enable_watchpoints_in_psr_lwp, NULL); return 0; } @@ -611,36 +628,56 @@ ia64_linux_remove_watchpoint (CORE_ADDR for (idx = 0; idx < max_watchpoints; idx++) { - fetch_debug_register_pair (ptid, idx, &dbr_addr, &dbr_mask); + dbr_addr = debug_registers[2 * idx]; + dbr_mask = debug_registers[2 * idx + 1]; if ((dbr_mask & (0x3UL << 62)) && addr == (CORE_ADDR) dbr_addr) { - dbr_addr = 0; - dbr_mask = 0; - store_debug_register_pair (ptid, idx, &dbr_addr, &dbr_mask); + struct store_debug_register_pair_args args; + + debug_registers[2 * idx] = 0; + debug_registers[2 * idx + 1] = 0; + args.idx = idx; + args.dbr_addr = 0; + args.dbr_mask = 0; + iterate_over_lwps (store_debug_register_pair_lwp, &args); + return 0; } } return -1; } +static void +ia64_linux_debug_register_update (ptid_t ptid) +{ + int i, any; + + if (!linux_nat_lwp_is_new (ptid)) + return; + + any = 0; + for (i = 0; i < 8; i++) + { + if (debug_registers[i] != 0) + any = 1; + store_debug_register (ptid, i, debug_registers[i]); + } + + if (any) + enable_watchpoints_in_psr (ptid); +} + static int ia64_linux_stopped_data_address (struct target_ops *ops, CORE_ADDR *addr_p) { CORE_ADDR psr; - int tid; - struct siginfo siginfo; - ptid_t ptid = inferior_ptid; + struct siginfo *siginfo_p; struct regcache *regcache = get_current_regcache (); - tid = TIDGET(ptid); - if (tid == 0) - tid = PIDGET (ptid); - - errno = 0; - ptrace (PTRACE_GETSIGINFO, tid, (PTRACE_TYPE_ARG3) 0, &siginfo); + siginfo_p = linux_nat_get_siginfo (inferior_ptid); - if (errno != 0 || siginfo.si_signo != SIGTRAP || - (siginfo.si_code & 0xffff) != 0x0004 /* TRAP_HWBKPT */) + if (siginfo_p->si_signo != SIGTRAP + || (siginfo_p->si_code & 0xffff) != 0x0004 /* TRAP_HWBKPT */) return 0; regcache_cooked_read_unsigned (regcache, IA64_PSR_REGNUM, &psr); @@ -648,7 +685,7 @@ ia64_linux_stopped_data_address (struct for the next instruction */ regcache_cooked_write_unsigned (regcache, IA64_PSR_REGNUM, psr); - *addr_p = (CORE_ADDR)siginfo.si_addr; + *addr_p = (CORE_ADDR)siginfo_p->si_addr; return 1; } @@ -781,6 +818,7 @@ ia64_linux_store_registers (struct regca static LONGEST (*super_xfer_partial) (struct target_ops *, enum target_object, const char *, gdb_byte *, const gdb_byte *, ULONGEST, LONGEST); +static void (*super_resume) (ptid_t ptid, int step, enum target_signal signal); static LONGEST ia64_linux_xfer_partial (struct target_ops *ops, @@ -796,6 +834,13 @@ ia64_linux_xfer_partial (struct target_o offset, len); } +static void +ia64_linux_resume (ptid_t ptid, int step, enum target_signal signal) +{ + ia64_linux_debug_register_update (ptid); + super_resume (ptid, step, signal); +} + void _initialize_ia64_linux_nat (void); void @@ -814,6 +859,10 @@ _initialize_ia64_linux_nat (void) super_xfer_partial = t->to_xfer_partial; t->to_xfer_partial = ia64_linux_xfer_partial; + /* Override the default ptrace resume method. */ + super_resume = t->to_resume; + t->to_resume = ia64_linux_resume; + /* Override watchpoint routines. */ /* The IA-64 architecture can step over a watch point (without triggering Index: infrun.c =================================================================== RCS file: /cvs/src/src/gdb/infrun.c,v retrieving revision 1.245 diff -u -p -r1.245 infrun.c --- infrun.c 23 Aug 2007 18:08:35 -0000 1.245 +++ infrun.c 10 Sep 2007 00:20:25 -0000 @@ -882,6 +882,7 @@ enum infwait_states { infwait_normal_state, infwait_thread_hop_state, + infwait_step_watch_state, infwait_nonstep_watch_state }; @@ -1221,17 +1222,12 @@ adjust_pc_after_break (struct execution_ by an event from the inferior, figure out what it means and take appropriate action. */ -int stepped_after_stopped_by_watchpoint; - void handle_inferior_event (struct execution_control_state *ecs) { - /* NOTE: bje/2005-05-02: If you're looking at this code and thinking - that the variable stepped_after_stopped_by_watchpoint isn't used, - then you're wrong! See remote.c:remote_stopped_data_address. */ - int sw_single_step_trap_p = 0; - int stopped_by_watchpoint = -1; /* Mark as unknown. */ + int stopped_by_watchpoint; + int stepped_after_stopped_by_watchpoint = 0; /* Cache the last pid/waitstatus. */ target_last_wait_ptid = ecs->ptid; @@ -1251,7 +1247,17 @@ handle_inferior_event (struct execution_ case infwait_normal_state: if (debug_infrun) fprintf_unfiltered (gdb_stdlog, "infrun: infwait_normal_state\n"); - stepped_after_stopped_by_watchpoint = 0; + break; + + case infwait_step_watch_state: + if (debug_infrun) + fprintf_unfiltered (gdb_stdlog, + "infrun: infwait_step_watch_state\n"); + + /* FIXME-maybe: is this cleaner than setting a flag? Does it + handle things like signals arriving and other things happening + in combination correctly? */ + stepped_after_stopped_by_watchpoint = 1; break; case infwait_nonstep_watch_state: @@ -1440,7 +1446,7 @@ handle_inferior_event (struct execution_ stop_pc = read_pc (); - stop_bpstat = bpstat_stop_status (stop_pc, ecs->ptid, 0); + stop_bpstat = bpstat_stop_status (stop_pc, ecs->ptid); ecs->random_signal = !bpstat_explains_signal (stop_bpstat); @@ -1488,7 +1494,7 @@ handle_inferior_event (struct execution_ ecs->saved_inferior_ptid = inferior_ptid; inferior_ptid = ecs->ptid; - stop_bpstat = bpstat_stop_status (stop_pc, ecs->ptid, 0); + stop_bpstat = bpstat_stop_status (stop_pc, ecs->ptid); ecs->random_signal = !bpstat_explains_signal (stop_bpstat); inferior_ptid = ecs->saved_inferior_ptid; @@ -1770,24 +1776,20 @@ handle_inferior_event (struct execution_ singlestep_breakpoints_inserted_p = 0; } - /* It may not be necessary to disable the watchpoint to stop over - it. For example, the PA can (with some kernel cooperation) - single step over a watchpoint without disabling the watchpoint. */ - if (HAVE_STEPPABLE_WATCHPOINT && STOPPED_BY_WATCHPOINT (ecs->ws)) + if (stepped_after_stopped_by_watchpoint) + stopped_by_watchpoint = 0; + else + stopped_by_watchpoint = watchpoints_triggered (&ecs->ws); + + /* If necessary, step over this watchpoint. We'll be back to display + it in a moment. */ + if (stopped_by_watchpoint + && (HAVE_STEPPABLE_WATCHPOINT + || gdbarch_have_nonsteppable_watchpoint (current_gdbarch))) { if (debug_infrun) fprintf_unfiltered (gdb_stdlog, "infrun: STOPPED_BY_WATCHPOINT\n"); - resume (1, 0); - prepare_to_wait (ecs); - return; - } - /* It is far more common to need to disable a watchpoint to step - the inferior over it. FIXME. What else might a debug - register or page protection watchpoint scheme need here? */ - if (gdbarch_have_nonsteppable_watchpoint (current_gdbarch) - && STOPPED_BY_WATCHPOINT (ecs->ws)) - { /* At this point, we are stopped at an instruction which has attempted to write to a piece of memory under control of a watchpoint. The instruction hasn't actually executed @@ -1797,31 +1799,31 @@ handle_inferior_event (struct execution_ In order to make watchpoints work `right', we really need to complete the memory write, and then evaluate the - watchpoint expression. The following code does that by - removing the watchpoint (actually, all watchpoints and - breakpoints), single-stepping the target, re-inserting - watchpoints, and then falling through to let normal - single-step processing handle proceed. Since this - includes evaluating watchpoints, things will come to a - stop in the correct manner. */ + watchpoint expression. We do this by single-stepping the + target. - if (debug_infrun) - fprintf_unfiltered (gdb_stdlog, "infrun: STOPPED_BY_WATCHPOINT\n"); - remove_breakpoints (); + It may not be necessary to disable the watchpoint to stop over + it. For example, the PA can (with some kernel cooperation) + single step over a watchpoint without disabling the watchpoint. + + It is far more common to need to disable a watchpoint to step + the inferior over it. If we have non-steppable watchpoints, + we must disable the current watchpoint; it's simplest to + disable all watchpoints and breakpoints. */ + + if (!HAVE_STEPPABLE_WATCHPOINT) + remove_breakpoints (); registers_changed (); target_resume (ecs->ptid, 1, TARGET_SIGNAL_0); /* Single step */ - ecs->waiton_ptid = ecs->ptid; - ecs->wp = &(ecs->ws); - ecs->infwait_state = infwait_nonstep_watch_state; + if (HAVE_STEPPABLE_WATCHPOINT) + ecs->infwait_state = infwait_step_watch_state; + else + ecs->infwait_state = infwait_nonstep_watch_state; prepare_to_wait (ecs); return; } - /* It may be possible to simply continue after a watchpoint. */ - if (HAVE_CONTINUABLE_WATCHPOINT) - stopped_by_watchpoint = STOPPED_BY_WATCHPOINT (ecs->ws); - ecs->stop_func_start = 0; ecs->stop_func_end = 0; ecs->stop_func_name = 0; @@ -1943,8 +1945,7 @@ handle_inferior_event (struct execution_ else { /* See if there is a breakpoint at the current PC. */ - stop_bpstat = bpstat_stop_status (stop_pc, ecs->ptid, - stopped_by_watchpoint); + stop_bpstat = bpstat_stop_status (stop_pc, ecs->ptid); /* Following in case break condition called a function. */ Index: linux-nat.c =================================================================== RCS file: /cvs/src/src/gdb/linux-nat.c,v retrieving revision 1.67 diff -u -p -r1.67 linux-nat.c --- linux-nat.c 2 Sep 2007 14:04:31 -0000 1.67 +++ linux-nat.c 10 Sep 2007 00:20:25 -0000 @@ -503,12 +503,13 @@ linux_child_follow_fork (struct target_o target_detach (NULL, 0); } - inferior_ptid = pid_to_ptid (child_pid); + inferior_ptid = ptid_build (child_pid, child_pid, 0); /* Reinstall ourselves, since we might have been removed in target_detach (which does other necessary cleanup). */ push_target (ops); + linux_nat_switch_fork (inferior_ptid); /* Reset breakpoints in the child as appropriate. */ follow_inferior_reset_breakpoints (); @@ -673,6 +674,7 @@ add_lwp (ptid_t ptid) lp->waitstatus.kind = TARGET_WAITKIND_IGNORE; lp->ptid = ptid; + lp->new_lwp = 1; lp->next = lwp_list; lwp_list = lp; @@ -1077,8 +1079,6 @@ resume_callback (struct lwp_info *lp, vo { if (lp->stopped && lp->status == 0) { - struct thread_info *tp; - linux_ops->to_resume (pid_to_ptid (GET_LWP (lp->ptid)), 0, TARGET_SIGNAL_0); if (debug_linux_nat) @@ -1087,6 +1087,8 @@ resume_callback (struct lwp_info *lp, vo target_pid_to_str (lp->ptid)); lp->stopped = 0; lp->step = 0; + lp->new_lwp = 0; + memset (&lp->siginfo, 0, sizeof (lp->siginfo)); } return 0; @@ -1136,68 +1138,70 @@ linux_nat_resume (ptid_t ptid, int step, ptid = inferior_ptid; lp = find_lwp_pid (ptid); - if (lp) - { - ptid = pid_to_ptid (GET_LWP (lp->ptid)); - - /* Remember if we're stepping. */ - lp->step = step; + gdb_assert (lp != NULL); - /* Mark this LWP as resumed. */ - lp->resumed = 1; + ptid = pid_to_ptid (GET_LWP (lp->ptid)); - /* If we have a pending wait status for this thread, there is no - point in resuming the process. But first make sure that - linux_nat_wait won't preemptively handle the event - we - should never take this short-circuit if we are going to - leave LP running, since we have skipped resuming all the - other threads. This bit of code needs to be synchronized - with linux_nat_wait. */ - - if (lp->status && WIFSTOPPED (lp->status)) - { - int saved_signo = target_signal_from_host (WSTOPSIG (lp->status)); - - if (signal_stop_state (saved_signo) == 0 - && signal_print_state (saved_signo) == 0 - && signal_pass_state (saved_signo) == 1) - { - if (debug_linux_nat) - fprintf_unfiltered (gdb_stdlog, - "LLR: Not short circuiting for ignored " - "status 0x%x\n", lp->status); + /* Remember if we're stepping. */ + lp->step = step; - /* FIXME: What should we do if we are supposed to continue - this thread with a signal? */ - gdb_assert (signo == TARGET_SIGNAL_0); - signo = saved_signo; - lp->status = 0; - } - } + /* Mark this LWP as resumed. */ + lp->resumed = 1; - if (lp->status) + /* If we have a pending wait status for this thread, there is no + point in resuming the process. But first make sure that + linux_nat_wait won't preemptively handle the event - we + should never take this short-circuit if we are going to + leave LP running, since we have skipped resuming all the + other threads. This bit of code needs to be synchronized + with linux_nat_wait. */ + + if (lp->status && WIFSTOPPED (lp->status)) + { + int saved_signo = target_signal_from_host (WSTOPSIG (lp->status)); + + if (signal_stop_state (saved_signo) == 0 + && signal_print_state (saved_signo) == 0 + && signal_pass_state (saved_signo) == 1) { + if (debug_linux_nat) + fprintf_unfiltered (gdb_stdlog, + "LLR: Not short circuiting for ignored " + "status 0x%x\n", lp->status); + /* FIXME: What should we do if we are supposed to continue this thread with a signal? */ gdb_assert (signo == TARGET_SIGNAL_0); + signo = saved_signo; + lp->status = 0; + } + } - if (debug_linux_nat) - fprintf_unfiltered (gdb_stdlog, - "LLR: Short circuiting for status 0x%x\n", - lp->status); + if (lp->status) + { + /* FIXME: What should we do if we are supposed to continue + this thread with a signal? */ + gdb_assert (signo == TARGET_SIGNAL_0); - return; - } + if (debug_linux_nat) + fprintf_unfiltered (gdb_stdlog, + "LLR: Short circuiting for status 0x%x\n", + lp->status); - /* Mark LWP as not stopped to prevent it from being continued by - resume_callback. */ - lp->stopped = 0; + return; } + /* Mark LWP as not stopped to prevent it from being continued by + resume_callback. */ + lp->stopped = 0; + if (resume_all) iterate_over_lwps (resume_callback, NULL); linux_ops->to_resume (ptid, step, signo); + lp->new_lwp = 0; + memset (&lp->siginfo, 0, sizeof (lp->siginfo)); + if (debug_linux_nat) fprintf_unfiltered (gdb_stdlog, "LLR: %s %s, %s (resume event thread)\n", @@ -1416,6 +1420,22 @@ wait_lwp (struct lwp_info *lp) return status; } +/* Save the most recent siginfo for LP. This is currently only called + for SIGTRAP; some ports use the si_addr field for + target_stopped_data_address. In the future, it may also be used to + restore the siginfo of requeued signals. */ + +static void +save_siginfo (struct lwp_info *lp) +{ + errno = 0; + ptrace (PTRACE_GETSIGINFO, GET_LWP (lp->ptid), + (PTRACE_TYPE_ARG3) 0, &lp->siginfo); + + if (errno != 0) + memset (&lp->siginfo, 0, sizeof (lp->siginfo)); +} + /* Send a SIGSTOP to LP. */ static int @@ -1501,6 +1521,9 @@ stop_wait_callback (struct lwp_info *lp, user will delete or disable the breakpoint, but the thread will have already tripped on it. */ + /* Save the trap's siginfo in case we need it later. */ + save_siginfo (lp); + /* Now resume this LWP and get the SIGSTOP event. */ errno = 0; ptrace (PTRACE_CONT, GET_LWP (lp->ptid), 0, 0); @@ -2058,6 +2081,10 @@ retry: target_pid_to_str (lp->ptid)); } + /* Save the trap's siginfo in case we need it later. */ + if (WIFSTOPPED (status) && WSTOPSIG (status) == SIGTRAP) + save_siginfo (lp); + /* Handle GNU/Linux's extended waitstatus for trace events. */ if (WIFSTOPPED (status) && WSTOPSIG (status) == SIGTRAP && status >> 16 != 0) { @@ -3250,6 +3277,28 @@ linux_nat_add_target (struct target_ops thread_db_init (t); } +/* Return non-zero if the specified LWP is new. */ +int +linux_nat_lwp_is_new (ptid_t ptid) +{ + struct lwp_info *lp = find_lwp_pid (ptid); + + gdb_assert (lp != NULL); + + return lp->new_lwp; +} + +/* Return the saved siginfo associated with PTID. */ +struct siginfo * +linux_nat_get_siginfo (ptid_t ptid) +{ + struct lwp_info *lp = find_lwp_pid (ptid); + + gdb_assert (lp != NULL); + + return &lp->siginfo; +} + void _initialize_linux_nat (void) { Index: linux-nat.h =================================================================== RCS file: /cvs/src/src/gdb/linux-nat.h,v retrieving revision 1.19 diff -u -p -r1.19 linux-nat.h --- linux-nat.h 23 Aug 2007 18:08:35 -0000 1.19 +++ linux-nat.h 10 Sep 2007 00:20:25 -0000 @@ -20,6 +20,8 @@ #include "target.h" +#include + /* Structure describing an LWP. */ struct lwp_info @@ -54,6 +56,14 @@ struct lwp_info /* Non-zero if we were stepping this LWP. */ int step; + /* Non-zero if this LWP is new. An LWP is considered new from the + moment it is attached until just after it has been resumed once. */ + int new_lwp; + + /* Non-zero si_signo if this LWP stopped with a trap. si_addr may + be the address of a hardware watchpoint. */ + struct siginfo siginfo; + /* If WAITSTATUS->KIND != TARGET_WAITKIND_SPURIOUS, the waitstatus for this LWP's last event. This may correspond to STATUS above, or to a local variable in lin_lwp_wait. */ @@ -98,3 +108,9 @@ void linux_nat_add_target (struct target /* Update linux-nat internal state when changing from one fork to another. */ void linux_nat_switch_fork (ptid_t new_ptid); + +/* Return non-zero if the specified LWP is new. */ +int linux_nat_lwp_is_new (ptid_t ptid); + +/* Return the saved siginfo associated with PTID. */ +struct siginfo *linux_nat_get_siginfo (ptid_t ptid); Index: ppc-linux-nat.c =================================================================== RCS file: /cvs/src/src/gdb/ppc-linux-nat.c,v retrieving revision 1.70 diff -u -p -r1.70 ppc-linux-nat.c --- ppc-linux-nat.c 30 Aug 2007 13:13:59 -0000 1.70 +++ ppc-linux-nat.c 10 Sep 2007 00:20:25 -0000 @@ -142,8 +142,6 @@ struct gdb_evrregset_t error. */ int have_ptrace_getvrregs = 1; -static CORE_ADDR last_stopped_data_address = 0; - /* Non-zero if our kernel may support the PTRACE_GETEVRREGS and PTRACE_SETEVRREGS requests, for reading and writing the SPE registers. Zero if we've tried one of them and gotten an @@ -805,13 +803,29 @@ ppc_linux_region_ok_for_hw_watchpoint (C return 1; } +/* The cached DABR value, to install in new threads. */ +static long saved_dabr_value; + +static int +ppc_linux_set_debugreg_lwp (struct lwp_info *lp, void *arg) +{ + int tid; + long dabr_value = *(long *) arg; + + /* We do not need to set the debug registers for new threads now; + they'll be done at resume anyway. */ + if (linux_nat_lwp_is_new (lp->ptid)) + return 0; + + ptrace (PTRACE_SET_DEBUGREG, TIDGET (lp->ptid), 0, dabr_value); + return 0; +} + /* Set a watchpoint of type TYPE at address ADDR. */ static int ppc_linux_insert_watchpoint (CORE_ADDR addr, int len, int rw) { - int tid; long dabr_value; - ptid_t ptid = inferior_ptid; dabr_value = addr & ~7; switch (rw) @@ -830,61 +844,57 @@ ppc_linux_insert_watchpoint (CORE_ADDR a break; } - tid = TIDGET (ptid); - if (tid == 0) - tid = PIDGET (ptid); - - return ptrace (PTRACE_SET_DEBUGREG, tid, 0, dabr_value); + saved_dabr_value = dabr_value; + iterate_over_lwps (ppc_linux_set_debugreg_lwp, &dabr_value); + return 0; } static int ppc_linux_remove_watchpoint (CORE_ADDR addr, int len, int rw) { - int tid; - ptid_t ptid = inferior_ptid; - - tid = TIDGET (ptid); - if (tid == 0) - tid = PIDGET (ptid); + long dabr_value = 0; - return ptrace (PTRACE_SET_DEBUGREG, tid, 0, 0); + saved_dabr_value = 0; + iterate_over_lwps (ppc_linux_set_debugreg_lwp, &dabr_value); + return 0; } -static int -ppc_linux_stopped_data_address (struct target_ops *target, CORE_ADDR *addr_p) +static void +ppc_linux_debugreg_update (ptid_t ptid) { - if (last_stopped_data_address) - { - *addr_p = last_stopped_data_address; - last_stopped_data_address = 0; - return 1; - } - return 0; + int tid = TIDGET (inferior_ptid); + + if (!linux_nat_lwp_is_new (ptid)) + return; + + if (tid == 0) + tid = PIDGET (inferior_ptid); + ptrace (PTRACE_SET_DEBUGREG, tid, 0, saved_dabr_value); } static int -ppc_linux_stopped_by_watchpoint (void) +ppc_linux_stopped_data_address (struct target_ops *target, CORE_ADDR *addr_p) { - int tid; - struct siginfo siginfo; - ptid_t ptid = inferior_ptid; + struct siginfo *siginfo_p; CORE_ADDR *addr_p; - tid = TIDGET(ptid); - if (tid == 0) - tid = PIDGET (ptid); - - errno = 0; - ptrace (PTRACE_GETSIGINFO, tid, (PTRACE_TYPE_ARG3) 0, &siginfo); + siginfo_p = linux_nat_get_siginfo (inferior_ptid); - if (errno != 0 || siginfo.si_signo != SIGTRAP || - (siginfo.si_code & 0xffff) != 0x0004) + if (siginfo_p->si_signo != SIGTRAP + || (siginfo_p->si_code & 0xffff) != 0x0004 /* TRAP_HWBKPT */) return 0; - last_stopped_data_address = (uintptr_t) siginfo.si_addr; + *addr_p = (CORE_ADDR) siginfo_p->si_addr; return 1; } +static int +ppc_linux_stopped_by_watchpoint (void) +{ + CORE_ADDR addr; + return ppc_linux_stopped_data_address (¤t_target, &addr); +} + static void ppc_linux_store_inferior_registers (struct regcache *regcache, int regno) { @@ -945,6 +955,15 @@ fill_fpregset (const struct regcache *re fpregsetp, sizeof (*fpregsetp)); } +static void (*super_resume) (ptid_t ptid, int step, enum target_signal signal); + +static void +ppc_linux_resume (ptid_t ptid, int step, enum target_signal signal) +{ + ppc_linux_debugreg_update (ptid); + super_resume (ptid, step, signal); +} + void _initialize_ppc_linux_nat (void); void @@ -959,6 +978,10 @@ _initialize_ppc_linux_nat (void) t->to_fetch_registers = ppc_linux_fetch_inferior_registers; t->to_store_registers = ppc_linux_store_inferior_registers; + /* Override the default ptrace resume method. */ + super_resume = t->to_resume; + t->to_resume = ppc_linux_resume; + /* Add our watchpoint methods. */ t->to_can_use_hw_breakpoint = ppc_linux_check_watch_resources; t->to_region_ok_for_hw_watchpoint = ppc_linux_region_ok_for_hw_watchpoint; Index: remote.c =================================================================== RCS file: /cvs/src/src/gdb/remote.c,v retrieving revision 1.266 diff -u -p -r1.266 remote.c --- remote.c 23 Aug 2007 18:08:36 -0000 1.266 +++ remote.c 10 Sep 2007 00:20:25 -0000 @@ -5410,14 +5410,11 @@ remote_stopped_by_watchpoint (void) return remote_stopped_by_watchpoint_p; } -extern int stepped_after_stopped_by_watchpoint; - static int remote_stopped_data_address (struct target_ops *target, CORE_ADDR *addr_p) { int rc = 0; - if (remote_stopped_by_watchpoint () - || stepped_after_stopped_by_watchpoint) + if (remote_stopped_by_watchpoint ()) { *addr_p = remote_watch_data_address; rc = 1; Index: s390-nat.c =================================================================== RCS file: /cvs/src/src/gdb/s390-nat.c,v retrieving revision 1.25 diff -u -p -r1.25 s390-nat.c --- s390-nat.c 23 Aug 2007 18:08:37 -0000 1.25 +++ s390-nat.c 10 Sep 2007 00:20:25 -0000 @@ -252,6 +252,7 @@ s390_stopped_by_watchpoint (void) { per_lowcore_bits per_lowcore; ptrace_area parea; + int result; /* Speed up common case. */ if (!watch_base) @@ -263,14 +264,24 @@ s390_stopped_by_watchpoint (void) if (ptrace (PTRACE_PEEKUSR_AREA, s390_inferior_tid (), &parea) < 0) perror_with_name (_("Couldn't retrieve watchpoint status")); - return per_lowcore.perc_storage_alteration == 1 - && per_lowcore.perc_store_real_address == 0; + result = (per_lowcore.perc_storage_alteration == 1 + && per_lowcore.perc_store_real_address == 0); + + if (result) + { + /* Do not report this watchpoint again. */ + memset (&per_lowcore, 0, sizeof (per_lowcore)); + if (ptrace (PTRACE_POKEUSR_AREA, s390_inferior_tid (), &parea) < 0) + perror_with_name (_("Couldn't clear watchpoint status")); + } + + return result; } static void -s390_fix_watch_points (void) +s390_fix_watch_points (ptid_t ptid) { - int tid = s390_inferior_tid (); + int tid; per_struct per_info; ptrace_area parea; @@ -278,6 +289,10 @@ s390_fix_watch_points (void) CORE_ADDR watch_lo_addr = (CORE_ADDR)-1, watch_hi_addr = 0; struct watch_area *area; + tid = TIDGET (ptid); + if (tid == 0) + tid = PIDGET (ptid); + for (area = watch_base; area; area = area->next) { watch_lo_addr = min (watch_lo_addr, area->lo_addr); @@ -308,6 +323,18 @@ s390_fix_watch_points (void) } static int +s390_fix_watch_points_lwp (struct lwp_info *lp, void *unused) +{ + /* We do not need to set the debug registers for new threads now; + they'll be done at resume anyway. */ + if (linux_nat_lwp_is_new (lp->ptid)) + return 0; + + s390_fix_watch_points (lp->ptid); + return 0; +} + +static int s390_insert_watchpoint (CORE_ADDR addr, int len, int type) { struct watch_area *area = xmalloc (sizeof (struct watch_area)); @@ -320,7 +347,7 @@ s390_insert_watchpoint (CORE_ADDR addr, area->next = watch_base; watch_base = area; - s390_fix_watch_points (); + iterate_over_lwps (s390_fix_watch_points_lwp, NULL); return 0; } @@ -345,7 +372,7 @@ s390_remove_watchpoint (CORE_ADDR addr, *parea = area->next; xfree (area); - s390_fix_watch_points (); + iterate_over_lwps (s390_fix_watch_points_lwp, NULL); return 0; } @@ -361,6 +388,15 @@ s390_region_ok_for_hw_watchpoint (CORE_A return 1; } +static void (*super_resume) (ptid_t ptid, int step, enum target_signal signal); + +static void +s390_resume (ptid_t ptid, int step, enum target_signal signal) +{ + if (linux_nat_lwp_is_new (ptid)) + s390_fix_watch_points (ptid); + super_resume (ptid, step, signal); +} void _initialize_s390_nat (void); @@ -376,6 +412,10 @@ _initialize_s390_nat (void) t->to_fetch_registers = s390_linux_fetch_inferior_registers; t->to_store_registers = s390_linux_store_inferior_registers; + /* Override the default ptrace resume method. */ + super_resume = t->to_resume; + t->to_resume = s390_resume; + /* Add our watchpoint methods. */ t->to_can_use_hw_breakpoint = s390_can_use_hw_breakpoint; t->to_region_ok_for_hw_watchpoint = s390_region_ok_for_hw_watchpoint; Index: testsuite/gdb.threads/watchthreads.c =================================================================== RCS file: /cvs/src/src/gdb/testsuite/gdb.threads/watchthreads.c,v retrieving revision 1.3 diff -u -p -r1.3 watchthreads.c --- testsuite/gdb.threads/watchthreads.c 23 Aug 2007 18:08:50 -0000 1.3 +++ testsuite/gdb.threads/watchthreads.c 10 Sep 2007 00:20:26 -0000 @@ -56,7 +56,7 @@ void *thread_function(void *arg) { /* Don't run forever. Run just short of it :) */ while (*myp > 0) { - (*myp) ++; /* Loop increment. */ + (*myp) ++; usleep (1); /* Loop increment. */ } pthread_exit(NULL); Index: testsuite/gdb.threads/watchthreads.exp =================================================================== RCS file: /cvs/src/src/gdb/testsuite/gdb.threads/watchthreads.exp,v retrieving revision 1.4 diff -u -p -r1.4 watchthreads.exp --- testsuite/gdb.threads/watchthreads.exp 23 Aug 2007 18:14:19 -0000 1.4 +++ testsuite/gdb.threads/watchthreads.exp 10 Sep 2007 00:20:26 -0000 @@ -30,6 +30,10 @@ if [target_info exists gdb,no_hardware_w return 0; } +proc target_no_stopped_data { } { + return [istarget s390*-*-*] +} + set testfile "watchthreads" set srcfile ${testfile}.c set binfile ${objdir}/${subdir}/${testfile} @@ -61,20 +65,56 @@ gdb_test "watch args\[1\]" "Hardware wat set init_line [expr [gdb_get_line_number "Init value"]+1] set inc_line [gdb_get_line_number "Loop increment"] +set main_loc "main \\\(\\\) at .*watchthreads.c:$init_line" +set thread0_loc "thread_function \\\(arg=0x0\\\) at .*watchthreads.c:$inc_line" +set thread1_loc "thread_function \\\(arg=0x1\\\) at .*watchthreads.c:$inc_line" # Loop and continue to allow both watchpoints to be triggered. for {set i 0} {$i < 30} {incr i} { + set test_flag_0 0 + set test_flag_1 0 set test_flag 0 gdb_test_multiple "continue" "threaded watch loop" { - -re "Hardware watchpoint 2: args\\\[0\\\].*Old value = 0.*New value = 1.*main \\\(\\\) at .*watchthreads.c:$init_line.*$gdb_prompt $" - { set args_0 1; set test_flag 1 } - -re "Hardware watchpoint 3: args\\\[1\\\].*Old value = 0.*New value = 1.*main \\\(\\\) at .*watchthreads.c:$init_line.*$gdb_prompt $" - { set args_1 1; set test_flag 1 } - -re "Hardware watchpoint 2: args\\\[0\\\].*Old value = $args_0.*New value = [expr $args_0+1].*in thread_function \\\(arg=0x0\\\) at .*watchthreads.c:$inc_line.*$gdb_prompt $" - { set args_0 [expr $args_0+1]; set test_flag 1 } - -re "Hardware watchpoint 3: args\\\[1\\\].*Old value = $args_1.*New value = [expr $args_1+1].*in thread_function \\\(arg=0x1\\\) at .*watchthreads.c:$inc_line.*$gdb_prompt $" - { set args_1 [expr $args_1+1]; set test_flag 1 } + -re "(.*Hardware watchpoint.*)$gdb_prompt $" { + # At least one hardware watchpoint was hit. Check if both were. + set string $expect_out(1,string) + + if [regexp "Hardware watchpoint 2: args\\\[0\\\]\[^\r\]*\r\[^\r\]*\r\[^\r\]*Old value = $args_0\[^\r\]*\r\[^\r\]*New value = [expr $args_0+1]\r" $string] { + incr args_0 + incr test_flag_0 + } + if [regexp "Hardware watchpoint 3: args\\\[1\\\]\[^\r\]*\r\[^\r\]*\r\[^\r\]*Old value = $args_1\[^\r\]*\r\[^\r\]*New value = [expr $args_1+1]\r" $string] { + incr args_1 + incr test_flag_1 + } + + set expected_loc "bogus location" + if { $test_flag_0 == 1 && $test_flag_1 == 0 && $args_0 == 1 } { + set expected_loc $main_loc + } elseif { $test_flag_0 == 0 && $test_flag_1 == 1 && $args_1 == 1 } { + set expected_loc $main_loc + } elseif { $test_flag_0 == 1 && $test_flag_1 == 0 } { + set expected_loc $thread0_loc + } elseif { $test_flag_0 == 0 && $test_flag_1 == 1 } { + set expected_loc $thread1_loc + } elseif { $test_flag_0 + $test_flag_1 == 2 } { + # On S/390, or any other system which can not report the + # stopped data address, it is OK to report two watchpoints + # at once in this test. Make sure the reported location + # corresponds to at least one of the watchpoints (and not, + # e.g., __nptl_create_event). On other systems, we should + # report the two watchpoints serially. + if { [target_no_stopped_data] } { + set expected_loc "($main_loc|$thread0_loc|$thread1_loc)" + } + } + + if [ regexp "$expected_loc" $string ] { + set test_flag 1 + } + } } + # If we fail above, don't bother continuing loop if { $test_flag == 0 } { set i 30; @@ -84,6 +124,8 @@ for {set i 0} {$i < 30} {incr i} { # Print success message if loop succeeded. if { $test_flag == 1 } { pass "threaded watch loop" +} else { + fail "threaded watch loop" } # Verify that we hit first watchpoint in main thread. @@ -120,7 +162,14 @@ if { $args_1 > 1 } { # Verify that all watchpoint hits are accounted for. set message "combination of threaded watchpoints = 30" -if { [expr $args_0+$args_1] == 30 } { +if { [target_no_stopped_data] } { + # See above. If we allow two watchpoints to be hit at once, we + # may have more than 30 hits total. + set result [expr $args_0 + $args_1 >= 30] +} else { + set result [expr $args_0 + $args_1 == 30] +} +if { $result } { pass $message } else { fail $message