This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] Fix for PR tdep/15653: Implement SystemTap SDT probe support for AArch64


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 somewhat in others. The syntax used specifically for indirect register offset addressing is very similar. It might make sense in some circumstances to share code that encapsulates u-architectural details that underpin an a64/a32/t32 implementation, but, building a shared assembler syntax parser in the fashion proposed could be described as 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 == '#' /* Literal number.  */

The '#' before a literal is optional, therefore this clause ought to permit isdigit()?

+	  || *s == '[' /* 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 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 == '[')
      {
@@ -1183,7 +1177,7 @@ arm_stap_parse_special_token (struct gdbarch *gdbarch,

        ++tmp;
        tmp = skip_spaces_const (tmp);
-      if (*tmp++ != '#')
+      if (skip_hash_sign && *tmp++ != '#')

The '#' at this point is optional in a64.

@@ -59,6 +61,24 @@ void arm_linux_collect_nwfpe (const struct regset *regset,
  			      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 AArch64)
+

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


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]