Hi,
In the perf todo list (https://perf.wiki.kernel.org/index.php/Todo),
there is an entry related with SDT markers support; SDT tracepoints
were already supported by perf but, so far, the probes arguments are
skipped. Here are 2 small patches which adds support of SDT probes
arguments:
$ perf record -e sdt_libfoo:table_frob -e sdt_libfoo:table_diddle test_sdt
$ perf script
test_sdt ... 666.255678: sdt_libfoo:table_frob: (4004d7) arg0=0 arg1=0
test_sdt ... 666.255683: sdt_libfoo:table_diddle: (40051a) arg0=0 arg1=0
test_sdt ... 666.255686: sdt_libfoo:table_frob: (4004d7) arg0=1 arg1=2
test_sdt ... 666.255689: sdt_libfoo:table_diddle: (40051a) arg0=3 arg1=4
test_sdt ... 666.255692: sdt_libfoo:table_frob: (4004d7) arg0=2 arg1=4
test_sdt ... 666.255694: sdt_libfoo:table_diddle: (40051a) arg0=6 arg1=8
The patches were generated against tip/perf/core.
Alexis.
Alexis Berlemont (2):
perf sdt: add scanning of sdt probles arguments
perf probe: add sdt probes arguments into the uprobe cmd string
tools/perf/util/probe-file.c | 176 ++++++++++++++++++++++++++++++++++++++++++-
tools/perf/util/symbol-elf.c | 16 +++-
tools/perf/util/symbol.h | 1 +
3 files changed, 188 insertions(+), 5 deletions(-)
--
2.10.2
An sdt probe can be associated with arguments but they were not passed
to the user probe tracing interface (uprobe_events); this patch adapts
the sdt argument descriptors according to the uprobe input format.
As the uprobe parser does not support scaled address mode, perf will
skip arguments which cannot be adapted to the uprobe format.
Here are the results:
$ perf buildid-cache -v --add test_sdt
$ perf probe -x test_sdt sdt_libfoo:table_frob
$ perf probe -x test_sdt sdt_libfoo:table_diddle
$ perf record -e sdt_libfoo:table_frob -e sdt_libfoo:table_diddle test_sdt
$ perf script
test_sdt ... 666.255678: sdt_libfoo:table_frob: (4004d7) arg0=0 arg1=0
test_sdt ... 666.255683: sdt_libfoo:table_diddle: (40051a) arg0=0 arg1=0
test_sdt ... 666.255686: sdt_libfoo:table_frob: (4004d7) arg0=1 arg1=2
test_sdt ... 666.255689: sdt_libfoo:table_diddle: (40051a) arg0=3 arg1=4
test_sdt ... 666.255692: sdt_libfoo:table_frob: (4004d7) arg0=2 arg1=4
test_sdt ... 666.255694: sdt_libfoo:table_diddle: (40051a) arg0=6 arg1=8
Signed-off-by: Alexis Berlemont <[email protected]>
---
tools/perf/util/probe-file.c | 176 ++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 172 insertions(+), 4 deletions(-)
diff --git a/tools/perf/util/probe-file.c b/tools/perf/util/probe-file.c
index 436b647..a97a170 100644
--- a/tools/perf/util/probe-file.c
+++ b/tools/perf/util/probe-file.c
@@ -28,6 +28,46 @@
#include "probe-file.h"
#include "session.h"
+#ifdef HAVE_GELF_GETNOTE_SUPPORT
+
+/*
+ * Local declarations needed for adjusting gcc/gas-generated registers
+ * before filling the uprobe tracer interface.
+ */
+
+struct sdt_reg_renaming {
+ const char *sdt_name;
+ const char *uprobe_name;
+};
+
+#define REG_RENAMING(n, m) {.sdt_name = "%" #n, .uprobe_name = "%" #m}
+#define REG_RENAMING_END {.sdt_name = NULL, .uprobe_name = NULL}
+
+static const struct sdt_reg_renaming sdt_reg_renaming_table[] = {
+ REG_RENAMING(eax, ax),
+ REG_RENAMING(rax, ax),
+ REG_RENAMING(ebx, bx),
+ REG_RENAMING(rbx, bx),
+ REG_RENAMING(ecx, cx),
+ REG_RENAMING(rcx, cx),
+ REG_RENAMING(edx, dx),
+ REG_RENAMING(rdx, dx),
+ REG_RENAMING(esi, si),
+ REG_RENAMING(rsi, si),
+ REG_RENAMING(edi, di),
+ REG_RENAMING(rdi, di),
+ REG_RENAMING(ebp, bp),
+ REG_RENAMING(rbp, bp),
+ REG_RENAMING_END,
+};
+
+static const char * const type_to_suffix[] = {
+ ":s64", "", "", "", ":s32", "", ":s16", ":s8",
+ "", ":u8", ":u16", "", ":u32", "", "", "", ":u64"
+};
+
+#endif /* HAVE_GELF_GETNOTE_SUPPORT */
+
#define MAX_CMDLEN 256
static void print_open_warning(int err, bool uprobe)
@@ -687,6 +727,133 @@ static unsigned long long sdt_note__get_addr(struct sdt_note *note)
: (unsigned long long)note->addr.a64[0];
}
+static int synthesize_sdt_probe_arg(struct strbuf *buf, int i, const char *arg)
+{
+ const struct sdt_reg_renaming *rnames;
+ char *tmp, *desc = strdup(arg);
+ const char *suffix = "";
+ int ret = -1;
+
+ if (desc == NULL) {
+ pr_debug4("Allocation error\n");
+ return ret;
+ }
+
+ /*
+ * The uprobe tracer format does not support all the
+ * addressing modes (notably: in x86 the scaled mode); so, we
+ * detect ',' characters, if there is just one, there is no
+ * use converting the sdt arg into a uprobe one.
+ */
+ if (strchr(desc, ',')) {
+ pr_debug4("SDT argument format not supported\n");
+ goto out;
+ }
+
+ tmp = strchr(desc, '@');
+ if (tmp) {
+ long type_idx;
+ /*
+ * Isolate the string number and convert it into a
+ * binary value; this will be an index to get suffix
+ * of the uprobe name (defining the type)
+ */
+ tmp[0] = '\0';
+ type_idx = strtol(desc, NULL, 10);
+ if (type_idx == LONG_MIN ||
+ type_idx == LONG_MAX) {
+ pr_debug4("Failed to get sdt type\n");
+ goto error;
+ }
+ suffix = type_to_suffix[type_idx + 8];
+ /* Get rid of the sdt prefix which is now useless */
+ tmp++;
+ memmove(desc, tmp, strlen(tmp) + 1);
+ }
+
+ /*
+ * The uprobe parser does not support all gas register names;
+ * so, we have to replace them (ex. for x86_64: %rax -> %ax);
+ * the loop below performs all the needed renamings if needed.
+ */
+
+ for (rnames = sdt_reg_renaming_table;
+ rnames->sdt_name != NULL; rnames++) {
+ char *new_desc, *sdt_name;
+ size_t prefix_len, uprobe_len, mid_ofs, desc_len;
+
+ sdt_name = strstr(desc, rnames->sdt_name);
+ if (sdt_name == NULL)
+ continue;
+
+ new_desc = zalloc(strlen(desc) + 1 +
+ strlen(rnames->uprobe_name) -
+ strlen(rnames->sdt_name));
+ if (new_desc == NULL)
+ goto error;
+
+ prefix_len = sdt_name - desc;
+ if (prefix_len != 0)
+ memcpy(new_desc, desc, prefix_len);
+
+ uprobe_len = strlen(rnames->uprobe_name);
+ memcpy(new_desc + prefix_len, rnames->uprobe_name, uprobe_len);
+
+ mid_ofs = prefix_len + strlen(rnames->sdt_name);
+ desc_len = strlen(desc);
+ if (mid_ofs < desc_len)
+ memcpy(new_desc + prefix_len + uprobe_len,
+ desc + mid_ofs, desc_len - mid_ofs);
+
+ free(desc);
+ desc = new_desc;
+ }
+
+ if (strbuf_addf(buf, " arg%d=%s%s", i, desc, suffix) < 0)
+ goto error;
+
+out:
+ ret = 0;
+error:
+ free(desc);
+ return ret;
+}
+
+static char *synthesize_sdt_probe_command(struct sdt_note *note,
+ const char *pathname,
+ const char *sdtgrp)
+{
+ struct strbuf buf;
+ char *ret = NULL, **args;
+ int i, args_count;
+
+ if (strbuf_init(&buf, 32) < 0)
+ return NULL;
+
+ if (strbuf_addf(&buf, "p:%s/%s %s:0x%llx",
+ sdtgrp, note->name, pathname,
+ sdt_note__get_addr(note)) < 0)
+ goto error;
+
+ if (!note->args)
+ goto out;
+
+ if (note->args) {
+ args = argv_split(note->args, &args_count);
+
+ for (i = 0; i < args_count; ++i) {
+ if (synthesize_sdt_probe_arg(&buf, i, args[i]) < 0)
+ goto error;
+ }
+ }
+
+out:
+ ret = strbuf_detach(&buf, NULL);
+error:
+ strbuf_release(&buf);
+ return ret;
+}
+
int probe_cache__scan_sdt(struct probe_cache *pcache, const char *pathname)
{
struct probe_cache_entry *entry = NULL;
@@ -723,11 +890,12 @@ int probe_cache__scan_sdt(struct probe_cache *pcache, const char *pathname)
entry->pev.group = strdup(sdtgrp);
list_add_tail(&entry->node, &pcache->entries);
}
- ret = asprintf(&buf, "p:%s/%s %s:0x%llx",
- sdtgrp, note->name, pathname,
- sdt_note__get_addr(note));
- if (ret < 0)
+ buf = synthesize_sdt_probe_command(note, pathname, sdtgrp);
+ if (!buf) {
+ ret = -ENOMEM;
break;
+ }
+
strlist__add(entry->tevlist, buf);
free(buf);
entry = NULL;
--
2.10.2
During a "perf buildid-cache --add" command, the section
".note.stapsdt" of the "added" binary is scanned in order to list the
available SDT markers available in a binary. The parts containing the
probes arguments were left unscanned.
The whole section is now parsed; the probe arguments are extracted for
later use.
Signed-off-by: Alexis Berlemont <[email protected]>
---
tools/perf/util/symbol-elf.c | 16 +++++++++++++++-
tools/perf/util/symbol.h | 1 +
2 files changed, 16 insertions(+), 1 deletion(-)
diff --git a/tools/perf/util/symbol-elf.c b/tools/perf/util/symbol-elf.c
index 99400b0..0fbe0b2 100644
--- a/tools/perf/util/symbol-elf.c
+++ b/tools/perf/util/symbol-elf.c
@@ -1822,7 +1822,7 @@ void kcore_extract__delete(struct kcore_extract *kce)
static int populate_sdt_note(Elf **elf, const char *data, size_t len,
struct list_head *sdt_notes)
{
- const char *provider, *name;
+ const char *provider, *name, *args;
struct sdt_note *tmp = NULL;
GElf_Ehdr ehdr;
GElf_Addr base_off = 0;
@@ -1881,6 +1881,20 @@ static int populate_sdt_note(Elf **elf, const char *data, size_t len,
goto out_free_prov;
}
+ args = (const char *)memchr(name, '\0', data + len - name);
+
+ /*
+ * There is no argument if:
+ * - We reached the end of the note;
+ * - There is not enough room to hold a potential string;
+ * - The argument string is empty or just contains ':'.
+ */
+ if (args == NULL || data + len - args < 2 ||
+ args[1] == ':' || args[1] == '\0')
+ tmp->args = NULL;
+ else
+ tmp->args = strdup(++args);
+
if (gelf_getclass(*elf) == ELFCLASS32) {
memcpy(&tmp->addr, &buf, 3 * sizeof(Elf32_Addr));
tmp->bit32 = true;
diff --git a/tools/perf/util/symbol.h b/tools/perf/util/symbol.h
index 2d0a905..913be07 100644
--- a/tools/perf/util/symbol.h
+++ b/tools/perf/util/symbol.h
@@ -347,6 +347,7 @@ int arch__choose_best_symbol(struct symbol *syma, struct symbol *symb);
struct sdt_note {
char *name; /* name of the note*/
char *provider; /* provider name */
+ char *args;
bool bit32; /* whether the location is 32 bits? */
union { /* location, base and semaphore addrs */
Elf64_Addr a64[3];
--
2.10.2
Hi Alexis,
On 11/17/2016 05:26 AM, Alexis Berlemont wrote:
> An sdt probe can be associated with arguments but they were not passed
> to the user probe tracing interface (uprobe_events); this patch adapts
> the sdt argument descriptors according to the uprobe input format.
>
> As the uprobe parser does not support scaled address mode, perf will
> skip arguments which cannot be adapted to the uprobe format.
>
> Here are the results:
>
> $ perf buildid-cache -v --add test_sdt
> $ perf probe -x test_sdt sdt_libfoo:table_frob
> $ perf probe -x test_sdt sdt_libfoo:table_diddle
> $ perf record -e sdt_libfoo:table_frob -e sdt_libfoo:table_diddle test_sdt
> $ perf script
> test_sdt ... 666.255678: sdt_libfoo:table_frob: (4004d7) arg0=0 arg1=0
> test_sdt ... 666.255683: sdt_libfoo:table_diddle: (40051a) arg0=0 arg1=0
> test_sdt ... 666.255686: sdt_libfoo:table_frob: (4004d7) arg0=1 arg1=2
> test_sdt ... 666.255689: sdt_libfoo:table_diddle: (40051a) arg0=3 arg1=4
> test_sdt ... 666.255692: sdt_libfoo:table_frob: (4004d7) arg0=2 arg1=4
> test_sdt ... 666.255694: sdt_libfoo:table_diddle: (40051a) arg0=6 arg1=8
Cool! Thanks for working on this.
I have a comment below.
> Signed-off-by: Alexis Berlemont <[email protected]>
> ---
> tools/perf/util/probe-file.c | 176 ++++++++++++++++++++++++++++++++++++++++++-
> 1 file changed, 172 insertions(+), 4 deletions(-)
>
> diff --git a/tools/perf/util/probe-file.c b/tools/perf/util/probe-file.c
> index 436b647..a97a170 100644
> --- a/tools/perf/util/probe-file.c
> +++ b/tools/perf/util/probe-file.c
> @@ -28,6 +28,46 @@
> #include "probe-file.h"
> #include "session.h"
>
> +#ifdef HAVE_GELF_GETNOTE_SUPPORT
> +
> +/*
> + * Local declarations needed for adjusting gcc/gas-generated registers
> + * before filling the uprobe tracer interface.
> + */
> +
> +struct sdt_reg_renaming {
> + const char *sdt_name;
> + const char *uprobe_name;
> +};
> +
> +#define REG_RENAMING(n, m) {.sdt_name = "%" #n, .uprobe_name = "%" #m}
> +#define REG_RENAMING_END {.sdt_name = NULL, .uprobe_name = NULL}
> +
> +static const struct sdt_reg_renaming sdt_reg_renaming_table[] = {
> + REG_RENAMING(eax, ax),
> + REG_RENAMING(rax, ax),
> + REG_RENAMING(ebx, bx),
> + REG_RENAMING(rbx, bx),
> + REG_RENAMING(ecx, cx),
> + REG_RENAMING(rcx, cx),
> + REG_RENAMING(edx, dx),
> + REG_RENAMING(rdx, dx),
> + REG_RENAMING(esi, si),
> + REG_RENAMING(rsi, si),
> + REG_RENAMING(edi, di),
> + REG_RENAMING(rdi, di),
> + REG_RENAMING(ebp, bp),
> + REG_RENAMING(rbp, bp),
> + REG_RENAMING_END,
> +};
Please put the above in arch helper headers for x86, as these register
names and their conversions are specific to x86.
[SNIP]
--
Thanks,
Hemant Kumar
During a "perf buildid-cache --add" command, the section
".note.stapsdt" of the "added" binary is scanned in order to list the
available SDT markers available in a binary. The parts containing the
probes arguments were left unscanned.
The whole section is now parsed; the probe arguments are extracted for
later use.
Signed-off-by: Alexis Berlemont <[email protected]>
---
tools/perf/util/symbol-elf.c | 16 +++++++++++++++-
tools/perf/util/symbol.h | 1 +
2 files changed, 16 insertions(+), 1 deletion(-)
diff --git a/tools/perf/util/symbol-elf.c b/tools/perf/util/symbol-elf.c
index 99400b0..0fbe0b2 100644
--- a/tools/perf/util/symbol-elf.c
+++ b/tools/perf/util/symbol-elf.c
@@ -1822,7 +1822,7 @@ void kcore_extract__delete(struct kcore_extract *kce)
static int populate_sdt_note(Elf **elf, const char *data, size_t len,
struct list_head *sdt_notes)
{
- const char *provider, *name;
+ const char *provider, *name, *args;
struct sdt_note *tmp = NULL;
GElf_Ehdr ehdr;
GElf_Addr base_off = 0;
@@ -1881,6 +1881,20 @@ static int populate_sdt_note(Elf **elf, const char *data, size_t len,
goto out_free_prov;
}
+ args = (const char *)memchr(name, '\0', data + len - name);
+
+ /*
+ * There is no argument if:
+ * - We reached the end of the note;
+ * - There is not enough room to hold a potential string;
+ * - The argument string is empty or just contains ':'.
+ */
+ if (args == NULL || data + len - args < 2 ||
+ args[1] == ':' || args[1] == '\0')
+ tmp->args = NULL;
+ else
+ tmp->args = strdup(++args);
+
if (gelf_getclass(*elf) == ELFCLASS32) {
memcpy(&tmp->addr, &buf, 3 * sizeof(Elf32_Addr));
tmp->bit32 = true;
diff --git a/tools/perf/util/symbol.h b/tools/perf/util/symbol.h
index 2d0a905..913be07 100644
--- a/tools/perf/util/symbol.h
+++ b/tools/perf/util/symbol.h
@@ -347,6 +347,7 @@ int arch__choose_best_symbol(struct symbol *syma, struct symbol *symb);
struct sdt_note {
char *name; /* name of the note*/
char *provider; /* provider name */
+ char *args;
bool bit32; /* whether the location is 32 bits? */
union { /* location, base and semaphore addrs */
Elf64_Addr a64[3];
--
2.10.2
Hi Hemant,
Many thanks for your answer.
Here is another proposal in which the x86 register renaming table has
been moved into the x86-specific part.
Thanks,
Alexis.
Alexis Berlemont (2):
perf sdt: add scanning of sdt probles arguments
perf probe: add sdt probes arguments into the uprobe cmd string
tools/perf/arch/x86/util/perf_regs.c | 18 +++++
tools/perf/util/perf_regs.c | 4 +
tools/perf/util/perf_regs.h | 13 ++++
tools/perf/util/probe-file.c | 141 ++++++++++++++++++++++++++++++++++-
tools/perf/util/symbol-elf.c | 16 +++-
tools/perf/util/symbol.h | 1 +
6 files changed, 188 insertions(+), 5 deletions(-)
--
2.10.2
An sdt probe can be associated with arguments but they were not passed
to the user probe tracing interface (uprobe_events); this patch adapts
the sdt argument descriptors according to the uprobe input format.
As the uprobe parser does not support scaled address mode, perf will
skip arguments which cannot be adapted to the uprobe format.
Here are the results:
$ perf buildid-cache -v --add test_sdt
$ perf probe -x test_sdt sdt_libfoo:table_frob
$ perf probe -x test_sdt sdt_libfoo:table_diddle
$ perf record -e sdt_libfoo:table_frob -e sdt_libfoo:table_diddle test_sdt
$ perf script
test_sdt ... 666.255678: sdt_libfoo:table_frob: (4004d7) arg0=0 arg1=0
test_sdt ... 666.255683: sdt_libfoo:table_diddle: (40051a) arg0=0 arg1=0
test_sdt ... 666.255686: sdt_libfoo:table_frob: (4004d7) arg0=1 arg1=2
test_sdt ... 666.255689: sdt_libfoo:table_diddle: (40051a) arg0=3 arg1=4
test_sdt ... 666.255692: sdt_libfoo:table_frob: (4004d7) arg0=2 arg1=4
test_sdt ... 666.255694: sdt_libfoo:table_diddle: (40051a) arg0=6 arg1=8
Signed-off-by: Alexis Berlemont <[email protected]>
---
tools/perf/arch/x86/util/perf_regs.c | 18 +++++
tools/perf/util/perf_regs.c | 4 +
tools/perf/util/perf_regs.h | 13 ++++
tools/perf/util/probe-file.c | 141 ++++++++++++++++++++++++++++++++++-
4 files changed, 172 insertions(+), 4 deletions(-)
diff --git a/tools/perf/arch/x86/util/perf_regs.c b/tools/perf/arch/x86/util/perf_regs.c
index c5db14f..52a1e65 100644
--- a/tools/perf/arch/x86/util/perf_regs.c
+++ b/tools/perf/arch/x86/util/perf_regs.c
@@ -26,3 +26,21 @@ const struct sample_reg sample_reg_masks[] = {
#endif
SMPL_REG_END
};
+
+const struct sdt_name_reg sdt_reg_renamings[] = {
+ SDT_NAME_REG(eax, ax),
+ SDT_NAME_REG(rax, ax),
+ SDT_NAME_REG(ebx, bx),
+ SDT_NAME_REG(rbx, bx),
+ SDT_NAME_REG(ecx, cx),
+ SDT_NAME_REG(rcx, cx),
+ SDT_NAME_REG(edx, dx),
+ SDT_NAME_REG(rdx, dx),
+ SDT_NAME_REG(esi, si),
+ SDT_NAME_REG(rsi, si),
+ SDT_NAME_REG(edi, di),
+ SDT_NAME_REG(rdi, di),
+ SDT_NAME_REG(ebp, bp),
+ SDT_NAME_REG(rbp, bp),
+ SDT_NAME_REG_END,
+};
diff --git a/tools/perf/util/perf_regs.c b/tools/perf/util/perf_regs.c
index c4023f2..1c21150 100644
--- a/tools/perf/util/perf_regs.c
+++ b/tools/perf/util/perf_regs.c
@@ -6,6 +6,10 @@ const struct sample_reg __weak sample_reg_masks[] = {
SMPL_REG_END
};
+const struct sdt_name_reg __weak sdt_reg_renamings[] = {
+ SDT_NAME_REG_END,
+};
+
#ifdef HAVE_PERF_REGS_SUPPORT
int perf_reg_value(u64 *valp, struct regs_dump *regs, int id)
{
diff --git a/tools/perf/util/perf_regs.h b/tools/perf/util/perf_regs.h
index 679d6e4..41815ca 100644
--- a/tools/perf/util/perf_regs.h
+++ b/tools/perf/util/perf_regs.h
@@ -15,6 +15,19 @@ struct sample_reg {
extern const struct sample_reg sample_reg_masks[];
+struct sdt_name_reg {
+ const char *sdt_name;
+ const char *uprobe_name;
+};
+#define SDT_NAME_REG(n, m) {.sdt_name = "%" #n, .uprobe_name = "%" #m}
+#define SDT_NAME_REG_END {.sdt_name = NULL, .uprobe_name = NULL}
+
+/*
+ * The table sdt_reg_renamings is used for adjusting gcc/gas-generated
+ * registers before filling the uprobe tracer interface.
+ */
+extern const struct sdt_name_reg sdt_reg_renamings[];
+
#ifdef HAVE_PERF_REGS_SUPPORT
#include <perf_regs.h>
diff --git a/tools/perf/util/probe-file.c b/tools/perf/util/probe-file.c
index 436b647..587763d 100644
--- a/tools/perf/util/probe-file.c
+++ b/tools/perf/util/probe-file.c
@@ -27,6 +27,7 @@
#include "probe-event.h"
#include "probe-file.h"
#include "session.h"
+#include "perf_regs.h"
#define MAX_CMDLEN 256
@@ -687,6 +688,137 @@ static unsigned long long sdt_note__get_addr(struct sdt_note *note)
: (unsigned long long)note->addr.a64[0];
}
+static const char * const type_to_suffix[] = {
+ ":s64", "", "", "", ":s32", "", ":s16", ":s8",
+ "", ":u8", ":u16", "", ":u32", "", "", "", ":u64"
+};
+
+static int synthesize_sdt_probe_arg(struct strbuf *buf, int i, const char *arg)
+{
+ const struct sdt_name_reg *rnames;
+ char *tmp, *desc = strdup(arg);
+ const char *suffix = "";
+ int ret = -1;
+
+ if (desc == NULL) {
+ pr_debug4("Allocation error\n");
+ return ret;
+ }
+
+ /*
+ * The uprobe tracer format does not support all the
+ * addressing modes (notably: in x86 the scaled mode); so, we
+ * detect ',' characters, if there is just one, there is no
+ * use converting the sdt arg into a uprobe one.
+ */
+ if (strchr(desc, ',')) {
+ pr_debug4("SDT argument format not supported\n");
+ goto out;
+ }
+
+ tmp = strchr(desc, '@');
+ if (tmp) {
+ long type_idx;
+ /*
+ * Isolate the string number and convert it into a
+ * binary value; this will be an index to get suffix
+ * of the uprobe name (defining the type)
+ */
+ tmp[0] = '\0';
+ type_idx = strtol(desc, NULL, 10);
+ if (type_idx == LONG_MIN ||
+ type_idx == LONG_MAX) {
+ pr_debug4("Failed to get sdt type\n");
+ goto error;
+ }
+ suffix = type_to_suffix[type_idx + 8];
+ /* Get rid of the sdt prefix which is now useless */
+ tmp++;
+ memmove(desc, tmp, strlen(tmp) + 1);
+ }
+
+ /*
+ * The uprobe parser does not support all gas register names;
+ * so, we have to replace them (ex. for x86_64: %rax -> %ax);
+ * the loop below performs all the needed renamings if needed.
+ */
+
+ for (rnames = sdt_reg_renamings; rnames->sdt_name != NULL; rnames++) {
+ char *new_desc, *sdt_name;
+ size_t prefix_len, uprobe_len, mid_ofs, desc_len;
+
+ sdt_name = strstr(desc, rnames->sdt_name);
+ if (sdt_name == NULL)
+ continue;
+
+ new_desc = zalloc(strlen(desc) + 1 +
+ strlen(rnames->uprobe_name) -
+ strlen(rnames->sdt_name));
+ if (new_desc == NULL)
+ goto error;
+
+ prefix_len = sdt_name - desc;
+ if (prefix_len != 0)
+ memcpy(new_desc, desc, prefix_len);
+
+ uprobe_len = strlen(rnames->uprobe_name);
+ memcpy(new_desc + prefix_len, rnames->uprobe_name, uprobe_len);
+
+ mid_ofs = prefix_len + strlen(rnames->sdt_name);
+ desc_len = strlen(desc);
+ if (mid_ofs < desc_len)
+ memcpy(new_desc + prefix_len + uprobe_len,
+ desc + mid_ofs, desc_len - mid_ofs);
+
+ free(desc);
+ desc = new_desc;
+ }
+
+ if (strbuf_addf(buf, " arg%d=%s%s", i, desc, suffix) < 0)
+ goto error;
+
+out:
+ ret = 0;
+error:
+ free(desc);
+ return ret;
+}
+
+static char *synthesize_sdt_probe_command(struct sdt_note *note,
+ const char *pathname,
+ const char *sdtgrp)
+{
+ struct strbuf buf;
+ char *ret = NULL, **args;
+ int i, args_count;
+
+ if (strbuf_init(&buf, 32) < 0)
+ return NULL;
+
+ if (strbuf_addf(&buf, "p:%s/%s %s:0x%llx",
+ sdtgrp, note->name, pathname,
+ sdt_note__get_addr(note)) < 0)
+ goto error;
+
+ if (!note->args)
+ goto out;
+
+ if (note->args) {
+ args = argv_split(note->args, &args_count);
+
+ for (i = 0; i < args_count; ++i) {
+ if (synthesize_sdt_probe_arg(&buf, i, args[i]) < 0)
+ goto error;
+ }
+ }
+
+out:
+ ret = strbuf_detach(&buf, NULL);
+error:
+ strbuf_release(&buf);
+ return ret;
+}
+
int probe_cache__scan_sdt(struct probe_cache *pcache, const char *pathname)
{
struct probe_cache_entry *entry = NULL;
@@ -723,11 +855,12 @@ int probe_cache__scan_sdt(struct probe_cache *pcache, const char *pathname)
entry->pev.group = strdup(sdtgrp);
list_add_tail(&entry->node, &pcache->entries);
}
- ret = asprintf(&buf, "p:%s/%s %s:0x%llx",
- sdtgrp, note->name, pathname,
- sdt_note__get_addr(note));
- if (ret < 0)
+ buf = synthesize_sdt_probe_command(note, pathname, sdtgrp);
+ if (!buf) {
+ ret = -ENOMEM;
break;
+ }
+
strlist__add(entry->tevlist, buf);
free(buf);
entry = NULL;
--
2.10.2
Hi Alexis,
On 11/19/2016 05:26 AM, Alexis Berlemont wrote:
> An sdt probe can be associated with arguments but they were not passed
> to the user probe tracing interface (uprobe_events); this patch adapts
> the sdt argument descriptors according to the uprobe input format.
>
> As the uprobe parser does not support scaled address mode, perf will
> skip arguments which cannot be adapted to the uprobe format.
>
> Here are the results:
>
> $ perf buildid-cache -v --add test_sdt
> $ perf probe -x test_sdt sdt_libfoo:table_frob
Working for some binaries and not working for others. On some digging, I
think, this patchset doesn't take care of the offset sign. So, when we
try to probe with these patches applied :
# ./perf probe -x /lib64/libpthread.so.0 sdt_libpthread:pthread_start
Failed to write event: Invalid argument
Error: Failed to add events.
With -v, the string to be written in uprobe_events is shown as :
...
Writing event: p:sdt_libpthread/pthread_start
/usr/lib64/libpthread-2.23.so:0x75b8 arg0=%ax:u64 arg1=1600(%ax):u64
arg2=1608(%ax):u64
...
The offsets mentioned in the string, for e.g., arg1=1600(%ax):u64 (to be
written into the uprobe_events file don't have any sign. That is the
issue here, i.e, we need to have a sign before the offsets.
This works fine :
# echo "p:sdt_libpthread/pthread_start
/usr/lib64/libpthread-2.23.so:0x75b8 arg0=%ax:u64 arg1=+1600(%ax):u64
arg2=+1608(%ax):u64" > /sys/kernel/debug/tracing/uprobe_events
For the negative offsets, i.e., '-' sign before the offsets, this works
fine because, it takes the sign as is.
So, we need to explicitly mention the offset sign.
--
Thanks,
Hemant Kumar
> $ perf probe -x test_sdt sdt_libfoo:table_diddle
> $ perf record -e sdt_libfoo:table_frob -e sdt_libfoo:table_diddle test_sdt
> $ perf script
> test_sdt ... 666.255678: sdt_libfoo:table_frob: (4004d7) arg0=0 arg1=0
> test_sdt ... 666.255683: sdt_libfoo:table_diddle: (40051a) arg0=0 arg1=0
> test_sdt ... 666.255686: sdt_libfoo:table_frob: (4004d7) arg0=1 arg1=2
> test_sdt ... 666.255689: sdt_libfoo:table_diddle: (40051a) arg0=3 arg1=4
> test_sdt ... 666.255692: sdt_libfoo:table_frob: (4004d7) arg0=2 arg1=4
> test_sdt ... 666.255694: sdt_libfoo:table_diddle: (40051a) arg0=6 arg1=8
>
> Signed-off-by: Alexis Berlemont <[email protected]>
> ---
> tools/perf/arch/x86/util/perf_regs.c | 18 +++++
> tools/perf/util/perf_regs.c | 4 +
> tools/perf/util/perf_regs.h | 13 ++++
> tools/perf/util/probe-file.c | 141 ++++++++++++++++++++++++++++++++++-
> 4 files changed, 172 insertions(+), 4 deletions(-)
>
> diff --git a/tools/perf/arch/x86/util/perf_regs.c b/tools/perf/arch/x86/util/perf_regs.c
> index c5db14f..52a1e65 100644
> --- a/tools/perf/arch/x86/util/perf_regs.c
> +++ b/tools/perf/arch/x86/util/perf_regs.c
> @@ -26,3 +26,21 @@ const struct sample_reg sample_reg_masks[] = {
> #endif
> SMPL_REG_END
> };
> +
> +const struct sdt_name_reg sdt_reg_renamings[] = {
> + SDT_NAME_REG(eax, ax),
> + SDT_NAME_REG(rax, ax),
> + SDT_NAME_REG(ebx, bx),
> + SDT_NAME_REG(rbx, bx),
> + SDT_NAME_REG(ecx, cx),
> + SDT_NAME_REG(rcx, cx),
> + SDT_NAME_REG(edx, dx),
> + SDT_NAME_REG(rdx, dx),
> + SDT_NAME_REG(esi, si),
> + SDT_NAME_REG(rsi, si),
> + SDT_NAME_REG(edi, di),
> + SDT_NAME_REG(rdi, di),
> + SDT_NAME_REG(ebp, bp),
> + SDT_NAME_REG(rbp, bp),
> + SDT_NAME_REG_END,
> +};
> diff --git a/tools/perf/util/perf_regs.c b/tools/perf/util/perf_regs.c
> index c4023f2..1c21150 100644
> --- a/tools/perf/util/perf_regs.c
> +++ b/tools/perf/util/perf_regs.c
> @@ -6,6 +6,10 @@ const struct sample_reg __weak sample_reg_masks[] = {
> SMPL_REG_END
> };
>
> +const struct sdt_name_reg __weak sdt_reg_renamings[] = {
> + SDT_NAME_REG_END,
> +};
> +
> #ifdef HAVE_PERF_REGS_SUPPORT
> int perf_reg_value(u64 *valp, struct regs_dump *regs, int id)
> {
> diff --git a/tools/perf/util/perf_regs.h b/tools/perf/util/perf_regs.h
> index 679d6e4..41815ca 100644
> --- a/tools/perf/util/perf_regs.h
> +++ b/tools/perf/util/perf_regs.h
> @@ -15,6 +15,19 @@ struct sample_reg {
>
> extern const struct sample_reg sample_reg_masks[];
>
> +struct sdt_name_reg {
> + const char *sdt_name;
> + const char *uprobe_name;
> +};
> +#define SDT_NAME_REG(n, m) {.sdt_name = "%" #n, .uprobe_name = "%" #m}
> +#define SDT_NAME_REG_END {.sdt_name = NULL, .uprobe_name = NULL}
> +
> +/*
> + * The table sdt_reg_renamings is used for adjusting gcc/gas-generated
> + * registers before filling the uprobe tracer interface.
> + */
> +extern const struct sdt_name_reg sdt_reg_renamings[];
> +
> #ifdef HAVE_PERF_REGS_SUPPORT
> #include <perf_regs.h>
>
> diff --git a/tools/perf/util/probe-file.c b/tools/perf/util/probe-file.c
> index 436b647..587763d 100644
> --- a/tools/perf/util/probe-file.c
> +++ b/tools/perf/util/probe-file.c
> @@ -27,6 +27,7 @@
> #include "probe-event.h"
> #include "probe-file.h"
> #include "session.h"
> +#include "perf_regs.h"
>
> #define MAX_CMDLEN 256
>
> @@ -687,6 +688,137 @@ static unsigned long long sdt_note__get_addr(struct sdt_note *note)
> : (unsigned long long)note->addr.a64[0];
> }
>
> +static const char * const type_to_suffix[] = {
> + ":s64", "", "", "", ":s32", "", ":s16", ":s8",
> + "", ":u8", ":u16", "", ":u32", "", "", "", ":u64"
> +};
> +
> +static int synthesize_sdt_probe_arg(struct strbuf *buf, int i, const char *arg)
> +{
> + const struct sdt_name_reg *rnames;
> + char *tmp, *desc = strdup(arg);
> + const char *suffix = "";
> + int ret = -1;
> +
> + if (desc == NULL) {
> + pr_debug4("Allocation error\n");
> + return ret;
> + }
> +
> + /*
> + * The uprobe tracer format does not support all the
> + * addressing modes (notably: in x86 the scaled mode); so, we
> + * detect ',' characters, if there is just one, there is no
> + * use converting the sdt arg into a uprobe one.
> + */
> + if (strchr(desc, ',')) {
> + pr_debug4("SDT argument format not supported\n");
> + goto out;
> + }
> +
> + tmp = strchr(desc, '@');
> + if (tmp) {
> + long type_idx;
> + /*
> + * Isolate the string number and convert it into a
> + * binary value; this will be an index to get suffix
> + * of the uprobe name (defining the type)
> + */
> + tmp[0] = '\0';
> + type_idx = strtol(desc, NULL, 10);
> + if (type_idx == LONG_MIN ||
> + type_idx == LONG_MAX) {
> + pr_debug4("Failed to get sdt type\n");
> + goto error;
> + }
> + suffix = type_to_suffix[type_idx + 8];
> + /* Get rid of the sdt prefix which is now useless */
> + tmp++;
> + memmove(desc, tmp, strlen(tmp) + 1);
> + }
> +
> + /*
> + * The uprobe parser does not support all gas register names;
> + * so, we have to replace them (ex. for x86_64: %rax -> %ax);
> + * the loop below performs all the needed renamings if needed.
> + */
> +
> + for (rnames = sdt_reg_renamings; rnames->sdt_name != NULL; rnames++) {
> + char *new_desc, *sdt_name;
> + size_t prefix_len, uprobe_len, mid_ofs, desc_len;
> +
> + sdt_name = strstr(desc, rnames->sdt_name);
> + if (sdt_name == NULL)
> + continue;
> +
> + new_desc = zalloc(strlen(desc) + 1 +
> + strlen(rnames->uprobe_name) -
> + strlen(rnames->sdt_name));
> + if (new_desc == NULL)
> + goto error;
> +
> + prefix_len = sdt_name - desc;
> + if (prefix_len != 0)
> + memcpy(new_desc, desc, prefix_len);
> +
> + uprobe_len = strlen(rnames->uprobe_name);
> + memcpy(new_desc + prefix_len, rnames->uprobe_name, uprobe_len);
> +
> + mid_ofs = prefix_len + strlen(rnames->sdt_name);
> + desc_len = strlen(desc);
> + if (mid_ofs < desc_len)
> + memcpy(new_desc + prefix_len + uprobe_len,
> + desc + mid_ofs, desc_len - mid_ofs);
> +
> + free(desc);
> + desc = new_desc;
> + }
> +
> + if (strbuf_addf(buf, " arg%d=%s%s", i, desc, suffix) < 0)
> + goto error;
> +
> +out:
> + ret = 0;
> +error:
> + free(desc);
> + return ret;
> +}
> +
> +static char *synthesize_sdt_probe_command(struct sdt_note *note,
> + const char *pathname,
> + const char *sdtgrp)
> +{
> + struct strbuf buf;
> + char *ret = NULL, **args;
> + int i, args_count;
> +
> + if (strbuf_init(&buf, 32) < 0)
> + return NULL;
> +
> + if (strbuf_addf(&buf, "p:%s/%s %s:0x%llx",
> + sdtgrp, note->name, pathname,
> + sdt_note__get_addr(note)) < 0)
> + goto error;
> +
> + if (!note->args)
> + goto out;
> +
> + if (note->args) {
> + args = argv_split(note->args, &args_count);
> +
> + for (i = 0; i < args_count; ++i) {
> + if (synthesize_sdt_probe_arg(&buf, i, args[i]) < 0)
> + goto error;
> + }
> + }
> +
> +out:
> + ret = strbuf_detach(&buf, NULL);
> +error:
> + strbuf_release(&buf);
> + return ret;
> +}
> +
> int probe_cache__scan_sdt(struct probe_cache *pcache, const char *pathname)
> {
> struct probe_cache_entry *entry = NULL;
> @@ -723,11 +855,12 @@ int probe_cache__scan_sdt(struct probe_cache *pcache, const char *pathname)
> entry->pev.group = strdup(sdtgrp);
> list_add_tail(&entry->node, &pcache->entries);
> }
> - ret = asprintf(&buf, "p:%s/%s %s:0x%llx",
> - sdtgrp, note->name, pathname,
> - sdt_note__get_addr(note));
> - if (ret < 0)
> + buf = synthesize_sdt_probe_command(note, pathname, sdtgrp);
> + if (!buf) {
> + ret = -ENOMEM;
> break;
> + }
> +
> strlist__add(entry->tevlist, buf);
> free(buf);
> entry = NULL;
During a "perf buildid-cache --add" command, the section
".note.stapsdt" of the "added" binary is scanned in order to list the
available SDT markers available in a binary. The parts containing the
probes arguments were left unscanned.
The whole section is now parsed; the probe arguments are extracted for
later use.
Signed-off-by: Alexis Berlemont <[email protected]>
---
tools/perf/util/symbol-elf.c | 16 +++++++++++++++-
tools/perf/util/symbol.h | 1 +
2 files changed, 16 insertions(+), 1 deletion(-)
diff --git a/tools/perf/util/symbol-elf.c b/tools/perf/util/symbol-elf.c
index 99400b0..0fbe0b2 100644
--- a/tools/perf/util/symbol-elf.c
+++ b/tools/perf/util/symbol-elf.c
@@ -1822,7 +1822,7 @@ void kcore_extract__delete(struct kcore_extract *kce)
static int populate_sdt_note(Elf **elf, const char *data, size_t len,
struct list_head *sdt_notes)
{
- const char *provider, *name;
+ const char *provider, *name, *args;
struct sdt_note *tmp = NULL;
GElf_Ehdr ehdr;
GElf_Addr base_off = 0;
@@ -1881,6 +1881,20 @@ static int populate_sdt_note(Elf **elf, const char *data, size_t len,
goto out_free_prov;
}
+ args = (const char *)memchr(name, '\0', data + len - name);
+
+ /*
+ * There is no argument if:
+ * - We reached the end of the note;
+ * - There is not enough room to hold a potential string;
+ * - The argument string is empty or just contains ':'.
+ */
+ if (args == NULL || data + len - args < 2 ||
+ args[1] == ':' || args[1] == '\0')
+ tmp->args = NULL;
+ else
+ tmp->args = strdup(++args);
+
if (gelf_getclass(*elf) == ELFCLASS32) {
memcpy(&tmp->addr, &buf, 3 * sizeof(Elf32_Addr));
tmp->bit32 = true;
diff --git a/tools/perf/util/symbol.h b/tools/perf/util/symbol.h
index 2d0a905..913be07 100644
--- a/tools/perf/util/symbol.h
+++ b/tools/perf/util/symbol.h
@@ -347,6 +347,7 @@ int arch__choose_best_symbol(struct symbol *syma, struct symbol *symb);
struct sdt_note {
char *name; /* name of the note*/
char *provider; /* provider name */
+ char *args;
bool bit32; /* whether the location is 32 bits? */
union { /* location, base and semaphore addrs */
Elf64_Addr a64[3];
--
2.10.2
An sdt probe can be associated with arguments but they were not passed
to the user probe tracing interface (uprobe_events); this patch adapts
the sdt argument descriptors according to the uprobe input format.
As the uprobe parser does not support scaled address mode, perf will
skip arguments which cannot be adapted to the uprobe format.
Here are the results:
$ perf buildid-cache -v --add test_sdt
$ perf probe -x test_sdt sdt_libfoo:table_frob
$ perf probe -x test_sdt sdt_libfoo:table_diddle
$ perf record -e sdt_libfoo:table_frob -e sdt_libfoo:table_diddle test_sdt
$ perf script
test_sdt ... 666.255678: sdt_libfoo:table_frob: (4004d7) arg0=0 arg1=0
test_sdt ... 666.255683: sdt_libfoo:table_diddle: (40051a) arg0=0 arg1=0
test_sdt ... 666.255686: sdt_libfoo:table_frob: (4004d7) arg0=1 arg1=2
test_sdt ... 666.255689: sdt_libfoo:table_diddle: (40051a) arg0=3 arg1=4
test_sdt ... 666.255692: sdt_libfoo:table_frob: (4004d7) arg0=2 arg1=4
test_sdt ... 666.255694: sdt_libfoo:table_diddle: (40051a) arg0=6 arg1=8
Signed-off-by: Alexis Berlemont <[email protected]>
---
tools/perf/arch/x86/util/perf_regs.c | 18 ++++
tools/perf/util/perf_regs.c | 4 +
tools/perf/util/perf_regs.h | 13 +++
tools/perf/util/probe-file.c | 169 ++++++++++++++++++++++++++++++++++-
4 files changed, 200 insertions(+), 4 deletions(-)
diff --git a/tools/perf/arch/x86/util/perf_regs.c b/tools/perf/arch/x86/util/perf_regs.c
index c5db14f..52a1e65 100644
--- a/tools/perf/arch/x86/util/perf_regs.c
+++ b/tools/perf/arch/x86/util/perf_regs.c
@@ -26,3 +26,21 @@ const struct sample_reg sample_reg_masks[] = {
#endif
SMPL_REG_END
};
+
+const struct sdt_name_reg sdt_reg_renamings[] = {
+ SDT_NAME_REG(eax, ax),
+ SDT_NAME_REG(rax, ax),
+ SDT_NAME_REG(ebx, bx),
+ SDT_NAME_REG(rbx, bx),
+ SDT_NAME_REG(ecx, cx),
+ SDT_NAME_REG(rcx, cx),
+ SDT_NAME_REG(edx, dx),
+ SDT_NAME_REG(rdx, dx),
+ SDT_NAME_REG(esi, si),
+ SDT_NAME_REG(rsi, si),
+ SDT_NAME_REG(edi, di),
+ SDT_NAME_REG(rdi, di),
+ SDT_NAME_REG(ebp, bp),
+ SDT_NAME_REG(rbp, bp),
+ SDT_NAME_REG_END,
+};
diff --git a/tools/perf/util/perf_regs.c b/tools/perf/util/perf_regs.c
index c4023f2..1c21150 100644
--- a/tools/perf/util/perf_regs.c
+++ b/tools/perf/util/perf_regs.c
@@ -6,6 +6,10 @@ const struct sample_reg __weak sample_reg_masks[] = {
SMPL_REG_END
};
+const struct sdt_name_reg __weak sdt_reg_renamings[] = {
+ SDT_NAME_REG_END,
+};
+
#ifdef HAVE_PERF_REGS_SUPPORT
int perf_reg_value(u64 *valp, struct regs_dump *regs, int id)
{
diff --git a/tools/perf/util/perf_regs.h b/tools/perf/util/perf_regs.h
index 679d6e4..41815ca 100644
--- a/tools/perf/util/perf_regs.h
+++ b/tools/perf/util/perf_regs.h
@@ -15,6 +15,19 @@ struct sample_reg {
extern const struct sample_reg sample_reg_masks[];
+struct sdt_name_reg {
+ const char *sdt_name;
+ const char *uprobe_name;
+};
+#define SDT_NAME_REG(n, m) {.sdt_name = "%" #n, .uprobe_name = "%" #m}
+#define SDT_NAME_REG_END {.sdt_name = NULL, .uprobe_name = NULL}
+
+/*
+ * The table sdt_reg_renamings is used for adjusting gcc/gas-generated
+ * registers before filling the uprobe tracer interface.
+ */
+extern const struct sdt_name_reg sdt_reg_renamings[];
+
#ifdef HAVE_PERF_REGS_SUPPORT
#include <perf_regs.h>
diff --git a/tools/perf/util/probe-file.c b/tools/perf/util/probe-file.c
index 436b647..75033c7 100644
--- a/tools/perf/util/probe-file.c
+++ b/tools/perf/util/probe-file.c
@@ -27,6 +27,7 @@
#include "probe-event.h"
#include "probe-file.h"
#include "session.h"
+#include "perf_regs.h"
#define MAX_CMDLEN 256
@@ -687,6 +688,165 @@ static unsigned long long sdt_note__get_addr(struct sdt_note *note)
: (unsigned long long)note->addr.a64[0];
}
+static const char * const type_to_suffix[] = {
+ ":s64", "", "", "", ":s32", "", ":s16", ":s8",
+ "", ":u8", ":u16", "", ":u32", "", "", "", ":u64"
+};
+
+static int synthesize_sdt_probe_arg(struct strbuf *buf, int i, const char *arg)
+{
+ const struct sdt_name_reg *rnames;
+ char *tmp, *desc = strdup(arg);
+ const char *prefix = "", *suffix = "";
+ int ret = -1;
+
+ if (desc == NULL) {
+ pr_debug4("Allocation error\n");
+ return ret;
+ }
+
+ tmp = strchr(desc, '@');
+ if (tmp) {
+ long type_idx;
+ /*
+ * Isolate the string number and convert it into a
+ * binary value; this will be an index to get suffix
+ * of the uprobe name (defining the type)
+ */
+ tmp[0] = '\0';
+ type_idx = strtol(desc, NULL, 10);
+ if (type_idx == LONG_MIN ||
+ type_idx == LONG_MAX) {
+ pr_debug4("Failed to get sdt type\n");
+ goto error;
+ }
+ suffix = type_to_suffix[type_idx + 8];
+ /* Get rid of the sdt prefix which is now useless */
+ tmp++;
+ memmove(desc, tmp, strlen(tmp) + 1);
+ }
+
+ /*
+ * The uprobe tracer format does not support all the
+ * addressing modes (notably: in x86 the scaled mode); so, we
+ * detect ',' characters, if there is just one, there is no
+ * use converting the sdt arg into a uprobe one.
+ */
+ if (strchr(desc, ',')) {
+ pr_debug4("SDT argument format not supported\n");
+ goto out;
+ }
+
+ /*
+ * If the argument addressing mode is indirect, we must check
+ * a few things...
+ */
+ tmp = strchr(desc, '(');
+ if (tmp) {
+ int j;
+
+ /*
+ * ...if the addressing mode is indirect with a
+ * positive offset (ex.: "1608(%ax)"), we need to add
+ * a '+' prefix so as to be compliant with uprobe
+ * format.
+ */
+ if (desc[0] != '+' && desc[0] != '-')
+ prefix = "+";
+
+ /*
+ * ...or if the addressing mode is indirect with a symbol
+ * as offset, the argument will not be supported by
+ * the uprobe tracer format; so, let's skip this one.
+ */
+ for (j = 0; j < tmp - desc; j++) {
+ if (desc[j] != '+' && desc[j] != '-' &&
+ !isdigit(desc[j]))
+ goto out;
+ }
+ }
+
+ /*
+ * The uprobe parser does not support all gas register names;
+ * so, we have to replace them (ex. for x86_64: %rax -> %ax);
+ * the loop below performs all the needed renamings if needed.
+ */
+ for (rnames = sdt_reg_renamings; rnames->sdt_name != NULL; rnames++) {
+ char *new_desc, *sdt_name;
+ size_t prefix_len, uprobe_len, mid_ofs, desc_len;
+
+ sdt_name = strstr(desc, rnames->sdt_name);
+ if (sdt_name == NULL)
+ continue;
+
+ new_desc = zalloc(strlen(desc) + 1 +
+ strlen(rnames->uprobe_name) -
+ strlen(rnames->sdt_name));
+ if (new_desc == NULL)
+ goto error;
+
+ prefix_len = sdt_name - desc;
+ if (prefix_len != 0)
+ memcpy(new_desc, desc, prefix_len);
+
+ uprobe_len = strlen(rnames->uprobe_name);
+ memcpy(new_desc + prefix_len, rnames->uprobe_name, uprobe_len);
+
+ mid_ofs = prefix_len + strlen(rnames->sdt_name);
+ desc_len = strlen(desc);
+ if (mid_ofs < desc_len)
+ memcpy(new_desc + prefix_len + uprobe_len,
+ desc + mid_ofs, desc_len - mid_ofs);
+
+ free(desc);
+ desc = new_desc;
+ }
+
+ if (strbuf_addf(buf, " arg%d=%s%s%s", i, prefix, desc, suffix) < 0)
+ goto error;
+
+out:
+ ret = 0;
+error:
+ free(desc);
+ return ret;
+}
+
+static char *synthesize_sdt_probe_command(struct sdt_note *note,
+ const char *pathname,
+ const char *sdtgrp)
+{
+ struct strbuf buf;
+ char *ret = NULL, **args;
+ int i, args_count;
+
+ if (strbuf_init(&buf, 32) < 0)
+ return NULL;
+
+ if (strbuf_addf(&buf, "p:%s/%s %s:0x%llx",
+ sdtgrp, note->name, pathname,
+ sdt_note__get_addr(note)) < 0)
+ goto error;
+
+ if (!note->args)
+ goto out;
+
+ if (note->args) {
+ args = argv_split(note->args, &args_count);
+
+ for (i = 0; i < args_count; ++i) {
+ if (synthesize_sdt_probe_arg(&buf, i, args[i]) < 0)
+ goto error;
+ }
+ }
+
+out:
+ ret = strbuf_detach(&buf, NULL);
+error:
+ strbuf_release(&buf);
+ return ret;
+}
+
int probe_cache__scan_sdt(struct probe_cache *pcache, const char *pathname)
{
struct probe_cache_entry *entry = NULL;
@@ -723,11 +883,12 @@ int probe_cache__scan_sdt(struct probe_cache *pcache, const char *pathname)
entry->pev.group = strdup(sdtgrp);
list_add_tail(&entry->node, &pcache->entries);
}
- ret = asprintf(&buf, "p:%s/%s %s:0x%llx",
- sdtgrp, note->name, pathname,
- sdt_note__get_addr(note));
- if (ret < 0)
+ buf = synthesize_sdt_probe_command(note, pathname, sdtgrp);
+ if (!buf) {
+ ret = -ENOMEM;
break;
+ }
+
strlist__add(entry->tevlist, buf);
free(buf);
entry = NULL;
--
2.10.2
Hi Hemant,
Once more thank you for your answer.
Sorry for this bug: I tested the patches only on small sample
binaries. Now it is tested against the systemtap-enabled libraries
libc and libpthread. There were 2 problems:
* The one you disclosed: in indirect addressing mode, positive offsets
not prefixed with a '+' character;
* Another one still in indirect addressing mode: offsets which were
not constants but symbols;
The following patches solves them both.
# cat /sys/kernel/tracing/uprobe_events
p:sdt_libpthread/pthread_start /lib/libpthread-2.23.so:0x0000000000007448 arg0=%ax:u64 arg1=+1600(%ax):u64 arg2=+1608(%ax):u64
p:sdt_libpthread/pthread_create /lib/libpthread-2.23.so:0x0000000000007be9 arg0=%ax:u64 arg1=-184(%bp):u64 arg2=-160(%bp):u64 arg3=-168(%bp):u64
p:sdt_libpthread/pthread_join /lib/libpthread-2.23.so:0x000000000000864d arg0=%di:u64
p:sdt_libpthread/pthread_join_ret /lib/libpthread-2.23.so:0x00000000000086fe arg0=%bx:u64 arg1=%ax:s32 arg2=+1584(%bx):u64
p:sdt_libpthread/mutex_init /lib/libpthread-2.23.so:0x000000000000938b arg0=%di:u64
p:sdt_libpthread/mutex_destroy /lib/libpthread-2.23.so:0x0000000000009410 arg0=%di:u64
p:sdt_libpthread/mutex_acquired /lib/libpthread-2.23.so:0x0000000000009685 arg0=%bx:u64
p:sdt_libpthread/mutex_acquired_1 /lib/libpthread-2.23.so:0x0000000000009b31 arg0=%r8:u64
...
# cat /sys/kernel/tracing/uprobe_events
p:sdt_libc/setjmp /lib/libc-2.23.so:0x0000000000033181 arg0=%di:u64 arg1=%si:s32 arg2=%ax:u64
p:sdt_libc/longjmp /lib/libc-2.23.so:0x0000000000033263 arg0=%di:u64 arg1=%si:s32 arg2=%dx:u64
p:sdt_libc/longjmp_1 /lib/libc-2.23.so:0x00000000000f7fc3 arg0=%di:u64 arg1=%si:s32 arg2=%dx:u64
p:sdt_libc/longjmp_target /lib/libc-2.23.so:0x000000000003327f arg0=%di:u64 arg1=%ax:s32 arg2=%dx:u64
p:sdt_libc/longjmp_target_1 /lib/libc-2.23.so:0x00000000000f7fdf arg0=%di:u64 arg1=%ax:s32 arg2=%dx:u64
...
Thanks,
Alexis.
Alexis Berlemont (2):
perf sdt: add scanning of sdt probles arguments
perf probe: add sdt probes arguments into the uprobe cmd string
tools/perf/arch/x86/util/perf_regs.c | 18 ++++
tools/perf/util/perf_regs.c | 4 +
tools/perf/util/perf_regs.h | 13 +++
tools/perf/util/probe-file.c | 169 ++++++++++++++++++++++++++++++++++-
tools/perf/util/symbol-elf.c | 16 +++-
tools/perf/util/symbol.h | 1 +
6 files changed, 216 insertions(+), 5 deletions(-)
--
2.10.2
Em Sat, Nov 19, 2016 at 12:56:36AM +0100, Alexis Berlemont escreveu:
> During a "perf buildid-cache --add" command, the section
> ".note.stapsdt" of the "added" binary is scanned in order to list the
> available SDT markers available in a binary. The parts containing the
> probes arguments were left unscanned.
>
> The whole section is now parsed; the probe arguments are extracted for
> later use.
>
> Signed-off-by: Alexis Berlemont <[email protected]>
> ---
> tools/perf/util/symbol-elf.c | 16 +++++++++++++++-
> tools/perf/util/symbol.h | 1 +
> 2 files changed, 16 insertions(+), 1 deletion(-)
>
> diff --git a/tools/perf/util/symbol-elf.c b/tools/perf/util/symbol-elf.c
> index 99400b0..0fbe0b2 100644
> --- a/tools/perf/util/symbol-elf.c
> +++ b/tools/perf/util/symbol-elf.c
> @@ -1822,7 +1822,7 @@ void kcore_extract__delete(struct kcore_extract *kce)
> static int populate_sdt_note(Elf **elf, const char *data, size_t len,
> struct list_head *sdt_notes)
> {
> - const char *provider, *name;
> + const char *provider, *name, *args;
> struct sdt_note *tmp = NULL;
> GElf_Ehdr ehdr;
> GElf_Addr base_off = 0;
> @@ -1881,6 +1881,20 @@ static int populate_sdt_note(Elf **elf, const char *data, size_t len,
> goto out_free_prov;
> }
>
> + args = (const char *)memchr(name, '\0', data + len - name);
Humm, no need for casting?
> +
> + /*
> + * There is no argument if:
> + * - We reached the end of the note;
> + * - There is not enough room to hold a potential string;
> + * - The argument string is empty or just contains ':'.
> + */
> + if (args == NULL || data + len - args < 2 ||
> + args[1] == ':' || args[1] == '\0')
> + tmp->args = NULL;
> + else
> + tmp->args = strdup(++args);
Shouldn't we check this and do error back propagation? I.e. if there are
args and we don't handle them, silently, that looks bad
> +
> if (gelf_getclass(*elf) == ELFCLASS32) {
> memcpy(&tmp->addr, &buf, 3 * sizeof(Elf32_Addr));
> tmp->bit32 = true;
> diff --git a/tools/perf/util/symbol.h b/tools/perf/util/symbol.h
> index 2d0a905..913be07 100644
> --- a/tools/perf/util/symbol.h
> +++ b/tools/perf/util/symbol.h
> @@ -347,6 +347,7 @@ int arch__choose_best_symbol(struct symbol *syma, struct symbol *symb);
> struct sdt_note {
> char *name; /* name of the note*/
> char *provider; /* provider name */
> + char *args;
> bool bit32; /* whether the location is 32 bits? */
> union { /* location, base and semaphore addrs */
> Elf64_Addr a64[3];
> --
> 2.10.2
Hi Arnaldo,
Here is another patch set which fixes the issues you noticed.
Thank you.
Alexis.
Alexis Berlemont (2):
perf sdt: add scanning of sdt probles arguments
perf probe: add sdt probes arguments into the uprobe cmd string
tools/perf/arch/x86/util/perf_regs.c | 18 ++++
tools/perf/util/perf_regs.c | 4 +
tools/perf/util/perf_regs.h | 13 +++
tools/perf/util/probe-file.c | 169 ++++++++++++++++++++++++++++++++++-
tools/perf/util/symbol-elf.c | 25 +++++-
tools/perf/util/symbol.h | 1 +
6 files changed, 224 insertions(+), 6 deletions(-)
--
2.10.2
During a "perf buildid-cache --add" command, the section
".note.stapsdt" of the "added" binary is scanned in order to list the
available SDT markers available in a binary. The parts containing the
probes arguments were left unscanned.
The whole section is now parsed; the probe arguments are extracted for
later use.
Signed-off-by: Alexis Berlemont <[email protected]>
---
tools/perf/util/symbol-elf.c | 25 +++++++++++++++++++++++--
tools/perf/util/symbol.h | 1 +
2 files changed, 24 insertions(+), 2 deletions(-)
diff --git a/tools/perf/util/symbol-elf.c b/tools/perf/util/symbol-elf.c
index 99400b0..7725c3f 100644
--- a/tools/perf/util/symbol-elf.c
+++ b/tools/perf/util/symbol-elf.c
@@ -1822,7 +1822,7 @@ void kcore_extract__delete(struct kcore_extract *kce)
static int populate_sdt_note(Elf **elf, const char *data, size_t len,
struct list_head *sdt_notes)
{
- const char *provider, *name;
+ const char *provider, *name, *args;
struct sdt_note *tmp = NULL;
GElf_Ehdr ehdr;
GElf_Addr base_off = 0;
@@ -1881,6 +1881,25 @@ static int populate_sdt_note(Elf **elf, const char *data, size_t len,
goto out_free_prov;
}
+ args = memchr(name, '\0', data + len - name);
+
+ /*
+ * There is no argument if:
+ * - We reached the end of the note;
+ * - There is not enough room to hold a potential string;
+ * - The argument string is empty or just contains ':'.
+ */
+ if (args == NULL || data + len - args < 2 ||
+ args[1] == ':' || args[1] == '\0')
+ tmp->args = NULL;
+ else {
+ tmp->args = strdup(++args);
+ if (!tmp->args) {
+ ret = -ENOMEM;
+ goto out_free_name;
+ }
+ }
+
if (gelf_getclass(*elf) == ELFCLASS32) {
memcpy(&tmp->addr, &buf, 3 * sizeof(Elf32_Addr));
tmp->bit32 = true;
@@ -1892,7 +1911,7 @@ static int populate_sdt_note(Elf **elf, const char *data, size_t len,
if (!gelf_getehdr(*elf, &ehdr)) {
pr_debug("%s : cannot get elf header.\n", __func__);
ret = -EBADF;
- goto out_free_name;
+ goto out_free_args;
}
/* Adjust the prelink effect :
@@ -1917,6 +1936,8 @@ static int populate_sdt_note(Elf **elf, const char *data, size_t len,
list_add_tail(&tmp->note_list, sdt_notes);
return 0;
+out_free_args:
+ free(tmp->args);
out_free_name:
free(tmp->name);
out_free_prov:
diff --git a/tools/perf/util/symbol.h b/tools/perf/util/symbol.h
index dec7e2d4..db1953e 100644
--- a/tools/perf/util/symbol.h
+++ b/tools/perf/util/symbol.h
@@ -348,6 +348,7 @@ int arch__choose_best_symbol(struct symbol *syma, struct symbol *symb);
struct sdt_note {
char *name; /* name of the note*/
char *provider; /* provider name */
+ char *args;
bool bit32; /* whether the location is 32 bits? */
union { /* location, base and semaphore addrs */
Elf64_Addr a64[3];
--
2.10.2
An sdt probe can be associated with arguments but they were not passed
to the user probe tracing interface (uprobe_events); this patch adapts
the sdt argument descriptors according to the uprobe input format.
As the uprobe parser does not support scaled address mode, perf will
skip arguments which cannot be adapted to the uprobe format.
Here are the results:
$ perf buildid-cache -v --add test_sdt
$ perf probe -x test_sdt sdt_libfoo:table_frob
$ perf probe -x test_sdt sdt_libfoo:table_diddle
$ perf record -e sdt_libfoo:table_frob -e sdt_libfoo:table_diddle test_sdt
$ perf script
test_sdt ... 666.255678: sdt_libfoo:table_frob: (4004d7) arg0=0 arg1=0
test_sdt ... 666.255683: sdt_libfoo:table_diddle: (40051a) arg0=0 arg1=0
test_sdt ... 666.255686: sdt_libfoo:table_frob: (4004d7) arg0=1 arg1=2
test_sdt ... 666.255689: sdt_libfoo:table_diddle: (40051a) arg0=3 arg1=4
test_sdt ... 666.255692: sdt_libfoo:table_frob: (4004d7) arg0=2 arg1=4
test_sdt ... 666.255694: sdt_libfoo:table_diddle: (40051a) arg0=6 arg1=8
Signed-off-by: Alexis Berlemont <[email protected]>
---
tools/perf/arch/x86/util/perf_regs.c | 18 ++++
tools/perf/util/perf_regs.c | 4 +
tools/perf/util/perf_regs.h | 13 +++
tools/perf/util/probe-file.c | 169 ++++++++++++++++++++++++++++++++++-
4 files changed, 200 insertions(+), 4 deletions(-)
diff --git a/tools/perf/arch/x86/util/perf_regs.c b/tools/perf/arch/x86/util/perf_regs.c
index c5db14f..52a1e65 100644
--- a/tools/perf/arch/x86/util/perf_regs.c
+++ b/tools/perf/arch/x86/util/perf_regs.c
@@ -26,3 +26,21 @@ const struct sample_reg sample_reg_masks[] = {
#endif
SMPL_REG_END
};
+
+const struct sdt_name_reg sdt_reg_renamings[] = {
+ SDT_NAME_REG(eax, ax),
+ SDT_NAME_REG(rax, ax),
+ SDT_NAME_REG(ebx, bx),
+ SDT_NAME_REG(rbx, bx),
+ SDT_NAME_REG(ecx, cx),
+ SDT_NAME_REG(rcx, cx),
+ SDT_NAME_REG(edx, dx),
+ SDT_NAME_REG(rdx, dx),
+ SDT_NAME_REG(esi, si),
+ SDT_NAME_REG(rsi, si),
+ SDT_NAME_REG(edi, di),
+ SDT_NAME_REG(rdi, di),
+ SDT_NAME_REG(ebp, bp),
+ SDT_NAME_REG(rbp, bp),
+ SDT_NAME_REG_END,
+};
diff --git a/tools/perf/util/perf_regs.c b/tools/perf/util/perf_regs.c
index c4023f2..1c21150 100644
--- a/tools/perf/util/perf_regs.c
+++ b/tools/perf/util/perf_regs.c
@@ -6,6 +6,10 @@ const struct sample_reg __weak sample_reg_masks[] = {
SMPL_REG_END
};
+const struct sdt_name_reg __weak sdt_reg_renamings[] = {
+ SDT_NAME_REG_END,
+};
+
#ifdef HAVE_PERF_REGS_SUPPORT
int perf_reg_value(u64 *valp, struct regs_dump *regs, int id)
{
diff --git a/tools/perf/util/perf_regs.h b/tools/perf/util/perf_regs.h
index 679d6e4..41815ca 100644
--- a/tools/perf/util/perf_regs.h
+++ b/tools/perf/util/perf_regs.h
@@ -15,6 +15,19 @@ struct sample_reg {
extern const struct sample_reg sample_reg_masks[];
+struct sdt_name_reg {
+ const char *sdt_name;
+ const char *uprobe_name;
+};
+#define SDT_NAME_REG(n, m) {.sdt_name = "%" #n, .uprobe_name = "%" #m}
+#define SDT_NAME_REG_END {.sdt_name = NULL, .uprobe_name = NULL}
+
+/*
+ * The table sdt_reg_renamings is used for adjusting gcc/gas-generated
+ * registers before filling the uprobe tracer interface.
+ */
+extern const struct sdt_name_reg sdt_reg_renamings[];
+
#ifdef HAVE_PERF_REGS_SUPPORT
#include <perf_regs.h>
diff --git a/tools/perf/util/probe-file.c b/tools/perf/util/probe-file.c
index 436b647..75033c7 100644
--- a/tools/perf/util/probe-file.c
+++ b/tools/perf/util/probe-file.c
@@ -27,6 +27,7 @@
#include "probe-event.h"
#include "probe-file.h"
#include "session.h"
+#include "perf_regs.h"
#define MAX_CMDLEN 256
@@ -687,6 +688,165 @@ static unsigned long long sdt_note__get_addr(struct sdt_note *note)
: (unsigned long long)note->addr.a64[0];
}
+static const char * const type_to_suffix[] = {
+ ":s64", "", "", "", ":s32", "", ":s16", ":s8",
+ "", ":u8", ":u16", "", ":u32", "", "", "", ":u64"
+};
+
+static int synthesize_sdt_probe_arg(struct strbuf *buf, int i, const char *arg)
+{
+ const struct sdt_name_reg *rnames;
+ char *tmp, *desc = strdup(arg);
+ const char *prefix = "", *suffix = "";
+ int ret = -1;
+
+ if (desc == NULL) {
+ pr_debug4("Allocation error\n");
+ return ret;
+ }
+
+ tmp = strchr(desc, '@');
+ if (tmp) {
+ long type_idx;
+ /*
+ * Isolate the string number and convert it into a
+ * binary value; this will be an index to get suffix
+ * of the uprobe name (defining the type)
+ */
+ tmp[0] = '\0';
+ type_idx = strtol(desc, NULL, 10);
+ if (type_idx == LONG_MIN ||
+ type_idx == LONG_MAX) {
+ pr_debug4("Failed to get sdt type\n");
+ goto error;
+ }
+ suffix = type_to_suffix[type_idx + 8];
+ /* Get rid of the sdt prefix which is now useless */
+ tmp++;
+ memmove(desc, tmp, strlen(tmp) + 1);
+ }
+
+ /*
+ * The uprobe tracer format does not support all the
+ * addressing modes (notably: in x86 the scaled mode); so, we
+ * detect ',' characters, if there is just one, there is no
+ * use converting the sdt arg into a uprobe one.
+ */
+ if (strchr(desc, ',')) {
+ pr_debug4("SDT argument format not supported\n");
+ goto out;
+ }
+
+ /*
+ * If the argument addressing mode is indirect, we must check
+ * a few things...
+ */
+ tmp = strchr(desc, '(');
+ if (tmp) {
+ int j;
+
+ /*
+ * ...if the addressing mode is indirect with a
+ * positive offset (ex.: "1608(%ax)"), we need to add
+ * a '+' prefix so as to be compliant with uprobe
+ * format.
+ */
+ if (desc[0] != '+' && desc[0] != '-')
+ prefix = "+";
+
+ /*
+ * ...or if the addressing mode is indirect with a symbol
+ * as offset, the argument will not be supported by
+ * the uprobe tracer format; so, let's skip this one.
+ */
+ for (j = 0; j < tmp - desc; j++) {
+ if (desc[j] != '+' && desc[j] != '-' &&
+ !isdigit(desc[j]))
+ goto out;
+ }
+ }
+
+ /*
+ * The uprobe parser does not support all gas register names;
+ * so, we have to replace them (ex. for x86_64: %rax -> %ax);
+ * the loop below performs all the needed renamings if needed.
+ */
+ for (rnames = sdt_reg_renamings; rnames->sdt_name != NULL; rnames++) {
+ char *new_desc, *sdt_name;
+ size_t prefix_len, uprobe_len, mid_ofs, desc_len;
+
+ sdt_name = strstr(desc, rnames->sdt_name);
+ if (sdt_name == NULL)
+ continue;
+
+ new_desc = zalloc(strlen(desc) + 1 +
+ strlen(rnames->uprobe_name) -
+ strlen(rnames->sdt_name));
+ if (new_desc == NULL)
+ goto error;
+
+ prefix_len = sdt_name - desc;
+ if (prefix_len != 0)
+ memcpy(new_desc, desc, prefix_len);
+
+ uprobe_len = strlen(rnames->uprobe_name);
+ memcpy(new_desc + prefix_len, rnames->uprobe_name, uprobe_len);
+
+ mid_ofs = prefix_len + strlen(rnames->sdt_name);
+ desc_len = strlen(desc);
+ if (mid_ofs < desc_len)
+ memcpy(new_desc + prefix_len + uprobe_len,
+ desc + mid_ofs, desc_len - mid_ofs);
+
+ free(desc);
+ desc = new_desc;
+ }
+
+ if (strbuf_addf(buf, " arg%d=%s%s%s", i, prefix, desc, suffix) < 0)
+ goto error;
+
+out:
+ ret = 0;
+error:
+ free(desc);
+ return ret;
+}
+
+static char *synthesize_sdt_probe_command(struct sdt_note *note,
+ const char *pathname,
+ const char *sdtgrp)
+{
+ struct strbuf buf;
+ char *ret = NULL, **args;
+ int i, args_count;
+
+ if (strbuf_init(&buf, 32) < 0)
+ return NULL;
+
+ if (strbuf_addf(&buf, "p:%s/%s %s:0x%llx",
+ sdtgrp, note->name, pathname,
+ sdt_note__get_addr(note)) < 0)
+ goto error;
+
+ if (!note->args)
+ goto out;
+
+ if (note->args) {
+ args = argv_split(note->args, &args_count);
+
+ for (i = 0; i < args_count; ++i) {
+ if (synthesize_sdt_probe_arg(&buf, i, args[i]) < 0)
+ goto error;
+ }
+ }
+
+out:
+ ret = strbuf_detach(&buf, NULL);
+error:
+ strbuf_release(&buf);
+ return ret;
+}
+
int probe_cache__scan_sdt(struct probe_cache *pcache, const char *pathname)
{
struct probe_cache_entry *entry = NULL;
@@ -723,11 +883,12 @@ int probe_cache__scan_sdt(struct probe_cache *pcache, const char *pathname)
entry->pev.group = strdup(sdtgrp);
list_add_tail(&entry->node, &pcache->entries);
}
- ret = asprintf(&buf, "p:%s/%s %s:0x%llx",
- sdtgrp, note->name, pathname,
- sdt_note__get_addr(note));
- if (ret < 0)
+ buf = synthesize_sdt_probe_command(note, pathname, sdtgrp);
+ if (!buf) {
+ ret = -ENOMEM;
break;
+ }
+
strlist__add(entry->tevlist, buf);
free(buf);
entry = NULL;
--
2.10.2
Alexis Berlemont wrote:
> Hi Arnaldo,
>
> Here is another patch set which fixes the issues you noticed.
>
Could you indicate me a way to improve these patches so as to move
forward ?
Regards,
Alexis.
> Thank you.
>
> Alexis.
>
> Alexis Berlemont (2):
> perf sdt: add scanning of sdt probles arguments
> perf probe: add sdt probes arguments into the uprobe cmd string
>
> tools/perf/arch/x86/util/perf_regs.c | 18 ++++
> tools/perf/util/perf_regs.c | 4 +
> tools/perf/util/perf_regs.h | 13 +++
> tools/perf/util/probe-file.c | 169 ++++++++++++++++++++++++++++++++++-
> tools/perf/util/symbol-elf.c | 25 +++++-
> tools/perf/util/symbol.h | 1 +
> 6 files changed, 224 insertions(+), 6 deletions(-)
>
> --
> 2.10.2
>
Em Tue, Dec 06, 2016 at 12:42:45AM +0100, Alexis Berlemont escreveu:
> Alexis Berlemont wrote:
> > Hi Arnaldo,
> >
> > Here is another patch set which fixes the issues you noticed.
> >
>
> Could you indicate me a way to improve these patches so as to move
> forward ?
Masami, Hemant, are you guys ok with this? Can I have your Acked-by or
Tested-by, etc?
- Arnaldo
> Regards,
>
> Alexis.
>
> > Thank you.
> >
> > Alexis.
> >
> > Alexis Berlemont (2):
> > perf sdt: add scanning of sdt probles arguments
> > perf probe: add sdt probes arguments into the uprobe cmd string
> >
> > tools/perf/arch/x86/util/perf_regs.c | 18 ++++
> > tools/perf/util/perf_regs.c | 4 +
> > tools/perf/util/perf_regs.h | 13 +++
> > tools/perf/util/probe-file.c | 169 ++++++++++++++++++++++++++++++++++-
> > tools/perf/util/symbol-elf.c | 25 +++++-
> > tools/perf/util/symbol.h | 1 +
> > 6 files changed, 224 insertions(+), 6 deletions(-)
> >
> > --
> > 2.10.2
> >
On Sat, 26 Nov 2016 01:58:02 +0100
Alexis Berlemont <[email protected]> wrote:
> During a "perf buildid-cache --add" command, the section
> ".note.stapsdt" of the "added" binary is scanned in order to list the
> available SDT markers available in a binary. The parts containing the
> probes arguments were left unscanned.
>
> The whole section is now parsed; the probe arguments are extracted for
> later use.
>
Looks OK for me :)
Acked-by: Masami Hiramatsu <[email protected]>
Thanks!
> Signed-off-by: Alexis Berlemont <[email protected]>
> ---
> tools/perf/util/symbol-elf.c | 25 +++++++++++++++++++++++--
> tools/perf/util/symbol.h | 1 +
> 2 files changed, 24 insertions(+), 2 deletions(-)
>
> diff --git a/tools/perf/util/symbol-elf.c b/tools/perf/util/symbol-elf.c
> index 99400b0..7725c3f 100644
> --- a/tools/perf/util/symbol-elf.c
> +++ b/tools/perf/util/symbol-elf.c
> @@ -1822,7 +1822,7 @@ void kcore_extract__delete(struct kcore_extract *kce)
> static int populate_sdt_note(Elf **elf, const char *data, size_t len,
> struct list_head *sdt_notes)
> {
> - const char *provider, *name;
> + const char *provider, *name, *args;
> struct sdt_note *tmp = NULL;
> GElf_Ehdr ehdr;
> GElf_Addr base_off = 0;
> @@ -1881,6 +1881,25 @@ static int populate_sdt_note(Elf **elf, const char *data, size_t len,
> goto out_free_prov;
> }
>
> + args = memchr(name, '\0', data + len - name);
> +
> + /*
> + * There is no argument if:
> + * - We reached the end of the note;
> + * - There is not enough room to hold a potential string;
> + * - The argument string is empty or just contains ':'.
> + */
> + if (args == NULL || data + len - args < 2 ||
> + args[1] == ':' || args[1] == '\0')
> + tmp->args = NULL;
> + else {
> + tmp->args = strdup(++args);
> + if (!tmp->args) {
> + ret = -ENOMEM;
> + goto out_free_name;
> + }
> + }
> +
> if (gelf_getclass(*elf) == ELFCLASS32) {
> memcpy(&tmp->addr, &buf, 3 * sizeof(Elf32_Addr));
> tmp->bit32 = true;
> @@ -1892,7 +1911,7 @@ static int populate_sdt_note(Elf **elf, const char *data, size_t len,
> if (!gelf_getehdr(*elf, &ehdr)) {
> pr_debug("%s : cannot get elf header.\n", __func__);
> ret = -EBADF;
> - goto out_free_name;
> + goto out_free_args;
> }
>
> /* Adjust the prelink effect :
> @@ -1917,6 +1936,8 @@ static int populate_sdt_note(Elf **elf, const char *data, size_t len,
> list_add_tail(&tmp->note_list, sdt_notes);
> return 0;
>
> +out_free_args:
> + free(tmp->args);
> out_free_name:
> free(tmp->name);
> out_free_prov:
> diff --git a/tools/perf/util/symbol.h b/tools/perf/util/symbol.h
> index dec7e2d4..db1953e 100644
> --- a/tools/perf/util/symbol.h
> +++ b/tools/perf/util/symbol.h
> @@ -348,6 +348,7 @@ int arch__choose_best_symbol(struct symbol *syma, struct symbol *symb);
> struct sdt_note {
> char *name; /* name of the note*/
> char *provider; /* provider name */
> + char *args;
> bool bit32; /* whether the location is 32 bits? */
> union { /* location, base and semaphore addrs */
> Elf64_Addr a64[3];
> --
> 2.10.2
>
--
Masami Hiramatsu <[email protected]>
Hello Alexis,
On Sat, 26 Nov 2016 01:58:03 +0100
Alexis Berlemont <[email protected]> wrote:
> An sdt probe can be associated with arguments but they were not passed
> to the user probe tracing interface (uprobe_events); this patch adapts
> the sdt argument descriptors according to the uprobe input format.
Great!
>
> As the uprobe parser does not support scaled address mode, perf will
> skip arguments which cannot be adapted to the uprobe format.
OK, it seems that skipping argument is a good idea :)
I just tried to support fixed-number arguments in probe events,
but skipping it is better with older kernel.
I have some comments.
> Here are the results:
>
> $ perf buildid-cache -v --add test_sdt
> $ perf probe -x test_sdt sdt_libfoo:table_frob
> $ perf probe -x test_sdt sdt_libfoo:table_diddle
> $ perf record -e sdt_libfoo:table_frob -e sdt_libfoo:table_diddle test_sdt
> $ perf script
> test_sdt ... 666.255678: sdt_libfoo:table_frob: (4004d7) arg0=0 arg1=0
> test_sdt ... 666.255683: sdt_libfoo:table_diddle: (40051a) arg0=0 arg1=0
> test_sdt ... 666.255686: sdt_libfoo:table_frob: (4004d7) arg0=1 arg1=2
> test_sdt ... 666.255689: sdt_libfoo:table_diddle: (40051a) arg0=3 arg1=4
> test_sdt ... 666.255692: sdt_libfoo:table_frob: (4004d7) arg0=2 arg1=4
> test_sdt ... 666.255694: sdt_libfoo:table_diddle: (40051a) arg0=6 arg1=8
We'd better start with arg1, since sdt.h and original Dtrace SDT starts
arguments from arg1 (I'm not sure why) and dtrace/systemtap scripts
call it "arg1".
>
> Signed-off-by: Alexis Berlemont <[email protected]>
> ---
> tools/perf/arch/x86/util/perf_regs.c | 18 ++++
> tools/perf/util/perf_regs.c | 4 +
> tools/perf/util/perf_regs.h | 13 +++
> tools/perf/util/probe-file.c | 169 ++++++++++++++++++++++++++++++++++-
> 4 files changed, 200 insertions(+), 4 deletions(-)
>
> diff --git a/tools/perf/arch/x86/util/perf_regs.c b/tools/perf/arch/x86/util/perf_regs.c
> index c5db14f..52a1e65 100644
> --- a/tools/perf/arch/x86/util/perf_regs.c
> +++ b/tools/perf/arch/x86/util/perf_regs.c
> @@ -26,3 +26,21 @@ const struct sample_reg sample_reg_masks[] = {
> #endif
> SMPL_REG_END
> };
> +
> +const struct sdt_name_reg sdt_reg_renamings[] = {
> + SDT_NAME_REG(eax, ax),
> + SDT_NAME_REG(rax, ax),
> + SDT_NAME_REG(ebx, bx),
> + SDT_NAME_REG(rbx, bx),
> + SDT_NAME_REG(ecx, cx),
> + SDT_NAME_REG(rcx, cx),
> + SDT_NAME_REG(edx, dx),
> + SDT_NAME_REG(rdx, dx),
> + SDT_NAME_REG(esi, si),
> + SDT_NAME_REG(rsi, si),
> + SDT_NAME_REG(edi, di),
> + SDT_NAME_REG(rdi, di),
> + SDT_NAME_REG(ebp, bp),
> + SDT_NAME_REG(rbp, bp),
> + SDT_NAME_REG_END,
> +};
It is not enough, rNN registers also have to take care, since
gcc adds 'd', 'w' or 'b'suffixes for those registers to indicate
its size. e.g. r15d means r15 register with 32 lower bits.
What we need is just cut them off, since probe event uses
length modifiers (like :u32)
> diff --git a/tools/perf/util/perf_regs.c b/tools/perf/util/perf_regs.c
> index c4023f2..1c21150 100644
> --- a/tools/perf/util/perf_regs.c
> +++ b/tools/perf/util/perf_regs.c
> @@ -6,6 +6,10 @@ const struct sample_reg __weak sample_reg_masks[] = {
> SMPL_REG_END
> };
>
> +const struct sdt_name_reg __weak sdt_reg_renamings[] = {
> + SDT_NAME_REG_END,
> +};
> +
> #ifdef HAVE_PERF_REGS_SUPPORT
> int perf_reg_value(u64 *valp, struct regs_dump *regs, int id)
> {
> diff --git a/tools/perf/util/perf_regs.h b/tools/perf/util/perf_regs.h
> index 679d6e4..41815ca 100644
> --- a/tools/perf/util/perf_regs.h
> +++ b/tools/perf/util/perf_regs.h
> @@ -15,6 +15,19 @@ struct sample_reg {
>
> extern const struct sample_reg sample_reg_masks[];
>
> +struct sdt_name_reg {
> + const char *sdt_name;
> + const char *uprobe_name;
> +};
> +#define SDT_NAME_REG(n, m) {.sdt_name = "%" #n, .uprobe_name = "%" #m}
> +#define SDT_NAME_REG_END {.sdt_name = NULL, .uprobe_name = NULL}
> +
> +/*
> + * The table sdt_reg_renamings is used for adjusting gcc/gas-generated
> + * registers before filling the uprobe tracer interface.
> + */
> +extern const struct sdt_name_reg sdt_reg_renamings[];
> +
> #ifdef HAVE_PERF_REGS_SUPPORT
> #include <perf_regs.h>
>
> diff --git a/tools/perf/util/probe-file.c b/tools/perf/util/probe-file.c
> index 436b647..75033c7 100644
> --- a/tools/perf/util/probe-file.c
> +++ b/tools/perf/util/probe-file.c
> @@ -27,6 +27,7 @@
> #include "probe-event.h"
> #include "probe-file.h"
> #include "session.h"
> +#include "perf_regs.h"
>
> #define MAX_CMDLEN 256
>
> @@ -687,6 +688,165 @@ static unsigned long long sdt_note__get_addr(struct sdt_note *note)
> : (unsigned long long)note->addr.a64[0];
> }
>
> +static const char * const type_to_suffix[] = {
> + ":s64", "", "", "", ":s32", "", ":s16", ":s8",
> + "", ":u8", ":u16", "", ":u32", "", "", "", ":u64"
> +};
> +
> +static int synthesize_sdt_probe_arg(struct strbuf *buf, int i, const char *arg)
> +{
> + const struct sdt_name_reg *rnames;
> + char *tmp, *desc = strdup(arg);
> + const char *prefix = "", *suffix = "";
> + int ret = -1;
> +
> + if (desc == NULL) {
> + pr_debug4("Allocation error\n");
> + return ret;
> + }
> +
> + tmp = strchr(desc, '@');
> + if (tmp) {
> + long type_idx;
> + /*
> + * Isolate the string number and convert it into a
> + * binary value; this will be an index to get suffix
> + * of the uprobe name (defining the type)
> + */
> + tmp[0] = '\0';
> + type_idx = strtol(desc, NULL, 10);
> + if (type_idx == LONG_MIN ||
> + type_idx == LONG_MAX) {
> + pr_debug4("Failed to get sdt type\n");
> + goto error;
> + }
You must ensure 0 <= type_idx + 8 <= 16 here.
> + suffix = type_to_suffix[type_idx + 8];
> + /* Get rid of the sdt prefix which is now useless */
> + tmp++;
> + memmove(desc, tmp, strlen(tmp) + 1);
> + }
> +
> + /*
> + * The uprobe tracer format does not support all the
> + * addressing modes (notably: in x86 the scaled mode); so, we
> + * detect ',' characters, if there is just one, there is no
> + * use converting the sdt arg into a uprobe one.
> + */
> + if (strchr(desc, ',')) {
> + pr_debug4("SDT argument format not supported\n");
Please print 'desc' by %s too.
> + goto out;
> + }
> +
> + /*
> + * If the argument addressing mode is indirect, we must check
> + * a few things...
> + */
> + tmp = strchr(desc, '(');
> + if (tmp) {
> + int j;
> +
> + /*
> + * ...if the addressing mode is indirect with a
> + * positive offset (ex.: "1608(%ax)"), we need to add
> + * a '+' prefix so as to be compliant with uprobe
> + * format.
> + */
> + if (desc[0] != '+' && desc[0] != '-')
> + prefix = "+";
> +
> + /*
> + * ...or if the addressing mode is indirect with a symbol
> + * as offset, the argument will not be supported by
> + * the uprobe tracer format; so, let's skip this one.
> + */
> + for (j = 0; j < tmp - desc; j++) {
> + if (desc[j] != '+' && desc[j] != '-' &&
> + !isdigit(desc[j]))
> + goto out;
> + }
> + }
> +
> + /*
> + * The uprobe parser does not support all gas register names;
> + * so, we have to replace them (ex. for x86_64: %rax -> %ax);
> + * the loop below performs all the needed renamings if needed.
> + */
> + for (rnames = sdt_reg_renamings; rnames->sdt_name != NULL; rnames++) {
> + char *new_desc, *sdt_name;
> + size_t prefix_len, uprobe_len, mid_ofs, desc_len;
> +
> + sdt_name = strstr(desc, rnames->sdt_name);
> + if (sdt_name == NULL)
> + continue;
It is better to search '%' from the desc and parse it.
And here, we also find fixed numbers which starts with '$',
since that is not supported yet.
For example, with your patch, I still see some entries which have fixed num.
$ perf buildid-cache --add /usr/lib64/libglib-2.0.so
$ grep \$[0-9] ~/.debug/usr/lib64/libglib-2.0.so.0.5000.2/fda1ca4181ba7135d41bf3cfadc813a432f31066/probes | tail -n 2
p:sdt_glib/mem__realloc /usr/lib64/libglib-2.0.so.0.5000.2:0x4f670 arg0=%ax:u64 arg1=%bx:u64 arg2=%bp:u32 arg3=$0:s32
p:sdt_glib/mem__realloc /usr/lib64/libglib-2.0.so.0.5000.2:0x4f75d arg0=%ax:u64 arg1=%bp:u64 arg2=%bx:u32 arg3=$1:s32
These arguments should be skipped.
Thank you,
> +
> + new_desc = zalloc(strlen(desc) + 1 +
> + strlen(rnames->uprobe_name) -
> + strlen(rnames->sdt_name));
> + if (new_desc == NULL)
> + goto error;
> +
> + prefix_len = sdt_name - desc;
> + if (prefix_len != 0)
> + memcpy(new_desc, desc, prefix_len);
> +
> + uprobe_len = strlen(rnames->uprobe_name);
> + memcpy(new_desc + prefix_len, rnames->uprobe_name, uprobe_len);
> +
> + mid_ofs = prefix_len + strlen(rnames->sdt_name);
> + desc_len = strlen(desc);
> + if (mid_ofs < desc_len)
> + memcpy(new_desc + prefix_len + uprobe_len,
> + desc + mid_ofs, desc_len - mid_ofs);
> +
> + free(desc);
> + desc = new_desc;
> + }
> +
> + if (strbuf_addf(buf, " arg%d=%s%s%s", i, prefix, desc, suffix) < 0)
> + goto error;
> +
> +out:
> + ret = 0;
> +error:
> + free(desc);
> + return ret;
> +}
> +
> +static char *synthesize_sdt_probe_command(struct sdt_note *note,
> + const char *pathname,
> + const char *sdtgrp)
> +{
> + struct strbuf buf;
> + char *ret = NULL, **args;
> + int i, args_count;
> +
> + if (strbuf_init(&buf, 32) < 0)
> + return NULL;
> +
> + if (strbuf_addf(&buf, "p:%s/%s %s:0x%llx",
> + sdtgrp, note->name, pathname,
> + sdt_note__get_addr(note)) < 0)
> + goto error;
> +
> + if (!note->args)
> + goto out;
> +
> + if (note->args) {
> + args = argv_split(note->args, &args_count);
> +
> + for (i = 0; i < args_count; ++i) {
> + if (synthesize_sdt_probe_arg(&buf, i, args[i]) < 0)
> + goto error;
> + }
> + }
> +
> +out:
> + ret = strbuf_detach(&buf, NULL);
> +error:
> + strbuf_release(&buf);
> + return ret;
> +}
> +
> int probe_cache__scan_sdt(struct probe_cache *pcache, const char *pathname)
> {
> struct probe_cache_entry *entry = NULL;
> @@ -723,11 +883,12 @@ int probe_cache__scan_sdt(struct probe_cache *pcache, const char *pathname)
> entry->pev.group = strdup(sdtgrp);
> list_add_tail(&entry->node, &pcache->entries);
> }
> - ret = asprintf(&buf, "p:%s/%s %s:0x%llx",
> - sdtgrp, note->name, pathname,
> - sdt_note__get_addr(note));
> - if (ret < 0)
> + buf = synthesize_sdt_probe_command(note, pathname, sdtgrp);
> + if (!buf) {
> + ret = -ENOMEM;
> break;
> + }
> +
> strlist__add(entry->tevlist, buf);
> free(buf);
> entry = NULL;
> --
> 2.10.2
>
--
Masami Hiramatsu <[email protected]>
Em Wed, Dec 07, 2016 at 12:26:10PM +0900, Masami Hiramatsu escreveu:
> Hello Alexis,
>
> On Sat, 26 Nov 2016 01:58:03 +0100
> Alexis Berlemont <[email protected]> wrote:
>
> > An sdt probe can be associated with arguments but they were not passed
> > to the user probe tracing interface (uprobe_events); this patch adapts
> > the sdt argument descriptors according to the uprobe input format.
>
> Great!
Yeah, good to see work in this area!
I applied the first patch, with Masami's ack, waiting for his concerns
on this one to be addressed, ok?
- Arnaldo
> >
> > As the uprobe parser does not support scaled address mode, perf will
> > skip arguments which cannot be adapted to the uprobe format.
>
> OK, it seems that skipping argument is a good idea :)
> I just tried to support fixed-number arguments in probe events,
> but skipping it is better with older kernel.
>
> I have some comments.
>
> > Here are the results:
> >
> > $ perf buildid-cache -v --add test_sdt
> > $ perf probe -x test_sdt sdt_libfoo:table_frob
> > $ perf probe -x test_sdt sdt_libfoo:table_diddle
> > $ perf record -e sdt_libfoo:table_frob -e sdt_libfoo:table_diddle test_sdt
> > $ perf script
> > test_sdt ... 666.255678: sdt_libfoo:table_frob: (4004d7) arg0=0 arg1=0
> > test_sdt ... 666.255683: sdt_libfoo:table_diddle: (40051a) arg0=0 arg1=0
> > test_sdt ... 666.255686: sdt_libfoo:table_frob: (4004d7) arg0=1 arg1=2
> > test_sdt ... 666.255689: sdt_libfoo:table_diddle: (40051a) arg0=3 arg1=4
> > test_sdt ... 666.255692: sdt_libfoo:table_frob: (4004d7) arg0=2 arg1=4
> > test_sdt ... 666.255694: sdt_libfoo:table_diddle: (40051a) arg0=6 arg1=8
>
> We'd better start with arg1, since sdt.h and original Dtrace SDT starts
> arguments from arg1 (I'm not sure why) and dtrace/systemtap scripts
> call it "arg1".
>
> >
> > Signed-off-by: Alexis Berlemont <[email protected]>
> > ---
> > tools/perf/arch/x86/util/perf_regs.c | 18 ++++
> > tools/perf/util/perf_regs.c | 4 +
> > tools/perf/util/perf_regs.h | 13 +++
> > tools/perf/util/probe-file.c | 169 ++++++++++++++++++++++++++++++++++-
> > 4 files changed, 200 insertions(+), 4 deletions(-)
> >
> > diff --git a/tools/perf/arch/x86/util/perf_regs.c b/tools/perf/arch/x86/util/perf_regs.c
> > index c5db14f..52a1e65 100644
> > --- a/tools/perf/arch/x86/util/perf_regs.c
> > +++ b/tools/perf/arch/x86/util/perf_regs.c
> > @@ -26,3 +26,21 @@ const struct sample_reg sample_reg_masks[] = {
> > #endif
> > SMPL_REG_END
> > };
> > +
> > +const struct sdt_name_reg sdt_reg_renamings[] = {
> > + SDT_NAME_REG(eax, ax),
> > + SDT_NAME_REG(rax, ax),
> > + SDT_NAME_REG(ebx, bx),
> > + SDT_NAME_REG(rbx, bx),
> > + SDT_NAME_REG(ecx, cx),
> > + SDT_NAME_REG(rcx, cx),
> > + SDT_NAME_REG(edx, dx),
> > + SDT_NAME_REG(rdx, dx),
> > + SDT_NAME_REG(esi, si),
> > + SDT_NAME_REG(rsi, si),
> > + SDT_NAME_REG(edi, di),
> > + SDT_NAME_REG(rdi, di),
> > + SDT_NAME_REG(ebp, bp),
> > + SDT_NAME_REG(rbp, bp),
> > + SDT_NAME_REG_END,
> > +};
>
> It is not enough, rNN registers also have to take care, since
> gcc adds 'd', 'w' or 'b'suffixes for those registers to indicate
> its size. e.g. r15d means r15 register with 32 lower bits.
> What we need is just cut them off, since probe event uses
> length modifiers (like :u32)
>
> > diff --git a/tools/perf/util/perf_regs.c b/tools/perf/util/perf_regs.c
> > index c4023f2..1c21150 100644
> > --- a/tools/perf/util/perf_regs.c
> > +++ b/tools/perf/util/perf_regs.c
> > @@ -6,6 +6,10 @@ const struct sample_reg __weak sample_reg_masks[] = {
> > SMPL_REG_END
> > };
> >
> > +const struct sdt_name_reg __weak sdt_reg_renamings[] = {
> > + SDT_NAME_REG_END,
> > +};
> > +
> > #ifdef HAVE_PERF_REGS_SUPPORT
> > int perf_reg_value(u64 *valp, struct regs_dump *regs, int id)
> > {
> > diff --git a/tools/perf/util/perf_regs.h b/tools/perf/util/perf_regs.h
> > index 679d6e4..41815ca 100644
> > --- a/tools/perf/util/perf_regs.h
> > +++ b/tools/perf/util/perf_regs.h
> > @@ -15,6 +15,19 @@ struct sample_reg {
> >
> > extern const struct sample_reg sample_reg_masks[];
> >
> > +struct sdt_name_reg {
> > + const char *sdt_name;
> > + const char *uprobe_name;
> > +};
> > +#define SDT_NAME_REG(n, m) {.sdt_name = "%" #n, .uprobe_name = "%" #m}
> > +#define SDT_NAME_REG_END {.sdt_name = NULL, .uprobe_name = NULL}
> > +
> > +/*
> > + * The table sdt_reg_renamings is used for adjusting gcc/gas-generated
> > + * registers before filling the uprobe tracer interface.
> > + */
> > +extern const struct sdt_name_reg sdt_reg_renamings[];
> > +
> > #ifdef HAVE_PERF_REGS_SUPPORT
> > #include <perf_regs.h>
> >
> > diff --git a/tools/perf/util/probe-file.c b/tools/perf/util/probe-file.c
> > index 436b647..75033c7 100644
> > --- a/tools/perf/util/probe-file.c
> > +++ b/tools/perf/util/probe-file.c
> > @@ -27,6 +27,7 @@
> > #include "probe-event.h"
> > #include "probe-file.h"
> > #include "session.h"
> > +#include "perf_regs.h"
> >
> > #define MAX_CMDLEN 256
> >
> > @@ -687,6 +688,165 @@ static unsigned long long sdt_note__get_addr(struct sdt_note *note)
> > : (unsigned long long)note->addr.a64[0];
> > }
> >
> > +static const char * const type_to_suffix[] = {
> > + ":s64", "", "", "", ":s32", "", ":s16", ":s8",
> > + "", ":u8", ":u16", "", ":u32", "", "", "", ":u64"
> > +};
> > +
> > +static int synthesize_sdt_probe_arg(struct strbuf *buf, int i, const char *arg)
> > +{
> > + const struct sdt_name_reg *rnames;
> > + char *tmp, *desc = strdup(arg);
> > + const char *prefix = "", *suffix = "";
> > + int ret = -1;
> > +
> > + if (desc == NULL) {
> > + pr_debug4("Allocation error\n");
> > + return ret;
> > + }
> > +
> > + tmp = strchr(desc, '@');
> > + if (tmp) {
> > + long type_idx;
> > + /*
> > + * Isolate the string number and convert it into a
> > + * binary value; this will be an index to get suffix
> > + * of the uprobe name (defining the type)
> > + */
> > + tmp[0] = '\0';
> > + type_idx = strtol(desc, NULL, 10);
> > + if (type_idx == LONG_MIN ||
> > + type_idx == LONG_MAX) {
> > + pr_debug4("Failed to get sdt type\n");
> > + goto error;
> > + }
>
> You must ensure 0 <= type_idx + 8 <= 16 here.
>
> > + suffix = type_to_suffix[type_idx + 8];
> > + /* Get rid of the sdt prefix which is now useless */
> > + tmp++;
> > + memmove(desc, tmp, strlen(tmp) + 1);
> > + }
> > +
> > + /*
> > + * The uprobe tracer format does not support all the
> > + * addressing modes (notably: in x86 the scaled mode); so, we
> > + * detect ',' characters, if there is just one, there is no
> > + * use converting the sdt arg into a uprobe one.
> > + */
> > + if (strchr(desc, ',')) {
> > + pr_debug4("SDT argument format not supported\n");
>
> Please print 'desc' by %s too.
>
> > + goto out;
> > + }
> > +
> > + /*
> > + * If the argument addressing mode is indirect, we must check
> > + * a few things...
> > + */
> > + tmp = strchr(desc, '(');
> > + if (tmp) {
> > + int j;
> > +
> > + /*
> > + * ...if the addressing mode is indirect with a
> > + * positive offset (ex.: "1608(%ax)"), we need to add
> > + * a '+' prefix so as to be compliant with uprobe
> > + * format.
> > + */
> > + if (desc[0] != '+' && desc[0] != '-')
> > + prefix = "+";
> > +
> > + /*
> > + * ...or if the addressing mode is indirect with a symbol
> > + * as offset, the argument will not be supported by
> > + * the uprobe tracer format; so, let's skip this one.
> > + */
> > + for (j = 0; j < tmp - desc; j++) {
> > + if (desc[j] != '+' && desc[j] != '-' &&
> > + !isdigit(desc[j]))
> > + goto out;
> > + }
> > + }
> > +
> > + /*
> > + * The uprobe parser does not support all gas register names;
> > + * so, we have to replace them (ex. for x86_64: %rax -> %ax);
> > + * the loop below performs all the needed renamings if needed.
> > + */
> > + for (rnames = sdt_reg_renamings; rnames->sdt_name != NULL; rnames++) {
> > + char *new_desc, *sdt_name;
> > + size_t prefix_len, uprobe_len, mid_ofs, desc_len;
> > +
> > + sdt_name = strstr(desc, rnames->sdt_name);
> > + if (sdt_name == NULL)
> > + continue;
>
> It is better to search '%' from the desc and parse it.
> And here, we also find fixed numbers which starts with '$',
> since that is not supported yet.
>
> For example, with your patch, I still see some entries which have fixed num.
>
> $ perf buildid-cache --add /usr/lib64/libglib-2.0.so
> $ grep \$[0-9] ~/.debug/usr/lib64/libglib-2.0.so.0.5000.2/fda1ca4181ba7135d41bf3cfadc813a432f31066/probes | tail -n 2
> p:sdt_glib/mem__realloc /usr/lib64/libglib-2.0.so.0.5000.2:0x4f670 arg0=%ax:u64 arg1=%bx:u64 arg2=%bp:u32 arg3=$0:s32
> p:sdt_glib/mem__realloc /usr/lib64/libglib-2.0.so.0.5000.2:0x4f75d arg0=%ax:u64 arg1=%bp:u64 arg2=%bx:u32 arg3=$1:s32
>
> These arguments should be skipped.
>
> Thank you,
>
> > +
> > + new_desc = zalloc(strlen(desc) + 1 +
> > + strlen(rnames->uprobe_name) -
> > + strlen(rnames->sdt_name));
> > + if (new_desc == NULL)
> > + goto error;
> > +
> > + prefix_len = sdt_name - desc;
> > + if (prefix_len != 0)
> > + memcpy(new_desc, desc, prefix_len);
> > +
> > + uprobe_len = strlen(rnames->uprobe_name);
> > + memcpy(new_desc + prefix_len, rnames->uprobe_name, uprobe_len);
> > +
> > + mid_ofs = prefix_len + strlen(rnames->sdt_name);
> > + desc_len = strlen(desc);
> > + if (mid_ofs < desc_len)
> > + memcpy(new_desc + prefix_len + uprobe_len,
> > + desc + mid_ofs, desc_len - mid_ofs);
> > +
> > + free(desc);
> > + desc = new_desc;
> > + }
> > +
> > + if (strbuf_addf(buf, " arg%d=%s%s%s", i, prefix, desc, suffix) < 0)
> > + goto error;
> > +
> > +out:
> > + ret = 0;
> > +error:
> > + free(desc);
> > + return ret;
> > +}
> > +
> > +static char *synthesize_sdt_probe_command(struct sdt_note *note,
> > + const char *pathname,
> > + const char *sdtgrp)
> > +{
> > + struct strbuf buf;
> > + char *ret = NULL, **args;
> > + int i, args_count;
> > +
> > + if (strbuf_init(&buf, 32) < 0)
> > + return NULL;
> > +
> > + if (strbuf_addf(&buf, "p:%s/%s %s:0x%llx",
> > + sdtgrp, note->name, pathname,
> > + sdt_note__get_addr(note)) < 0)
> > + goto error;
> > +
> > + if (!note->args)
> > + goto out;
> > +
> > + if (note->args) {
> > + args = argv_split(note->args, &args_count);
> > +
> > + for (i = 0; i < args_count; ++i) {
> > + if (synthesize_sdt_probe_arg(&buf, i, args[i]) < 0)
> > + goto error;
> > + }
> > + }
> > +
> > +out:
> > + ret = strbuf_detach(&buf, NULL);
> > +error:
> > + strbuf_release(&buf);
> > + return ret;
> > +}
> > +
> > int probe_cache__scan_sdt(struct probe_cache *pcache, const char *pathname)
> > {
> > struct probe_cache_entry *entry = NULL;
> > @@ -723,11 +883,12 @@ int probe_cache__scan_sdt(struct probe_cache *pcache, const char *pathname)
> > entry->pev.group = strdup(sdtgrp);
> > list_add_tail(&entry->node, &pcache->entries);
> > }
> > - ret = asprintf(&buf, "p:%s/%s %s:0x%llx",
> > - sdtgrp, note->name, pathname,
> > - sdt_note__get_addr(note));
> > - if (ret < 0)
> > + buf = synthesize_sdt_probe_command(note, pathname, sdtgrp);
> > + if (!buf) {
> > + ret = -ENOMEM;
> > break;
> > + }
> > +
> > strlist__add(entry->tevlist, buf);
> > free(buf);
> > entry = NULL;
> > --
> > 2.10.2
> >
>
>
> --
> Masami Hiramatsu <[email protected]>
On Fri, 9 Dec 2016 12:14:30 -0300
Arnaldo Carvalho de Melo <[email protected]> wrote:
> Em Wed, Dec 07, 2016 at 12:26:10PM +0900, Masami Hiramatsu escreveu:
> > Hello Alexis,
> >
> > On Sat, 26 Nov 2016 01:58:03 +0100
> > Alexis Berlemont <[email protected]> wrote:
> >
> > > An sdt probe can be associated with arguments but they were not passed
> > > to the user probe tracing interface (uprobe_events); this patch adapts
> > > the sdt argument descriptors according to the uprobe input format.
> >
> > Great!
>
> Yeah, good to see work in this area!
>
> I applied the first patch, with Masami's ack, waiting for his concerns
> on this one to be addressed, ok?
Yes, I'm OK. Alexis, I'm happy to review/ack your next version! :)
Thank you,
>
> - Arnaldo
>
> > >
> > > As the uprobe parser does not support scaled address mode, perf will
> > > skip arguments which cannot be adapted to the uprobe format.
> >
> > OK, it seems that skipping argument is a good idea :)
> > I just tried to support fixed-number arguments in probe events,
> > but skipping it is better with older kernel.
> >
> > I have some comments.
> >
> > > Here are the results:
> > >
> > > $ perf buildid-cache -v --add test_sdt
> > > $ perf probe -x test_sdt sdt_libfoo:table_frob
> > > $ perf probe -x test_sdt sdt_libfoo:table_diddle
> > > $ perf record -e sdt_libfoo:table_frob -e sdt_libfoo:table_diddle test_sdt
> > > $ perf script
> > > test_sdt ... 666.255678: sdt_libfoo:table_frob: (4004d7) arg0=0 arg1=0
> > > test_sdt ... 666.255683: sdt_libfoo:table_diddle: (40051a) arg0=0 arg1=0
> > > test_sdt ... 666.255686: sdt_libfoo:table_frob: (4004d7) arg0=1 arg1=2
> > > test_sdt ... 666.255689: sdt_libfoo:table_diddle: (40051a) arg0=3 arg1=4
> > > test_sdt ... 666.255692: sdt_libfoo:table_frob: (4004d7) arg0=2 arg1=4
> > > test_sdt ... 666.255694: sdt_libfoo:table_diddle: (40051a) arg0=6 arg1=8
> >
> > We'd better start with arg1, since sdt.h and original Dtrace SDT starts
> > arguments from arg1 (I'm not sure why) and dtrace/systemtap scripts
> > call it "arg1".
> >
> > >
> > > Signed-off-by: Alexis Berlemont <[email protected]>
> > > ---
> > > tools/perf/arch/x86/util/perf_regs.c | 18 ++++
> > > tools/perf/util/perf_regs.c | 4 +
> > > tools/perf/util/perf_regs.h | 13 +++
> > > tools/perf/util/probe-file.c | 169 ++++++++++++++++++++++++++++++++++-
> > > 4 files changed, 200 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/tools/perf/arch/x86/util/perf_regs.c b/tools/perf/arch/x86/util/perf_regs.c
> > > index c5db14f..52a1e65 100644
> > > --- a/tools/perf/arch/x86/util/perf_regs.c
> > > +++ b/tools/perf/arch/x86/util/perf_regs.c
> > > @@ -26,3 +26,21 @@ const struct sample_reg sample_reg_masks[] = {
> > > #endif
> > > SMPL_REG_END
> > > };
> > > +
> > > +const struct sdt_name_reg sdt_reg_renamings[] = {
> > > + SDT_NAME_REG(eax, ax),
> > > + SDT_NAME_REG(rax, ax),
> > > + SDT_NAME_REG(ebx, bx),
> > > + SDT_NAME_REG(rbx, bx),
> > > + SDT_NAME_REG(ecx, cx),
> > > + SDT_NAME_REG(rcx, cx),
> > > + SDT_NAME_REG(edx, dx),
> > > + SDT_NAME_REG(rdx, dx),
> > > + SDT_NAME_REG(esi, si),
> > > + SDT_NAME_REG(rsi, si),
> > > + SDT_NAME_REG(edi, di),
> > > + SDT_NAME_REG(rdi, di),
> > > + SDT_NAME_REG(ebp, bp),
> > > + SDT_NAME_REG(rbp, bp),
> > > + SDT_NAME_REG_END,
> > > +};
> >
> > It is not enough, rNN registers also have to take care, since
> > gcc adds 'd', 'w' or 'b'suffixes for those registers to indicate
> > its size. e.g. r15d means r15 register with 32 lower bits.
> > What we need is just cut them off, since probe event uses
> > length modifiers (like :u32)
> >
> > > diff --git a/tools/perf/util/perf_regs.c b/tools/perf/util/perf_regs.c
> > > index c4023f2..1c21150 100644
> > > --- a/tools/perf/util/perf_regs.c
> > > +++ b/tools/perf/util/perf_regs.c
> > > @@ -6,6 +6,10 @@ const struct sample_reg __weak sample_reg_masks[] = {
> > > SMPL_REG_END
> > > };
> > >
> > > +const struct sdt_name_reg __weak sdt_reg_renamings[] = {
> > > + SDT_NAME_REG_END,
> > > +};
> > > +
> > > #ifdef HAVE_PERF_REGS_SUPPORT
> > > int perf_reg_value(u64 *valp, struct regs_dump *regs, int id)
> > > {
> > > diff --git a/tools/perf/util/perf_regs.h b/tools/perf/util/perf_regs.h
> > > index 679d6e4..41815ca 100644
> > > --- a/tools/perf/util/perf_regs.h
> > > +++ b/tools/perf/util/perf_regs.h
> > > @@ -15,6 +15,19 @@ struct sample_reg {
> > >
> > > extern const struct sample_reg sample_reg_masks[];
> > >
> > > +struct sdt_name_reg {
> > > + const char *sdt_name;
> > > + const char *uprobe_name;
> > > +};
> > > +#define SDT_NAME_REG(n, m) {.sdt_name = "%" #n, .uprobe_name = "%" #m}
> > > +#define SDT_NAME_REG_END {.sdt_name = NULL, .uprobe_name = NULL}
> > > +
> > > +/*
> > > + * The table sdt_reg_renamings is used for adjusting gcc/gas-generated
> > > + * registers before filling the uprobe tracer interface.
> > > + */
> > > +extern const struct sdt_name_reg sdt_reg_renamings[];
> > > +
> > > #ifdef HAVE_PERF_REGS_SUPPORT
> > > #include <perf_regs.h>
> > >
> > > diff --git a/tools/perf/util/probe-file.c b/tools/perf/util/probe-file.c
> > > index 436b647..75033c7 100644
> > > --- a/tools/perf/util/probe-file.c
> > > +++ b/tools/perf/util/probe-file.c
> > > @@ -27,6 +27,7 @@
> > > #include "probe-event.h"
> > > #include "probe-file.h"
> > > #include "session.h"
> > > +#include "perf_regs.h"
> > >
> > > #define MAX_CMDLEN 256
> > >
> > > @@ -687,6 +688,165 @@ static unsigned long long sdt_note__get_addr(struct sdt_note *note)
> > > : (unsigned long long)note->addr.a64[0];
> > > }
> > >
> > > +static const char * const type_to_suffix[] = {
> > > + ":s64", "", "", "", ":s32", "", ":s16", ":s8",
> > > + "", ":u8", ":u16", "", ":u32", "", "", "", ":u64"
> > > +};
> > > +
> > > +static int synthesize_sdt_probe_arg(struct strbuf *buf, int i, const char *arg)
> > > +{
> > > + const struct sdt_name_reg *rnames;
> > > + char *tmp, *desc = strdup(arg);
> > > + const char *prefix = "", *suffix = "";
> > > + int ret = -1;
> > > +
> > > + if (desc == NULL) {
> > > + pr_debug4("Allocation error\n");
> > > + return ret;
> > > + }
> > > +
> > > + tmp = strchr(desc, '@');
> > > + if (tmp) {
> > > + long type_idx;
> > > + /*
> > > + * Isolate the string number and convert it into a
> > > + * binary value; this will be an index to get suffix
> > > + * of the uprobe name (defining the type)
> > > + */
> > > + tmp[0] = '\0';
> > > + type_idx = strtol(desc, NULL, 10);
> > > + if (type_idx == LONG_MIN ||
> > > + type_idx == LONG_MAX) {
> > > + pr_debug4("Failed to get sdt type\n");
> > > + goto error;
> > > + }
> >
> > You must ensure 0 <= type_idx + 8 <= 16 here.
> >
> > > + suffix = type_to_suffix[type_idx + 8];
> > > + /* Get rid of the sdt prefix which is now useless */
> > > + tmp++;
> > > + memmove(desc, tmp, strlen(tmp) + 1);
> > > + }
> > > +
> > > + /*
> > > + * The uprobe tracer format does not support all the
> > > + * addressing modes (notably: in x86 the scaled mode); so, we
> > > + * detect ',' characters, if there is just one, there is no
> > > + * use converting the sdt arg into a uprobe one.
> > > + */
> > > + if (strchr(desc, ',')) {
> > > + pr_debug4("SDT argument format not supported\n");
> >
> > Please print 'desc' by %s too.
> >
> > > + goto out;
> > > + }
> > > +
> > > + /*
> > > + * If the argument addressing mode is indirect, we must check
> > > + * a few things...
> > > + */
> > > + tmp = strchr(desc, '(');
> > > + if (tmp) {
> > > + int j;
> > > +
> > > + /*
> > > + * ...if the addressing mode is indirect with a
> > > + * positive offset (ex.: "1608(%ax)"), we need to add
> > > + * a '+' prefix so as to be compliant with uprobe
> > > + * format.
> > > + */
> > > + if (desc[0] != '+' && desc[0] != '-')
> > > + prefix = "+";
> > > +
> > > + /*
> > > + * ...or if the addressing mode is indirect with a symbol
> > > + * as offset, the argument will not be supported by
> > > + * the uprobe tracer format; so, let's skip this one.
> > > + */
> > > + for (j = 0; j < tmp - desc; j++) {
> > > + if (desc[j] != '+' && desc[j] != '-' &&
> > > + !isdigit(desc[j]))
> > > + goto out;
> > > + }
> > > + }
> > > +
> > > + /*
> > > + * The uprobe parser does not support all gas register names;
> > > + * so, we have to replace them (ex. for x86_64: %rax -> %ax);
> > > + * the loop below performs all the needed renamings if needed.
> > > + */
> > > + for (rnames = sdt_reg_renamings; rnames->sdt_name != NULL; rnames++) {
> > > + char *new_desc, *sdt_name;
> > > + size_t prefix_len, uprobe_len, mid_ofs, desc_len;
> > > +
> > > + sdt_name = strstr(desc, rnames->sdt_name);
> > > + if (sdt_name == NULL)
> > > + continue;
> >
> > It is better to search '%' from the desc and parse it.
> > And here, we also find fixed numbers which starts with '$',
> > since that is not supported yet.
> >
> > For example, with your patch, I still see some entries which have fixed num.
> >
> > $ perf buildid-cache --add /usr/lib64/libglib-2.0.so
> > $ grep \$[0-9] ~/.debug/usr/lib64/libglib-2.0.so.0.5000.2/fda1ca4181ba7135d41bf3cfadc813a432f31066/probes | tail -n 2
> > p:sdt_glib/mem__realloc /usr/lib64/libglib-2.0.so.0.5000.2:0x4f670 arg0=%ax:u64 arg1=%bx:u64 arg2=%bp:u32 arg3=$0:s32
> > p:sdt_glib/mem__realloc /usr/lib64/libglib-2.0.so.0.5000.2:0x4f75d arg0=%ax:u64 arg1=%bp:u64 arg2=%bx:u32 arg3=$1:s32
> >
> > These arguments should be skipped.
> >
> > Thank you,
> >
> > > +
> > > + new_desc = zalloc(strlen(desc) + 1 +
> > > + strlen(rnames->uprobe_name) -
> > > + strlen(rnames->sdt_name));
> > > + if (new_desc == NULL)
> > > + goto error;
> > > +
> > > + prefix_len = sdt_name - desc;
> > > + if (prefix_len != 0)
> > > + memcpy(new_desc, desc, prefix_len);
> > > +
> > > + uprobe_len = strlen(rnames->uprobe_name);
> > > + memcpy(new_desc + prefix_len, rnames->uprobe_name, uprobe_len);
> > > +
> > > + mid_ofs = prefix_len + strlen(rnames->sdt_name);
> > > + desc_len = strlen(desc);
> > > + if (mid_ofs < desc_len)
> > > + memcpy(new_desc + prefix_len + uprobe_len,
> > > + desc + mid_ofs, desc_len - mid_ofs);
> > > +
> > > + free(desc);
> > > + desc = new_desc;
> > > + }
> > > +
> > > + if (strbuf_addf(buf, " arg%d=%s%s%s", i, prefix, desc, suffix) < 0)
> > > + goto error;
> > > +
> > > +out:
> > > + ret = 0;
> > > +error:
> > > + free(desc);
> > > + return ret;
> > > +}
> > > +
> > > +static char *synthesize_sdt_probe_command(struct sdt_note *note,
> > > + const char *pathname,
> > > + const char *sdtgrp)
> > > +{
> > > + struct strbuf buf;
> > > + char *ret = NULL, **args;
> > > + int i, args_count;
> > > +
> > > + if (strbuf_init(&buf, 32) < 0)
> > > + return NULL;
> > > +
> > > + if (strbuf_addf(&buf, "p:%s/%s %s:0x%llx",
> > > + sdtgrp, note->name, pathname,
> > > + sdt_note__get_addr(note)) < 0)
> > > + goto error;
> > > +
> > > + if (!note->args)
> > > + goto out;
> > > +
> > > + if (note->args) {
> > > + args = argv_split(note->args, &args_count);
> > > +
> > > + for (i = 0; i < args_count; ++i) {
> > > + if (synthesize_sdt_probe_arg(&buf, i, args[i]) < 0)
> > > + goto error;
> > > + }
> > > + }
> > > +
> > > +out:
> > > + ret = strbuf_detach(&buf, NULL);
> > > +error:
> > > + strbuf_release(&buf);
> > > + return ret;
> > > +}
> > > +
> > > int probe_cache__scan_sdt(struct probe_cache *pcache, const char *pathname)
> > > {
> > > struct probe_cache_entry *entry = NULL;
> > > @@ -723,11 +883,12 @@ int probe_cache__scan_sdt(struct probe_cache *pcache, const char *pathname)
> > > entry->pev.group = strdup(sdtgrp);
> > > list_add_tail(&entry->node, &pcache->entries);
> > > }
> > > - ret = asprintf(&buf, "p:%s/%s %s:0x%llx",
> > > - sdtgrp, note->name, pathname,
> > > - sdt_note__get_addr(note));
> > > - if (ret < 0)
> > > + buf = synthesize_sdt_probe_command(note, pathname, sdtgrp);
> > > + if (!buf) {
> > > + ret = -ENOMEM;
> > > break;
> > > + }
> > > +
> > > strlist__add(entry->tevlist, buf);
> > > free(buf);
> > > entry = NULL;
> > > --
> > > 2.10.2
> > >
> >
> >
> > --
> > Masami Hiramatsu <[email protected]>
--
Masami Hiramatsu <[email protected]>
Hi Masami,
Many thanks for your mail.
Here is another patch set which tries to fix the points you mentioned:
* Skip the arguments containing a constant ($123);
* Review the code in charge of the register renaming (search for '%'
and parse it);
* Minor changes (print the argument in case of error, skipping, check
the sdt arg type index);
Many thanks,
Alexis.
Alexis Berlemont (2):
perf sdt: add scanning of sdt probles arguments
perf probe: add sdt probes arguments into the uprobe cmd string
tools/perf/arch/x86/util/perf_regs.c | 83 +++++++++++++++++
tools/perf/util/perf_regs.c | 6 ++
tools/perf/util/perf_regs.h | 6 ++
tools/perf/util/probe-file.c | 170 ++++++++++++++++++++++++++++++++++-
tools/perf/util/symbol-elf.c | 25 +++++-
tools/perf/util/symbol.h | 1 +
6 files changed, 285 insertions(+), 6 deletions(-)
--
2.10.2
An sdt probe can be associated with arguments but they were not passed
to the user probe tracing interface (uprobe_events); this patch adapts
the sdt argument descriptors according to the uprobe input format.
As the uprobe parser does not support scaled address mode, perf will
skip arguments which cannot be adapted to the uprobe format.
Here are the results:
$ perf buildid-cache -v --add test_sdt
$ perf probe -x test_sdt sdt_libfoo:table_frob
$ perf probe -x test_sdt sdt_libfoo:table_diddle
$ perf record -e sdt_libfoo:table_frob -e sdt_libfoo:table_diddle test_sdt
$ perf script
test_sdt ... 666.255678: sdt_libfoo:table_frob: (4004d7) arg0=0 arg1=0
test_sdt ... 666.255683: sdt_libfoo:table_diddle: (40051a) arg0=0 arg1=0
test_sdt ... 666.255686: sdt_libfoo:table_frob: (4004d7) arg0=1 arg1=2
test_sdt ... 666.255689: sdt_libfoo:table_diddle: (40051a) arg0=3 arg1=4
test_sdt ... 666.255692: sdt_libfoo:table_frob: (4004d7) arg0=2 arg1=4
test_sdt ... 666.255694: sdt_libfoo:table_diddle: (40051a) arg0=6 arg1=8
Signed-off-by: Alexis Berlemont <[email protected]>
---
tools/perf/arch/x86/util/perf_regs.c | 83 +++++++++++++++++
tools/perf/util/perf_regs.c | 6 ++
tools/perf/util/perf_regs.h | 6 ++
tools/perf/util/probe-file.c | 170 ++++++++++++++++++++++++++++++++++-
4 files changed, 261 insertions(+), 4 deletions(-)
diff --git a/tools/perf/arch/x86/util/perf_regs.c b/tools/perf/arch/x86/util/perf_regs.c
index c5db14f..09a7f55 100644
--- a/tools/perf/arch/x86/util/perf_regs.c
+++ b/tools/perf/arch/x86/util/perf_regs.c
@@ -1,4 +1,7 @@
+#include <string.h>
+
#include "../../perf.h"
+#include "../../util/util.h"
#include "../../util/perf_regs.h"
const struct sample_reg sample_reg_masks[] = {
@@ -26,3 +29,83 @@ const struct sample_reg sample_reg_masks[] = {
#endif
SMPL_REG_END
};
+
+struct sdt_name_reg {
+ const char *sdt_name;
+ const char *uprobe_name;
+};
+#define SDT_NAME_REG(n, m) {.sdt_name = "%" #n, .uprobe_name = "%" #m}
+#define SDT_NAME_REG_END {.sdt_name = NULL, .uprobe_name = NULL}
+
+static const struct sdt_name_reg sdt_reg_renamings[] = {
+ SDT_NAME_REG(eax, ax),
+ SDT_NAME_REG(rax, ax),
+ SDT_NAME_REG(ebx, bx),
+ SDT_NAME_REG(rbx, bx),
+ SDT_NAME_REG(ecx, cx),
+ SDT_NAME_REG(rcx, cx),
+ SDT_NAME_REG(edx, dx),
+ SDT_NAME_REG(rdx, dx),
+ SDT_NAME_REG(esi, si),
+ SDT_NAME_REG(rsi, si),
+ SDT_NAME_REG(edi, di),
+ SDT_NAME_REG(rdi, di),
+ SDT_NAME_REG(ebp, bp),
+ SDT_NAME_REG(rbp, bp),
+ SDT_NAME_REG_END,
+};
+
+int sdt_rename_register(char **pdesc, char *old_name)
+{
+ const struct sdt_name_reg *rnames = sdt_reg_renamings;
+ char *new_desc, *old_desc = *pdesc;
+ size_t prefix_len, sdt_len, uprobe_len, old_desc_len, offset;
+ int ret = -1;
+
+ while (ret != 0 && rnames->sdt_name != NULL) {
+ sdt_len = strlen(rnames->sdt_name);
+ ret = strncmp(old_name, rnames->sdt_name, sdt_len);
+ rnames += !!ret;
+ }
+
+ if (rnames->sdt_name == NULL)
+ return 0;
+
+ sdt_len = strlen(rnames->sdt_name);
+ uprobe_len = strlen(rnames->uprobe_name);
+ old_desc_len = strlen(old_desc) + 1;
+
+ new_desc = zalloc(old_desc_len + uprobe_len - sdt_len);
+ if (new_desc == NULL)
+ return -1;
+
+ /* Copy the chars before the register name (at least '%') */
+ prefix_len = old_name - old_desc;
+ memcpy(new_desc, old_desc, prefix_len);
+
+ /* Copy the new register name */
+ memcpy(new_desc + prefix_len, rnames->uprobe_name, uprobe_len);
+
+ /* Copy the chars after the register name (if need be) */
+ offset = prefix_len + sdt_len;
+ if (offset < old_desc_len) {
+ /*
+ * The orginal register name can be suffixed by 'b',
+ * 'w' or 'd' to indicate its size; so, we need to
+ * skip this char if we met one.
+ */
+ char sfx = old_desc[offset];
+
+ if (sfx == 'b' || sfx == 'w' || sfx == 'd')
+ offset++;
+ }
+
+ if (offset < old_desc_len)
+ memcpy(new_desc + prefix_len + uprobe_len,
+ old_desc + offset, old_desc_len - offset);
+
+ free(old_desc);
+ *pdesc = new_desc;
+
+ return 0;
+}
diff --git a/tools/perf/util/perf_regs.c b/tools/perf/util/perf_regs.c
index c4023f2..a37e593 100644
--- a/tools/perf/util/perf_regs.c
+++ b/tools/perf/util/perf_regs.c
@@ -6,6 +6,12 @@ const struct sample_reg __weak sample_reg_masks[] = {
SMPL_REG_END
};
+int __weak sdt_rename_register(char **pdesc __maybe_unused,
+ char *old_name __maybe_unused)
+{
+ return 0;
+}
+
#ifdef HAVE_PERF_REGS_SUPPORT
int perf_reg_value(u64 *valp, struct regs_dump *regs, int id)
{
diff --git a/tools/perf/util/perf_regs.h b/tools/perf/util/perf_regs.h
index 679d6e4..7544a15 100644
--- a/tools/perf/util/perf_regs.h
+++ b/tools/perf/util/perf_regs.h
@@ -15,6 +15,12 @@ struct sample_reg {
extern const struct sample_reg sample_reg_masks[];
+/*
+ * The table sdt_reg_renamings is used for adjusting gcc/gas-generated
+ * registers before filling the uprobe tracer interface.
+ */
+int sdt_rename_register(char **pdesc, char *old_name);
+
#ifdef HAVE_PERF_REGS_SUPPORT
#include <perf_regs.h>
diff --git a/tools/perf/util/probe-file.c b/tools/perf/util/probe-file.c
index 436b647..d8a169e 100644
--- a/tools/perf/util/probe-file.c
+++ b/tools/perf/util/probe-file.c
@@ -27,6 +27,7 @@
#include "probe-event.h"
#include "probe-file.h"
#include "session.h"
+#include "perf_regs.h"
#define MAX_CMDLEN 256
@@ -687,6 +688,166 @@ static unsigned long long sdt_note__get_addr(struct sdt_note *note)
: (unsigned long long)note->addr.a64[0];
}
+static const char * const type_to_suffix[] = {
+ ":s64", "", "", "", ":s32", "", ":s16", ":s8",
+ "", ":u8", ":u16", "", ":u32", "", "", "", ":u64"
+};
+
+static int synthesize_sdt_probe_arg(struct strbuf *buf, int i, const char *arg)
+{
+ char *tmp, *desc = strdup(arg);
+ const char *prefix = "", *suffix = "";
+ int ret = -1;
+
+ if (desc == NULL) {
+ pr_debug4("Allocation error\n");
+ return ret;
+ }
+
+ tmp = strchr(desc, '@');
+ if (tmp) {
+ long type_idx;
+ /*
+ * Isolate the string number and convert it into a
+ * binary value; this will be an index to get suffix
+ * of the uprobe name (defining the type)
+ */
+ tmp[0] = '\0';
+ type_idx = strtol(desc, NULL, 10);
+ /* Check that the conversion went OK */
+ if (type_idx == LONG_MIN || type_idx == LONG_MAX) {
+ pr_debug4("Failed to parse sdt type\n");
+ goto error;
+ }
+ /* Check that the converted value is OK */
+ if (type_idx < -8 || type_idx > 8) {
+ pr_debug4("Failed to get a valid sdt type\n");
+ goto error;
+ }
+ suffix = type_to_suffix[type_idx + 8];
+ /* Get rid of the sdt prefix which is now useless */
+ tmp++;
+ memmove(desc, tmp, strlen(tmp) + 1);
+ }
+
+ /*
+ * The uprobe tracer format does not support all the
+ * addressing modes (notably: in x86 the scaled mode); so, we
+ * detect ',' characters, if there is just one, there is no
+ * use converting the sdt arg into a uprobe one.
+ */
+ if (strchr(desc, ',')) {
+ pr_debug4("Skipping unsupported SDT argument; %s\n", desc);
+ goto out;
+ }
+
+ /*
+ * If the argument addressing mode is indirect, we must check
+ * a few things...
+ */
+ tmp = strchr(desc, '(');
+ if (tmp) {
+ int j;
+
+ /*
+ * ...if the addressing mode is indirect with a
+ * positive offset (ex.: "1608(%ax)"), we need to add
+ * a '+' prefix so as to be compliant with uprobe
+ * format.
+ */
+ if (desc[0] != '+' && desc[0] != '-')
+ prefix = "+";
+
+ /*
+ * ...or if the addressing mode is indirect with a symbol
+ * as offset, the argument will not be supported by
+ * the uprobe tracer format; so, let's skip this one.
+ */
+ for (j = 0; j < tmp - desc; j++) {
+ if (desc[j] != '+' && desc[j] != '-' &&
+ !isdigit(desc[j])) {
+ pr_debug4("Skipping unsupported SDT argument; "
+ "%s\n", desc);
+ goto out;
+ }
+ }
+ }
+
+ /*
+ * The uprobe tracer format does not support constants; if we
+ * find one in the current argument, let's skip the argument.
+ */
+ if (strchr(desc, '$')) {
+ pr_debug4("Skipping unsupported SDT argument; %s\n", desc);
+ goto out;
+ }
+
+ /*
+ * The uprobe parser does not support all gas register names;
+ * so, we have to replace them (ex. for x86_64: %rax -> %ax);
+ * the loop below looks for the register names (starting with
+ * a '%' and tries to perform the needed renamings.
+ */
+ tmp = strchr(desc, '%');
+ while (tmp) {
+ size_t offset = tmp - desc;
+
+ ret = sdt_rename_register(&desc, desc + offset);
+ if (ret < 0)
+ goto error;
+
+ /*
+ * The desc pointer might have changed; so, let's not
+ * try to reuse tmp for next lookup
+ */
+ tmp = strchr(desc + offset + 1, '%');
+ }
+
+ if (strbuf_addf(buf, " arg%d=%s%s%s", i + 1, prefix, desc, suffix) < 0)
+ goto error;
+
+out:
+ ret = 0;
+error:
+ free(desc);
+ return ret;
+}
+
+static char *synthesize_sdt_probe_command(struct sdt_note *note,
+ const char *pathname,
+ const char *sdtgrp)
+{
+ struct strbuf buf;
+ char *ret = NULL, **args;
+ int i, args_count;
+
+ if (strbuf_init(&buf, 32) < 0)
+ return NULL;
+
+ if (strbuf_addf(&buf, "p:%s/%s %s:0x%llx",
+ sdtgrp, note->name, pathname,
+ sdt_note__get_addr(note)) < 0)
+ goto error;
+
+ if (!note->args)
+ goto out;
+
+ if (note->args) {
+ args = argv_split(note->args, &args_count);
+
+ for (i = 0; i < args_count; ++i) {
+ if (synthesize_sdt_probe_arg(&buf, i, args[i]) < 0)
+ goto error;
+ }
+ }
+
+out:
+ ret = strbuf_detach(&buf, NULL);
+error:
+ strbuf_release(&buf);
+ return ret;
+}
+
int probe_cache__scan_sdt(struct probe_cache *pcache, const char *pathname)
{
struct probe_cache_entry *entry = NULL;
@@ -723,11 +884,12 @@ int probe_cache__scan_sdt(struct probe_cache *pcache, const char *pathname)
entry->pev.group = strdup(sdtgrp);
list_add_tail(&entry->node, &pcache->entries);
}
- ret = asprintf(&buf, "p:%s/%s %s:0x%llx",
- sdtgrp, note->name, pathname,
- sdt_note__get_addr(note));
- if (ret < 0)
+ buf = synthesize_sdt_probe_command(note, pathname, sdtgrp);
+ if (!buf) {
+ ret = -ENOMEM;
break;
+ }
+
strlist__add(entry->tevlist, buf);
free(buf);
entry = NULL;
--
2.10.2
During a "perf buildid-cache --add" command, the section
".note.stapsdt" of the "added" binary is scanned in order to list the
available SDT markers available in a binary. The parts containing the
probes arguments were left unscanned.
The whole section is now parsed; the probe arguments are extracted for
later use.
Signed-off-by: Alexis Berlemont <[email protected]>
---
tools/perf/util/symbol-elf.c | 25 +++++++++++++++++++++++--
tools/perf/util/symbol.h | 1 +
2 files changed, 24 insertions(+), 2 deletions(-)
diff --git a/tools/perf/util/symbol-elf.c b/tools/perf/util/symbol-elf.c
index 99400b0..7725c3f 100644
--- a/tools/perf/util/symbol-elf.c
+++ b/tools/perf/util/symbol-elf.c
@@ -1822,7 +1822,7 @@ void kcore_extract__delete(struct kcore_extract *kce)
static int populate_sdt_note(Elf **elf, const char *data, size_t len,
struct list_head *sdt_notes)
{
- const char *provider, *name;
+ const char *provider, *name, *args;
struct sdt_note *tmp = NULL;
GElf_Ehdr ehdr;
GElf_Addr base_off = 0;
@@ -1881,6 +1881,25 @@ static int populate_sdt_note(Elf **elf, const char *data, size_t len,
goto out_free_prov;
}
+ args = memchr(name, '\0', data + len - name);
+
+ /*
+ * There is no argument if:
+ * - We reached the end of the note;
+ * - There is not enough room to hold a potential string;
+ * - The argument string is empty or just contains ':'.
+ */
+ if (args == NULL || data + len - args < 2 ||
+ args[1] == ':' || args[1] == '\0')
+ tmp->args = NULL;
+ else {
+ tmp->args = strdup(++args);
+ if (!tmp->args) {
+ ret = -ENOMEM;
+ goto out_free_name;
+ }
+ }
+
if (gelf_getclass(*elf) == ELFCLASS32) {
memcpy(&tmp->addr, &buf, 3 * sizeof(Elf32_Addr));
tmp->bit32 = true;
@@ -1892,7 +1911,7 @@ static int populate_sdt_note(Elf **elf, const char *data, size_t len,
if (!gelf_getehdr(*elf, &ehdr)) {
pr_debug("%s : cannot get elf header.\n", __func__);
ret = -EBADF;
- goto out_free_name;
+ goto out_free_args;
}
/* Adjust the prelink effect :
@@ -1917,6 +1936,8 @@ static int populate_sdt_note(Elf **elf, const char *data, size_t len,
list_add_tail(&tmp->note_list, sdt_notes);
return 0;
+out_free_args:
+ free(tmp->args);
out_free_name:
free(tmp->name);
out_free_prov:
diff --git a/tools/perf/util/symbol.h b/tools/perf/util/symbol.h
index 6c358b7..9222c7e 100644
--- a/tools/perf/util/symbol.h
+++ b/tools/perf/util/symbol.h
@@ -351,6 +351,7 @@ int arch__choose_best_symbol(struct symbol *syma, struct symbol *symb);
struct sdt_note {
char *name; /* name of the note*/
char *provider; /* provider name */
+ char *args;
bool bit32; /* whether the location is 32 bits? */
union { /* location, base and semaphore addrs */
Elf64_Addr a64[3];
--
2.10.2
* Alexis Berlemont <[email protected]> wrote:
> Hi Masami,
>
> Many thanks for your mail.
>
> Here is another patch set which tries to fix the points you mentioned:
>
> * Skip the arguments containing a constant ($123);
> * Review the code in charge of the register renaming (search for '%'
> and parse it);
> * Minor changes (print the argument in case of error, skipping, check
> the sdt arg type index);
>
> Many thanks,
>
> Alexis.
>
> Alexis Berlemont (2):
> perf sdt: add scanning of sdt probles arguments
> perf probe: add sdt probes arguments into the uprobe cmd string
I'd like to hijack this thread to report an SDT oddity - one of my boxen reports
lots of SDT tracepoints in 'perf list':
mem:<addr>[/len][:access] [Hardware breakpoint]
sdt_libc:lll_lock_wait_private [SDT event]
sdt_libc:longjmp [SDT event]
sdt_libc:longjmp_target [SDT event]
sdt_libc:memory_arena_new [SDT event]
sdt_libc:memory_arena_retry [SDT event]
sdt_libc:memory_arena_reuse [SDT event]
sdt_libc:memory_arena_reuse_free_list [SDT event]
sdt_libc:memory_arena_reuse_wait [SDT event]
sdt_libc:memory_calloc_retry [SDT event]
sdt_libc:memory_heap_free [SDT event]
...
But none of them work:
Error: No permissions to read /sys/kernel/debug/tracing/events/sdt_libc/longjmp
Hint: Try 'sudo mount -o remount,mode=755 /sys/kernel/debug/tracing'
...
Error: File /sys/kernel/debug/tracing/events/sdt_libc/longjmp not found.
Hint: Perhaps this kernel misses some CONFIG_ setting to enable this feature?.
What kind of patches are required for SDT probes to work?
Thanks,
Ingo
On Wed, 14 Dec 2016 01:07:31 +0100
Alexis Berlemont <[email protected]> wrote:
> During a "perf buildid-cache --add" command, the section
> ".note.stapsdt" of the "added" binary is scanned in order to list the
> available SDT markers available in a binary. The parts containing the
> probes arguments were left unscanned.
>
> The whole section is now parsed; the probe arguments are extracted for
> later use.
>
Looks good to me.
Acked-by: Masami Hiramatsu <[email protected]>
Thanks!
> Signed-off-by: Alexis Berlemont <[email protected]>
> ---
> tools/perf/util/symbol-elf.c | 25 +++++++++++++++++++++++--
> tools/perf/util/symbol.h | 1 +
> 2 files changed, 24 insertions(+), 2 deletions(-)
>
> diff --git a/tools/perf/util/symbol-elf.c b/tools/perf/util/symbol-elf.c
> index 99400b0..7725c3f 100644
> --- a/tools/perf/util/symbol-elf.c
> +++ b/tools/perf/util/symbol-elf.c
> @@ -1822,7 +1822,7 @@ void kcore_extract__delete(struct kcore_extract *kce)
> static int populate_sdt_note(Elf **elf, const char *data, size_t len,
> struct list_head *sdt_notes)
> {
> - const char *provider, *name;
> + const char *provider, *name, *args;
> struct sdt_note *tmp = NULL;
> GElf_Ehdr ehdr;
> GElf_Addr base_off = 0;
> @@ -1881,6 +1881,25 @@ static int populate_sdt_note(Elf **elf, const char *data, size_t len,
> goto out_free_prov;
> }
>
> + args = memchr(name, '\0', data + len - name);
> +
> + /*
> + * There is no argument if:
> + * - We reached the end of the note;
> + * - There is not enough room to hold a potential string;
> + * - The argument string is empty or just contains ':'.
> + */
> + if (args == NULL || data + len - args < 2 ||
> + args[1] == ':' || args[1] == '\0')
> + tmp->args = NULL;
> + else {
> + tmp->args = strdup(++args);
> + if (!tmp->args) {
> + ret = -ENOMEM;
> + goto out_free_name;
> + }
> + }
> +
> if (gelf_getclass(*elf) == ELFCLASS32) {
> memcpy(&tmp->addr, &buf, 3 * sizeof(Elf32_Addr));
> tmp->bit32 = true;
> @@ -1892,7 +1911,7 @@ static int populate_sdt_note(Elf **elf, const char *data, size_t len,
> if (!gelf_getehdr(*elf, &ehdr)) {
> pr_debug("%s : cannot get elf header.\n", __func__);
> ret = -EBADF;
> - goto out_free_name;
> + goto out_free_args;
> }
>
> /* Adjust the prelink effect :
> @@ -1917,6 +1936,8 @@ static int populate_sdt_note(Elf **elf, const char *data, size_t len,
> list_add_tail(&tmp->note_list, sdt_notes);
> return 0;
>
> +out_free_args:
> + free(tmp->args);
> out_free_name:
> free(tmp->name);
> out_free_prov:
> diff --git a/tools/perf/util/symbol.h b/tools/perf/util/symbol.h
> index 6c358b7..9222c7e 100644
> --- a/tools/perf/util/symbol.h
> +++ b/tools/perf/util/symbol.h
> @@ -351,6 +351,7 @@ int arch__choose_best_symbol(struct symbol *syma, struct symbol *symb);
> struct sdt_note {
> char *name; /* name of the note*/
> char *provider; /* provider name */
> + char *args;
> bool bit32; /* whether the location is 32 bits? */
> union { /* location, base and semaphore addrs */
> Elf64_Addr a64[3];
> --
> 2.10.2
>
--
Masami Hiramatsu <[email protected]>
On Wed, 14 Dec 2016 01:07:32 +0100
Alexis Berlemont <[email protected]> wrote:
> An sdt probe can be associated with arguments but they were not passed
> to the user probe tracing interface (uprobe_events); this patch adapts
> the sdt argument descriptors according to the uprobe input format.
>
> As the uprobe parser does not support scaled address mode, perf will
> skip arguments which cannot be adapted to the uprobe format.
>
> Here are the results:
>
> $ perf buildid-cache -v --add test_sdt
> $ perf probe -x test_sdt sdt_libfoo:table_frob
> $ perf probe -x test_sdt sdt_libfoo:table_diddle
> $ perf record -e sdt_libfoo:table_frob -e sdt_libfoo:table_diddle test_sdt
> $ perf script
> test_sdt ... 666.255678: sdt_libfoo:table_frob: (4004d7) arg0=0 arg1=0
> test_sdt ... 666.255683: sdt_libfoo:table_diddle: (40051a) arg0=0 arg1=0
> test_sdt ... 666.255686: sdt_libfoo:table_frob: (4004d7) arg0=1 arg1=2
> test_sdt ... 666.255689: sdt_libfoo:table_diddle: (40051a) arg0=3 arg1=4
> test_sdt ... 666.255692: sdt_libfoo:table_frob: (4004d7) arg0=2 arg1=4
> test_sdt ... 666.255694: sdt_libfoo:table_diddle: (40051a) arg0=6 arg1=8
>
OK, this looks good to me, good job!
Acked-by: Masami Hiramatsu <[email protected]>
Thanks!
> Signed-off-by: Alexis Berlemont <[email protected]>
> ---
> tools/perf/arch/x86/util/perf_regs.c | 83 +++++++++++++++++
> tools/perf/util/perf_regs.c | 6 ++
> tools/perf/util/perf_regs.h | 6 ++
> tools/perf/util/probe-file.c | 170 ++++++++++++++++++++++++++++++++++-
> 4 files changed, 261 insertions(+), 4 deletions(-)
>
> diff --git a/tools/perf/arch/x86/util/perf_regs.c b/tools/perf/arch/x86/util/perf_regs.c
> index c5db14f..09a7f55 100644
> --- a/tools/perf/arch/x86/util/perf_regs.c
> +++ b/tools/perf/arch/x86/util/perf_regs.c
> @@ -1,4 +1,7 @@
> +#include <string.h>
> +
> #include "../../perf.h"
> +#include "../../util/util.h"
> #include "../../util/perf_regs.h"
>
> const struct sample_reg sample_reg_masks[] = {
> @@ -26,3 +29,83 @@ const struct sample_reg sample_reg_masks[] = {
> #endif
> SMPL_REG_END
> };
> +
> +struct sdt_name_reg {
> + const char *sdt_name;
> + const char *uprobe_name;
> +};
> +#define SDT_NAME_REG(n, m) {.sdt_name = "%" #n, .uprobe_name = "%" #m}
> +#define SDT_NAME_REG_END {.sdt_name = NULL, .uprobe_name = NULL}
> +
> +static const struct sdt_name_reg sdt_reg_renamings[] = {
> + SDT_NAME_REG(eax, ax),
> + SDT_NAME_REG(rax, ax),
> + SDT_NAME_REG(ebx, bx),
> + SDT_NAME_REG(rbx, bx),
> + SDT_NAME_REG(ecx, cx),
> + SDT_NAME_REG(rcx, cx),
> + SDT_NAME_REG(edx, dx),
> + SDT_NAME_REG(rdx, dx),
> + SDT_NAME_REG(esi, si),
> + SDT_NAME_REG(rsi, si),
> + SDT_NAME_REG(edi, di),
> + SDT_NAME_REG(rdi, di),
> + SDT_NAME_REG(ebp, bp),
> + SDT_NAME_REG(rbp, bp),
> + SDT_NAME_REG_END,
> +};
> +
> +int sdt_rename_register(char **pdesc, char *old_name)
> +{
> + const struct sdt_name_reg *rnames = sdt_reg_renamings;
> + char *new_desc, *old_desc = *pdesc;
> + size_t prefix_len, sdt_len, uprobe_len, old_desc_len, offset;
> + int ret = -1;
> +
> + while (ret != 0 && rnames->sdt_name != NULL) {
> + sdt_len = strlen(rnames->sdt_name);
> + ret = strncmp(old_name, rnames->sdt_name, sdt_len);
> + rnames += !!ret;
> + }
> +
> + if (rnames->sdt_name == NULL)
> + return 0;
> +
> + sdt_len = strlen(rnames->sdt_name);
> + uprobe_len = strlen(rnames->uprobe_name);
> + old_desc_len = strlen(old_desc) + 1;
> +
> + new_desc = zalloc(old_desc_len + uprobe_len - sdt_len);
> + if (new_desc == NULL)
> + return -1;
> +
> + /* Copy the chars before the register name (at least '%') */
> + prefix_len = old_name - old_desc;
> + memcpy(new_desc, old_desc, prefix_len);
> +
> + /* Copy the new register name */
> + memcpy(new_desc + prefix_len, rnames->uprobe_name, uprobe_len);
> +
> + /* Copy the chars after the register name (if need be) */
> + offset = prefix_len + sdt_len;
> + if (offset < old_desc_len) {
> + /*
> + * The orginal register name can be suffixed by 'b',
> + * 'w' or 'd' to indicate its size; so, we need to
> + * skip this char if we met one.
> + */
> + char sfx = old_desc[offset];
> +
> + if (sfx == 'b' || sfx == 'w' || sfx == 'd')
> + offset++;
> + }
> +
> + if (offset < old_desc_len)
> + memcpy(new_desc + prefix_len + uprobe_len,
> + old_desc + offset, old_desc_len - offset);
> +
> + free(old_desc);
> + *pdesc = new_desc;
> +
> + return 0;
> +}
> diff --git a/tools/perf/util/perf_regs.c b/tools/perf/util/perf_regs.c
> index c4023f2..a37e593 100644
> --- a/tools/perf/util/perf_regs.c
> +++ b/tools/perf/util/perf_regs.c
> @@ -6,6 +6,12 @@ const struct sample_reg __weak sample_reg_masks[] = {
> SMPL_REG_END
> };
>
> +int __weak sdt_rename_register(char **pdesc __maybe_unused,
> + char *old_name __maybe_unused)
> +{
> + return 0;
> +}
> +
> #ifdef HAVE_PERF_REGS_SUPPORT
> int perf_reg_value(u64 *valp, struct regs_dump *regs, int id)
> {
> diff --git a/tools/perf/util/perf_regs.h b/tools/perf/util/perf_regs.h
> index 679d6e4..7544a15 100644
> --- a/tools/perf/util/perf_regs.h
> +++ b/tools/perf/util/perf_regs.h
> @@ -15,6 +15,12 @@ struct sample_reg {
>
> extern const struct sample_reg sample_reg_masks[];
>
> +/*
> + * The table sdt_reg_renamings is used for adjusting gcc/gas-generated
> + * registers before filling the uprobe tracer interface.
> + */
> +int sdt_rename_register(char **pdesc, char *old_name);
> +
> #ifdef HAVE_PERF_REGS_SUPPORT
> #include <perf_regs.h>
>
> diff --git a/tools/perf/util/probe-file.c b/tools/perf/util/probe-file.c
> index 436b647..d8a169e 100644
> --- a/tools/perf/util/probe-file.c
> +++ b/tools/perf/util/probe-file.c
> @@ -27,6 +27,7 @@
> #include "probe-event.h"
> #include "probe-file.h"
> #include "session.h"
> +#include "perf_regs.h"
>
> #define MAX_CMDLEN 256
>
> @@ -687,6 +688,166 @@ static unsigned long long sdt_note__get_addr(struct sdt_note *note)
> : (unsigned long long)note->addr.a64[0];
> }
>
> +static const char * const type_to_suffix[] = {
> + ":s64", "", "", "", ":s32", "", ":s16", ":s8",
> + "", ":u8", ":u16", "", ":u32", "", "", "", ":u64"
> +};
> +
> +static int synthesize_sdt_probe_arg(struct strbuf *buf, int i, const char *arg)
> +{
> + char *tmp, *desc = strdup(arg);
> + const char *prefix = "", *suffix = "";
> + int ret = -1;
> +
> + if (desc == NULL) {
> + pr_debug4("Allocation error\n");
> + return ret;
> + }
> +
> + tmp = strchr(desc, '@');
> + if (tmp) {
> + long type_idx;
> + /*
> + * Isolate the string number and convert it into a
> + * binary value; this will be an index to get suffix
> + * of the uprobe name (defining the type)
> + */
> + tmp[0] = '\0';
> + type_idx = strtol(desc, NULL, 10);
> + /* Check that the conversion went OK */
> + if (type_idx == LONG_MIN || type_idx == LONG_MAX) {
> + pr_debug4("Failed to parse sdt type\n");
> + goto error;
> + }
> + /* Check that the converted value is OK */
> + if (type_idx < -8 || type_idx > 8) {
> + pr_debug4("Failed to get a valid sdt type\n");
> + goto error;
> + }
> + suffix = type_to_suffix[type_idx + 8];
> + /* Get rid of the sdt prefix which is now useless */
> + tmp++;
> + memmove(desc, tmp, strlen(tmp) + 1);
> + }
> +
> + /*
> + * The uprobe tracer format does not support all the
> + * addressing modes (notably: in x86 the scaled mode); so, we
> + * detect ',' characters, if there is just one, there is no
> + * use converting the sdt arg into a uprobe one.
> + */
> + if (strchr(desc, ',')) {
> + pr_debug4("Skipping unsupported SDT argument; %s\n", desc);
> + goto out;
> + }
> +
> + /*
> + * If the argument addressing mode is indirect, we must check
> + * a few things...
> + */
> + tmp = strchr(desc, '(');
> + if (tmp) {
> + int j;
> +
> + /*
> + * ...if the addressing mode is indirect with a
> + * positive offset (ex.: "1608(%ax)"), we need to add
> + * a '+' prefix so as to be compliant with uprobe
> + * format.
> + */
> + if (desc[0] != '+' && desc[0] != '-')
> + prefix = "+";
> +
> + /*
> + * ...or if the addressing mode is indirect with a symbol
> + * as offset, the argument will not be supported by
> + * the uprobe tracer format; so, let's skip this one.
> + */
> + for (j = 0; j < tmp - desc; j++) {
> + if (desc[j] != '+' && desc[j] != '-' &&
> + !isdigit(desc[j])) {
> + pr_debug4("Skipping unsupported SDT argument; "
> + "%s\n", desc);
> + goto out;
> + }
> + }
> + }
> +
> + /*
> + * The uprobe tracer format does not support constants; if we
> + * find one in the current argument, let's skip the argument.
> + */
> + if (strchr(desc, '$')) {
> + pr_debug4("Skipping unsupported SDT argument; %s\n", desc);
> + goto out;
> + }
> +
> + /*
> + * The uprobe parser does not support all gas register names;
> + * so, we have to replace them (ex. for x86_64: %rax -> %ax);
> + * the loop below looks for the register names (starting with
> + * a '%' and tries to perform the needed renamings.
> + */
> + tmp = strchr(desc, '%');
> + while (tmp) {
> + size_t offset = tmp - desc;
> +
> + ret = sdt_rename_register(&desc, desc + offset);
> + if (ret < 0)
> + goto error;
> +
> + /*
> + * The desc pointer might have changed; so, let's not
> + * try to reuse tmp for next lookup
> + */
> + tmp = strchr(desc + offset + 1, '%');
> + }
> +
> + if (strbuf_addf(buf, " arg%d=%s%s%s", i + 1, prefix, desc, suffix) < 0)
> + goto error;
> +
> +out:
> + ret = 0;
> +error:
> + free(desc);
> + return ret;
> +}
> +
> +static char *synthesize_sdt_probe_command(struct sdt_note *note,
> + const char *pathname,
> + const char *sdtgrp)
> +{
> + struct strbuf buf;
> + char *ret = NULL, **args;
> + int i, args_count;
> +
> + if (strbuf_init(&buf, 32) < 0)
> + return NULL;
> +
> + if (strbuf_addf(&buf, "p:%s/%s %s:0x%llx",
> + sdtgrp, note->name, pathname,
> + sdt_note__get_addr(note)) < 0)
> + goto error;
> +
> + if (!note->args)
> + goto out;
> +
> + if (note->args) {
> + args = argv_split(note->args, &args_count);
> +
> + for (i = 0; i < args_count; ++i) {
> + if (synthesize_sdt_probe_arg(&buf, i, args[i]) < 0)
> + goto error;
> + }
> + }
> +
> +out:
> + ret = strbuf_detach(&buf, NULL);
> +error:
> + strbuf_release(&buf);
> + return ret;
> +}
> +
> int probe_cache__scan_sdt(struct probe_cache *pcache, const char *pathname)
> {
> struct probe_cache_entry *entry = NULL;
> @@ -723,11 +884,12 @@ int probe_cache__scan_sdt(struct probe_cache *pcache, const char *pathname)
> entry->pev.group = strdup(sdtgrp);
> list_add_tail(&entry->node, &pcache->entries);
> }
> - ret = asprintf(&buf, "p:%s/%s %s:0x%llx",
> - sdtgrp, note->name, pathname,
> - sdt_note__get_addr(note));
> - if (ret < 0)
> + buf = synthesize_sdt_probe_command(note, pathname, sdtgrp);
> + if (!buf) {
> + ret = -ENOMEM;
> break;
> + }
> +
> strlist__add(entry->tevlist, buf);
> free(buf);
> entry = NULL;
> --
> 2.10.2
>
--
Masami Hiramatsu <[email protected]>
Hello Alexis and Arnaldo,
I think this series is now good to go.
Could you pick these?
Acked-by: Masami Hiramatsu <[email protected]>
for this series.
Thank you,
On Wed, 14 Dec 2016 01:07:30 +0100
Alexis Berlemont <[email protected]> wrote:
> Hi Masami,
>
> Many thanks for your mail.
>
> Here is another patch set which tries to fix the points you mentioned:
>
> * Skip the arguments containing a constant ($123);
> * Review the code in charge of the register renaming (search for '%'
> and parse it);
> * Minor changes (print the argument in case of error, skipping, check
> the sdt arg type index);
>
> Many thanks,
>
> Alexis.
>
> Alexis Berlemont (2):
> perf sdt: add scanning of sdt probles arguments
> perf probe: add sdt probes arguments into the uprobe cmd string
>
> tools/perf/arch/x86/util/perf_regs.c | 83 +++++++++++++++++
> tools/perf/util/perf_regs.c | 6 ++
> tools/perf/util/perf_regs.h | 6 ++
> tools/perf/util/probe-file.c | 170 ++++++++++++++++++++++++++++++++++-
> tools/perf/util/symbol-elf.c | 25 +++++-
> tools/perf/util/symbol.h | 1 +
> 6 files changed, 285 insertions(+), 6 deletions(-)
>
> --
> 2.10.2
>
--
Masami Hiramatsu <[email protected]>
Em Mon, Mar 06, 2017 at 02:39:15PM +0100, Masami Hiramatsu escreveu:
> On Wed, 14 Dec 2016 01:07:31 +0100
> Alexis Berlemont <[email protected]> wrote:
>
> > During a "perf buildid-cache --add" command, the section
> > ".note.stapsdt" of the "added" binary is scanned in order to list the
> > available SDT markers available in a binary. The parts containing the
> > probes arguments were left unscanned.
> >
> > The whole section is now parsed; the probe arguments are extracted for
> > later use.
> >
>
> Looks good to me.
>
> Acked-by: Masami Hiramatsu <[email protected]>
Thanks, applied.
> Thanks!
>
> > Signed-off-by: Alexis Berlemont <[email protected]>
> > ---
> > tools/perf/util/symbol-elf.c | 25 +++++++++++++++++++++++--
> > tools/perf/util/symbol.h | 1 +
> > 2 files changed, 24 insertions(+), 2 deletions(-)
> >
> > diff --git a/tools/perf/util/symbol-elf.c b/tools/perf/util/symbol-elf.c
> > index 99400b0..7725c3f 100644
> > --- a/tools/perf/util/symbol-elf.c
> > +++ b/tools/perf/util/symbol-elf.c
> > @@ -1822,7 +1822,7 @@ void kcore_extract__delete(struct kcore_extract *kce)
> > static int populate_sdt_note(Elf **elf, const char *data, size_t len,
> > struct list_head *sdt_notes)
> > {
> > - const char *provider, *name;
> > + const char *provider, *name, *args;
> > struct sdt_note *tmp = NULL;
> > GElf_Ehdr ehdr;
> > GElf_Addr base_off = 0;
> > @@ -1881,6 +1881,25 @@ static int populate_sdt_note(Elf **elf, const char *data, size_t len,
> > goto out_free_prov;
> > }
> >
> > + args = memchr(name, '\0', data + len - name);
> > +
> > + /*
> > + * There is no argument if:
> > + * - We reached the end of the note;
> > + * - There is not enough room to hold a potential string;
> > + * - The argument string is empty or just contains ':'.
> > + */
> > + if (args == NULL || data + len - args < 2 ||
> > + args[1] == ':' || args[1] == '\0')
> > + tmp->args = NULL;
> > + else {
> > + tmp->args = strdup(++args);
> > + if (!tmp->args) {
> > + ret = -ENOMEM;
> > + goto out_free_name;
> > + }
> > + }
> > +
> > if (gelf_getclass(*elf) == ELFCLASS32) {
> > memcpy(&tmp->addr, &buf, 3 * sizeof(Elf32_Addr));
> > tmp->bit32 = true;
> > @@ -1892,7 +1911,7 @@ static int populate_sdt_note(Elf **elf, const char *data, size_t len,
> > if (!gelf_getehdr(*elf, &ehdr)) {
> > pr_debug("%s : cannot get elf header.\n", __func__);
> > ret = -EBADF;
> > - goto out_free_name;
> > + goto out_free_args;
> > }
> >
> > /* Adjust the prelink effect :
> > @@ -1917,6 +1936,8 @@ static int populate_sdt_note(Elf **elf, const char *data, size_t len,
> > list_add_tail(&tmp->note_list, sdt_notes);
> > return 0;
> >
> > +out_free_args:
> > + free(tmp->args);
> > out_free_name:
> > free(tmp->name);
> > out_free_prov:
> > diff --git a/tools/perf/util/symbol.h b/tools/perf/util/symbol.h
> > index 6c358b7..9222c7e 100644
> > --- a/tools/perf/util/symbol.h
> > +++ b/tools/perf/util/symbol.h
> > @@ -351,6 +351,7 @@ int arch__choose_best_symbol(struct symbol *syma, struct symbol *symb);
> > struct sdt_note {
> > char *name; /* name of the note*/
> > char *provider; /* provider name */
> > + char *args;
> > bool bit32; /* whether the location is 32 bits? */
> > union { /* location, base and semaphore addrs */
> > Elf64_Addr a64[3];
> > --
> > 2.10.2
> >
>
>
> --
> Masami Hiramatsu <[email protected]>
Commit-ID: be88184b1c7054719296387c6063748fb48fa645
Gitweb: http://git.kernel.org/tip/be88184b1c7054719296387c6063748fb48fa645
Author: Alexis Berlemont <[email protected]>
AuthorDate: Wed, 14 Dec 2016 01:07:31 +0100
Committer: Arnaldo Carvalho de Melo <[email protected]>
CommitDate: Tue, 21 Mar 2017 10:56:28 -0300
perf sdt: Add scanning of sdt probes arguments
During a "perf buildid-cache --add" command, the section ".note.stapsdt"
of the "added" binary is scanned in order to list the available SDT
markers available in a binary. The parts containing the probes arguments
were left unscanned.
The whole section is now parsed; the probe arguments are extracted for
later use.
Signed-off-by: Alexis Berlemont <[email protected]>
Acked-by: Masami Hiramatsu <[email protected]>
Cc: Alexander Shishkin <[email protected]>
Cc: Hemant Kumar <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Ravi Bangoria <[email protected]>
Link: http://lkml.kernel.org/r/[email protected]
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
---
tools/perf/util/symbol-elf.c | 25 +++++++++++++++++++++++--
tools/perf/util/symbol.h | 1 +
2 files changed, 24 insertions(+), 2 deletions(-)
diff --git a/tools/perf/util/symbol-elf.c b/tools/perf/util/symbol-elf.c
index 4e59dde..0e660db 100644
--- a/tools/perf/util/symbol-elf.c
+++ b/tools/perf/util/symbol-elf.c
@@ -1828,7 +1828,7 @@ void kcore_extract__delete(struct kcore_extract *kce)
static int populate_sdt_note(Elf **elf, const char *data, size_t len,
struct list_head *sdt_notes)
{
- const char *provider, *name;
+ const char *provider, *name, *args;
struct sdt_note *tmp = NULL;
GElf_Ehdr ehdr;
GElf_Addr base_off = 0;
@@ -1887,6 +1887,25 @@ static int populate_sdt_note(Elf **elf, const char *data, size_t len,
goto out_free_prov;
}
+ args = memchr(name, '\0', data + len - name);
+
+ /*
+ * There is no argument if:
+ * - We reached the end of the note;
+ * - There is not enough room to hold a potential string;
+ * - The argument string is empty or just contains ':'.
+ */
+ if (args == NULL || data + len - args < 2 ||
+ args[1] == ':' || args[1] == '\0')
+ tmp->args = NULL;
+ else {
+ tmp->args = strdup(++args);
+ if (!tmp->args) {
+ ret = -ENOMEM;
+ goto out_free_name;
+ }
+ }
+
if (gelf_getclass(*elf) == ELFCLASS32) {
memcpy(&tmp->addr, &buf, 3 * sizeof(Elf32_Addr));
tmp->bit32 = true;
@@ -1898,7 +1917,7 @@ static int populate_sdt_note(Elf **elf, const char *data, size_t len,
if (!gelf_getehdr(*elf, &ehdr)) {
pr_debug("%s : cannot get elf header.\n", __func__);
ret = -EBADF;
- goto out_free_name;
+ goto out_free_args;
}
/* Adjust the prelink effect :
@@ -1923,6 +1942,8 @@ static int populate_sdt_note(Elf **elf, const char *data, size_t len,
list_add_tail(&tmp->note_list, sdt_notes);
return 0;
+out_free_args:
+ free(tmp->args);
out_free_name:
free(tmp->name);
out_free_prov:
diff --git a/tools/perf/util/symbol.h b/tools/perf/util/symbol.h
index 6c358b7..9222c7e 100644
--- a/tools/perf/util/symbol.h
+++ b/tools/perf/util/symbol.h
@@ -351,6 +351,7 @@ int arch__choose_best_symbol(struct symbol *syma, struct symbol *symb);
struct sdt_note {
char *name; /* name of the note*/
char *provider; /* provider name */
+ char *args;
bool bit32; /* whether the location is 32 bits? */
union { /* location, base and semaphore addrs */
Elf64_Addr a64[3];
Commit-ID: 3b1f8311f6963cd11a7d1efbcd2fd900d472ba5c
Gitweb: http://git.kernel.org/tip/3b1f8311f6963cd11a7d1efbcd2fd900d472ba5c
Author: Alexis Berlemont <[email protected]>
AuthorDate: Wed, 14 Dec 2016 01:07:32 +0100
Committer: Arnaldo Carvalho de Melo <[email protected]>
CommitDate: Tue, 21 Mar 2017 10:59:01 -0300
perf probe: Add sdt probes arguments into the uprobe cmd string
An sdt probe can be associated with arguments but they were not passed
to the user probe tracing interface (uprobe_events); this patch adapts
the sdt argument descriptors according to the uprobe input format.
As the uprobe parser does not support scaled address mode, perf will
skip arguments which cannot be adapted to the uprobe format.
Here are the results:
$ perf buildid-cache -v --add test_sdt
$ perf probe -x test_sdt sdt_libfoo:table_frob
$ perf probe -x test_sdt sdt_libfoo:table_diddle
$ perf record -e sdt_libfoo:table_frob -e sdt_libfoo:table_diddle test_sdt
$ perf script
test_sdt ... 666.255678: sdt_libfoo:table_frob: (4004d7) arg0=0 arg1=0
test_sdt ... 666.255683: sdt_libfoo:table_diddle: (40051a) arg0=0 arg1=0
test_sdt ... 666.255686: sdt_libfoo:table_frob: (4004d7) arg0=1 arg1=2
test_sdt ... 666.255689: sdt_libfoo:table_diddle: (40051a) arg0=3 arg1=4
test_sdt ... 666.255692: sdt_libfoo:table_frob: (4004d7) arg0=2 arg1=4
test_sdt ... 666.255694: sdt_libfoo:table_diddle: (40051a) arg0=6 arg1=8
Signed-off-by: Alexis Berlemont <[email protected]>
Acked-by: Masami Hiramatsu <[email protected]>
Cc: Alexander Shishkin <[email protected]>
Cc: Hemant Kumar <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Ravi Bangoria <[email protected]>
Link: http://lkml.kernel.org/r/[email protected]
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
---
tools/perf/arch/x86/util/perf_regs.c | 83 +++++++++++++++++
tools/perf/util/perf_regs.c | 6 ++
tools/perf/util/perf_regs.h | 6 ++
tools/perf/util/probe-file.c | 170 ++++++++++++++++++++++++++++++++++-
4 files changed, 261 insertions(+), 4 deletions(-)
diff --git a/tools/perf/arch/x86/util/perf_regs.c b/tools/perf/arch/x86/util/perf_regs.c
index c5db14f..09a7f55 100644
--- a/tools/perf/arch/x86/util/perf_regs.c
+++ b/tools/perf/arch/x86/util/perf_regs.c
@@ -1,4 +1,7 @@
+#include <string.h>
+
#include "../../perf.h"
+#include "../../util/util.h"
#include "../../util/perf_regs.h"
const struct sample_reg sample_reg_masks[] = {
@@ -26,3 +29,83 @@ const struct sample_reg sample_reg_masks[] = {
#endif
SMPL_REG_END
};
+
+struct sdt_name_reg {
+ const char *sdt_name;
+ const char *uprobe_name;
+};
+#define SDT_NAME_REG(n, m) {.sdt_name = "%" #n, .uprobe_name = "%" #m}
+#define SDT_NAME_REG_END {.sdt_name = NULL, .uprobe_name = NULL}
+
+static const struct sdt_name_reg sdt_reg_renamings[] = {
+ SDT_NAME_REG(eax, ax),
+ SDT_NAME_REG(rax, ax),
+ SDT_NAME_REG(ebx, bx),
+ SDT_NAME_REG(rbx, bx),
+ SDT_NAME_REG(ecx, cx),
+ SDT_NAME_REG(rcx, cx),
+ SDT_NAME_REG(edx, dx),
+ SDT_NAME_REG(rdx, dx),
+ SDT_NAME_REG(esi, si),
+ SDT_NAME_REG(rsi, si),
+ SDT_NAME_REG(edi, di),
+ SDT_NAME_REG(rdi, di),
+ SDT_NAME_REG(ebp, bp),
+ SDT_NAME_REG(rbp, bp),
+ SDT_NAME_REG_END,
+};
+
+int sdt_rename_register(char **pdesc, char *old_name)
+{
+ const struct sdt_name_reg *rnames = sdt_reg_renamings;
+ char *new_desc, *old_desc = *pdesc;
+ size_t prefix_len, sdt_len, uprobe_len, old_desc_len, offset;
+ int ret = -1;
+
+ while (ret != 0 && rnames->sdt_name != NULL) {
+ sdt_len = strlen(rnames->sdt_name);
+ ret = strncmp(old_name, rnames->sdt_name, sdt_len);
+ rnames += !!ret;
+ }
+
+ if (rnames->sdt_name == NULL)
+ return 0;
+
+ sdt_len = strlen(rnames->sdt_name);
+ uprobe_len = strlen(rnames->uprobe_name);
+ old_desc_len = strlen(old_desc) + 1;
+
+ new_desc = zalloc(old_desc_len + uprobe_len - sdt_len);
+ if (new_desc == NULL)
+ return -1;
+
+ /* Copy the chars before the register name (at least '%') */
+ prefix_len = old_name - old_desc;
+ memcpy(new_desc, old_desc, prefix_len);
+
+ /* Copy the new register name */
+ memcpy(new_desc + prefix_len, rnames->uprobe_name, uprobe_len);
+
+ /* Copy the chars after the register name (if need be) */
+ offset = prefix_len + sdt_len;
+ if (offset < old_desc_len) {
+ /*
+ * The orginal register name can be suffixed by 'b',
+ * 'w' or 'd' to indicate its size; so, we need to
+ * skip this char if we met one.
+ */
+ char sfx = old_desc[offset];
+
+ if (sfx == 'b' || sfx == 'w' || sfx == 'd')
+ offset++;
+ }
+
+ if (offset < old_desc_len)
+ memcpy(new_desc + prefix_len + uprobe_len,
+ old_desc + offset, old_desc_len - offset);
+
+ free(old_desc);
+ *pdesc = new_desc;
+
+ return 0;
+}
diff --git a/tools/perf/util/perf_regs.c b/tools/perf/util/perf_regs.c
index c4023f2..a37e593 100644
--- a/tools/perf/util/perf_regs.c
+++ b/tools/perf/util/perf_regs.c
@@ -6,6 +6,12 @@ const struct sample_reg __weak sample_reg_masks[] = {
SMPL_REG_END
};
+int __weak sdt_rename_register(char **pdesc __maybe_unused,
+ char *old_name __maybe_unused)
+{
+ return 0;
+}
+
#ifdef HAVE_PERF_REGS_SUPPORT
int perf_reg_value(u64 *valp, struct regs_dump *regs, int id)
{
diff --git a/tools/perf/util/perf_regs.h b/tools/perf/util/perf_regs.h
index 679d6e4..7544a15 100644
--- a/tools/perf/util/perf_regs.h
+++ b/tools/perf/util/perf_regs.h
@@ -15,6 +15,12 @@ struct sample_reg {
extern const struct sample_reg sample_reg_masks[];
+/*
+ * The table sdt_reg_renamings is used for adjusting gcc/gas-generated
+ * registers before filling the uprobe tracer interface.
+ */
+int sdt_rename_register(char **pdesc, char *old_name);
+
#ifdef HAVE_PERF_REGS_SUPPORT
#include <perf_regs.h>
diff --git a/tools/perf/util/probe-file.c b/tools/perf/util/probe-file.c
index c3c2871..d741634 100644
--- a/tools/perf/util/probe-file.c
+++ b/tools/perf/util/probe-file.c
@@ -27,6 +27,7 @@
#include "probe-event.h"
#include "probe-file.h"
#include "session.h"
+#include "perf_regs.h"
/* 4096 - 2 ('\n' + '\0') */
#define MAX_CMDLEN 4094
@@ -688,6 +689,166 @@ static unsigned long long sdt_note__get_addr(struct sdt_note *note)
: (unsigned long long)note->addr.a64[0];
}
+static const char * const type_to_suffix[] = {
+ ":s64", "", "", "", ":s32", "", ":s16", ":s8",
+ "", ":u8", ":u16", "", ":u32", "", "", "", ":u64"
+};
+
+static int synthesize_sdt_probe_arg(struct strbuf *buf, int i, const char *arg)
+{
+ char *tmp, *desc = strdup(arg);
+ const char *prefix = "", *suffix = "";
+ int ret = -1;
+
+ if (desc == NULL) {
+ pr_debug4("Allocation error\n");
+ return ret;
+ }
+
+ tmp = strchr(desc, '@');
+ if (tmp) {
+ long type_idx;
+ /*
+ * Isolate the string number and convert it into a
+ * binary value; this will be an index to get suffix
+ * of the uprobe name (defining the type)
+ */
+ tmp[0] = '\0';
+ type_idx = strtol(desc, NULL, 10);
+ /* Check that the conversion went OK */
+ if (type_idx == LONG_MIN || type_idx == LONG_MAX) {
+ pr_debug4("Failed to parse sdt type\n");
+ goto error;
+ }
+ /* Check that the converted value is OK */
+ if (type_idx < -8 || type_idx > 8) {
+ pr_debug4("Failed to get a valid sdt type\n");
+ goto error;
+ }
+ suffix = type_to_suffix[type_idx + 8];
+ /* Get rid of the sdt prefix which is now useless */
+ tmp++;
+ memmove(desc, tmp, strlen(tmp) + 1);
+ }
+
+ /*
+ * The uprobe tracer format does not support all the
+ * addressing modes (notably: in x86 the scaled mode); so, we
+ * detect ',' characters, if there is just one, there is no
+ * use converting the sdt arg into a uprobe one.
+ */
+ if (strchr(desc, ',')) {
+ pr_debug4("Skipping unsupported SDT argument; %s\n", desc);
+ goto out;
+ }
+
+ /*
+ * If the argument addressing mode is indirect, we must check
+ * a few things...
+ */
+ tmp = strchr(desc, '(');
+ if (tmp) {
+ int j;
+
+ /*
+ * ...if the addressing mode is indirect with a
+ * positive offset (ex.: "1608(%ax)"), we need to add
+ * a '+' prefix so as to be compliant with uprobe
+ * format.
+ */
+ if (desc[0] != '+' && desc[0] != '-')
+ prefix = "+";
+
+ /*
+ * ...or if the addressing mode is indirect with a symbol
+ * as offset, the argument will not be supported by
+ * the uprobe tracer format; so, let's skip this one.
+ */
+ for (j = 0; j < tmp - desc; j++) {
+ if (desc[j] != '+' && desc[j] != '-' &&
+ !isdigit(desc[j])) {
+ pr_debug4("Skipping unsupported SDT argument; "
+ "%s\n", desc);
+ goto out;
+ }
+ }
+ }
+
+ /*
+ * The uprobe tracer format does not support constants; if we
+ * find one in the current argument, let's skip the argument.
+ */
+ if (strchr(desc, '$')) {
+ pr_debug4("Skipping unsupported SDT argument; %s\n", desc);
+ goto out;
+ }
+
+ /*
+ * The uprobe parser does not support all gas register names;
+ * so, we have to replace them (ex. for x86_64: %rax -> %ax);
+ * the loop below looks for the register names (starting with
+ * a '%' and tries to perform the needed renamings.
+ */
+ tmp = strchr(desc, '%');
+ while (tmp) {
+ size_t offset = tmp - desc;
+
+ ret = sdt_rename_register(&desc, desc + offset);
+ if (ret < 0)
+ goto error;
+
+ /*
+ * The desc pointer might have changed; so, let's not
+ * try to reuse tmp for next lookup
+ */
+ tmp = strchr(desc + offset + 1, '%');
+ }
+
+ if (strbuf_addf(buf, " arg%d=%s%s%s", i + 1, prefix, desc, suffix) < 0)
+ goto error;
+
+out:
+ ret = 0;
+error:
+ free(desc);
+ return ret;
+}
+
+static char *synthesize_sdt_probe_command(struct sdt_note *note,
+ const char *pathname,
+ const char *sdtgrp)
+{
+ struct strbuf buf;
+ char *ret = NULL, **args;
+ int i, args_count;
+
+ if (strbuf_init(&buf, 32) < 0)
+ return NULL;
+
+ if (strbuf_addf(&buf, "p:%s/%s %s:0x%llx",
+ sdtgrp, note->name, pathname,
+ sdt_note__get_addr(note)) < 0)
+ goto error;
+
+ if (!note->args)
+ goto out;
+
+ if (note->args) {
+ args = argv_split(note->args, &args_count);
+
+ for (i = 0; i < args_count; ++i) {
+ if (synthesize_sdt_probe_arg(&buf, i, args[i]) < 0)
+ goto error;
+ }
+ }
+
+out:
+ ret = strbuf_detach(&buf, NULL);
+error:
+ strbuf_release(&buf);
+ return ret;
+}
+
int probe_cache__scan_sdt(struct probe_cache *pcache, const char *pathname)
{
struct probe_cache_entry *entry = NULL;
@@ -724,11 +885,12 @@ int probe_cache__scan_sdt(struct probe_cache *pcache, const char *pathname)
entry->pev.group = strdup(sdtgrp);
list_add_tail(&entry->node, &pcache->entries);
}
- ret = asprintf(&buf, "p:%s/%s %s:0x%llx",
- sdtgrp, note->name, pathname,
- sdt_note__get_addr(note));
- if (ret < 0)
+ buf = synthesize_sdt_probe_command(note, pathname, sdtgrp);
+ if (!buf) {
+ ret = -ENOMEM;
break;
+ }
+
strlist__add(entry->tevlist, buf);
free(buf);
entry = NULL;