From patchwork Thu Aug 27 13:44:35 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Yao Qi X-Patchwork-Id: 8478 Received: (qmail 97443 invoked by alias); 27 Aug 2015 13:44:49 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 97427 invoked by uid 89); 27 Aug 2015 13:44:47 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.2 required=5.0 tests=AWL, BAYES_00, FREEMAIL_FROM, RCVD_IN_DNSWL_LOW, SPF_PASS autolearn=ham version=3.3.2 X-HELO: mail-pa0-f45.google.com Received: from mail-pa0-f45.google.com (HELO mail-pa0-f45.google.com) (209.85.220.45) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-GCM-SHA256 encrypted) ESMTPS; Thu, 27 Aug 2015 13:44:46 +0000 Received: by pacdd16 with SMTP id dd16so26336815pac.2 for ; Thu, 27 Aug 2015 06:44:44 -0700 (PDT) X-Received: by 10.68.224.162 with SMTP id rd2mr6651410pbc.33.1440683084301; Thu, 27 Aug 2015 06:44:44 -0700 (PDT) Received: from E107787-LIN.cambridge.arm.com (gcc1-power7.osuosl.org. [140.211.15.137]) by smtp.gmail.com with ESMTPSA id h4sm2538117pdd.91.2015.08.27.06.44.43 for (version=TLS1_2 cipher=ECDHE-RSA-AES128-SHA bits=128/128); Thu, 27 Aug 2015 06:44:43 -0700 (PDT) From: Yao Qi X-Google-Original-From: Yao Qi To: gdb-patches@sourceware.org Subject: [PATCH] [aarch64] Check region OK for HW watchpoint in GDBserver Date: Thu, 27 Aug 2015 14:44:35 +0100 Message-Id: <1440683075-26462-1-git-send-email-yao.qi@linaro.org> X-IsSubscribed: yes Nowadays, if user requests HW watchpoint to monitor a large memory area or unaligned area, aarch64 GDB will split into multiple aligned areas, and use multiple debugging registers to watch them. However, the registers are not updated in a transaction way. GDBserver doesn't revert updates in previous iterations if some debugging registers fail to update due to some reason, like no free debugging registers available, in the latter iteration. For example, if we have a char buf[34], and watch buf in gdb, (gdb) watch buf Hardware watchpoint 2: buf (gdb) c Continuing. infrun: clear_proceed_status_thread (Thread 13466) infrun: proceed (addr=0xffffffffffffffff, signal=GDB_SIGNAL_DEFAULT) infrun: step-over queue now empty infrun: resuming [Thread 13466] for step-over Sending packet: $m410838,22#35...Packet received: 00000000000000000000000000000000000000000000000000000000000000000000 infrun: skipping breakpoint: stepping past insn at: 0x400524 infrun: skipping breakpoint: stepping past insn at: 0x400524 Sending packet: $Z2,410838,22#80...Packet received: E01 <----- [1] Packet Z2 (write-watchpoint) is supported Sending packet: $Z0,7fb7fe0a8c,4#43...Packet received: OK Warning: Could not insert hardware watchpoint 2. Could not insert hardware breakpoints: You may have requested too many hardware breakpoints/watchpoints. GDB receives E01 for Z2 packet [1] but GDBserver updates the debugging register status, insert_point (addr=0x00410838, len=34, type=hw-write-watchpoint): BREAKPOINTs: BP0: addr=0x0, ctrl=0x00000000, ref.count=0 BP1: addr=0x0, ctrl=0x00000000, ref.count=0 BP2: addr=0x0, ctrl=0x00000000, ref.count=0 BP3: addr=0x0, ctrl=0x00000000, ref.count=0 BP4: addr=0x0, ctrl=0x00000000, ref.count=0 BP5: addr=0x0, ctrl=0x00000000, ref.count=0 WATCHPOINTs: WP0: addr=0x410850, ctrl=0x00001ff5, ref.count=1 WP1: addr=0x410848, ctrl=0x00001ff5, ref.count=1 WP2: addr=0x410840, ctrl=0x00001ff5, ref.count=1 WP3: addr=0x410838, ctrl=0x00001ff5, ref.count=1 four debugging registers can not monitor 34-byte long area, so the last iteration of updating debugging register state fails but previous iterations succeed. This makes GDB think no HW watchpoint is inserted but some debugging registers are used. This problem was exposed by "watch buf" gdb.base/watchpoint.exp with aarch64 GDBserver debugging arm 32-bit program. The buf is 30-byte long but 4-byte aligned, and four debugging registers can't cover 34-byte (extend 4 bytes to be 8-byte aligned) area. However, this problem does exist on non-multi-arch debugging scenario as well. This patch moves code in aarch64_linux_region_ok_for_hw_watchpoint to aarch64_linux_region_ok_for_watchpoint in nat/aarch64-linux-hw-point.c. Then, checks with aarch64_linux_region_ok_for_watchpoint, like what we are doing in GDB. If the region is OK, call aarch64_handle_watchpoint. Regression tested on aarch64 with both 64-bit program and 32-bit program. Some fails in gdb.base/watchpoint.exp are fixed. gdb: 2015-08-27 Yao Qi * aarch64-linux-nat.c (aarch64_linux_region_ok_for_hw_watchpoint): Move code to aarch64_linux_region_ok_for_watchpoint. Call aarch64_linux_region_ok_for_watchpoint. * nat/aarch64-linux-hw-point.c (aarch64_linux_region_ok_for_watchpoint): New function. * nat/aarch64-linux-hw-point.h (aarch64_linux_region_ok_for_watchpoint): Declare it. gdb/gdbserver: 2015-08-27 Yao Qi * linux-aarch64-low.c (aarch64_insert_point): Call aarch64_handle_watchpoint if aarch64_linux_region_ok_for_watchpoint returns true. --- gdb/aarch64-linux-nat.c | 33 +------------------------------ gdb/gdbserver/linux-aarch64-low.c | 10 +++++++--- gdb/nat/aarch64-linux-hw-point.c | 41 +++++++++++++++++++++++++++++++++++++++ gdb/nat/aarch64-linux-hw-point.h | 2 ++ 4 files changed, 51 insertions(+), 35 deletions(-) diff --git a/gdb/aarch64-linux-nat.c b/gdb/aarch64-linux-nat.c index f6edb68..345a63f 100644 --- a/gdb/aarch64-linux-nat.c +++ b/gdb/aarch64-linux-nat.c @@ -717,38 +717,7 @@ static int aarch64_linux_region_ok_for_hw_watchpoint (struct target_ops *self, CORE_ADDR addr, int len) { - CORE_ADDR aligned_addr; - - /* Can not set watchpoints for zero or negative lengths. */ - if (len <= 0) - return 0; - - /* Must have hardware watchpoint debug register(s). */ - if (aarch64_num_wp_regs == 0) - return 0; - - /* We support unaligned watchpoint address and arbitrary length, - as long as the size of the whole watched area after alignment - doesn't exceed size of the total area that all watchpoint debug - registers can watch cooperatively. - - This is a very relaxed rule, but unfortunately there are - limitations, e.g. false-positive hits, due to limited support of - hardware debug registers in the kernel. See comment above - aarch64_align_watchpoint for more information. */ - - aligned_addr = addr & ~(AARCH64_HWP_MAX_LEN_PER_REG - 1); - if (aligned_addr + aarch64_num_wp_regs * AARCH64_HWP_MAX_LEN_PER_REG - < addr + len) - return 0; - - /* All tests passed so we are likely to be able to set the watchpoint. - The reason that it is 'likely' rather than 'must' is because - we don't check the current usage of the watchpoint registers, and - there may not be enough registers available for this watchpoint. - Ideally we should check the cached debug register state, however - the checking is costly. */ - return 1; + return aarch64_linux_region_ok_for_watchpoint (addr, len); } /* Implement the "to_stopped_data_address" target_ops method. */ diff --git a/gdb/gdbserver/linux-aarch64-low.c b/gdb/gdbserver/linux-aarch64-low.c index dbe4951..8ebf010 100644 --- a/gdb/gdbserver/linux-aarch64-low.c +++ b/gdb/gdbserver/linux-aarch64-low.c @@ -302,9 +302,13 @@ aarch64_insert_point (enum raw_bkpt_type type, CORE_ADDR addr, targ_type = raw_bkpt_type_to_target_hw_bp_type (type); if (targ_type != hw_execute) - ret = - aarch64_handle_watchpoint (targ_type, addr, len, 1 /* is_insert */, - state); + { + if (aarch64_linux_region_ok_for_watchpoint (addr, len)) + ret = aarch64_handle_watchpoint (targ_type, addr, len, + 1 /* is_insert */, state); + else + ret = -1; + } else ret = aarch64_handle_breakpoint (targ_type, addr, len, 1 /* is_insert */, diff --git a/gdb/nat/aarch64-linux-hw-point.c b/gdb/nat/aarch64-linux-hw-point.c index a3c923a..c4bcabe 100644 --- a/gdb/nat/aarch64-linux-hw-point.c +++ b/gdb/nat/aarch64-linux-hw-point.c @@ -517,6 +517,7 @@ aarch64_handle_watchpoint (enum target_hw_bp_type type, CORE_ADDR addr, int len, int is_insert, struct aarch64_debug_reg_state *state) { + if (aarch64_point_is_aligned (1 /* is_watchpoint */ , addr, len)) return aarch64_handle_aligned_watchpoint (type, addr, len, is_insert, state); @@ -645,3 +646,43 @@ aarch64_linux_get_debug_reg_capacity (int tid) aarch64_num_bp_regs = 0; } } + +/* Return true if we can watch a memory region that starts address + ADDR and whose length is LEN in bytes. */ + +int +aarch64_linux_region_ok_for_watchpoint (CORE_ADDR addr, int len) +{ + CORE_ADDR aligned_addr; + + /* Can not set watchpoints for zero or negative lengths. */ + if (len <= 0) + return 0; + + /* Must have hardware watchpoint debug register(s). */ + if (aarch64_num_wp_regs == 0) + return 0; + + /* We support unaligned watchpoint address and arbitrary length, + as long as the size of the whole watched area after alignment + doesn't exceed size of the total area that all watchpoint debug + registers can watch cooperatively. + + This is a very relaxed rule, but unfortunately there are + limitations, e.g. false-positive hits, due to limited support of + hardware debug registers in the kernel. See comment above + aarch64_align_watchpoint for more information. */ + + aligned_addr = addr & ~(AARCH64_HWP_MAX_LEN_PER_REG - 1); + if (aligned_addr + aarch64_num_wp_regs * AARCH64_HWP_MAX_LEN_PER_REG + < addr + len) + return 0; + + /* All tests passed so we are likely to be able to set the watchpoint. + The reason that it is 'likely' rather than 'must' is because + we don't check the current usage of the watchpoint registers, and + there may not be enough registers available for this watchpoint. + Ideally we should check the cached debug register state, however + the checking is costly. */ + return 1; +} diff --git a/gdb/nat/aarch64-linux-hw-point.h b/gdb/nat/aarch64-linux-hw-point.h index a27a201..a3cf7d9 100644 --- a/gdb/nat/aarch64-linux-hw-point.h +++ b/gdb/nat/aarch64-linux-hw-point.h @@ -182,4 +182,6 @@ void aarch64_linux_get_debug_reg_capacity (int tid); struct aarch64_debug_reg_state *aarch64_get_debug_reg_state (pid_t pid); +int aarch64_linux_region_ok_for_watchpoint (CORE_ADDR addr, int len); + #endif /* AARCH64_LINUX_HW_POINT_H */