From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 5326 invoked by alias); 30 Nov 2016 20:46:17 -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 5312 invoked by uid 89); 30 Nov 2016 20:46:16 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.9 required=5.0 tests=AWL,BAYES_00,RCVD_IN_DNSWL_NONE,SPF_PASS autolearn=ham version=3.3.2 spammy=2638 X-HELO: relay1.mentorg.com Received: from relay1.mentorg.com (HELO relay1.mentorg.com) (192.94.38.131) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 30 Nov 2016 20:46:15 +0000 Received: from svr-orw-mbx-03.mgc.mentorg.com ([147.34.90.203]) by relay1.mentorg.com with esmtp id 1cCBld-0000TA-Fl from Luis_Gustavo@mentor.com ; Wed, 30 Nov 2016 12:46:13 -0800 Received: from [172.30.5.15] (147.34.91.1) by svr-orw-mbx-03.mgc.mentorg.com (147.34.90.203) with Microsoft SMTP Server (TLS) id 15.0.1210.3; Wed, 30 Nov 2016 12:46:10 -0800 Subject: Re: [PATCH 1/3] Fix inferior memory reading in GDBServer for arm/aarch32. References: <20161128122758.7762-1-antoine.tremblay@ericsson.com> To: Antoine Tremblay , Reply-To: Luis Machado From: Luis Machado Message-ID: Date: Wed, 30 Nov 2016 20:46:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.4.0 MIME-Version: 1.0 In-Reply-To: <20161128122758.7762-1-antoine.tremblay@ericsson.com> Content-Type: text/plain; charset="windows-1252"; format=flowed Content-Transfer-Encoding: 7bit X-ClientProxiedBy: svr-orw-mbx-01.mgc.mentorg.com (147.34.90.201) To svr-orw-mbx-03.mgc.mentorg.com (147.34.90.203) X-IsSubscribed: yes X-SW-Source: 2016-11/txt/msg01021.txt.bz2 On 11/28/2016 06:27 AM, Antoine Tremblay wrote: > Before this patch, some functions would read the inferior memory with > (*the_target)->read_memory, which returns the raw memory, rather than the > shadowed memory. > > This is wrong since these functions do not expect to read a breakpoint > instruction and can lead to invalid behavior. > > Use of raw memory in get_next_pcs_read_memory_unsigned_integer for example > could lead to get_next_pc returning an invalid pc. > > Tested on gdbserver-native/-m{thumb,arm} no regressions. > > gdb/gdbserver/ChangeLog: > > * linux-aarch32-low.c (arm_breakpoint_kind_from_pc): Use > target_read_memory. > * linux-arm-low.c (get_next_pcs_read_memory_unsigned_integer): Likewise. > (arm_sigreturn_next_pc): Likewise. > (get_next_pcs_syscall_next_pc): Likewise. > (arm_get_syscall_trapinfo): Likewise. > --- > gdb/gdbserver/linux-aarch32-low.c | 4 ++-- > gdb/gdbserver/linux-arm-low.c | 13 +++++++------ > 2 files changed, 9 insertions(+), 8 deletions(-) > > diff --git a/gdb/gdbserver/linux-aarch32-low.c b/gdb/gdbserver/linux-aarch32-low.c > index 5547cf6..4ff34b6 100644 > --- a/gdb/gdbserver/linux-aarch32-low.c > +++ b/gdb/gdbserver/linux-aarch32-low.c > @@ -237,11 +237,11 @@ arm_breakpoint_kind_from_pc (CORE_ADDR *pcptr) > *pcptr = UNMAKE_THUMB_ADDR (*pcptr); > > /* Check whether we are replacing a thumb2 32-bit instruction. */ > - if ((*the_target->read_memory) (*pcptr, buf, 2) == 0) > + if (target_read_memory (*pcptr, buf, 2) == 0) > { > unsigned short inst1 = 0; > > - (*the_target->read_memory) (*pcptr, (gdb_byte *) &inst1, 2); > + target_read_memory (*pcptr, (gdb_byte *) &inst1, 2); > if (thumb_insn_size (inst1) == 4) > return ARM_BP_KIND_THUMB2; > } > diff --git a/gdb/gdbserver/linux-arm-low.c b/gdb/gdbserver/linux-arm-low.c > index ed9b356..b8365cf 100644 > --- a/gdb/gdbserver/linux-arm-low.c > +++ b/gdb/gdbserver/linux-arm-low.c > @@ -263,7 +263,8 @@ get_next_pcs_read_memory_unsigned_integer (CORE_ADDR memaddr, > ULONGEST res; > > res = 0; > - (*the_target->read_memory) (memaddr, (unsigned char *) &res, len); > + target_read_memory (memaddr, (unsigned char *) &res, len); > + > return res; > } > > @@ -769,15 +770,15 @@ arm_sigreturn_next_pc (struct regcache *regcache, int svc_number, > gdb_assert (svc_number == __NR_sigreturn || svc_number == __NR_rt_sigreturn); > > collect_register_by_name (regcache, "sp", &sp); > - (*the_target->read_memory) (sp, (unsigned char *) &sp_data, 4); > + target_read_memory (sp, (unsigned char *) &sp_data, 4); > > pc_offset = arm_linux_sigreturn_next_pc_offset > (sp, sp_data, svc_number, __NR_sigreturn == svc_number ? 1 : 0); > > - (*the_target->read_memory) (sp + pc_offset, (unsigned char *) &next_pc, 4); > + target_read_memory (sp + pc_offset, (unsigned char *) &next_pc, 4); > > /* Set IS_THUMB according the CPSR saved on the stack. */ > - (*the_target->read_memory) (sp + pc_offset + 4, (unsigned char *) &cpsr, 4); > + target_read_memory (sp + pc_offset + 4, (unsigned char *) &cpsr, 4); > *is_thumb = ((cpsr & CPSR_T) != 0); > > return next_pc; > @@ -804,7 +805,7 @@ get_next_pcs_syscall_next_pc (struct arm_get_next_pcs *self) > unsigned long this_instr; > unsigned long svc_operand; > > - (*the_target->read_memory) (pc, (unsigned char *) &this_instr, 4); > + target_read_memory (pc, (unsigned char *) &this_instr, 4); > svc_operand = (0x00ffffff & this_instr); > > if (svc_operand) /* OABI. */ > @@ -965,7 +966,7 @@ arm_get_syscall_trapinfo (struct regcache *regcache, int *sysno) > > collect_register_by_name (regcache, "pc", &pc); > > - if ((*the_target->read_memory) (pc - 4, (unsigned char *) &insn, 4)) > + if (target_read_memory (pc - 4, (unsigned char *) &insn, 4)) > *sysno = UNKNOWN_SYSCALL; > else > { > The series LGTM. Fairly mechanical changes.