Fix regression on aarch64-linux gdbserver
Checks
Context |
Check |
Description |
linaro-tcwg-bot/tcwg_gdb_build--master-aarch64 |
success
|
Testing passed
|
linaro-tcwg-bot/tcwg_gdb_build--master-arm |
success
|
Testing passed
|
linaro-tcwg-bot/tcwg_gdb_check--master-arm |
success
|
Testing passed
|
linaro-tcwg-bot/tcwg_gdb_check--master-aarch64 |
success
|
Testing passed
|
Commit Message
Commit 9a03f218 ("Fix gdb.base/watchpoint-unaligned.exp on aarch64")
fixed a watchpoint bug in gdb -- but did not touch the corresponding
code in gdbserver.
This patch moves the gdb code into gdb/nat, so that it can be shared
with gdbserver, and then changes gdbserver to use it, fixing the bug.
This is yet another case where having a single back end would prevent
bugs.
I tested this using the AdaCore internal gdb testsuite.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=29423
---
gdb/aarch64-nat.c | 115 ---------------------------------
gdb/aarch64-nat.h | 8 ---
gdb/nat/aarch64-hw-point.c | 115 +++++++++++++++++++++++++++++++++
gdb/nat/aarch64-hw-point.h | 8 +++
gdbserver/linux-aarch64-low.cc | 38 +----------
5 files changed, 126 insertions(+), 158 deletions(-)
Comments
Hi Tom,
Sorry, missed this amidst all the other submissions.
On 4/22/24 14:51, Tom Tromey wrote:
> Commit 9a03f218 ("Fix gdb.base/watchpoint-unaligned.exp on aarch64")
> fixed a watchpoint bug in gdb -- but did not touch the corresponding
> code in gdbserver.
>
> This patch moves the gdb code into gdb/nat, so that it can be shared
> with gdbserver, and then changes gdbserver to use it, fixing the bug.
>
> This is yet another case where having a single back end would prevent
> bugs.
I agree.
>
> I tested this using the AdaCore internal gdb testsuite.
>
> Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=29423
Thanks for catching this and for the refactoring.
Approved-By: Luis Machado <luis.machado@arm.com>
> ---
> gdb/aarch64-nat.c | 115 ---------------------------------
> gdb/aarch64-nat.h | 8 ---
> gdb/nat/aarch64-hw-point.c | 115 +++++++++++++++++++++++++++++++++
> gdb/nat/aarch64-hw-point.h | 8 +++
> gdbserver/linux-aarch64-low.cc | 38 +----------
> 5 files changed, 126 insertions(+), 158 deletions(-)
>
> diff --git a/gdb/aarch64-nat.c b/gdb/aarch64-nat.c
> index 894fb73095b..1ba9c4c19a7 100644
> --- a/gdb/aarch64-nat.c
> +++ b/gdb/aarch64-nat.c
> @@ -224,121 +224,6 @@ aarch64_remove_watchpoint (CORE_ADDR addr, int len, enum target_hw_bp_type type,
> return ret;
> }
>
> -/* See aarch64-nat.h. */
> -
> -bool
> -aarch64_stopped_data_address (const struct aarch64_debug_reg_state *state,
> - CORE_ADDR addr_trap, CORE_ADDR *addr_p)
> -{
> - bool found = false;
> - for (int phase = 0; phase <= 1; ++phase)
> - for (int i = aarch64_num_wp_regs - 1; i >= 0; --i)
> - {
> - if (!(state->dr_ref_count_wp[i]
> - && DR_CONTROL_ENABLED (state->dr_ctrl_wp[i])))
> - {
> - /* Watchpoint disabled. */
> - continue;
> - }
> -
> - const enum target_hw_bp_type type
> - = aarch64_watchpoint_type (state->dr_ctrl_wp[i]);
> - if (type == hw_execute)
> - {
> - /* Watchpoint disabled. */
> - continue;
> - }
> -
> - if (phase == 0)
> - {
> - /* Phase 0: No hw_write. */
> - if (type == hw_write)
> - continue;
> - }
> - else
> - {
> - /* Phase 1: Only hw_write. */
> - if (type != hw_write)
> - continue;
> - }
> -
> - const unsigned int offset
> - = aarch64_watchpoint_offset (state->dr_ctrl_wp[i]);
> - const unsigned int len
> - = aarch64_watchpoint_length (state->dr_ctrl_wp[i]);
> - const CORE_ADDR addr_watch = state->dr_addr_wp[i] + offset;
> - const CORE_ADDR addr_watch_aligned
> - = align_down (state->dr_addr_wp[i], AARCH64_HWP_MAX_LEN_PER_REG);
> - const CORE_ADDR addr_orig = state->dr_addr_orig_wp[i];
> -
> - /* ADDR_TRAP reports the first address of the memory range
> - accessed by the CPU, regardless of what was the memory
> - range watched. Thus, a large CPU access that straddles
> - the ADDR_WATCH..ADDR_WATCH+LEN range may result in an
> - ADDR_TRAP that is lower than the
> - ADDR_WATCH..ADDR_WATCH+LEN range. E.g.:
> -
> - addr: | 4 | 5 | 6 | 7 | 8 |
> - |---- range watched ----|
> - |----------- range accessed ------------|
> -
> - In this case, ADDR_TRAP will be 4.
> -
> - The access size also can be larger than that of the watchpoint
> - itself. For instance, the access size of an stp instruction is 16.
> - So, if we use stp to store to address p, and set a watchpoint on
> - address p + 8, the reported ADDR_TRAP can be p + 8 (observed on
> - RK3399 SOC). But it also can be p (observed on M1 SOC). Checking
> - for this situation introduces the possibility of false positives,
> - so we only do this for hw_write watchpoints. */
> - const CORE_ADDR max_access_size = type == hw_write ? 16 : 8;
> - const CORE_ADDR addr_watch_base = addr_watch_aligned -
> - (max_access_size - AARCH64_HWP_MAX_LEN_PER_REG);
> - if (!(addr_trap >= addr_watch_base
> - && addr_trap < addr_watch + len))
> - {
> - /* Not a match. */
> - continue;
> - }
> -
> - /* To match a watchpoint known to GDB core, we must never
> - report *ADDR_P outside of any ADDR_WATCH..ADDR_WATCH+LEN
> - range. ADDR_WATCH <= ADDR_TRAP < ADDR_ORIG is a false
> - positive on kernels older than 4.10. See PR
> - external/20207. */
> - if (addr_p != nullptr)
> - *addr_p = addr_orig;
> -
> - if (phase == 0)
> - {
> - /* Phase 0: Return first match. */
> - return true;
> - }
> -
> - /* Phase 1. */
> - if (addr_p == nullptr)
> - {
> - /* First match, and we don't need to report an address. No need
> - to look for other matches. */
> - return true;
> - }
> -
> - if (!found)
> - {
> - /* First match, and we need to report an address. Look for other
> - matches. */
> - found = true;
> - continue;
> - }
> -
> - /* More than one match, and we need to return an address. No need to
> - look for further matches. */
> - return false;
> - }
> -
> - return found;
> -}
> -
> /* Define AArch64 maintenance commands. */
>
> static void
> diff --git a/gdb/aarch64-nat.h b/gdb/aarch64-nat.h
> index 947d2805602..ec85524b2d4 100644
> --- a/gdb/aarch64-nat.h
> +++ b/gdb/aarch64-nat.h
> @@ -45,14 +45,6 @@ struct aarch64_debug_reg_state *aarch64_get_debug_reg_state (pid_t pid);
>
> void aarch64_remove_debug_reg_state (pid_t pid);
>
> -/* Helper for the "stopped_data_address" target method. Returns TRUE
> - if a hardware watchpoint trap at ADDR_TRAP matches a set
> - watchpoint. The address of the matched watchpoint is returned in
> - *ADDR_P. */
> -
> -bool aarch64_stopped_data_address (const struct aarch64_debug_reg_state *state,
> - CORE_ADDR addr_trap, CORE_ADDR *addr_p);
> -
> /* Helper functions used by aarch64_nat_target below. See their
> definitions. */
>
> diff --git a/gdb/nat/aarch64-hw-point.c b/gdb/nat/aarch64-hw-point.c
> index b62c4627d96..6acee0fb814 100644
> --- a/gdb/nat/aarch64-hw-point.c
> +++ b/gdb/nat/aarch64-hw-point.c
> @@ -645,3 +645,118 @@ aarch64_region_ok_for_watchpoint (CORE_ADDR addr, int len)
> the checking is costly. */
> return 1;
> }
> +
> +/* See nat/aarch64-hw-point.h. */
> +
> +bool
> +aarch64_stopped_data_address (const struct aarch64_debug_reg_state *state,
> + CORE_ADDR addr_trap, CORE_ADDR *addr_p)
> +{
> + bool found = false;
> + for (int phase = 0; phase <= 1; ++phase)
> + for (int i = aarch64_num_wp_regs - 1; i >= 0; --i)
> + {
> + if (!(state->dr_ref_count_wp[i]
> + && DR_CONTROL_ENABLED (state->dr_ctrl_wp[i])))
> + {
> + /* Watchpoint disabled. */
> + continue;
> + }
> +
> + const enum target_hw_bp_type type
> + = aarch64_watchpoint_type (state->dr_ctrl_wp[i]);
> + if (type == hw_execute)
> + {
> + /* Watchpoint disabled. */
> + continue;
> + }
> +
> + if (phase == 0)
> + {
> + /* Phase 0: No hw_write. */
> + if (type == hw_write)
> + continue;
> + }
> + else
> + {
> + /* Phase 1: Only hw_write. */
> + if (type != hw_write)
> + continue;
> + }
> +
> + const unsigned int offset
> + = aarch64_watchpoint_offset (state->dr_ctrl_wp[i]);
> + const unsigned int len
> + = aarch64_watchpoint_length (state->dr_ctrl_wp[i]);
> + const CORE_ADDR addr_watch = state->dr_addr_wp[i] + offset;
> + const CORE_ADDR addr_watch_aligned
> + = align_down (state->dr_addr_wp[i], AARCH64_HWP_MAX_LEN_PER_REG);
> + const CORE_ADDR addr_orig = state->dr_addr_orig_wp[i];
> +
> + /* ADDR_TRAP reports the first address of the memory range
> + accessed by the CPU, regardless of what was the memory
> + range watched. Thus, a large CPU access that straddles
> + the ADDR_WATCH..ADDR_WATCH+LEN range may result in an
> + ADDR_TRAP that is lower than the
> + ADDR_WATCH..ADDR_WATCH+LEN range. E.g.:
> +
> + addr: | 4 | 5 | 6 | 7 | 8 |
> + |---- range watched ----|
> + |----------- range accessed ------------|
> +
> + In this case, ADDR_TRAP will be 4.
> +
> + The access size also can be larger than that of the watchpoint
> + itself. For instance, the access size of an stp instruction is 16.
> + So, if we use stp to store to address p, and set a watchpoint on
> + address p + 8, the reported ADDR_TRAP can be p + 8 (observed on
> + RK3399 SOC). But it also can be p (observed on M1 SOC). Checking
> + for this situation introduces the possibility of false positives,
> + so we only do this for hw_write watchpoints. */
> + const CORE_ADDR max_access_size = type == hw_write ? 16 : 8;
> + const CORE_ADDR addr_watch_base = addr_watch_aligned -
> + (max_access_size - AARCH64_HWP_MAX_LEN_PER_REG);
> + if (!(addr_trap >= addr_watch_base
> + && addr_trap < addr_watch + len))
> + {
> + /* Not a match. */
> + continue;
> + }
> +
> + /* To match a watchpoint known to GDB core, we must never
> + report *ADDR_P outside of any ADDR_WATCH..ADDR_WATCH+LEN
> + range. ADDR_WATCH <= ADDR_TRAP < ADDR_ORIG is a false
> + positive on kernels older than 4.10. See PR
> + external/20207. */
> + if (addr_p != nullptr)
> + *addr_p = addr_orig;
> +
> + if (phase == 0)
> + {
> + /* Phase 0: Return first match. */
> + return true;
> + }
> +
> + /* Phase 1. */
> + if (addr_p == nullptr)
> + {
> + /* First match, and we don't need to report an address. No need
> + to look for other matches. */
> + return true;
> + }
> +
> + if (!found)
> + {
> + /* First match, and we need to report an address. Look for other
> + matches. */
> + found = true;
> + continue;
> + }
> +
> + /* More than one match, and we need to return an address. No need to
> + look for further matches. */
> + return false;
> + }
> +
> + return found;
> +}
> diff --git a/gdb/nat/aarch64-hw-point.h b/gdb/nat/aarch64-hw-point.h
> index bdcca932e57..0d50eaab0de 100644
> --- a/gdb/nat/aarch64-hw-point.h
> +++ b/gdb/nat/aarch64-hw-point.h
> @@ -110,6 +110,14 @@ unsigned int aarch64_watchpoint_offset (unsigned int ctrl);
> unsigned int aarch64_watchpoint_length (unsigned int ctrl);
> enum target_hw_bp_type aarch64_watchpoint_type (unsigned int ctrl);
>
> +/* Helper for the "stopped_data_address" target method. Returns TRUE
> + if a hardware watchpoint trap at ADDR_TRAP matches a set
> + watchpoint. The address of the matched watchpoint is returned in
> + *ADDR_P. */
> +
> +bool aarch64_stopped_data_address (const struct aarch64_debug_reg_state *state,
> + CORE_ADDR addr_trap, CORE_ADDR *addr_p);
> +
> int aarch64_handle_breakpoint (enum target_hw_bp_type type, CORE_ADDR addr,
> int len, int is_insert, ptid_t ptid,
> struct aarch64_debug_reg_state *state);
> diff --git a/gdbserver/linux-aarch64-low.cc b/gdbserver/linux-aarch64-low.cc
> index 5df67fccd08..da5c1fd0629 100644
> --- a/gdbserver/linux-aarch64-low.cc
> +++ b/gdbserver/linux-aarch64-low.cc
> @@ -576,41 +576,9 @@ aarch64_target::low_stopped_data_address ()
>
> /* Check if the address matches any watched address. */
> state = aarch64_get_debug_reg_state (pid_of (current_thread));
> - for (i = aarch64_num_wp_regs - 1; i >= 0; --i)
> - {
> - const unsigned int offset
> - = aarch64_watchpoint_offset (state->dr_ctrl_wp[i]);
> - const unsigned int len = aarch64_watchpoint_length (state->dr_ctrl_wp[i]);
> - const CORE_ADDR addr_watch = state->dr_addr_wp[i] + offset;
> - const CORE_ADDR addr_watch_aligned = align_down (state->dr_addr_wp[i], 8);
> - const CORE_ADDR addr_orig = state->dr_addr_orig_wp[i];
> -
> - if (state->dr_ref_count_wp[i]
> - && DR_CONTROL_ENABLED (state->dr_ctrl_wp[i])
> - && addr_trap >= addr_watch_aligned
> - && addr_trap < addr_watch + len)
> - {
> - /* ADDR_TRAP reports the first address of the memory range
> - accessed by the CPU, regardless of what was the memory
> - range watched. Thus, a large CPU access that straddles
> - the ADDR_WATCH..ADDR_WATCH+LEN range may result in an
> - ADDR_TRAP that is lower than the
> - ADDR_WATCH..ADDR_WATCH+LEN range. E.g.:
> -
> - addr: | 4 | 5 | 6 | 7 | 8 |
> - |---- range watched ----|
> - |----------- range accessed ------------|
> -
> - In this case, ADDR_TRAP will be 4.
> -
> - To match a watchpoint known to GDB core, we must never
> - report *ADDR_P outside of any ADDR_WATCH..ADDR_WATCH+LEN
> - range. ADDR_WATCH <= ADDR_TRAP < ADDR_ORIG is a false
> - positive on kernels older than 4.10. See PR
> - external/20207. */
> - return addr_orig;
> - }
> - }
> + CORE_ADDR result;
> + if (aarch64_stopped_data_address (state, addr_trap, &result))
> + return result;
>
> return (CORE_ADDR) 0;
> }
Hi,
On Mon, Apr 22, 2024 at 07:51:49AM -0600, Tom Tromey wrote:
> diff --git a/gdbserver/linux-aarch64-low.cc b/gdbserver/linux-aarch64-low.cc
> index 5df67fccd08..da5c1fd0629 100644
> --- a/gdbserver/linux-aarch64-low.cc
> +++ b/gdbserver/linux-aarch64-low.cc
> @@ -576,41 +576,9 @@ aarch64_target::low_stopped_data_address ()
>
> /* Check if the address matches any watched address. */
> state = aarch64_get_debug_reg_state (pid_of (current_thread));
> - for (i = aarch64_num_wp_regs - 1; i >= 0; --i)
> - {
> - const unsigned int offset
> - = aarch64_watchpoint_offset (state->dr_ctrl_wp[i]);
> - const unsigned int len = aarch64_watchpoint_length (state->dr_ctrl_wp[i]);
> - const CORE_ADDR addr_watch = state->dr_addr_wp[i] + offset;
> - const CORE_ADDR addr_watch_aligned = align_down (state->dr_addr_wp[i], 8);
> - const CORE_ADDR addr_orig = state->dr_addr_orig_wp[i];
> -
> - if (state->dr_ref_count_wp[i]
> - && DR_CONTROL_ENABLED (state->dr_ctrl_wp[i])
> - && addr_trap >= addr_watch_aligned
> - && addr_trap < addr_watch + len)
> - {
> - /* ADDR_TRAP reports the first address of the memory range
> - accessed by the CPU, regardless of what was the memory
> - range watched. Thus, a large CPU access that straddles
> - the ADDR_WATCH..ADDR_WATCH+LEN range may result in an
> - ADDR_TRAP that is lower than the
> - ADDR_WATCH..ADDR_WATCH+LEN range. E.g.:
> -
> - addr: | 4 | 5 | 6 | 7 | 8 |
> - |---- range watched ----|
> - |----------- range accessed ------------|
> -
> - In this case, ADDR_TRAP will be 4.
> -
> - To match a watchpoint known to GDB core, we must never
> - report *ADDR_P outside of any ADDR_WATCH..ADDR_WATCH+LEN
> - range. ADDR_WATCH <= ADDR_TRAP < ADDR_ORIG is a false
> - positive on kernels older than 4.10. See PR
> - external/20207. */
> - return addr_orig;
> - }
> - }
> + CORE_ADDR result;
> + if (aarch64_stopped_data_address (state, addr_trap, &result))
> + return result;
>
> return (CORE_ADDR) 0;
> }
This broke the build on the gdb-fedora-arm64 buildbot:
https://builder.sourceware.org/buildbot/#/builders/181/builds/6402
Fix attached.
Cheers,
Mark
From f27b8de72a42650a948fef0adc17e82517c14c2a Mon Sep 17 00:00:00 2001
From: Mark Wielaard <mark@klomp.org>
Date: Thu, 2 May 2024 23:58:20 +0200
Subject: [PATCH] Fix gdbserver/linux-aarch64-low.cc build
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Commit 0ee25f97d21e ("Fix regression on aarch64-linux gdbserver")
removed the last use of i in gdbserver/linux-aarch64-low.cc
(aarch64_target::low_stopped_data_address). Breaking the build on
aarch64 with:
gdbserver/linux-aarch64-low.cc: In member function ‘virtual CORE_ADDR aarch64_target::low_stopped_data_address()’:
gdbserver/linux-aarch64-low.cc:557:12: error: unused variable ‘i’ [-Werror=unused-variable]
557 | int pid, i;
| ^
cc1plus: all warnings being treated as errors
Fix this by removing the variable i completely.
Fixes: 0ee25f97d21e ("Fix regression on aarch64-linux gdbserver")
---
gdbserver/linux-aarch64-low.cc | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/gdbserver/linux-aarch64-low.cc b/gdbserver/linux-aarch64-low.cc
index da5c1fd06298..eb30c31f8f80 100644
--- a/gdbserver/linux-aarch64-low.cc
+++ b/gdbserver/linux-aarch64-low.cc
@@ -554,7 +554,7 @@ CORE_ADDR
aarch64_target::low_stopped_data_address ()
{
siginfo_t siginfo;
- int pid, i;
+ int pid;
struct aarch64_debug_reg_state *state;
pid = lwpid_of (current_thread);
On 5/3/24 00:07, Mark Wielaard wrote:
> Hi,
>
> On Mon, Apr 22, 2024 at 07:51:49AM -0600, Tom Tromey wrote:
>> diff --git a/gdbserver/linux-aarch64-low.cc b/gdbserver/linux-aarch64-low.cc
>> index 5df67fccd08..da5c1fd0629 100644
>> --- a/gdbserver/linux-aarch64-low.cc
>> +++ b/gdbserver/linux-aarch64-low.cc
>> @@ -576,41 +576,9 @@ aarch64_target::low_stopped_data_address ()
>>
>> /* Check if the address matches any watched address. */
>> state = aarch64_get_debug_reg_state (pid_of (current_thread));
>> - for (i = aarch64_num_wp_regs - 1; i >= 0; --i)
>> - {
>> - const unsigned int offset
>> - = aarch64_watchpoint_offset (state->dr_ctrl_wp[i]);
>> - const unsigned int len = aarch64_watchpoint_length (state->dr_ctrl_wp[i]);
>> - const CORE_ADDR addr_watch = state->dr_addr_wp[i] + offset;
>> - const CORE_ADDR addr_watch_aligned = align_down (state->dr_addr_wp[i], 8);
>> - const CORE_ADDR addr_orig = state->dr_addr_orig_wp[i];
>> -
>> - if (state->dr_ref_count_wp[i]
>> - && DR_CONTROL_ENABLED (state->dr_ctrl_wp[i])
>> - && addr_trap >= addr_watch_aligned
>> - && addr_trap < addr_watch + len)
>> - {
>> - /* ADDR_TRAP reports the first address of the memory range
>> - accessed by the CPU, regardless of what was the memory
>> - range watched. Thus, a large CPU access that straddles
>> - the ADDR_WATCH..ADDR_WATCH+LEN range may result in an
>> - ADDR_TRAP that is lower than the
>> - ADDR_WATCH..ADDR_WATCH+LEN range. E.g.:
>> -
>> - addr: | 4 | 5 | 6 | 7 | 8 |
>> - |---- range watched ----|
>> - |----------- range accessed ------------|
>> -
>> - In this case, ADDR_TRAP will be 4.
>> -
>> - To match a watchpoint known to GDB core, we must never
>> - report *ADDR_P outside of any ADDR_WATCH..ADDR_WATCH+LEN
>> - range. ADDR_WATCH <= ADDR_TRAP < ADDR_ORIG is a false
>> - positive on kernels older than 4.10. See PR
>> - external/20207. */
>> - return addr_orig;
>> - }
>> - }
>> + CORE_ADDR result;
>> + if (aarch64_stopped_data_address (state, addr_trap, &result))
>> + return result;
>>
>> return (CORE_ADDR) 0;
>> }
>
> This broke the build on the gdb-fedora-arm64 buildbot:
> https://builder.sourceware.org/buildbot/#/builders/181/builds/6402
>
> Fix attached.
>
Hi Mark,
thanks for the fix.
I ran into the build breaker on an aarch64-linux system, and managed to
complete the build after applying your fix.
Pushed.
Thanks,
- Tom
>>>>> "Mark" == Mark Wielaard <mark@klomp.org> writes:
Mark> This broke the build on the gdb-fedora-arm64 buildbot:
Mark> https://builder.sourceware.org/buildbot/#/builders/181/builds/6402
Sorry about that. Somehow it's escaped my notice that the AdaCore
internal builds disable -Werror, which is why I didn't notice this.
Tom
@@ -224,121 +224,6 @@ aarch64_remove_watchpoint (CORE_ADDR addr, int len, enum target_hw_bp_type type,
return ret;
}
-/* See aarch64-nat.h. */
-
-bool
-aarch64_stopped_data_address (const struct aarch64_debug_reg_state *state,
- CORE_ADDR addr_trap, CORE_ADDR *addr_p)
-{
- bool found = false;
- for (int phase = 0; phase <= 1; ++phase)
- for (int i = aarch64_num_wp_regs - 1; i >= 0; --i)
- {
- if (!(state->dr_ref_count_wp[i]
- && DR_CONTROL_ENABLED (state->dr_ctrl_wp[i])))
- {
- /* Watchpoint disabled. */
- continue;
- }
-
- const enum target_hw_bp_type type
- = aarch64_watchpoint_type (state->dr_ctrl_wp[i]);
- if (type == hw_execute)
- {
- /* Watchpoint disabled. */
- continue;
- }
-
- if (phase == 0)
- {
- /* Phase 0: No hw_write. */
- if (type == hw_write)
- continue;
- }
- else
- {
- /* Phase 1: Only hw_write. */
- if (type != hw_write)
- continue;
- }
-
- const unsigned int offset
- = aarch64_watchpoint_offset (state->dr_ctrl_wp[i]);
- const unsigned int len
- = aarch64_watchpoint_length (state->dr_ctrl_wp[i]);
- const CORE_ADDR addr_watch = state->dr_addr_wp[i] + offset;
- const CORE_ADDR addr_watch_aligned
- = align_down (state->dr_addr_wp[i], AARCH64_HWP_MAX_LEN_PER_REG);
- const CORE_ADDR addr_orig = state->dr_addr_orig_wp[i];
-
- /* ADDR_TRAP reports the first address of the memory range
- accessed by the CPU, regardless of what was the memory
- range watched. Thus, a large CPU access that straddles
- the ADDR_WATCH..ADDR_WATCH+LEN range may result in an
- ADDR_TRAP that is lower than the
- ADDR_WATCH..ADDR_WATCH+LEN range. E.g.:
-
- addr: | 4 | 5 | 6 | 7 | 8 |
- |---- range watched ----|
- |----------- range accessed ------------|
-
- In this case, ADDR_TRAP will be 4.
-
- The access size also can be larger than that of the watchpoint
- itself. For instance, the access size of an stp instruction is 16.
- So, if we use stp to store to address p, and set a watchpoint on
- address p + 8, the reported ADDR_TRAP can be p + 8 (observed on
- RK3399 SOC). But it also can be p (observed on M1 SOC). Checking
- for this situation introduces the possibility of false positives,
- so we only do this for hw_write watchpoints. */
- const CORE_ADDR max_access_size = type == hw_write ? 16 : 8;
- const CORE_ADDR addr_watch_base = addr_watch_aligned -
- (max_access_size - AARCH64_HWP_MAX_LEN_PER_REG);
- if (!(addr_trap >= addr_watch_base
- && addr_trap < addr_watch + len))
- {
- /* Not a match. */
- continue;
- }
-
- /* To match a watchpoint known to GDB core, we must never
- report *ADDR_P outside of any ADDR_WATCH..ADDR_WATCH+LEN
- range. ADDR_WATCH <= ADDR_TRAP < ADDR_ORIG is a false
- positive on kernels older than 4.10. See PR
- external/20207. */
- if (addr_p != nullptr)
- *addr_p = addr_orig;
-
- if (phase == 0)
- {
- /* Phase 0: Return first match. */
- return true;
- }
-
- /* Phase 1. */
- if (addr_p == nullptr)
- {
- /* First match, and we don't need to report an address. No need
- to look for other matches. */
- return true;
- }
-
- if (!found)
- {
- /* First match, and we need to report an address. Look for other
- matches. */
- found = true;
- continue;
- }
-
- /* More than one match, and we need to return an address. No need to
- look for further matches. */
- return false;
- }
-
- return found;
-}
-
/* Define AArch64 maintenance commands. */
static void
@@ -45,14 +45,6 @@ struct aarch64_debug_reg_state *aarch64_get_debug_reg_state (pid_t pid);
void aarch64_remove_debug_reg_state (pid_t pid);
-/* Helper for the "stopped_data_address" target method. Returns TRUE
- if a hardware watchpoint trap at ADDR_TRAP matches a set
- watchpoint. The address of the matched watchpoint is returned in
- *ADDR_P. */
-
-bool aarch64_stopped_data_address (const struct aarch64_debug_reg_state *state,
- CORE_ADDR addr_trap, CORE_ADDR *addr_p);
-
/* Helper functions used by aarch64_nat_target below. See their
definitions. */
@@ -645,3 +645,118 @@ aarch64_region_ok_for_watchpoint (CORE_ADDR addr, int len)
the checking is costly. */
return 1;
}
+
+/* See nat/aarch64-hw-point.h. */
+
+bool
+aarch64_stopped_data_address (const struct aarch64_debug_reg_state *state,
+ CORE_ADDR addr_trap, CORE_ADDR *addr_p)
+{
+ bool found = false;
+ for (int phase = 0; phase <= 1; ++phase)
+ for (int i = aarch64_num_wp_regs - 1; i >= 0; --i)
+ {
+ if (!(state->dr_ref_count_wp[i]
+ && DR_CONTROL_ENABLED (state->dr_ctrl_wp[i])))
+ {
+ /* Watchpoint disabled. */
+ continue;
+ }
+
+ const enum target_hw_bp_type type
+ = aarch64_watchpoint_type (state->dr_ctrl_wp[i]);
+ if (type == hw_execute)
+ {
+ /* Watchpoint disabled. */
+ continue;
+ }
+
+ if (phase == 0)
+ {
+ /* Phase 0: No hw_write. */
+ if (type == hw_write)
+ continue;
+ }
+ else
+ {
+ /* Phase 1: Only hw_write. */
+ if (type != hw_write)
+ continue;
+ }
+
+ const unsigned int offset
+ = aarch64_watchpoint_offset (state->dr_ctrl_wp[i]);
+ const unsigned int len
+ = aarch64_watchpoint_length (state->dr_ctrl_wp[i]);
+ const CORE_ADDR addr_watch = state->dr_addr_wp[i] + offset;
+ const CORE_ADDR addr_watch_aligned
+ = align_down (state->dr_addr_wp[i], AARCH64_HWP_MAX_LEN_PER_REG);
+ const CORE_ADDR addr_orig = state->dr_addr_orig_wp[i];
+
+ /* ADDR_TRAP reports the first address of the memory range
+ accessed by the CPU, regardless of what was the memory
+ range watched. Thus, a large CPU access that straddles
+ the ADDR_WATCH..ADDR_WATCH+LEN range may result in an
+ ADDR_TRAP that is lower than the
+ ADDR_WATCH..ADDR_WATCH+LEN range. E.g.:
+
+ addr: | 4 | 5 | 6 | 7 | 8 |
+ |---- range watched ----|
+ |----------- range accessed ------------|
+
+ In this case, ADDR_TRAP will be 4.
+
+ The access size also can be larger than that of the watchpoint
+ itself. For instance, the access size of an stp instruction is 16.
+ So, if we use stp to store to address p, and set a watchpoint on
+ address p + 8, the reported ADDR_TRAP can be p + 8 (observed on
+ RK3399 SOC). But it also can be p (observed on M1 SOC). Checking
+ for this situation introduces the possibility of false positives,
+ so we only do this for hw_write watchpoints. */
+ const CORE_ADDR max_access_size = type == hw_write ? 16 : 8;
+ const CORE_ADDR addr_watch_base = addr_watch_aligned -
+ (max_access_size - AARCH64_HWP_MAX_LEN_PER_REG);
+ if (!(addr_trap >= addr_watch_base
+ && addr_trap < addr_watch + len))
+ {
+ /* Not a match. */
+ continue;
+ }
+
+ /* To match a watchpoint known to GDB core, we must never
+ report *ADDR_P outside of any ADDR_WATCH..ADDR_WATCH+LEN
+ range. ADDR_WATCH <= ADDR_TRAP < ADDR_ORIG is a false
+ positive on kernels older than 4.10. See PR
+ external/20207. */
+ if (addr_p != nullptr)
+ *addr_p = addr_orig;
+
+ if (phase == 0)
+ {
+ /* Phase 0: Return first match. */
+ return true;
+ }
+
+ /* Phase 1. */
+ if (addr_p == nullptr)
+ {
+ /* First match, and we don't need to report an address. No need
+ to look for other matches. */
+ return true;
+ }
+
+ if (!found)
+ {
+ /* First match, and we need to report an address. Look for other
+ matches. */
+ found = true;
+ continue;
+ }
+
+ /* More than one match, and we need to return an address. No need to
+ look for further matches. */
+ return false;
+ }
+
+ return found;
+}
@@ -110,6 +110,14 @@ unsigned int aarch64_watchpoint_offset (unsigned int ctrl);
unsigned int aarch64_watchpoint_length (unsigned int ctrl);
enum target_hw_bp_type aarch64_watchpoint_type (unsigned int ctrl);
+/* Helper for the "stopped_data_address" target method. Returns TRUE
+ if a hardware watchpoint trap at ADDR_TRAP matches a set
+ watchpoint. The address of the matched watchpoint is returned in
+ *ADDR_P. */
+
+bool aarch64_stopped_data_address (const struct aarch64_debug_reg_state *state,
+ CORE_ADDR addr_trap, CORE_ADDR *addr_p);
+
int aarch64_handle_breakpoint (enum target_hw_bp_type type, CORE_ADDR addr,
int len, int is_insert, ptid_t ptid,
struct aarch64_debug_reg_state *state);
@@ -576,41 +576,9 @@ aarch64_target::low_stopped_data_address ()
/* Check if the address matches any watched address. */
state = aarch64_get_debug_reg_state (pid_of (current_thread));
- for (i = aarch64_num_wp_regs - 1; i >= 0; --i)
- {
- const unsigned int offset
- = aarch64_watchpoint_offset (state->dr_ctrl_wp[i]);
- const unsigned int len = aarch64_watchpoint_length (state->dr_ctrl_wp[i]);
- const CORE_ADDR addr_watch = state->dr_addr_wp[i] + offset;
- const CORE_ADDR addr_watch_aligned = align_down (state->dr_addr_wp[i], 8);
- const CORE_ADDR addr_orig = state->dr_addr_orig_wp[i];
-
- if (state->dr_ref_count_wp[i]
- && DR_CONTROL_ENABLED (state->dr_ctrl_wp[i])
- && addr_trap >= addr_watch_aligned
- && addr_trap < addr_watch + len)
- {
- /* ADDR_TRAP reports the first address of the memory range
- accessed by the CPU, regardless of what was the memory
- range watched. Thus, a large CPU access that straddles
- the ADDR_WATCH..ADDR_WATCH+LEN range may result in an
- ADDR_TRAP that is lower than the
- ADDR_WATCH..ADDR_WATCH+LEN range. E.g.:
-
- addr: | 4 | 5 | 6 | 7 | 8 |
- |---- range watched ----|
- |----------- range accessed ------------|
-
- In this case, ADDR_TRAP will be 4.
-
- To match a watchpoint known to GDB core, we must never
- report *ADDR_P outside of any ADDR_WATCH..ADDR_WATCH+LEN
- range. ADDR_WATCH <= ADDR_TRAP < ADDR_ORIG is a false
- positive on kernels older than 4.10. See PR
- external/20207. */
- return addr_orig;
- }
- }
+ CORE_ADDR result;
+ if (aarch64_stopped_data_address (state, addr_trap, &result))
+ return result;
return (CORE_ADDR) 0;
}