Message ID | 20161128122758.7762-1-antoine.tremblay@ericsson.com |
---|---|
State | New, archived |
Headers |
Received: (qmail 20553 invoked by alias); 28 Nov 2016 12:28:24 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: <gdb-patches.sourceware.org> List-Unsubscribe: <mailto:gdb-patches-unsubscribe-##L=##H@sourceware.org> List-Subscribe: <mailto:gdb-patches-subscribe@sourceware.org> List-Archive: <http://sourceware.org/ml/gdb-patches/> List-Post: <mailto:gdb-patches@sourceware.org> List-Help: <mailto:gdb-patches-help@sourceware.org>, <http://sourceware.org/ml/#faqs> Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 20540 invoked by uid 89); 28 Nov 2016 12:28:23 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.9 required=5.0 tests=BAYES_00, SPF_PASS autolearn=ham version=3.3.2 spammy=thumb2, HX-Envelope-From:sk:antoine, 2637, H*r:Security 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 ESMTP; Mon, 28 Nov 2016 12:28:13 +0000 Received: from EUSAAHC007.ericsson.se (Unknown_Domain [147.117.188.93]) by (Symantec Mail Security) with SMTP id 88.8C.29529.2A72C385; Mon, 28 Nov 2016 13:48:35 +0100 (CET) Received: from elxa4wqvvz1.ca.am.ericsson.se (147.117.188.8) by smtps-am.internal.ericsson.com (147.117.188.93) with Microsoft SMTP Server (TLS) id 14.3.319.2; Mon, 28 Nov 2016 07:28:11 -0500 From: Antoine Tremblay <antoine.tremblay@ericsson.com> To: <gdb-patches@sourceware.org> CC: Antoine Tremblay <antoine.tremblay@ericsson.com> Subject: [PATCH 1/3] Fix inferior memory reading in GDBServer for arm/aarch32. Date: Mon, 28 Nov 2016 07:27:56 -0500 Message-ID: <20161128122758.7762-1-antoine.tremblay@ericsson.com> MIME-Version: 1.0 Content-Type: text/plain X-IsSubscribed: yes |
Commit Message
Antoine Tremblay
Nov. 28, 2016, 12:27 p.m. UTC
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(-)
Comments
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.
On Mon, Nov 28, 2016 at 07:27:56AM -0500, 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. Can you elaborate under what circumstance breakpoints are still in memory when these functions are called? Can we have a test case? > @@ -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); We are reading from stack, so we don't need to check weather there is a breakpoint or not.
Yao Qi writes: > On Mon, Nov 28, 2016 at 07:27:56AM -0500, 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. > > Can you elaborate under what circumstance breakpoints are still in memory > when these functions are called? Can we have a test case? > Here is an example: In non-stop mode multiple threads are stepping, like in the non-stop-fair-events.exp test. GDB: thread 1 step& GDBServer: thread 1 is at instruction A installs single step breakpoint on instruction B GDB: thread 2 step& GDBServer: thread 2 is at instruction B GDBServer needs to install a single step breakpoint at the next instruction from B. To do so get_next_pc is called, but since the single step breakpoint for thread 1 at instruction B is there. get_next_pc reads the current instruction as a breakpoint instruction and fails. Note that I used a user driven example here to make it more clear but this is also true while range-stepping in a loop for example: - thread 1 hits its single-step breakpoint deletes it - it's not out of a range-step so - tries to install a single-step breakpoint at the next instruction - but thread 2 has a breakpoint at thread 1's current instruction and get_next_pc fails. This is already tested by non-stop-fair-events.exp, the test will fail without this patch. Note that this test is testing both range-stepping and the user stepping. >> @@ -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); > > We are reading from stack, so we don't need to check weather there is > a breakpoint or not. Ho right, is it worth it to make the distinction however ? I mean, would it be better general practice to use target_read_memory unless we absolutely need to use the_target->read_memory like with breakpoint_at funcs.. ? The counterpart looks more error prone for the developer...
Antoine Tremblay writes: > Yao Qi writes: > >> On Mon, Nov 28, 2016 at 07:27:56AM -0500, 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. >> >> Can you elaborate under what circumstance breakpoints are still in memory >> when these functions are called? Can we have a test case? >> > > Here is an example: > > In non-stop mode multiple threads are stepping, like in the > non-stop-fair-events.exp test. > > GDB: > thread 1 > step& > > GDBServer: > thread 1 is at instruction A > installs single step breakpoint on instruction B > > GDB: > thread 2 > step& > > GDBServer: > > thread 2 is at instruction B > > GDBServer needs to install a single step breakpoint at the next > instruction from B. > > To do so get_next_pc is called, but since the single step > breakpoint for thread 1 at instruction B is there. get_next_pc > reads the current instruction as a breakpoint instruction and fails. > > Note that I used a user driven example here to make it more clear but > this is also true while range-stepping in a loop for example: > > - thread 1 hits its single-step breakpoint deletes it > - it's not out of a range-step so > - tries to install a single-step breakpoint at the next > instruction > - but thread 2 has a breakpoint at thread 1's current > instruction and get_next_pc fails. > > This is already tested by non-stop-fair-events.exp, the test will fail > without this patch. > > Note that this test is testing both range-stepping and the user > stepping. > Sorry I got confused with the code patched with the latest 2 patches I sent refactoring the single stepping code. Considering the current code this is handled by the step-over process, and should not be an issue as it will always step-over before installing any single-step breakpoints. And step-over removes all breakpoints when stepping over thus get_next_pc is ok. This becomes an issue like I said before with https://sourceware.org/ml/gdb-patches/2016-11/msg00939.html Since with this it's possible to install single-step breakpoints without a step-over check. We could consider this patch a preparation for https://sourceware.org/ml/gdb-patches/2016-11/msg00939.html or just a good pratice to use target_read_memory. Thanks, Antoine
Antoine Tremblay writes: > Antoine Tremblay writes: > >> Yao Qi writes: >> >>> On Mon, Nov 28, 2016 at 07:27:56AM -0500, 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. >>> >>> Can you elaborate under what circumstance breakpoints are still in memory >>> when these functions are called? Can we have a test case? >>> >> >> Here is an example: >> >> In non-stop mode multiple threads are stepping, like in the >> non-stop-fair-events.exp test. >> >> GDB: >> thread 1 >> step& >> >> GDBServer: >> thread 1 is at instruction A >> installs single step breakpoint on instruction B >> >> GDB: >> thread 2 >> step& >> >> GDBServer: >> >> thread 2 is at instruction B >> >> GDBServer needs to install a single step breakpoint at the next >> instruction from B. >> >> To do so get_next_pc is called, but since the single step >> breakpoint for thread 1 at instruction B is there. get_next_pc >> reads the current instruction as a breakpoint instruction and fails. >> >> Note that I used a user driven example here to make it more clear but >> this is also true while range-stepping in a loop for example: >> >> - thread 1 hits its single-step breakpoint deletes it >> - it's not out of a range-step so >> - tries to install a single-step breakpoint at the next >> instruction >> - but thread 2 has a breakpoint at thread 1's current >> instruction and get_next_pc fails. >> >> This is already tested by non-stop-fair-events.exp, the test will fail >> without this patch. >> >> Note that this test is testing both range-stepping and the user >> stepping. >> > > Sorry I got confused with the code patched with the latest 2 patches I > sent refactoring the single stepping code. > > Considering the current code this is handled by the step-over process, > and should not be an issue as it will always step-over before installing > any single-step breakpoints. > > And step-over removes all breakpoints when stepping over thus > get_next_pc is ok. > > This becomes an issue like I said before with > https://sourceware.org/ml/gdb-patches/2016-11/msg00939.html > > Since with this it's possible to install single-step breakpoints without > a step-over check. > > We could consider this patch a preparation for > https://sourceware.org/ml/gdb-patches/2016-11/msg00939.html > > or just a good pratice to use target_read_memory. > > Thanks, > Antoine Just to supplement about: https://sourceware.org/ml/gdb-patches/2016-11/msg00939.html If we consider this patch the is 2 reasons we can't install step over breakpoints. One is to be able to delay a step-over. The other is since GDBServer inserts single-step breakpoints when it processes the resume requests and threads are about to be resumed. If threads still have pending status, single-step breakpoints are not installed, so it needs to install them in proceed_all_lwp. And in this case the single-step breakpoints are inserted outside of a step-over process.
Antoine Tremblay writes: > Antoine Tremblay writes: > >> Antoine Tremblay writes: >> >>> Yao Qi writes: >>> >>>> On Mon, Nov 28, 2016 at 07:27:56AM -0500, 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. >>>> >>>> Can you elaborate under what circumstance breakpoints are still in memory >>>> when these functions are called? Can we have a test case? >>>> >>> >>> Here is an example: >>> >>> In non-stop mode multiple threads are stepping, like in the >>> non-stop-fair-events.exp test. >>> >>> GDB: >>> thread 1 >>> step& >>> >>> GDBServer: >>> thread 1 is at instruction A >>> installs single step breakpoint on instruction B >>> >>> GDB: >>> thread 2 >>> step& >>> >>> GDBServer: >>> >>> thread 2 is at instruction B >>> >>> GDBServer needs to install a single step breakpoint at the next >>> instruction from B. >>> >>> To do so get_next_pc is called, but since the single step >>> breakpoint for thread 1 at instruction B is there. get_next_pc >>> reads the current instruction as a breakpoint instruction and fails. >>> >>> Note that I used a user driven example here to make it more clear but >>> this is also true while range-stepping in a loop for example: >>> >>> - thread 1 hits its single-step breakpoint deletes it >>> - it's not out of a range-step so >>> - tries to install a single-step breakpoint at the next >>> instruction >>> - but thread 2 has a breakpoint at thread 1's current >>> instruction and get_next_pc fails. >>> >>> This is already tested by non-stop-fair-events.exp, the test will fail >>> without this patch. >>> >>> Note that this test is testing both range-stepping and the user >>> stepping. >>> >> >> Sorry I got confused with the code patched with the latest 2 patches I >> sent refactoring the single stepping code. >> >> Considering the current code this is handled by the step-over process, >> and should not be an issue as it will always step-over before installing >> any single-step breakpoints. >> >> And step-over removes all breakpoints when stepping over thus >> get_next_pc is ok. >> >> This becomes an issue like I said before with >> https://sourceware.org/ml/gdb-patches/2016-11/msg00939.html >> >> Since with this it's possible to install single-step breakpoints without >> a step-over check. >> >> We could consider this patch a preparation for >> https://sourceware.org/ml/gdb-patches/2016-11/msg00939.html >> >> or just a good pratice to use target_read_memory. >> >> Thanks, >> Antoine > > Just to supplement about: > https://sourceware.org/ml/gdb-patches/2016-11/msg00939.html > > If we consider this patch the is 2 reasons we can't install step over > breakpoints. > > One is to be able to delay a step-over. > > The other is since GDBServer inserts single-step breakpoints when it > processes the resume requests and threads are about to be resumed. If > threads still have pending status, single-step breakpoints are not > installed, so it needs to install them in proceed_all_lwp. And in this > case the single-step breakpoints are inserted outside of a step-over > process. After some more thought, it can happen even with current code too that single step breakpoints are installed without a step-over. Consider this situation: In non-stop: the user issues: thread 1 step& thread 2 step& thread 3 step& In a similar way as non-stop-fair-events.exp (threads are looping). GDBServer: linux_resume is called GDBServer has pending events, threads are not resumed and single-step breakpoint for thread 1 not installed. linux_wait_1 is called with a pending event on thread 2 at pc A GDBServer handles the event and calls proceed_all_lwps This calls proceed_one_lwp and installs single-step breakpoints on all the threads that need one. Now since thread 1 needs to install a single-step breakpoint and is at pc B (different than thread 2), a step-over is not initiated and get_next_pc is called to figure out the next instruction from pc B. However it may just be that thread 3 as a single step breakpoint at pc B. And thus get_next_pc fails. This situation is tested with non-stop-fair-events.exp. Sorry for the confusion, you can consider only the two last replies as valid.
On 16-12-01 10:28:14, Antoine Tremblay wrote: > >> @@ -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); > > > > We are reading from stack, so we don't need to check weather there is > > a breakpoint or not. > > Ho right, is it worth it to make the distinction however ? > > I mean, would it be better general practice to use target_read_memory > unless we absolutely need to use the_target->read_memory like with > breakpoint_at funcs.. ? The counterpart looks more error prone for the > developer... This distinction between target_read_memory and the_target->read_memory is clear to me. If we know we are accessing some places where breakpoints are impossible installed, like stack, use the_target->read_memory. Otherwise, use target_read_memory. Change in arm_get_syscall_trapinfo is not necessary to me. I can't figure out a case that program calls syscall instruction, and the previous instruction is a breakpoint.
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 {