* [PATCH v2 3/3] [PowerPC] Fix debug register issues in ppc-linux-nat
2020-02-14 20:55 [PATCH v2 0/3] low_new_clone in linux-nat.c and powerpc watchpoint fixes Pedro Franco de Carvalho
@ 2020-02-14 20:55 ` Pedro Franco de Carvalho
2020-02-17 17:47 ` Ulrich Weigand
2020-02-14 20:55 ` [PATCH v2 1/3] Add low_new_clone method to linux_nat_target Pedro Franco de Carvalho
2020-02-14 20:55 ` [PATCH v2 2/3] [PowerPC] Move up some register access routines Pedro Franco de Carvalho
2 siblings, 1 reply; 15+ messages in thread
From: Pedro Franco de Carvalho @ 2020-02-14 20:55 UTC (permalink / raw)
To: gdb-patches; +Cc: ulrich.weigand, rcardoso
This patch fixes some issues with debug register handling for the powerpc
linux native target.
Currently, the target methods for installing and removing hardware
breakpoints and watchpoints in ppc-linux-nat.c affect all threads known to
linux-nat, including threads of different processes.
This patch changes ppc-linux-nat.c so that only the process of
inferior_ptid is affected by these target methods, as GDB expects.
This is done in the same way as various other architectures. The
install/remove target methods only register a hardware breakpoint or
watchpoint, and then send a stop signal to the threads. The debug
registers are only changed with ptrace right before each thread is next
resumed, using low_prepare_to_resume.
There are two interfaces to modify debug registers for linux running on
powerpc, with different sets of ptrace requests:
- PPC_PTRACE_GETHWDBGINFO, PPC_PTRACE_SETHWDEBUG, and
PPC_PTRACE_DELHWDEBUG.
Or
- PTRACE_SET_DEBUGREG and PTRACE_GET_DEBUGREG
The first set (HWDEBUG) is the more flexible one and allows setting
watchpoints with a variable watched region length and, for certain
embedded processors, multiple types of debug registers (e.g. hardware
breakpoints and hardware-assisted conditions for watchpoints).
Currently, server processors only provide one watchpoint. The second one
(DEBUGREG) only allows setting one debug register, a watchpoint, so we
only use it if the first one is not available.
The HWDEBUG interface handles debug registers with slot numbers. Once a
hardware watchpoint or breakpoint is installed (with
PPC_PTRACE_SETHWDEBUG), ptrace returns a slot number. This slot number
can then be used to remove the watchpoint or breakpoint from the inferior
(with PPC_PTRACE_DELHWDEBUG). The first interface also provides a
bitmask of available debug register features, which can be obtained with
PPC_PTRACE_GETHWDBGINFO.
When GDB first tries to use debug registers, we try the first interface
with a ptrace call, and if it isn't available, we fall back to the second
one, if available. We use EIO as an indicator that an interface is not
available in the kernel. For simplicity, with any other error we
immediately assume no interface is available. Unfortunately this means
that if a process is killed by a signal right before we try to detect the
interface, we might get an ESRCH, which would prevent debug registers to
be used in the GDB session. However, it isn't clear that we can safely
raise an exception and try again in the future in all the contexts where
we try to detect the interface.
If the HWDEBUG interface works but provides no feature bits, the target
falls back to the DEBUGREG interface. When the kernel is configured
without CONFIG_HW_BREAKPOINTS (selected by CONFIG_PERF_EVENTS), there is
a bug that causes watchpoints installed with the HWDEBUG interface not to
trigger. When this is the case, the feature bits will be zero, which is
used as the indicator to fall back to the DEBUGREG interface. This isn't
ideal, but has always been the behavior of GDB before this patch, so I
decided not to change it.
A flag indicates for each thread if its debug registers need to be
updated the next time it is resumed. The flag is set whenever the upper
layers request or remove a hardware watchpoint or breakpoint, or when a
new thread is detected. Because some kernel configurations disable
watchpoints after they are hit, we also use the last stop reason of the
LWP to determine whether we should update the debug registers. It isn't
clear that this is also true of BookE hardware breakpoints, but we also
check their stop reason to be on the safe side, since it doesn't hurt.
A map from process numbers to hardware watchpoint or breakpoint objects
keeps track of what has been requested by the upper layers of GDB, since
for GDB installing a hardware watchpoint or breakpoint means doing so for
the whole process.
When using the HWDEBUG interface we also have to keep track of which
slots were last installed in each thread with a map from threads to the
slots, so that they can be removed when needed. When resuming a thread,
we remove all the slots using this map, then we install all the hardware
watchpoints and breakpoints from the per-process map of requests, and
then update the per-thread map accordingly.
This per-thread state is also used for copying the debug register state
after a fork or a clone is detected. The kernel might do this depending
on the configuration. Recent kernels running on server processors that
were configured with CONFIG_PERF_EVENTS (and therefore
CONFIG_HW_BREAKPOINTS) don't copy debug registers across forks and
clones. Recent kernels without CONFIG_HW_BREAKPOINTS copy this state. I
believe that on embedded processors (e.g. a ppc440) the debug register
state is copied, but I haven't been able to test this. To handle both
cases, the per-thread state is always copied when forks and clones are
detected, and when we resume the thread and delete the debug register
slots before updating them, we ignore ENOENT errors.
We don't need to handle this when using the DEBUGREG interface since it
only allows one hardware watchpoint and doesn't return slot numbers, we
just set or clear this watchpoint when needed.
Since we signal running threads to stop after a request is processed, so
that we can update their debug registers when they are next resumed,
there will be a time between signalling the threads and their stop during
which the debug registers haven't been updated, even if the target
methods completed.
The tests in gdb.threads/watchpoint-fork.exp no longer fail with this
patch.
gdb/ChangeLog:
YYYY-MM-DD Pedro Franco de Carvalho <pedromfc@linux.ibm.com>
* ppc-linux-nat.c: Include <algorithm>, <unordered_map>, and
<list>. Remove inlcusion of observable.h.
(PPC_DEBUG_CURRENT_VERSION): Move up define.
(struct arch_lwp_info): New struct.
(class ppc_linux_dreg_interface): New class.
(struct ppc_linux_nat_target) <low_delete_thread, low_new_fork>
<low_new_clone, low_forget_process, low_prepare_to_resume>
<copy_thread_dreg_state, mark_thread_stale>
<mark_debug_registers_changed, register_hw_breakpoint>
<clear_hw_breakpoint, register_wp, clear_wp>
<can_use_watchpoint_cond_accel, calculate_dvc, check_condition>
<num_memory_accesses, get_trigger_type>
<create_watchpoint_request, hwdebug_point_cmp>
<init_arch_lwp_info, get_arch_lwp_info>
<low_stopped_by_watchpoint, low_stopped_data_address>: Declare as
methods.
<struct ptid_hash>: New inner struct.
<m_dreg_interface, m_requested_hw_bps, m_requested_wp_vals>
<m_installed_hw_bps>: Declare members.
(saved_dabr_value, hwdebug_info, max_slots_number)
(struct hw_break_tuple, struct thread_points, ppc_threads)
(have_ptrace_hwdebug_interface)
(hwdebug_find_thread_points_by_tid)
(hwdebug_insert_point, hwdebug_remove_point): Remove.
(ppc_linux_nat_target::can_use_hw_breakpoint): Use
m_dreg_interface, remove call to PTRACE_SET_DEBUGREG.
(ppc_linux_nat_target::region_ok_for_hw_watchpoint): Add comment,
use m_dreg_interface.
(hwdebug_point_cmp): Change to...
(ppc_linux_nat_target::hwdebug_point_cmp): ...this method. Use
reference arguments instead of pointers.
(ppc_linux_nat_target::ranged_break_num_registers): Use
m_dreg_interface.
(ppc_linux_nat_target::insert_hw_breakpoint): Add comment, use
m_dreg_interface. Call register_hw_breakpoint.
(ppc_linux_nat_target::remove_hw_breakpoint): Add comment, use
m_dreg_interface. Call clear_hw_breakpoint.
(get_trigger_type): Change to...
(ppc_linux_nat_target::get_trigger_type): ...this method. Add
comment.
(ppc_linux_nat_target::insert_mask_watchpoint): Update comment,
use m_dreg_interface. Call register_hw_breakpoint.
(ppc_linux_nat_target::remove_mask_watchpoint): Update comment,
use m_dreg_interface. Call clear_hw_breakpoint.
(can_use_watchpoint_cond_accel): Change to...
(ppc_linux_nat_target::can_use_watchpoint_cond_accel): ...this
method. Update comment, use m_dreg_interface and
m_process_hw_breakpoints.
(calculate_dvc): Change to...
(ppc_linux_nat_target::calculate_dvc): ...this method. Use
m_dreg_interface.
(num_memory_accesses): Change to...
(ppc_linux_nat_target::num_memory_accesses): ...this method.
(check_condition): Change to...
(ppc_linux_nat_target::check_condition): ...this method.
(ppc_linux_nat_target::can_accel_watchpoint_condition): Update
comment, use m_dreg_interface.
(create_watchpoint_request): Change to...
(ppc_linux_nat_target::create_watchpoint_request): ...this
method. Use m_dreg_interface.
(ppc_linux_nat_target::insert_watchpoint): Add comment, use
m_dreg_interface. Call register_hw_breakpoint or register_wp.
(ppc_linux_nat_target::remove_watchpoint): Add comment, use
m_dreg_interface. Call clear_hw_breakpoint or clear_wp.
(ppc_linux_nat_target::low_forget_process)
(ppc_linux_nat_target::low_new_fork)
(ppc_linux_nat_target::low_new_clone)
(ppc_linux_nat_target::low_delete_thread)
(ppc_linux_nat_target::low_prepare_to_resume): New methods.
(ppc_linux_nat_target::low_new_thread): Remove previous logic,
only call mark_thread_stale.
(ppc_linux_thread_exit): Remove.
(ppc_linux_nat_target::stopped_data_address): Change to...
(ppc_linux_nat_target::low_stopped_data_address): This. Add
comment, use m_dreg_interface and m_thread_hw_breakpoints.
(ppc_linux_nat_target::stopped_by_watchpoint): Change to...
(ppc_linux_nat_target::stopped_by_watchpoint): This. Add
comment. Call low_stopped_data_address.
(ppc_linux_nat_target::watchpoint_addr_within_range): Use
m_dreg_interface.
(ppc_linux_nat_target::masked_watch_num_registers): Use
m_dreg_interface.
(ppc_linux_nat_target::copy_thread_dreg_state)
(ppc_linux_nat_target::mark_thread_stale)
(ppc_linux_nat_target::mark_debug_registers_changed)
(ppc_linux_nat_target::register_hw_breakpoint)
(ppc_linux_nat_target::clear_hw_breakpoint)
(ppc_linux_nat_target::register_wp)
(ppc_linux_nat_target::clear_wp)
(ppc_linux_nat_target::init_arch_lwp_info)
(ppc_linux_nat_target::get_arch_lwp_info): New methods.
(_initialize_ppc_linux_nat): Remove observer callback.
---
gdb/ppc-linux-nat.c | 1308 ++++++++++++++++++++++++++++++-------------
1 file changed, 912 insertions(+), 396 deletions(-)
diff --git a/gdb/ppc-linux-nat.c b/gdb/ppc-linux-nat.c
index 2295406234..0382345c0f 100644
--- a/gdb/ppc-linux-nat.c
+++ b/gdb/ppc-linux-nat.c
@@ -18,7 +18,6 @@
along with this program. If not, see <http://www.gnu.org/licenses/>. */
#include "defs.h"
-#include "observable.h"
#include "frame.h"
#include "inferior.h"
#include "gdbthread.h"
@@ -38,6 +37,9 @@
#include "nat/gdb_ptrace.h"
#include "nat/linux-ptrace.h"
#include "inf-ptrace.h"
+#include <algorithm>
+#include <unordered_map>
+#include <list>
/* Prototypes for supply_gregset etc. */
#include "gregset.h"
@@ -136,6 +138,10 @@ struct ppc_hw_breakpoint
#define PPC_DEBUG_FEATURE_DATA_BP_DAWR 0x10
#endif /* PPC_DEBUG_FEATURE_DATA_BP_DAWR */
+/* The version of the PowerPC HWDEBUG kernel interface that we will use, if
+ available. */
+#define PPC_DEBUG_CURRENT_VERSION 1
+
/* Similarly for the general-purpose (gp0 -- gp31)
and floating-point registers (fp0 -- fp31). */
#ifndef PTRACE_GETREGS
@@ -270,6 +276,199 @@ int have_ptrace_getsetregs = 1;
them and gotten an error. */
int have_ptrace_getsetfpregs = 1;
+/* Private arch info associated with each thread lwp_info object, used
+ for debug register handling. */
+
+struct arch_lwp_info
+{
+ /* When true, indicates that the debug registers installed in the
+ thread no longer correspond to the watchpoints and breakpoints
+ requested by GDB. */
+ bool debug_regs_stale;
+
+ /* We need a back-reference to the PTID of the thread so that we can
+ cleanup the debug register state of the thread in
+ low_delete_thread. */
+ ptid_t lwp_ptid;
+};
+
+/* Class used to detect which set of ptrace requests that
+ ppc_linux_nat_target will use to install and remove hardware
+ breakpoints and watchpoints.
+
+ The first time the interface is requested it is detected, and
+ subsequent requests will return the same value. This value can
+ indicate that no interface is available.
+
+ The Linux kernel provides two different sets of ptrace requests to
+ handle hardware watchpoints and breakpoints for Power:
+
+ - PPC_PTRACE_GETHWDBGINFO, PPC_PTRACE_SETHWDEBUG, and
+ PPC_PTRACE_DELHWDEBUG.
+
+ Or
+
+ - PTRACE_SET_DEBUGREG and PTRACE_GET_DEBUGREG
+
+ The first set is the more flexible one and allows setting watchpoints
+ with a variable watched region length and, for BookE processors,
+ multiple types of debug registers (e.g. hardware breakpoints and
+ hardware-assisted conditions for watchpoints). The second one only
+ allows setting one debug register, a watchpoint, so we only use it if
+ the first one is not available. */
+
+class ppc_linux_dreg_interface
+{
+public:
+
+ ppc_linux_dreg_interface ()
+ : m_interface (), m_hwdebug_info ()
+ {
+ };
+
+ DISABLE_COPY_AND_ASSIGN (ppc_linux_dreg_interface);
+
+ /* Returns true if the interface has been detected and is not
+ UNAVAILABLE. If it hasn't been detected, returns false without
+ causing it to be detected. This is useful for cases when we know
+ there is no work to be done if the interface hasn't been detected
+ yet. */
+ bool available_p ()
+ {
+ return (m_interface.has_value ()
+ && interface () != UNAVAILABLE);
+ }
+
+ /* One and only one of these three functions returns true, indicating
+ whether the corresponding interface is the one we detected. If the
+ interface hasn't been detected before, these functions cause it to
+ be detected. */
+
+ bool hwdebug_p ()
+ {
+ return interface () == HWDEBUG;
+ }
+
+ bool debugreg_p ()
+ {
+ return interface () == DEBUGREG;
+ }
+
+ bool unavailable_p ()
+ {
+ return interface () == UNAVAILABLE;
+ }
+
+ /* Returns the debug register capabilities of the target. Should only
+ be called if the interface is HWDEBUG. */
+ const struct ppc_debug_info &hwdebug_info ()
+ {
+ gdb_assert (m_interface.has_value () && hwdebug_p ());
+
+ return m_hwdebug_info;
+ }
+
+private:
+
+ /* HWDEBUG represents the set of calls PPC_PTRACE_GETHWDBGINFO,
+ PPC_PTRACE_SETHWDEBUG and PPC_PTRACE_DELHWDEBUG.
+
+ DEBUGREG represents the set of calls PTRACE_SET_DEBUGREG and
+ PTRACE_GET_DEBUGREG.
+
+ UNAVAILABLE can indicate that the kernel doesn't support any of the
+ two sets of requests or that there was an error when we tried to
+ detect wich interface is available. */
+
+ enum debug_reg_interface
+ {
+ UNAVAILABLE,
+ HWDEBUG,
+ DEBUGREG
+ };
+
+ /* Return the interface, first detecting it if hasn't been detected
+ before. */
+
+ enum debug_reg_interface interface ()
+ {
+ if (m_interface.has_value ())
+ return *m_interface;
+
+ gdb_assert (inferior_ptid.lwp_p ());
+
+ long lwp = inferior_ptid.lwp ();
+
+ bool no_features = false;
+
+ if (ptrace (PPC_PTRACE_GETHWDBGINFO, lwp, 0, &m_hwdebug_info)
+ != -1)
+ {
+ /* If there are no advertised features, we don't use the
+ HWDEBUG interface and try the DEBUGREG interface instead.
+ It shouldn't be necessary to do this, however, when the
+ kernel is configured without CONFIG_HW_BREAKPOINTS (selected
+ by CONFIG_PERF_EVENTS), there is a bug that causes
+ watchpoints installed with the HWDEBUG interface not to
+ trigger. When this is the case, features will be zero,
+ which we use as an indicator to fall back to the DEBUGREG
+ interface. */
+ if (m_hwdebug_info.features != 0)
+ {
+ m_interface.emplace (HWDEBUG);
+ return *m_interface;
+ }
+ else
+ no_features = true;
+ }
+
+ /* EIO indicates that the request is invalid, so we try DEBUGREG
+ next. Technically, it can also indicate other failures, but we
+ can't differentiate those.
+
+ Other errors could happen for various reasons. We could get an
+ ESRCH if the traced thread was killed by a signal. Trying to
+ detect the interface with another thread in the future would be
+ complicated, as callers would have to handle an "unknown
+ interface" case. It's also unclear if raising an exception
+ here would be safe.
+
+ Other errors, such as ENODEV, could be more permanent and cause
+ a failure for any thread.
+
+ For simplicity, with all errors other than EIO, we set the
+ interface to UNAVAILABLE. It's unlikely that trying the
+ DEBUGREG interface with this same thread would work. This means
+ that these errors will cause hardware watchpoints and
+ breakpoints to become unavailable throughout a GDB session. */
+
+ if (no_features || errno == EIO)
+ {
+ unsigned long wp;
+
+ if (ptrace (PTRACE_GET_DEBUGREG, lwp, 0, &wp) != -1)
+ {
+ m_interface.emplace (DEBUGREG);
+ return *m_interface;
+ }
+ }
+
+ if (errno != EIO)
+ warning (_("Error when detecting the debug register interface. "
+ "Debug registers will be unavailable."));
+
+ m_interface.emplace (UNAVAILABLE);
+ return *m_interface;
+ }
+
+ /* The interface option. Initialized if has_value () returns true. */
+ gdb::optional<enum debug_reg_interface> m_interface;
+
+ /* The info returned by the kernel with PPC_PTRACE_GETHWDBGINFO. Only
+ valid if we determined that the interface is HWDEBUG. */
+ struct ppc_debug_info m_hwdebug_info;
+};
+
struct ppc_linux_nat_target final : public linux_nat_target
{
/* Add our register access methods. */
@@ -299,10 +498,6 @@ struct ppc_linux_nat_target final : public linux_nat_target
int remove_mask_watchpoint (CORE_ADDR, CORE_ADDR, enum target_hw_bp_type)
override;
- bool stopped_by_watchpoint () override;
-
- bool stopped_data_address (CORE_ADDR *) override;
-
bool watchpoint_addr_within_range (CORE_ADDR, CORE_ADDR, int) override;
bool can_accel_watchpoint_condition (CORE_ADDR, int, int, struct expression *)
@@ -319,7 +514,105 @@ struct ppc_linux_nat_target final : public linux_nat_target
override;
/* Override linux_nat_target low methods. */
+ bool low_stopped_by_watchpoint () override;
+
+ bool low_stopped_data_address (CORE_ADDR *) override;
+
void low_new_thread (struct lwp_info *lp) override;
+
+ void low_delete_thread (arch_lwp_info *) override;
+
+ void low_new_fork (struct lwp_info *, pid_t) override;
+
+ void low_new_clone (struct lwp_info *, pid_t) override;
+
+ void low_forget_process (pid_t pid) override;
+
+ void low_prepare_to_resume (struct lwp_info *) override;
+
+private:
+
+ void copy_thread_dreg_state (const ptid_t &parent_ptid,
+ const ptid_t &child_ptid);
+
+ void mark_thread_stale (struct lwp_info *lp);
+
+ void mark_debug_registers_changed (pid_t pid);
+
+ void register_hw_breakpoint (pid_t pid,
+ const struct ppc_hw_breakpoint &bp);
+
+ void clear_hw_breakpoint (pid_t pid,
+ const struct ppc_hw_breakpoint &a);
+
+ void register_wp (pid_t pid, long wp_value);
+
+ void clear_wp (pid_t pid);
+
+ bool can_use_watchpoint_cond_accel (void);
+
+ void calculate_dvc (CORE_ADDR addr, int len,
+ CORE_ADDR data_value,
+ uint32_t *condition_mode,
+ uint64_t *condition_value);
+
+ int check_condition (CORE_ADDR watch_addr,
+ struct expression *cond,
+ CORE_ADDR *data_value, int *len);
+
+ int num_memory_accesses (const std::vector<value_ref_ptr> &chain);
+
+ int get_trigger_type (enum target_hw_bp_type type);
+
+ void create_watchpoint_request (struct ppc_hw_breakpoint *p,
+ CORE_ADDR addr,
+ int len,
+ enum target_hw_bp_type type,
+ struct expression *cond,
+ int insert);
+
+ bool hwdebug_point_cmp (const struct ppc_hw_breakpoint &a,
+ const struct ppc_hw_breakpoint &b);
+
+ void init_arch_lwp_info (struct lwp_info *lp);
+
+ arch_lwp_info *get_arch_lwp_info (struct lwp_info *lp);
+
+ /* The ptrace interface we'll use to install hardware watchpoints and
+ breakpoints (debug registers). */
+ ppc_linux_dreg_interface m_dreg_interface;
+
+ /* A map from pid numbers to the list of hardware watchpoints and
+ breakpoints that GDB requested for each process.
+
+ Only used when the interface is HWDEBUG. */
+ std::unordered_map<pid_t, std::list<ppc_hw_breakpoint>>
+ m_requested_hw_bps;
+
+ /* A map from pid numbers to the watchpoint value that GDB requested
+ for each process.
+
+ Only used when the interface is DEBUGREG. */
+ std::unordered_map<pid_t, long> m_requested_wp_vals;
+
+
+ /* Callable object to hash ptids by their lwp number. */
+ struct ptid_hash
+ {
+ std::size_t operator() (const ptid_t &ptid) const
+ {
+ return std::hash<long>{} (ptid.lwp ());
+ }
+ };
+
+ /* A map from ptid_t objects to a list of pairs of slots and hardware
+ breakpoint objects. This keeps track of which hardware breakpoints
+ and watchpoints were last installed in each slot of each thread.
+
+ Only used when the interface is HWDEBUG. */
+ std::unordered_map <ptid_t,
+ std::list<std::pair<long, ppc_hw_breakpoint>>,
+ ptid_hash> m_installed_hw_bps;
};
static ppc_linux_nat_target the_ppc_linux_nat_target;
@@ -1719,101 +2012,48 @@ ppc_linux_nat_target::read_description ()
return ppc_linux_match_description (features);
}
-/* The cached DABR value, to install in new threads.
- This variable is used when the PowerPC HWDEBUG ptrace
- interface is not available. */
-static long saved_dabr_value;
-
-/* Global structure that will store information about the available
- features provided by the PowerPC HWDEBUG ptrace interface. */
-static struct ppc_debug_info hwdebug_info;
-
-/* Global variable that holds the maximum number of slots that the
- kernel will use. This is only used when PowerPC HWDEBUG ptrace interface
- is available. */
-static size_t max_slots_number = 0;
-
-struct hw_break_tuple
-{
- long slot;
- struct ppc_hw_breakpoint *hw_break;
-};
-
-/* This is an internal vector created to store information about *points
- inserted for each thread. This is used when PowerPC HWDEBUG ptrace
- interface is available. */
-struct thread_points
- {
- /* The TID to which this *point relates. */
- int tid;
- /* Information about the *point, such as its address, type, etc.
-
- Each element inside this vector corresponds to a hardware
- breakpoint or watchpoint in the thread represented by TID. The maximum
- size of these vector is MAX_SLOTS_NUMBER. If the hw_break element of
- the tuple is NULL, then the position in the vector is free. */
- struct hw_break_tuple *hw_breaks;
- };
-
-static std::vector<thread_points *> ppc_threads;
-
-/* The version of the PowerPC HWDEBUG kernel interface that we will use, if
- available. */
-#define PPC_DEBUG_CURRENT_VERSION 1
-
-/* Returns non-zero if we support the PowerPC HWDEBUG ptrace interface. */
-static int
-have_ptrace_hwdebug_interface (void)
-{
- static int have_ptrace_hwdebug_interface = -1;
-
- if (have_ptrace_hwdebug_interface == -1)
- {
- int tid;
-
- tid = inferior_ptid.lwp ();
- if (tid == 0)
- tid = inferior_ptid.pid ();
-
- /* Check for kernel support for PowerPC HWDEBUG ptrace interface. */
- if (ptrace (PPC_PTRACE_GETHWDBGINFO, tid, 0, &hwdebug_info) >= 0)
- {
- /* Check whether PowerPC HWDEBUG ptrace interface is functional and
- provides any supported feature. */
- if (hwdebug_info.features != 0)
- {
- have_ptrace_hwdebug_interface = 1;
- max_slots_number = hwdebug_info.num_instruction_bps
- + hwdebug_info.num_data_bps
- + hwdebug_info.num_condition_regs;
- return have_ptrace_hwdebug_interface;
- }
- }
- /* Old school interface and no PowerPC HWDEBUG ptrace support. */
- have_ptrace_hwdebug_interface = 0;
- memset (&hwdebug_info, 0, sizeof (struct ppc_debug_info));
- }
-
- return have_ptrace_hwdebug_interface;
-}
+/* Routines for installing hardware watchpoints and breakpoints. When
+ GDB requests a hardware watchpoint or breakpoint to be installed, we
+ register the request for the pid of inferior_ptid in a map with one
+ entry per process. We then issue a stop request to all the threads of
+ this process, and mark a per-thread flag indicating that their debug
+ registers should be updated. Right before they are next resumed, we
+ remove all previously installed debug registers and install all the
+ ones GDB requested. We then update a map with one entry per thread
+ that keeps track of what debug registers were last installed in each
+ thread.
+
+ We use this second map to remove installed registers before installing
+ the ones requested by GDB, and to copy the debug register state after
+ a thread clones or forks, since depending on the kernel configuration,
+ debug registers can be inherited. */
+
+/* Check if we support and have enough resources to install a hardware
+ watchpoint or breakpoint. See the description in target.h. */
int
-ppc_linux_nat_target::can_use_hw_breakpoint (enum bptype type, int cnt, int ot)
+ppc_linux_nat_target::can_use_hw_breakpoint (enum bptype type, int cnt,
+ int ot)
{
int total_hw_wp, total_hw_bp;
- if (have_ptrace_hwdebug_interface ())
+ if (m_dreg_interface.unavailable_p ())
+ return 0;
+
+ if (m_dreg_interface.hwdebug_p ())
{
/* When PowerPC HWDEBUG ptrace interface is available, the number of
available hardware watchpoints and breakpoints is stored at the
hwdebug_info struct. */
- total_hw_bp = hwdebug_info.num_instruction_bps;
- total_hw_wp = hwdebug_info.num_data_bps;
+ total_hw_bp = m_dreg_interface.hwdebug_info ().num_instruction_bps;
+ total_hw_wp = m_dreg_interface.hwdebug_info ().num_data_bps;
}
else
{
- /* When we do not have PowerPC HWDEBUG ptrace interface, we should
- consider having 1 hardware watchpoint and no hardware breakpoints. */
+ gdb_assert (m_dreg_interface.debugreg_p ());
+
+ /* With the DEBUGREG ptrace interface, we should consider having 1
+ hardware watchpoint and no hardware breakpoints. */
total_hw_bp = 0;
total_hw_wp = 1;
}
@@ -1821,39 +2061,28 @@ ppc_linux_nat_target::can_use_hw_breakpoint (enum bptype type, int cnt, int ot)
if (type == bp_hardware_watchpoint || type == bp_read_watchpoint
|| type == bp_access_watchpoint || type == bp_watchpoint)
{
- if (cnt + ot > total_hw_wp)
+ if (total_hw_wp == 0)
+ return 0;
+ else if (cnt + ot > total_hw_wp)
return -1;
+ else
+ return 1;
}
else if (type == bp_hardware_breakpoint)
{
if (total_hw_bp == 0)
- {
- /* No hardware breakpoint support. */
- return 0;
- }
- if (cnt > total_hw_bp)
- return -1;
- }
-
- if (!have_ptrace_hwdebug_interface ())
- {
- int tid;
- ptid_t ptid = inferior_ptid;
-
- /* We need to know whether ptrace supports PTRACE_SET_DEBUGREG
- and whether the target has DABR. If either answer is no, the
- ptrace call will return -1. Fail in that case. */
- tid = ptid.lwp ();
- if (tid == 0)
- tid = ptid.pid ();
-
- if (ptrace (PTRACE_SET_DEBUGREG, tid, 0, 0) == -1)
return 0;
+ else if (cnt > total_hw_bp)
+ return -1;
+ else
+ return 1;
}
- return 1;
+ return 0;
}
+/* Returns 1 if we can watch LEN bytes at address ADDR, 0 otherwise. */
+
int
ppc_linux_nat_target::region_ok_for_hw_watchpoint (CORE_ADDR addr, int len)
{
@@ -1861,13 +2090,19 @@ ppc_linux_nat_target::region_ok_for_hw_watchpoint (CORE_ADDR addr, int len)
if (len <= 0)
return 0;
+ if (m_dreg_interface.unavailable_p ())
+ return 0;
+
/* The PowerPC HWDEBUG ptrace interface tells if there are alignment
restrictions for watchpoints in the processors. In that case, we use that
information to determine the hardcoded watchable region for
watchpoints. */
- if (have_ptrace_hwdebug_interface ())
+ if (m_dreg_interface.hwdebug_p ())
{
int region_size;
+ const struct ppc_debug_info &hwdebug_info = (m_dreg_interface
+ .hwdebug_info ());
+
/* Embedded DAC-based processors, like the PowerPC 440 have ranged
watchpoints and can watch any access within an arbitrary memory
region. This is useful to watch arrays and structs, for instance. It
@@ -1894,121 +2129,32 @@ ppc_linux_nat_target::region_ok_for_hw_watchpoint (CORE_ADDR addr, int len)
ptrace interface, DAC-based processors (i.e., embedded processors) will
use addresses aligned to 4-bytes due to the way the read/write flags are
passed in the old ptrace interface. */
- else if (((linux_get_hwcap (current_top_target ()) & PPC_FEATURE_BOOKE)
- && (addr + len) > (addr & ~3) + 4)
- || (addr + len) > (addr & ~7) + 8)
- return 0;
-
- return 1;
-}
-
-/* This function compares two ppc_hw_breakpoint structs field-by-field. */
-static int
-hwdebug_point_cmp (struct ppc_hw_breakpoint *a, struct ppc_hw_breakpoint *b)
-{
- return (a->trigger_type == b->trigger_type
- && a->addr_mode == b->addr_mode
- && a->condition_mode == b->condition_mode
- && a->addr == b->addr
- && a->addr2 == b->addr2
- && a->condition_value == b->condition_value);
-}
-
-/* This function can be used to retrieve a thread_points by the TID of the
- related process/thread. If nothing has been found, and ALLOC_NEW is 0,
- it returns NULL. If ALLOC_NEW is non-zero, a new thread_points for the
- provided TID will be created and returned. */
-static struct thread_points *
-hwdebug_find_thread_points_by_tid (int tid, int alloc_new)
-{
- for (thread_points *t : ppc_threads)
- {
- if (t->tid == tid)
- return t;
- }
-
- struct thread_points *t = NULL;
-
- /* Do we need to allocate a new point_item
- if the wanted one does not exist? */
- if (alloc_new)
+ else
{
- t = XNEW (struct thread_points);
- t->hw_breaks = XCNEWVEC (struct hw_break_tuple, max_slots_number);
- t->tid = tid;
- ppc_threads.push_back (t);
- }
+ gdb_assert (m_dreg_interface.debugreg_p ());
- return t;
-}
-
-/* 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
-hwdebug_insert_point (struct ppc_hw_breakpoint *b, int tid)
-{
- int i;
- long slot;
- gdb::unique_xmalloc_ptr<ppc_hw_breakpoint> p (XDUP (ppc_hw_breakpoint, b));
- struct hw_break_tuple *hw_breaks;
- struct thread_points *t;
-
- errno = 0;
- slot = ptrace (PPC_PTRACE_SETHWDEBUG, tid, 0, p.get ());
- if (slot < 0)
- perror_with_name (_("Unexpected error setting breakpoint or watchpoint"));
-
- /* Everything went fine, so we have to register this *point. */
- t = hwdebug_find_thread_points_by_tid (tid, 1);
- gdb_assert (t != NULL);
- hw_breaks = t->hw_breaks;
-
- /* Find a free element in the hw_breaks vector. */
- for (i = 0; i < max_slots_number; i++)
- {
- if (hw_breaks[i].hw_break == NULL)
- {
- hw_breaks[i].slot = slot;
- hw_breaks[i].hw_break = p.release ();
- break;
- }
+ if (((linux_get_hwcap (current_top_target ()) & PPC_FEATURE_BOOKE)
+ && (addr + len) > (addr & ~3) + 4)
+ || (addr + len) > (addr & ~7) + 8)
+ return 0;
}
- gdb_assert (i != max_slots_number);
+ return 1;
}
-/* 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
-hwdebug_remove_point (struct ppc_hw_breakpoint *b, int tid)
-{
- int i;
- struct hw_break_tuple *hw_breaks;
- struct thread_points *t;
-
- t = hwdebug_find_thread_points_by_tid (tid, 0);
- gdb_assert (t != NULL);
- hw_breaks = t->hw_breaks;
+/* This function compares two ppc_hw_breakpoint structs
+ field-by-field. */
- for (i = 0; i < max_slots_number; i++)
- if (hw_breaks[i].hw_break && hwdebug_point_cmp (hw_breaks[i].hw_break, b))
- break;
-
- gdb_assert (i != max_slots_number);
-
- /* We have to ignore ENOENT errors because the kernel implements hardware
- breakpoints/watchpoints as "one-shot", that is, they are automatically
- deleted when hit. */
- 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"));
-
- xfree (hw_breaks[i].hw_break);
- hw_breaks[i].hw_break = NULL;
+bool
+ppc_linux_nat_target::hwdebug_point_cmp
+(const struct ppc_hw_breakpoint &a, const struct ppc_hw_breakpoint &b)
+{
+ return (a.trigger_type == b.trigger_type
+ && a.addr_mode == b.addr_mode
+ && a.condition_mode == b.condition_mode
+ && a.addr == b.addr
+ && a.addr2 == b.addr2
+ && a.condition_value == b.condition_value);
}
/* Return the number of registers needed for a ranged breakpoint. */
@@ -2016,22 +2162,24 @@ hwdebug_remove_point (struct ppc_hw_breakpoint *b, int tid)
int
ppc_linux_nat_target::ranged_break_num_registers ()
{
- return ((have_ptrace_hwdebug_interface ()
- && hwdebug_info.features & PPC_DEBUG_FEATURE_INSN_BP_RANGE)?
+ return ((m_dreg_interface.hwdebug_p ()
+ && (m_dreg_interface.hwdebug_info ().features
+ & PPC_DEBUG_FEATURE_INSN_BP_RANGE))?
2 : -1);
}
-/* Insert the hardware breakpoint described by BP_TGT. Returns 0 for
- success, 1 if hardware breakpoints are not supported or -1 for failure. */
+/* Register the hardware breakpoint described by BP_TGT, to be inserted
+ when the threads of inferior_ptid are resumed. Returns 0 for success,
+ or -1 if the HWDEBUG interface that we need for hardware breakpoints
+ is not available. */
int
ppc_linux_nat_target::insert_hw_breakpoint (struct gdbarch *gdbarch,
struct bp_target_info *bp_tgt)
{
- struct lwp_info *lp;
struct ppc_hw_breakpoint p;
- if (!have_ptrace_hwdebug_interface ())
+ if (!m_dreg_interface.hwdebug_p ())
return -1;
p.version = PPC_DEBUG_CURRENT_VERSION;
@@ -2054,20 +2202,23 @@ ppc_linux_nat_target::insert_hw_breakpoint (struct gdbarch *gdbarch,
p.addr2 = 0;
}
- ALL_LWPS (lp)
- hwdebug_insert_point (&p, lp->ptid.lwp ());
+ register_hw_breakpoint (inferior_ptid.pid (), p);
return 0;
}
+/* Clear a registration for the hardware breakpoint given by type BP_TGT.
+ It will be removed from the threads of inferior_ptid when they are
+ next resumed. Returns 0 for success, or -1 if the HWDEBUG interface
+ that we need for hardware breakpoints is not available. */
+
int
ppc_linux_nat_target::remove_hw_breakpoint (struct gdbarch *gdbarch,
struct bp_target_info *bp_tgt)
{
- struct lwp_info *lp;
struct ppc_hw_breakpoint p;
- if (!have_ptrace_hwdebug_interface ())
+ if (!m_dreg_interface.hwdebug_p ())
return -1;
p.version = PPC_DEBUG_CURRENT_VERSION;
@@ -2090,14 +2241,16 @@ ppc_linux_nat_target::remove_hw_breakpoint (struct gdbarch *gdbarch,
p.addr2 = 0;
}
- ALL_LWPS (lp)
- hwdebug_remove_point (&p, lp->ptid.lwp ());
+ clear_hw_breakpoint (inferior_ptid.pid (), p);
return 0;
}
-static int
-get_trigger_type (enum target_hw_bp_type type)
+/* Return the trigger value to set in a ppc_hw_breakpoint object for a
+ given hardware watchpoint TYPE. We assume type is not hw_execute. */
+
+int
+ppc_linux_nat_target::get_trigger_type (enum target_hw_bp_type type)
{
int t;
@@ -2111,19 +2264,18 @@ get_trigger_type (enum target_hw_bp_type type)
return t;
}
-/* Insert a new masked watchpoint at ADDR using the mask MASK.
- RW may be hw_read for a read watchpoint, hw_write for a write watchpoint
- or hw_access for an access watchpoint. Returns 0 on success and throws
- an error on failure. */
+/* Register a new masked watchpoint at ADDR using the mask MASK, to be
+ inserted when the threads of inferior_ptid are resumed. RW may be
+ hw_read for a read watchpoint, hw_write for a write watchpoint or
+ hw_access for an access watchpoint. */
int
ppc_linux_nat_target::insert_mask_watchpoint (CORE_ADDR addr, CORE_ADDR mask,
target_hw_bp_type rw)
{
- struct lwp_info *lp;
struct ppc_hw_breakpoint p;
- gdb_assert (have_ptrace_hwdebug_interface ());
+ gdb_assert (m_dreg_interface.hwdebug_p ());
p.version = PPC_DEBUG_CURRENT_VERSION;
p.trigger_type = get_trigger_type (rw);
@@ -2133,25 +2285,23 @@ ppc_linux_nat_target::insert_mask_watchpoint (CORE_ADDR addr, CORE_ADDR mask,
p.addr2 = mask;
p.condition_value = 0;
- ALL_LWPS (lp)
- hwdebug_insert_point (&p, lp->ptid.lwp ());
+ register_hw_breakpoint (inferior_ptid.pid (), p);
return 0;
}
-/* Remove a masked watchpoint at ADDR with the mask MASK.
- RW may be hw_read for a read watchpoint, hw_write for a write watchpoint
- or hw_access for an access watchpoint. Returns 0 on success and throws
- an error on failure. */
+/* Clear a registration for a masked watchpoint at ADDR with the mask
+ MASK. It will be removed from the threads of inferior_ptid when they
+ are next resumed. RW may be hw_read for a read watchpoint, hw_write
+ for a write watchpoint or hw_access for an access watchpoint. */
int
ppc_linux_nat_target::remove_mask_watchpoint (CORE_ADDR addr, CORE_ADDR mask,
target_hw_bp_type rw)
{
- struct lwp_info *lp;
struct ppc_hw_breakpoint p;
- gdb_assert (have_ptrace_hwdebug_interface ());
+ gdb_assert (m_dreg_interface.hwdebug_p ());
p.version = PPC_DEBUG_CURRENT_VERSION;
p.trigger_type = get_trigger_type (rw);
@@ -2161,39 +2311,40 @@ ppc_linux_nat_target::remove_mask_watchpoint (CORE_ADDR addr, CORE_ADDR mask,
p.addr2 = mask;
p.condition_value = 0;
- ALL_LWPS (lp)
- hwdebug_remove_point (&p, lp->ptid.lwp ());
+ clear_hw_breakpoint (inferior_ptid.pid (), p);
return 0;
}
-/* Check whether we have at least one free DVC register. */
-static int
-can_use_watchpoint_cond_accel (void)
+/* Check whether we have at least one free DVC register for the threads
+ of the pid of inferior_ptid. */
+
+bool
+ppc_linux_nat_target::can_use_watchpoint_cond_accel (void)
{
- struct thread_points *p;
- int tid = inferior_ptid.lwp ();
- int cnt = hwdebug_info.num_condition_regs, i;
+ if (!m_dreg_interface.hwdebug_p ())
+ return false;
- if (!have_ptrace_hwdebug_interface () || cnt == 0)
- return 0;
+ int cnt = m_dreg_interface.hwdebug_info ().num_condition_regs;
- p = hwdebug_find_thread_points_by_tid (tid, 0);
+ if (cnt == 0)
+ return false;
- 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--;
+ auto request_it = m_requested_hw_bps.find (inferior_ptid.pid ());
- /* There are no available slots now. */
- if (cnt <= 0)
- return 0;
- }
+ /* No breakpoints or watchpoints have been requested for this process,
+ we have at least one free DVC register. */
+ if (request_it == m_requested_hw_bps.end ())
+ return true;
- return 1;
+ for (const ppc_hw_breakpoint &bp : request_it->second)
+ if (bp.condition_mode != PPC_BREAKPOINT_CONDITION_NONE)
+ cnt--;
+
+ if (cnt <= 0)
+ return false;
+
+ return true;
}
/* Calculate the enable bits and the contents of the Data Value Compare
@@ -2204,10 +2355,16 @@ can_use_watchpoint_cond_accel (void)
On exit, CONDITION_MODE will hold the enable bits for the DVC, and
CONDITION_VALUE will hold the value which should be put in the
DVC register. */
-static void
-calculate_dvc (CORE_ADDR addr, int len, CORE_ADDR data_value,
- uint32_t *condition_mode, uint64_t *condition_value)
+
+void
+ppc_linux_nat_target::calculate_dvc (CORE_ADDR addr, int len,
+ CORE_ADDR data_value,
+ uint32_t *condition_mode,
+ uint64_t *condition_value)
{
+ const struct ppc_debug_info &hwdebug_info = (m_dreg_interface.
+ hwdebug_info ());
+
int i, num_byte_enable, align_offset, num_bytes_off_dvc,
rightmost_enabled_byte;
CORE_ADDR addr_end_data, addr_end_dvc;
@@ -2246,8 +2403,10 @@ calculate_dvc (CORE_ADDR addr, int len, CORE_ADDR data_value,
Returns -1 if there's any register access involved, or if there are
other kinds of values which are not acceptable in a condition
expression (e.g., lval_computed or lval_internalvar). */
-static int
-num_memory_accesses (const std::vector<value_ref_ptr> &chain)
+
+int
+ppc_linux_nat_target::num_memory_accesses (const std::vector<value_ref_ptr>
+ &chain)
{
int found_memory_cnt = 0;
@@ -2295,9 +2454,11 @@ num_memory_accesses (const std::vector<value_ref_ptr> &chain)
If the function returns 1, DATA_VALUE will contain the constant against
which the watch value should be compared and LEN will contain the size
of the constant. */
-static int
-check_condition (CORE_ADDR watch_addr, struct expression *cond,
- CORE_ADDR *data_value, int *len)
+
+int
+ppc_linux_nat_target::check_condition (CORE_ADDR watch_addr,
+ struct expression *cond,
+ CORE_ADDR *data_value, int *len)
{
int pc = 1, num_accesses_left, num_accesses_right;
struct value *left_val, *right_val;
@@ -2344,18 +2505,18 @@ check_condition (CORE_ADDR watch_addr, struct expression *cond,
return 1;
}
-/* Return non-zero if the target is capable of using hardware to evaluate
- the condition expression, thus only triggering the watchpoint when it is
+/* Return true if the target is capable of using hardware to evaluate the
+ condition expression, thus only triggering the watchpoint when it is
true. */
+
bool
-ppc_linux_nat_target::can_accel_watchpoint_condition (CORE_ADDR addr, int len,
- int rw,
- struct expression *cond)
+ppc_linux_nat_target::can_accel_watchpoint_condition
+(CORE_ADDR addr, int len, int rw, struct expression *cond)
{
CORE_ADDR data_value;
- return (have_ptrace_hwdebug_interface ()
- && hwdebug_info.num_condition_regs > 0
+ return (m_dreg_interface.hwdebug_p ()
+ && (m_dreg_interface.hwdebug_info ().num_condition_regs > 0)
&& check_condition (addr, cond, &data_value, &len));
}
@@ -2364,11 +2525,14 @@ ppc_linux_nat_target::can_accel_watchpoint_condition (CORE_ADDR addr, int len,
evaluated by hardware. INSERT tells if we are creating a request for
inserting or removing the watchpoint. */
-static void
-create_watchpoint_request (struct ppc_hw_breakpoint *p, CORE_ADDR addr,
- int len, enum target_hw_bp_type type,
- struct expression *cond, int insert)
+void
+ppc_linux_nat_target::create_watchpoint_request
+(struct ppc_hw_breakpoint *p, CORE_ADDR addr, int len,
+ enum target_hw_bp_type type, struct expression *cond, int insert)
{
+ const struct ppc_debug_info &hwdebug_info = (m_dreg_interface
+ .hwdebug_info ());
+
if (len == 1
|| !(hwdebug_info.features & PPC_DEBUG_FEATURE_DATA_BP_RANGE))
{
@@ -2410,28 +2574,31 @@ create_watchpoint_request (struct ppc_hw_breakpoint *p, CORE_ADDR addr,
p->addr = (uint64_t) addr;
}
+/* Register a watchpoint, to be inserted when the threads of the group of
+ inferior_ptid are next resumed. Returns 0 on success, and -1 if there
+ is no ptrace interface available to install the watchpoint. */
+
int
ppc_linux_nat_target::insert_watchpoint (CORE_ADDR addr, int len,
enum target_hw_bp_type type,
struct expression *cond)
{
- struct lwp_info *lp;
- int ret = -1;
+ if (m_dreg_interface.unavailable_p ())
+ return -1;
- if (have_ptrace_hwdebug_interface ())
+ if (m_dreg_interface.hwdebug_p ())
{
struct ppc_hw_breakpoint p;
create_watchpoint_request (&p, addr, len, type, cond, 1);
- ALL_LWPS (lp)
- hwdebug_insert_point (&p, lp->ptid.lwp ());
-
- ret = 0;
+ register_hw_breakpoint (inferior_ptid.pid (), p);
}
else
{
- long dabr_value;
+ gdb_assert (m_dreg_interface.debugreg_p ());
+
+ long wp_value;
long read_mode, write_mode;
if (linux_get_hwcap (current_top_target ()) & PPC_FEATURE_BOOKE)
@@ -2449,144 +2616,317 @@ ppc_linux_nat_target::insert_watchpoint (CORE_ADDR addr, int len,
write_mode = 6;
}
- dabr_value = addr & ~(read_mode | write_mode);
+ wp_value = addr & ~(read_mode | write_mode);
switch (type)
{
case hw_read:
/* Set read and translate bits. */
- dabr_value |= read_mode;
+ wp_value |= read_mode;
break;
case hw_write:
/* Set write and translate bits. */
- dabr_value |= write_mode;
+ wp_value |= write_mode;
break;
case hw_access:
/* Set read, write and translate bits. */
- dabr_value |= read_mode | write_mode;
+ wp_value |= read_mode | write_mode;
break;
}
- saved_dabr_value = dabr_value;
-
- ALL_LWPS (lp)
- if (ptrace (PTRACE_SET_DEBUGREG, lp->ptid.lwp (), 0,
- saved_dabr_value) < 0)
- return -1;
-
- ret = 0;
+ register_wp (inferior_ptid.pid (), wp_value);
}
- return ret;
+ return 0;
}
+/* Clear a registration for a hardware watchpoint. It will be removed
+ from the threads of the group of inferior_ptid when they are next
+ resumed. */
+
int
ppc_linux_nat_target::remove_watchpoint (CORE_ADDR addr, int len,
enum target_hw_bp_type type,
struct expression *cond)
{
- struct lwp_info *lp;
- int ret = -1;
+ gdb_assert (!m_dreg_interface.unavailable_p ());
- if (have_ptrace_hwdebug_interface ())
+ if (m_dreg_interface.hwdebug_p ())
{
struct ppc_hw_breakpoint p;
create_watchpoint_request (&p, addr, len, type, cond, 0);
- ALL_LWPS (lp)
- hwdebug_remove_point (&p, lp->ptid.lwp ());
-
- ret = 0;
+ clear_hw_breakpoint (inferior_ptid.pid (), p);
}
else
{
- saved_dabr_value = 0;
- ALL_LWPS (lp)
- if (ptrace (PTRACE_SET_DEBUGREG, lp->ptid.lwp (), 0,
- saved_dabr_value) < 0)
- return -1;
+ gdb_assert (m_dreg_interface.debugreg_p ());
- ret = 0;
+ clear_wp (inferior_ptid.pid ());
}
- return ret;
+ return 0;
}
+/* Clean up the debug register state associated with PID. We erase the
+ per-process state (the hardware watchpoints and breakpoints that GDB
+ requested), and when using the HWDEBUG interface, we also erase the
+ per-thread state of installed debug registers for all the threads that
+ belong to the group of PID.
+
+ Usually the thread state is cleaned up by low_delete_thread. We also
+ do it here because low_new_thread is not called for the initial LWP,
+ so low_delete_thread won't be able to clean up this state. */
+
void
-ppc_linux_nat_target::low_new_thread (struct lwp_info *lp)
+ppc_linux_nat_target::low_forget_process (pid_t pid)
{
- int tid = lp->ptid.lwp ();
+ if (!m_dreg_interface.available_p ())
+ return;
+
+ ptid_t pid_ptid (pid, 0, 0);
- if (have_ptrace_hwdebug_interface ())
+ if (m_dreg_interface.hwdebug_p ())
{
- int i;
- struct thread_points *p;
- struct hw_break_tuple *hw_breaks;
+ m_requested_hw_bps.erase (pid);
- if (ppc_threads.empty ())
- return;
+ for (auto it = m_installed_hw_bps.begin ();
+ it != m_installed_hw_bps.end ();)
+ {
+ if (it->first.matches (pid_ptid))
+ it = m_installed_hw_bps.erase (it);
+ else
+ it++;
+ }
+ }
+ else
+ {
+ gdb_assert (m_dreg_interface.debugreg_p ());
+
+ m_requested_wp_vals.erase (pid);
+ }
+}
+
+/* Copy the set of requested hardware breakpoints and watchpoints of the
+ pid of PARENT to the set of CHILD_PID. GDB expects that a forked
+ process will have the same hardware breakpoints and watchpoints as the
+ parent.
- /* Get a list of breakpoints from any thread. */
- p = ppc_threads.back ();
- hw_breaks = p->hw_breaks;
+ If we're using the HWDEBUG interface, also copy the thread debug
+ register state for the ptid of PARENT to the state for CHILD_PID.
- /* Copy that thread's breakpoints and watchpoints to the new thread. */
- for (i = 0; i < max_slots_number; i++)
- if (hw_breaks[i].hw_break)
- {
- /* Older kernels did not make new threads inherit their parent
- thread's debug state, so we always clear the slot and replicate
- the debug state ourselves, ensuring compatibility with all
- kernels. */
+ Like for clone events, we assume the kernel will copy the debug
+ registers from the parent thread to the child. The
+ low_prepare_to_resume function is made to work even if it doesn't.
- /* The ppc debug resource accounting is done through "slots".
- Ask the kernel the deallocate this specific *point's slot. */
- ptrace (PPC_PTRACE_DELHWDEBUG, tid, 0, hw_breaks[i].slot);
+ We copy the thread state here and not in low_new_thread since we don't
+ have the pid of the parent in low_new_thread. Even if we did,
+ low_new_thread might not be called immediately after the fork event is
+ detected. For instance, with the checkpointing system (see
+ linux-fork.c), the thread won't be added until GDB decides to switch
+ to a new checkpointed process. At that point, the debug register
+ state of the parent thread is unlikely to correspond to the state it
+ had at the point when it forked. */
+
+void
+ppc_linux_nat_target::low_new_fork (struct lwp_info *parent,
+ pid_t child_pid)
+{
+ if (!m_dreg_interface.available_p ())
+ return;
+
+ if (m_dreg_interface.hwdebug_p ())
+ {
+ auto request_it = m_requested_hw_bps.find (parent->ptid.pid ());
+ if (request_it != m_requested_hw_bps.end ())
+ m_requested_hw_bps[child_pid] =
+ m_requested_hw_bps[parent->ptid.pid ()];
- hwdebug_insert_point (hw_breaks[i].hw_break, tid);
- }
+ ptid_t child_ptid (child_pid, child_pid, 0);
+
+ copy_thread_dreg_state (parent->ptid, child_ptid);
}
else
- ptrace (PTRACE_SET_DEBUGREG, tid, 0, saved_dabr_value);
+ {
+ gdb_assert (m_dreg_interface.debugreg_p ());
+
+ auto request_it = m_requested_wp_vals.find (parent->ptid.pid ());
+
+ if (request_it != m_requested_wp_vals.end ())
+ m_requested_wp_vals[child_pid] =
+ m_requested_wp_vals[parent->ptid.pid ()];
+ }
}
-static void
-ppc_linux_thread_exit (struct thread_info *tp, int silent)
+/* Copy the thread debug register state from the PARENT thread to the the
+ state for CHILD_LWP, if we're using the HWDEBUG interface. We assume
+ the kernel copies the debug registers from one thread to another after
+ a clone event. The low_prepare_to_resume function is made to work
+ even if it doesn't. */
+
+void
+ppc_linux_nat_target::low_new_clone (struct lwp_info *parent,
+ pid_t child_lwp)
{
- int i;
- int tid = tp->ptid.lwp ();
- struct hw_break_tuple *hw_breaks;
- struct thread_points *t = NULL;
+ if (!m_dreg_interface.available_p ())
+ return;
+
+ if (m_dreg_interface.hwdebug_p ())
+ {
+ ptid_t child_ptid (parent->ptid.pid (), child_lwp, 0);
+
+ copy_thread_dreg_state (parent->ptid, child_ptid);
+ }
+}
+
+/* Initialize the arch-specific thread state for LP so that it contains
+ the ptid for lp, so that we can use it in low_delete_thread. Mark the
+ new thread LP as stale so that we update its debug registers before
+ resuming it. This is not called for the initial thread. */
+
+void
+ppc_linux_nat_target::low_new_thread (struct lwp_info *lp)
+{
+ init_arch_lwp_info (lp);
+
+ mark_thread_stale (lp);
+}
+
+/* Delete the per-thread debug register stale flag. */
+
+void
+ppc_linux_nat_target::low_delete_thread (struct arch_lwp_info
+ *lp_arch_info)
+{
+ if (lp_arch_info != NULL)
+ {
+ if (m_dreg_interface.available_p ()
+ && m_dreg_interface.hwdebug_p ())
+ m_installed_hw_bps.erase (lp_arch_info->lwp_ptid);
+
+ xfree (lp_arch_info);
+ }
+}
+
+/* Install or delete debug registers in thread LP so that it matches what
+ GDB requested before it is resumed. */
+
+void
+ppc_linux_nat_target::low_prepare_to_resume (struct lwp_info *lp)
+{
+ if (!m_dreg_interface.available_p ())
+ return;
+
+ /* We have to re-install or clear the debug registers if we set the
+ stale flag.
- if (!have_ptrace_hwdebug_interface ())
+ In addition, some kernels configurations can disable a hardware
+ watchpoint after it is hit. Usually, GDB will remove and re-install
+ a hardware watchpoint when the thread stops if "breakpoint
+ always-inserted" is off, or to single-step a watchpoint. But so
+ that we don't rely on this behavior, if we stop due to a hardware
+ breakpoint or watchpoint, we also refresh our debug registers. */
+
+ arch_lwp_info *lp_arch_info = get_arch_lwp_info (lp);
+
+ bool stale_dregs = false;
+
+ stale_dregs = (lp->stop_reason == TARGET_STOPPED_BY_WATCHPOINT
+ || lp->stop_reason == TARGET_STOPPED_BY_HW_BREAKPOINT
+ || lp_arch_info->debug_regs_stale);
+
+ if (!stale_dregs)
return;
- for (i = 0; i < ppc_threads.size (); i++)
+ gdb_assert (lp->ptid.lwp_p ());
+
+ if (m_dreg_interface.hwdebug_p ())
{
- if (ppc_threads[i]->tid == tid)
+ /* First, delete any hardware watchpoint or breakpoint installed in
+ the inferior and update the thread state. */
+ auto installed_it = m_installed_hw_bps.find (lp->ptid);
+
+ if (installed_it != m_installed_hw_bps.end ())
{
- t = ppc_threads[i];
- break;
+ auto &bp_list = installed_it->second;
+
+ for (auto bp_it = bp_list.begin (); bp_it != bp_list.end ();)
+ {
+ /* We ignore ENOENT to account for various possible kernel
+ behaviors, e.g. the kernel might or might not copy debug
+ registers across forks and clones, and we always copy
+ the debug register state when fork and clone events are
+ detected. */
+ if (ptrace (PPC_PTRACE_DELHWDEBUG, lp->ptid.lwp (), 0,
+ bp_it->first) == -1)
+ if (errno != ENOENT)
+ perror_with_name (_("Error deleting hardware "
+ "breakpoint or watchpoint"));
+
+ /* We erase the entries one at a time after successfuly
+ removing the corresponding slot form the thread so that
+ if we throw an exception above in a future iteration the
+ map remains consistent. */
+ bp_it = bp_list.erase (bp_it);
+ }
+
+ gdb_assert (bp_list.empty ());
+ }
+
+ /* Now we install all the requested hardware breakpoints and
+ watchpoints and update the thread state. */
+
+ auto request_it = m_requested_hw_bps.find (lp->ptid.pid ());
+
+ if (request_it != m_requested_hw_bps.end ())
+ {
+ auto &bp_list = m_installed_hw_bps[lp->ptid];
+
+ for (ppc_hw_breakpoint bp : request_it->second)
+ {
+ long slot = ptrace (PPC_PTRACE_SETHWDEBUG, lp->ptid.lwp (),
+ 0, &bp);
+
+ if (slot < 0)
+ perror_with_name (_("Error setting hardware "
+ "breakpoint or watchpoint"));
+
+ /* Keep track of which slots we installed in this
+ thread. */
+ bp_list.emplace (bp_list.begin (), slot, bp);
+ }
}
}
+ else
+ {
+ gdb_assert (m_dreg_interface.debugreg_p ());
- if (t == NULL)
- return;
+ /* Passing 0 to PTRACE_SET_DEBUGREG will clear the
+ watchpoint. */
+ long wp = 0;
+
+ auto request_it = (m_requested_wp_vals.find (lp->ptid.pid ()));
- unordered_remove (ppc_threads, i);
+ /* GDB requested a watchpoint to be installed. */
+ if (request_it != m_requested_wp_vals.end ())
+ wp = request_it->second;
- hw_breaks = t->hw_breaks;
+ long ret = ptrace (PTRACE_SET_DEBUGREG, lp->ptid.lwp (),
+ 0, wp);
- for (i = 0; i < max_slots_number; i++)
- if (hw_breaks[i].hw_break)
- xfree (hw_breaks[i].hw_break);
+ if (ret == -1)
+ perror_with_name (_("Error setting hardware watchpoint"));
+ }
- xfree (t->hw_breaks);
- xfree (t);
+ lp_arch_info->debug_regs_stale = false;
}
+/* Return true if INFERIOR_PTID is known to have been stopped by a
+ hardware watchpoint, false otherwise. If true is returned, write the
+ address that the kernel reported as causing the SIGTRAP in ADDR_P. */
+
bool
-ppc_linux_nat_target::stopped_data_address (CORE_ADDR *addr_p)
+ppc_linux_nat_target::low_stopped_data_address (CORE_ADDR *addr_p)
{
siginfo_t siginfo;
@@ -2597,38 +2937,43 @@ ppc_linux_nat_target::stopped_data_address (CORE_ADDR *addr_p)
|| (siginfo.si_code & 0xffff) != 0x0004 /* TRAP_HWBKPT */)
return false;
- if (have_ptrace_hwdebug_interface ())
+ /* Check if this signal corresponds to a hardware breakpoint. We only
+ need to check this if we're using the HWDEBUG interface, since the
+ DEBUGREG interface only allows setting one hardware watchpoint. */
+ if (m_dreg_interface.hwdebug_p ())
{
- int i;
- struct thread_points *t;
- struct hw_break_tuple *hw_breaks;
- /* The index (or slot) of the *point is passed in the si_errno field. */
+ /* The index (or slot) of the *point is passed in the si_errno
+ field. Currently, this is only the case if the kernel was
+ configured with CONFIG_PPC_ADV_DEBUG_REGS. If not, we assume
+ the kernel will set si_errno to a value that doesn't correspond
+ to any real slot. */
int slot = siginfo.si_errno;
- t = hwdebug_find_thread_points_by_tid (inferior_ptid.lwp (), 0);
+ auto installed_it = m_installed_hw_bps.find (inferior_ptid);
- /* Find out if this *point is a hardware breakpoint.
- If so, we should return 0. */
- if (t)
- {
- hw_breaks = t->hw_breaks;
- for (i = 0; i < max_slots_number; i++)
- if (hw_breaks[i].hw_break && hw_breaks[i].slot == slot
- && hw_breaks[i].hw_break->trigger_type
- == PPC_BREAKPOINT_TRIGGER_EXECUTE)
- return false;
- }
+ /* We must have installed slots for the thread if it got a
+ TRAP_HWBKPT signal. */
+ gdb_assert (installed_it != m_installed_hw_bps.end ());
+
+ for (const auto & slot_bp_pair : installed_it->second)
+ if (slot_bp_pair.first == slot
+ && (slot_bp_pair.second.trigger_type
+ == PPC_BREAKPOINT_TRIGGER_EXECUTE))
+ return false;
}
*addr_p = (CORE_ADDR) (uintptr_t) siginfo.si_addr;
return true;
}
+/* Return true if INFERIOR_PTID is known to have been stopped by a
+ hardware watchpoint, false otherwise. */
+
bool
-ppc_linux_nat_target::stopped_by_watchpoint ()
+ppc_linux_nat_target::low_stopped_by_watchpoint ()
{
CORE_ADDR addr;
- return stopped_data_address (&addr);
+ return low_stopped_data_address (&addr);
}
bool
@@ -2636,9 +2981,11 @@ ppc_linux_nat_target::watchpoint_addr_within_range (CORE_ADDR addr,
CORE_ADDR start,
int length)
{
+ gdb_assert (m_dreg_interface.available_p ());
+
int mask;
- if (have_ptrace_hwdebug_interface ()
+ if (m_dreg_interface.hwdebug_p ()
&& linux_get_hwcap (current_top_target ()) & PPC_FEATURE_BOOKE)
return start <= addr && start + length >= addr;
else if (linux_get_hwcap (current_top_target ()) & PPC_FEATURE_BOOKE)
@@ -2655,10 +3002,12 @@ ppc_linux_nat_target::watchpoint_addr_within_range (CORE_ADDR addr,
/* Return the number of registers needed for a masked hardware watchpoint. */
int
-ppc_linux_nat_target::masked_watch_num_registers (CORE_ADDR addr, CORE_ADDR mask)
+ppc_linux_nat_target::masked_watch_num_registers (CORE_ADDR addr,
+ CORE_ADDR mask)
{
- if (!have_ptrace_hwdebug_interface ()
- || (hwdebug_info.features & PPC_DEBUG_FEATURE_DATA_BP_MASK) == 0)
+ if (!m_dreg_interface.hwdebug_p ()
+ || (m_dreg_interface.hwdebug_info ().features
+ & PPC_DEBUG_FEATURE_DATA_BP_MASK) == 0)
return -1;
else if ((mask & 0xC0000000) != 0xC0000000)
{
@@ -2671,14 +3020,181 @@ ppc_linux_nat_target::masked_watch_num_registers (CORE_ADDR addr, CORE_ADDR mask
return 2;
}
+/* Copy the per-thread debug register state, if any, from thread
+ PARENT_PTID to thread CHILD_PTID, if the debug register being used is
+ HWDEBUG. */
+
+void
+ppc_linux_nat_target::copy_thread_dreg_state (const ptid_t &parent_ptid,
+ const ptid_t &child_ptid)
+{
+ gdb_assert (m_dreg_interface.hwdebug_p ());
+
+ auto installed_it = m_installed_hw_bps.find (parent_ptid);
+
+ if (installed_it != m_installed_hw_bps.end ())
+ m_installed_hw_bps[child_ptid] = m_installed_hw_bps[parent_ptid];
+}
+
+/* Mark the debug register stale flag for the new thread, if we have
+ already detected which debug register interface we use. */
+
+void
+ppc_linux_nat_target::mark_thread_stale (struct lwp_info *lp)
+{
+ if (!m_dreg_interface.available_p ())
+ return;
+
+ arch_lwp_info *lp_arch_info = get_arch_lwp_info (lp);
+
+ lp_arch_info->debug_regs_stale = true;
+}
+
+/* Mark all the threads of the group of PID as stale with respect to
+ debug registers and issue a stop request to each such thread that
+ isn't already stopped. */
+
+void
+ppc_linux_nat_target::mark_debug_registers_changed (pid_t pid)
+{
+ /* We do this in two passes to make sure all threads are marked even if
+ we get an exception when stopping one of them. */
+
+ iterate_over_lwps (ptid_t (pid),
+ [this] (struct lwp_info *lp) -> int {
+ this->mark_thread_stale (lp);
+ return 0;
+ });
+
+ iterate_over_lwps (ptid_t (pid),
+ [] (struct lwp_info *lp) -> int {
+ if (!lwp_is_stopped (lp))
+ linux_stop_lwp (lp);
+ return 0;
+ });
+}
+
+/* Register a hardware breakpoint or watchpoint BP for the pid PID, then
+ mark the stale flag for all threads of the group of PID, and issue a
+ stop request for them. The breakpoint or watchpoint will be installed
+ the next time each thread is resumed. Should only be used if the
+ debug register interface is HWDEBUG. */
+
+void
+ppc_linux_nat_target::register_hw_breakpoint (pid_t pid,
+ const struct
+ ppc_hw_breakpoint &bp)
+{
+ gdb_assert (m_dreg_interface.hwdebug_p ());
+
+ m_requested_hw_bps[pid].push_back (bp);
+
+ mark_debug_registers_changed (pid);
+}
+
+/* Clear a registration for a hardware breakpoint or watchpoint BP for
+ the pid PID, then mark the stale flag for all threads of the group of
+ PID, and issue a stop request for them. The breakpoint or watchpoint
+ will be removed the next time each thread is resumed. Should only be
+ used if the debug register interface is HWDEBUG. */
+
+void
+ppc_linux_nat_target::clear_hw_breakpoint
+(pid_t pid, const struct ppc_hw_breakpoint &bp)
+{
+ gdb_assert (m_dreg_interface.hwdebug_p ());
+
+ auto request_it = m_requested_hw_bps.find (pid);
+
+ gdb_assert (request_it != m_requested_hw_bps.end ());
+
+ auto bp_it = std::find_if (request_it->second.begin (),
+ request_it->second.end (),
+ [&bp, this]
+ (const struct ppc_hw_breakpoint &curr)
+ { return hwdebug_point_cmp (bp, curr); }
+ );
+
+ /* If GDB is removing a watchpoint, it must have been inserted. */
+ gdb_assert (bp_it != request_it->second.end ());
+
+ request_it->second.erase (bp_it);
+
+ mark_debug_registers_changed (pid);
+}
+
+/* Register the hardware watchpoint value WP_VALUE for the pid PID,
+ then mark the stale flag for all threads of the group of PID, and
+ issue a stop request for them. The breakpoint or watchpoint will be
+ installed the next time each thread is resumed. Should only be used
+ if the debug register interface is DEBUGREG. */
+
+void
+ppc_linux_nat_target::register_wp (pid_t pid, long wp_value)
+{
+ gdb_assert (m_dreg_interface.debugreg_p ());
+
+ /* Our other functions should have told GDB that we only have one
+ hardware watchpoint with this interface. */
+ gdb_assert (m_requested_wp_vals.find (pid)
+ == m_requested_wp_vals.end ());
+
+ m_requested_wp_vals[pid] = wp_value;
+
+ mark_debug_registers_changed (pid);
+}
+
+/* Clear the hardware watchpoint registration for the pid PID, then mark
+ the stale flag for all threads of the group of PID, and issue a stop
+ request for them. The breakpoint or watchpoint will be installed the
+ next time each thread is resumed. Should only be used if the debug
+ register interface is DEBUGREG. */
+
+void
+ppc_linux_nat_target::clear_wp (pid_t pid)
+{
+ gdb_assert (m_dreg_interface.debugreg_p ());
+
+ auto wp_it = m_requested_wp_vals.find (pid);
+
+ gdb_assert (wp_it != m_requested_wp_vals.end ());
+
+ m_requested_wp_vals.erase (wp_it);
+
+ mark_debug_registers_changed (pid);
+}
+
+/* Initialize the arch-specific thread state for LWP, if it not already
+ created. */
+
+void
+ppc_linux_nat_target::init_arch_lwp_info (struct lwp_info *lp)
+{
+ if (lwp_arch_private_info (lp) == NULL)
+ {
+ lwp_set_arch_private_info (lp, XCNEW (struct arch_lwp_info));
+ lwp_arch_private_info (lp)->debug_regs_stale = false;
+ lwp_arch_private_info (lp)->lwp_ptid = lp->ptid;
+ }
+}
+
+/* Get the arch-specific thread state for LWP, creating it if
+ necessary. */
+
+arch_lwp_info *
+ppc_linux_nat_target::get_arch_lwp_info (struct lwp_info *lp)
+{
+ init_arch_lwp_info (lp);
+
+ return lwp_arch_private_info (lp);
+}
+
void _initialize_ppc_linux_nat ();
void
_initialize_ppc_linux_nat ()
{
linux_target = &the_ppc_linux_nat_target;
- gdb::observers::thread_exit.attach (ppc_linux_thread_exit);
-
/* Register the target. */
add_inf_child_target (linux_target);
}
--
2.24.1
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH v2 0/3] low_new_clone in linux-nat.c and powerpc watchpoint fixes
@ 2020-02-14 20:55 Pedro Franco de Carvalho
2020-02-14 20:55 ` [PATCH v2 3/3] [PowerPC] Fix debug register issues in ppc-linux-nat Pedro Franco de Carvalho
` (2 more replies)
0 siblings, 3 replies; 15+ messages in thread
From: Pedro Franco de Carvalho @ 2020-02-14 20:55 UTC (permalink / raw)
To: gdb-patches; +Cc: ulrich.weigand, rcardoso
This patch series fixes some issues with hardware breakpoints and
watchpoints in the powerpc linux native target.
The first patch adds a new "low" method to linux-nat.c, which is needed
in ppc-linux-nat.c. The second one is a small refactoring, and the third
one is the fix itself.
The differences compared to v1 are:
* Changed the stopped_data_addres/stopped_by_watchpoint so the low_*
versions so that we get the right stop_reason in the stop_reason field
of lwp_info. We can then use stop_reason in low_prepare_to_resume as
an additional indicator that the debug registers are stale, since some
kernel configurations disable a watchpoint after a hit.
* Instead of using an observer callback to clean-up the thread state, I
did this in low_delete_thread, because I'm more confident that this
will be called (some lwp_info object might not end up having an
associated GDB thread object).
Pedro Franco de Carvalho (3):
Add low_new_clone method to linux_nat_target.
[PowerPC] Move up some register access routines
[PowerPC] Fix debug register issues in ppc-linux-nat
gdb/linux-nat.c | 4 +
gdb/linux-nat.h | 4 +
gdb/ppc-linux-nat.c | 1584 ++++++++++++++++++++++++++++---------------
3 files changed, 1058 insertions(+), 534 deletions(-)
--
2.24.1
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH v2 1/3] Add low_new_clone method to linux_nat_target.
2020-02-14 20:55 [PATCH v2 0/3] low_new_clone in linux-nat.c and powerpc watchpoint fixes Pedro Franco de Carvalho
2020-02-14 20:55 ` [PATCH v2 3/3] [PowerPC] Fix debug register issues in ppc-linux-nat Pedro Franco de Carvalho
@ 2020-02-14 20:55 ` Pedro Franco de Carvalho
2020-02-17 17:49 ` Ulrich Weigand
2020-02-14 20:55 ` [PATCH v2 2/3] [PowerPC] Move up some register access routines Pedro Franco de Carvalho
2 siblings, 1 reply; 15+ messages in thread
From: Pedro Franco de Carvalho @ 2020-02-14 20:55 UTC (permalink / raw)
To: gdb-patches; +Cc: ulrich.weigand, rcardoso
This patch adds a low_new_clone method to linux_nat_target, called after
a PTRACE_EVENT_CLONE is detected, similar to how low_new_fork is called
after PTRACE_EVENT_(V)FORK.
This is useful for targets that need to copy state associated with a
thread that is inherited across clones.
gdb/ChangeLog:
YYYY-MM-DD Pedro Franco de Carvalho <pedromfc@linux.ibm.com>
* linux-nat.h (low_new_clone): New method.
* linux-nat.c (linux_handle_extended_wait): Call low_new_clone.
---
gdb/linux-nat.c | 4 ++++
gdb/linux-nat.h | 4 ++++
2 files changed, 8 insertions(+)
diff --git a/gdb/linux-nat.c b/gdb/linux-nat.c
index 81af83c4ac..1af7825c92 100644
--- a/gdb/linux-nat.c
+++ b/gdb/linux-nat.c
@@ -1978,6 +1978,10 @@ linux_handle_extended_wait (struct lwp_info *lp, int status)
inferior. */
linux_target->low_new_fork (lp, new_pid);
}
+ else if (event == PTRACE_EVENT_CLONE)
+ {
+ linux_target->low_new_clone (lp, new_pid);
+ }
if (event == PTRACE_EVENT_FORK
&& linux_fork_checkpointing_p (lp->ptid.pid ()))
diff --git a/gdb/linux-nat.h b/gdb/linux-nat.h
index f800e43997..4e2c622ec4 100644
--- a/gdb/linux-nat.h
+++ b/gdb/linux-nat.h
@@ -164,6 +164,10 @@ public:
virtual void low_new_fork (struct lwp_info *parent, pid_t child_pid)
{}
+ /* The method to call, if any, when a new clone event is detected. */
+ virtual void low_new_clone (struct lwp_info *parent, pid_t child_lwp)
+ {}
+
/* The method to call, if any, when a process is no longer
attached. */
virtual void low_forget_process (pid_t pid)
--
2.24.1
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH v2 2/3] [PowerPC] Move up some register access routines
2020-02-14 20:55 [PATCH v2 0/3] low_new_clone in linux-nat.c and powerpc watchpoint fixes Pedro Franco de Carvalho
2020-02-14 20:55 ` [PATCH v2 3/3] [PowerPC] Fix debug register issues in ppc-linux-nat Pedro Franco de Carvalho
2020-02-14 20:55 ` [PATCH v2 1/3] Add low_new_clone method to linux_nat_target Pedro Franco de Carvalho
@ 2020-02-14 20:55 ` Pedro Franco de Carvalho
2020-02-17 17:48 ` Ulrich Weigand
2 siblings, 1 reply; 15+ messages in thread
From: Pedro Franco de Carvalho @ 2020-02-14 20:55 UTC (permalink / raw)
To: gdb-patches; +Cc: ulrich.weigand, rcardoso
Keep the routines related to register access grouped together.
gdb/ChangeLog:
YYYY-MM-DD Pedro Franco de Carvalho <pedromfc@linux.ibm.com>
* ppc-linux-nat.c (ppc_linux_nat_target::store_registers)
(ppc_linux_nat_target::auxv_parse)
(ppc_linux_nat_target::read_description)
(supply_gregset, fill_gregset, supply_fpregset, fill_fpregset):
Move up.
---
gdb/ppc-linux-nat.c | 328 ++++++++++++++++++++++----------------------
1 file changed, 164 insertions(+), 164 deletions(-)
diff --git a/gdb/ppc-linux-nat.c b/gdb/ppc-linux-nat.c
index 27fa7f93e2..2295406234 100644
--- a/gdb/ppc-linux-nat.c
+++ b/gdb/ppc-linux-nat.c
@@ -1555,6 +1555,170 @@ store_ppc_registers (const struct regcache *regcache, int tid)
function to fail most of the time, so we ignore them. */
}
+void
+ppc_linux_nat_target::store_registers (struct regcache *regcache, int regno)
+{
+ pid_t tid = get_ptrace_pid (regcache->ptid ());
+
+ if (regno >= 0)
+ store_register (regcache, tid, regno);
+ else
+ store_ppc_registers (regcache, tid);
+}
+
+/* Functions for transferring registers between a gregset_t or fpregset_t
+ (see sys/ucontext.h) and gdb's regcache. The word size is that used
+ by the ptrace interface, not the current program's ABI. Eg. if a
+ powerpc64-linux gdb is being used to debug a powerpc32-linux app, we
+ read or write 64-bit gregsets. This is to suit the host libthread_db. */
+
+void
+supply_gregset (struct regcache *regcache, const gdb_gregset_t *gregsetp)
+{
+ const struct regset *regset = ppc_linux_gregset (sizeof (long));
+
+ ppc_supply_gregset (regset, regcache, -1, gregsetp, sizeof (*gregsetp));
+}
+
+void
+fill_gregset (const struct regcache *regcache,
+ gdb_gregset_t *gregsetp, int regno)
+{
+ const struct regset *regset = ppc_linux_gregset (sizeof (long));
+
+ if (regno == -1)
+ memset (gregsetp, 0, sizeof (*gregsetp));
+ ppc_collect_gregset (regset, regcache, regno, gregsetp, sizeof (*gregsetp));
+}
+
+void
+supply_fpregset (struct regcache *regcache, const gdb_fpregset_t * fpregsetp)
+{
+ const struct regset *regset = ppc_linux_fpregset ();
+
+ ppc_supply_fpregset (regset, regcache, -1,
+ fpregsetp, sizeof (*fpregsetp));
+}
+
+void
+fill_fpregset (const struct regcache *regcache,
+ gdb_fpregset_t *fpregsetp, int regno)
+{
+ const struct regset *regset = ppc_linux_fpregset ();
+
+ ppc_collect_fpregset (regset, regcache, regno,
+ fpregsetp, sizeof (*fpregsetp));
+}
+
+int
+ppc_linux_nat_target::auxv_parse (gdb_byte **readptr,
+ gdb_byte *endptr, CORE_ADDR *typep,
+ CORE_ADDR *valp)
+{
+ int tid = inferior_ptid.lwp ();
+ if (tid == 0)
+ tid = inferior_ptid.pid ();
+
+ int sizeof_auxv_field = ppc_linux_target_wordsize (tid);
+
+ enum bfd_endian byte_order = gdbarch_byte_order (target_gdbarch ());
+ gdb_byte *ptr = *readptr;
+
+ if (endptr == ptr)
+ return 0;
+
+ if (endptr - ptr < sizeof_auxv_field * 2)
+ return -1;
+
+ *typep = extract_unsigned_integer (ptr, sizeof_auxv_field, byte_order);
+ ptr += sizeof_auxv_field;
+ *valp = extract_unsigned_integer (ptr, sizeof_auxv_field, byte_order);
+ ptr += sizeof_auxv_field;
+
+ *readptr = ptr;
+ return 1;
+}
+
+const struct target_desc *
+ppc_linux_nat_target::read_description ()
+{
+ int tid = inferior_ptid.lwp ();
+ if (tid == 0)
+ tid = inferior_ptid.pid ();
+
+ if (have_ptrace_getsetevrregs)
+ {
+ struct gdb_evrregset_t evrregset;
+
+ if (ptrace (PTRACE_GETEVRREGS, tid, 0, &evrregset) >= 0)
+ return tdesc_powerpc_e500l;
+
+ /* EIO means that the PTRACE_GETEVRREGS request isn't supported.
+ Anything else needs to be reported. */
+ else if (errno != EIO)
+ perror_with_name (_("Unable to fetch SPE registers"));
+ }
+
+ struct ppc_linux_features features = ppc_linux_no_features;
+
+ features.wordsize = ppc_linux_target_wordsize (tid);
+
+ CORE_ADDR hwcap = linux_get_hwcap (current_top_target ());
+ CORE_ADDR hwcap2 = linux_get_hwcap2 (current_top_target ());
+
+ if (have_ptrace_getsetvsxregs
+ && (hwcap & PPC_FEATURE_HAS_VSX))
+ {
+ gdb_vsxregset_t vsxregset;
+
+ if (ptrace (PTRACE_GETVSXREGS, tid, 0, &vsxregset) >= 0)
+ features.vsx = true;
+
+ /* EIO means that the PTRACE_GETVSXREGS request isn't supported.
+ Anything else needs to be reported. */
+ else if (errno != EIO)
+ perror_with_name (_("Unable to fetch VSX registers"));
+ }
+
+ if (have_ptrace_getvrregs
+ && (hwcap & PPC_FEATURE_HAS_ALTIVEC))
+ {
+ gdb_vrregset_t vrregset;
+
+ if (ptrace (PTRACE_GETVRREGS, tid, 0, &vrregset) >= 0)
+ features.altivec = true;
+
+ /* EIO means that the PTRACE_GETVRREGS request isn't supported.
+ Anything else needs to be reported. */
+ else if (errno != EIO)
+ perror_with_name (_("Unable to fetch AltiVec registers"));
+ }
+
+ features.isa205 = ppc_linux_has_isa205 (hwcap);
+
+ if ((hwcap2 & PPC_FEATURE2_DSCR)
+ && check_regset (tid, NT_PPC_PPR, PPC_LINUX_SIZEOF_PPRREGSET)
+ && check_regset (tid, NT_PPC_DSCR, PPC_LINUX_SIZEOF_DSCRREGSET))
+ {
+ features.ppr_dscr = true;
+ if ((hwcap2 & PPC_FEATURE2_ARCH_2_07)
+ && (hwcap2 & PPC_FEATURE2_TAR)
+ && (hwcap2 & PPC_FEATURE2_EBB)
+ && check_regset (tid, NT_PPC_TAR, PPC_LINUX_SIZEOF_TARREGSET)
+ && check_regset (tid, NT_PPC_EBB, PPC_LINUX_SIZEOF_EBBREGSET)
+ && check_regset (tid, NT_PPC_PMU, PPC_LINUX_SIZEOF_PMUREGSET))
+ {
+ features.isa207 = true;
+ if ((hwcap2 & PPC_FEATURE2_HTM)
+ && check_regset (tid, NT_PPC_TM_SPR,
+ PPC_LINUX_SIZEOF_TM_SPRREGSET))
+ features.htm = true;
+ }
+ }
+
+ return ppc_linux_match_description (features);
+}
+
/* The cached DABR value, to install in new threads.
This variable is used when the PowerPC HWDEBUG ptrace
interface is not available. */
@@ -2507,170 +2671,6 @@ ppc_linux_nat_target::masked_watch_num_registers (CORE_ADDR addr, CORE_ADDR mask
return 2;
}
-void
-ppc_linux_nat_target::store_registers (struct regcache *regcache, int regno)
-{
- pid_t tid = get_ptrace_pid (regcache->ptid ());
-
- if (regno >= 0)
- store_register (regcache, tid, regno);
- else
- store_ppc_registers (regcache, tid);
-}
-
-/* Functions for transferring registers between a gregset_t or fpregset_t
- (see sys/ucontext.h) and gdb's regcache. The word size is that used
- by the ptrace interface, not the current program's ABI. Eg. if a
- powerpc64-linux gdb is being used to debug a powerpc32-linux app, we
- read or write 64-bit gregsets. This is to suit the host libthread_db. */
-
-void
-supply_gregset (struct regcache *regcache, const gdb_gregset_t *gregsetp)
-{
- const struct regset *regset = ppc_linux_gregset (sizeof (long));
-
- ppc_supply_gregset (regset, regcache, -1, gregsetp, sizeof (*gregsetp));
-}
-
-void
-fill_gregset (const struct regcache *regcache,
- gdb_gregset_t *gregsetp, int regno)
-{
- const struct regset *regset = ppc_linux_gregset (sizeof (long));
-
- if (regno == -1)
- memset (gregsetp, 0, sizeof (*gregsetp));
- ppc_collect_gregset (regset, regcache, regno, gregsetp, sizeof (*gregsetp));
-}
-
-void
-supply_fpregset (struct regcache *regcache, const gdb_fpregset_t * fpregsetp)
-{
- const struct regset *regset = ppc_linux_fpregset ();
-
- ppc_supply_fpregset (regset, regcache, -1,
- fpregsetp, sizeof (*fpregsetp));
-}
-
-void
-fill_fpregset (const struct regcache *regcache,
- gdb_fpregset_t *fpregsetp, int regno)
-{
- const struct regset *regset = ppc_linux_fpregset ();
-
- ppc_collect_fpregset (regset, regcache, regno,
- fpregsetp, sizeof (*fpregsetp));
-}
-
-int
-ppc_linux_nat_target::auxv_parse (gdb_byte **readptr,
- gdb_byte *endptr, CORE_ADDR *typep,
- CORE_ADDR *valp)
-{
- int tid = inferior_ptid.lwp ();
- if (tid == 0)
- tid = inferior_ptid.pid ();
-
- int sizeof_auxv_field = ppc_linux_target_wordsize (tid);
-
- enum bfd_endian byte_order = gdbarch_byte_order (target_gdbarch ());
- gdb_byte *ptr = *readptr;
-
- if (endptr == ptr)
- return 0;
-
- if (endptr - ptr < sizeof_auxv_field * 2)
- return -1;
-
- *typep = extract_unsigned_integer (ptr, sizeof_auxv_field, byte_order);
- ptr += sizeof_auxv_field;
- *valp = extract_unsigned_integer (ptr, sizeof_auxv_field, byte_order);
- ptr += sizeof_auxv_field;
-
- *readptr = ptr;
- return 1;
-}
-
-const struct target_desc *
-ppc_linux_nat_target::read_description ()
-{
- int tid = inferior_ptid.lwp ();
- if (tid == 0)
- tid = inferior_ptid.pid ();
-
- if (have_ptrace_getsetevrregs)
- {
- struct gdb_evrregset_t evrregset;
-
- if (ptrace (PTRACE_GETEVRREGS, tid, 0, &evrregset) >= 0)
- return tdesc_powerpc_e500l;
-
- /* EIO means that the PTRACE_GETEVRREGS request isn't supported.
- Anything else needs to be reported. */
- else if (errno != EIO)
- perror_with_name (_("Unable to fetch SPE registers"));
- }
-
- struct ppc_linux_features features = ppc_linux_no_features;
-
- features.wordsize = ppc_linux_target_wordsize (tid);
-
- CORE_ADDR hwcap = linux_get_hwcap (current_top_target ());
- CORE_ADDR hwcap2 = linux_get_hwcap2 (current_top_target ());
-
- if (have_ptrace_getsetvsxregs
- && (hwcap & PPC_FEATURE_HAS_VSX))
- {
- gdb_vsxregset_t vsxregset;
-
- if (ptrace (PTRACE_GETVSXREGS, tid, 0, &vsxregset) >= 0)
- features.vsx = true;
-
- /* EIO means that the PTRACE_GETVSXREGS request isn't supported.
- Anything else needs to be reported. */
- else if (errno != EIO)
- perror_with_name (_("Unable to fetch VSX registers"));
- }
-
- if (have_ptrace_getvrregs
- && (hwcap & PPC_FEATURE_HAS_ALTIVEC))
- {
- gdb_vrregset_t vrregset;
-
- if (ptrace (PTRACE_GETVRREGS, tid, 0, &vrregset) >= 0)
- features.altivec = true;
-
- /* EIO means that the PTRACE_GETVRREGS request isn't supported.
- Anything else needs to be reported. */
- else if (errno != EIO)
- perror_with_name (_("Unable to fetch AltiVec registers"));
- }
-
- features.isa205 = ppc_linux_has_isa205 (hwcap);
-
- if ((hwcap2 & PPC_FEATURE2_DSCR)
- && check_regset (tid, NT_PPC_PPR, PPC_LINUX_SIZEOF_PPRREGSET)
- && check_regset (tid, NT_PPC_DSCR, PPC_LINUX_SIZEOF_DSCRREGSET))
- {
- features.ppr_dscr = true;
- if ((hwcap2 & PPC_FEATURE2_ARCH_2_07)
- && (hwcap2 & PPC_FEATURE2_TAR)
- && (hwcap2 & PPC_FEATURE2_EBB)
- && check_regset (tid, NT_PPC_TAR, PPC_LINUX_SIZEOF_TARREGSET)
- && check_regset (tid, NT_PPC_EBB, PPC_LINUX_SIZEOF_EBBREGSET)
- && check_regset (tid, NT_PPC_PMU, PPC_LINUX_SIZEOF_PMUREGSET))
- {
- features.isa207 = true;
- if ((hwcap2 & PPC_FEATURE2_HTM)
- && check_regset (tid, NT_PPC_TM_SPR,
- PPC_LINUX_SIZEOF_TM_SPRREGSET))
- features.htm = true;
- }
- }
-
- return ppc_linux_match_description (features);
-}
-
void _initialize_ppc_linux_nat ();
void
_initialize_ppc_linux_nat ()
--
2.24.1
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2 3/3] [PowerPC] Fix debug register issues in ppc-linux-nat
2020-02-14 20:55 ` [PATCH v2 3/3] [PowerPC] Fix debug register issues in ppc-linux-nat Pedro Franco de Carvalho
@ 2020-02-17 17:47 ` Ulrich Weigand
2020-02-18 20:31 ` Pedro Franco de Carvalho
2020-03-13 20:19 ` Pedro Franco de Carvalho
0 siblings, 2 replies; 15+ messages in thread
From: Ulrich Weigand @ 2020-02-17 17:47 UTC (permalink / raw)
To: Pedro Franco de Carvalho; +Cc: gdb-patches, ulrich.weigand, rcardoso
Pedro Franco de Carvalho wrote:
> This patch fixes some issues with debug register handling for the powerpc
> linux native target.
This looks quite good to me, I just have a couple of comments / questions
on implementation details remaining, primarily about the data structures.
> * ppc-linux-nat.c: Include <algorithm>, <unordered_map>, and
> <list>. Remove inlcusion of observable.h.
Typo.
> +/* Private arch info associated with each thread lwp_info object, used
> + for debug register handling. */
> +
> +struct arch_lwp_info
> +{
> + /* When true, indicates that the debug registers installed in the
> + thread no longer correspond to the watchpoints and breakpoints
> + requested by GDB. */
> + bool debug_regs_stale;
> +
> + /* We need a back-reference to the PTID of the thread so that we can
> + cleanup the debug register state of the thread in
> + low_delete_thread. */
> + ptid_t lwp_ptid;
Can we simply store the installed slots map in here, instead of requiring
a whole new per-lwp map in m_installed_hw_bps?
> +/* Class used to detect which set of ptrace requests that
> + ppc_linux_nat_target will use to install and remove hardware
> + breakpoints and watchpoints.
> +
> + The first time the interface is requested it is detected, and
> + subsequent requests will return the same value. This value can
> + indicate that no interface is available.
The one thing I'm wondering about here is that in order to detect
the interface, the code will rely on inferior_ptid. This introduces
a new use of the inferior_ptid deeply buried within a function that
is called from a large number of places -- are we sure all those
callers actually always have a (correct) inferior_ptid in place?
Now, in practice, inferior_ptid is only needed for the *first*
call, and that first call will likely happen only from a small
subset of all the possible callers here.
But it would seem cleaner to make this explicit by having an
explicit "initialize" or "detect" call, which gets called in
those places we expect to be "first", and which gets passed
a ptid_t to use (where the callers will still pass inferior_ptid,
but then at least the dependency will be explicit.
> + /* The ptrace interface we'll use to install hardware watchpoints and
> + breakpoints (debug registers). */
> + ppc_linux_dreg_interface m_dreg_interface;
> +
> + /* A map from pid numbers to the list of hardware watchpoints and
> + breakpoints that GDB requested for each process.
> +
> + Only used when the interface is HWDEBUG. */
> + std::unordered_map<pid_t, std::list<ppc_hw_breakpoint>>
> + m_requested_hw_bps;
> +
> + /* A map from pid numbers to the watchpoint value that GDB requested
> + for each process.
> +
> + Only used when the interface is DEBUGREG. */
> + std::unordered_map<pid_t, long> m_requested_wp_vals;
I'm wondering if it might be preferable to have a single map from pid_t
to a "per-process HW break/watchpoint" structure, which tracks the
lifetime of the process (cleaned up completely in low_forget_process),
and holds all the data (list of ppc_hw_breakpoint structs, plus a WP
value)?
[ *Maybe* (and I'm not sure here) it would even make sense to move the
ppc_linux_dreg_interface into that per-process struct, to clearly
associate it with the pid that was used to query the kernel? ]
> + /* Callable object to hash ptids by their lwp number. */
> + struct ptid_hash
> + {
> + std::size_t operator() (const ptid_t &ptid) const
> + {
> + return std::hash<long>{} (ptid.lwp ());
> + }
> + };
> +
> + /* A map from ptid_t objects to a list of pairs of slots and hardware
> + breakpoint objects. This keeps track of which hardware breakpoints
> + and watchpoints were last installed in each slot of each thread.
> +
> + Only used when the interface is HWDEBUG. */
> + std::unordered_map <ptid_t,
> + std::list<std::pair<long, ppc_hw_breakpoint>>,
> + ptid_hash> m_installed_hw_bps;
As mentioned above, why not move this to arch_lwp_info?
> +ppc_linux_nat_target::hwdebug_point_cmp
> +(const struct ppc_hw_breakpoint &a, const struct ppc_hw_breakpoint &b)
You're using this style in a number of places, but I don't think this
complies with the GNU coding style ... (The '(' should not be in the
first column.)
> + bool stale_dregs = false;
Initializer unnecessary here.
> +
> + stale_dregs = (lp->stop_reason == TARGET_STOPPED_BY_WATCHPOINT
> + || lp->stop_reason == TARGET_STOPPED_BY_HW_BREAKPOINT
> + || lp_arch_info->debug_regs_stale);
> +
> + if (!stale_dregs)
> return;
Bye,
Ulrich
--
Dr. Ulrich Weigand
GNU/Linux compilers and toolchain
Ulrich.Weigand@de.ibm.com
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2 2/3] [PowerPC] Move up some register access routines
2020-02-14 20:55 ` [PATCH v2 2/3] [PowerPC] Move up some register access routines Pedro Franco de Carvalho
@ 2020-02-17 17:48 ` Ulrich Weigand
0 siblings, 0 replies; 15+ messages in thread
From: Ulrich Weigand @ 2020-02-17 17:48 UTC (permalink / raw)
To: Pedro Franco de Carvalho; +Cc: gdb-patches, ulrich.weigand, rcardoso
Pedro Franco de Carvalho wrote:
> * ppc-linux-nat.c (ppc_linux_nat_target::store_registers)
> (ppc_linux_nat_target::auxv_parse)
> (ppc_linux_nat_target::read_description)
> (supply_gregset, fill_gregset, supply_fpregset, fill_fpregset):
> Move up.
This is OK. (Obvious change , really :-))
Bye,
Ulrich
--
Dr. Ulrich Weigand
GNU/Linux compilers and toolchain
Ulrich.Weigand@de.ibm.com
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2 1/3] Add low_new_clone method to linux_nat_target.
2020-02-14 20:55 ` [PATCH v2 1/3] Add low_new_clone method to linux_nat_target Pedro Franco de Carvalho
@ 2020-02-17 17:49 ` Ulrich Weigand
0 siblings, 0 replies; 15+ messages in thread
From: Ulrich Weigand @ 2020-02-17 17:49 UTC (permalink / raw)
To: Pedro Franco de Carvalho; +Cc: gdb-patches, ulrich.weigand, rcardoso
Pedro Franco de Carvalho wrote:
> This patch adds a low_new_clone method to linux_nat_target, called after
> a PTRACE_EVENT_CLONE is detected, similar to how low_new_fork is called
> after PTRACE_EVENT_(V)FORK.
>
> This is useful for targets that need to copy state associated with a
> thread that is inherited across clones.
Given the discussion we had last year, this is really required for a
correct Power implementation of watchpoints. Given that the implementation
in common code is straightforward, I this this is fine.
> * linux-nat.h (low_new_clone): New method.
> * linux-nat.c (linux_handle_extended_wait): Call low_new_clone.
OK.
Thanks,
Ulrich
--
Dr. Ulrich Weigand
GNU/Linux compilers and toolchain
Ulrich.Weigand@de.ibm.com
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2 3/3] [PowerPC] Fix debug register issues in ppc-linux-nat
2020-02-17 17:47 ` Ulrich Weigand
@ 2020-02-18 20:31 ` Pedro Franco de Carvalho
2020-02-19 13:46 ` Ulrich Weigand
2020-03-13 20:19 ` Pedro Franco de Carvalho
1 sibling, 1 reply; 15+ messages in thread
From: Pedro Franco de Carvalho @ 2020-02-18 20:31 UTC (permalink / raw)
To: Ulrich Weigand; +Cc: gdb-patches, ulrich.weigand, rcardoso
"Ulrich Weigand" <uweigand@de.ibm.com> writes:
> Can we simply store the installed slots map in here, instead of requiring
> a whole new per-lwp map in m_installed_hw_bps?
I had considered doing this, however, low_new_fork needs to copy the
per-lwp state in case the debug registers are copied across forks, and
this function is called before the lwp_info object for the new forked
thread is constructed, which only happens in
linux_nat_target::follow_fork.
> But it would seem cleaner to make this explicit by having an
> explicit "initialize" or "detect" call, which gets called in
> those places we expect to be "first", and which gets passed
> a ptid_t to use (where the callers will still pass inferior_ptid,
> but then at least the dependency will be explicit.
Agreed. I'm investigating the best way to do this.
> I'm wondering if it might be preferable to have a single map from pid_t
> to a "per-process HW break/watchpoint" structure, which tracks the
> lifetime of the process (cleaned up completely in low_forget_process),
> and holds all the data (list of ppc_hw_breakpoint structs, plus a WP
> value)?
Yes, that would probably be cleaner.
> [ *Maybe* (and I'm not sure here) it would even make sense to move the
> ppc_linux_dreg_interface into that per-process struct, to clearly
> associate it with the pid that was used to query the kernel? ]
I'm not yet sure about this one, I have to think a bit more.
>> +ppc_linux_nat_target::hwdebug_point_cmp
>> +(const struct ppc_hw_breakpoint &a, const struct ppc_hw_breakpoint &b)
>
> You're using this style in a number of places, but I don't think this
> complies with the GNU coding style ... (The '(' should not be in the
> first column.)
I will change this. I had done this because even if I broke the line
after the first argument, the line still had more than the soft limit of
columns (74):
ppc_linux_nat_target::hwdebug_point_cmp (const struct ppc_hw_breakpoint &a,
const struct ppc_hw_breakpoint &b)
Is this a reasonable reason to exceed the soft limit column limit? It's
under the hard limit (80). If it's not reasonable, I'll have to do
something like:
bool
ppc_linux_nat_target::hwdebug_point_cmp (const struct
ppc_hw_breakpoint &a,
const struct
ppc_hw_breakpoint &b)
Thanks a lot for the review!
--
Pedro Franco de Carvalho
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2 3/3] [PowerPC] Fix debug register issues in ppc-linux-nat
2020-02-18 20:31 ` Pedro Franco de Carvalho
@ 2020-02-19 13:46 ` Ulrich Weigand
0 siblings, 0 replies; 15+ messages in thread
From: Ulrich Weigand @ 2020-02-19 13:46 UTC (permalink / raw)
To: Pedro Franco de Carvalho; +Cc: gdb-patches, ulrich.weigand, rcardoso
Pedro Franco de Carvalho wrote:
> "Ulrich Weigand" <uweigand@de.ibm.com> writes:
>
> > Can we simply store the installed slots map in here, instead of requiring
> > a whole new per-lwp map in m_installed_hw_bps?
>
> I had considered doing this, however, low_new_fork needs to copy the
> per-lwp state in case the debug registers are copied across forks, and
> this function is called before the lwp_info object for the new forked
> thread is constructed, which only happens in
> linux_nat_target::follow_fork.
Huh, indeed. It is strange that linux_handle_extended_wait creates
the new lwp_info for PTRACE_EVENT_CLONE events, but *not* for
PTRACE_EVENT_FORK / PTRACE_EVENT_VFORK. In the end, maybe everything
would be simpler if we did that ...
But that's probably not something that should hold up this patch,
so I guess I'm fine with the extra list.
> >> +ppc_linux_nat_target::hwdebug_point_cmp
> >> +(const struct ppc_hw_breakpoint &a, const struct ppc_hw_breakpoint &b)
> >
> > You're using this style in a number of places, but I don't think this
> > complies with the GNU coding style ... (The '(' should not be in the
> > first column.)
>
> I will change this. I had done this because even if I broke the line
> after the first argument, the line still had more than the soft limit of
> columns (74):
>
> ppc_linux_nat_target::hwdebug_point_cmp (const struct ppc_hw_breakpoint &a,
> const struct ppc_hw_breakpoint &b)
As long as it's under 80 columns, I think this should be fine.
Bye,
Ulrich
--
Dr. Ulrich Weigand
GNU/Linux compilers and toolchain
Ulrich.Weigand@de.ibm.com
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2 3/3] [PowerPC] Fix debug register issues in ppc-linux-nat
2020-02-17 17:47 ` Ulrich Weigand
2020-02-18 20:31 ` Pedro Franco de Carvalho
@ 2020-03-13 20:19 ` Pedro Franco de Carvalho
2020-03-27 18:50 ` Pedro Franco de Carvalho
1 sibling, 1 reply; 15+ messages in thread
From: Pedro Franco de Carvalho @ 2020-03-13 20:19 UTC (permalink / raw)
To: Ulrich Weigand; +Cc: gdb-patches, ulrich.weigand, rcardoso
"Ulrich Weigand" <uweigand@de.ibm.com> writes:
> The one thing I'm wondering about here is that in order to detect
> the interface, the code will rely on inferior_ptid. This introduces
> a new use of the inferior_ptid deeply buried within a function that
> is called from a large number of places -- are we sure all those
> callers actually always have a (correct) inferior_ptid in place?
>
> Now, in practice, inferior_ptid is only needed for the *first*
> call, and that first call will likely happen only from a small
> subset of all the possible callers here.
>
> But it would seem cleaner to make this explicit by having an
> explicit "initialize" or "detect" call, which gets called in
> those places we expect to be "first", and which gets passed
> a ptid_t to use (where the callers will still pass inferior_ptid,
> but then at least the dependency will be explicit.
I'm taking a while to post an updated patch because I've been thinking
about this issue in some detail.
I agree that it would be better to make the uses of inferior_ptid
explicit with a "detect" method that takes the pid.
I don't know if all the target methods guarantee that inferior_ptid will
be properly set. However, after going through them, I don't believe I
have introduced any additional effective uses of inferior_ptid, compared
to the old have_ptrace_hwdebug_interface function (which is also a
deeply buried use of inferior_ptid), with the exception of
low_stopped_data_address and low_stopped_by_watchpoint, but I've checked
that linux_nat_target explicitly sets inferior_ptid to the stopped lwp
before calling these, so I assume they are allowed to use it. Another
exception is that the interface class can call ptrace twice when
detecting the interface, since it now also checks for the DEBUGREG
interface, while have_ptrace_hwdebug_interface only checked for the
HWDEBUG interface.
All the new and modified low_ methods only use the interface if it has
already been detected previously, otherwise, they don't have any work to
do. So in practice they never use inferior_ptid. But here too it would
be cleaner if the interface class had an explicit "is_detected" method
and these low_ methods don't use "detect".
Another solution I considered at length that seemed more natural is to
detect the interface in post_startup_inferior and post_attach. ARM does
something similar to figure out their debug register capabilities. The
post_startup_inferior method seems to clearly work. I'm not sure about
post_attach. I saw some weird cases where attaching to a threaded
program causes post_attach to be called with inferior_ptid set to one of
the extra threads, even though post_attach takes only the pid as an
argument and all implementations (e.g. in linux_nat_target) use the this
process pid for their ptrace calls. This happens because in all-stop
mode all the threads are forced stopped and setup_inferior is called as
a consequence of that. I suspect that using this pid would still work
since the main thread which is attached to is explicitly waited for in
linux_nat_target::attach.
I'm not sure if other weird effects can happen in non-stop mode that
would cause a watchpoint target method to be called before post_attach
is called, e.g. a thread stopping after an attach command is issued, but
before post_attach is executed, and the user trying to set a watchpoint
in this instant, when the interface won't have been detected yet. I'm
having some trouble reasoning about all these possibilities in infrun.
Note that linux_nat_target sets ptrace flags in post_attach, but also
sets a bool in every additional lwp object to set these same flags for
every other thread the next time they stop.
Considering these issues, would you advise me to:
1. Just use inferior ptid in the target methods, like it was done
before, except more explicitly, with a "detect method".
2. Assume that post_startup_inferior and post_startup will always have
been called before any watchpoint-related target method has a chance
to be called, and detect the interface only in these two places. I
don't know if this assumption holds true.
- If so, should every target method that needs the interface assert
that it is detected when they are called? This would probably be
the easiest thing to do, as I don't know if there are any issues in
case something else is done (such as returning an error code) and
the interface is not detected in one call but is in a subsequent
call. However, considering the issues above, these asserts could
be triggered in some weird scenarios.
> [ *Maybe* (and I'm not sure here) it would even make sense to move the
> ppc_linux_dreg_interface into that per-process struct, to clearly
> associate it with the pid that was used to query the kernel? ]
This would naturally solve the problem of getting an ESRCH when querying
the kernel if the process is unlucky enough to get a sigkill right
before the ptrace call. In this case, only the signalled process would
get no interface and be prevented from using hardware watchpoints. With
the current version of the patch, if this happens, the whole GDB session
will be prevented from using hardware watchpoints, since the interface
is associated with the native target and there's only one global native
target.
Before the patch, this would only prevent the HWDEBUG interface from
being used, since GDB would still try the DEBUGREG interface in
can_use_hw_breakpoint, but this seems accidental and I think is more
confusing that testing if DEBUGREG is available in the interface class.
However, doing this would introduce additional dependencies on
inferior_ptid in all the target methods, since they would need the pid
to access the interface. Most of the target methods don't actually use
the inferior_ptid, because by the time they are called, the interface
will already have been detected, but associating the interface object
with the per-process struct would necessarily require using the pid in
inferior_ptid.
Thanks!
--
Pedro Franco de Carvalho
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2 3/3] [PowerPC] Fix debug register issues in ppc-linux-nat
2020-03-13 20:19 ` Pedro Franco de Carvalho
@ 2020-03-27 18:50 ` Pedro Franco de Carvalho
2020-03-27 18:54 ` [PATCH] " Pedro Franco de Carvalho
2020-03-30 13:03 ` [PATCH v2 3/3] " Ulrich Weigand
0 siblings, 2 replies; 15+ messages in thread
From: Pedro Franco de Carvalho @ 2020-03-27 18:50 UTC (permalink / raw)
To: Ulrich Weigand; +Cc: ulrich.weigand, gdb-patches
I'll post in reply to this e-mail one possible new version of the patch,
that just makes the inferior_ptid uses explicit throughout
ppc-linux-nat.c.
It doesn't solve the ESRCH issue described in the previous e-mail.
Thanks!
--
Pedro Franco de Carvalho
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH] [PowerPC] Fix debug register issues in ppc-linux-nat
2020-03-27 18:50 ` Pedro Franco de Carvalho
@ 2020-03-27 18:54 ` Pedro Franco de Carvalho
2020-03-30 13:04 ` Ulrich Weigand
2020-03-30 13:03 ` [PATCH v2 3/3] " Ulrich Weigand
1 sibling, 1 reply; 15+ messages in thread
From: Pedro Franco de Carvalho @ 2020-03-27 18:54 UTC (permalink / raw)
To: gdb-patches; +Cc: ulrich.weigand, rcardoso
This patch fixes some issues with debug register handling for the powerpc
linux native target.
Currently, the target methods for installing and removing hardware
breakpoints and watchpoints in ppc-linux-nat.c affect all threads known to
linux-nat, including threads of different processes.
This patch changes ppc-linux-nat.c so that only the process of
inferior_ptid is affected by these target methods, as GDB expects.
This is done in the same way as various other architectures. The
install/remove target methods only register a hardware breakpoint or
watchpoint, and then send a stop signal to the threads. The debug
registers are only changed with ptrace right before each thread is next
resumed, using low_prepare_to_resume.
There are two interfaces to modify debug registers for linux running on
powerpc, with different sets of ptrace requests:
- PPC_PTRACE_GETHWDBGINFO, PPC_PTRACE_SETHWDEBUG, and
PPC_PTRACE_DELHWDEBUG.
Or
- PTRACE_SET_DEBUGREG and PTRACE_GET_DEBUGREG
The first set (HWDEBUG) is the more flexible one and allows setting
watchpoints with a variable watched region length and, for certain
embedded processors, multiple types of debug registers (e.g. hardware
breakpoints and hardware-assisted conditions for watchpoints).
Currently, server processors only provide one watchpoint. The second one
(DEBUGREG) only allows setting one debug register, a watchpoint, so we
only use it if the first one is not available.
The HWDEBUG interface handles debug registers with slot numbers. Once a
hardware watchpoint or breakpoint is installed (with
PPC_PTRACE_SETHWDEBUG), ptrace returns a slot number. This slot number
can then be used to remove the watchpoint or breakpoint from the inferior
(with PPC_PTRACE_DELHWDEBUG). The first interface also provides a
bitmask of available debug register features, which can be obtained with
PPC_PTRACE_GETHWDBGINFO.
When GDB first tries to use debug registers, we try the first interface
with a ptrace call, and if it isn't available, we fall back to the second
one, if available. We use EIO as an indicator that an interface is not
available in the kernel. For simplicity, with any other error we
immediately assume no interface is available. Unfortunately this means
that if a process is killed by a signal right before we try to detect the
interface, we might get an ESRCH, which would prevent debug registers to
be used in the GDB session. However, it isn't clear that we can safely
raise an exception and try again in the future in all the contexts where
we try to detect the interface.
If the HWDEBUG interface works but provides no feature bits, the target
falls back to the DEBUGREG interface. When the kernel is configured
without CONFIG_HW_BREAKPOINTS (selected by CONFIG_PERF_EVENTS), there is
a bug that causes watchpoints installed with the HWDEBUG interface not to
trigger. When this is the case, the feature bits will be zero, which is
used as the indicator to fall back to the DEBUGREG interface. This isn't
ideal, but has always been the behavior of GDB before this patch, so I
decided not to change it.
A flag indicates for each thread if its debug registers need to be
updated the next time it is resumed. The flag is set whenever the upper
layers request or remove a hardware watchpoint or breakpoint, or when a
new thread is detected. Because some kernel configurations disable
watchpoints after they are hit, we also use the last stop reason of the
LWP to determine whether we should update the debug registers. It isn't
clear that this is also true of BookE hardware breakpoints, but we also
check their stop reason to be on the safe side, since it doesn't hurt.
A map from process numbers to hardware watchpoint or breakpoint objects
keeps track of what has been requested by the upper layers of GDB, since
for GDB installing a hardware watchpoint or breakpoint means doing so for
the whole process.
When using the HWDEBUG interface we also have to keep track of which
slots were last installed in each thread with a map from threads to the
slots, so that they can be removed when needed. When resuming a thread,
we remove all the slots using this map, then we install all the hardware
watchpoints and breakpoints from the per-process map of requests, and
then update the per-thread map accordingly.
This per-thread state is also used for copying the debug register state
after a fork or a clone is detected. The kernel might do this depending
on the configuration. Recent kernels running on server processors that
were configured with CONFIG_PERF_EVENTS (and therefore
CONFIG_HW_BREAKPOINTS) don't copy debug registers across forks and
clones. Recent kernels without CONFIG_HW_BREAKPOINTS copy this state. I
believe that on embedded processors (e.g. a ppc440) the debug register
state is copied, but I haven't been able to test this. To handle both
cases, the per-thread state is always copied when forks and clones are
detected, and when we resume the thread and delete the debug register
slots before updating them, we ignore ENOENT errors.
We don't need to handle this when using the DEBUGREG interface since it
only allows one hardware watchpoint and doesn't return slot numbers, we
just set or clear this watchpoint when needed.
Since we signal running threads to stop after a request is processed, so
that we can update their debug registers when they are next resumed,
there will be a time between signalling the threads and their stop during
which the debug registers haven't been updated, even if the target
methods completed.
The tests in gdb.threads/watchpoint-fork.exp no longer fail with this
patch.
gdb/ChangeLog:
YYYY-MM-DD Pedro Franco de Carvalho <pedromfc@linux.ibm.com>
* ppc-linux-nat.c: Include <algorithm>, <unordered_map>, and
<list>. Remove inclusion of observable.h.
(PPC_DEBUG_CURRENT_VERSION): Move up define.
(struct arch_lwp_info): New struct.
(class ppc_linux_dreg_interface): New class.
(struct ppc_linux_process_info): New struct.
(struct ppc_linux_nat_target) <low_delete_thread, low_new_fork>
<low_new_clone, low_forget_process, low_prepare_to_resume>
<copy_thread_dreg_state, mark_thread_stale>
<mark_debug_registers_changed, register_hw_breakpoint>
<clear_hw_breakpoint, register_wp, clear_wp>
<can_use_watchpoint_cond_accel, calculate_dvc, check_condition>
<num_memory_accesses, get_trigger_type>
<create_watchpoint_request, hwdebug_point_cmp>
<init_arch_lwp_info, get_arch_lwp_info>
<low_stopped_by_watchpoint, low_stopped_data_address>: Declare as
methods.
<struct ptid_hash>: New inner struct.
<m_dreg_interface, m_process_info, m_installed_hw_bps>: Declare
members.
(saved_dabr_value, hwdebug_info, max_slots_number)
(struct hw_break_tuple, struct thread_points, ppc_threads)
(have_ptrace_hwdebug_interface)
(hwdebug_find_thread_points_by_tid)
(hwdebug_insert_point, hwdebug_remove_point): Remove.
(ppc_linux_nat_target::can_use_hw_breakpoint): Use
m_dreg_interface, remove call to PTRACE_SET_DEBUGREG.
(ppc_linux_nat_target::region_ok_for_hw_watchpoint): Add comment,
use m_dreg_interface.
(hwdebug_point_cmp): Change to...
(ppc_linux_nat_target::hwdebug_point_cmp): ...this method. Use
reference arguments instead of pointers.
(ppc_linux_nat_target::ranged_break_num_registers): Use
m_dreg_interface.
(ppc_linux_nat_target::insert_hw_breakpoint): Add comment, use
m_dreg_interface. Call register_hw_breakpoint.
(ppc_linux_nat_target::remove_hw_breakpoint): Add comment, use
m_dreg_interface. Call clear_hw_breakpoint.
(get_trigger_type): Change to...
(ppc_linux_nat_target::get_trigger_type): ...this method. Add
comment.
(ppc_linux_nat_target::insert_mask_watchpoint): Update comment,
use m_dreg_interface. Call register_hw_breakpoint.
(ppc_linux_nat_target::remove_mask_watchpoint): Update comment,
use m_dreg_interface. Call clear_hw_breakpoint.
(can_use_watchpoint_cond_accel): Change to...
(ppc_linux_nat_target::can_use_watchpoint_cond_accel): ...this
method. Update comment, use m_dreg_interface and
m_process_info.
(calculate_dvc): Change to...
(ppc_linux_nat_target::calculate_dvc): ...this method. Use
m_dreg_interface.
(num_memory_accesses): Change to...
(ppc_linux_nat_target::num_memory_accesses): ...this method.
(check_condition): Change to...
(ppc_linux_nat_target::check_condition): ...this method.
(ppc_linux_nat_target::can_accel_watchpoint_condition): Update
comment, use m_dreg_interface.
(create_watchpoint_request): Change to...
(ppc_linux_nat_target::create_watchpoint_request): ...this
method. Use m_dreg_interface.
(ppc_linux_nat_target::insert_watchpoint): Add comment, use
m_dreg_interface. Call register_hw_breakpoint or register_wp.
(ppc_linux_nat_target::remove_watchpoint): Add comment, use
m_dreg_interface. Call clear_hw_breakpoint or clear_wp.
(ppc_linux_nat_target::low_forget_process)
(ppc_linux_nat_target::low_new_fork)
(ppc_linux_nat_target::low_new_clone)
(ppc_linux_nat_target::low_delete_thread)
(ppc_linux_nat_target::low_prepare_to_resume): New methods.
(ppc_linux_nat_target::low_new_thread): Remove previous logic,
only call mark_thread_stale.
(ppc_linux_thread_exit): Remove.
(ppc_linux_nat_target::stopped_data_address): Change to...
(ppc_linux_nat_target::low_stopped_data_address): This. Add
comment, use m_dreg_interface and m_thread_hw_breakpoints.
(ppc_linux_nat_target::stopped_by_watchpoint): Change to...
(ppc_linux_nat_target::stopped_by_watchpoint): This. Add
comment. Call low_stopped_data_address.
(ppc_linux_nat_target::watchpoint_addr_within_range): Use
m_dreg_interface.
(ppc_linux_nat_target::masked_watch_num_registers): Use
m_dreg_interface.
(ppc_linux_nat_target::copy_thread_dreg_state)
(ppc_linux_nat_target::mark_thread_stale)
(ppc_linux_nat_target::mark_debug_registers_changed)
(ppc_linux_nat_target::register_hw_breakpoint)
(ppc_linux_nat_target::clear_hw_breakpoint)
(ppc_linux_nat_target::register_wp)
(ppc_linux_nat_target::clear_wp)
(ppc_linux_nat_target::init_arch_lwp_info)
(ppc_linux_nat_target::get_arch_lwp_info): New methods.
(_initialize_ppc_linux_nat): Remove observer callback.
---
gdb/ppc-linux-nat.c | 1320 ++++++++++++++++++++++++++++++-------------
1 file changed, 924 insertions(+), 396 deletions(-)
diff --git a/gdb/ppc-linux-nat.c b/gdb/ppc-linux-nat.c
index 2295406234..6be8f022a7 100644
--- a/gdb/ppc-linux-nat.c
+++ b/gdb/ppc-linux-nat.c
@@ -18,7 +18,6 @@
along with this program. If not, see <http://www.gnu.org/licenses/>. */
#include "defs.h"
-#include "observable.h"
#include "frame.h"
#include "inferior.h"
#include "gdbthread.h"
@@ -38,6 +37,9 @@
#include "nat/gdb_ptrace.h"
#include "nat/linux-ptrace.h"
#include "inf-ptrace.h"
+#include <algorithm>
+#include <unordered_map>
+#include <list>
/* Prototypes for supply_gregset etc. */
#include "gregset.h"
@@ -136,6 +138,10 @@ struct ppc_hw_breakpoint
#define PPC_DEBUG_FEATURE_DATA_BP_DAWR 0x10
#endif /* PPC_DEBUG_FEATURE_DATA_BP_DAWR */
+/* The version of the PowerPC HWDEBUG kernel interface that we will use, if
+ available. */
+#define PPC_DEBUG_CURRENT_VERSION 1
+
/* Similarly for the general-purpose (gp0 -- gp31)
and floating-point registers (fp0 -- fp31). */
#ifndef PTRACE_GETREGS
@@ -270,6 +276,214 @@ int have_ptrace_getsetregs = 1;
them and gotten an error. */
int have_ptrace_getsetfpregs = 1;
+/* Private arch info associated with each thread lwp_info object, used
+ for debug register handling. */
+
+struct arch_lwp_info
+{
+ /* When true, indicates that the debug registers installed in the
+ thread no longer correspond to the watchpoints and breakpoints
+ requested by GDB. */
+ bool debug_regs_stale;
+
+ /* We need a back-reference to the PTID of the thread so that we can
+ cleanup the debug register state of the thread in
+ low_delete_thread. */
+ ptid_t lwp_ptid;
+};
+
+/* Class used to detect which set of ptrace requests that
+ ppc_linux_nat_target will use to install and remove hardware
+ breakpoints and watchpoints.
+
+ The interface is only detected once, testing the ptrace calls. The
+ result can indicate that no interface is available.
+
+ The Linux kernel provides two different sets of ptrace requests to
+ handle hardware watchpoints and breakpoints for Power:
+
+ - PPC_PTRACE_GETHWDBGINFO, PPC_PTRACE_SETHWDEBUG, and
+ PPC_PTRACE_DELHWDEBUG.
+
+ Or
+
+ - PTRACE_SET_DEBUGREG and PTRACE_GET_DEBUGREG
+
+ The first set is the more flexible one and allows setting watchpoints
+ with a variable watched region length and, for BookE processors,
+ multiple types of debug registers (e.g. hardware breakpoints and
+ hardware-assisted conditions for watchpoints). The second one only
+ allows setting one debug register, a watchpoint, so we only use it if
+ the first one is not available. */
+
+class ppc_linux_dreg_interface
+{
+public:
+
+ ppc_linux_dreg_interface ()
+ : m_interface (), m_hwdebug_info ()
+ {
+ };
+
+ DISABLE_COPY_AND_ASSIGN (ppc_linux_dreg_interface);
+
+ /* One and only one of these three functions returns true, indicating
+ whether the corresponding interface is the one we detected. The
+ interface must already have been detected as a precontidion. */
+
+ bool hwdebug_p ()
+ {
+ gdb_assert (detected_p ());
+ return *m_interface == HWDEBUG;
+ }
+
+ bool debugreg_p ()
+ {
+ gdb_assert (detected_p ());
+ return *m_interface == DEBUGREG;
+ }
+
+ bool unavailable_p ()
+ {
+ gdb_assert (detected_p ());
+ return *m_interface == UNAVAILABLE;
+ }
+
+ /* Returns the debug register capabilities of the target. Should only
+ be called if the interface is HWDEBUG. */
+ const struct ppc_debug_info &hwdebug_info ()
+ {
+ gdb_assert (hwdebug_p ());
+
+ return m_hwdebug_info;
+ }
+
+ /* Returns true if the interface has already been detected. This is
+ useful for cases when we know there is no work to be done if the
+ interface hasn't been detected yet. */
+ bool detected_p ()
+ {
+ return m_interface.has_value ();
+ }
+
+ /* Detect the available interface, if any, if it hasn't been detected
+ before, using PTID for the necessary ptrace calls. */
+
+ void detect (const ptid_t &ptid)
+ {
+ if (m_interface.has_value ())
+ return;
+
+ gdb_assert (ptid.lwp_p ());
+
+ bool no_features = false;
+
+ if (ptrace (PPC_PTRACE_GETHWDBGINFO, ptid.lwp (), 0, &m_hwdebug_info)
+ != -1)
+ {
+ /* If there are no advertised features, we don't use the
+ HWDEBUG interface and try the DEBUGREG interface instead.
+ It shouldn't be necessary to do this, however, when the
+ kernel is configured without CONFIG_HW_BREAKPOINTS (selected
+ by CONFIG_PERF_EVENTS), there is a bug that causes
+ watchpoints installed with the HWDEBUG interface not to
+ trigger. When this is the case, features will be zero,
+ which we use as an indicator to fall back to the DEBUGREG
+ interface. */
+ if (m_hwdebug_info.features != 0)
+ {
+ m_interface.emplace (HWDEBUG);
+ return;
+ }
+ else
+ no_features = true;
+ }
+
+ /* EIO indicates that the request is invalid, so we try DEBUGREG
+ next. Technically, it can also indicate other failures, but we
+ can't differentiate those.
+
+ Other errors could happen for various reasons. We could get an
+ ESRCH if the traced thread was killed by a signal. Trying to
+ detect the interface with another thread in the future would be
+ complicated, as callers would have to handle an "unknown
+ interface" case. It's also unclear if raising an exception
+ here would be safe.
+
+ Other errors, such as ENODEV, could be more permanent and cause
+ a failure for any thread.
+
+ For simplicity, with all errors other than EIO, we set the
+ interface to UNAVAILABLE and don't try DEBUGREG. If DEBUGREG
+ fails too, we'll also set the interface to UNAVAILABLE. It's
+ unlikely that trying the DEBUGREG interface with this same thread
+ would work, for errors other than EIO. This means that these
+ errors will cause hardware watchpoints and breakpoints to become
+ unavailable throughout a GDB session. */
+
+ if (no_features || errno == EIO)
+ {
+ unsigned long wp;
+
+ if (ptrace (PTRACE_GET_DEBUGREG, ptid.lwp (), 0, &wp) != -1)
+ {
+ m_interface.emplace (DEBUGREG);
+ return;
+ }
+ }
+
+ if (errno != EIO)
+ warning (_("Error when detecting the debug register interface. "
+ "Debug registers will be unavailable."));
+
+ m_interface.emplace (UNAVAILABLE);
+ return;
+ }
+
+private:
+
+ /* HWDEBUG represents the set of calls PPC_PTRACE_GETHWDBGINFO,
+ PPC_PTRACE_SETHWDEBUG and PPC_PTRACE_DELHWDEBUG.
+
+ DEBUGREG represents the set of calls PTRACE_SET_DEBUGREG and
+ PTRACE_GET_DEBUGREG.
+
+ UNAVAILABLE can indicate that the kernel doesn't support any of the
+ two sets of requests or that there was an error when we tried to
+ detect wich interface is available. */
+
+ enum debug_reg_interface
+ {
+ UNAVAILABLE,
+ HWDEBUG,
+ DEBUGREG
+ };
+
+ /* The interface option. Initialized if has_value () returns true. */
+ gdb::optional<enum debug_reg_interface> m_interface;
+
+ /* The info returned by the kernel with PPC_PTRACE_GETHWDBGINFO. Only
+ valid if we determined that the interface is HWDEBUG. */
+ struct ppc_debug_info m_hwdebug_info;
+};
+
+/* Per-process information. This includes the hardware watchpoints and
+ breakpoints that GDB requested to this target. */
+
+struct ppc_linux_process_info
+{
+ /* The list of hardware watchpoints and breakpoints that GDB requested
+ for this process.
+
+ Only used when the interface is HWDEBUG. */
+ std::list<struct ppc_hw_breakpoint> requested_hw_bps;
+
+ /* The watchpoint value that GDB requested for this process.
+
+ Only used when the interface is DEBUGREG. */
+ gdb::optional<long> requested_wp_val;
+};
+
struct ppc_linux_nat_target final : public linux_nat_target
{
/* Add our register access methods. */
@@ -299,10 +513,6 @@ struct ppc_linux_nat_target final : public linux_nat_target
int remove_mask_watchpoint (CORE_ADDR, CORE_ADDR, enum target_hw_bp_type)
override;
- bool stopped_by_watchpoint () override;
-
- bool stopped_data_address (CORE_ADDR *) override;
-
bool watchpoint_addr_within_range (CORE_ADDR, CORE_ADDR, int) override;
bool can_accel_watchpoint_condition (CORE_ADDR, int, int, struct expression *)
@@ -319,7 +529,95 @@ struct ppc_linux_nat_target final : public linux_nat_target
override;
/* Override linux_nat_target low methods. */
+ bool low_stopped_by_watchpoint () override;
+
+ bool low_stopped_data_address (CORE_ADDR *) override;
+
void low_new_thread (struct lwp_info *lp) override;
+
+ void low_delete_thread (arch_lwp_info *) override;
+
+ void low_new_fork (struct lwp_info *, pid_t) override;
+
+ void low_new_clone (struct lwp_info *, pid_t) override;
+
+ void low_forget_process (pid_t pid) override;
+
+ void low_prepare_to_resume (struct lwp_info *) override;
+
+private:
+
+ void copy_thread_dreg_state (const ptid_t &parent_ptid,
+ const ptid_t &child_ptid);
+
+ void mark_thread_stale (struct lwp_info *lp);
+
+ void mark_debug_registers_changed (pid_t pid);
+
+ void register_hw_breakpoint (pid_t pid,
+ const struct ppc_hw_breakpoint &bp);
+
+ void clear_hw_breakpoint (pid_t pid,
+ const struct ppc_hw_breakpoint &a);
+
+ void register_wp (pid_t pid, long wp_value);
+
+ void clear_wp (pid_t pid);
+
+ bool can_use_watchpoint_cond_accel (void);
+
+ void calculate_dvc (CORE_ADDR addr, int len,
+ CORE_ADDR data_value,
+ uint32_t *condition_mode,
+ uint64_t *condition_value);
+
+ int check_condition (CORE_ADDR watch_addr,
+ struct expression *cond,
+ CORE_ADDR *data_value, int *len);
+
+ int num_memory_accesses (const std::vector<value_ref_ptr> &chain);
+
+ int get_trigger_type (enum target_hw_bp_type type);
+
+ void create_watchpoint_request (struct ppc_hw_breakpoint *p,
+ CORE_ADDR addr,
+ int len,
+ enum target_hw_bp_type type,
+ struct expression *cond,
+ int insert);
+
+ bool hwdebug_point_cmp (const struct ppc_hw_breakpoint &a,
+ const struct ppc_hw_breakpoint &b);
+
+ void init_arch_lwp_info (struct lwp_info *lp);
+
+ arch_lwp_info *get_arch_lwp_info (struct lwp_info *lp);
+
+ /* The ptrace interface we'll use to install hardware watchpoints and
+ breakpoints (debug registers). */
+ ppc_linux_dreg_interface m_dreg_interface;
+
+ /* A map from pids to structs containing info specific to each
+ process. */
+ std::unordered_map<pid_t, ppc_linux_process_info> m_process_info;
+
+ /* Callable object to hash ptids by their lwp number. */
+ struct ptid_hash
+ {
+ std::size_t operator() (const ptid_t &ptid) const
+ {
+ return std::hash<long>{} (ptid.lwp ());
+ }
+ };
+
+ /* A map from ptid_t objects to a list of pairs of slots and hardware
+ breakpoint objects. This keeps track of which hardware breakpoints
+ and watchpoints were last installed in each slot of each thread.
+
+ Only used when the interface is HWDEBUG. */
+ std::unordered_map <ptid_t,
+ std::list<std::pair<long, ppc_hw_breakpoint>>,
+ ptid_hash> m_installed_hw_bps;
};
static ppc_linux_nat_target the_ppc_linux_nat_target;
@@ -1719,101 +2017,50 @@ ppc_linux_nat_target::read_description ()
return ppc_linux_match_description (features);
}
-/* The cached DABR value, to install in new threads.
- This variable is used when the PowerPC HWDEBUG ptrace
- interface is not available. */
-static long saved_dabr_value;
-
-/* Global structure that will store information about the available
- features provided by the PowerPC HWDEBUG ptrace interface. */
-static struct ppc_debug_info hwdebug_info;
-
-/* Global variable that holds the maximum number of slots that the
- kernel will use. This is only used when PowerPC HWDEBUG ptrace interface
- is available. */
-static size_t max_slots_number = 0;
-
-struct hw_break_tuple
-{
- long slot;
- struct ppc_hw_breakpoint *hw_break;
-};
-
-/* This is an internal vector created to store information about *points
- inserted for each thread. This is used when PowerPC HWDEBUG ptrace
- interface is available. */
-struct thread_points
- {
- /* The TID to which this *point relates. */
- int tid;
- /* Information about the *point, such as its address, type, etc.
-
- Each element inside this vector corresponds to a hardware
- breakpoint or watchpoint in the thread represented by TID. The maximum
- size of these vector is MAX_SLOTS_NUMBER. If the hw_break element of
- the tuple is NULL, then the position in the vector is free. */
- struct hw_break_tuple *hw_breaks;
- };
-
-static std::vector<thread_points *> ppc_threads;
-
-/* The version of the PowerPC HWDEBUG kernel interface that we will use, if
- available. */
-#define PPC_DEBUG_CURRENT_VERSION 1
-
-/* Returns non-zero if we support the PowerPC HWDEBUG ptrace interface. */
-static int
-have_ptrace_hwdebug_interface (void)
-{
- static int have_ptrace_hwdebug_interface = -1;
-
- if (have_ptrace_hwdebug_interface == -1)
- {
- int tid;
-
- tid = inferior_ptid.lwp ();
- if (tid == 0)
- tid = inferior_ptid.pid ();
-
- /* Check for kernel support for PowerPC HWDEBUG ptrace interface. */
- if (ptrace (PPC_PTRACE_GETHWDBGINFO, tid, 0, &hwdebug_info) >= 0)
- {
- /* Check whether PowerPC HWDEBUG ptrace interface is functional and
- provides any supported feature. */
- if (hwdebug_info.features != 0)
- {
- have_ptrace_hwdebug_interface = 1;
- max_slots_number = hwdebug_info.num_instruction_bps
- + hwdebug_info.num_data_bps
- + hwdebug_info.num_condition_regs;
- return have_ptrace_hwdebug_interface;
- }
- }
- /* Old school interface and no PowerPC HWDEBUG ptrace support. */
- have_ptrace_hwdebug_interface = 0;
- memset (&hwdebug_info, 0, sizeof (struct ppc_debug_info));
- }
-
- return have_ptrace_hwdebug_interface;
-}
+/* Routines for installing hardware watchpoints and breakpoints. When
+ GDB requests a hardware watchpoint or breakpoint to be installed, we
+ register the request for the pid of inferior_ptid in a map with one
+ entry per process. We then issue a stop request to all the threads of
+ this process, and mark a per-thread flag indicating that their debug
+ registers should be updated. Right before they are next resumed, we
+ remove all previously installed debug registers and install all the
+ ones GDB requested. We then update a map with one entry per thread
+ that keeps track of what debug registers were last installed in each
+ thread.
+
+ We use this second map to remove installed registers before installing
+ the ones requested by GDB, and to copy the debug register state after
+ a thread clones or forks, since depending on the kernel configuration,
+ debug registers can be inherited. */
+
+/* Check if we support and have enough resources to install a hardware
+ watchpoint or breakpoint. See the description in target.h. */
int
-ppc_linux_nat_target::can_use_hw_breakpoint (enum bptype type, int cnt, int ot)
+ppc_linux_nat_target::can_use_hw_breakpoint (enum bptype type, int cnt,
+ int ot)
{
int total_hw_wp, total_hw_bp;
- if (have_ptrace_hwdebug_interface ())
+ m_dreg_interface.detect (inferior_ptid);
+
+ if (m_dreg_interface.unavailable_p ())
+ return 0;
+
+ if (m_dreg_interface.hwdebug_p ())
{
/* When PowerPC HWDEBUG ptrace interface is available, the number of
available hardware watchpoints and breakpoints is stored at the
hwdebug_info struct. */
- total_hw_bp = hwdebug_info.num_instruction_bps;
- total_hw_wp = hwdebug_info.num_data_bps;
+ total_hw_bp = m_dreg_interface.hwdebug_info ().num_instruction_bps;
+ total_hw_wp = m_dreg_interface.hwdebug_info ().num_data_bps;
}
else
{
- /* When we do not have PowerPC HWDEBUG ptrace interface, we should
- consider having 1 hardware watchpoint and no hardware breakpoints. */
+ gdb_assert (m_dreg_interface.debugreg_p ());
+
+ /* With the DEBUGREG ptrace interface, we should consider having 1
+ hardware watchpoint and no hardware breakpoints. */
total_hw_bp = 0;
total_hw_wp = 1;
}
@@ -1821,39 +2068,28 @@ ppc_linux_nat_target::can_use_hw_breakpoint (enum bptype type, int cnt, int ot)
if (type == bp_hardware_watchpoint || type == bp_read_watchpoint
|| type == bp_access_watchpoint || type == bp_watchpoint)
{
- if (cnt + ot > total_hw_wp)
+ if (total_hw_wp == 0)
+ return 0;
+ else if (cnt + ot > total_hw_wp)
return -1;
+ else
+ return 1;
}
else if (type == bp_hardware_breakpoint)
{
if (total_hw_bp == 0)
- {
- /* No hardware breakpoint support. */
- return 0;
- }
- if (cnt > total_hw_bp)
- return -1;
- }
-
- if (!have_ptrace_hwdebug_interface ())
- {
- int tid;
- ptid_t ptid = inferior_ptid;
-
- /* We need to know whether ptrace supports PTRACE_SET_DEBUGREG
- and whether the target has DABR. If either answer is no, the
- ptrace call will return -1. Fail in that case. */
- tid = ptid.lwp ();
- if (tid == 0)
- tid = ptid.pid ();
-
- if (ptrace (PTRACE_SET_DEBUGREG, tid, 0, 0) == -1)
return 0;
+ else if (cnt > total_hw_bp)
+ return -1;
+ else
+ return 1;
}
- return 1;
+ return 0;
}
+/* Returns 1 if we can watch LEN bytes at address ADDR, 0 otherwise. */
+
int
ppc_linux_nat_target::region_ok_for_hw_watchpoint (CORE_ADDR addr, int len)
{
@@ -1861,13 +2097,21 @@ ppc_linux_nat_target::region_ok_for_hw_watchpoint (CORE_ADDR addr, int len)
if (len <= 0)
return 0;
+ m_dreg_interface.detect (inferior_ptid);
+
+ if (m_dreg_interface.unavailable_p ())
+ return 0;
+
/* The PowerPC HWDEBUG ptrace interface tells if there are alignment
restrictions for watchpoints in the processors. In that case, we use that
information to determine the hardcoded watchable region for
watchpoints. */
- if (have_ptrace_hwdebug_interface ())
+ if (m_dreg_interface.hwdebug_p ())
{
int region_size;
+ const struct ppc_debug_info &hwdebug_info = (m_dreg_interface
+ .hwdebug_info ());
+
/* Embedded DAC-based processors, like the PowerPC 440 have ranged
watchpoints and can watch any access within an arbitrary memory
region. This is useful to watch arrays and structs, for instance. It
@@ -1894,121 +2138,32 @@ ppc_linux_nat_target::region_ok_for_hw_watchpoint (CORE_ADDR addr, int len)
ptrace interface, DAC-based processors (i.e., embedded processors) will
use addresses aligned to 4-bytes due to the way the read/write flags are
passed in the old ptrace interface. */
- else if (((linux_get_hwcap (current_top_target ()) & PPC_FEATURE_BOOKE)
- && (addr + len) > (addr & ~3) + 4)
- || (addr + len) > (addr & ~7) + 8)
- return 0;
-
- return 1;
-}
-
-/* This function compares two ppc_hw_breakpoint structs field-by-field. */
-static int
-hwdebug_point_cmp (struct ppc_hw_breakpoint *a, struct ppc_hw_breakpoint *b)
-{
- return (a->trigger_type == b->trigger_type
- && a->addr_mode == b->addr_mode
- && a->condition_mode == b->condition_mode
- && a->addr == b->addr
- && a->addr2 == b->addr2
- && a->condition_value == b->condition_value);
-}
-
-/* This function can be used to retrieve a thread_points by the TID of the
- related process/thread. If nothing has been found, and ALLOC_NEW is 0,
- it returns NULL. If ALLOC_NEW is non-zero, a new thread_points for the
- provided TID will be created and returned. */
-static struct thread_points *
-hwdebug_find_thread_points_by_tid (int tid, int alloc_new)
-{
- for (thread_points *t : ppc_threads)
- {
- if (t->tid == tid)
- return t;
- }
-
- struct thread_points *t = NULL;
-
- /* Do we need to allocate a new point_item
- if the wanted one does not exist? */
- if (alloc_new)
+ else
{
- t = XNEW (struct thread_points);
- t->hw_breaks = XCNEWVEC (struct hw_break_tuple, max_slots_number);
- t->tid = tid;
- ppc_threads.push_back (t);
- }
-
- return t;
-}
-
-/* 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
-hwdebug_insert_point (struct ppc_hw_breakpoint *b, int tid)
-{
- int i;
- long slot;
- gdb::unique_xmalloc_ptr<ppc_hw_breakpoint> p (XDUP (ppc_hw_breakpoint, b));
- struct hw_break_tuple *hw_breaks;
- struct thread_points *t;
+ gdb_assert (m_dreg_interface.debugreg_p ());
- errno = 0;
- slot = ptrace (PPC_PTRACE_SETHWDEBUG, tid, 0, p.get ());
- if (slot < 0)
- perror_with_name (_("Unexpected error setting breakpoint or watchpoint"));
-
- /* Everything went fine, so we have to register this *point. */
- t = hwdebug_find_thread_points_by_tid (tid, 1);
- gdb_assert (t != NULL);
- hw_breaks = t->hw_breaks;
-
- /* Find a free element in the hw_breaks vector. */
- for (i = 0; i < max_slots_number; i++)
- {
- if (hw_breaks[i].hw_break == NULL)
- {
- hw_breaks[i].slot = slot;
- hw_breaks[i].hw_break = p.release ();
- break;
- }
+ if (((linux_get_hwcap (current_top_target ()) & PPC_FEATURE_BOOKE)
+ && (addr + len) > (addr & ~3) + 4)
+ || (addr + len) > (addr & ~7) + 8)
+ return 0;
}
- gdb_assert (i != max_slots_number);
+ return 1;
}
-/* 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
-hwdebug_remove_point (struct ppc_hw_breakpoint *b, int tid)
-{
- int i;
- struct hw_break_tuple *hw_breaks;
- struct thread_points *t;
-
- t = hwdebug_find_thread_points_by_tid (tid, 0);
- gdb_assert (t != NULL);
- hw_breaks = t->hw_breaks;
+/* This function compares two ppc_hw_breakpoint structs
+ field-by-field. */
- for (i = 0; i < max_slots_number; i++)
- if (hw_breaks[i].hw_break && hwdebug_point_cmp (hw_breaks[i].hw_break, b))
- break;
-
- gdb_assert (i != max_slots_number);
-
- /* We have to ignore ENOENT errors because the kernel implements hardware
- breakpoints/watchpoints as "one-shot", that is, they are automatically
- deleted when hit. */
- 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"));
-
- xfree (hw_breaks[i].hw_break);
- hw_breaks[i].hw_break = NULL;
+bool
+ppc_linux_nat_target::hwdebug_point_cmp (const struct ppc_hw_breakpoint &a,
+ const struct ppc_hw_breakpoint &b)
+{
+ return (a.trigger_type == b.trigger_type
+ && a.addr_mode == b.addr_mode
+ && a.condition_mode == b.condition_mode
+ && a.addr == b.addr
+ && a.addr2 == b.addr2
+ && a.condition_value == b.condition_value);
}
/* Return the number of registers needed for a ranged breakpoint. */
@@ -2016,22 +2171,28 @@ hwdebug_remove_point (struct ppc_hw_breakpoint *b, int tid)
int
ppc_linux_nat_target::ranged_break_num_registers ()
{
- return ((have_ptrace_hwdebug_interface ()
- && hwdebug_info.features & PPC_DEBUG_FEATURE_INSN_BP_RANGE)?
+ m_dreg_interface.detect (inferior_ptid);
+
+ return ((m_dreg_interface.hwdebug_p ()
+ && (m_dreg_interface.hwdebug_info ().features
+ & PPC_DEBUG_FEATURE_INSN_BP_RANGE))?
2 : -1);
}
-/* Insert the hardware breakpoint described by BP_TGT. Returns 0 for
- success, 1 if hardware breakpoints are not supported or -1 for failure. */
+/* Register the hardware breakpoint described by BP_TGT, to be inserted
+ when the threads of inferior_ptid are resumed. Returns 0 for success,
+ or -1 if the HWDEBUG interface that we need for hardware breakpoints
+ is not available. */
int
ppc_linux_nat_target::insert_hw_breakpoint (struct gdbarch *gdbarch,
struct bp_target_info *bp_tgt)
{
- struct lwp_info *lp;
struct ppc_hw_breakpoint p;
- if (!have_ptrace_hwdebug_interface ())
+ m_dreg_interface.detect (inferior_ptid);
+
+ if (!m_dreg_interface.hwdebug_p ())
return -1;
p.version = PPC_DEBUG_CURRENT_VERSION;
@@ -2054,20 +2215,25 @@ ppc_linux_nat_target::insert_hw_breakpoint (struct gdbarch *gdbarch,
p.addr2 = 0;
}
- ALL_LWPS (lp)
- hwdebug_insert_point (&p, lp->ptid.lwp ());
+ register_hw_breakpoint (inferior_ptid.pid (), p);
return 0;
}
+/* Clear a registration for the hardware breakpoint given by type BP_TGT.
+ It will be removed from the threads of inferior_ptid when they are
+ next resumed. Returns 0 for success, or -1 if the HWDEBUG interface
+ that we need for hardware breakpoints is not available. */
+
int
ppc_linux_nat_target::remove_hw_breakpoint (struct gdbarch *gdbarch,
struct bp_target_info *bp_tgt)
{
- struct lwp_info *lp;
struct ppc_hw_breakpoint p;
- if (!have_ptrace_hwdebug_interface ())
+ m_dreg_interface.detect (inferior_ptid);
+
+ if (!m_dreg_interface.hwdebug_p ())
return -1;
p.version = PPC_DEBUG_CURRENT_VERSION;
@@ -2090,14 +2256,16 @@ ppc_linux_nat_target::remove_hw_breakpoint (struct gdbarch *gdbarch,
p.addr2 = 0;
}
- ALL_LWPS (lp)
- hwdebug_remove_point (&p, lp->ptid.lwp ());
+ clear_hw_breakpoint (inferior_ptid.pid (), p);
return 0;
}
-static int
-get_trigger_type (enum target_hw_bp_type type)
+/* Return the trigger value to set in a ppc_hw_breakpoint object for a
+ given hardware watchpoint TYPE. We assume type is not hw_execute. */
+
+int
+ppc_linux_nat_target::get_trigger_type (enum target_hw_bp_type type)
{
int t;
@@ -2111,19 +2279,18 @@ get_trigger_type (enum target_hw_bp_type type)
return t;
}
-/* Insert a new masked watchpoint at ADDR using the mask MASK.
- RW may be hw_read for a read watchpoint, hw_write for a write watchpoint
- or hw_access for an access watchpoint. Returns 0 on success and throws
- an error on failure. */
+/* Register a new masked watchpoint at ADDR using the mask MASK, to be
+ inserted when the threads of inferior_ptid are resumed. RW may be
+ hw_read for a read watchpoint, hw_write for a write watchpoint or
+ hw_access for an access watchpoint. */
int
ppc_linux_nat_target::insert_mask_watchpoint (CORE_ADDR addr, CORE_ADDR mask,
target_hw_bp_type rw)
{
- struct lwp_info *lp;
struct ppc_hw_breakpoint p;
- gdb_assert (have_ptrace_hwdebug_interface ());
+ gdb_assert (m_dreg_interface.hwdebug_p ());
p.version = PPC_DEBUG_CURRENT_VERSION;
p.trigger_type = get_trigger_type (rw);
@@ -2133,25 +2300,23 @@ ppc_linux_nat_target::insert_mask_watchpoint (CORE_ADDR addr, CORE_ADDR mask,
p.addr2 = mask;
p.condition_value = 0;
- ALL_LWPS (lp)
- hwdebug_insert_point (&p, lp->ptid.lwp ());
+ register_hw_breakpoint (inferior_ptid.pid (), p);
return 0;
}
-/* Remove a masked watchpoint at ADDR with the mask MASK.
- RW may be hw_read for a read watchpoint, hw_write for a write watchpoint
- or hw_access for an access watchpoint. Returns 0 on success and throws
- an error on failure. */
+/* Clear a registration for a masked watchpoint at ADDR with the mask
+ MASK. It will be removed from the threads of inferior_ptid when they
+ are next resumed. RW may be hw_read for a read watchpoint, hw_write
+ for a write watchpoint or hw_access for an access watchpoint. */
int
ppc_linux_nat_target::remove_mask_watchpoint (CORE_ADDR addr, CORE_ADDR mask,
target_hw_bp_type rw)
{
- struct lwp_info *lp;
struct ppc_hw_breakpoint p;
- gdb_assert (have_ptrace_hwdebug_interface ());
+ gdb_assert (m_dreg_interface.hwdebug_p ());
p.version = PPC_DEBUG_CURRENT_VERSION;
p.trigger_type = get_trigger_type (rw);
@@ -2161,39 +2326,42 @@ ppc_linux_nat_target::remove_mask_watchpoint (CORE_ADDR addr, CORE_ADDR mask,
p.addr2 = mask;
p.condition_value = 0;
- ALL_LWPS (lp)
- hwdebug_remove_point (&p, lp->ptid.lwp ());
+ clear_hw_breakpoint (inferior_ptid.pid (), p);
return 0;
}
-/* Check whether we have at least one free DVC register. */
-static int
-can_use_watchpoint_cond_accel (void)
+/* Check whether we have at least one free DVC register for the threads
+ of the pid of inferior_ptid. */
+
+bool
+ppc_linux_nat_target::can_use_watchpoint_cond_accel (void)
{
- struct thread_points *p;
- int tid = inferior_ptid.lwp ();
- int cnt = hwdebug_info.num_condition_regs, i;
+ m_dreg_interface.detect (inferior_ptid);
- if (!have_ptrace_hwdebug_interface () || cnt == 0)
- return 0;
+ if (!m_dreg_interface.hwdebug_p ())
+ return false;
- p = hwdebug_find_thread_points_by_tid (tid, 0);
+ int cnt = m_dreg_interface.hwdebug_info ().num_condition_regs;
- 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--;
+ if (cnt == 0)
+ return false;
- /* There are no available slots now. */
- if (cnt <= 0)
- return 0;
- }
+ auto process_it = m_process_info.find (inferior_ptid.pid ());
- return 1;
+ /* No breakpoints or watchpoints have been requested for this process,
+ we have at least one free DVC register. */
+ if (process_it == m_process_info.end ())
+ return true;
+
+ for (const ppc_hw_breakpoint &bp : process_it->second.requested_hw_bps)
+ if (bp.condition_mode != PPC_BREAKPOINT_CONDITION_NONE)
+ cnt--;
+
+ if (cnt <= 0)
+ return false;
+
+ return true;
}
/* Calculate the enable bits and the contents of the Data Value Compare
@@ -2204,10 +2372,16 @@ can_use_watchpoint_cond_accel (void)
On exit, CONDITION_MODE will hold the enable bits for the DVC, and
CONDITION_VALUE will hold the value which should be put in the
DVC register. */
-static void
-calculate_dvc (CORE_ADDR addr, int len, CORE_ADDR data_value,
- uint32_t *condition_mode, uint64_t *condition_value)
+
+void
+ppc_linux_nat_target::calculate_dvc (CORE_ADDR addr, int len,
+ CORE_ADDR data_value,
+ uint32_t *condition_mode,
+ uint64_t *condition_value)
{
+ const struct ppc_debug_info &hwdebug_info = (m_dreg_interface.
+ hwdebug_info ());
+
int i, num_byte_enable, align_offset, num_bytes_off_dvc,
rightmost_enabled_byte;
CORE_ADDR addr_end_data, addr_end_dvc;
@@ -2246,8 +2420,10 @@ calculate_dvc (CORE_ADDR addr, int len, CORE_ADDR data_value,
Returns -1 if there's any register access involved, or if there are
other kinds of values which are not acceptable in a condition
expression (e.g., lval_computed or lval_internalvar). */
-static int
-num_memory_accesses (const std::vector<value_ref_ptr> &chain)
+
+int
+ppc_linux_nat_target::num_memory_accesses (const std::vector<value_ref_ptr>
+ &chain)
{
int found_memory_cnt = 0;
@@ -2295,9 +2471,11 @@ num_memory_accesses (const std::vector<value_ref_ptr> &chain)
If the function returns 1, DATA_VALUE will contain the constant against
which the watch value should be compared and LEN will contain the size
of the constant. */
-static int
-check_condition (CORE_ADDR watch_addr, struct expression *cond,
- CORE_ADDR *data_value, int *len)
+
+int
+ppc_linux_nat_target::check_condition (CORE_ADDR watch_addr,
+ struct expression *cond,
+ CORE_ADDR *data_value, int *len)
{
int pc = 1, num_accesses_left, num_accesses_right;
struct value *left_val, *right_val;
@@ -2344,18 +2522,21 @@ check_condition (CORE_ADDR watch_addr, struct expression *cond,
return 1;
}
-/* Return non-zero if the target is capable of using hardware to evaluate
- the condition expression, thus only triggering the watchpoint when it is
+/* Return true if the target is capable of using hardware to evaluate the
+ condition expression, thus only triggering the watchpoint when it is
true. */
+
bool
-ppc_linux_nat_target::can_accel_watchpoint_condition (CORE_ADDR addr, int len,
- int rw,
+ppc_linux_nat_target::can_accel_watchpoint_condition (CORE_ADDR addr,
+ int len, int rw,
struct expression *cond)
{
CORE_ADDR data_value;
- return (have_ptrace_hwdebug_interface ()
- && hwdebug_info.num_condition_regs > 0
+ m_dreg_interface.detect (inferior_ptid);
+
+ return (m_dreg_interface.hwdebug_p ()
+ && (m_dreg_interface.hwdebug_info ().num_condition_regs > 0)
&& check_condition (addr, cond, &data_value, &len));
}
@@ -2364,11 +2545,16 @@ ppc_linux_nat_target::can_accel_watchpoint_condition (CORE_ADDR addr, int len,
evaluated by hardware. INSERT tells if we are creating a request for
inserting or removing the watchpoint. */
-static void
-create_watchpoint_request (struct ppc_hw_breakpoint *p, CORE_ADDR addr,
- int len, enum target_hw_bp_type type,
- struct expression *cond, int insert)
+void
+ppc_linux_nat_target::create_watchpoint_request (struct ppc_hw_breakpoint *p,
+ CORE_ADDR addr, int len,
+ enum target_hw_bp_type type,
+ struct expression *cond,
+ int insert)
{
+ const struct ppc_debug_info &hwdebug_info = (m_dreg_interface
+ .hwdebug_info ());
+
if (len == 1
|| !(hwdebug_info.features & PPC_DEBUG_FEATURE_DATA_BP_RANGE))
{
@@ -2410,28 +2596,33 @@ create_watchpoint_request (struct ppc_hw_breakpoint *p, CORE_ADDR addr,
p->addr = (uint64_t) addr;
}
+/* Register a watchpoint, to be inserted when the threads of the group of
+ inferior_ptid are next resumed. Returns 0 on success, and -1 if there
+ is no ptrace interface available to install the watchpoint. */
+
int
ppc_linux_nat_target::insert_watchpoint (CORE_ADDR addr, int len,
enum target_hw_bp_type type,
struct expression *cond)
{
- struct lwp_info *lp;
- int ret = -1;
+ m_dreg_interface.detect (inferior_ptid);
+
+ if (m_dreg_interface.unavailable_p ())
+ return -1;
- if (have_ptrace_hwdebug_interface ())
+ if (m_dreg_interface.hwdebug_p ())
{
struct ppc_hw_breakpoint p;
create_watchpoint_request (&p, addr, len, type, cond, 1);
- ALL_LWPS (lp)
- hwdebug_insert_point (&p, lp->ptid.lwp ());
-
- ret = 0;
+ register_hw_breakpoint (inferior_ptid.pid (), p);
}
else
{
- long dabr_value;
+ gdb_assert (m_dreg_interface.debugreg_p ());
+
+ long wp_value;
long read_mode, write_mode;
if (linux_get_hwcap (current_top_target ()) & PPC_FEATURE_BOOKE)
@@ -2449,144 +2640,300 @@ ppc_linux_nat_target::insert_watchpoint (CORE_ADDR addr, int len,
write_mode = 6;
}
- dabr_value = addr & ~(read_mode | write_mode);
+ wp_value = addr & ~(read_mode | write_mode);
switch (type)
{
case hw_read:
/* Set read and translate bits. */
- dabr_value |= read_mode;
+ wp_value |= read_mode;
break;
case hw_write:
/* Set write and translate bits. */
- dabr_value |= write_mode;
+ wp_value |= write_mode;
break;
case hw_access:
/* Set read, write and translate bits. */
- dabr_value |= read_mode | write_mode;
+ wp_value |= read_mode | write_mode;
break;
}
- saved_dabr_value = dabr_value;
-
- ALL_LWPS (lp)
- if (ptrace (PTRACE_SET_DEBUGREG, lp->ptid.lwp (), 0,
- saved_dabr_value) < 0)
- return -1;
-
- ret = 0;
+ register_wp (inferior_ptid.pid (), wp_value);
}
- return ret;
+ return 0;
}
+/* Clear a registration for a hardware watchpoint. It will be removed
+ from the threads of the group of inferior_ptid when they are next
+ resumed. */
+
int
ppc_linux_nat_target::remove_watchpoint (CORE_ADDR addr, int len,
enum target_hw_bp_type type,
struct expression *cond)
{
- struct lwp_info *lp;
- int ret = -1;
+ gdb_assert (!m_dreg_interface.unavailable_p ());
- if (have_ptrace_hwdebug_interface ())
+ if (m_dreg_interface.hwdebug_p ())
{
struct ppc_hw_breakpoint p;
create_watchpoint_request (&p, addr, len, type, cond, 0);
- ALL_LWPS (lp)
- hwdebug_remove_point (&p, lp->ptid.lwp ());
-
- ret = 0;
+ clear_hw_breakpoint (inferior_ptid.pid (), p);
}
else
{
- saved_dabr_value = 0;
- ALL_LWPS (lp)
- if (ptrace (PTRACE_SET_DEBUGREG, lp->ptid.lwp (), 0,
- saved_dabr_value) < 0)
- return -1;
+ gdb_assert (m_dreg_interface.debugreg_p ());
- ret = 0;
+ clear_wp (inferior_ptid.pid ());
}
- return ret;
+ return 0;
}
+/* Clean up the per-process info associated with PID. When using the
+ HWDEBUG interface, we also erase the per-thread state of installed
+ debug registers for all the threads that belong to the group of PID.
+
+ Usually the thread state is cleaned up by low_delete_thread. We also
+ do it here because low_new_thread is not called for the initial LWP,
+ so low_delete_thread won't be able to clean up this state. */
+
void
-ppc_linux_nat_target::low_new_thread (struct lwp_info *lp)
+ppc_linux_nat_target::low_forget_process (pid_t pid)
{
- int tid = lp->ptid.lwp ();
+ if ((!m_dreg_interface.detected_p ())
+ || (m_dreg_interface.unavailable_p ()))
+ return;
- if (have_ptrace_hwdebug_interface ())
+ ptid_t pid_ptid (pid, 0, 0);
+
+ m_process_info.erase (pid);
+
+ if (m_dreg_interface.hwdebug_p ())
{
- int i;
- struct thread_points *p;
- struct hw_break_tuple *hw_breaks;
+ for (auto it = m_installed_hw_bps.begin ();
+ it != m_installed_hw_bps.end ();)
+ {
+ if (it->first.matches (pid_ptid))
+ it = m_installed_hw_bps.erase (it);
+ else
+ it++;
+ }
+ }
+}
- if (ppc_threads.empty ())
- return;
+/* Copy the per-process state associated with the pid of PARENT to the
+ sate of CHILD_PID. GDB expects that a forked process will have the
+ same hardware breakpoints and watchpoints as the parent.
- /* Get a list of breakpoints from any thread. */
- p = ppc_threads.back ();
- hw_breaks = p->hw_breaks;
+ If we're using the HWDEBUG interface, also copy the thread debug
+ register state for the ptid of PARENT to the state for CHILD_PID.
- /* Copy that thread's breakpoints and watchpoints to the new thread. */
- for (i = 0; i < max_slots_number; i++)
- if (hw_breaks[i].hw_break)
- {
- /* Older kernels did not make new threads inherit their parent
- thread's debug state, so we always clear the slot and replicate
- the debug state ourselves, ensuring compatibility with all
- kernels. */
+ Like for clone events, we assume the kernel will copy the debug
+ registers from the parent thread to the child. The
+ low_prepare_to_resume function is made to work even if it doesn't.
- /* The ppc debug resource accounting is done through "slots".
- Ask the kernel the deallocate this specific *point's slot. */
- ptrace (PPC_PTRACE_DELHWDEBUG, tid, 0, hw_breaks[i].slot);
+ We copy the thread state here and not in low_new_thread since we don't
+ have the pid of the parent in low_new_thread. Even if we did,
+ low_new_thread might not be called immediately after the fork event is
+ detected. For instance, with the checkpointing system (see
+ linux-fork.c), the thread won't be added until GDB decides to switch
+ to a new checkpointed process. At that point, the debug register
+ state of the parent thread is unlikely to correspond to the state it
+ had at the point when it forked. */
- hwdebug_insert_point (hw_breaks[i].hw_break, tid);
- }
+void
+ppc_linux_nat_target::low_new_fork (struct lwp_info *parent,
+ pid_t child_pid)
+{
+ if ((!m_dreg_interface.detected_p ())
+ || (m_dreg_interface.unavailable_p ()))
+ return;
+
+ auto process_it = m_process_info.find (parent->ptid.pid ());
+
+ if (process_it != m_process_info.end ())
+ m_process_info[child_pid] = m_process_info[parent->ptid.pid ()];
+
+ if (m_dreg_interface.hwdebug_p ())
+ {
+ ptid_t child_ptid (child_pid, child_pid, 0);
+
+ copy_thread_dreg_state (parent->ptid, child_ptid);
}
- else
- ptrace (PTRACE_SET_DEBUGREG, tid, 0, saved_dabr_value);
}
-static void
-ppc_linux_thread_exit (struct thread_info *tp, int silent)
+/* Copy the thread debug register state from the PARENT thread to the the
+ state for CHILD_LWP, if we're using the HWDEBUG interface. We assume
+ the kernel copies the debug registers from one thread to another after
+ a clone event. The low_prepare_to_resume function is made to work
+ even if it doesn't. */
+
+void
+ppc_linux_nat_target::low_new_clone (struct lwp_info *parent,
+ pid_t child_lwp)
{
- int i;
- int tid = tp->ptid.lwp ();
- struct hw_break_tuple *hw_breaks;
- struct thread_points *t = NULL;
+ if ((!m_dreg_interface.detected_p ())
+ || (m_dreg_interface.unavailable_p ()))
+ return;
+
+ if (m_dreg_interface.hwdebug_p ())
+ {
+ ptid_t child_ptid (parent->ptid.pid (), child_lwp, 0);
+
+ copy_thread_dreg_state (parent->ptid, child_ptid);
+ }
+}
+
+/* Initialize the arch-specific thread state for LP so that it contains
+ the ptid for lp, so that we can use it in low_delete_thread. Mark the
+ new thread LP as stale so that we update its debug registers before
+ resuming it. This is not called for the initial thread. */
+
+void
+ppc_linux_nat_target::low_new_thread (struct lwp_info *lp)
+{
+ init_arch_lwp_info (lp);
+
+ mark_thread_stale (lp);
+}
+
+/* Delete the per-thread debug register stale flag. */
+
+void
+ppc_linux_nat_target::low_delete_thread (struct arch_lwp_info
+ *lp_arch_info)
+{
+ if (lp_arch_info != NULL)
+ {
+ if (m_dreg_interface.detected_p ()
+ && m_dreg_interface.hwdebug_p ())
+ m_installed_hw_bps.erase (lp_arch_info->lwp_ptid);
+
+ xfree (lp_arch_info);
+ }
+}
+
+/* Install or delete debug registers in thread LP so that it matches what
+ GDB requested before it is resumed. */
+
+void
+ppc_linux_nat_target::low_prepare_to_resume (struct lwp_info *lp)
+{
+ if ((!m_dreg_interface.detected_p ())
+ || (m_dreg_interface.unavailable_p ()))
+ return;
+
+ /* We have to re-install or clear the debug registers if we set the
+ stale flag.
+
+ In addition, some kernels configurations can disable a hardware
+ watchpoint after it is hit. Usually, GDB will remove and re-install
+ a hardware watchpoint when the thread stops if "breakpoint
+ always-inserted" is off, or to single-step a watchpoint. But so
+ that we don't rely on this behavior, if we stop due to a hardware
+ breakpoint or watchpoint, we also refresh our debug registers. */
+
+ arch_lwp_info *lp_arch_info = get_arch_lwp_info (lp);
- if (!have_ptrace_hwdebug_interface ())
+ bool stale_dregs = (lp->stop_reason == TARGET_STOPPED_BY_WATCHPOINT
+ || lp->stop_reason == TARGET_STOPPED_BY_HW_BREAKPOINT
+ || lp_arch_info->debug_regs_stale);
+
+ if (!stale_dregs)
return;
- for (i = 0; i < ppc_threads.size (); i++)
+ gdb_assert (lp->ptid.lwp_p ());
+
+ auto process_it = m_process_info.find (lp->ptid.pid ());
+
+ if (m_dreg_interface.hwdebug_p ())
{
- if (ppc_threads[i]->tid == tid)
+ /* First, delete any hardware watchpoint or breakpoint installed in
+ the inferior and update the thread state. */
+ auto installed_it = m_installed_hw_bps.find (lp->ptid);
+
+ if (installed_it != m_installed_hw_bps.end ())
{
- t = ppc_threads[i];
- break;
+ auto &bp_list = installed_it->second;
+
+ for (auto bp_it = bp_list.begin (); bp_it != bp_list.end ();)
+ {
+ /* We ignore ENOENT to account for various possible kernel
+ behaviors, e.g. the kernel might or might not copy debug
+ registers across forks and clones, and we always copy
+ the debug register state when fork and clone events are
+ detected. */
+ if (ptrace (PPC_PTRACE_DELHWDEBUG, lp->ptid.lwp (), 0,
+ bp_it->first) == -1)
+ if (errno != ENOENT)
+ perror_with_name (_("Error deleting hardware "
+ "breakpoint or watchpoint"));
+
+ /* We erase the entries one at a time after successfuly
+ removing the corresponding slot form the thread so that
+ if we throw an exception above in a future iteration the
+ map remains consistent. */
+ bp_it = bp_list.erase (bp_it);
+ }
+
+ gdb_assert (bp_list.empty ());
+ }
+
+ /* Now we install all the requested hardware breakpoints and
+ watchpoints and update the thread state. */
+
+ if (process_it != m_process_info.end ())
+ {
+ auto &bp_list = m_installed_hw_bps[lp->ptid];
+
+ for (ppc_hw_breakpoint bp
+ : process_it->second.requested_hw_bps)
+ {
+ long slot = ptrace (PPC_PTRACE_SETHWDEBUG, lp->ptid.lwp (),
+ 0, &bp);
+
+ if (slot < 0)
+ perror_with_name (_("Error setting hardware "
+ "breakpoint or watchpoint"));
+
+ /* Keep track of which slots we installed in this
+ thread. */
+ bp_list.emplace (bp_list.begin (), slot, bp);
+ }
}
}
+ else
+ {
+ gdb_assert (m_dreg_interface.debugreg_p ());
- if (t == NULL)
- return;
+ /* Passing 0 to PTRACE_SET_DEBUGREG will clear the
+ watchpoint. */
+ long wp = 0;
- unordered_remove (ppc_threads, i);
+ /* GDB requested a watchpoint to be installed. */
+ if (process_it != m_process_info.end ()
+ && process_it->second.requested_wp_val.has_value ())
+ wp = *(process_it->second.requested_wp_val);
- hw_breaks = t->hw_breaks;
+ long ret = ptrace (PTRACE_SET_DEBUGREG, lp->ptid.lwp (),
+ 0, wp);
- for (i = 0; i < max_slots_number; i++)
- if (hw_breaks[i].hw_break)
- xfree (hw_breaks[i].hw_break);
+ if (ret == -1)
+ perror_with_name (_("Error setting hardware watchpoint"));
+ }
- xfree (t->hw_breaks);
- xfree (t);
+ lp_arch_info->debug_regs_stale = false;
}
+/* Return true if INFERIOR_PTID is known to have been stopped by a
+ hardware watchpoint, false otherwise. If true is returned, write the
+ address that the kernel reported as causing the SIGTRAP in ADDR_P. */
+
bool
-ppc_linux_nat_target::stopped_data_address (CORE_ADDR *addr_p)
+ppc_linux_nat_target::low_stopped_data_address (CORE_ADDR *addr_p)
{
siginfo_t siginfo;
@@ -2597,38 +2944,45 @@ ppc_linux_nat_target::stopped_data_address (CORE_ADDR *addr_p)
|| (siginfo.si_code & 0xffff) != 0x0004 /* TRAP_HWBKPT */)
return false;
- if (have_ptrace_hwdebug_interface ())
+ gdb_assert (!m_dreg_interface.unavailable_p ());
+
+ /* Check if this signal corresponds to a hardware breakpoint. We only
+ need to check this if we're using the HWDEBUG interface, since the
+ DEBUGREG interface only allows setting one hardware watchpoint. */
+ if (m_dreg_interface.hwdebug_p ())
{
- int i;
- struct thread_points *t;
- struct hw_break_tuple *hw_breaks;
- /* The index (or slot) of the *point is passed in the si_errno field. */
+ /* The index (or slot) of the *point is passed in the si_errno
+ field. Currently, this is only the case if the kernel was
+ configured with CONFIG_PPC_ADV_DEBUG_REGS. If not, we assume
+ the kernel will set si_errno to a value that doesn't correspond
+ to any real slot. */
int slot = siginfo.si_errno;
- t = hwdebug_find_thread_points_by_tid (inferior_ptid.lwp (), 0);
+ auto installed_it = m_installed_hw_bps.find (inferior_ptid);
- /* Find out if this *point is a hardware breakpoint.
- If so, we should return 0. */
- if (t)
- {
- hw_breaks = t->hw_breaks;
- for (i = 0; i < max_slots_number; i++)
- if (hw_breaks[i].hw_break && hw_breaks[i].slot == slot
- && hw_breaks[i].hw_break->trigger_type
- == PPC_BREAKPOINT_TRIGGER_EXECUTE)
- return false;
- }
+ /* We must have installed slots for the thread if it got a
+ TRAP_HWBKPT signal. */
+ gdb_assert (installed_it != m_installed_hw_bps.end ());
+
+ for (const auto & slot_bp_pair : installed_it->second)
+ if (slot_bp_pair.first == slot
+ && (slot_bp_pair.second.trigger_type
+ == PPC_BREAKPOINT_TRIGGER_EXECUTE))
+ return false;
}
*addr_p = (CORE_ADDR) (uintptr_t) siginfo.si_addr;
return true;
}
+/* Return true if INFERIOR_PTID is known to have been stopped by a
+ hardware watchpoint, false otherwise. */
+
bool
-ppc_linux_nat_target::stopped_by_watchpoint ()
+ppc_linux_nat_target::low_stopped_by_watchpoint ()
{
CORE_ADDR addr;
- return stopped_data_address (&addr);
+ return low_stopped_data_address (&addr);
}
bool
@@ -2636,9 +2990,11 @@ ppc_linux_nat_target::watchpoint_addr_within_range (CORE_ADDR addr,
CORE_ADDR start,
int length)
{
+ gdb_assert (!m_dreg_interface.unavailable_p ());
+
int mask;
- if (have_ptrace_hwdebug_interface ()
+ if (m_dreg_interface.hwdebug_p ()
&& linux_get_hwcap (current_top_target ()) & PPC_FEATURE_BOOKE)
return start <= addr && start + length >= addr;
else if (linux_get_hwcap (current_top_target ()) & PPC_FEATURE_BOOKE)
@@ -2655,10 +3011,14 @@ ppc_linux_nat_target::watchpoint_addr_within_range (CORE_ADDR addr,
/* Return the number of registers needed for a masked hardware watchpoint. */
int
-ppc_linux_nat_target::masked_watch_num_registers (CORE_ADDR addr, CORE_ADDR mask)
+ppc_linux_nat_target::masked_watch_num_registers (CORE_ADDR addr,
+ CORE_ADDR mask)
{
- if (!have_ptrace_hwdebug_interface ()
- || (hwdebug_info.features & PPC_DEBUG_FEATURE_DATA_BP_MASK) == 0)
+ m_dreg_interface.detect (inferior_ptid);
+
+ if (!m_dreg_interface.hwdebug_p ()
+ || (m_dreg_interface.hwdebug_info ().features
+ & PPC_DEBUG_FEATURE_DATA_BP_MASK) == 0)
return -1;
else if ((mask & 0xC0000000) != 0xC0000000)
{
@@ -2671,14 +3031,182 @@ ppc_linux_nat_target::masked_watch_num_registers (CORE_ADDR addr, CORE_ADDR mask
return 2;
}
+/* Copy the per-thread debug register state, if any, from thread
+ PARENT_PTID to thread CHILD_PTID, if the debug register being used is
+ HWDEBUG. */
+
+void
+ppc_linux_nat_target::copy_thread_dreg_state (const ptid_t &parent_ptid,
+ const ptid_t &child_ptid)
+{
+ gdb_assert (m_dreg_interface.hwdebug_p ());
+
+ auto installed_it = m_installed_hw_bps.find (parent_ptid);
+
+ if (installed_it != m_installed_hw_bps.end ())
+ m_installed_hw_bps[child_ptid] = m_installed_hw_bps[parent_ptid];
+}
+
+/* Mark the debug register stale flag for the new thread, if we have
+ already detected which debug register interface we use. */
+
+void
+ppc_linux_nat_target::mark_thread_stale (struct lwp_info *lp)
+{
+ if ((!m_dreg_interface.detected_p ())
+ || (m_dreg_interface.unavailable_p ()))
+ return;
+
+ arch_lwp_info *lp_arch_info = get_arch_lwp_info (lp);
+
+ lp_arch_info->debug_regs_stale = true;
+}
+
+/* Mark all the threads of the group of PID as stale with respect to
+ debug registers and issue a stop request to each such thread that
+ isn't already stopped. */
+
+void
+ppc_linux_nat_target::mark_debug_registers_changed (pid_t pid)
+{
+ /* We do this in two passes to make sure all threads are marked even if
+ we get an exception when stopping one of them. */
+
+ iterate_over_lwps (ptid_t (pid),
+ [this] (struct lwp_info *lp) -> int {
+ this->mark_thread_stale (lp);
+ return 0;
+ });
+
+ iterate_over_lwps (ptid_t (pid),
+ [] (struct lwp_info *lp) -> int {
+ if (!lwp_is_stopped (lp))
+ linux_stop_lwp (lp);
+ return 0;
+ });
+}
+
+/* Register a hardware breakpoint or watchpoint BP for the pid PID, then
+ mark the stale flag for all threads of the group of PID, and issue a
+ stop request for them. The breakpoint or watchpoint will be installed
+ the next time each thread is resumed. Should only be used if the
+ debug register interface is HWDEBUG. */
+
+void
+ppc_linux_nat_target::register_hw_breakpoint (pid_t pid,
+ const struct
+ ppc_hw_breakpoint &bp)
+{
+ gdb_assert (m_dreg_interface.hwdebug_p ());
+
+ m_process_info[pid].requested_hw_bps.push_back (bp);
+
+ mark_debug_registers_changed (pid);
+}
+
+/* Clear a registration for a hardware breakpoint or watchpoint BP for
+ the pid PID, then mark the stale flag for all threads of the group of
+ PID, and issue a stop request for them. The breakpoint or watchpoint
+ will be removed the next time each thread is resumed. Should only be
+ used if the debug register interface is HWDEBUG. */
+
+void
+ppc_linux_nat_target::clear_hw_breakpoint (pid_t pid,
+ const struct ppc_hw_breakpoint &bp)
+{
+ gdb_assert (m_dreg_interface.hwdebug_p ());
+
+ auto process_it = m_process_info.find (pid);
+
+ gdb_assert (process_it != m_process_info.end ());
+
+ auto bp_it = std::find_if (process_it->second.requested_hw_bps.begin (),
+ process_it->second.requested_hw_bps.end (),
+ [&bp, this]
+ (const struct ppc_hw_breakpoint &curr)
+ { return hwdebug_point_cmp (bp, curr); }
+ );
+
+ /* If GDB is removing a watchpoint, it must have been inserted. */
+ gdb_assert (bp_it != process_it->second.requested_hw_bps.end ());
+
+ process_it->second.requested_hw_bps.erase (bp_it);
+
+ mark_debug_registers_changed (pid);
+}
+
+/* Register the hardware watchpoint value WP_VALUE for the pid PID,
+ then mark the stale flag for all threads of the group of PID, and
+ issue a stop request for them. The breakpoint or watchpoint will be
+ installed the next time each thread is resumed. Should only be used
+ if the debug register interface is DEBUGREG. */
+
+void
+ppc_linux_nat_target::register_wp (pid_t pid, long wp_value)
+{
+ gdb_assert (m_dreg_interface.debugreg_p ());
+
+ /* Our other functions should have told GDB that we only have one
+ hardware watchpoint with this interface. */
+ gdb_assert (!m_process_info[pid].requested_wp_val.has_value ());
+
+ m_process_info[pid].requested_wp_val.emplace (wp_value);
+
+ mark_debug_registers_changed (pid);
+}
+
+/* Clear the hardware watchpoint registration for the pid PID, then mark
+ the stale flag for all threads of the group of PID, and issue a stop
+ request for them. The breakpoint or watchpoint will be installed the
+ next time each thread is resumed. Should only be used if the debug
+ register interface is DEBUGREG. */
+
+void
+ppc_linux_nat_target::clear_wp (pid_t pid)
+{
+ gdb_assert (m_dreg_interface.debugreg_p ());
+
+ auto process_it = m_process_info.find (pid);
+
+ gdb_assert (process_it != m_process_info.end ());
+ gdb_assert (process_it->second.requested_wp_val.has_value ());
+
+ process_it->second.requested_wp_val.reset ();
+
+ mark_debug_registers_changed (pid);
+}
+
+/* Initialize the arch-specific thread state for LWP, if it not already
+ created. */
+
+void
+ppc_linux_nat_target::init_arch_lwp_info (struct lwp_info *lp)
+{
+ if (lwp_arch_private_info (lp) == NULL)
+ {
+ lwp_set_arch_private_info (lp, XCNEW (struct arch_lwp_info));
+ lwp_arch_private_info (lp)->debug_regs_stale = false;
+ lwp_arch_private_info (lp)->lwp_ptid = lp->ptid;
+ }
+}
+
+/* Get the arch-specific thread state for LWP, creating it if
+ necessary. */
+
+arch_lwp_info *
+ppc_linux_nat_target::get_arch_lwp_info (struct lwp_info *lp)
+{
+ init_arch_lwp_info (lp);
+
+ return lwp_arch_private_info (lp);
+}
+
void _initialize_ppc_linux_nat ();
void
_initialize_ppc_linux_nat ()
{
linux_target = &the_ppc_linux_nat_target;
- gdb::observers::thread_exit.attach (ppc_linux_thread_exit);
-
/* Register the target. */
add_inf_child_target (linux_target);
}
--
2.24.1
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2 3/3] [PowerPC] Fix debug register issues in ppc-linux-nat
2020-03-27 18:50 ` Pedro Franco de Carvalho
2020-03-27 18:54 ` [PATCH] " Pedro Franco de Carvalho
@ 2020-03-30 13:03 ` Ulrich Weigand
2020-03-30 15:13 ` Pedro Franco de Carvalho
1 sibling, 1 reply; 15+ messages in thread
From: Ulrich Weigand @ 2020-03-30 13:03 UTC (permalink / raw)
To: Pedro Franco de Carvalho; +Cc: gdb-patches
Pedro Franco de Carvalho wrote:
> I'll post in reply to this e-mail one possible new version of the patch,
> that just makes the inferior_ptid uses explicit throughout
> ppc-linux-nat.c.
>
> It doesn't solve the ESRCH issue described in the previous e-mail.
Sorry for the late reply to the previous mail. I agree that this
approach is probably the best for now. The ESRCH issue is a pre-
existing problem; if we want to address it in the future, this can
still be done as a follow-on.
Bye,
Ulrich
--
Dr. Ulrich Weigand
GNU/Linux compilers and toolchain
Ulrich.Weigand@de.ibm.com
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] [PowerPC] Fix debug register issues in ppc-linux-nat
2020-03-27 18:54 ` [PATCH] " Pedro Franco de Carvalho
@ 2020-03-30 13:04 ` Ulrich Weigand
0 siblings, 0 replies; 15+ messages in thread
From: Ulrich Weigand @ 2020-03-30 13:04 UTC (permalink / raw)
To: Pedro Franco de Carvalho; +Cc: gdb-patches
Pedro Franco de Carvalho wrote:
> This patch fixes some issues with debug register handling for the powerpc
> linux native target.
OK, let's go with this version. Thanks again!
> gdb/ChangeLog:
> YYYY-MM-DD Pedro Franco de Carvalho <pedromfc@linux.ibm.com>
>
> * ppc-linux-nat.c: Include <algorithm>, <unordered_map>, and
> <list>. Remove inclusion of observable.h.
> (PPC_DEBUG_CURRENT_VERSION): Move up define.
> (struct arch_lwp_info): New struct.
> (class ppc_linux_dreg_interface): New class.
> (struct ppc_linux_process_info): New struct.
> (struct ppc_linux_nat_target) <low_delete_thread, low_new_fork>
> <low_new_clone, low_forget_process, low_prepare_to_resume>
> <copy_thread_dreg_state, mark_thread_stale>
> <mark_debug_registers_changed, register_hw_breakpoint>
> <clear_hw_breakpoint, register_wp, clear_wp>
> <can_use_watchpoint_cond_accel, calculate_dvc, check_condition>
> <num_memory_accesses, get_trigger_type>
> <create_watchpoint_request, hwdebug_point_cmp>
> <init_arch_lwp_info, get_arch_lwp_info>
> <low_stopped_by_watchpoint, low_stopped_data_address>: Declare as
> methods.
> <struct ptid_hash>: New inner struct.
> <m_dreg_interface, m_process_info, m_installed_hw_bps>: Declare
> members.
> (saved_dabr_value, hwdebug_info, max_slots_number)
> (struct hw_break_tuple, struct thread_points, ppc_threads)
> (have_ptrace_hwdebug_interface)
> (hwdebug_find_thread_points_by_tid)
> (hwdebug_insert_point, hwdebug_remove_point): Remove.
> (ppc_linux_nat_target::can_use_hw_breakpoint): Use
> m_dreg_interface, remove call to PTRACE_SET_DEBUGREG.
> (ppc_linux_nat_target::region_ok_for_hw_watchpoint): Add comment,
> use m_dreg_interface.
> (hwdebug_point_cmp): Change to...
> (ppc_linux_nat_target::hwdebug_point_cmp): ...this method. Use
> reference arguments instead of pointers.
> (ppc_linux_nat_target::ranged_break_num_registers): Use
> m_dreg_interface.
> (ppc_linux_nat_target::insert_hw_breakpoint): Add comment, use
> m_dreg_interface. Call register_hw_breakpoint.
> (ppc_linux_nat_target::remove_hw_breakpoint): Add comment, use
> m_dreg_interface. Call clear_hw_breakpoint.
> (get_trigger_type): Change to...
> (ppc_linux_nat_target::get_trigger_type): ...this method. Add
> comment.
> (ppc_linux_nat_target::insert_mask_watchpoint): Update comment,
> use m_dreg_interface. Call register_hw_breakpoint.
> (ppc_linux_nat_target::remove_mask_watchpoint): Update comment,
> use m_dreg_interface. Call clear_hw_breakpoint.
> (can_use_watchpoint_cond_accel): Change to...
> (ppc_linux_nat_target::can_use_watchpoint_cond_accel): ...this
> method. Update comment, use m_dreg_interface and
> m_process_info.
> (calculate_dvc): Change to...
> (ppc_linux_nat_target::calculate_dvc): ...this method. Use
> m_dreg_interface.
> (num_memory_accesses): Change to...
> (ppc_linux_nat_target::num_memory_accesses): ...this method.
> (check_condition): Change to...
> (ppc_linux_nat_target::check_condition): ...this method.
> (ppc_linux_nat_target::can_accel_watchpoint_condition): Update
> comment, use m_dreg_interface.
> (create_watchpoint_request): Change to...
> (ppc_linux_nat_target::create_watchpoint_request): ...this
> method. Use m_dreg_interface.
> (ppc_linux_nat_target::insert_watchpoint): Add comment, use
> m_dreg_interface. Call register_hw_breakpoint or register_wp.
> (ppc_linux_nat_target::remove_watchpoint): Add comment, use
> m_dreg_interface. Call clear_hw_breakpoint or clear_wp.
> (ppc_linux_nat_target::low_forget_process)
> (ppc_linux_nat_target::low_new_fork)
> (ppc_linux_nat_target::low_new_clone)
> (ppc_linux_nat_target::low_delete_thread)
> (ppc_linux_nat_target::low_prepare_to_resume): New methods.
> (ppc_linux_nat_target::low_new_thread): Remove previous logic,
> only call mark_thread_stale.
> (ppc_linux_thread_exit): Remove.
> (ppc_linux_nat_target::stopped_data_address): Change to...
> (ppc_linux_nat_target::low_stopped_data_address): This. Add
> comment, use m_dreg_interface and m_thread_hw_breakpoints.
> (ppc_linux_nat_target::stopped_by_watchpoint): Change to...
> (ppc_linux_nat_target::stopped_by_watchpoint): This. Add
> comment. Call low_stopped_data_address.
> (ppc_linux_nat_target::watchpoint_addr_within_range): Use
> m_dreg_interface.
> (ppc_linux_nat_target::masked_watch_num_registers): Use
> m_dreg_interface.
> (ppc_linux_nat_target::copy_thread_dreg_state)
> (ppc_linux_nat_target::mark_thread_stale)
> (ppc_linux_nat_target::mark_debug_registers_changed)
> (ppc_linux_nat_target::register_hw_breakpoint)
> (ppc_linux_nat_target::clear_hw_breakpoint)
> (ppc_linux_nat_target::register_wp)
> (ppc_linux_nat_target::clear_wp)
> (ppc_linux_nat_target::init_arch_lwp_info)
> (ppc_linux_nat_target::get_arch_lwp_info): New methods.
> (_initialize_ppc_linux_nat): Remove observer callback.
This is OK.
Bye,
Ulrich
--
Dr. Ulrich Weigand
GNU/Linux compilers and toolchain
Ulrich.Weigand@de.ibm.com
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2 3/3] [PowerPC] Fix debug register issues in ppc-linux-nat
2020-03-30 13:03 ` [PATCH v2 3/3] " Ulrich Weigand
@ 2020-03-30 15:13 ` Pedro Franco de Carvalho
0 siblings, 0 replies; 15+ messages in thread
From: Pedro Franco de Carvalho @ 2020-03-30 15:13 UTC (permalink / raw)
To: Ulrich Weigand; +Cc: gdb-patches
"Ulrich Weigand" <uweigand@de.ibm.com> writes:
Hello,
> Sorry for the late reply to the previous mail. I agree that this
> approach is probably the best for now. The ESRCH issue is a pre-
> existing problem; if we want to address it in the future, this can
> still be done as a follow-on.
Ok! Strictly speaking only the HWDEBUG interface would be unavailable
for the duration of the GDB session in the previous version of the code,
the DEBUGREG one would still be tried the next time. The current one
would have neither. But I do think worth sacrificing this improbable
case for more clarity in the interface detection code.
I've checked the patches in. Thanks a lot for the reviews!!
--
Pedro Franco de Carvalho
^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2020-03-30 15:14 UTC | newest]
Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-02-14 20:55 [PATCH v2 0/3] low_new_clone in linux-nat.c and powerpc watchpoint fixes Pedro Franco de Carvalho
2020-02-14 20:55 ` [PATCH v2 3/3] [PowerPC] Fix debug register issues in ppc-linux-nat Pedro Franco de Carvalho
2020-02-17 17:47 ` Ulrich Weigand
2020-02-18 20:31 ` Pedro Franco de Carvalho
2020-02-19 13:46 ` Ulrich Weigand
2020-03-13 20:19 ` Pedro Franco de Carvalho
2020-03-27 18:50 ` Pedro Franco de Carvalho
2020-03-27 18:54 ` [PATCH] " Pedro Franco de Carvalho
2020-03-30 13:04 ` Ulrich Weigand
2020-03-30 13:03 ` [PATCH v2 3/3] " Ulrich Weigand
2020-03-30 15:13 ` Pedro Franco de Carvalho
2020-02-14 20:55 ` [PATCH v2 1/3] Add low_new_clone method to linux_nat_target Pedro Franco de Carvalho
2020-02-17 17:49 ` Ulrich Weigand
2020-02-14 20:55 ` [PATCH v2 2/3] [PowerPC] Move up some register access routines Pedro Franco de Carvalho
2020-02-17 17:48 ` Ulrich Weigand
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).