From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 71203 invoked by alias); 11 Dec 2015 12:41: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 71151 invoked by uid 89); 11 Dec 2015 12:41:54 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.1 required=5.0 tests=AWL,BAYES_00,KAM_LAZY_DOMAIN_SECURITY autolearn=no version=3.3.2 X-HELO: usplmg21.ericsson.net Received: from usplmg21.ericsson.net (HELO usplmg21.ericsson.net) (198.24.6.65) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-SHA encrypted) ESMTPS; Fri, 11 Dec 2015 12:41:52 +0000 Received: from EUSAAHC002.ericsson.se (Unknown_Domain [147.117.188.78]) by usplmg21.ericsson.net (Symantec Mail Security) with SMTP id A5.BB.32102.884CA665; Fri, 11 Dec 2015 13:41:44 +0100 (CET) Received: from [142.133.110.95] (147.117.188.8) by smtp-am.internal.ericsson.com (147.117.188.80) with Microsoft SMTP Server id 14.3.248.2; Fri, 11 Dec 2015 07:41:49 -0500 Subject: Re: [PATCH v7 4/8] Refactor arm_software_single_step to use regcache. To: Yao Qi References: <1449583641-18156-1-git-send-email-antoine.tremblay@ericsson.com> <1449583641-18156-5-git-send-email-antoine.tremblay@ericsson.com> <86egetqkrg.fsf@gmail.com> CC: From: Antoine Tremblay Message-ID: <566AC48C.3080006@ericsson.com> Date: Fri, 11 Dec 2015 12:41: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: <86egetqkrg.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/msg00225.txt.bz2 On 12/11/2015 06:37 AM, Yao Qi wrote: > Antoine Tremblay writes: > > Hi Antoine, > Patch is OK to me, two nits below. > >> diff --git a/gdb/arm-linux-tdep.c b/gdb/arm-linux-tdep.c >> index acb5701..eb4341c 100644 >> --- a/gdb/arm-linux-tdep.c >> +++ b/gdb/arm-linux-tdep.c >> @@ -808,6 +808,68 @@ arm_linux_sigreturn_return_addr (struct frame_info *frame, >> return 0; >> } >> >> +/* Calculate the offset from stack pointer of the pc register on the stack >> + in the case of a sigreturn or sigreturn_rt syscall. */ >> +static int >> +arm_linux_sigreturn_next_pc_offset (unsigned long sp, >> + unsigned long sp_data, >> + unsigned long svc_number, >> + int is_sigreturn) >> +{ >> + /* Offset of R0 register. */ >> + int r0_offset = 0; >> + /* Offset of PC register. */ >> + int pc_offset = 0; >> + >> + if (is_sigreturn) >> + { >> + if (sp_data == ARM_NEW_SIGFRAME_MAGIC) >> + r0_offset = ARM_UCONTEXT_SIGCONTEXT + ARM_SIGCONTEXT_R0; >> + else >> + r0_offset = ARM_SIGCONTEXT_R0; >> + } >> + else >> + { >> + if (sp_data == sp + ARM_OLD_RT_SIGFRAME_SIGINFO) >> + r0_offset = ARM_OLD_RT_SIGFRAME_UCONTEXT + ARM_UCONTEXT_SIGCONTEXT >> + + ARM_SIGCONTEXT_R0; >> + else >> + r0_offset = ARM_NEW_RT_SIGFRAME_UCONTEXT + ARM_UCONTEXT_SIGCONTEXT >> + + ARM_SIGCONTEXT_R0; > > Nit: we can write these long lines like this, > > if (sp_data == sp + ARM_OLD_RT_SIGFRAME_SIGINFO) > r0_offset = ARM_OLD_RT_SIGFRAME_UCONTEXT; > else > r0_offset = ARM_NEW_RT_SIGFRAME_UCONTEXT; > > r0_offset += (ARM_UCONTEXT_SIGCONTEXT + ARM_SIGCONTEXT_R0); > OK. I removed the parens however after r0_offset += .. >> >> - arm_linux_sigreturn_return_addr (frame, svc_number, &return_addr, &is_thumb); >> + if (is_syscall (gdbarch, "sigreturn", svc_number) >> + || is_syscall (gdbarch, "rt_sigreturn", svc_number)) >> + next_pc = arm_linux_sigreturn_next_pc (regcache, svc_number); >> >> /* Addresses for calling Thumb functions have the bit 0 set. */ >> if (is_thumb) >> - return_addr |= 1; >> + next_pc |= 1; > > Nit: use MAKE_THUMB_ADDR. > OK.