From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 6059 invoked by alias); 25 Nov 2013 09:52:02 -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 6049 invoked by uid 89); 25 Nov 2013 09:52:02 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-0.5 required=5.0 tests=AWL,BAYES_50,RDNS_NONE,SPF_PASS autolearn=no version=3.3.2 X-HELO: service87.mimecast.com Received: from Unknown (HELO service87.mimecast.com) (91.220.42.44) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Mon, 25 Nov 2013 09:52:01 +0000 Received: from cam-owa1.Emea.Arm.com (fw-tnat.cambridge.arm.com [217.140.96.21]) by service87.mimecast.com; Mon, 25 Nov 2013 09:51:52 +0000 Received: from [10.1.207.140] ([10.1.255.212]) by cam-owa1.Emea.Arm.com with Microsoft SMTPSVC(6.0.3790.3959); Mon, 25 Nov 2013 09:51:52 +0000 Message-ID: <52931DB7.6030205@arm.com> Date: Mon, 25 Nov 2013 12:18:00 -0000 From: Marcus Shawcroft User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:24.0) Gecko/20100101 Thunderbird/24.1.1 MIME-Version: 1.0 To: Sergio Durigan Junior , GDB Patches Subject: Re: [PATCH] Fix for PR tdep/15653: Implement SystemTap SDT probe support for AArch64 References: <1385336092-19621-1-git-send-email-sergiodj@redhat.com> In-Reply-To: <1385336092-19621-1-git-send-email-sergiodj@redhat.com> X-MC-Unique: 113112509515204301 Content-Type: text/plain; charset=WINDOWS-1252; format=flowed Content-Transfer-Encoding: quoted-printable X-SW-Source: 2013-11/txt/msg00761.txt.bz2 Hi, On 24/11/13 23:34, Sergio Durigan Junior wrote: > This commit implements the needed bits for SystemTap SDT probe support > on AArch64 architectures. The core of the patch is in the > aarch64-linux-tdep.c file; all the rest are support/cleanup > modifications. > > First, I started by looking at AArch64 assembly specification and > filling the necessary options on gdbarch's stap machinery in order to > make the generic asm parser (implemented in stap-probe.c) recognize > AArch64's asm. > > AArch64 has the same idiosincrasy as 32-bit ARM: the syntax for > register indirection is very specific, and demands its own function to > parse this special expression. However, both AArch64 and 32-bit ARM > share the same syntax (with only one little difference between them), A64 assembler syntax is similar to A32 and T32 in some areas and differs=20 somewhat in others. The syntax used specifically for indirect register=20 offset addressing is very similar. It might make sense in some=20 circumstances to share code that encapsulates u-architectural details=20 that underpin an a64/a32/t32 implementation, but, building a shared=20 assembler syntax parser in the fashion proposed could be described as=20 coincidental cohesion. Is that desirable going forward? > which made me think of a way to share the specific parser between both > +/* Implementation of gdbarch_stap_is_single_operand. */ > + > +static int > +aarch64_stap_is_single_operand (struct gdbarch *gdbarch, const char *s) > +{ > + return (*s =3D=3D '#' /* Literal number. */ The '#' before a literal is optional, therefore this clause ought to=20 permit isdigit()? > + || *s =3D=3D '[' /* Register indirection or displacement. */ > + || isalpha (*s)); /* Register value. */ > +} > + > +/* Implementation of gdbarch_stap_parse_special_token. */ > + > +static int > +aarch64_stap_parse_special_token (struct gdbarch *gdbarch, > + struct stap_parse_info *p) > +{ > + return arm_generic_stap_parse_special_token (gdbarch, p, 0); > +} > Looking over in arm-linux-tdep.c:arm_generic_stap_parse_special_token(): """ /* If we are dealing with a register whose name begins with a digit, it means we should prefix the name with the letter `r', because GDB expects this name pattern. Otherwise (e.g., we are dealing with the register `fp'), we don't need to add such a prefix. */ """ Does this really apply to a64 where there are no register names that=20 begin with 'r'? > +int > +arm_generic_stap_parse_special_token (struct gdbarch *gdbarch, > + struct stap_parse_info *p, > + int skip_hash_sign) > { > if (*p->arg =3D=3D '[') > { > @@ -1183,7 +1177,7 @@ arm_stap_parse_special_token (struct gdbarch *gdbar= ch, > > ++tmp; > tmp =3D skip_spaces_const (tmp); > - if (*tmp++ !=3D '#') > + if (skip_hash_sign && *tmp++ !=3D '#') The '#' at this point is optional in a64. > @@ -59,6 +61,24 @@ void arm_linux_collect_nwfpe (const struct regset *reg= set, > const struct regcache *regcache, > int regnum, void *regs_buf, size_t len); > > +/* This routine is used to parse a special token in ARM's assembly. > + It works for both 32-bit and 64-bit ARM architectures. > + > + The special tokens parsed by it are: > + > + - Register displacement (e.g, [fp, #-8] on ARM, and [fp, -8] on AAr= ch64) > + A # is optional in a64, therefore [fp, #-8] is legal. > + SKIP_HASH_SIGN is 1 is the parser should skip the hash sign > + before integers (e.g., #-8 for ARM), or 0 if the parser should > + not bother with it (e.g., -8 for AArch64). Cheers /Marcus