This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
[RFC] Make insert/remove breakpoint/watchpoint functions in ppc-linux-nat.c return errors.
- From: Thiago Jung Bauermann <bauerman at br dot ibm dot com>
- To: gdb-patches ml <gdb-patches at sourceware dot org>
- Date: Mon, 10 Jan 2011 15:41:45 -0200
- Subject: [RFC] Make insert/remove breakpoint/watchpoint functions in ppc-linux-nat.c return errors.
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 <bauerman@br.ibm.com>
* 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;