From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 33076 invoked by alias); 23 Sep 2015 10:51:47 -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 33061 invoked by uid 89); 23 Sep 2015 10:51:46 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.2 required=5.0 tests=AWL,BAYES_00,FREEMAIL_FROM,RCVD_IN_DNSWL_LOW,SPF_PASS autolearn=ham version=3.3.2 X-HELO: mail-pa0-f44.google.com Received: from mail-pa0-f44.google.com (HELO mail-pa0-f44.google.com) (209.85.220.44) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-GCM-SHA256 encrypted) ESMTPS; Wed, 23 Sep 2015 10:51:44 +0000 Received: by pacgz1 with SMTP id gz1so3529476pac.3 for ; Wed, 23 Sep 2015 03:51:43 -0700 (PDT) X-Received: by 10.68.205.231 with SMTP id lj7mr15868522pbc.36.1443005503059; Wed, 23 Sep 2015 03:51:43 -0700 (PDT) Received: from E107787-LIN (power-aix.osuosl.org. [140.211.15.154]) by smtp.gmail.com with ESMTPSA id zf5sm7301675pbc.36.2015.09.23.03.51.39 (version=TLS1_2 cipher=AES128-SHA256 bits=128/128); Wed, 23 Sep 2015 03:51:42 -0700 (PDT) From: Yao Qi To: Antoine Tremblay Cc: Subject: Re: [PATCH 1/5] Support multiple breakpoint types per target in GDBServer. References: <1442577749-6650-1-git-send-email-antoine.tremblay@ericsson.com> <1442577749-6650-2-git-send-email-antoine.tremblay@ericsson.com> Date: Wed, 23 Sep 2015 10:51:00 -0000 In-Reply-To: <1442577749-6650-2-git-send-email-antoine.tremblay@ericsson.com> (Antoine Tremblay's message of "Fri, 18 Sep 2015 08:02:25 -0400") Message-ID: <86oagt9znr.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-09/txt/msg00545.txt.bz2 Antoine Tremblay writes: > +static const unsigned char * > +aarch64_breakpoint_from_pc (CORE_ADDR *pcptr, int *len) Please add comment like this to all these $ARCH_breakpoint_from_pc function= s. /* Implementation of linux_target_ops method "breakpoint_from_pc". */ > diff --git a/gdb/gdbserver/linux-cris-low.c b/gdb/gdbserver/linux-cris-lo= w.c > index e0bfa1a..da5876d 100644 > --- a/gdb/gdbserver/linux-cris-low.c > +++ b/gdb/gdbserver/linux-cris-low.c > @@ -62,7 +62,7 @@ cris_cannot_fetch_register (int regno) > extern int debug_threads; >=20=20 > static CORE_ADDR > -cris_get_pc (struct regcache *regcache, void) > +cris_get_pc (struct regcache *regcache) > { > unsigned long pc; > collect_register_by_name (regcache, "pc", &pc); This is a unrelated change. Please move it out this patch, and submit it separately. > diff --git a/gdb/gdbserver/linux-low.c b/gdb/gdbserver/linux-low.c > index f5b64ab..bb08761 100644 > --- a/gdb/gdbserver/linux-low.c > +++ b/gdb/gdbserver/linux-low.c > @@ -3012,7 +3012,11 @@ linux_wait_1 (ptid_t ptid, > if (!ptid_equal (step_over_bkpt, null_ptid) > && event_child->stop_reason =3D=3D TARGET_STOPPED_BY_SW_BREAKPOINT) > { > - unsigned int increment_pc =3D the_low_target.breakpoint_len; > + int increment_pc =3D 0; > + CORE_ADDR stop_pc =3D event_child->stop_pc; > + > + (*the_low_target.breakpoint_from_pc) > + (&stop_pc, &increment_pc); >=20=20 There was a problem here and your patch doesn't fix it. I want to raise it here first. It is incorrect to get increment_pc by the_low_target.breakpoint_len or (*the_low_target.breakpoint_from_pc) for arm/thumb, because given the stop_pc, we can't tell the breakpoint size (2-byte or 4-byte). We need a new target hook, say breakpoint_from_current_state, and its default implementation is breakpoint_from_pc. However, its arm implementation checks whether the program is in thumb mode by CPSR and return the right breakpoint size. IIUC, the code here is used for step-over GDBserver breakpoint, so it is not used for arm target until we support conditional breakpoint or tracepoint, but we should fix it before supporting conditional breakpoint and tracepoint for arm target. > if (debug_threads) > { > @@ -6932,6 +6936,17 @@ current_lwp_ptid (void) > return ptid_of (current_thread); > } >=20=20 > +const unsigned char * > +linux_breakpoint_from_pc (CORE_ADDR *pcptr, int *lenptr) > +{ > + if (the_low_target.breakpoint_from_pc !=3D NULL) > + { > + return (*the_low_target.breakpoint_from_pc) (pcptr, lenptr); > + } "{" and "}" is not needed. > diff --git a/gdb/gdbserver/linux-low.h b/gdb/gdbserver/linux-low.h > index f8f6e78..c623150 100644 > --- a/gdb/gdbserver/linux-low.h > +++ b/gdb/gdbserver/linux-low.h > @@ -141,8 +141,13 @@ struct linux_target_ops >=20=20 > CORE_ADDR (*get_pc) (struct regcache *regcache); > void (*set_pc) (struct regcache *regcache, CORE_ADDR newpc); > - const unsigned char *breakpoint; > - int breakpoint_len; > + > + /* Return the raw breakpoint for this target based on PC. Note that t= he PC s/PC/*PCPTR/ > + can be NULL, the default breakpoint for the target should be return= ed in PC can't be NULL after your patch #2. You can remove the second sentence in this patch or patch #2. > + this case. The PC is ajusted to the real memory location in case a = flag was > + present in the PC. */ > + const unsigned char *(*breakpoint_from_pc) (CORE_ADDR *pcptr, int *len= ptr); > + --=20 Yao (=E9=BD=90=E5=B0=A7)