From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 111721 invoked by alias); 29 Jun 2015 15:54:08 -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 111710 invoked by uid 89); 29 Jun 2015 15:54:07 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-0.8 required=5.0 tests=AWL,BAYES_00,SPF_PASS autolearn=ham version=3.3.2 X-HELO: eu-smtp-delivery-143.mimecast.com Received: from eu-smtp-delivery-143.mimecast.com (HELO eu-smtp-delivery-143.mimecast.com) (207.82.80.143) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Mon, 29 Jun 2015 15:54:06 +0000 Received: from cam-owa1.Emea.Arm.com (fw-tnat.cambridge.arm.com [217.140.96.140]) by eu-smtp-1.mimecast.com with ESMTP id uk-mta-14-_SWUP5FwQHak0phCQyqmXw-1 Received: from [10.2.207.36] ([10.1.2.79]) by cam-owa1.Emea.Arm.com with Microsoft SMTPSVC(6.0.3790.3959); Mon, 29 Jun 2015 16:54:03 +0100 Message-ID: <55916A1A.6090807@arm.com> Date: Mon, 29 Jun 2015 15:54:00 -0000 From: Pierre Langlois User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.7.0 MIME-Version: 1.0 To: gdb-patches@sourceware.org, Wei-cheng Wang Subject: Re: [PATCH 4/5 v4] Tracepoint for ppc64. References: <1435422102-39438-1-git-send-email-cole945@gmail.com> <1435422102-39438-4-git-send-email-cole945@gmail.com> In-Reply-To: <1435422102-39438-4-git-send-email-cole945@gmail.com> X-MC-Unique: _SWUP5FwQHak0phCQyqmXw-1 Content-Type: text/plain; charset=WINDOWS-1252 Content-Transfer-Encoding: quoted-printable X-IsSubscribed: yes X-SW-Source: 2015-06/txt/msg00606.txt.bz2 On 27/06/15 17:21, Wei-cheng Wang wrote: > Ulrich Weigand wrote: >> Wei-cheng Wang wrote: >>> Ulrich Weigand wrote: >>>> Wei-cheng Wang wrote: >>>>> +static int >>>>> +ppc_fast_tracepoint_valid_at (struct gdbarch *gdbarch, >>>>> + CORE_ADDR addr, int *isize, char **msg) >>>>> +{ >>>>> + if (isize) >>>>> + *isize =3D gdbarch_max_insn_length (gdbarch); >>>>> + if (msg) >>>>> + *msg =3D NULL; >>>>> + return 1; >>>>> +} >>>> >>>> Should/can we check here where the jump to the jump pad will be in >>>> range? Might be better to detect this early ... >>> >>> Client has no idea about where the jump pad will be installed. >>> If it's out of range, gdbserver will report it right after user >>> entered 'tstart' command >> Well, but we know the logic the stub uses. For example, we know that >> we certainly cannot install a fast tracepoint in any shared library code, >> since the jump pad will definitely be too far away. We can check for >> this condition here. (We could also check for tracepoints in executables >> that have a text section larger than 32 MB ...) >=20 > Now in this path, ppc_fast_tracepoint_valid_at will check the distance > and return 0 (invalid) if ADDR is too far from jumppad. >=20 > However, if a tracepoint was pending and later found it's not valid, > it will cause an internal-error. See remote.c >=20 > if (gdbarch_fast_tracepoint_valid_at (target_gdbarch (), > tpaddr, &isize, NULL)) > xsnprintf (buf + strlen (buf), BUF_SIZE - strlen (buf), ":F%x", > isize); > else > /* If it passed validation at definition but fails now, > ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ > something is very wrong. */ > internal_error (__FILE__, __LINE__, _("Fast tracepoint not " > "valid during download")); >=20 > If the tracepoint is pending at definition, it won't be checked at all. >=20 > /* Fast tracepoints may have additional restrictions on location. */ > if (!pending && type_wanted =3D=3D bp_fast_tracepoint) > ^^^^^^^^ > { > for (ix =3D 0; VEC_iterate (linespec_sals, canonical.sals, ix, iter= ); ++ix) > check_fast_tracepoint_sals (gdbarch, &iter->sals); > } >=20 > Maybe we use "error" instead of "internal_error"? Hi Wei-cheng, Is it OK for GDB to guess where the jump pad is based assumptions on how/wh= ere GDBServer installs it? I'm thinking one could implement another stub suppo= rting fast tracepoints differently (?). Maybe we could extend/add a tracepoint packet in order for GDB to ask the s= tub what the jump pad address is. Or change the fast_tracepoint_valid_at gdbar= ch method into a target method, leaving GDBServer to decide if a fast tracepoi= nt is valid early on. Any thoughts? Thanks, Pierre > diff --git a/gdb/rs6000-tdep.c b/gdb/rs6000-tdep.c > index d947879..7924227 100644 > --- a/gdb/rs6000-tdep.c > +++ b/gdb/rs6000-tdep.c > @@ -83,6 +83,9 @@ > #include "features/rs6000/powerpc-e500.c" > #include "features/rs6000/rs6000.c" >=20 > +#include "ax.h" > +#include "ax-gdb.h" > + > /* Determine if regnum is an SPE pseudo-register. */ > #define IS_SPE_PSEUDOREG(tdep, regnum) ((tdep)->ppc_ev0_regnum >=3D 0 \ > && (regnum) >=3D (tdep)->ppc_ev0_regnum \ > @@ -966,6 +969,53 @@ rs6000_breakpoint_from_pc (struct gdbarch *gdbarch, = CORE_ADDR *bp_addr, > return little_breakpoint; > } >=20 > +/* Return true if ADDR is a valid address for tracepoint. Set *ISZIE > + to the number of bytes the target should copy elsewhere for the > + tracepoint. */ > + > +static int > +ppc_fast_tracepoint_valid_at (struct gdbarch *gdbarch, > + CORE_ADDR addr, int *isize, char **msg) > +{ > + CORE_ADDR base, pagesz; > + const int SCRATCH_BUFFER_NPAGES =3D 20; > + int isValid =3D 1; > + > + if (isize) > + *isize =3D gdbarch_max_insn_length (gdbarch); > + > + /* If we can figure out where is the jump-pad, check whether > + the address for tracepoint is too far away. Otherwise, > + assume it is valid. */ > + if (target_auxv_search (¤t_target, AT_PHDR, &base) > 0 > + && target_auxv_search (¤t_target, AT_PAGESZ, &pagesz) > 0) > + { > + /* The jump-pad is supposed to be mapped here. > + See gdbserver/linux-ppc-ipa.c and gdbserver/tracepoint.c. */ > + long dist; > + CORE_ADDR jpad_base > + =3D (base & ~(pagesz - 1)) - SCRATCH_BUFFER_NPAGES * pagesz; > + > + dist =3D jpad_base - addr; > + if (dist >=3D (1 << 25) || dist < -(1 << 25)) > + isValid =3D 0; > + } > + > + if (isValid) > + { > + if (msg) > + *msg =3D NULL; > + } > + else > + { > + if (msg) > + *msg =3D xstrdup (_("The address is too far for " > + "inserting fast tracepoint.")); > + } > + > + return isValid; > +} > + > /* Instruction masks for displaced stepping. */ > #define BRANCH_MASK 0xfc000000 > #define BP_MASK 0xFC0007FE