From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 107199 invoked by alias); 11 Dec 2015 17:52:57 -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 107186 invoked by uid 89); 11 Dec 2015 17:52:57 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.6 required=5.0 tests=AWL,BAYES_00,SPF_PASS autolearn=ham version=3.3.2 X-HELO: usplmg20.ericsson.net Received: from usplmg20.ericsson.net (HELO usplmg20.ericsson.net) (198.24.6.45) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-SHA encrypted) ESMTPS; Fri, 11 Dec 2015 17:52:55 +0000 Received: from EUSAAHC003.ericsson.se (Unknown_Domain [147.117.188.81]) by usplmg20.ericsson.net (Symantec Mail Security) with SMTP id 8B.F2.06940.8EC0B665; Fri, 11 Dec 2015 18:50:32 +0100 (CET) Received: from [142.133.110.95] (147.117.188.8) by smtp-am.internal.ericsson.com (147.117.188.83) with Microsoft SMTP Server id 14.3.248.2; Fri, 11 Dec 2015 12:52:52 -0500 Subject: Re: [PATCH v7.2] Support software single step on ARM in GDBServer To: Yao Qi References: <566AF5A0.2040606@ericsson.com> <1449851185-9066-1-git-send-email-antoine.tremblay@ericsson.com> <86lh90q4hf.fsf@gmail.com> CC: From: Antoine Tremblay Message-ID: <566B0D74.4080205@ericsson.com> Date: Fri, 11 Dec 2015 17:52:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.4.0 MIME-Version: 1.0 In-Reply-To: <86lh90q4hf.fsf@gmail.com> Content-Type: text/plain; charset="utf-8"; format=flowed Content-Transfer-Encoding: 7bit X-IsSubscribed: yes X-SW-Source: 2015-12/txt/msg00242.txt.bz2 On 12/11/2015 12:28 PM, Yao Qi wrote: > Antoine Tremblay writes: > >> Change the name of the ops read_memory_unsigned_integer to read_mem_uint >> >> OK like that ? > > Yes. > >> >> --- >> This patch teaches GDBServer how to software single step on ARM >> linux by sharing code with GDB. >> >> The arm_get_next_pcs function in GDB is now shared with GDBServer. So that >> GDBServer can use the function to return the possible addresses of the next PC. >> >> A proper shared context was also needed so that we could share the code, this >> context is described in the arm_get_next_pcs structure. >> >> Testing : >> >> No regressions, tested on ubuntu 14.04 ARMv7 and x86. >> With gdbserver-{native,extended} / { -marm -mthumb } >> >> gdb/ChangeLog: >> >> * Makefile.in (ALL_TARGET_OBS): Append arm-get-next-pcs.o, arm-linux.o. >> (ALLDEPFILES): Append arm-get-next-pcs.c, arm-linux.c >> (arm-linux.o): New rule. >> (arm-get-next-pcs.o): New rule. >> * 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.c: Include common-regcache.c. >> (thumb_advance_itstate): Moved from arm-tdep.c. >> (arm_instruction_changes_pc): Likewise. >> (thumb_instruction_changes_pc): Likewise. >> (thumb2_instruction_changes_pc): Likewise. >> (shifted_reg_val): Likewise. >> * arm.h (submask): Move macro from arm-tdep.h >> (bit): Likewise. >> (bits): Likewise. >> (sbits): Likewise. >> (BranchDest): Likewise. >> (thumb_advance_itstate): Moved declaration from arm-tdep.h >> (arm_instruction_changes_pc): Likewise. >> (thumb_instruction_changes_pc): Likewise. >> (thumb2_instruction_changes_pc): Likewise. >> (shifted_reg_val): Likewise. >> * 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.h. >> (bit): Likewise. >> (bits): Likewise. >> (sbits): Likewise. >> (BranchDest): Likewise. >> (thumb_instruction_changes_pc): Move to arm.c >> (thumb2_instruction_changes_pc): Likewise. >> (arm_instruction_changes_pc): Likewise. >> (shifted_reg_val): Likewise. >> (thumb_advance_itstate): Likewise. >> (thumb_get_next_pc_raw): Move to arm-get-next-pcs.c. >> (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_get_next_pcs_is_thumb): Likewise. >> (arm_deal_with_atomic_sequence: Remove declaration. >> * common/gdb_vecs.h: Add CORE_ADDR vector definition. >> * configure.tgt (aarch64*-*-linux): Add arm-get-next-pcs.o, arm-linux.o. > > ChangeLog line max length is 74. > https://sourceware.org/gdb/wiki/ContributionChecklist > Yes thanks, I have to edit my changelog mode for that. Fixed. >> (arm*-wince-pe): Add arm-get-next-pcs.o. >> (arm*-*-linux*): Add arm-get-next-pcs.o, arm-linux.o, arm-get-next-pcs.o >> (arm*-*-netbsd* | arm*-*-knetbsd*-gnu): Add arm-get-next-pcs.o. > > Replace "|" with ",". > Done. >> (arm*-*-openbsd*): Likewise. >> (arm*-*-symbianelf*): Likewise. >> (arm*-*-*): Likewise. >> * symtab.h: Move CORE_ADDR vector definition to gdb_vecs.h. >> >> gdb/gdbserver/ChangeLog: >> >> * Makefile.in (SFILES): Append arch/arm-linux.c, >> arch/arm-get-next-pcs.c. >> (arm-linux.o): New rule. >> (arm-get-next-pcs.o): New rule. >> * configure.srv (arm*-*-linux*): Add arm-get-next-pcs.o, >> arm-linux.o. >> * linux-aarch32-low.c (arm_abi_breakpoint): Remove macro. Moved >> to linux-aarch32-low.c. >> (arm_eabi_breakpoint, arm_breakpoint): Likewise. >> (arm_breakpoint_len, thumb_breakpoint): Likewise. >> (thumb_breakpoint_len, thumb2_breakpoint): Likewise. >> (thumb2_breakpoint_len): Likewise. > > These are not started with TAB. > Fixed. >> + >> + if ((inst1 & 0xff00) == 0xbd00) /* pop {rlist, pc} */ >> + { >> + CORE_ADDR sp; >> + >> + /* Fetch the saved PC from the stack. It's stored above >> + all of the other registers. */ >> + offset = bitcount (bits (inst1, 0, 7)) * INT_REGISTER_SIZE; >> + sp = regcache_raw_get_unsigned (regcache, ARM_SP_REGNUM); >> + nextpc = self->ops->read_mem_uint (sp + offset, 4, byte_order); > > Redundant spaces. > Yes this somehow was showing up ok in emacs, strange, but fixed. Thanks for the review!!