* [PATCH] gdb+gdbserver/Linux: Remove USE_SIGTRAP_SIGINFO fallback
@ 2022-06-27 21:10 Pedro Alves
2024-04-17 16:40 ` Pedro Alves
0 siblings, 1 reply; 2+ messages in thread
From: Pedro Alves @ 2022-06-27 21:10 UTC (permalink / raw)
To: gdb-patches
It's been over 7 years (since commit faf09f0119da) since Linux GDB and
GDBserver started relying on SIGTRAP si_code to tell whether a
breakpoint triggered, which is important for non-stop mode. When that
then-new code was added, I had left the then-old code as fallback, in
case some architectured still needed it. Given AFAIK there haven't
been complaints since, this commit finally removes the fallback code,
along with USE_SIGTRAP_SIGINFO.
Change-Id: I140a5333a9fe70e90dbd186aca1f081549b2e63d
---
gdb/linux-nat.c | 53 ++----------------------------------------
gdb/nat/linux-ptrace.h | 11 ++++-----
gdbserver/linux-low.cc | 39 ++-----------------------------
3 files changed, 8 insertions(+), 95 deletions(-)
diff --git a/gdb/linux-nat.c b/gdb/linux-nat.c
index b9164e621db..008791c12dc 100644
--- a/gdb/linux-nat.c
+++ b/gdb/linux-nat.c
@@ -2439,17 +2439,6 @@ status_callback (struct lwp_info *lp)
discard = 1;
}
-#if !USE_SIGTRAP_SIGINFO
- else if (!breakpoint_inserted_here_p (regcache->aspace (), pc))
- {
- linux_nat_debug_printf ("previous breakpoint of %s, at %s gone",
- lp->ptid.to_string ().c_str (),
- paddress (target_gdbarch (), lp->stop_pc));
-
- discard = 1;
- }
-#endif
-
if (discard)
{
linux_nat_debug_printf ("pending event of %s cancelled.",
@@ -2529,9 +2518,7 @@ save_stop_reason (struct lwp_info *lp)
struct gdbarch *gdbarch;
CORE_ADDR pc;
CORE_ADDR sw_bp_pc;
-#if USE_SIGTRAP_SIGINFO
siginfo_t siginfo;
-#endif
gdb_assert (lp->stop_reason == TARGET_STOPPED_BY_NO_REASON);
gdb_assert (lp->status != 0);
@@ -2545,7 +2532,6 @@ save_stop_reason (struct lwp_info *lp)
pc = regcache_read_pc (regcache);
sw_bp_pc = pc - gdbarch_decr_pc_after_break (gdbarch);
-#if USE_SIGTRAP_SIGINFO
if (linux_nat_get_siginfo (lp->ptid, &siginfo))
{
if (siginfo.si_signo == SIGTRAP)
@@ -2588,22 +2574,6 @@ save_stop_reason (struct lwp_info *lp)
}
}
}
-#else
- if ((!lp->step || lp->stop_pc == sw_bp_pc)
- && software_breakpoint_inserted_here_p (regcache->aspace (),
- sw_bp_pc))
- {
- /* The LWP was either continued, or stepped a software
- breakpoint instruction. */
- lp->stop_reason = TARGET_STOPPED_BY_SW_BREAKPOINT;
- }
-
- if (hardware_breakpoint_inserted_here_p (regcache->aspace (), pc))
- lp->stop_reason = TARGET_STOPPED_BY_HW_BREAKPOINT;
-
- if (lp->stop_reason == TARGET_STOPPED_BY_NO_REASON)
- check_stopped_by_watchpoint (lp);
-#endif
if (lp->stop_reason == TARGET_STOPPED_BY_SW_BREAKPOINT)
{
@@ -2649,7 +2619,7 @@ linux_nat_target::stopped_by_sw_breakpoint ()
bool
linux_nat_target::supports_stopped_by_sw_breakpoint ()
{
- return USE_SIGTRAP_SIGINFO;
+ return true;
}
/* Returns true if the LWP had stopped for a hardware
@@ -2670,7 +2640,7 @@ linux_nat_target::stopped_by_hw_breakpoint ()
bool
linux_nat_target::supports_stopped_by_hw_breakpoint ()
{
- return USE_SIGTRAP_SIGINFO;
+ return true;
}
/* Select one LWP out of those that have events pending. */
@@ -3252,25 +3222,6 @@ linux_nat_wait_1 (ptid_t ptid, struct target_waitstatus *ourstatus,
gdb_assert (lp != NULL);
- /* Now that we've selected our final event LWP, un-adjust its PC if
- it was a software breakpoint, and we can't reliably support the
- "stopped by software breakpoint" stop reason. */
- if (lp->stop_reason == TARGET_STOPPED_BY_SW_BREAKPOINT
- && !USE_SIGTRAP_SIGINFO)
- {
- struct regcache *regcache = get_thread_regcache (linux_target, lp->ptid);
- struct gdbarch *gdbarch = regcache->arch ();
- int decr_pc = gdbarch_decr_pc_after_break (gdbarch);
-
- if (decr_pc != 0)
- {
- CORE_ADDR pc;
-
- pc = regcache_read_pc (regcache);
- regcache_write_pc (regcache, pc + decr_pc);
- }
- }
-
/* We'll need this to determine whether to report a SIGSTOP as
GDB_SIGNAL_0. Need to take a copy because resume_clear_callback
clears it. */
diff --git a/gdb/nat/linux-ptrace.h b/gdb/nat/linux-ptrace.h
index 4694046e1e8..3edc2363aa2 100644
--- a/gdb/nat/linux-ptrace.h
+++ b/gdb/nat/linux-ptrace.h
@@ -97,9 +97,9 @@ struct buffer;
#define __WALL 0x40000000 /* Wait for any child. */
#endif
-/* True if whether a breakpoint/watchpoint triggered can be determined
- from the si_code of SIGTRAP's siginfo_t (TRAP_BRKPT/TRAP_HWBKPT).
- That is, if the kernel can tell us whether the thread executed a
+/* Whether a breakpoint/watchpoint triggered can be determined from
+ the si_code of SIGTRAP's siginfo_t (TRAP_BRKPT/TRAP_HWBKPT). That
+ is, since the kernel can tell us whether the thread executed a
software breakpoint, we trust it. The kernel will be determining
that from the hardware (e.g., from which exception was raised in
the CPU). Relying on whether a breakpoint is planted in memory at
@@ -112,10 +112,7 @@ struct buffer;
architectures. The moribund location mechanism helps with that
somewhat but it is an heuristic, and can well fail. Getting that
information out of the kernel and ultimately out of the CPU is the
- way to go. That said, some architecture may get the si_code wrong,
- and as such we're leaving fallback code in place. We'll remove
- this after a while if no problem is reported. */
-#define USE_SIGTRAP_SIGINFO 1
+ way to go. */
/* The x86 kernel gets some of the si_code values backwards, like
this:
diff --git a/gdbserver/linux-low.cc b/gdbserver/linux-low.cc
index 8b8614f6ed4..9305928bbbf 100644
--- a/gdbserver/linux-low.cc
+++ b/gdbserver/linux-low.cc
@@ -790,9 +790,7 @@ linux_process_target::save_stop_reason (lwp_info *lwp)
{
CORE_ADDR pc;
CORE_ADDR sw_breakpoint_pc;
-#if USE_SIGTRAP_SIGINFO
siginfo_t siginfo;
-#endif
if (!low_supports_breakpoints ())
return false;
@@ -804,7 +802,6 @@ linux_process_target::save_stop_reason (lwp_info *lwp)
scoped_restore_current_thread restore_thread;
switch_to_thread (get_lwp_thread (lwp));
-#if USE_SIGTRAP_SIGINFO
if (ptrace (PTRACE_GETSIGINFO, lwpid_of (current_thread),
(PTRACE_TYPE_ARG3) 0, &siginfo) == 0)
{
@@ -846,21 +843,6 @@ linux_process_target::save_stop_reason (lwp_info *lwp)
}
}
}
-#else
- /* We may have just stepped a breakpoint instruction. E.g., in
- non-stop mode, GDB first tells the thread A to step a range, and
- then the user inserts a breakpoint inside the range. In that
- case we need to report the breakpoint PC. */
- if ((!lwp->stepping || lwp->stop_pc == sw_breakpoint_pc)
- && low_breakpoint_at (sw_breakpoint_pc))
- lwp->stop_reason = TARGET_STOPPED_BY_SW_BREAKPOINT;
-
- if (hardware_breakpoint_inserted_here (pc))
- lwp->stop_reason = TARGET_STOPPED_BY_HW_BREAKPOINT;
-
- if (lwp->stop_reason == TARGET_STOPPED_BY_NO_REASON)
- check_stopped_by_watchpoint (lwp);
-#endif
if (lwp->stop_reason == TARGET_STOPPED_BY_SW_BREAKPOINT)
{
@@ -1648,23 +1630,6 @@ linux_process_target::thread_still_has_status_pending (thread_info *thread)
discard = 1;
}
-#if !USE_SIGTRAP_SIGINFO
- else if (lp->stop_reason == TARGET_STOPPED_BY_SW_BREAKPOINT
- && !low_breakpoint_at (pc))
- {
- threads_debug_printf ("previous SW breakpoint of %ld gone",
- lwpid_of (thread));
- discard = 1;
- }
- else if (lp->stop_reason == TARGET_STOPPED_BY_HW_BREAKPOINT
- && !hardware_breakpoint_inserted_here (pc))
- {
- threads_debug_printf ("previous HW breakpoint of %ld gone",
- lwpid_of (thread));
- discard = 1;
- }
-#endif
-
if (discard)
{
threads_debug_printf ("discarding pending breakpoint status");
@@ -5555,7 +5520,7 @@ linux_process_target::stopped_by_sw_breakpoint ()
bool
linux_process_target::supports_stopped_by_sw_breakpoint ()
{
- return USE_SIGTRAP_SIGINFO;
+ return true;
}
/* Implement the stopped_by_hw_breakpoint target_ops
@@ -5575,7 +5540,7 @@ linux_process_target::stopped_by_hw_breakpoint ()
bool
linux_process_target::supports_stopped_by_hw_breakpoint ()
{
- return USE_SIGTRAP_SIGINFO;
+ return true;
}
/* Implement the supports_hardware_single_step target_ops method. */
base-commit: f18acc9c4e5d18f4783f3a7d59e3ec95d7af0199
--
2.36.0
^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: [PATCH] gdb+gdbserver/Linux: Remove USE_SIGTRAP_SIGINFO fallback
2022-06-27 21:10 [PATCH] gdb+gdbserver/Linux: Remove USE_SIGTRAP_SIGINFO fallback Pedro Alves
@ 2024-04-17 16:40 ` Pedro Alves
0 siblings, 0 replies; 2+ messages in thread
From: Pedro Alves @ 2024-04-17 16:40 UTC (permalink / raw)
To: gdb-patches
On 2022-06-27 22:10, Pedro Alves wrote:
> It's been over 7 years (since commit faf09f0119da) since Linux GDB and
> GDBserver started relying on SIGTRAP si_code to tell whether a
> breakpoint triggered, which is important for non-stop mode. When that
> then-new code was added, I had left the then-old code as fallback, in
> case some architectured still needed it. Given AFAIK there haven't
> been complaints since, this commit finally removes the fallback code,
> along with USE_SIGTRAP_SIGINFO.
>
> Change-Id: I140a5333a9fe70e90dbd186aca1f081549b2e63d
W00t. I was looking at linux-nat.c today, and noticed the USE_SIGTRAP_SIGINFO
code was still around, which made me go like "but hadn't I removed this before??".
Well, turns out I forgot to merge this.
I'm going to merge it, after I download the patch from the list and
update that "7 years". :-)
Pedro Alves
> ---
> gdb/linux-nat.c | 53 ++----------------------------------------
> gdb/nat/linux-ptrace.h | 11 ++++-----
> gdbserver/linux-low.cc | 39 ++-----------------------------
> 3 files changed, 8 insertions(+), 95 deletions(-)
>
> diff --git a/gdb/linux-nat.c b/gdb/linux-nat.c
> index b9164e621db..008791c12dc 100644
> --- a/gdb/linux-nat.c
> +++ b/gdb/linux-nat.c
> @@ -2439,17 +2439,6 @@ status_callback (struct lwp_info *lp)
> discard = 1;
> }
>
> -#if !USE_SIGTRAP_SIGINFO
> - else if (!breakpoint_inserted_here_p (regcache->aspace (), pc))
> - {
> - linux_nat_debug_printf ("previous breakpoint of %s, at %s gone",
> - lp->ptid.to_string ().c_str (),
> - paddress (target_gdbarch (), lp->stop_pc));
> -
> - discard = 1;
> - }
> -#endif
> -
> if (discard)
> {
> linux_nat_debug_printf ("pending event of %s cancelled.",
> @@ -2529,9 +2518,7 @@ save_stop_reason (struct lwp_info *lp)
> struct gdbarch *gdbarch;
> CORE_ADDR pc;
> CORE_ADDR sw_bp_pc;
> -#if USE_SIGTRAP_SIGINFO
> siginfo_t siginfo;
> -#endif
>
> gdb_assert (lp->stop_reason == TARGET_STOPPED_BY_NO_REASON);
> gdb_assert (lp->status != 0);
> @@ -2545,7 +2532,6 @@ save_stop_reason (struct lwp_info *lp)
> pc = regcache_read_pc (regcache);
> sw_bp_pc = pc - gdbarch_decr_pc_after_break (gdbarch);
>
> -#if USE_SIGTRAP_SIGINFO
> if (linux_nat_get_siginfo (lp->ptid, &siginfo))
> {
> if (siginfo.si_signo == SIGTRAP)
> @@ -2588,22 +2574,6 @@ save_stop_reason (struct lwp_info *lp)
> }
> }
> }
> -#else
> - if ((!lp->step || lp->stop_pc == sw_bp_pc)
> - && software_breakpoint_inserted_here_p (regcache->aspace (),
> - sw_bp_pc))
> - {
> - /* The LWP was either continued, or stepped a software
> - breakpoint instruction. */
> - lp->stop_reason = TARGET_STOPPED_BY_SW_BREAKPOINT;
> - }
> -
> - if (hardware_breakpoint_inserted_here_p (regcache->aspace (), pc))
> - lp->stop_reason = TARGET_STOPPED_BY_HW_BREAKPOINT;
> -
> - if (lp->stop_reason == TARGET_STOPPED_BY_NO_REASON)
> - check_stopped_by_watchpoint (lp);
> -#endif
>
> if (lp->stop_reason == TARGET_STOPPED_BY_SW_BREAKPOINT)
> {
> @@ -2649,7 +2619,7 @@ linux_nat_target::stopped_by_sw_breakpoint ()
> bool
> linux_nat_target::supports_stopped_by_sw_breakpoint ()
> {
> - return USE_SIGTRAP_SIGINFO;
> + return true;
> }
>
> /* Returns true if the LWP had stopped for a hardware
> @@ -2670,7 +2640,7 @@ linux_nat_target::stopped_by_hw_breakpoint ()
> bool
> linux_nat_target::supports_stopped_by_hw_breakpoint ()
> {
> - return USE_SIGTRAP_SIGINFO;
> + return true;
> }
>
> /* Select one LWP out of those that have events pending. */
> @@ -3252,25 +3222,6 @@ linux_nat_wait_1 (ptid_t ptid, struct target_waitstatus *ourstatus,
>
> gdb_assert (lp != NULL);
>
> - /* Now that we've selected our final event LWP, un-adjust its PC if
> - it was a software breakpoint, and we can't reliably support the
> - "stopped by software breakpoint" stop reason. */
> - if (lp->stop_reason == TARGET_STOPPED_BY_SW_BREAKPOINT
> - && !USE_SIGTRAP_SIGINFO)
> - {
> - struct regcache *regcache = get_thread_regcache (linux_target, lp->ptid);
> - struct gdbarch *gdbarch = regcache->arch ();
> - int decr_pc = gdbarch_decr_pc_after_break (gdbarch);
> -
> - if (decr_pc != 0)
> - {
> - CORE_ADDR pc;
> -
> - pc = regcache_read_pc (regcache);
> - regcache_write_pc (regcache, pc + decr_pc);
> - }
> - }
> -
> /* We'll need this to determine whether to report a SIGSTOP as
> GDB_SIGNAL_0. Need to take a copy because resume_clear_callback
> clears it. */
> diff --git a/gdb/nat/linux-ptrace.h b/gdb/nat/linux-ptrace.h
> index 4694046e1e8..3edc2363aa2 100644
> --- a/gdb/nat/linux-ptrace.h
> +++ b/gdb/nat/linux-ptrace.h
> @@ -97,9 +97,9 @@ struct buffer;
> #define __WALL 0x40000000 /* Wait for any child. */
> #endif
>
> -/* True if whether a breakpoint/watchpoint triggered can be determined
> - from the si_code of SIGTRAP's siginfo_t (TRAP_BRKPT/TRAP_HWBKPT).
> - That is, if the kernel can tell us whether the thread executed a
> +/* Whether a breakpoint/watchpoint triggered can be determined from
> + the si_code of SIGTRAP's siginfo_t (TRAP_BRKPT/TRAP_HWBKPT). That
> + is, since the kernel can tell us whether the thread executed a
> software breakpoint, we trust it. The kernel will be determining
> that from the hardware (e.g., from which exception was raised in
> the CPU). Relying on whether a breakpoint is planted in memory at
> @@ -112,10 +112,7 @@ struct buffer;
> architectures. The moribund location mechanism helps with that
> somewhat but it is an heuristic, and can well fail. Getting that
> information out of the kernel and ultimately out of the CPU is the
> - way to go. That said, some architecture may get the si_code wrong,
> - and as such we're leaving fallback code in place. We'll remove
> - this after a while if no problem is reported. */
> -#define USE_SIGTRAP_SIGINFO 1
> + way to go. */
>
> /* The x86 kernel gets some of the si_code values backwards, like
> this:
> diff --git a/gdbserver/linux-low.cc b/gdbserver/linux-low.cc
> index 8b8614f6ed4..9305928bbbf 100644
> --- a/gdbserver/linux-low.cc
> +++ b/gdbserver/linux-low.cc
> @@ -790,9 +790,7 @@ linux_process_target::save_stop_reason (lwp_info *lwp)
> {
> CORE_ADDR pc;
> CORE_ADDR sw_breakpoint_pc;
> -#if USE_SIGTRAP_SIGINFO
> siginfo_t siginfo;
> -#endif
>
> if (!low_supports_breakpoints ())
> return false;
> @@ -804,7 +802,6 @@ linux_process_target::save_stop_reason (lwp_info *lwp)
> scoped_restore_current_thread restore_thread;
> switch_to_thread (get_lwp_thread (lwp));
>
> -#if USE_SIGTRAP_SIGINFO
> if (ptrace (PTRACE_GETSIGINFO, lwpid_of (current_thread),
> (PTRACE_TYPE_ARG3) 0, &siginfo) == 0)
> {
> @@ -846,21 +843,6 @@ linux_process_target::save_stop_reason (lwp_info *lwp)
> }
> }
> }
> -#else
> - /* We may have just stepped a breakpoint instruction. E.g., in
> - non-stop mode, GDB first tells the thread A to step a range, and
> - then the user inserts a breakpoint inside the range. In that
> - case we need to report the breakpoint PC. */
> - if ((!lwp->stepping || lwp->stop_pc == sw_breakpoint_pc)
> - && low_breakpoint_at (sw_breakpoint_pc))
> - lwp->stop_reason = TARGET_STOPPED_BY_SW_BREAKPOINT;
> -
> - if (hardware_breakpoint_inserted_here (pc))
> - lwp->stop_reason = TARGET_STOPPED_BY_HW_BREAKPOINT;
> -
> - if (lwp->stop_reason == TARGET_STOPPED_BY_NO_REASON)
> - check_stopped_by_watchpoint (lwp);
> -#endif
>
> if (lwp->stop_reason == TARGET_STOPPED_BY_SW_BREAKPOINT)
> {
> @@ -1648,23 +1630,6 @@ linux_process_target::thread_still_has_status_pending (thread_info *thread)
> discard = 1;
> }
>
> -#if !USE_SIGTRAP_SIGINFO
> - else if (lp->stop_reason == TARGET_STOPPED_BY_SW_BREAKPOINT
> - && !low_breakpoint_at (pc))
> - {
> - threads_debug_printf ("previous SW breakpoint of %ld gone",
> - lwpid_of (thread));
> - discard = 1;
> - }
> - else if (lp->stop_reason == TARGET_STOPPED_BY_HW_BREAKPOINT
> - && !hardware_breakpoint_inserted_here (pc))
> - {
> - threads_debug_printf ("previous HW breakpoint of %ld gone",
> - lwpid_of (thread));
> - discard = 1;
> - }
> -#endif
> -
> if (discard)
> {
> threads_debug_printf ("discarding pending breakpoint status");
> @@ -5555,7 +5520,7 @@ linux_process_target::stopped_by_sw_breakpoint ()
> bool
> linux_process_target::supports_stopped_by_sw_breakpoint ()
> {
> - return USE_SIGTRAP_SIGINFO;
> + return true;
> }
>
> /* Implement the stopped_by_hw_breakpoint target_ops
> @@ -5575,7 +5540,7 @@ linux_process_target::stopped_by_hw_breakpoint ()
> bool
> linux_process_target::supports_stopped_by_hw_breakpoint ()
> {
> - return USE_SIGTRAP_SIGINFO;
> + return true;
> }
>
> /* Implement the supports_hardware_single_step target_ops method. */
>
> base-commit: f18acc9c4e5d18f4783f3a7d59e3ec95d7af0199
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2024-04-17 16:40 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-06-27 21:10 [PATCH] gdb+gdbserver/Linux: Remove USE_SIGTRAP_SIGINFO fallback Pedro Alves
2024-04-17 16:40 ` Pedro Alves
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).