From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 23989 invoked by alias); 15 Oct 2015 08:14:58 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 22212 invoked by uid 89); 15 Oct 2015 08:14:57 -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-f49.google.com Received: from mail-pa0-f49.google.com (HELO mail-pa0-f49.google.com) (209.85.220.49) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-GCM-SHA256 encrypted) ESMTPS; Thu, 15 Oct 2015 08:14:54 +0000 Received: by payp3 with SMTP id p3so32898049pay.1 for ; Thu, 15 Oct 2015 01:14:53 -0700 (PDT) X-Received: by 10.66.132.37 with SMTP id or5mr8549687pab.5.1444896893097; Thu, 15 Oct 2015 01:14:53 -0700 (PDT) Received: from E107787-LIN (gcc2-power8.osuosl.org. [140.211.9.43]) by smtp.gmail.com with ESMTPSA id iu2sm11414566pbd.16.2015.10.15.01.14.50 (version=TLS1_2 cipher=AES128-SHA bits=128/128); Thu, 15 Oct 2015 01:14:52 -0700 (PDT) From: Yao Qi To: Pedro Alves Cc: Yao Qi , gdb-patches@sourceware.org Subject: Re: [PATCH] aarch64 multi-arch part 6: HW breakpoint on unaligned address In-Reply-To: <561D4008.90009@redhat.com> (Pedro Alves's message of "Tue, 13 Oct 2015 18:31:52 +0100") References: <1444731060-16237-1-git-send-email-yao.qi@linaro.org> <561CE5D2.8030505@redhat.com> <861tcy6b84.fsf@gmail.com> <561D4008.90009@redhat.com> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/24.3 (gnu/linux) Date: Thu, 15 Oct 2015 08:14:00 -0000 Message-ID: <86fv1c4kg7.fsf@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-IsSubscribed: yes X-SW-Source: 2015-10/txt/msg00235.txt.bz2 Pedro Alves writes: >>>> aarch64_point_is_aligned (int is_watchpoint, CORE_ADDR addr, int len) >>>> { >>>> - unsigned int alignment =3D is_watchpoint ? AARCH64_HWP_ALIGNMENT >>>> - : AARCH64_HBP_ALIGNMENT; >>>> + unsigned int alignment =3D 0; >>>> + >>>> + if (is_watchpoint) >>>> + alignment =3D AARCH64_HWP_ALIGNMENT; >>>> + else >>>> + { >>>> + /* Set alignment to 2 only if the current process is 32-bit, >>>> + since thumb instruction can be 2-byte aligned. Otherwise, set >>>> + alignment to AARCH64_HBP_ALIGNMENT. */ >>>> + alignment =3D 2; >>> >>> Is some other code doing what the comment says? I'm not seeing >>> any obvious 32-bit check. >>=20 >> No, I don't do the 32-bit check here. Ideally, we should set alignment >> to 2 only when the process is 32-bit, and still use 4 as alignment >> otherwise. However, I don't find an easy way to do the 32-bit check >> here, because this code is used by both GDB and GDBserver. We can do >> the 32-bit check in GDB and GDBserver respectively, and pass the result >> to nat/aarch64-linux-hw-point.c, but I don't like putting information do= wn >> multiple levels like this. > > At least the comment should be updated. It's quite misleading as is. In order to do 32-bit check in nat/aarch64-linux-hw-point.c, I add a new regcache interface regcache_register_size which is defined in both GDB and GDBserver. It has two arguments, regcache and number, which looks more reasonable than register_size, IMO. With regcache_register_size in place, we can check 32-bit like this, struct regcache *regcache =3D get_thread_regcache_for_ptid (current_lwp_ptid ()); /* Set alignment to 2 only if the current process is 32-bit, since thumb instruction can be 2-byte aligned. Otherwise, set alignment to AARCH64_HBP_ALIGNMENT. */ if (regcache_register_size (regcache, 0) =3D=3D 8) alignment =3D AARCH64_HBP_ALIGNMENT; else alignment =3D 2; on the other hand, a lot of register_size calls in GDB and GDBserver can be replaced by regcache_register_size. This can be done separately. Here is the patch V2, regression tested on aarch64-linux. --=20 Yao (=E9=BD=90=E5=B0=A7) =46rom 65e2b12411ee8431c24bdeb17bc3eee65f32c1b0 Mon Sep 17 00:00:00 2001 From: Yao Qi Date: Thu, 9 Jul 2015 09:28:27 +0100 Subject: [PATCH] aarch64 multi-arch part 6: HW breakpoint on unaligned addr= ess Nowadays, both aarch64 GDB and linux kernel assumes that address for setting breakpoint should be 4-byte aligned. However that is not true after we support multi-arch, because thumb instruction can be at 2-byte aligned address. Patch http://lists.infradead.org/pipermail/linux-arm-kern= el/2015-October/375141.html to linux kernel is to teach kernel to handle 2-byte aligned address for HW breakpoint, while this patch is to teach aarch64 GDB handle 2-byte aligned address. First of all, we call gdbarch_breakpoint_from_pc to get the instruction length rather than using hard-coded 4. Secondly, in GDBserver, we set length back to 2 if it is 3, because GDB encode 3 in it to indicate it is a 32-bit thumb breakpoint. Then we relax the address alignment check from 4-byte aligned to 2-byte aligned. This patch enables some tests (such as gdb.base/break-idempotent.exp, gdb.base/cond-eval-mode.exp, gdb.base/watchpoint-reuse-slot.exp,) and fixes many fails (such as gdb.base/hbreak2.exp) when the program is compiled in thumb mode on aarch64. Regression tested on aarch64-linux, both native and gdbserver. This is the last patch of multi-arch work. gdb: 2015-10-14 Yao Qi * aarch64-linux-nat.c (aarch64_linux_insert_hw_breakpoint): Call gdbarch_breakpoint_from_pc to instruction length. (aarch64_linux_remove_hw_breakpoint): Likewise. * common/common-regcache.h (regcache_register_size): Declare. * nat/aarch64-linux-hw-point.c: Include "common-regcache.h". (aarch64_point_is_aligned): Set alignment to 2 for breakpoint if the process is 32bit, otherwise set alignment to 4. (aarch64_handle_breakpoint): Update comments. * regcache.c (regcache_register_size): New function. gdb/gdbserver: 2015-10-14 Yao Qi * linux-aarch64-low.c (aarch64_insert_point): Set len to 2 if it is 3. (aarch64_remove_point): Likewise. * regcache.c (regcache_register_size): New function. diff --git a/gdb/aarch64-linux-nat.c b/gdb/aarch64-linux-nat.c index c9f439f..4d3d55a 100644 --- a/gdb/aarch64-linux-nat.c +++ b/gdb/aarch64-linux-nat.c @@ -608,11 +608,13 @@ aarch64_linux_insert_hw_breakpoint (struct target_ops= *self, { int ret; CORE_ADDR addr =3D bp_tgt->placed_address =3D bp_tgt->reqstd_address; - const int len =3D 4; + int len; const enum target_hw_bp_type type =3D hw_execute; struct aarch64_debug_reg_state *state =3D aarch64_get_debug_reg_state (ptid_get_pid (inferior_ptid)); =20 + gdbarch_breakpoint_from_pc (gdbarch, &addr, &len); + if (show_debug_regs) fprintf_unfiltered (gdb_stdlog, @@ -640,11 +642,13 @@ aarch64_linux_remove_hw_breakpoint (struct target_ops= *self, { int ret; CORE_ADDR addr =3D bp_tgt->placed_address; - const int len =3D 4; + int len =3D 4; const enum target_hw_bp_type type =3D hw_execute; struct aarch64_debug_reg_state *state =3D aarch64_get_debug_reg_state (ptid_get_pid (inferior_ptid)); =20 + gdbarch_breakpoint_from_pc (gdbarch, &addr, &len); + if (show_debug_regs) fprintf_unfiltered (gdb_stdlog, "remove_hw_breakpoint on entry (addr=3D0x%08lx, len=3D%= d))\n", diff --git a/gdb/common/common-regcache.h b/gdb/common/common-regcache.h index a922316..c470603 100644 --- a/gdb/common/common-regcache.h +++ b/gdb/common/common-regcache.h @@ -28,6 +28,11 @@ =20 extern struct regcache *get_thread_regcache_for_ptid (ptid_t ptid); =20 +/* Return the size of register numbered N in REGCACHE. This function + must be provided by the client. */ + +extern int regcache_register_size (const struct regcache *regcache, int n); + /* Read the PC register. This function must be provided by the client. */ =20 diff --git a/gdb/gdbserver/linux-aarch64-low.c b/gdb/gdbserver/linux-aarch6= 4-low.c index 9cefdda..780c5e3 100644 --- a/gdb/gdbserver/linux-aarch64-low.c +++ b/gdb/gdbserver/linux-aarch64-low.c @@ -315,9 +315,17 @@ aarch64_insert_point (enum raw_bkpt_type type, CORE_AD= DR addr, ret =3D -1; } else - ret =3D - aarch64_handle_breakpoint (targ_type, addr, len, 1 /* is_insert */, - state); + { + if (len =3D=3D 3) + { + /* LEN is 3 means the breakpoint is set on a 32-bit thumb + instruction. Set it to 2 to correctly encode length bit + mask in hardware/watchpoint control register. */ + len =3D 2; + } + ret =3D aarch64_handle_breakpoint (targ_type, addr, len, + 1 /* is_insert */, state); + } =20 if (show_debug_regs) aarch64_show_debug_reg_state (state, "insert_point", addr, len, @@ -353,9 +361,17 @@ aarch64_remove_point (enum raw_bkpt_type type, CORE_AD= DR addr, aarch64_handle_watchpoint (targ_type, addr, len, 0 /* is_insert */, state); else - ret =3D - aarch64_handle_breakpoint (targ_type, addr, len, 0 /* is_insert */, - state); + { + if (len =3D=3D 3) + { + /* LEN is 3 means the breakpoint is set on a 32-bit thumb + instruction. Set it to 2 to correctly encode length bit + mask in hardware/watchpoint control register. */ + len =3D 2; + } + ret =3D aarch64_handle_breakpoint (targ_type, addr, len, + 0 /* is_insert */, state); + } =20 if (show_debug_regs) aarch64_show_debug_reg_state (state, "remove_point", addr, len, diff --git a/gdb/gdbserver/regcache.c b/gdb/gdbserver/regcache.c index f79063c..e11b173 100644 --- a/gdb/gdbserver/regcache.c +++ b/gdb/gdbserver/regcache.c @@ -316,6 +316,14 @@ register_size (const struct target_desc *tdesc, int n) return tdesc->reg_defs[n].size / 8; } =20 +/* See common/common-regcache.h. */ + +int +regcache_register_size (const struct regcache *regcache, int n) +{ + return register_size (regcache->tdesc, n); +} + static unsigned char * register_data (struct regcache *regcache, int n, int fetch) { diff --git a/gdb/nat/aarch64-linux-hw-point.c b/gdb/nat/aarch64-linux-hw-po= int.c index bca6ec1..1a5fa6a 100644 --- a/gdb/nat/aarch64-linux-hw-point.c +++ b/gdb/nat/aarch64-linux-hw-point.c @@ -18,6 +18,7 @@ =20 #include "common-defs.h" #include "break-common.h" +#include "common-regcache.h" #include "nat/linux-nat.h" #include "aarch64-linux-hw-point.h" =20 @@ -112,8 +113,23 @@ aarch64_point_encode_ctrl_reg (enum target_hw_bp_type = type, int len) static int aarch64_point_is_aligned (int is_watchpoint, CORE_ADDR addr, int len) { - unsigned int alignment =3D is_watchpoint ? AARCH64_HWP_ALIGNMENT - : AARCH64_HBP_ALIGNMENT; + unsigned int alignment =3D 0; + + if (is_watchpoint) + alignment =3D AARCH64_HWP_ALIGNMENT; + else + { + struct regcache *regcache + =3D get_thread_regcache_for_ptid (current_lwp_ptid ()); + + /* Set alignment to 2 only if the current process is 32-bit, + since thumb instruction can be 2-byte aligned. Otherwise, set + alignment to AARCH64_HBP_ALIGNMENT. */ + if (regcache_register_size (regcache, 0) =3D=3D 8) + alignment =3D AARCH64_HBP_ALIGNMENT; + else + alignment =3D 2; + } =20 if (addr & (alignment - 1)) return 0; @@ -445,7 +461,7 @@ aarch64_handle_breakpoint (enum target_hw_bp_type type,= CORE_ADDR addr, struct aarch64_debug_reg_state *state) { /* The hardware breakpoint on AArch64 should always be 4-byte - aligned. */ + aligned, but on AArch32, it can be 2-byte aligned. */ if (!aarch64_point_is_aligned (0 /* is_watchpoint */ , addr, len)) return -1; =20 diff --git a/gdb/regcache.c b/gdb/regcache.c index 74d883a..5ee31fb 100644 --- a/gdb/regcache.c +++ b/gdb/regcache.c @@ -179,6 +179,14 @@ register_size (struct gdbarch *gdbarch, int regnum) return size; } =20 +/* See common/common-regcache.h. */ + +int +regcache_register_size (const struct regcache *regcache, int n) +{ + return register_size (get_regcache_arch (regcache), n); +} + /* The register cache for storing raw register values. */ =20 struct regcache