From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: by sourceware.org (Postfix, from userid 2205) id 1EE803858C39; Thu, 14 Mar 2024 10:24:21 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 1EE803858C39 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1710411861; bh=NWTvhlyYZU+dZqw6okbb8ObbOXVvvCRkEY6v65pGNZc=; h=From:To:Subject:Date:From; b=Q5w9lyACZ21lMsNasH3Fv85hF+27MLl7zjgCGBbGUI7s6tJcYxGLXbr4dkTFEDHdY L5oAoJkMC65HdPek5eocevK+aHBbHehLz/gMTeLH5fbRZ1KzcSCF19EbswgzFTfjer E4SePEbmGkiHA8PGXkTkq0j/josYHXQ/4y5GF/hw= Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable From: Tom de Vries To: gdb-cvs@sourceware.org Subject: [binutils-gdb] [gdb/tdep] Fix gdb.base/watchpoint-unaligned.exp on aarch64 X-Act-Checkin: binutils-gdb X-Git-Author: Tom de Vries X-Git-Refname: refs/heads/master X-Git-Oldrev: 3a4c6f1aa958739705ee69526fdeed7c69d7243c X-Git-Newrev: 9a03f2185347bd8f20da9bf535bc68a8d0f18ce8 Message-Id: <20240314102421.1EE803858C39@sourceware.org> Date: Thu, 14 Mar 2024 10:24:21 +0000 (GMT) List-Id: https://sourceware.org/git/gitweb.cgi?p=3Dbinutils-gdb.git;h=3D9a03f2185347= bd8f20da9bf535bc68a8d0f18ce8 commit 9a03f2185347bd8f20da9bf535bc68a8d0f18ce8 Author: Tom de Vries Date: Thu Mar 14 11:25:10 2024 +0100 [gdb/tdep] Fix gdb.base/watchpoint-unaligned.exp on aarch64 =20 On aarch64-linux, with test-case gdb.base/watchpoint-unaligned.exp I ru= n into: ... (gdb) watch data.u.size8twice[1]^M Hardware watchpoint 241: data.u.size8twice[1]^M (gdb) PASS: gdb.base/watchpoint-unaligned.exp: watch data.u.size8twice[= 1] continue^M Continuing.^M FAIL: gdb.base/watchpoint-unaligned.exp: continue (timeout) FAIL: gdb.base/watchpoint-unaligned.exp: size8twice write ... =20 This happens as follows. =20 We start the exec and set an 8-byte hardware watchpoint on data.u.size8twice[1] at address 0x440048: ... (gdb) p sizeof (data.u.size8twice[1]) $1 =3D 8 (gdb) p &data.u.size8twice[1] $2 =3D (uint64_t *) 0x440048 ... =20 We continue execution, and a 16-byte write at address 0x440040 triggers= the hardware watchpoint: ... 4101c8: a9000801 stp x1, x2, [x0] ... =20 When checking whether a watchpoint has triggered in aarch64_stopped_data_address, we check against address 0x440040 (passed= in parameter addr_trap). This behaviour is documented: ... /* ADDR_TRAP reports the first address of the memory range accessed by the CPU, regardless of what was the memory range watched. ... */ ... and consequently the matching logic compares against an addr_watch_alig= ned: ... && addr_trap >=3D addr_watch_aligned && addr_trap < addr_watch + len) ... =20 However, the comparison fails: ... (gdb) p /x addr_watch_aligned $3 =3D 0x440048 (gdb) p addr_trap >=3D addr_watch_aligned $4 =3D false ... =20 Consequently, aarch64_stopped_data_address returns false, and stopped_by_watchpoint returns false, and watchpoints_triggered returns = 0, which make infrun think it's looking at a delayed hardware breakpoint/watchpoint trap: ... [infrun] handle_signal_stop: stop_pc=3D0x4101c8 [infrun] handle_signal_stop: delayed hardware breakpoint/watchpoint t= rap, ignoring ... Infrun then ignores the trap and continues, but runs into the same situ= ation again and again, causing a hang which then causes the test timeout. =20 Fix this by allowing a match 8 bytes below addr_watch_aligned. This introduces the possibility for false positives, so we only do this for = regular "value changed" watchpoints. =20 An earlier version of this patch worked by aligning addr_watch_aligned = to 16 instead of 8: ... - const CORE_ADDR addr_watch_aligned =3D align_down (state->dr_addr_wp= [i], 8); + const CORE_ADDR addr_watch_aligned =3D align_down (state->dr_addr_wp= [i], 16); ... but while that fixed the test-case, it didn't fix the problem completel= y, so extend the test-case to check more scenarios. =20 Tested on aarch64-linux. =20 Tested-By: Luis Machado Approved-By: Luis Machado =20 PR tdep/29423 Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=3D29423 Diff: --- gdb/aarch64-nat.c | 17 +++++- gdb/testsuite/gdb.base/watchpoint-unaligned.c | 11 ++-- gdb/testsuite/gdb.base/watchpoint-unaligned.exp | 78 +++++++++++++++------= ---- 3 files changed, 68 insertions(+), 38 deletions(-) diff --git a/gdb/aarch64-nat.c b/gdb/aarch64-nat.c index 6c72a8d6d9f..802bab6d682 100644 --- a/gdb/aarch64-nat.c +++ b/gdb/aarch64-nat.c @@ -269,7 +269,7 @@ aarch64_stopped_data_address (const struct aarch64_debu= g_reg_state *state, =3D aarch64_watchpoint_length (state->dr_ctrl_wp[i]); const CORE_ADDR addr_watch =3D state->dr_addr_wp[i] + offset; const CORE_ADDR addr_watch_aligned - =3D align_down (state->dr_addr_wp[i], 8); + =3D align_down (state->dr_addr_wp[i], AARCH64_HWP_MAX_LEN_PER_REG); const CORE_ADDR addr_orig =3D state->dr_addr_orig_wp[i]; =20 /* ADDR_TRAP reports the first address of the memory range @@ -283,8 +283,19 @@ aarch64_stopped_data_address (const struct aarch64_deb= ug_reg_state *state, |---- range watched ----| |----------- range accessed ------------| =20 - In this case, ADDR_TRAP will be 4. */ - if (!(addr_trap >=3D 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 =3D type =3D=3D hw_write ? 16 : 8; + const CORE_ADDR addr_watch_base =3D addr_watch_aligned - + (max_access_size - AARCH64_HWP_MAX_LEN_PER_REG); + if (!(addr_trap >=3D addr_watch_base && addr_trap < addr_watch + len)) { /* Not a match. */ diff --git a/gdb/testsuite/gdb.base/watchpoint-unaligned.c b/gdb/testsuite/= gdb.base/watchpoint-unaligned.c index 64728bb9ea3..6f709259b6c 100644 --- a/gdb/testsuite/gdb.base/watchpoint-unaligned.c +++ b/gdb/testsuite/gdb.base/watchpoint-unaligned.c @@ -29,7 +29,7 @@ static volatile struct uint32_t size4[2]; uint16_t size2[4]; uint8_t size1[8]; - uint64_t size8twice[2]; + uint64_t size8twice[3]; } u; } data; @@ -44,13 +44,14 @@ write_size8twice (void) static const uint64_t second =3D 2; =20 #ifdef __aarch64__ + volatile void *p =3D &data.u.size8twice[offset]; asm volatile ("stp %1, %2, [%0]" : /* output */ - : "r" (data.u.size8twice), "r" (first), "r" (second) /* input */ + : "r" (p), "r" (first), "r" (second) /* input */ : "memory" /* clobber */); #else - data.u.size8twice[0] =3D first; - data.u.size8twice[1] =3D second; + data.u.size8twice[offset] =3D first; + data.u.size8twice[offset + 1] =3D second; #endif } =20 @@ -59,7 +60,7 @@ main (void) { volatile uint64_t local; =20 - assert (sizeof (data) =3D=3D 8 + 2 * 8); + assert (sizeof (data) =3D=3D 8 + 3 * 8); =20 write_size8twice (); =20 diff --git a/gdb/testsuite/gdb.base/watchpoint-unaligned.exp b/gdb/testsuit= e/gdb.base/watchpoint-unaligned.exp index 8d985c03bfc..35e8868d39d 100644 --- a/gdb/testsuite/gdb.base/watchpoint-unaligned.exp +++ b/gdb/testsuite/gdb.base/watchpoint-unaligned.exp @@ -151,38 +151,56 @@ foreach wpcount {4 7} { gdb_assert $got_hit $test } =20 -if ![runto_main] { - return -1 -} -gdb_breakpoint [gdb_get_line_number "final_return"] "Breakpoint $decimal a= t $hex" "final_return" -set test {watch data.u.size8twice[1]} -set wpnum 0 -gdb_test_multiple $test $test { - -re "Hardware watchpoint (\[0-9\]+): .*\r\n$gdb_prompt $" { - set wpnum $expect_out(1,string) - pass $gdb_test_name - } - -re "Watchpoint (\[0-9\]+): .*\r\n$gdb_prompt $" { - if {[istarget "arm*-*-*"]} { - untested $gdb_test_name - } else { - fail $gdb_test_name - } - } -} -if {$wpnum} { - set test "continue" - set got_hit 0 - gdb_test_multiple $test $test { - -re "\r\nCould not insert hardware watchpoint .*\r\n$gdb_prompt $" { +# We've got an array with 3 8-byte elements. Do a store of 16 bytes, +# to: +# - elements 0 and 1 (offset =3D=3D 0), and +# - elements 1 and 2 (offset =3D=3D 1). +# For each case, check setting a watchpoint at: +# - the first written element (index =3D=3D 0), and +# - the second element (index =3D=3D 1). +foreach_with_prefix offset { 0 1 } { + foreach_with_prefix index { 0 1 } { + + clean_restart $binfile + + if ![runto_main] { + return -1 } - -re "Hardware watchpoint $wpnum:.*New value =3D .*\r\n$gdb_prompt $" { - set got_hit 1 - send_gdb "continue\n" - exp_continue + + gdb_test_no_output "set var offset =3D $offset" + gdb_breakpoint [gdb_get_line_number "final_return"] \ + "Breakpoint $decimal at $hex" "final_return" + set watch_index [expr $offset + $index] + set test "watch data.u.size8twice\[$watch_index\]" + set wpnum 0 + gdb_test_multiple $test $test { + -re "Hardware watchpoint (\[0-9\]+): .*\r\n$gdb_prompt $" { + set wpnum $expect_out(1,string) + pass $gdb_test_name + } + -re "Watchpoint (\[0-9\]+): .*\r\n$gdb_prompt $" { + if {[istarget "arm*-*-*"]} { + untested $gdb_test_name + } else { + fail $gdb_test_name + } + } } - -re " final_return .*\r\n$gdb_prompt $" { + if {$wpnum} { + set test "continue" + set got_hit 0 + gdb_test_multiple $test $test { + -re "\r\nCould not insert hardware watchpoint .*\r\n$gdb_prompt $" { + } + -re "Hardware watchpoint $wpnum:.*New value =3D .*\r\n$gdb_prompt $" { + set got_hit 1 + send_gdb "continue\n" + exp_continue + } + -re " final_return .*\r\n$gdb_prompt $" { + } + } + gdb_assert $got_hit "size8twice write" } } - gdb_assert $got_hit "size8twice write" }