From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 8402 invoked by alias); 28 Sep 2015 10:29:54 -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 7130 invoked by uid 89); 28 Sep 2015 10:29:53 -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-f44.google.com Received: from mail-pa0-f44.google.com (HELO mail-pa0-f44.google.com) (209.85.220.44) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-GCM-SHA256 encrypted) ESMTPS; Mon, 28 Sep 2015 10:29:51 +0000 Received: by padhy16 with SMTP id hy16so171132524pad.1 for ; Mon, 28 Sep 2015 03:29:50 -0700 (PDT) X-Received: by 10.68.179.133 with SMTP id dg5mr26420398pbc.0.1443436190148; Mon, 28 Sep 2015 03:29:50 -0700 (PDT) Received: from E107787-LIN (power-aix.osuosl.org. [140.211.15.154]) by smtp.gmail.com with ESMTPSA id rb8sm18604666pab.14.2015.09.28.03.29.47 (version=TLSv1.2 cipher=RC4-SHA bits=128/128); Mon, 28 Sep 2015 03:29:49 -0700 (PDT) From: Yao Qi To: Antoine Tremblay Cc: Subject: Re: [PATCH 3/5] Add support for ARM breakpoint types in GDBServer. References: <1442577749-6650-1-git-send-email-antoine.tremblay@ericsson.com> <1442577749-6650-4-git-send-email-antoine.tremblay@ericsson.com> Date: Mon, 28 Sep 2015 10:29:00 -0000 In-Reply-To: <1442577749-6650-4-git-send-email-antoine.tremblay@ericsson.com> (Antoine Tremblay's message of "Fri, 18 Sep 2015 08:02:27 -0400") Message-ID: <8637xy96qx.fsf@gmail.com> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/24.3 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-IsSubscribed: yes X-SW-Source: 2015-09/txt/msg00599.txt.bz2 Antoine Tremblay writes: > gdb/ChangeLog: > * Makefile.in: Add arm-common.o. > * arm-tdep.c (thumb_insn_size): Move to arm-common.h/c. "Move to arm-common.c". Some macros are moved, we should mention the move here as well. > * common/arm-common.c: New file. > * common/arm-common.h: New file. arm-common.c is not a good file name to me, how about arm-insn.c? Please move this file to arch/ directory rather than common/ > * configure.tgt: Add arm-common.o. > * gdbserver/Makefile.in: Add arm-common.c/o. ChangeLog entries for GDBserver should be written separately. > * gdbserver/configure.srv: Add arm-common.o. > * gdbserver/linux-arm-low.c (arm_breakpoint_at): Adjust for > breakpoint types. > (arm_breakpoint_from_pc): New function. arm_breakpoint_from_pc is not a new function, but arm_is_thumb_mode is. > diff --git a/gdb/common/arm-common.c b/gdb/common/arm-common.c > new file mode 100644 > index 0000000..861b249 > --- /dev/null > +++ b/gdb/common/arm-common.c > @@ -0,0 +1,32 @@ > +/* Common code for generic ARM support. > + > + Copyright (C) 2015 Free Software Foundation, Inc. > + Contents in file are moved from existing file, so we should still keep the year range of the original file. It should be Copyright (C) 1988-2015 Free Software Foundation, Inc. > diff --git a/gdb/configure.tgt b/gdb/configure.tgt > index c42b4df..e831f59 100644 > --- a/gdb/configure.tgt > +++ b/gdb/configure.tgt > @@ -89,7 +89,7 @@ arm*-wince-pe | arm*-*-mingw32ce*) > ;; > arm*-*-linux*) > # Target: ARM based machine running GNU/Linux > - gdb_target_obs=3D"arm-tdep.o arm-linux-tdep.o glibc-tdep.o \ > + gdb_target_obs=3D"arm-common.o arm-tdep.o arm-linux-tdep.o glibc-tdep.o= \ > solib-svr4.o symfile-mem.o linux-tdep.o linux-record.o" > build_gdbserver=3Dyes since arm-common.o is moved out of arm-tdep.o, so it should be added to every target which uses arm-tdep.o, such as aarch64*-*-linux*, arm*-wince-pe, etc. > diff --git a/gdb/gdbserver/linux-arm-low.c b/gdb/gdbserver/linux-arm-low.c > index 367c704..15ecb70 100644 > --- a/gdb/gdbserver/linux-arm-low.c > +++ b/gdb/gdbserver/linux-arm-low.c > @@ -30,6 +30,8 @@ > #include "nat/gdb_ptrace.h" > #include >=20=20 > +#include "common/arm-common.h" > + > /* Defined in auto-generated files. */ > void init_registers_arm (void); > extern const struct target_desc *tdesc_arm; > @@ -234,19 +236,28 @@ arm_set_pc (struct regcache *regcache, CORE_ADDR pc) > } >=20=20 > /* Correct in either endianness. */ > -static const unsigned long arm_breakpoint =3D 0xef9f0001; > -#define arm_breakpoint_len 4 > -static const unsigned short thumb_breakpoint =3D 0xde01; > -static const unsigned short thumb2_breakpoint[] =3D { 0xf7f0, 0xa000 }; > +#define arm_abi_breakpoint 0xef9f0001UL >=20=20 > /* For new EABI binaries. We recognize it regardless of which ABI > is used for gdbserver, so single threaded debugging should work > OK, but for multi-threaded debugging we only insert the current > ABI's breakpoint instruction. For now at least. */ > -static const unsigned long arm_eabi_breakpoint =3D 0xe7f001f0; > +#define arm_eabi_breakpoint 0xe7f001f0UL > + > +#ifndef __ARM_EABI__ > +static const unsigned long arm_breakpoint =3D arm_abi_breakpoint; > +#else > +static const unsigned long arm_breakpoint =3D arm_eabi_breakpoint; > +#endif > + > +#define arm_breakpoint_len 4 > +static const unsigned short thumb_breakpoint =3D 0xde01; > +#define thumb_breakpoint_len 2 > +static const unsigned short thumb2_breakpoint[] =3D { 0xf7f0, 0xa000 }; > +#define thumb2_breakpoint_len 4 I am confused by your changes here. Why do you change them? >=20=20 > static int > -arm_breakpoint_at (CORE_ADDR where) > +arm_is_thumb_mode (void) > { > struct regcache *regcache =3D get_thread_regcache (current_thread, 1); > unsigned long cpsr; > @@ -254,6 +265,17 @@ arm_breakpoint_at (CORE_ADDR where) > collect_register_by_name (regcache, "cpsr", &cpsr); >=20=20 > if (cpsr & 0x20) > + return 1; > + else > + return 0; Indentation looks odd. > +} > + > +/* Returns 1 if there is a software breakpoint at location. */ > + > +static int > +arm_breakpoint_at (CORE_ADDR where) > +{ > + if (arm_is_thumb_mode ()) > { > /* Thumb mode. */ > unsigned short insn; > @@ -275,7 +297,7 @@ arm_breakpoint_at (CORE_ADDR where) > unsigned long insn; >=20=20 > (*the_target->read_memory) (where, (unsigned char *) &insn, 4); > - if (insn =3D=3D arm_breakpoint) > + if (insn =3D=3D arm_abi_breakpoint) > return 1; >=20=20 > if (insn =3D=3D arm_eabi_breakpoint) > @@ -285,6 +307,53 @@ arm_breakpoint_at (CORE_ADDR where) > return 0; > } >=20=20 > +/* Determine the type and size of breakpoint to insert at PCPTR. Uses > + the program counter value to determine whether a 16-bit or 32-bit > + breakpoint should be used. It returns a pointer to a string of > + bytes that encode a breakpoint instruction, stores the length of > + the string to *lenptr, and adjusts the program counter (if > + necessary) to point to the actual memory location where the > + breakpoint should be inserted. */ > + > +static const unsigned char * > +arm_breakpoint_from_pc (CORE_ADDR *pcptr, int *lenptr) > +{ > + /* Default if no pc is set to arm breakpoint. */ > + if (pcptr =3D=3D NULL) Such NULL checking is no longer needed, right? --=20 Yao (=E9=BD=90=E5=B0=A7)