diff mbox series

[09/48] perf dwarf-regs: Add get_dwarf_regnum()

Message ID 20231012035111.676789-10-namhyung@kernel.org
State New
Headers
Series perf tools: Introduce data type profiling (v1) |

Commit Message

Namhyung Kim Oct. 12, 2023, 3:50 a.m. UTC 
  The get_dwarf_regnum() returns a DWARF register number from a register
name string according to the psABI.  Also add two pseudo encodings of
DWARF_REG_PC which is a register that are used by PC-relative addressing
and DWARF_REG_FB which is a frame base register.  They need to be
handled in a special way.

Cc: Masami Hiramatsu <mhiramat@kernel.org>
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/arch/x86/util/dwarf-regs.c | 38 +++++++++++++++++++++++++++
 tools/perf/util/dwarf-regs.c          | 33 +++++++++++++++++++++++
 tools/perf/util/include/dwarf-regs.h  | 11 ++++++++
 3 files changed, 82 insertions(+)

Comments

Masami Hiramatsu (Google) Nov. 5, 2023, 8:36 a.m. UTC | #1 
On Wed, 11 Oct 2023 20:50:32 -0700
Namhyung Kim <namhyung@kernel.org> wrote:

> The get_dwarf_regnum() returns a DWARF register number from a register
> name string according to the psABI.  Also add two pseudo encodings of
> DWARF_REG_PC which is a register that are used by PC-relative addressing
> and DWARF_REG_FB which is a frame base register.  They need to be
> handled in a special way.
> 

I have some comments below.

> Cc: Masami Hiramatsu <mhiramat@kernel.org>
> Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> ---
>  tools/perf/arch/x86/util/dwarf-regs.c | 38 +++++++++++++++++++++++++++
>  tools/perf/util/dwarf-regs.c          | 33 +++++++++++++++++++++++
>  tools/perf/util/include/dwarf-regs.h  | 11 ++++++++
>  3 files changed, 82 insertions(+)
> 
> diff --git a/tools/perf/arch/x86/util/dwarf-regs.c b/tools/perf/arch/x86/util/dwarf-regs.c
> index 530934805710..79835b897cae 100644
> --- a/tools/perf/arch/x86/util/dwarf-regs.c
> +++ b/tools/perf/arch/x86/util/dwarf-regs.c
> @@ -113,3 +113,41 @@ int regs_query_register_offset(const char *name)
>  			return roff->offset;
>  	return -EINVAL;
>  }
> +
> +struct dwarf_regs_idx {
> +	const char *name;
> +	int idx;
> +};
> +
> +static const struct dwarf_regs_idx x86_regidx_table[] = {
> +	{ "rax", 0 }, { "eax", 0 }, { "ax", 0 }, { "al", 0 },
> +	{ "rdx", 1 }, { "edx", 1 }, { "dx", 1 }, { "dl", 1 },
> +	{ "rcx", 2 }, { "ecx", 2 }, { "cx", 2 }, { "cl", 2 },
> +	{ "rbx", 3 }, { "edx", 3 }, { "bx", 3 }, { "bl", 3 },
> +	{ "rsi", 4 }, { "esi", 4 }, { "si", 4 }, { "sil", 4 },
> +	{ "rdi", 5 }, { "edi", 5 }, { "di", 5 }, { "dil", 5 },
> +	{ "rbp", 6 }, { "ebp", 6 }, { "bp", 6 }, { "bpl", 6 },
> +	{ "rsp", 7 }, { "esp", 7 }, { "sp", 7 }, { "spl", 7 },
> +	{ "r8", 8 }, { "r8d", 8 }, { "r8w", 8 }, { "r8b", 8 },
> +	{ "r9", 9 }, { "r9d", 9 }, { "r9w", 9 }, { "r9b", 9 },
> +	{ "r10", 10 }, { "r10d", 10 }, { "r10w", 10 }, { "r10b", 10 },
> +	{ "r11", 11 }, { "r11d", 11 }, { "r11w", 11 }, { "r11b", 11 },
> +	{ "r12", 12 }, { "r12d", 12 }, { "r12w", 12 }, { "r12b", 12 },
> +	{ "r13", 13 }, { "r13d", 13 }, { "r13w", 13 }, { "r13b", 13 },
> +	{ "r14", 14 }, { "r14d", 14 }, { "r14w", 14 }, { "r14b", 14 },
> +	{ "r15", 15 }, { "r15d", 15 }, { "r15w", 15 }, { "r15b", 15 },
> +	{ "rip", DWARF_REG_PC },
> +};
> +
> +int get_arch_regnum(const char *name)
> +{
> +	unsigned int i;
> +
> +	if (*name != '%')
> +		return -1;

Isn't it better to return an error code? e.g. -EINVAL.

> +
> +	for (i = 0; i < ARRAY_SIZE(x86_regidx_table); i++)
> +		if (!strcmp(x86_regidx_table[i].name, name + 1))
> +			return x86_regidx_table[i].idx;
> +	return -1;

And -ENOENT.

> +}
> diff --git a/tools/perf/util/dwarf-regs.c b/tools/perf/util/dwarf-regs.c
> index 69cfaa5953bf..28d786c7df55 100644
> --- a/tools/perf/util/dwarf-regs.c
> +++ b/tools/perf/util/dwarf-regs.c
> @@ -5,6 +5,8 @@
>   * Written by: Masami Hiramatsu <mhiramat@kernel.org>
>   */
>  
> +#include <stdlib.h>
> +#include <string.h>
>  #include <debug.h>
>  #include <dwarf-regs.h>
>  #include <elf.h>
> @@ -68,3 +70,34 @@ const char *get_dwarf_regstr(unsigned int n, unsigned int machine)
>  	}
>  	return NULL;
>  }
> +
> +__weak int get_arch_regnum(const char *name __maybe_unused)
> +{
> +	return -1;

And -EOPNOTSUPP.

> +}
> +
> +/* Return DWARF register number from architecture register name */
> +int get_dwarf_regnum(const char *name, unsigned int machine)
> +{
> +	char *regname = strdup(name);
> +	int reg = -1;
> +	char *p;
> +
> +	if (regname == NULL)
> +		return -1;

Here, -EINVAL.

> +
> +	/* For convenience, remove trailing characters */
> +	p = strpbrk(regname, " ,)");
> +	if (p)
> +		*p = '\0';
> +
> +	switch (machine) {
> +	case EM_NONE:	/* Generic arch - use host arch */
> +		reg = get_arch_regnum(regname);
> +		break;
> +	default:
> +		pr_err("ELF MACHINE %x is not supported.\n", machine);
> +	}
> +	free(regname);
> +	return reg;
> +}
> diff --git a/tools/perf/util/include/dwarf-regs.h b/tools/perf/util/include/dwarf-regs.h
> index 7d99a084e82d..b515f694f55e 100644
> --- a/tools/perf/util/include/dwarf-regs.h
> +++ b/tools/perf/util/include/dwarf-regs.h
> @@ -2,6 +2,9 @@
>  #ifndef _PERF_DWARF_REGS_H_
>  #define _PERF_DWARF_REGS_H_
>  
> +#define DWARF_REG_PC  0xd3af9c /* random number */
> +#define DWARF_REG_FB  0xd3affb /* random number */
> +

Is this for other patch in the series?

>  #ifdef HAVE_DWARF_SUPPORT
>  const char *get_arch_regstr(unsigned int n);
>  /*
> @@ -10,6 +13,14 @@ const char *get_arch_regstr(unsigned int n);
>   * machine: ELF machine signature (EM_*)
>   */
>  const char *get_dwarf_regstr(unsigned int n, unsigned int machine);
> +
> +int get_arch_regnum(const char *name);
> +/*
> + * get_dwarf_regnum - Returns DWARF regnum from register name
> + * name: architecture register name
> + * machine: ELF machine signature (EM_*)
> + */
> +int get_dwarf_regnum(const char *name, unsigned int machine);
>  #endif
>  
>  #ifdef HAVE_ARCH_REGS_QUERY_REGISTER_OFFSET
> -- 
> 2.42.0.655.g421f12c284-goog
> 

Thank you,
Namhyung Kim Nov. 6, 2023, 4:12 a.m. UTC | #2 
On Sun, Nov 5, 2023 at 1:36 AM Masami Hiramatsu <mhiramat@kernel.org> wrote:
>
> On Wed, 11 Oct 2023 20:50:32 -0700
> Namhyung Kim <namhyung@kernel.org> wrote:
>
> > The get_dwarf_regnum() returns a DWARF register number from a register
> > name string according to the psABI.  Also add two pseudo encodings of
> > DWARF_REG_PC which is a register that are used by PC-relative addressing
> > and DWARF_REG_FB which is a frame base register.  They need to be
> > handled in a special way.
> >
>
> I have some comments below.

Sure! :)

>
> > Cc: Masami Hiramatsu <mhiramat@kernel.org>
> > Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> > ---
> >  tools/perf/arch/x86/util/dwarf-regs.c | 38 +++++++++++++++++++++++++++
> >  tools/perf/util/dwarf-regs.c          | 33 +++++++++++++++++++++++
> >  tools/perf/util/include/dwarf-regs.h  | 11 ++++++++
> >  3 files changed, 82 insertions(+)
> >
> > diff --git a/tools/perf/arch/x86/util/dwarf-regs.c b/tools/perf/arch/x86/util/dwarf-regs.c
> > index 530934805710..79835b897cae 100644
> > --- a/tools/perf/arch/x86/util/dwarf-regs.c
> > +++ b/tools/perf/arch/x86/util/dwarf-regs.c
> > @@ -113,3 +113,41 @@ int regs_query_register_offset(const char *name)
> >                       return roff->offset;
> >       return -EINVAL;
> >  }
> > +
> > +struct dwarf_regs_idx {
> > +     const char *name;
> > +     int idx;
> > +};
> > +
> > +static const struct dwarf_regs_idx x86_regidx_table[] = {
> > +     { "rax", 0 }, { "eax", 0 }, { "ax", 0 }, { "al", 0 },
> > +     { "rdx", 1 }, { "edx", 1 }, { "dx", 1 }, { "dl", 1 },
> > +     { "rcx", 2 }, { "ecx", 2 }, { "cx", 2 }, { "cl", 2 },
> > +     { "rbx", 3 }, { "edx", 3 }, { "bx", 3 }, { "bl", 3 },
> > +     { "rsi", 4 }, { "esi", 4 }, { "si", 4 }, { "sil", 4 },
> > +     { "rdi", 5 }, { "edi", 5 }, { "di", 5 }, { "dil", 5 },
> > +     { "rbp", 6 }, { "ebp", 6 }, { "bp", 6 }, { "bpl", 6 },
> > +     { "rsp", 7 }, { "esp", 7 }, { "sp", 7 }, { "spl", 7 },
> > +     { "r8", 8 }, { "r8d", 8 }, { "r8w", 8 }, { "r8b", 8 },
> > +     { "r9", 9 }, { "r9d", 9 }, { "r9w", 9 }, { "r9b", 9 },
> > +     { "r10", 10 }, { "r10d", 10 }, { "r10w", 10 }, { "r10b", 10 },
> > +     { "r11", 11 }, { "r11d", 11 }, { "r11w", 11 }, { "r11b", 11 },
> > +     { "r12", 12 }, { "r12d", 12 }, { "r12w", 12 }, { "r12b", 12 },
> > +     { "r13", 13 }, { "r13d", 13 }, { "r13w", 13 }, { "r13b", 13 },
> > +     { "r14", 14 }, { "r14d", 14 }, { "r14w", 14 }, { "r14b", 14 },
> > +     { "r15", 15 }, { "r15d", 15 }, { "r15w", 15 }, { "r15b", 15 },
> > +     { "rip", DWARF_REG_PC },
> > +};
> > +
> > +int get_arch_regnum(const char *name)
> > +{
> > +     unsigned int i;
> > +
> > +     if (*name != '%')
> > +             return -1;
>
> Isn't it better to return an error code? e.g. -EINVAL.

Maybe.. any negative error code would work.

>
> > +
> > +     for (i = 0; i < ARRAY_SIZE(x86_regidx_table); i++)
> > +             if (!strcmp(x86_regidx_table[i].name, name + 1))
> > +                     return x86_regidx_table[i].idx;
> > +     return -1;
>
> And -ENOENT.
>
> > +}
> > diff --git a/tools/perf/util/dwarf-regs.c b/tools/perf/util/dwarf-regs.c
> > index 69cfaa5953bf..28d786c7df55 100644
> > --- a/tools/perf/util/dwarf-regs.c
> > +++ b/tools/perf/util/dwarf-regs.c
> > @@ -5,6 +5,8 @@
> >   * Written by: Masami Hiramatsu <mhiramat@kernel.org>
> >   */
> >
> > +#include <stdlib.h>
> > +#include <string.h>
> >  #include <debug.h>
> >  #include <dwarf-regs.h>
> >  #include <elf.h>
> > @@ -68,3 +70,34 @@ const char *get_dwarf_regstr(unsigned int n, unsigned int machine)
> >       }
> >       return NULL;
> >  }
> > +
> > +__weak int get_arch_regnum(const char *name __maybe_unused)
> > +{
> > +     return -1;
>
> And -EOPNOTSUPP.

Could be -ENOTSUP.

>
> > +}
> > +
> > +/* Return DWARF register number from architecture register name */
> > +int get_dwarf_regnum(const char *name, unsigned int machine)
> > +{
> > +     char *regname = strdup(name);
> > +     int reg = -1;
> > +     char *p;
> > +
> > +     if (regname == NULL)
> > +             return -1;
>
> Here, -EINVAL.
>
> > +
> > +     /* For convenience, remove trailing characters */
> > +     p = strpbrk(regname, " ,)");
> > +     if (p)
> > +             *p = '\0';
> > +
> > +     switch (machine) {
> > +     case EM_NONE:   /* Generic arch - use host arch */
> > +             reg = get_arch_regnum(regname);
> > +             break;
> > +     default:
> > +             pr_err("ELF MACHINE %x is not supported.\n", machine);
> > +     }
> > +     free(regname);
> > +     return reg;
> > +}
> > diff --git a/tools/perf/util/include/dwarf-regs.h b/tools/perf/util/include/dwarf-regs.h
> > index 7d99a084e82d..b515f694f55e 100644
> > --- a/tools/perf/util/include/dwarf-regs.h
> > +++ b/tools/perf/util/include/dwarf-regs.h
> > @@ -2,6 +2,9 @@
> >  #ifndef _PERF_DWARF_REGS_H_
> >  #define _PERF_DWARF_REGS_H_
> >
> > +#define DWARF_REG_PC  0xd3af9c /* random number */
> > +#define DWARF_REG_FB  0xd3affb /* random number */
> > +
>
> Is this for other patch in the series?

Yes, they are to support global and local variables later.

Thanks,
Namhyung

>
> >  #ifdef HAVE_DWARF_SUPPORT
> >  const char *get_arch_regstr(unsigned int n);
> >  /*
> > @@ -10,6 +13,14 @@ const char *get_arch_regstr(unsigned int n);
> >   * machine: ELF machine signature (EM_*)
> >   */
> >  const char *get_dwarf_regstr(unsigned int n, unsigned int machine);
> > +
> > +int get_arch_regnum(const char *name);
> > +/*
> > + * get_dwarf_regnum - Returns DWARF regnum from register name
> > + * name: architecture register name
> > + * machine: ELF machine signature (EM_*)
> > + */
> > +int get_dwarf_regnum(const char *name, unsigned int machine);
> >  #endif
> >
> >  #ifdef HAVE_ARCH_REGS_QUERY_REGISTER_OFFSET
> > --
> > 2.42.0.655.g421f12c284-goog
> >
>
> Thank you,
>
> --
> Masami Hiramatsu (Google) <mhiramat@kernel.org>
diff mbox series

Patch

diff --git a/tools/perf/arch/x86/util/dwarf-regs.c b/tools/perf/arch/x86/util/dwarf-regs.c
index 530934805710..79835b897cae 100644
--- a/tools/perf/arch/x86/util/dwarf-regs.c
+++ b/tools/perf/arch/x86/util/dwarf-regs.c
@@ -113,3 +113,41 @@  int regs_query_register_offset(const char *name)
 			return roff->offset;
 	return -EINVAL;
 }
+
+struct dwarf_regs_idx {
+	const char *name;
+	int idx;
+};
+
+static const struct dwarf_regs_idx x86_regidx_table[] = {
+	{ "rax", 0 }, { "eax", 0 }, { "ax", 0 }, { "al", 0 },
+	{ "rdx", 1 }, { "edx", 1 }, { "dx", 1 }, { "dl", 1 },
+	{ "rcx", 2 }, { "ecx", 2 }, { "cx", 2 }, { "cl", 2 },
+	{ "rbx", 3 }, { "edx", 3 }, { "bx", 3 }, { "bl", 3 },
+	{ "rsi", 4 }, { "esi", 4 }, { "si", 4 }, { "sil", 4 },
+	{ "rdi", 5 }, { "edi", 5 }, { "di", 5 }, { "dil", 5 },
+	{ "rbp", 6 }, { "ebp", 6 }, { "bp", 6 }, { "bpl", 6 },
+	{ "rsp", 7 }, { "esp", 7 }, { "sp", 7 }, { "spl", 7 },
+	{ "r8", 8 }, { "r8d", 8 }, { "r8w", 8 }, { "r8b", 8 },
+	{ "r9", 9 }, { "r9d", 9 }, { "r9w", 9 }, { "r9b", 9 },
+	{ "r10", 10 }, { "r10d", 10 }, { "r10w", 10 }, { "r10b", 10 },
+	{ "r11", 11 }, { "r11d", 11 }, { "r11w", 11 }, { "r11b", 11 },
+	{ "r12", 12 }, { "r12d", 12 }, { "r12w", 12 }, { "r12b", 12 },
+	{ "r13", 13 }, { "r13d", 13 }, { "r13w", 13 }, { "r13b", 13 },
+	{ "r14", 14 }, { "r14d", 14 }, { "r14w", 14 }, { "r14b", 14 },
+	{ "r15", 15 }, { "r15d", 15 }, { "r15w", 15 }, { "r15b", 15 },
+	{ "rip", DWARF_REG_PC },
+};
+
+int get_arch_regnum(const char *name)
+{
+	unsigned int i;
+
+	if (*name != '%')
+		return -1;
+
+	for (i = 0; i < ARRAY_SIZE(x86_regidx_table); i++)
+		if (!strcmp(x86_regidx_table[i].name, name + 1))
+			return x86_regidx_table[i].idx;
+	return -1;
+}
diff --git a/tools/perf/util/dwarf-regs.c b/tools/perf/util/dwarf-regs.c
index 69cfaa5953bf..28d786c7df55 100644
--- a/tools/perf/util/dwarf-regs.c
+++ b/tools/perf/util/dwarf-regs.c
@@ -5,6 +5,8 @@ 
  * Written by: Masami Hiramatsu <mhiramat@kernel.org>
  */
 
+#include <stdlib.h>
+#include <string.h>
 #include <debug.h>
 #include <dwarf-regs.h>
 #include <elf.h>
@@ -68,3 +70,34 @@  const char *get_dwarf_regstr(unsigned int n, unsigned int machine)
 	}
 	return NULL;
 }
+
+__weak int get_arch_regnum(const char *name __maybe_unused)
+{
+	return -1;
+}
+
+/* Return DWARF register number from architecture register name */
+int get_dwarf_regnum(const char *name, unsigned int machine)
+{
+	char *regname = strdup(name);
+	int reg = -1;
+	char *p;
+
+	if (regname == NULL)
+		return -1;
+
+	/* For convenience, remove trailing characters */
+	p = strpbrk(regname, " ,)");
+	if (p)
+		*p = '\0';
+
+	switch (machine) {
+	case EM_NONE:	/* Generic arch - use host arch */
+		reg = get_arch_regnum(regname);
+		break;
+	default:
+		pr_err("ELF MACHINE %x is not supported.\n", machine);
+	}
+	free(regname);
+	return reg;
+}
diff --git a/tools/perf/util/include/dwarf-regs.h b/tools/perf/util/include/dwarf-regs.h
index 7d99a084e82d..b515f694f55e 100644
--- a/tools/perf/util/include/dwarf-regs.h
+++ b/tools/perf/util/include/dwarf-regs.h
@@ -2,6 +2,9 @@ 
 #ifndef _PERF_DWARF_REGS_H_
 #define _PERF_DWARF_REGS_H_
 
+#define DWARF_REG_PC  0xd3af9c /* random number */
+#define DWARF_REG_FB  0xd3affb /* random number */
+
 #ifdef HAVE_DWARF_SUPPORT
 const char *get_arch_regstr(unsigned int n);
 /*
@@ -10,6 +13,14 @@  const char *get_arch_regstr(unsigned int n);
  * machine: ELF machine signature (EM_*)
  */
 const char *get_dwarf_regstr(unsigned int n, unsigned int machine);
+
+int get_arch_regnum(const char *name);
+/*
+ * get_dwarf_regnum - Returns DWARF regnum from register name
+ * name: architecture register name
+ * machine: ELF machine signature (EM_*)
+ */
+int get_dwarf_regnum(const char *name, unsigned int machine);
 #endif
 
 #ifdef HAVE_ARCH_REGS_QUERY_REGISTER_OFFSET