From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 25356 invoked by alias); 10 Jan 2011 17:42:00 -0000 Received: (qmail 25346 invoked by uid 22791); 10 Jan 2011 17:41:59 -0000 X-SWARE-Spam-Status: No, hits=-1.4 required=5.0 tests=AWL,BAYES_00,TW_CP,T_RP_MATCHES_RCVD X-Spam-Check-By: sourceware.org Received: from e24smtp04.br.ibm.com (HELO e24smtp04.br.ibm.com) (32.104.18.25) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Mon, 10 Jan 2011 17:41:53 +0000 Received: from mailhub3.br.ibm.com (mailhub3.br.ibm.com [9.18.232.110]) by e24smtp04.br.ibm.com (8.14.4/8.13.1) with ESMTP id p0AHWN74011312 for ; Mon, 10 Jan 2011 15:32:23 -0200 Received: from d24av04.br.ibm.com (d24av04.br.ibm.com [9.8.31.97]) by mailhub3.br.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id p0AHnSTf2977796 for ; Mon, 10 Jan 2011 15:49:28 -0200 Received: from d24av04.br.ibm.com (loopback [127.0.0.1]) by d24av04.br.ibm.com (8.14.4/8.13.1/NCO v10.0 AVout) with ESMTP id p0AHfkwU004992 for ; Mon, 10 Jan 2011 15:41:46 -0200 Received: from [9.78.138.219] ([9.78.138.219]) by d24av04.br.ibm.com (8.14.4/8.13.1/NCO v10.0 AVin) with ESMTP id p0AHfk1R004981 for ; Mon, 10 Jan 2011 15:41:46 -0200 Subject: [RFC] Make insert/remove breakpoint/watchpoint functions in ppc-linux-nat.c return errors. From: Thiago Jung Bauermann To: gdb-patches ml Content-Type: text/plain; charset="UTF-8" Date: Mon, 10 Jan 2011 17:42:00 -0000 Message-ID: <1294681305.3162.15.camel@hactar> Mime-Version: 1.0 Content-Transfer-Encoding: 7bit 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: 2011-01/txt/msg00193.txt.bz2 Hi, I noticed an issue in ppc-linux-nat.c. I'm under the impression that breakpoint.c:insert_bp_location is not allowed to throw exceptions. But ppc_linux_{insert,remove}_watchpoint and ppc_linux_{insert,remove}_hw_breakpoint throw an exception if ptrace fails. This patches changes them to return -1, which seems to be the correct thing to do. I also took the opportunity to do some formatting fixes on the functions I'm touching. Is my interpretation corret? If so, is this patch ok? -- []'s Thiago Jung Bauermann IBM Linux Technology Center 2011-01-10 Thiago Jung Bauermann * ppc-linux-nat.c (booke_insert_point): Return -1 on error instead of throwing an exception. (booke_remove_point): Likewise. (ppc_linux_insert_watchpoint): Likewise. (ppc_linux_remove_watchpoint): Likewise. (ppc_linux_insert_hw_breakpoint): Likewise. Also, return 1 if hardware breakpoints are not supported. (ppc_linux_remove_hw_breakpoint): Likewise. diff --git a/gdb/ppc-linux-nat.c b/gdb/ppc-linux-nat.c index ca7312b..d76549c 100644 --- a/gdb/ppc-linux-nat.c +++ b/gdb/ppc-linux-nat.c @@ -1551,8 +1551,10 @@ booke_find_thread_points_by_tid (int tid, int alloc_new) /* 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. */ -static void + kernel) and registering it internally in GDB. Returns 0 for success + or -1 for failure. */ + +static int booke_insert_point (struct ppc_hw_breakpoint *b, int tid) { int i; @@ -1565,10 +1567,13 @@ booke_insert_point (struct ppc_hw_breakpoint *b, int tid) memcpy (p, b, sizeof (struct ppc_hw_breakpoint)); - errno = 0; slot = ptrace (PPC_PTRACE_SETHWDEBUG, tid, 0, p); if (slot < 0) - perror_with_name (_("Unexpected error setting breakpoint or watchpoint")); + { + do_cleanups (c); + + return -1; + } /* Everything went fine, so we have to register this *point. */ t = booke_find_thread_points_by_tid (tid, 1); @@ -1587,15 +1592,19 @@ booke_insert_point (struct ppc_hw_breakpoint *b, int tid) gdb_assert (i != max_slots_number); discard_cleanups (c); + + return 0; } /* This function is a generic wrapper that is responsible for removing a *point (i.e., calling `ptrace' in order to issue the request to the - kernel), and unregistering it internally at GDB. */ -static void + kernel), and unregistering it internally at GDB. Returns 0 for + success or -1 for failure. */ + +static int booke_remove_point (struct ppc_hw_breakpoint *b, int tid) { - int i; + int i, ret = 0; struct hw_break_tuple *hw_breaks; struct thread_points *t; @@ -1607,7 +1616,9 @@ booke_remove_point (struct ppc_hw_breakpoint *b, int tid) if (hw_breaks[i].hw_break && booke_cmp_hw_point (hw_breaks[i].hw_break, b)) break; - gdb_assert (i != max_slots_number); + /* There's no such breakpoint/watchpoint for this thread. */ + if (i == max_slots_number) + return -1; /* We have to ignore ENOENT errors because the kernel implements hardware breakpoints/watchpoints as "one-shot", that is, they are automatically @@ -1615,60 +1626,81 @@ booke_remove_point (struct ppc_hw_breakpoint *b, int tid) errno = 0; if (ptrace (PPC_PTRACE_DELHWDEBUG, tid, 0, hw_breaks[i].slot) < 0) if (errno != ENOENT) - perror_with_name (_("Unexpected error deleting breakpoint or watchpoint")); + ret = -1; xfree (hw_breaks[i].hw_break); hw_breaks[i].hw_break = NULL; + + return ret; } +/* Insert the hardware breakpoint described by BP_TGT. Returns 0 for + success, 1 if hardware breakpoints are not supported or -1 for failure. */ + static int ppc_linux_insert_hw_breakpoint (struct gdbarch *gdbarch, - struct bp_target_info *bp_tgt) + struct bp_target_info *bp_tgt) { + int ret; ptid_t ptid; struct lwp_info *lp; struct ppc_hw_breakpoint p; if (!have_ptrace_booke_interface ()) - return -1; - - p.version = PPC_DEBUG_CURRENT_VERSION; - p.trigger_type = PPC_BREAKPOINT_TRIGGER_EXECUTE; - p.addr_mode = PPC_BREAKPOINT_MODE_EXACT; - p.condition_mode = PPC_BREAKPOINT_CONDITION_NONE; - p.addr = (uint64_t) bp_tgt->placed_address; - p.addr2 = 0; + return 1; + + p.version = PPC_DEBUG_CURRENT_VERSION; + p.trigger_type = PPC_BREAKPOINT_TRIGGER_EXECUTE; + p.addr_mode = PPC_BREAKPOINT_MODE_EXACT; + p.condition_mode = PPC_BREAKPOINT_CONDITION_NONE; + p.addr = (uint64_t) bp_tgt->placed_address; + p.addr2 = 0; p.condition_value = 0; ALL_LWPS (lp, ptid) - booke_insert_point (&p, TIDGET (ptid)); + { + ret = booke_insert_point (&p, TIDGET (ptid)); + if (ret) + break; + } - return 0; + /* If an error occurred, remove the breakpoint from all threads, to + keep consistency (all threads must have the same breakpoints and + watchpoints intalled.) */ + if (ret) + ALL_LWPS (lp, ptid) + booke_remove_point (&p, TIDGET (ptid)); + + return ret; } +/* Remove the hardware breakpoint described by BP_TGT. Returns 0 for + success, 1 if hardware breakpoints are not supported or -1 for failure. */ + static int ppc_linux_remove_hw_breakpoint (struct gdbarch *gdbarch, - struct bp_target_info *bp_tgt) + struct bp_target_info *bp_tgt) { + int ret = 0; ptid_t ptid; struct lwp_info *lp; struct ppc_hw_breakpoint p; if (!have_ptrace_booke_interface ()) - return -1; - - p.version = PPC_DEBUG_CURRENT_VERSION; - p.trigger_type = PPC_BREAKPOINT_TRIGGER_EXECUTE; - p.addr_mode = PPC_BREAKPOINT_MODE_EXACT; - p.condition_mode = PPC_BREAKPOINT_CONDITION_NONE; - p.addr = (uint64_t) bp_tgt->placed_address; - p.addr2 = 0; + return 1; + + p.version = PPC_DEBUG_CURRENT_VERSION; + p.trigger_type = PPC_BREAKPOINT_TRIGGER_EXECUTE; + p.addr_mode = PPC_BREAKPOINT_MODE_EXACT; + p.condition_mode = PPC_BREAKPOINT_CONDITION_NONE; + p.addr = (uint64_t) bp_tgt->placed_address; + p.addr2 = 0; p.condition_value = 0; ALL_LWPS (lp, ptid) - booke_remove_point (&p, TIDGET (ptid)); + ret |= booke_remove_point (&p, TIDGET (ptid)); - return 0; + return ret; } static int @@ -1879,13 +1911,18 @@ ppc_linux_can_accel_watchpoint_condition (CORE_ADDR addr, int len, int rw, && check_condition (addr, cond, &data_value)); } +/* Insert a hardware watchpoint at address ADDR, spanning LEN bytes. + RW may be hw_read for a read watchpoint, hw_write for a write watchpoint + or hw_access for an access watchpoint. Returns 0 for success, 1 if + hardware watchpoints are not supported or -1 for failure. */ + static int ppc_linux_insert_watchpoint (CORE_ADDR addr, int len, int rw, struct expression *cond) { struct lwp_info *lp; ptid_t ptid; - int ret = -1; + int ret; if (have_ptrace_booke_interface ()) { @@ -1898,20 +1935,29 @@ ppc_linux_insert_watchpoint (CORE_ADDR addr, int len, int rw, &p.condition_value); else { - p.condition_mode = PPC_BREAKPOINT_CONDITION_NONE; + 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.addr = (uint64_t) addr; - p.addr2 = 0; + p.version = PPC_DEBUG_CURRENT_VERSION; + p.trigger_type = get_trigger_type (rw); + p.addr_mode = PPC_BREAKPOINT_MODE_EXACT; + p.addr = (uint64_t) addr; + p.addr2 = 0; ALL_LWPS (lp, ptid) - booke_insert_point (&p, TIDGET (ptid)); + { + ret = booke_insert_point (&p, TIDGET (ptid)); + if (ret) + break; + } - ret = 0; + /* If an error occurred, remove the breakpoint from all threads, to + keep consistency (all threads must have the same breakpoints and + watchpoints intalled.) */ + if (ret) + ALL_LWPS (lp, ptid) + booke_remove_point (&p, TIDGET (ptid)); } else { @@ -1922,14 +1968,14 @@ ppc_linux_insert_watchpoint (CORE_ADDR addr, int len, int rw, { /* PowerPC 440 requires only the read/write flags to be passed to the kernel. */ - read_mode = 1; + read_mode = 1; write_mode = 2; } else { /* PowerPC 970 and other DABR-based processors are required to pass the Breakpoint Translation bit together with the flags. */ - read_mode = 5; + read_mode = 5; write_mode = 6; } @@ -1963,13 +2009,18 @@ ppc_linux_insert_watchpoint (CORE_ADDR addr, int len, int rw, return ret; } +/* Remove a hardware watchpoint at address ADDR, spanning LEN bytes. + RW may be hw_read for a read watchpoint, hw_write for a write watchpoint + or hw_access for an access watchpoint. Returns 0 for success, 1 if + hardware watchpoints are not supported or -1 for failure. */ + static int ppc_linux_remove_watchpoint (CORE_ADDR addr, int len, int rw, struct expression *cond) { struct lwp_info *lp; ptid_t ptid; - int ret = -1; + int ret = 0; if (have_ptrace_booke_interface ()) { @@ -1982,30 +2033,27 @@ ppc_linux_remove_watchpoint (CORE_ADDR addr, int len, int rw, &p.condition_value); else { - p.condition_mode = PPC_BREAKPOINT_CONDITION_NONE; + 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.addr = (uint64_t) addr; - p.addr2 = 0; + p.version = PPC_DEBUG_CURRENT_VERSION; + p.trigger_type = get_trigger_type (rw); + p.addr_mode = PPC_BREAKPOINT_MODE_EXACT; + p.addr = (uint64_t) addr; + p.addr2 = 0; ALL_LWPS (lp, ptid) - booke_remove_point (&p, TIDGET (ptid)); - - ret = 0; + ret |= booke_remove_point (&p, TIDGET (ptid)); } else { saved_dabr_value = 0; + ALL_LWPS (lp, ptid) if (ptrace (PTRACE_SET_DEBUGREG, TIDGET (ptid), 0, saved_dabr_value) < 0) return -1; - - ret = 0; } return ret;