From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 21137 invoked by alias); 8 Feb 2010 20:01:27 -0000 Received: (qmail 18569 invoked by uid 22791); 8 Feb 2010 20:01:23 -0000 X-SWARE-Spam-Status: No, hits=-2.7 required=5.0 tests=AWL,BAYES_00,SPF_PASS X-Spam-Check-By: sourceware.org Received: from e24smtp02.br.ibm.com (HELO e24smtp02.br.ibm.com) (32.104.18.86) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Mon, 08 Feb 2010 20:01:15 +0000 Received: from mailhub1.br.ibm.com (mailhub1.br.ibm.com [9.18.232.109]) by e24smtp02.br.ibm.com (8.14.3/8.13.1) with ESMTP id o18KBoMM021645 for ; Mon, 8 Feb 2010 18:11:50 -0200 Received: from d24av04.br.ibm.com (d24av04.br.ibm.com [9.8.31.97]) by mailhub1.br.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id o18K3j741065324 for ; Mon, 8 Feb 2010 18:03:46 -0200 Received: from d24av04.br.ibm.com (loopback [127.0.0.1]) by d24av04.br.ibm.com (8.14.3/8.13.1/NCO v10.0 AVout) with ESMTP id o18K1AM5015018 for ; Mon, 8 Feb 2010 18:01:11 -0200 Received: from hactar.localnet ([9.8.12.111]) by d24av04.br.ibm.com (8.14.3/8.13.1/NCO v10.0 AVin) with ESMTP id o18K19Ta015001 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Mon, 8 Feb 2010 18:01:09 -0200 From: Thiago Jung Bauermann To: Joel Brobecker Subject: Re: [PATCH 2/4] Hardware accelerated watchpoint conditions Date: Mon, 08 Feb 2010 20:01:00 -0000 User-Agent: KMail/1.12.2 (Linux/2.6.30-2-686; KDE/4.3.4; i686; ; ) Cc: gdb-patches@sourceware.org, Luis Machado , Matt Tyrlik References: <200912232231.01798.bauerman@br.ibm.com> <201002041948.33792.bauerman@br.ibm.com> <20100205051555.GA4335@adacore.com> In-Reply-To: <20100205051555.GA4335@adacore.com> MIME-Version: 1.0 Content-Type: Multipart/Mixed; boundary="Boundary-00=_B2GcL1U+PMDDKmB" Message-Id: <201002081801.05214.bauerman@br.ibm.com> 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: 2010-02/txt/msg00227.txt.bz2 --Boundary-00=_B2GcL1U+PMDDKmB Content-Type: Text/Plain; charset="iso-8859-1" Content-Transfer-Encoding: 7bit Content-length: 8726 On Fri 05 Feb 2010 03:15:55 Joel Brobecker wrote: > Good news (see below). Great. :-) > > > I think, from your patch, that GDB will still evaluate the condition > > > once after the watchpoint and its condition trigger. I think that > > > we might want to fix that eventually, but I am actually more than > > > happy to ignore this minor issue for now. I like baby steps :). > > > > If I'm following your thought here, I added that on purpose. Before > > letting GDB evaluate the condition one more time, GDB wouldn't know > > which watchpoint triggered if there were two at the same location, > > with different conditions. > > I knew that! (if the name DiNozzo comes to mind, then I don't > know what you're talking about ;-) ) Oh, sorry then! > Just a few minor nits - pre-approved after the comments are addressed. I addressed your comments. I won't commit the patch yet because the kernel patch enabling this code to work hasn't been committed upstream yet. I expect it will be accepted soon, so I hope to get this in in time for 7.1. The same applies to the first patch in this series (adapting ppc-linux-nat.c to the new ptrace interface), which you approved a while ago... > > +/* This function is used to determine whether the condition associated > > + with bp_location B is of the form: > > + > > + watch *
if == > > + > > + If it is, then it sets DATA_VALUE to LITERAL and returns 1. > > + Otherwise, it returns 0. */ > > Can you add a note that VAR must be stored at ADDRESS too? Same for > similar functions... Done. > > + /* At this point, all verifications were positive, so we can use > > + hardware-assisted data-matching. Set the data value, and return > > + non-zero. */ > > The comment here is a bit too purpose-specific. It betrays its origin :), > but I think we should either drop it (I think that the function description > is sufficently clear), or change it to something more neutral. Again, > same for all other functions that used the same kind of comment. Dropped. > > +/* Return greater than zero if the condition associated with > > + the watchpoint `b' can be treated by the hardware; zero otherwise. > > + > > + Also stores the data value in `data_value'. */ > > I'd drop this comment in breakpoint.h. It duplicates the descriptions > in breakpoint.c... Dropped. > > +static int > > +ppc_linux_can_use_watchpoint_cond_accel (void) > > This function now needs a short description. It's kind of obvious, > what this function does, I agree, but we're trying to be consistent > in our style, and provide always function descriptions unless the > function implements a routine (function pointer) that's already > documented (eg a gdbarch method, or a target_ops routine). I added this concise description: /* Check whether we have at least one free DVC register. */ > > + if (p->hw_breaks[i].hw_break != NULL > > + && p->hw_breaks[i].hw_break->condition_mode > > + != PPC_BREAKPOINT_CONDITION_NONE) > > I'm not quite sure, here, but I think that the GNU Coding Style asks > us to parenthesize your not-equal condition, purely for the benefit of > GNU indent: > > if (p->hw_breaks[i].hw_break != NULL > && (p->hw_breaks[i].hw_break->condition_mode > != PPC_BREAKPOINT_CONDITION_NONE)) > > Can you double-check for me? It does mention that. Changed. > > + if (cond && ppc_linux_can_use_watchpoint_cond_accel () > > + && (watch_address_if_address_equal_literal (exp, cond, &cond_addr) > > + || watch_var_if_var_equal_literal (exp, cond, &cond_addr) > > + || watch_var_if_address_equal_literal (exp, cond, &cond_addr) > > + || watch_address_if_var_equal_literal (exp, cond, &cond_addr))) { > > Formatting nit: The curly brace should be on the next line. There are > a few instances where this needs to be fixed. Fixed this and the other instances. > > + p.condition_mode = PPC_BREAKPOINT_CONDITION_AND > > + | PPC_BREAKPOINT_CONDITION_BE (byte_to_enable); > > Similar to above, I think you'll need to parenthesize the rhs. > Fixed this and the other instance. > > diff --git a/gdb/target.c b/gdb/target.c > > index e6659c9..7adc96e 100644 > > --- a/gdb/target.c > > +++ b/gdb/target.c > > @@ -44,6 +44,8 @@ > > #include "exec.h" > > #include "inline-frame.h" > > > > +struct expression; > > + > > This change is unnecessary (already declared in target.h)... Dropped. > > diff --git a/gdb/target.h b/gdb/target.h > > index 7103ab2..a65e900 100644 > > --- a/gdb/target.h > > +++ b/gdb/target.h > > @@ -36,6 +36,10 @@ struct trace_status; > > struct uploaded_tsv; > > struct uploaded_tp; > > > > +struct bp_location; > > +struct breakpoint; > > +struct expression; > > I don't think that declaring struct bp_location is necessary. > I just see that struct breakpoint should probably have been declared > before your patch - so OK. Dropped struct bp_location, kept the other two. > > + int (*to_remove_watchpoint) (CORE_ADDR, int, int, struct expression > > *, + struct expression *); > > + int (*to_insert_watchpoint) (CORE_ADDR, int, int, struct expression > > *, + struct expression *); > > Would you mind adding a comment explaining that documentation of what > these routines are expected to do is provided with the corresponding > target_* macros? I personally think that the complete description > of what these routines are supposed to do should be right there rather > than with the target_ macros, but that's only a very minor matter and > nothing to do with your patch. Added: /* Documentation of what the two routines below are expected to do is provided with the corresponding target_* macros. */ > > +#ifndef target_region_ok_for_hw_watchpoint > > #define target_region_ok_for_hw_watchpoint(addr, len) \ > > (*current_target.to_region_ok_for_hw_watchpoint) (addr, len) > > - > > +#endif > > This is not right - we no longer allow a macro to be overridden. Ok, dropped. > > /* Set/clear a hardware watchpoint starting at ADDR, for LEN bytes. > > TYPE is 0 for write, 1 for read, and 2 for read/write accesses. Returns > > 0 for success, non-zero for failure. */ > > > > -#define target_insert_watchpoint(addr, len, type) \ > > - (*current_target.to_insert_watchpoint) (addr, len, type) > > +#define target_insert_watchpoint(addr, len, type, watch_exp, cond) \ > > + (*current_target.to_insert_watchpoint) (addr, len, type, watch_exp, > > cond) > > The macro description needs to be updated (to mention the two new > parameters). Updated. > > -#define target_remove_watchpoint(addr, len, type) \ > > - (*current_target.to_remove_watchpoint) (addr, len, type) > > +#define target_remove_watchpoint(addr, len, type, watch_exp, cond) \ > > + (*current_target.to_remove_watchpoint) (addr, len, type, watch_exp, > > cond) > > Can you add a short documentation for this macro? It is already documented, in a comment above target_insert_watchpoint (which is defined right before target_remove_watchpoint): /* Set/clear a hardware watchpoint starting at ADDR, for LEN bytes. TYPE is 0 for write, 1 for read, and 2 for read/write accesses. WATCH_EXP is the watchpoint's expression and COND is the expression for its condition, or NULL if there's none. Returns 0 for success, non-zero for failure. */ -- []'s Thiago Jung Bauermann IBM Linux Technology Center 2010-02-08 Sergio Durigan Junior Thiago Jung Bauermann * target.h: Add opaque declarations for bp_location, breakpoint and expression. (struct target_ops) , : Add new arguments to pass the watchpoint expression and its condition. Update all callers and implementations. (target_region_ok_for_hw_watchpoint): Surround with ifndef. * breakpoint.c (exp_is_address): New function. (exp_is_var): Ditto. (exp_is_address_equal_literal): Ditto. (exp_is_var_equal_literal): Ditto. (watch_address_if_var_equal_literal): Ditto. (watch_var_if_address_equal_literal): Ditto. (watch_var_if_var_equal_literal): Ditto. (watch_address_if_address_equal_literal): Ditto. * breakpoint.h (watch_address_if_address_equal_literal): Declare. (watch_var_if_var_equal_literal): Ditto. (watch_address_if_var_equal_literal): Ditto. (watch_var_if_address_equal_literal): Ditto. * ppc-linux-nat.c (ppc_linux_can_use_watchpoint_cond_accel): New function. (ppc_linux_insert_watchpoint): Check if it is possible to use hardware-accelerated condition checking. (ppc_linux_remove_watchpoint): Check if the watchpoint to delete has hardware-accelerated condition checking. --Boundary-00=_B2GcL1U+PMDDKmB Content-Type: text/x-patch; charset="UTF-8"; name="ppc476-condition.diff" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="ppc476-condition.diff" Content-length: 26461 commit f29957ec421daceca6e557d16549e0efc0358ff8 Author: Thiago Jung Bauermann Date: Mon Dec 28 14:25:01 2009 -0200 Hardware accelerated watchpoint conditions. diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c index 0d55862..98f1cd1 100644 --- a/gdb/breakpoint.c +++ b/gdb/breakpoint.c @@ -1485,9 +1485,10 @@ Note: automatically using hardware breakpoints for read-only addresses.\n")); watchpoints. It's not clear that it's necessary... */ && bpt->owner->disposition != disp_del_at_next_stop) { - val = target_insert_watchpoint (bpt->address, + val = target_insert_watchpoint (bpt->address, bpt->length, - bpt->watchpoint_type); + bpt->watchpoint_type, + bpt->owner->exp, bpt->cond); bpt->inserted = (val != -1); } @@ -2109,12 +2110,10 @@ remove_breakpoint_1 (struct bp_location *b, insertion_state_t is) } else if (b->loc_type == bp_loc_hardware_watchpoint) { - struct value *v; - struct value *n; - b->inserted = (is == mark_inserted); - val = target_remove_watchpoint (b->address, b->length, - b->watchpoint_type); + val = target_remove_watchpoint (b->address, b->length, + b->watchpoint_type, + b->owner->exp, b->cond); /* Failure to remove any of the hardware watchpoints comes here. */ if ((is == mark_uninserted) && (b->inserted)) @@ -10156,6 +10155,210 @@ tracepoint_save_command (char *args, int from_tty) return; } +/* This function checks if the expression is of the form "*
". + It returns 1 if it is, 0 otherwise. */ +static int +exp_is_address (struct expression *exp) +{ + if (exp->nelts != 5 + || exp->elts[0].opcode != UNOP_IND + || exp->elts[1].opcode != OP_LONG) + return 0; + return 1; +} + +/* This function checks if the expression is of the form "". + It returns 1 if it is, 0 otherwise. */ +static int +exp_is_var (struct expression *exp) +{ + if (exp->nelts != 4 + || exp->elts[0].opcode != OP_VAR_VALUE) + return 0; + return 1; +} + +/* This function checks if the expression is of the form + "*
== LITERAL". It returns 1 if it is, 0 otherwise. */ +static int +exp_is_address_equal_literal (struct expression *exp) +{ + if (exp->nelts == 10 + && exp->elts[0].opcode == BINOP_EQUAL + && exp->elts[1].opcode == UNOP_IND + && exp->elts[2].opcode == OP_LONG + && exp->elts[6].opcode == OP_LONG) + return 1; + else if (exp->nelts == 11 + && exp->elts[0].opcode == BINOP_EQUAL + && exp->elts[1].opcode == UNOP_IND + && exp->elts[2].opcode == OP_LONG + && exp->elts[6].opcode == UNOP_NEG + && exp->elts[7].opcode == OP_LONG) + return 1; + + return 0; +} + +/* This function checks if the expression is of the form " == LITERAL". + It returns 1 if it is, 0 otherwise. */ +static int +exp_is_var_equal_literal (struct expression *exp) +{ + if (exp->nelts == 9 + && exp->elts[0].opcode == BINOP_EQUAL + && exp->elts[1].opcode == OP_VAR_VALUE + && exp->elts[5].opcode == OP_LONG) + return 1; + else if (exp->nelts == 10 + && exp->elts[0].opcode == BINOP_EQUAL + && exp->elts[1].opcode == OP_VAR_VALUE + && exp->elts[5].opcode == UNOP_NEG + && exp->elts[6].opcode == OP_LONG) + return 1; + + return 0; +} + +/* This function is used to determine whether the condition associated + with bp_location B is of the form: + + watch *
if == + + It also checks whether is stored at
. If these + conditions are met, then it sets DATA_VALUE to LITERAL and returns 1. + Otherwise, it returns 0. */ +int +watch_address_if_var_equal_literal (struct expression *exp, + struct expression *cond, + CORE_ADDR *data_value) +{ + int pc; + CORE_ADDR exp_address, cond_address; + + if (!exp_is_address (exp) + || !exp_is_var_equal_literal (cond)) + return 0; + + exp_address = exp->elts[3].longconst; + cond_address = value_as_address (address_of_variable (cond->elts[3].symbol, + cond->elts[2].block)); + + /* Make sure that the two addresses are the same. */ + if (exp_address != cond_address) + return 0; + + pc = 5; + *data_value = value_as_long (evaluate_subexp (NULL_TYPE, cond, &pc, + EVAL_NORMAL)); + + return 1; +} + +/* This function is used to determine whether the condition associated + with bp_location B is of the form: + + watch if *
== + + It also checks whether is stored at
. If these + conditions are met, then it sets DATA_VALUE to LITERAL and returns 1. + Otherwise, it returns 0. */ +int +watch_var_if_address_equal_literal (struct expression *exp, + struct expression *cond, + CORE_ADDR *data_value) +{ + int pc; + CORE_ADDR exp_address, cond_address; + + if (!exp_is_var (exp) + || !exp_is_address_equal_literal (cond)) + return 0; + + exp_address = value_as_address (address_of_variable (exp->elts[2].symbol, + exp->elts[1].block)); + cond_address = cond->elts[4].longconst; + + /* Make sure that the two addresses are the same. */ + if (exp_address != cond_address) + return 0; + + pc = 6; + *data_value = value_as_long (evaluate_subexp (NULL_TYPE, cond, &pc, + EVAL_NORMAL)); + + return 1; +} + +/* This function is used to determine whether the condition associated + with bp_location B is of the form: + + watch if == + + It also checks whether and are stored at the same address. + If these conditions are met, then it sets DATA_VALUE to LITERAL and + returns 1. Otherwise, it returns 0. */ +int +watch_var_if_var_equal_literal (struct expression *exp, + struct expression *cond, + CORE_ADDR *data_value) +{ + int pc; + CORE_ADDR exp_address, cond_address; + + if (!exp_is_var (exp) + || !exp_is_var_equal_literal (cond)) + return 0; + + exp_address = value_as_address (address_of_variable (exp->elts[2].symbol, + exp->elts[1].block)); + cond_address = value_as_address (address_of_variable (cond->elts[3].symbol, + cond->elts[2].block)); + + if (exp_address != cond_address) + return 0; + + pc = 5; + *data_value = value_as_long (evaluate_subexp (NULL_TYPE, cond, &pc, + EVAL_NORMAL)); + + return 1; +} + +/* This function is used to determine whether the condition associated + with bp_location B is of the form: + + watch * if * == + + It also checks whether AND are the same. If these + conditions are met, then it sets DATA_VALUE to LITERAL and returns 1. + Otherwise, it returns 0. */ +int +watch_address_if_address_equal_literal (struct expression *exp, + struct expression *cond, + CORE_ADDR *data_value) +{ + int pc; + CORE_ADDR exp_address, cond_address; + + if (!exp_is_address (exp) + || !exp_is_address_equal_literal (cond)) + return 0; + + exp_address = exp->elts[3].longconst; + cond_address = cond->elts[4].longconst; + + /* Make sure that the two addresses are the same. */ + if (exp_address != cond_address) + return 0; + + pc = 6; + *data_value = value_as_long (evaluate_subexp (NULL_TYPE, cond, &pc, + EVAL_NORMAL)); + + return 1; +} + /* Create a vector of all tracepoints. */ VEC(breakpoint_p) * diff --git a/gdb/breakpoint.h b/gdb/breakpoint.h index 6b373a3..1864a84 100644 --- a/gdb/breakpoint.h +++ b/gdb/breakpoint.h @@ -998,4 +998,20 @@ extern struct breakpoint *get_tracepoint_by_number (char **arg, int multi_p, is newly allocated; the caller should free when done with it. */ extern VEC(breakpoint_p) *all_tracepoints (void); +extern int watch_address_if_address_equal_literal (struct expression *exp, + struct expression *cond, + CORE_ADDR *data_value); + +extern int watch_var_if_var_equal_literal (struct expression *exp, + struct expression *cond, + CORE_ADDR *data_value); + +extern int watch_address_if_var_equal_literal (struct expression *exp, + struct expression *cond, + CORE_ADDR *data_value); + +extern int watch_var_if_address_equal_literal (struct expression *exp, + struct expression *cond, + CORE_ADDR *data_value); + #endif /* !defined (BREAKPOINT_H) */ diff --git a/gdb/i386-nat.c b/gdb/i386-nat.c index fa0cce6..1062b74 100644 --- a/gdb/i386-nat.c +++ b/gdb/i386-nat.c @@ -483,7 +483,8 @@ Invalid value %d of operation in i386_handle_nonaligned_watchpoint.\n"), of the type TYPE. Return 0 on success, -1 on failure. */ static int -i386_insert_watchpoint (CORE_ADDR addr, int len, int type) +i386_insert_watchpoint (CORE_ADDR addr, int len, int type, + struct expression *exp, struct expression *cond) { int retval; @@ -507,7 +508,8 @@ i386_insert_watchpoint (CORE_ADDR addr, int len, int type) address ADDR, whose length is LEN bytes, and for accesses of the type TYPE. Return 0 on success, -1 on failure. */ static int -i386_remove_watchpoint (CORE_ADDR addr, int len, int type) +i386_remove_watchpoint (CORE_ADDR addr, int len, int type, + struct expression *exp, struct expression *cond) { int retval; diff --git a/gdb/ia64-linux-nat.c b/gdb/ia64-linux-nat.c index e6a7077..f68f77a 100644 --- a/gdb/ia64-linux-nat.c +++ b/gdb/ia64-linux-nat.c @@ -530,7 +530,8 @@ is_power_of_2 (int val) } static int -ia64_linux_insert_watchpoint (CORE_ADDR addr, int len, int rw) +ia64_linux_insert_watchpoint (CORE_ADDR addr, int len, int rw, + struct expression *exp, struct expression *cond) { struct lwp_info *lp; ptid_t ptid; @@ -584,7 +585,8 @@ ia64_linux_insert_watchpoint (CORE_ADDR addr, int len, int rw) } static int -ia64_linux_remove_watchpoint (CORE_ADDR addr, int len, int type) +ia64_linux_remove_watchpoint (CORE_ADDR addr, int len, int type, + struct expression *exp, struct expression *cond) { int idx; long dbr_addr, dbr_mask; diff --git a/gdb/inf-ttrace.c b/gdb/inf-ttrace.c index c9ab548..dab9065 100644 --- a/gdb/inf-ttrace.c +++ b/gdb/inf-ttrace.c @@ -314,7 +314,8 @@ inf_ttrace_disable_page_protections (pid_t pid) type TYPE. */ static int -inf_ttrace_insert_watchpoint (CORE_ADDR addr, int len, int type) +inf_ttrace_insert_watchpoint (CORE_ADDR addr, int len, int type, + struct expression *exp, struct expression *cond) { const int pagesize = inf_ttrace_page_dict.pagesize; pid_t pid = ptid_get_pid (inferior_ptid); @@ -337,7 +338,8 @@ inf_ttrace_insert_watchpoint (CORE_ADDR addr, int len, int type) type TYPE. */ static int -inf_ttrace_remove_watchpoint (CORE_ADDR addr, int len, int type) +inf_ttrace_remove_watchpoint (CORE_ADDR addr, int len, int type, + struct expression *exp, struct expression *cond) { const int pagesize = inf_ttrace_page_dict.pagesize; pid_t pid = ptid_get_pid (inferior_ptid); diff --git a/gdb/mips-linux-nat.c b/gdb/mips-linux-nat.c index fe05192..9d36a39 100644 --- a/gdb/mips-linux-nat.c +++ b/gdb/mips-linux-nat.c @@ -926,7 +926,8 @@ populate_regs_from_watches (struct pt_watch_regs *regs) watch. Return zero on success. */ static int -mips_linux_insert_watchpoint (CORE_ADDR addr, int len, int type) +mips_linux_insert_watchpoint (CORE_ADDR addr, int len, int type, + struct expression *exp, struct expression *cond) { struct pt_watch_regs regs; struct mips_watchpoint *new_watch; @@ -975,7 +976,8 @@ mips_linux_insert_watchpoint (CORE_ADDR addr, int len, int type) Return zero on success. */ static int -mips_linux_remove_watchpoint (CORE_ADDR addr, int len, int type) +mips_linux_remove_watchpoint (CORE_ADDR addr, int len, int type, + struct expression *exp, struct expression *cond) { int retval; int deleted_one; diff --git a/gdb/nto-procfs.c b/gdb/nto-procfs.c index d8f3c91..d37f259 100644 --- a/gdb/nto-procfs.c +++ b/gdb/nto-procfs.c @@ -1509,13 +1509,15 @@ procfs_can_use_hw_breakpoint (int type, int cnt, int othertype) } static int -procfs_remove_hw_watchpoint (CORE_ADDR addr, int len, int type) +procfs_remove_hw_watchpoint (CORE_ADDR addr, int len, int type, + struct expression *exp, struct expression *cond) { return procfs_hw_watchpoint (addr, -1, type); } static int -procfs_insert_hw_watchpoint (CORE_ADDR addr, int len, int type) +procfs_insert_hw_watchpoint (CORE_ADDR addr, int len, int type, + struct expression *exp, struct expression *cond) { return procfs_hw_watchpoint (addr, len, type); } diff --git a/gdb/ppc-linux-nat.c b/gdb/ppc-linux-nat.c index 9c76387..56c5aa7 100644 --- a/gdb/ppc-linux-nat.c +++ b/gdb/ppc-linux-nat.c @@ -1517,6 +1517,36 @@ booke_find_thread_points_by_tid (int tid, int alloc_new) return t; } +/* Check whether we have at least one free DVC register. */ +static int +ppc_linux_can_use_watchpoint_cond_accel (void) +{ + struct thread_points *p; + int tid = TIDGET (inferior_ptid); + int cnt = booke_debug_info.num_condition_regs, i; + CORE_ADDR tmp_value; + + if (!have_ptrace_new_debug_booke) + return 0; + + p = booke_find_thread_points_by_tid (tid, 0); + + if (p) + { + for (i = 0; i < max_slots_number; i++) + if (p->hw_breaks[i].hw_break != NULL + && (p->hw_breaks[i].hw_break->condition_mode + != PPC_BREAKPOINT_CONDITION_NONE)) + cnt--; + + /* There are no available slots now. */ + if (cnt <= 0) + return 0; + } + + return 1; +} + /* This function is a generic wrapper that is responsible for inserting a *point (i.e., calling `ptrace' in order to issue the request to the kernel) and registering it internally in GDB. */ @@ -1658,7 +1688,8 @@ get_trigger_type (int rw) } static int -ppc_linux_insert_watchpoint (CORE_ADDR addr, int len, int rw) +ppc_linux_insert_watchpoint (CORE_ADDR addr, int len, int rw, + struct expression *exp, struct expression *cond) { struct lwp_info *lp; ptid_t ptid; @@ -1666,15 +1697,33 @@ ppc_linux_insert_watchpoint (CORE_ADDR addr, int len, int rw) if (have_ptrace_new_debug_booke) { + int byte_to_enable; struct ppc_hw_breakpoint p; + CORE_ADDR cond_addr; + + if (cond && ppc_linux_can_use_watchpoint_cond_accel () + && (watch_address_if_address_equal_literal (exp, cond, &cond_addr) + || watch_var_if_var_equal_literal (exp, cond, &cond_addr) + || watch_var_if_address_equal_literal (exp, cond, &cond_addr) + || watch_address_if_var_equal_literal (exp, cond, &cond_addr))) + { + byte_to_enable = addr % 4; + cond_addr >>= (byte_to_enable * 8); + p.condition_mode = (PPC_BREAKPOINT_CONDITION_AND + | PPC_BREAKPOINT_CONDITION_BE (byte_to_enable)); + p.condition_value = (uint64_t) cond_addr; + } + else + { + p.condition_mode = PPC_BREAKPOINT_CONDITION_NONE; + p.condition_value = 0; + } p.version = PPC_DEBUG_CURRENT_VERSION; p.trigger_type = get_trigger_type (rw); p.addr_mode = PPC_BREAKPOINT_MODE_EXACT; - p.condition_mode = PPC_BREAKPOINT_CONDITION_NONE; p.addr = (uint64_t) addr; p.addr2 = 0; - p.condition_value = 0; ALL_LWPS (lp, ptid) booke_insert_point (&p, TIDGET (ptid)); @@ -1721,7 +1770,8 @@ ppc_linux_insert_watchpoint (CORE_ADDR addr, int len, int rw) saved_dabr_value = dabr_value; ALL_LWPS (lp, ptid) - if (ptrace (PTRACE_SET_DEBUGREG, TIDGET (ptid), 0, saved_dabr_value) < 0) + if (ptrace (PTRACE_SET_DEBUGREG, TIDGET (ptid), 0, + saved_dabr_value) < 0) return -1; ret = 0; @@ -1731,7 +1781,8 @@ ppc_linux_insert_watchpoint (CORE_ADDR addr, int len, int rw) } static int -ppc_linux_remove_watchpoint (CORE_ADDR addr, int len, int rw) +ppc_linux_remove_watchpoint (CORE_ADDR addr, int len, int rw, + struct expression *exp, struct expression *cond) { struct lwp_info *lp; ptid_t ptid; @@ -1739,15 +1790,33 @@ ppc_linux_remove_watchpoint (CORE_ADDR addr, int len, int rw) if (have_ptrace_new_debug_booke) { + int byte_to_enable; struct ppc_hw_breakpoint p; + CORE_ADDR cond_addr; + + if (cond && ppc_linux_can_use_watchpoint_cond_accel () + && (watch_address_if_address_equal_literal (exp, cond, &cond_addr) + || watch_var_if_var_equal_literal (exp, cond, &cond_addr) + || watch_var_if_address_equal_literal (exp, cond, &cond_addr) + || watch_address_if_var_equal_literal (exp, cond, &cond_addr))) + { + byte_to_enable = addr % 4; + cond_addr >>= (byte_to_enable * 8); + p.condition_mode = (PPC_BREAKPOINT_CONDITION_AND + | PPC_BREAKPOINT_CONDITION_BE (byte_to_enable)); + p.condition_value = (uint64_t) cond_addr; + } + else + { + p.condition_mode = PPC_BREAKPOINT_CONDITION_NONE; + p.condition_value = 0; + } p.version = PPC_DEBUG_CURRENT_VERSION; p.trigger_type = get_trigger_type (rw); p.addr_mode = PPC_BREAKPOINT_MODE_EXACT; - p.condition_mode = PPC_BREAKPOINT_CONDITION_NONE; p.addr = (uint64_t) addr; p.addr2 = 0; - p.condition_value = 0; ALL_LWPS (lp, ptid) booke_remove_point (&p, TIDGET (ptid)); @@ -1758,7 +1827,8 @@ ppc_linux_remove_watchpoint (CORE_ADDR addr, int len, int rw) { saved_dabr_value = 0; ALL_LWPS (lp, ptid) - if (ptrace (PTRACE_SET_DEBUGREG, TIDGET (ptid), 0, saved_dabr_value) < 0) + if (ptrace (PTRACE_SET_DEBUGREG, TIDGET (ptid), 0, + saved_dabr_value) < 0) return -1; ret = 0; diff --git a/gdb/procfs.c b/gdb/procfs.c index 1dd39b2..f373511 100644 --- a/gdb/procfs.c +++ b/gdb/procfs.c @@ -5502,7 +5502,8 @@ procfs_stopped_by_watchpoint (void) } static int -procfs_insert_watchpoint (CORE_ADDR addr, int len, int type) +procfs_insert_watchpoint (CORE_ADDR addr, int len, int type, + struct expression *exp, struct expression *cond) { if (!target_have_steppable_watchpoint && !gdbarch_have_nonsteppable_watchpoint (target_gdbarch)) @@ -5523,7 +5524,8 @@ procfs_insert_watchpoint (CORE_ADDR addr, int len, int type) } static int -procfs_remove_watchpoint (CORE_ADDR addr, int len, int type) +procfs_remove_watchpoint (CORE_ADDR addr, int len, int type, + struct expression *exp, struct expression *cond) { return procfs_set_watchpoint (inferior_ptid, addr, 0, 0, 0); } diff --git a/gdb/remote-m32r-sdi.c b/gdb/remote-m32r-sdi.c index be6a564..675bf15 100644 --- a/gdb/remote-m32r-sdi.c +++ b/gdb/remote-m32r-sdi.c @@ -1415,7 +1415,8 @@ m32r_can_use_hw_watchpoint (int type, int cnt, int othertype) watchpoint. */ static int -m32r_insert_watchpoint (CORE_ADDR addr, int len, int type) +m32r_insert_watchpoint (CORE_ADDR addr, int len, int type, + struct expression *exp, struct expression *cond) { int i; @@ -1439,7 +1440,8 @@ m32r_insert_watchpoint (CORE_ADDR addr, int len, int type) } static int -m32r_remove_watchpoint (CORE_ADDR addr, int len, int type) +m32r_remove_watchpoint (CORE_ADDR addr, int len, int type, + struct expression *exp, struct expression *cond) { int i; diff --git a/gdb/remote-mips.c b/gdb/remote-mips.c index f2fb8f3..cc4d9f8 100644 --- a/gdb/remote-mips.c +++ b/gdb/remote-mips.c @@ -2296,7 +2296,8 @@ calculate_mask (CORE_ADDR addr, int len) watchpoint. */ int -mips_insert_watchpoint (CORE_ADDR addr, int len, int type) +mips_insert_watchpoint (CORE_ADDR addr, int len, int type, + struct expression *exp, struct expression *cond) { if (mips_set_breakpoint (addr, len, type)) return -1; @@ -2305,7 +2306,8 @@ mips_insert_watchpoint (CORE_ADDR addr, int len, int type) } int -mips_remove_watchpoint (CORE_ADDR addr, int len, int type) +mips_remove_watchpoint (CORE_ADDR addr, int len, int type, + struct expression *exp, struct expression *cond) { if (mips_clear_breakpoint (addr, len, type)) return -1; diff --git a/gdb/remote.c b/gdb/remote.c index 709e424..c8a8906 100644 --- a/gdb/remote.c +++ b/gdb/remote.c @@ -7339,7 +7339,8 @@ watchpoint_to_Z_packet (int type) } static int -remote_insert_watchpoint (CORE_ADDR addr, int len, int type) +remote_insert_watchpoint (CORE_ADDR addr, int len, int type, + struct expression *watch_exp, struct expression *cond) { struct remote_state *rs = get_remote_state (); char *p; @@ -7371,7 +7372,8 @@ remote_insert_watchpoint (CORE_ADDR addr, int len, int type) static int -remote_remove_watchpoint (CORE_ADDR addr, int len, int type) +remote_remove_watchpoint (CORE_ADDR addr, int len, int type, + struct expression *watch_exp, struct expression *cond) { struct remote_state *rs = get_remote_state (); char *p; diff --git a/gdb/s390-nat.c b/gdb/s390-nat.c index 3af42ff..41ebf9b 100644 --- a/gdb/s390-nat.c +++ b/gdb/s390-nat.c @@ -335,7 +335,8 @@ s390_fix_watch_points (ptid_t ptid) } static int -s390_insert_watchpoint (CORE_ADDR addr, int len, int type) +s390_insert_watchpoint (CORE_ADDR addr, int len, int type, + struct expression *exp, struct expression *cond) { struct lwp_info *lp; ptid_t ptid; @@ -356,7 +357,8 @@ s390_insert_watchpoint (CORE_ADDR addr, int len, int type) } static int -s390_remove_watchpoint (CORE_ADDR addr, int len, int type) +s390_remove_watchpoint (CORE_ADDR addr, int len, int type, + struct expression *exp, struct expression *cond) { struct lwp_info *lp; ptid_t ptid; diff --git a/gdb/target.c b/gdb/target.c index e6659c9..95ab2fc 100644 --- a/gdb/target.c +++ b/gdb/target.c @@ -116,9 +116,13 @@ static int debug_to_insert_hw_breakpoint (struct gdbarch *, static int debug_to_remove_hw_breakpoint (struct gdbarch *, struct bp_target_info *); -static int debug_to_insert_watchpoint (CORE_ADDR, int, int); +static int debug_to_insert_watchpoint (CORE_ADDR, int, int, + struct expression *, + struct expression *); -static int debug_to_remove_watchpoint (CORE_ADDR, int, int); +static int debug_to_remove_watchpoint (CORE_ADDR, int, int, + struct expression *, + struct expression *); static int debug_to_stopped_by_watchpoint (void); @@ -705,10 +709,12 @@ update_current_target (void) (int (*) (struct gdbarch *, struct bp_target_info *)) return_minus_one); de_fault (to_insert_watchpoint, - (int (*) (CORE_ADDR, int, int)) + (int (*) (CORE_ADDR, int, int, struct expression *, + struct expression *)) return_minus_one); de_fault (to_remove_watchpoint, - (int (*) (CORE_ADDR, int, int)) + (int (*) (CORE_ADDR, int, int, struct expression *, + struct expression *)) return_minus_one); de_fault (to_stopped_by_watchpoint, (int (*) (void)) @@ -3252,11 +3258,13 @@ debug_to_remove_hw_breakpoint (struct gdbarch *gdbarch, } static int -debug_to_insert_watchpoint (CORE_ADDR addr, int len, int type) +debug_to_insert_watchpoint (CORE_ADDR addr, int len, int type, + struct expression *watch_exp, + struct expression *cond) { int retval; - retval = debug_target.to_insert_watchpoint (addr, len, type); + retval = debug_target.to_insert_watchpoint (addr, len, type, watch_exp, cond); fprintf_unfiltered (gdb_stdlog, "target_insert_watchpoint (0x%lx, %d, %d) = %ld\n", @@ -3265,11 +3273,13 @@ debug_to_insert_watchpoint (CORE_ADDR addr, int len, int type) } static int -debug_to_remove_watchpoint (CORE_ADDR addr, int len, int type) +debug_to_remove_watchpoint (CORE_ADDR addr, int len, int type, + struct expression *watch_exp, + struct expression *cond) { int retval; - retval = debug_target.to_remove_watchpoint (addr, len, type); + retval = debug_target.to_remove_watchpoint (addr, len, type, watch_exp, cond); fprintf_unfiltered (gdb_stdlog, "target_remove_watchpoint (0x%lx, %d, %d) = %ld\n", diff --git a/gdb/target.h b/gdb/target.h index 7103ab2..4f747c5 100644 --- a/gdb/target.h +++ b/gdb/target.h @@ -36,6 +36,9 @@ struct trace_status; struct uploaded_tsv; struct uploaded_tp; +struct breakpoint; +struct expression; + /* This include file defines the interface between the main part of the debugger, and the part which is target-specific, or specific to the communications interface between us and the @@ -420,8 +423,14 @@ struct target_ops int (*to_can_use_hw_breakpoint) (int, int, int); int (*to_insert_hw_breakpoint) (struct gdbarch *, struct bp_target_info *); int (*to_remove_hw_breakpoint) (struct gdbarch *, struct bp_target_info *); - int (*to_remove_watchpoint) (CORE_ADDR, int, int); - int (*to_insert_watchpoint) (CORE_ADDR, int, int); + + /* Documentation of what the two routines below are expected to do is + provided with the corresponding target_* macros. */ + int (*to_remove_watchpoint) (CORE_ADDR, int, int, struct expression *, + struct expression *); + int (*to_insert_watchpoint) (CORE_ADDR, int, int, struct expression *, + struct expression *); + int (*to_stopped_by_watchpoint) (void); int to_have_steppable_watchpoint; int to_have_continuable_watchpoint; @@ -1263,16 +1272,16 @@ extern char *normal_pid_to_str (ptid_t ptid); #define target_region_ok_for_hw_watchpoint(addr, len) \ (*current_target.to_region_ok_for_hw_watchpoint) (addr, len) - /* Set/clear a hardware watchpoint starting at ADDR, for LEN bytes. TYPE is 0 - for write, 1 for read, and 2 for read/write accesses. Returns 0 for - success, non-zero for failure. */ + for write, 1 for read, and 2 for read/write accesses. WATCH_EXP is the + watchpoint's expression and COND is the expression for its condition, + or NULL if there's none. Returns 0 for success, non-zero for failure. */ -#define target_insert_watchpoint(addr, len, type) \ - (*current_target.to_insert_watchpoint) (addr, len, type) +#define target_insert_watchpoint(addr, len, type, watch_exp, cond) \ + (*current_target.to_insert_watchpoint) (addr, len, type, watch_exp, cond) -#define target_remove_watchpoint(addr, len, type) \ - (*current_target.to_remove_watchpoint) (addr, len, type) +#define target_remove_watchpoint(addr, len, type, watch_exp, cond) \ + (*current_target.to_remove_watchpoint) (addr, len, type, watch_exp, cond) #define target_insert_hw_breakpoint(gdbarch, bp_tgt) \ (*current_target.to_insert_hw_breakpoint) (gdbarch, bp_tgt) --Boundary-00=_B2GcL1U+PMDDKmB--