From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 111125 invoked by alias); 7 Dec 2015 15:18:08 -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 111115 invoked by uid 89); 7 Dec 2015 15:18:07 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.3 required=5.0 tests=AWL,BAYES_00,FREEMAIL_FROM,RCVD_IN_DNSWL_LOW,SPF_PASS autolearn=ham version=3.3.2 X-HELO: mail-pf0-f180.google.com Received: from mail-pf0-f180.google.com (HELO mail-pf0-f180.google.com) (209.85.192.180) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-GCM-SHA256 encrypted) ESMTPS; Mon, 07 Dec 2015 15:18:05 +0000 Received: by pfu207 with SMTP id 207so66324142pfu.2 for ; Mon, 07 Dec 2015 07:18:04 -0800 (PST) X-Received: by 10.98.9.194 with SMTP id 63mr44219879pfj.30.1449501484197; Mon, 07 Dec 2015 07:18:04 -0800 (PST) Received: from E107787-LIN (gcc1-power7.osuosl.org. [140.211.15.137]) by smtp.gmail.com with ESMTPSA id q20sm35359243pfi.5.2015.12.07.07.17.59 (version=TLS1_2 cipher=AES128-SHA bits=128/128); Mon, 07 Dec 2015 07:18:03 -0800 (PST) From: Yao Qi To: Antoine Tremblay Cc: Subject: Re: [PATCH v6 5/7] Support software single step on ARM in GDBServer. References: <1449254773-19019-1-git-send-email-antoine.tremblay@ericsson.com> <1449254773-19019-6-git-send-email-antoine.tremblay@ericsson.com> Date: Mon, 07 Dec 2015 15:18:00 -0000 In-Reply-To: <1449254773-19019-6-git-send-email-antoine.tremblay@ericsson.com> (Antoine Tremblay's message of "Fri, 4 Dec 2015 13:46:11 -0500") Message-ID: <8637ves2xp.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-12/txt/msg00136.txt.bz2 Antoine Tremblay writes: > gdb/ChangeLog: > * Makefile.in: Add arm-get-next-pcs.c/o, arm-linux.c/o. > to all arm targets. Please describe how Makefile.in is changed, as I suggested in previous patc= h. > * aarch32-linux-nat.c: Include arch/arm-get-next-pcs.h. > * arch/arm-get-next-pcs.c: New file. > * arch/arm-get-next-pcs.h: New file. > * arch/arm-linux.h: New file. > * arch/arm-linux.c: New file. > * arm-linux-nat.c: Include arch/arm.h, arch/arm-get-next-pcs.h > arch/arm-linux.h. > * arm-linux-tdep.c: Include arch/arm.h,arch/arm-get-next-pcs.h > arch/arm-linux.h. > (arm_linux_get_next_pcs_ops): New struct. > (ARM_SIGCONTEXT_R0, ARM_UCONTEXT_SIGCONTEXT, > ARM_OLD_RT_SIGFRAME_SIGINFO, ARM_OLD_RT_SIGFRAME_UCONTEXT, > ARM_NEW_RT_SIGFRAME_UCONTEXT, ARM_NEW_SIGFRAME_MAGIC): Move stack > layout defines to arch/arm-linux.h. > (arm_linux_sigreturn_next_pc_offset): Move to arch/arm-linux.c. > (arm_linux_software_single_step): Adjust for > arm_get_next_pcs implementation. > * arm-tdep.c: Include arch/arm-get-next-pcs.h. > (arm_get_next_pcs_ops): New struct. > (submask): Move macro to arm-get-next-pcs.h. > (bit): Likewise. > (bits): Likewise. > (sbits): Likewise. > (BranchDest): Likewise. > (thumb_instruction_changes_pc): Move to arm-get-next-pcs.c > (thumb2_instruction_changes_pc): Likewise. > (arm_instruction_changes_pc): Likewise. I feel they should be moved to arch/arm.c, because they are not about getting the next pcs. > (shifted_reg_val): Likewise. > (thumb_advance_itstate): Likewise. and this one. > (thumb_get_next_pc_raw): Likewise. > (arm_get_next_pc_raw): Likewise. > (arm_get_next_pc): Likewise. > (thumb_deal_with_atomic_sequence_raw): Likewise. > (arm_deal_with_atomic_sequence_raw): Likewise. > (arm_deal_with_atomic_sequence): Likewise. > (arm_get_next_pcs_read_memory_unsigned_integer): New function. > (arm_get_next_pcs_addr_bits_remove): Likewise. > (arm_get_next_pcs_syscall_next_pc): Likewise. > (arm_get_next_pcs_is_thumb): Likewise. > (arm_software_single_step): Adjust for arm_get_next_pcs > implementation. > * arm-tdep.h: (arm_get_next_pc): Remove declaration. > (arm_get_next_pcs_read_memory_unsigned_integer): > New declaration. > (arm_get_next_pcs_addr_bits_remove): Likewise. > (arm_get_next_pcs_syscall_next_pc): Likewise. > (arm_software_single_step): Likewise. > (arm_deal_with_atomic_sequence: Remove declaration. > (arm_get_next_pcs_is_thumb): New delcaration. > * arm-wince-tdep.c: Include arch/arm-get-next-pcs.h. > * armbsd-tdep.c: Likewise. > * armnbsd-tdep.c: Likewise. > * armobsd-tdep.c: Likewise. Why do we need to include arm-get-next-pcs.h in these *-tdep.c files? > * common/gdb_vecs.h: Add CORE_ADDR vector definition. > * configure.tgt: Add arm-get-next-pcs.o, arm-linux.o to all > relevent arm targets. > * symtab.h: Move CORE_ADDR vector definition to gdb_vecs.h. > > gdb/gdbserver/ChangeLog: > * Makefile.in : Add arm-get-next-pcs.c/o, arm-linux.c/o. Please describe what are changed. > * configure.srv: Add arm-get-next-pcs.o, arm-linux.o > to needed arm targets. > * linux-arm-low.c: Include arch/arm-linux.h, > aarch/arm-get-next-pcs.h. > (get_next_pcs_ops): New struct. > (get_next_pcs_addr_bits_remove): New function. > (get_next_pcs_is_thumb): New function. > (get_next_pcs_read_memory_unsigned_integer): Likewise. > (arm_sigreturn_next_pc): Likewise. > (get_next_pcs_syscall_next_pc): Likewise. > (arm_gdbserver_get_next_pcs): Likewise. > (struct linux_target_ops) : > Initialize. > * linux-low.h: Move CORE_ADDR vector definition to gdb_vecs.h. > * server.h: Include gdb_vecs.h. > --- > gdb/Makefile.in | 16 +- > gdb/aarch32-linux-nat.c | 1 + > gdb/arch/arm-get-next-pcs.c | 1216 +++++++++++++++++++++++++++++++++ > gdb/arch/arm-get-next-pcs.h | 95 +++ > gdb/arch/arm-linux.c | 59 ++ > gdb/arch/arm-linux.h | 73 ++ > gdb/arm-linux-nat.c | 2 + > gdb/arm-linux-tdep.c | 136 ++-- > gdb/arm-tdep.c | 1495 +++++------------------------------= ------ > gdb/arm-tdep.h | 18 +- > gdb/arm-wince-tdep.c | 1 + > gdb/armbsd-tdep.c | 1 + > gdb/armnbsd-tdep.c | 1 + > gdb/armobsd-tdep.c | 1 + > gdb/common/gdb_vecs.h | 2 + > gdb/configure.tgt | 20 +- > gdb/gdbserver/Makefile.in | 9 +- > gdb/gdbserver/configure.srv | 2 + > gdb/gdbserver/linux-arm-low.c | 144 +++- > gdb/gdbserver/linux-low.h | 2 - > gdb/gdbserver/server.h | 1 + > gdb/symtab.h | 2 - > 22 files changed, 1871 insertions(+), 1426 deletions(-) > create mode 100644 gdb/arch/arm-get-next-pcs.c > create mode 100644 gdb/arch/arm-get-next-pcs.h > create mode 100644 gdb/arch/arm-linux.c > create mode 100644 gdb/arch/arm-linux.h > > + > +/* Checks for an atomic sequence of instructions beginning with a LDREX{= ,B,H,D} > + instruction and ending with a STREX{,B,H,D} instruction. If such a s= equence > + is found, attempt to step through it. The end of the sequence addres= s is > + added to the next_pcs list. */ > + > +static int > +thumb_deal_with_atomic_sequence_raw (struct arm_get_next_pcs *self, > + CORE_ADDR pc, > + VEC (CORE_ADDR) **next_pcs) We can return VEC (CORE_ADDR) * here, if it is NULL, the sequence is not found, otherwise, the end of the sequence addresses are returned in the vec= tor. > + > +/* Checks for an atomic sequence of instructions beginning with a LDREX{= ,B,H,D} > + instruction and ending with a STREX{,B,H,D} instruction. If such a s= equence > + is found, attempt to step through it. The end of the sequence addres= s is > + added to the next_pcs list. */ > + > +static int > +arm_deal_with_atomic_sequence_raw (struct arm_get_next_pcs *self, > + CORE_ADDR pc, > + VEC (CORE_ADDR) **next_pcs) Likewise. > + > +/* See arm-get-next-pcs.h. */ > + > +VEC (CORE_ADDR) * > +arm_get_next_pcs (struct arm_get_next_pcs *self, CORE_ADDR pc) > +{ > + VEC (CORE_ADDR) *next_pcs =3D NULL; > + > + if (self->ops->is_thumb (self)) > + { > + if (!thumb_deal_with_atomic_sequence_raw (self, pc, &next_pcs)) > + return thumb_get_next_pcs_raw (self, pc, &next_pcs); Why do we need to pass next_pcs to thumb_get_next_pcs_raw? Isn't simpler to write it like this, next_pcs =3D thumb_deal_with_atomic_sequence_raw (self, pc); if (next_pcs =3D=3D NULL) next_pcs =3D thumb_get_next_pcs_raw (self, pc); > + } > + else > + { > + if (!arm_deal_with_atomic_sequence_raw (self, pc, &next_pcs)) > + return arm_get_next_pcs_raw (self, pc, &next_pcs); > + } > + > + return next_pcs; > +} > diff --git a/gdb/arch/arm-get-next-pcs.h b/gdb/arch/arm-get-next-pcs.h > new file mode 100644 > index 0000000..39921e0 > --- /dev/null > +++ b/gdb/arch/arm-get-next-pcs.h > @@ -0,0 +1,95 @@ > +/* Common code for ARM software single stepping support. > + > + Copyright (C) 1988-2015 Free Software Foundation, Inc. > + > + This file is part of GDB. > + > + 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 .= */ > + > +#ifndef ARM_GET_NEXT_PCS_H > +#define ARM_GET_NEXT_PCS_H 1 > + > +/* Support routines for instruction parsing. */ > +#define submask(x) ((1L << ((x) + 1)) - 1) > +#define bits(obj,st,fn) (((obj) >> (st)) & submask ((fn) - (st))) > +#define bit(obj,st) (((obj) >> (st)) & 1) > +#define sbits(obj,st,fn) \ > + ((long) (bits(obj,st,fn) | ((long) bit(obj,fn) * ~ submask (fn - st)))) > +#define BranchDest(addr,instr) \ > + ((CORE_ADDR) (((unsigned long) (addr)) + 8 + (sbits (instr, 0, 23) << = 2))) > + These macros should be moved to arch/arm.h. --=20 Yao (=E9=BD=90=E5=B0=A7)