diff options
38 files changed, 645 insertions, 350 deletions
@@ -209,6 +209,12 @@ vFile:stat lstat rather than stat. This has now been corrected. The documentation has also been clarified. +T + The signal stop packet can now include multiple 'watch', 'rwatch', + and 'awatch' stop reason entries. GDB will select between all of + the possible watchpoint addresses that are return when presenting + the stop to the user. + * MI changes ** The =library-unloaded event now includes the 'ranges' field, which diff --git a/gdb/aarch64-fbsd-nat.c b/gdb/aarch64-fbsd-nat.c index ecf0bb2..c3b3205 100644 --- a/gdb/aarch64-fbsd-nat.c +++ b/gdb/aarch64-fbsd-nat.c @@ -57,7 +57,7 @@ struct aarch64_fbsd_nat_target final : public fbsd_nat_target #ifdef HAVE_DBREG /* Hardware breakpoints and watchpoints. */ bool stopped_by_watchpoint () override; - bool stopped_data_address (CORE_ADDR *) override; + std::vector<CORE_ADDR> stopped_data_addresses () override; bool stopped_by_hw_breakpoint () override; bool supports_stopped_by_hw_breakpoint () override; @@ -134,28 +134,28 @@ bool aarch64_fbsd_nat_target::debug_regs_probed; static std::unordered_set<lwpid_t> aarch64_debug_pending_threads; -/* Implement the "stopped_data_address" target_ops method. */ +/* Implement the "stopped_data_addresses" target_ops method. */ -bool -aarch64_fbsd_nat_target::stopped_data_address (CORE_ADDR *addr_p) +std::vector<CORE_ADDR> +aarch64_fbsd_nat_target::stopped_data_addresses () { siginfo_t siginfo; struct aarch64_debug_reg_state *state; if (!fbsd_nat_get_siginfo (inferior_ptid, &siginfo)) - return false; + return {}; /* This must be a hardware breakpoint. */ if (siginfo.si_signo != SIGTRAP || siginfo.si_code != TRAP_TRACE || siginfo.si_trapno != EXCP_WATCHPT_EL0) - return false; + return {}; const CORE_ADDR addr_trap = (CORE_ADDR) siginfo.si_addr; /* Check if the address matches any watched address. */ state = aarch64_get_debug_reg_state (inferior_ptid.pid ()); - return aarch64_stopped_data_address (state, addr_trap, addr_p); + return aarch64_stopped_data_addresses (state, addr_trap); } /* Implement the "stopped_by_watchpoint" target_ops method. */ @@ -163,7 +163,7 @@ aarch64_fbsd_nat_target::stopped_data_address (CORE_ADDR *addr_p) bool aarch64_fbsd_nat_target::stopped_by_watchpoint () { - return stopped_data_address (nullptr); + return !stopped_data_addresses ().empty (); } /* Implement the "stopped_by_hw_breakpoint" target_ops method. */ diff --git a/gdb/aarch64-linux-nat.c b/gdb/aarch64-linux-nat.c index 725c632..87176ab 100644 --- a/gdb/aarch64-linux-nat.c +++ b/gdb/aarch64-linux-nat.c @@ -74,7 +74,7 @@ public: /* Add our hardware breakpoint and watchpoint implementation. */ bool stopped_by_watchpoint () override; - bool stopped_data_address (CORE_ADDR *) override; + std::vector<CORE_ADDR> stopped_data_addresses () override; int can_do_single_step () override; @@ -922,21 +922,21 @@ aarch64_linux_nat_target::low_siginfo_fixup (siginfo_t *native, gdb_byte *inf, return false; } -/* Implement the "stopped_data_address" target_ops method. */ +/* Implement the "stopped_data_addresses" target_ops method. */ -bool -aarch64_linux_nat_target::stopped_data_address (CORE_ADDR *addr_p) +std::vector<CORE_ADDR> +aarch64_linux_nat_target::stopped_data_addresses () { siginfo_t siginfo; struct aarch64_debug_reg_state *state; if (!linux_nat_get_siginfo (inferior_ptid, &siginfo)) - return false; + return {}; /* This must be a hardware breakpoint. */ if (siginfo.si_signo != SIGTRAP || (siginfo.si_code & 0xffff) != TRAP_HWBKPT) - return false; + return {}; /* Make sure to ignore the top byte, otherwise we may not recognize a hardware watchpoint hit. The stopped data addresses coming from the @@ -947,7 +947,7 @@ aarch64_linux_nat_target::stopped_data_address (CORE_ADDR *addr_p) /* Check if the address matches any watched address. */ state = aarch64_get_debug_reg_state (inferior_ptid.pid ()); - return aarch64_stopped_data_address (state, addr_trap, addr_p); + return aarch64_stopped_data_addresses (state, addr_trap); } /* Implement the "stopped_by_watchpoint" target_ops method. */ @@ -955,7 +955,7 @@ aarch64_linux_nat_target::stopped_data_address (CORE_ADDR *addr_p) bool aarch64_linux_nat_target::stopped_by_watchpoint () { - return stopped_data_address (nullptr); + return !stopped_data_addresses ().empty (); } /* Implement the "can_do_single_step" target_ops method. */ diff --git a/gdb/arm-linux-nat.c b/gdb/arm-linux-nat.c index 813da8c..927ec9d 100644 --- a/gdb/arm-linux-nat.c +++ b/gdb/arm-linux-nat.c @@ -88,7 +88,7 @@ public: struct expression *) override; bool stopped_by_watchpoint () override; - bool stopped_data_address (CORE_ADDR *) override; + std::vector<CORE_ADDR> stopped_data_addresses () override; bool watchpoint_addr_within_range (CORE_ADDR, CORE_ADDR, int) override; @@ -1169,41 +1169,37 @@ arm_linux_nat_target::remove_watchpoint (CORE_ADDR addr, } /* What was the data address the target was stopped on accessing. */ -bool -arm_linux_nat_target::stopped_data_address (CORE_ADDR *addr_p) +std::vector<CORE_ADDR> +arm_linux_nat_target::stopped_data_addresses () { siginfo_t siginfo; - int slot; - if (!linux_nat_get_siginfo (inferior_ptid, &siginfo)) - return false; + return {}; /* This must be a hardware breakpoint. */ if (siginfo.si_signo != SIGTRAP || (siginfo.si_code & 0xffff) != 0x0004 /* TRAP_HWBKPT */) - return false; + return {}; /* We must be able to set hardware watchpoints. */ if (arm_linux_get_hw_watchpoint_count () == 0) - return 0; + return {}; - slot = siginfo.si_errno; + int slot = siginfo.si_errno; /* If we are in a positive slot then we're looking at a breakpoint and not a watchpoint. */ if (slot >= 0) - return false; + return {}; - *addr_p = (CORE_ADDR) (uintptr_t) siginfo.si_addr; - return true; + return { (CORE_ADDR) (uintptr_t) siginfo.si_addr }; } /* Has the target been stopped by hitting a watchpoint? */ bool arm_linux_nat_target::stopped_by_watchpoint () { - CORE_ADDR addr; - return stopped_data_address (&addr); + return !stopped_data_addresses ().empty (); } bool diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c index d704ad1..2b84dc8 100644 --- a/gdb/breakpoint.c +++ b/gdb/breakpoint.c @@ -2140,7 +2140,7 @@ add_dummy_location (struct breakpoint *b, The following constraints influence the location where we can reset hardware watchpoints: - * target_stopped_by_watchpoint and target_stopped_data_address are + * target_stopped_by_watchpoint and target_stopped_data_addresses are called several times when GDB stops. [linux] @@ -5243,10 +5243,7 @@ bpstat::bpstat () int watchpoints_triggered (const target_waitstatus &ws) { - bool stopped_by_watchpoint = target_stopped_by_watchpoint (); - CORE_ADDR addr; - - if (!stopped_by_watchpoint) + if (!target_stopped_by_watchpoint ()) { /* We were not stopped by a watchpoint. Mark all watchpoints as not triggered. */ @@ -5261,7 +5258,9 @@ watchpoints_triggered (const target_waitstatus &ws) return 0; } - if (!target_stopped_data_address (current_inferior ()->top_target (), &addr)) + std::vector<CORE_ADDR> addr_list + = target_stopped_data_addresses (current_inferior ()->top_target ()); + if (addr_list.empty ()) { /* We were stopped by a watchpoint, but we don't know where. Mark all watchpoints as unknown. */ @@ -5279,36 +5278,44 @@ watchpoints_triggered (const target_waitstatus &ws) /* The target could report the data address. Mark watchpoints affected by this data address as triggered, and all others as not triggered. */ - for (breakpoint &b : all_breakpoints ()) if (is_hardware_watchpoint (&b)) { watchpoint &w = gdb::checked_static_cast<watchpoint &> (b); - w.watchpoint_triggered = watch_triggered_no; - for (bp_location &loc : b.locations ()) + } + + for (const CORE_ADDR addr : addr_list) + { + for (breakpoint &b : all_breakpoints ()) + if (is_hardware_watchpoint (&b)) { - if (is_masked_watchpoint (&b)) + watchpoint &w = gdb::checked_static_cast<watchpoint &> (b); + + for (bp_location &loc : b.locations ()) { - CORE_ADDR newaddr = addr & w.hw_wp_mask; - CORE_ADDR start = loc.address & w.hw_wp_mask; + if (is_masked_watchpoint (&b)) + { + CORE_ADDR newaddr = addr & w.hw_wp_mask; + CORE_ADDR start = loc.address & w.hw_wp_mask; - if (newaddr == start) + if (newaddr == start) + { + w.watchpoint_triggered = watch_triggered_yes; + break; + } + } + /* Exact match not required. Within range is sufficient. */ + else if (target_watchpoint_addr_within_range + (current_inferior ()->top_target (), addr, loc.address, + loc.length)) { w.watchpoint_triggered = watch_triggered_yes; break; } } - /* Exact match not required. Within range is sufficient. */ - else if (target_watchpoint_addr_within_range - (current_inferior ()->top_target (), addr, loc.address, - loc.length)) - { - w.watchpoint_triggered = watch_triggered_yes; - break; - } } - } + } return 1; } @@ -5405,7 +5412,7 @@ watchpoint_check (bpstat *bs) if (is_masked_watchpoint (b)) /* Since we don't know the exact trigger address (from - stopped_data_address), just tell the user we've triggered + stopped_data_addresses), just tell the user we've triggered a mask watchpoint. */ return WP_VALUE_CHANGED; diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo index 2bbaf14..3673624 100644 --- a/gdb/doc/gdb.texinfo +++ b/gdb/doc/gdb.texinfo @@ -43909,8 +43909,15 @@ The currently defined stop reasons are: @item watch @itemx rwatch @itemx awatch -The packet indicates a watchpoint hit, and @var{r} is the data address, in -hex. +The packet indicates a watchpoint hit, and @var{r} is the data +address, in hex. + +Some targets, for example AArch64, are unable to accurately report the +address which triggered a watchpoint trap. As a consequence, multiple +watched addresses could explain a single watchpoint trap. In these +cases, multiple instances of these stop reasons can appear in a single +stop packet. @value{GDBN} will select between the multiple reported +stop addresses when displaying the stop to the user. @item syscall_entry @itemx syscall_return diff --git a/gdb/ia64-linux-nat.c b/gdb/ia64-linux-nat.c index e833f9d..891cbad 100644 --- a/gdb/ia64-linux-nat.c +++ b/gdb/ia64-linux-nat.c @@ -72,7 +72,7 @@ public: int can_use_hw_breakpoint (enum bptype, int, int) override; bool stopped_by_watchpoint () override; - bool stopped_data_address (CORE_ADDR *) override; + std::vector<CORE_ADDR> stopped_data_addresses () override; int insert_watchpoint (CORE_ADDR, int, enum target_hw_bp_type, struct expression *) override; int remove_watchpoint (CORE_ADDR, int, enum target_hw_bp_type, @@ -686,34 +686,32 @@ ia64_linux_nat_target::low_new_thread (struct lwp_info *lp) enable_watchpoints_in_psr (lp->ptid); } -bool -ia64_linux_nat_target::stopped_data_address (CORE_ADDR *addr_p) +std::vector<CORE_ADDR> +ia64_linux_nat_target::stopped_data_addresses () { CORE_ADDR psr; siginfo_t siginfo; regcache *regcache = get_thread_regcache (inferior_thread ()); if (!linux_nat_get_siginfo (inferior_ptid, &siginfo)) - return false; + return {}; if (siginfo.si_signo != SIGTRAP || (siginfo.si_code & 0xffff) != 0x0004 /* TRAP_HWBKPT */) - return false; + return {}; regcache_cooked_read_unsigned (regcache, IA64_PSR_REGNUM, &psr); psr |= IA64_PSR_DD; /* Set the dd bit - this will disable the watchpoint for the next instruction. */ regcache_cooked_write_unsigned (regcache, IA64_PSR_REGNUM, psr); - *addr_p = (CORE_ADDR) siginfo.si_addr; - return true; + return { (CORE_ADDR) siginfo.si_addr }; } bool ia64_linux_nat_target::stopped_by_watchpoint () { - CORE_ADDR addr; - return stopped_data_address (&addr); + return !stopped_data_addresses ().empty (); } int diff --git a/gdb/infrun.c b/gdb/infrun.c index 9d3e1b7..32c6a34 100644 --- a/gdb/infrun.c +++ b/gdb/infrun.c @@ -6889,16 +6889,26 @@ handle_signal_stop (struct execution_control_state *ecs) ("stop_pc=%s", paddress (reg_gdbarch, ecs->event_thread->stop_pc ())); if (target_stopped_by_watchpoint ()) { - CORE_ADDR addr; + auto inf_target = current_inferior ()->top_target (); + std::vector<CORE_ADDR> addr_list + = target_stopped_data_addresses (inf_target); - infrun_debug_printf ("stopped by watchpoint"); - - if (target_stopped_data_address (current_inferior ()->top_target (), - &addr)) - infrun_debug_printf ("stopped data address=%s", - paddress (reg_gdbarch, addr)); + std::string addr_str; + if (addr_list.empty ()) + addr_str = "(no data addressses available)"; else - infrun_debug_printf ("(no data address available)"); + { + for (const CORE_ADDR addr : addr_list) + { + if (addr_str.length () > 0) + addr_str += ", "; + + addr_str += paddress (reg_gdbarch, addr); + } + } + + infrun_debug_printf ("stopped by watchpoint, data addresses = %s", + addr_str.c_str ()); } } diff --git a/gdb/linux-nat.c b/gdb/linux-nat.c index f317927..ab0092a 100644 --- a/gdb/linux-nat.c +++ b/gdb/linux-nat.c @@ -2515,16 +2515,17 @@ linux_nat_target::stopped_by_watchpoint () return lp->stop_reason == TARGET_STOPPED_BY_WATCHPOINT; } -bool -linux_nat_target::stopped_data_address (CORE_ADDR *addr_p) +std::vector<CORE_ADDR> +linux_nat_target::stopped_data_addresses () { struct lwp_info *lp = find_lwp_pid (inferior_ptid); gdb_assert (lp != NULL); - *addr_p = lp->stopped_data_address; + if (lp->stopped_data_address_p) + return { lp->stopped_data_address }; - return lp->stopped_data_address_p; + return {}; } /* Commonly any breakpoint / watchpoint generate only SIGTRAP. */ diff --git a/gdb/linux-nat.h b/gdb/linux-nat.h index 7cbe9a9..0985b6f 100644 --- a/gdb/linux-nat.h +++ b/gdb/linux-nat.h @@ -70,7 +70,7 @@ public: bool stopped_by_watchpoint () override; - bool stopped_data_address (CORE_ADDR *) override; + std::vector<CORE_ADDR> stopped_data_addresses () override; bool stopped_by_sw_breakpoint () override; bool supports_stopped_by_sw_breakpoint () override; diff --git a/gdb/loongarch-linux-nat.c b/gdb/loongarch-linux-nat.c index 2b59b36..5a9ff94 100644 --- a/gdb/loongarch-linux-nat.c +++ b/gdb/loongarch-linux-nat.c @@ -77,7 +77,7 @@ public: /* Add our hardware breakpoint and watchpoint implementation. */ bool stopped_by_watchpoint () override; - bool stopped_data_address (CORE_ADDR *) override; + std::vector<CORE_ADDR> stopped_data_addresses () override; int insert_hw_breakpoint (struct gdbarch *gdbarch, struct bp_target_info *bp_tgt) override; @@ -590,26 +590,30 @@ loongarch_linux_nat_target::watchpoint_addr_within_range (CORE_ADDR addr, } -/* Implement the "stopped_data_address" target_ops method. */ +/* Implement the "stopped_data_addresses" target_ops method. */ -bool -loongarch_linux_nat_target::stopped_data_address (CORE_ADDR *addr_p) +std::vector<CORE_ADDR> +loongarch_linux_nat_target::stopped_data_addresses () { siginfo_t siginfo; struct loongarch_debug_reg_state *state; if (!linux_nat_get_siginfo (inferior_ptid, &siginfo)) - return false; + return {}; /* This must be a hardware breakpoint. */ if (siginfo.si_signo != SIGTRAP || (siginfo.si_code & 0xffff) != TRAP_HWBKPT) - return false; + return {}; /* Check if the address matches any watched address. */ state = loongarch_get_debug_reg_state (inferior_ptid.pid ()); - return - loongarch_stopped_data_address (state, (CORE_ADDR) siginfo.si_addr, addr_p); + CORE_ADDR addr; + if (loongarch_stopped_data_address (state, (CORE_ADDR) siginfo.si_addr, + &addr)) + return { addr }; + + return {}; } /* Implement the "stopped_by_watchpoint" target_ops method. */ @@ -617,9 +621,7 @@ loongarch_linux_nat_target::stopped_data_address (CORE_ADDR *addr_p) bool loongarch_linux_nat_target::stopped_by_watchpoint () { - CORE_ADDR addr; - - return stopped_data_address (&addr); + return !stopped_data_addresses ().empty (); } /* Insert a hardware-assisted breakpoint at BP_TGT->reqstd_address. diff --git a/gdb/mips-linux-nat.c b/gdb/mips-linux-nat.c index e22e904..ea00577 100644 --- a/gdb/mips-linux-nat.c +++ b/gdb/mips-linux-nat.c @@ -60,7 +60,7 @@ public: bool stopped_by_watchpoint () override; - bool stopped_data_address (CORE_ADDR *) override; + std::vector<CORE_ADDR> stopped_data_addresses () override; int region_ok_for_hw_watchpoint (CORE_ADDR, int) override; @@ -598,16 +598,17 @@ mips_linux_nat_target::stopped_by_watchpoint () return false; } -/* Target to_stopped_data_address implementation. Set the address - where the watch triggered (if known). Return 1 if the address was - known. */ +/* Target stopped_data_addresses implementation. Return a vector + containing the address(es) of the watchpoint(s) that triggered, if + known. Return an empty vector if it is unknown which watchpoint(s) + triggered. */ -bool -mips_linux_nat_target::stopped_data_address (CORE_ADDR *paddr) +std::vector<CORE_ADDR> +mips_linux_nat_target::stopped_data_addresses () { /* On mips we don't know the low order 3 bits of the data address, - so we must return false. */ - return false; + so we must return an empty vector. */ + return {}; } /* Target to_region_ok_for_hw_watchpoint implementation. Return 1 if diff --git a/gdb/nat/aarch64-hw-point.c b/gdb/nat/aarch64-hw-point.c index 6d8dce8..5d77a4a 100644 --- a/gdb/nat/aarch64-hw-point.c +++ b/gdb/nat/aarch64-hw-point.c @@ -215,14 +215,14 @@ aarch64_point_is_aligned (ptid_t ptid, int is_watchpoint, CORE_ADDR addr, Another limitation is that because the watched region is enlarged, the watchpoint fault address discovered by - aarch64_stopped_data_address may be outside of the original watched + aarch64_stopped_data_addresses may be outside of the original watched region, especially when the triggering instruction is accessing a larger region. When the fault address is not within any known range, watchpoints_triggered in gdb will get confused, as the higher-level watchpoint management is only aware of original watched regions, and will think that some unknown watchpoint has been triggered. To prevent such a case, - aarch64_stopped_data_address implementations in gdb and gdbserver + aarch64_stopped_data_addresses implementations in gdb and gdbserver try to match the trapped address with a watched region, and return an address within the latter. */ @@ -646,117 +646,110 @@ aarch64_region_ok_for_watchpoint (CORE_ADDR addr, int len) return 1; } +[[maybe_unused]] static void +apb_debug (const char *fmt, ...) +{ + va_list ap; + + if (getenv ("APB_DEBUG") == nullptr) + return; + + va_start (ap, fmt); + vfprintf (stderr, fmt, ap); + va_end (ap); +} + /* 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) +std::vector<CORE_ADDR> +aarch64_stopped_data_addresses (const struct aarch64_debug_reg_state *state, + CORE_ADDR addr_trap) { - 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.: + apb_debug ("APB: ---------- Enter: aarch64_stopped_data_addresses ----------\n"); + apb_debug ("APB: addr_trap = %s\n", core_addr_to_string_nz (addr_trap)); + + /* ... */ + std::vector<CORE_ADDR> matching_addresses; + + 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; + } + + 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 - + 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; + + + apb_debug ("APB: WP %d, %s..%s, addr_watch_aligned = %s, addr_watch = %s, addr_orig = %s, len = %d, offset = %d\n", + i, + core_addr_to_string_nz (addr_watch_base), + core_addr_to_string_nz (addr_watch + len), + core_addr_to_string_nz (addr_watch_aligned), core_addr_to_string_nz (addr_watch), + core_addr_to_string_nz (addr_orig), len, offset); + + + + if (!(addr_trap >= addr_watch_base + && addr_trap < addr_watch + len)) + { + /* Not a match. */ + continue; } - return found; + apb_debug ("APB: Match for %d, type: %s, range: %s..%s\n", + i, + (type == hw_write ? "write" : "access"), + core_addr_to_string_nz (addr_watch_base), + core_addr_to_string_nz (addr_watch + len)); + + matching_addresses.push_back (addr_orig); + } + + apb_debug ("APB: matching addresses:"); + for (CORE_ADDR &a : matching_addresses) + apb_debug (" %s", core_addr_to_string_nz (a)); + apb_debug ("\n"); + return matching_addresses; } diff --git a/gdb/nat/aarch64-hw-point.h b/gdb/nat/aarch64-hw-point.h index f8ab55f..a915597 100644 --- a/gdb/nat/aarch64-hw-point.h +++ b/gdb/nat/aarch64-hw-point.h @@ -110,13 +110,18 @@ 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); +/* Helper for the "stopped_data_addresses" target method. Returns a vector + containing the addresses of all hardware watchpoints that could account + for a watchpoint trap at ADDR_TRAP. Return an empty vector if no + suitable watchpoint addresses can be identified. + + It is possible that multiple watchpoints could account for a trap at + ADDR_TRAP, in which case all possible addresses are returned, and GDB + core is responsible for selecting a suitable watchpoint, or otherwise + letting the user know that there is some ambiguity. */ + +extern std::vector<CORE_ADDR> aarch64_stopped_data_addresses + (const struct aarch64_debug_reg_state *state, CORE_ADDR addr_trap); int aarch64_handle_breakpoint (enum target_hw_bp_type type, CORE_ADDR addr, int len, int is_insert, ptid_t ptid, diff --git a/gdb/nat/x86-dregs.c b/gdb/nat/x86-dregs.c index 4119214..d1a75a8 100644 --- a/gdb/nat/x86-dregs.c +++ b/gdb/nat/x86-dregs.c @@ -655,7 +655,7 @@ x86_dr_stopped_data_address (struct x86_debug_reg_state *state, /* This second condition makes sure DRi is set up for a data watchpoint, not a hardware breakpoint. The reason is that - GDB doesn't call the target_stopped_data_address method + GDB doesn't call the target_stopped_data_addresses method except for data watchpoints. In other words, I'm being paranoiac. */ if (X86_DR_GET_RW_LEN (control, i) != 0) diff --git a/gdb/procfs.c b/gdb/procfs.c index a10574a..45b69bd 100644 --- a/gdb/procfs.c +++ b/gdb/procfs.c @@ -159,7 +159,7 @@ public: int region_ok_for_hw_watchpoint (CORE_ADDR, int) override; int can_use_hw_breakpoint (enum bptype, int, int) override; - bool stopped_data_address (CORE_ADDR *) override; + std::vector<CORE_ADDR> stopped_data_addresses () override; void procfs_init_inferior (int pid); }; @@ -3045,19 +3045,19 @@ procfs_target::stopped_by_watchpoint () return false; } -/* Returns 1 if the OS knows the position of the triggered watchpoint, - and sets *ADDR to that address. Returns 0 if OS cannot report that - address. This function is only called if - procfs_stopped_by_watchpoint returned 1, thus no further checks are - done. The function also assumes that ADDR is not NULL. */ +/* Returns a vector containing the position of the triggered watchpoint. + Returns the empty vector if OS cannot report that address. This + function is only called if procfs_stopped_by_watchpoint returned 1, thus + no further checks are done. */ -bool -procfs_target::stopped_data_address (CORE_ADDR *addr) +std::vector<CORE_ADDR> +procfs_target::stopped_data_addresses () { - procinfo *pi; - - pi = find_procinfo_or_die (inferior_ptid.pid (), 0); - return proc_watchpoint_address (pi, addr); + procinfo *pi = find_procinfo_or_die (inferior_ptid.pid (), 0); + CORE_ADDR addr; + if (proc_watchpoint_address (pi, &addr)) + return { addr }; + return {}; } int diff --git a/gdb/ravenscar-thread.c b/gdb/ravenscar-thread.c index 4d79b3d..440541d 100644 --- a/gdb/ravenscar-thread.c +++ b/gdb/ravenscar-thread.c @@ -101,7 +101,7 @@ struct ravenscar_thread_target final : public target_ops bool stopped_by_watchpoint () override; - bool stopped_data_address (CORE_ADDR *) override; + std::vector<CORE_ADDR> stopped_data_addresses () override; enum target_xfer_status xfer_partial (enum target_object object, const char *annex, @@ -818,14 +818,14 @@ ravenscar_thread_target::stopped_by_watchpoint () return beneath ()->stopped_by_watchpoint (); } -/* Implement the to_stopped_data_address target_ops "method". */ +/* Implement the to_stopped_data_addresses target_ops "method". */ -bool -ravenscar_thread_target::stopped_data_address (CORE_ADDR *addr_p) +std::vector<CORE_ADDR> +ravenscar_thread_target::stopped_data_addresses () { scoped_restore_current_thread saver; set_base_thread_from_ravenscar_task (inferior_ptid); - return beneath ()->stopped_data_address (addr_p); + return beneath ()->stopped_data_addresses (); } void diff --git a/gdb/record-full.c b/gdb/record-full.c index 7e3da27..195ce8a 100644 --- a/gdb/record-full.c +++ b/gdb/record-full.c @@ -232,7 +232,7 @@ public: void async (bool) override; ptid_t wait (ptid_t, struct target_waitstatus *, target_wait_flags) override; bool stopped_by_watchpoint () override; - bool stopped_data_address (CORE_ADDR *) override; + std::vector<CORE_ADDR> stopped_data_addresses () override; bool stopped_by_sw_breakpoint () override; bool supports_stopped_by_sw_breakpoint () override; @@ -1501,13 +1501,13 @@ record_full_base_target::stopped_by_watchpoint () return beneath ()->stopped_by_watchpoint (); } -bool -record_full_base_target::stopped_data_address (CORE_ADDR *addr_p) +std::vector<CORE_ADDR> +record_full_base_target::stopped_data_addresses () { if (RECORD_FULL_IS_REPLAY) - return false; + return {}; else - return this->beneath ()->stopped_data_address (addr_p); + return this->beneath ()->stopped_data_addresses (); } /* The stopped_by_sw_breakpoint method of target record-full. */ diff --git a/gdb/remote.c b/gdb/remote.c index 6208a90..10d2da6 100644 --- a/gdb/remote.c +++ b/gdb/remote.c @@ -893,7 +893,7 @@ public: bool stopped_by_watchpoint () override; - bool stopped_data_address (CORE_ADDR *) override; + std::vector<CORE_ADDR> stopped_data_addresses () override; bool watchpoint_addr_within_range (CORE_ADDR, CORE_ADDR, int) override; @@ -1479,7 +1479,7 @@ struct stop_reply : public notif_event enum target_stop_reason stop_reason; - CORE_ADDR watch_data_address; + std::vector<CORE_ADDR> watch_data_address; int core; }; @@ -1637,9 +1637,13 @@ struct remote_thread_info : public private_thread_info /* Whether the target stopped for a breakpoint/watchpoint. */ enum target_stop_reason stop_reason = TARGET_STOPPED_BY_NO_REASON; - /* This is set to the data address of the access causing the target - to stop for a watchpoint. */ - CORE_ADDR watch_data_address = 0; + /* This is set to all the watchpoint addresses of the access causing the + target to stop for a watchpoint. For some targets (e.g. AArch64) + targets cannot watch small (e.g. single byte) regions, so multiple + watchpoints could account for a stop. All possible watchpoint + addresses are reported back to GDB, and GDB must select between + them. */ + std::vector<CORE_ADDR> watch_data_address; /* Get the thread's resume state. */ enum resume_state get_resume_state () const @@ -6881,7 +6885,7 @@ resume_clear_thread_private_info (struct thread_info *thread) remote_thread_info *priv = get_remote_thread_info (thread); priv->stop_reason = TARGET_STOPPED_BY_NO_REASON; - priv->watch_data_address = 0; + priv->watch_data_address.clear (); } } @@ -7497,7 +7501,7 @@ remote_target::remote_stop_ns (ptid_t ptid) sr->ws.set_stopped (GDB_SIGNAL_0); sr->arch = tp->inf->arch (); sr->stop_reason = TARGET_STOPPED_BY_NO_REASON; - sr->watch_data_address = 0; + sr->watch_data_address.clear (); sr->core = 0; this->push_stop_reply (std::move (sr)); @@ -8083,7 +8087,7 @@ Packet: '%s'\n"), { event->stop_reason = TARGET_STOPPED_BY_WATCHPOINT; p = unpack_varlen_hex (++p1, &addr); - event->watch_data_address = (CORE_ADDR) addr; + event->watch_data_address.push_back ((CORE_ADDR) addr); } else if (strprefix (p, p1, "swbreak")) { @@ -11401,20 +11405,16 @@ remote_target::stopped_by_watchpoint () == TARGET_STOPPED_BY_WATCHPOINT)); } -bool -remote_target::stopped_data_address (CORE_ADDR *addr_p) +std::vector<CORE_ADDR> +remote_target::stopped_data_addresses () { struct thread_info *thread = inferior_thread (); if (thread->priv != NULL - && (get_remote_thread_info (thread)->stop_reason - == TARGET_STOPPED_BY_WATCHPOINT)) - { - *addr_p = get_remote_thread_info (thread)->watch_data_address; - return true; - } + && (get_remote_thread_info (thread)->stop_reason == TARGET_STOPPED_BY_WATCHPOINT)) + return get_remote_thread_info (thread)->watch_data_address; - return false; + return {}; } diff --git a/gdb/target-debug.h b/gdb/target-debug.h index f7debe4..3e41e98 100644 --- a/gdb/target-debug.h +++ b/gdb/target-debug.h @@ -187,6 +187,10 @@ target_debug_print_std_vector_mem_region (const std::vector<mem_region> &vec) { return host_address_to_string (vec.data ()); } static std::string +target_debug_print_std_vector_CORE_ADDR (const std::vector<CORE_ADDR> &vec) +{ return host_address_to_string (vec.data ()); } + +static std::string target_debug_print_std_vector_static_tracepoint_marker (const std::vector<static_tracepoint_marker> &vec) { return host_address_to_string (vec.data ()); } diff --git a/gdb/target-delegates-gen.c b/gdb/target-delegates-gen.c index 164ddbb..9337182 100644 --- a/gdb/target-delegates-gen.c +++ b/gdb/target-delegates-gen.c @@ -56,7 +56,7 @@ struct dummy_target : public target_ops int remove_mask_watchpoint (CORE_ADDR arg0, CORE_ADDR arg1, enum target_hw_bp_type arg2) override; bool stopped_by_watchpoint () override; bool have_steppable_watchpoint () override; - bool stopped_data_address (CORE_ADDR *arg0) override; + std::vector<CORE_ADDR> stopped_data_addresses () override; bool watchpoint_addr_within_range (CORE_ADDR arg0, CORE_ADDR arg1, int arg2) override; int region_ok_for_hw_watchpoint (CORE_ADDR arg0, int arg1) override; bool can_accel_watchpoint_condition (CORE_ADDR arg0, int arg1, int arg2, struct expression *arg3) override; @@ -237,7 +237,7 @@ struct debug_target : public target_ops int remove_mask_watchpoint (CORE_ADDR arg0, CORE_ADDR arg1, enum target_hw_bp_type arg2) override; bool stopped_by_watchpoint () override; bool have_steppable_watchpoint () override; - bool stopped_data_address (CORE_ADDR *arg0) override; + std::vector<CORE_ADDR> stopped_data_addresses () override; bool watchpoint_addr_within_range (CORE_ADDR arg0, CORE_ADDR arg1, int arg2) override; int region_ok_for_hw_watchpoint (CORE_ADDR arg0, int arg1) override; bool can_accel_watchpoint_condition (CORE_ADDR arg0, int arg1, int arg2, struct expression *arg3) override; @@ -1020,28 +1020,27 @@ debug_target::have_steppable_watchpoint () return result; } -bool -target_ops::stopped_data_address (CORE_ADDR *arg0) +std::vector<CORE_ADDR> +target_ops::stopped_data_addresses () { - return this->beneath ()->stopped_data_address (arg0); + return this->beneath ()->stopped_data_addresses (); } -bool -dummy_target::stopped_data_address (CORE_ADDR *arg0) +std::vector<CORE_ADDR> +dummy_target::stopped_data_addresses () { - return false; + return std::vector<CORE_ADDR> (); } -bool -debug_target::stopped_data_address (CORE_ADDR *arg0) +std::vector<CORE_ADDR> +debug_target::stopped_data_addresses () { - target_debug_printf_nofunc ("-> %s->stopped_data_address (...)", this->beneath ()->shortname ()); - bool result - = this->beneath ()->stopped_data_address (arg0); - target_debug_printf_nofunc ("<- %s->stopped_data_address (%s) = %s", + target_debug_printf_nofunc ("-> %s->stopped_data_addresses (...)", this->beneath ()->shortname ()); + std::vector<CORE_ADDR> result + = this->beneath ()->stopped_data_addresses (); + target_debug_printf_nofunc ("<- %s->stopped_data_addresses () = %s", this->beneath ()->shortname (), - target_debug_print_CORE_ADDR_p (arg0).c_str (), - target_debug_print_bool (result).c_str ()); + target_debug_print_std_vector_CORE_ADDR (result).c_str ()); return result; } diff --git a/gdb/target.h b/gdb/target.h index 365e894..e7440e5 100644 --- a/gdb/target.h +++ b/gdb/target.h @@ -601,8 +601,8 @@ struct target_ops TARGET_DEFAULT_RETURN (false); virtual bool have_steppable_watchpoint () TARGET_DEFAULT_RETURN (false); - virtual bool stopped_data_address (CORE_ADDR *) - TARGET_DEFAULT_RETURN (false); + virtual std::vector<CORE_ADDR> stopped_data_addresses () + TARGET_DEFAULT_RETURN (std::vector<CORE_ADDR> ()); virtual bool watchpoint_addr_within_range (CORE_ADDR, CORE_ADDR, int) TARGET_DEFAULT_FUNC (default_watchpoint_addr_within_range); @@ -2169,11 +2169,22 @@ extern int target_remove_hw_breakpoint (gdbarch *gdbarch, extern int target_ranged_break_num_registers (void); -/* Return non-zero if target knows the data address which triggered this - target_stopped_by_watchpoint, in such case place it to *ADDR_P. Only the - INFERIOR_PTID task is being queried. */ -#define target_stopped_data_address(target, addr_p) \ - (target)->stopped_data_address (addr_p) +/* Return a vector containing the data addresses which triggered this + target_stopped_by_watchpoint if the addresses are known. If the + addresses are not known then an empty vector is returned. Only the + INFERIOR_PTID task is being queried. + + Some target, for example AArch64, can only watch ranges of memory, + e.g. 8 or 16 bytes. As a result, many watchpoints could fall within any + single region. In such a case, this method will return the address of + all possible watchpoints, and it is up to GDB core to select a suitable + watchpoint to display to the user, for example, by checking the value of + write watchpoints. Or GDB core could tell the user that it is unable to + disambiguate between multiple read watchpoints (though this isn't + currently done). */ + +#define target_stopped_data_addresses(target) \ + (target)->stopped_data_addresses () /* Return non-zero if ADDR is within the range of a watchpoint spanning LENGTH bytes beginning at START. */ diff --git a/gdb/testsuite/gdb.base/watchpoint-adjacent.c b/gdb/testsuite/gdb.base/watchpoint-adjacent.c new file mode 100644 index 0000000..b9e3a51 --- /dev/null +++ b/gdb/testsuite/gdb.base/watchpoint-adjacent.c @@ -0,0 +1,72 @@ +/* This testcase is part of GDB, the GNU debugger. + + Copyright 2025 Free Software Foundation, Inc. + + This program is free software; you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation; either version 3 of the License, or + (at your option) any later version. + + This program is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with this program. If not, see <http://www.gnu.org/licenses/>. */ + +#include <stdint.h> +#include <assert.h> + +typedef unsigned long long type_ll; + +#ifndef VAR_TYPE +# error "VAR_TYPE not defined" +#endif + +/* Place A and B within this wrapper struct. FIRST ensures that A is + (usually) going to start at an 8-byte boundary. The goal here is + that, when VAR_TYPE is less than 8 bytes, both A and B are placed + within the same 8-byte region, and that the region starts at an + 8-byte boundary. */ + +struct wrapper +{ + unsigned long long first; + + VAR_TYPE a, b; +}; + +volatile struct wrapper obj; + +/* Write to obj.a and obj.b, but don't read these fields. */ +void +writer (void) +{ + obj.a = 1; + obj.b = 2; +} + +/* Read from obj.a and obj.b, but don't write to these fields. */ +int +reader (void) +{ + int v = obj.b - obj.a; + v--; + return v; +} + +int +main (void) +{ + /* Ensure that obj.a, obj.b, and obj.c were placed as we needed. */ + assert ((((uintptr_t) &obj.a) & 0x7) == 0); + assert ((((uintptr_t) &obj.a) + sizeof (obj.a)) == (((uintptr_t) &obj.b))); + assert (sizeof (obj.a) == sizeof (obj.b)); + + writer (); + + int val = reader (); /* Break for read test. */ + + return val; +} diff --git a/gdb/testsuite/gdb.base/watchpoint-adjacent.exp b/gdb/testsuite/gdb.base/watchpoint-adjacent.exp new file mode 100644 index 0000000..40e0437 --- /dev/null +++ b/gdb/testsuite/gdb.base/watchpoint-adjacent.exp @@ -0,0 +1,182 @@ +# Copyright 2025 Free Software Foundation, Inc. +# +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 3 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see <http://www.gnu.org/licenses/>. + +# The inferior has two adjacent variables. We a 'watch' on one field, +# and an 'rwatch' on the other. Running the inferior writes to both +# fields. Check GDB reports the expected 'watch' watchpoint. +# +# Multiple inferiors are compiled, using a variety of types for the +# two fields. + +require allow_hw_watchpoint_multi_tests + +standard_testfile + +# When printing a value, for some variable types, GDB will add a +# suffix containing an alternative representation of the value. For +# example, characters will be printed as decimal, and then as the +# character. +# +# Return a regexp to match the suffix for a variable of VAR_TYPE. +# This doesn't match the specific value contents, it will match all +# possible suffix values for something of VAR_TYPE. +proc get_value_suffix { var_type } { + if { $var_type eq "char" } { + set suffix " '\[^'\]+'" + } else { + set suffix "" + } + + return $suffix +} + +# Start BINFILE, then set a watch and rwatch watchpoint on WATCH_VAR +# and RWATCH_VAR respectively. Continue the inferior and expect to +# see GDB stop due to WATCH_VAR being written too. +proc run_write_test { binfile var_type watch_var rwatch_var } { + clean_restart $binfile + + if { ![runto_main] } { + return + } + + delete_breakpoints + + gdb_test_no_output "set breakpoint always-inserted on" + + gdb_test "watch obj.$watch_var" \ + "Hardware watchpoint $::decimal: obj.$watch_var" + set wp_num [get_integer_valueof "\$bpnum" "*UNKNOWN*"] + gdb_test "rwatch obj.$rwatch_var" \ + "Hardware read watchpoint $::decimal: obj.$rwatch_var" + + if { $watch_var eq "a" } { + set new_val 1 + } else { + set new_val 2 + } + + set suffix [get_value_suffix $var_type] + + gdb_test "continue" \ + [multi_line \ + "Hardware watchpoint $wp_num: obj.$watch_var" \ + "" \ + "Old value = 0${suffix}" \ + "New value = ${new_val}${suffix}" \ + ".*"] + +} + +# Start BINFILE, continue until the call to the `reader` function in +# the inferior. Then create an 'rwatch' watchpoint on RWATCH var, +# which will be either 'a' or 'b'. Next create 'watch' watchpoints on +# both the 'a' and 'b' variables, watching for writes. +# +# Continue the inferior, both 'a' and 'b' are read, and GDB should stop +# and let us know that we stopped at the 'rwatch' watchpoint. +# +# On some architectures, for some variable sizes, the hardware cannot +# figure out which watchpoint triggered as the hardware can only watch +# (for example) 8-byte memory blocks. In this case GDB just reports +# the first watchpoint (in creation order) within the block. For this +# reason the test creates the 'rwatch' watchpoint first. +proc run_read_test { binfile var_type rwatch_var rwatch_first watch_vars } { + clean_restart $binfile + + if { ![runto_main] } { + return + } + + gdb_breakpoint [gdb_get_line_number "Break for read test"] + gdb_continue_to_breakpoint "prepare for read test" + delete_breakpoints + + gdb_test_no_output "set breakpoint always-inserted on" + + if { $rwatch_first } { + gdb_test "rwatch obj.${rwatch_var}" \ + "Hardware read watchpoint $::decimal: obj.$rwatch_var" + set wp_num [get_integer_valueof "\$bpnum" "*UNKNOWN*"] + } + + foreach v $watch_vars { + gdb_test "watch obj.$v" \ + "Hardware watchpoint $::decimal: obj.$v" + } + + if { !$rwatch_first } { + gdb_test "rwatch obj.${rwatch_var}" \ + "Hardware read watchpoint $::decimal: obj.$rwatch_var" + set wp_num [get_integer_valueof "\$bpnum" "*UNKNOWN*"] + } + + + if { $rwatch_var eq "a" } { + set val 1 + } else { + set val 2 + } + + set suffix [get_value_suffix $var_type] + + gdb_test "continue" \ + [multi_line \ + "Hardware read watchpoint ${wp_num}: obj.$rwatch_var" \ + "" \ + "Value = ${val}${suffix}" \ + ".*"] +} + +# Build a binary using VAR_TYPE as the test variable type. Then Call +# run_test twice. +proc build_and_run_test { var_type } { + set filename ${::testfile}-${var_type} + set binfile [standard_output_file $filename] + + set flags [list debug additional_flags=-DVAR_TYPE=${var_type}] + if {[build_executable "failed to build" $filename $::srcfile $flags]} { + return + } + + + set test_list [list \ + { a {a b} } \ + { b {a b} } \ + { a {b} } \ + { b {a} }] + foreach_with_prefix test $test_list { + set rwatch_var [lindex $test 0] + set watch_vars [lindex $test 1] + + foreach_with_prefix rwatch_first { true false } { + run_read_test $binfile $var_type $rwatch_var $rwatch_first $watch_vars + } + } + + foreach test { {a b} {b a} } { + set watch_var [lindex $test 0] + set rwatch_var [lindex $test 1] + + with_test_prefix "watch: ${watch_var}, rwatch: ${rwatch_var}" { + run_write_test $binfile $var_type $watch_var $rwatch_var + } + } +} + +# Run the test with a series of different types. +foreach_with_prefix var_type { type_ll int short char float double } { + build_and_run_test $var_type +} diff --git a/gdb/x86-linux-nat.h b/gdb/x86-linux-nat.h index a62cc4d..99a6112 100644 --- a/gdb/x86-linux-nat.h +++ b/gdb/x86-linux-nat.h @@ -52,14 +52,14 @@ struct x86_linux_nat_target : public x86_nat_target<linux_nat_target> bool stopped_by_watchpoint () override { return linux_nat_target::stopped_by_watchpoint (); } - bool stopped_data_address (CORE_ADDR *addr_p) override - { return linux_nat_target::stopped_data_address (addr_p); } + std::vector<CORE_ADDR> stopped_data_addresses () override + { return linux_nat_target::stopped_data_addresses (); } bool low_stopped_by_watchpoint () override { return x86_nat_target::stopped_by_watchpoint (); } bool low_stopped_data_address (CORE_ADDR *addr_p) override - { return x86_nat_target::stopped_data_address (addr_p); } + { return x86_stopped_data_address (addr_p); } void low_new_fork (struct lwp_info *parent, pid_t child_pid) override; diff --git a/gdb/x86-nat.h b/gdb/x86-nat.h index 6197218..5ec2397 100644 --- a/gdb/x86-nat.h +++ b/gdb/x86-nat.h @@ -104,8 +104,14 @@ struct x86_nat_target : public BaseTarget bool stopped_by_watchpoint () override { return x86_stopped_by_watchpoint (); } - bool stopped_data_address (CORE_ADDR *addr_p) override - { return x86_stopped_data_address (addr_p); } + std::vector<CORE_ADDR> stopped_data_addresses () override + { + CORE_ADDR addr; + if (x86_stopped_data_address (&addr)) + return { addr }; + + return {}; + } /* A target must provide an implementation of the "supports_stopped_by_hw_breakpoint" target method before this diff --git a/gdbserver/linux-aarch64-low.cc b/gdbserver/linux-aarch64-low.cc index 2eb3af6..0558c26 100644 --- a/gdbserver/linux-aarch64-low.cc +++ b/gdbserver/linux-aarch64-low.cc @@ -119,7 +119,7 @@ protected: bool low_stopped_by_watchpoint () override; - CORE_ADDR low_stopped_data_address () override; + std::vector<CORE_ADDR> low_stopped_data_addresses () override; bool low_siginfo_fixup (siginfo_t *native, gdb_byte *inf, int direction) override; @@ -548,10 +548,10 @@ aarch64_remove_non_address_bits (CORE_ADDR pointer) return aarch64_remove_top_bits (pointer, mask); } -/* Implementation of linux target ops method "low_stopped_data_address". */ +/* Implementation of linux target ops method "low_stopped_data_addresses". */ -CORE_ADDR -aarch64_target::low_stopped_data_address () +std::vector<CORE_ADDR> +aarch64_target::low_stopped_data_addresses () { siginfo_t siginfo; struct aarch64_debug_reg_state *state; @@ -559,12 +559,12 @@ aarch64_target::low_stopped_data_address () /* Get the siginfo. */ if (ptrace (PTRACE_GETSIGINFO, pid, NULL, &siginfo) != 0) - return (CORE_ADDR) 0; + return {}; /* Need to be a hardware breakpoint/watchpoint trap. */ if (siginfo.si_signo != SIGTRAP || (siginfo.si_code & 0xffff) != 0x0004 /* TRAP_HWBKPT */) - return (CORE_ADDR) 0; + return {}; /* Make sure to ignore the top byte, otherwise we may not recognize a hardware watchpoint hit. The stopped data addresses coming from the @@ -574,11 +574,7 @@ aarch64_target::low_stopped_data_address () /* Check if the address matches any watched address. */ state = aarch64_get_debug_reg_state (current_thread->id.pid ()); - CORE_ADDR result; - if (aarch64_stopped_data_address (state, addr_trap, &result)) - return result; - - return (CORE_ADDR) 0; + return aarch64_stopped_data_addresses (state, addr_trap); } /* Implementation of linux target ops method "low_stopped_by_watchpoint". */ @@ -586,7 +582,7 @@ aarch64_target::low_stopped_data_address () bool aarch64_target::low_stopped_by_watchpoint () { - return (low_stopped_data_address () != 0); + return !low_stopped_data_addresses ().empty (); } /* Fetch the thread-local storage pointer for libthread_db. */ diff --git a/gdbserver/linux-arm-low.cc b/gdbserver/linux-arm-low.cc index f4870ee..6c04eed 100644 --- a/gdbserver/linux-arm-low.cc +++ b/gdbserver/linux-arm-low.cc @@ -100,7 +100,7 @@ protected: bool low_stopped_by_watchpoint () override; - CORE_ADDR low_stopped_data_address () override; + std::vector<CORE_ADDR> low_stopped_data_addresses () override; arch_process_info *low_new_process () override; @@ -729,11 +729,11 @@ arm_target::low_stopped_by_watchpoint () /* Return data address that triggered watchpoint. Called only if low_stopped_by_watchpoint returned true. */ -CORE_ADDR -arm_target::low_stopped_data_address () +std::vector<CORE_ADDR> +arm_target::low_stopped_data_addresses () { struct lwp_info *lwp = get_thread_lwp (current_thread); - return lwp->arch_private->stopped_data_address; + return { lwp->arch_private->stopped_data_address }; } /* Called when a new process is created. */ diff --git a/gdbserver/linux-loongarch-low.cc b/gdbserver/linux-loongarch-low.cc index 62592a5..f3ff1c3 100644 --- a/gdbserver/linux-loongarch-low.cc +++ b/gdbserver/linux-loongarch-low.cc @@ -65,7 +65,7 @@ protected: bool low_stopped_by_watchpoint () override; - CORE_ADDR low_stopped_data_address () override; + std::vector<CORE_ADDR> low_stopped_data_addresses () override; arch_process_info *low_new_process () override; @@ -555,10 +555,10 @@ loongarch_target::low_remove_point (raw_bkpt_type type, CORE_ADDR addr, } -/* Implementation of linux target ops method "low_stopped_data_address". */ +/* Implementation of linux target ops method "low_stopped_data_addresses". */ -CORE_ADDR -loongarch_target::low_stopped_data_address () +std::vector<CORE_ADDR> +loongarch_target::low_stopped_data_addresses () { siginfo_t siginfo; struct loongarch_debug_reg_state *state; @@ -566,20 +566,20 @@ loongarch_target::low_stopped_data_address () /* Get the siginfo. */ if (ptrace (PTRACE_GETSIGINFO, pid, NULL, &siginfo) != 0) - return (CORE_ADDR) 0; + return {}; /* Need to be a hardware breakpoint/watchpoint trap. */ if (siginfo.si_signo != SIGTRAP || (siginfo.si_code & 0xffff) != 0x0004 /* TRAP_HWBKPT */) - return (CORE_ADDR) 0; + return {}; /* Check if the address matches any watched address. */ state = loongarch_get_debug_reg_state (current_thread->id.pid ()); CORE_ADDR result; if (loongarch_stopped_data_address (state, (CORE_ADDR) siginfo.si_addr, &result)) - return result; + return { result }; - return (CORE_ADDR) 0; + return {}; } /* Implementation of linux target ops method "low_stopped_by_watchpoint". */ @@ -587,7 +587,7 @@ loongarch_target::low_stopped_data_address () bool loongarch_target::low_stopped_by_watchpoint () { - return (low_stopped_data_address () != 0); + return !low_stopped_data_addresses ().empty (); } /* Implementation of linux target ops method "low_new_process". */ diff --git a/gdbserver/linux-low.cc b/gdbserver/linux-low.cc index 3964270..deacaf4 100644 --- a/gdbserver/linux-low.cc +++ b/gdbserver/linux-low.cc @@ -2194,7 +2194,7 @@ linux_process_target::check_stopped_by_watchpoint (lwp_info *child) if (low_stopped_by_watchpoint ()) { child->stop_reason = TARGET_STOPPED_BY_WATCHPOINT; - child->stopped_data_address = low_stopped_data_address (); + child->stopped_data_addresses = low_stopped_data_addresses (); } return child->stop_reason == TARGET_STOPPED_BY_WATCHPOINT; @@ -2206,10 +2206,10 @@ linux_process_target::low_stopped_by_watchpoint () return false; } -CORE_ADDR -linux_process_target::low_stopped_data_address () +std::vector<CORE_ADDR> +linux_process_target::low_stopped_data_addresses () { - return 0; + return {}; } /* Return the ptrace options that we want to try to enable. */ @@ -5641,12 +5641,12 @@ linux_process_target::stopped_by_watchpoint () return lwp->stop_reason == TARGET_STOPPED_BY_WATCHPOINT; } -CORE_ADDR -linux_process_target::stopped_data_address () +std::vector<CORE_ADDR> +linux_process_target::stopped_data_addresses () { struct lwp_info *lwp = get_thread_lwp (current_thread); - return lwp->stopped_data_address; + return lwp->stopped_data_addresses; } /* This is only used for targets that define PT_TEXT_ADDR, diff --git a/gdbserver/linux-low.h b/gdbserver/linux-low.h index e1c88ee..07872aa 100644 --- a/gdbserver/linux-low.h +++ b/gdbserver/linux-low.h @@ -199,7 +199,7 @@ public: bool stopped_by_watchpoint () override; - CORE_ADDR stopped_data_address () override; + std::vector<CORE_ADDR> stopped_data_addresses () override; bool supports_read_offsets () override; @@ -652,7 +652,7 @@ protected: virtual bool low_stopped_by_watchpoint (); - virtual CORE_ADDR low_stopped_data_address (); + virtual std::vector<CORE_ADDR> low_stopped_data_addresses (); /* Hooks to reformat register data for PEEKUSR/POKEUSR (in particular for registers smaller than an xfer unit). */ @@ -858,10 +858,9 @@ struct lwp_info enum target_stop_reason stop_reason = TARGET_STOPPED_BY_NO_REASON; /* On architectures where it is possible to know the data address of - a triggered watchpoint, STOPPED_DATA_ADDRESS is non-zero, and - contains such data address. Only valid if STOPPED_BY_WATCHPOINT - is true. */ - CORE_ADDR stopped_data_address = 0; + a triggered watchpoint, STOPPED_DATA_ADDRESS is the list of such + data addresses. Only valid if STOPPED_BY_WATCHPOINT is true. */ + std::vector<CORE_ADDR> stopped_data_addresses; /* If this is non-zero, it is a breakpoint to be reinserted at our next stop (SIGTRAP stops only). */ diff --git a/gdbserver/linux-mips-low.cc b/gdbserver/linux-mips-low.cc index 295eb87..612ae16 100644 --- a/gdbserver/linux-mips-low.cc +++ b/gdbserver/linux-mips-low.cc @@ -62,7 +62,7 @@ protected: bool low_stopped_by_watchpoint () override; - CORE_ADDR low_stopped_data_address () override; + std::vector<CORE_ADDR> low_stopped_data_addresses () override; void low_collect_ptrace_register (regcache *regcache, int regno, char *buf) override; @@ -658,10 +658,10 @@ mips_target::low_stopped_by_watchpoint () } /* This is the implementation of linux target ops method - low_stopped_data_address. */ + low_stopped_data_addresses. */ -CORE_ADDR -mips_target::low_stopped_data_address () +std::vector<CORE_ADDR> +mips_target::low_stopped_data_addresses () { struct process_info *proc = current_process (); struct arch_process_info *priv = proc->priv->arch_private; @@ -679,7 +679,7 @@ mips_target::low_stopped_data_address () &priv->watch_readback, &priv->watch_readback_valid, 0)) - return 0; + return {}; num_valid = mips_linux_watch_get_num_valid (&priv->watch_readback); @@ -711,12 +711,12 @@ mips_target::low_stopped_data_address () } /* Check for overlap of even a single byte. */ if (last_byte >= t_low && addr <= t_low + t_hi) - return addr; + return { addr }; } } /* Shouldn't happen. */ - return 0; + return {}; } /* Fetch the thread-local storage pointer for libthread_db. */ diff --git a/gdbserver/linux-x86-low.cc b/gdbserver/linux-x86-low.cc index 918630d..0230ea1 100644 --- a/gdbserver/linux-x86-low.cc +++ b/gdbserver/linux-x86-low.cc @@ -158,7 +158,7 @@ protected: bool low_stopped_by_watchpoint () override; - CORE_ADDR low_stopped_data_address () override; + std::vector<CORE_ADDR> low_stopped_data_addresses () override; /* collect_ptrace_register/supply_ptrace_register are not needed in the native i386 case (no registers smaller than an xfer unit), and are not @@ -712,15 +712,15 @@ x86_target::low_stopped_by_watchpoint () return x86_dr_stopped_by_watchpoint (&proc->priv->arch_private->debug_reg_state); } -CORE_ADDR -x86_target::low_stopped_data_address () +std::vector<CORE_ADDR> +x86_target::low_stopped_data_addresses () { struct process_info *proc = current_process (); CORE_ADDR addr; if (x86_dr_stopped_data_address (&proc->priv->arch_private->debug_reg_state, &addr)) - return addr; - return 0; + return { addr }; + return {}; } /* Called when a new process is created. */ diff --git a/gdbserver/remote-utils.cc b/gdbserver/remote-utils.cc index 15f073d..3b5db17 100644 --- a/gdbserver/remote-utils.cc +++ b/gdbserver/remote-utils.cc @@ -1182,21 +1182,21 @@ prepare_resume_reply (char *buf, ptid_t ptid, const target_waitstatus &status) if (the_target->stopped_by_watchpoint ()) { - CORE_ADDR addr; - int i; + std::vector<CORE_ADDR> addr_vec = the_target->stopped_data_addresses (); - memcpy (buf, "watch:", 6); - buf += 6; - - addr = the_target->stopped_data_address (); - - /* Convert each byte of the address into two hexadecimal - chars. Note that we take sizeof (void *) instead of - sizeof (addr); this is to avoid sending a 64-bit - address to a 32-bit GDB. */ - for (i = sizeof (void *) * 2; i > 0; i--) - *buf++ = tohex ((addr >> (i - 1) * 4) & 0xf); - *buf++ = ';'; + for (const CORE_ADDR addr : addr_vec) + { + memcpy (buf, "watch:", 6); + buf += 6; + + /* Convert each byte of the address into two hexadecimal + chars. Note that we take sizeof (void *) instead of + sizeof (addr); this is to avoid sending a 64-bit + address to a 32-bit GDB. */ + for (int i = sizeof (void *) * 2; i > 0; i--) + *buf++ = tohex ((addr >> (i - 1) * 4) & 0xf); + *buf++ = ';'; + } } else if (cs.swbreak_feature && target_stopped_by_sw_breakpoint ()) { diff --git a/gdbserver/target.cc b/gdbserver/target.cc index c400174c..b5b66d9 100644 --- a/gdbserver/target.cc +++ b/gdbserver/target.cc @@ -409,10 +409,10 @@ process_stratum_target::stopped_by_watchpoint () return false; } -CORE_ADDR -process_stratum_target::stopped_data_address () +std::vector<CORE_ADDR> +process_stratum_target::stopped_data_addresses () { - return 0; + return {}; } bool diff --git a/gdbserver/target.h b/gdbserver/target.h index 66ca72f..1532365 100644 --- a/gdbserver/target.h +++ b/gdbserver/target.h @@ -218,9 +218,9 @@ public: otherwise. */ virtual bool stopped_by_watchpoint (); - /* Returns the address associated with the watchpoint that hit, if any; - returns 0 otherwise. */ - virtual CORE_ADDR stopped_data_address (); + /* Returns the list of addresses associated with the watchpoint(s) + that were hit, if any; returns an empty vector otherwise. */ + virtual std::vector<CORE_ADDR> stopped_data_addresses (); /* Return true if the read_offsets target op is supported. */ virtual bool supports_read_offsets (); diff --git a/gdbserver/win32-low.cc b/gdbserver/win32-low.cc index 89831de..635479b 100644 --- a/gdbserver/win32-low.cc +++ b/gdbserver/win32-low.cc @@ -240,13 +240,13 @@ win32_process_target::stopped_by_watchpoint () return false; } -CORE_ADDR -win32_process_target::stopped_data_address () +std::vector<CORE_ADDR> +win32_process_target::stopped_data_addresses () { if (the_low_target.stopped_data_address != NULL) - return the_low_target.stopped_data_address (); + return { the_low_target.stopped_data_address () }; else - return 0; + return {}; } diff --git a/gdbserver/win32-low.h b/gdbserver/win32-low.h index a76ed9f..680ae2b 100644 --- a/gdbserver/win32-low.h +++ b/gdbserver/win32-low.h @@ -144,7 +144,7 @@ public: bool stopped_by_watchpoint () override; - CORE_ADDR stopped_data_address () override; + std::vector<CORE_ADDR> stopped_data_addresses () override; bool supports_qxfer_siginfo () override; |