Re: [PATCH v2 2/2] perf probe: add sdt probes arguments into the uprobe cmd string
From: Hemant Kumar
Date: Mon Nov 21 2016 - 05:25:24 EST
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 <alexis.berlemont@xxxxxxxxx>
---
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;