2020-10-14 02:55:17

by Jiri Olsa

[permalink] [raw]
Subject: [PATCHv2 0/9] perf tools: Add support for build id with different sizes

hi,
currently we support only one storage size (20 bytes) for build
ids. That fits for SHA1 build ids, but there can in theory be
any size. The gcc linker supports also MD5, which is 16 bytes.

Currently the MD5 build id will be stored in .debug cache with
additional zeros, like:

$ find ~/.debug
.../.debug/.build-id/a5/0e350e97c43b4708d09bcd85ebfff700000000
...
.../buildid-ex-md5/a50e350e97c43b4708d09bcd85ebfff700000000
.../buildid-ex-md5/a50e350e97c43b4708d09bcd85ebfff700000000/elf
.../buildid-ex-md5/a50e350e97c43b4708d09bcd85ebfff700000000/probes

And the same reflected in buildid-list as well:

$ perf buildid-list
17f4e448cc746582ea1881528deb549f7fdb3fd5 [kernel.kallsyms]
a50e350e97c43b4708d09bcd85ebfff700000000 .../buildid-ex-md5

This will cause problems in future when we will ask debuginfod
for binaries/debuginfo based on build id.

This patchset is adding 'struct build_id' object, that holds
the build id data and size and use it to store build ids and
changes all related functions to use it.

Also available in:
git://git.kernel.org/pub/scm/linux/kernel/git/jolsa/perf.git
perf/build_id_size

v2 changes:
- rebased on current perf/core
- updated test to build its own binaries [Namhyung]

thanks,
jirka


---
Jiri Olsa (9):
perf tools: Use build_id object in dso
perf tools: Pass build_id object to filename__read_build_id
perf tools: Pass build id object to sysfs__read_build_id
perf tools: Pass build_id object to build_id__sprintf
perf tools: Pass build_id object to dso__set_build_id
perf tools: Pass build_id object to dso__build_id_equal
perf tools: Add size to struct perf_record_header_build_id
perf tools: Align buildid list output for short build ids
perf tools: Add build id shell test

tools/lib/perf/include/perf/event.h | 12 +++++++++++-
tools/perf/bench/inject-buildid.c | 4 ++--
tools/perf/builtin-buildid-cache.c | 25 ++++++++++++------------
tools/perf/builtin-inject.c | 4 +---
tools/perf/tests/pe-file-parsing.c | 10 +++++-----
tools/perf/tests/sdt.c | 6 +++---
tools/perf/tests/shell/buildid.sh | 101 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
tools/perf/util/annotate.c | 3 +--
tools/perf/util/build-id.c | 48 +++++++++++++++++++++++++++-------------------
tools/perf/util/build-id.h | 8 +++++++-
tools/perf/util/dso.c | 23 ++++++++++------------
tools/perf/util/dso.h | 7 +++----
tools/perf/util/dsos.c | 9 +++++----
tools/perf/util/header.c | 15 ++++++++++-----
tools/perf/util/map.c | 4 +---
tools/perf/util/probe-event.c | 9 ++++++---
tools/perf/util/probe-finder.c | 8 +++++---
tools/perf/util/scripting-engines/trace-event-python.c | 2 +-
tools/perf/util/symbol-elf.c | 35 +++++++++++++++++++--------------
tools/perf/util/symbol-minimal.c | 31 +++++++++++++++---------------
tools/perf/util/symbol.c | 15 +++++++--------
tools/perf/util/symbol.h | 5 +++--
tools/perf/util/synthetic-events.c | 2 +-
23 files changed, 260 insertions(+), 126 deletions(-)
create mode 100755 tools/perf/tests/shell/buildid.sh


2020-10-14 02:56:11

by Jiri Olsa

[permalink] [raw]
Subject: [PATCH 4/9] perf tools: Pass build_id object to build_id__sprintf

Passing build_id object to build_id__sprintf function,
so it can operate with the proper size of build id.

This will create proper md5 build id readable names,
like following:
a50e350e97c43b4708d09bcd85ebfff7

instead of:
a50e350e97c43b4708d09bcd85ebfff700000000

Acked-by: Ian Rogers <[email protected]>
Signed-off-by: Jiri Olsa <[email protected]>
---
tools/perf/builtin-buildid-cache.c | 6 ++--
tools/perf/tests/sdt.c | 2 +-
tools/perf/util/annotate.c | 3 +-
tools/perf/util/build-id.c | 30 ++++++++++++-------
tools/perf/util/build-id.h | 3 +-
tools/perf/util/dso.c | 6 ++--
tools/perf/util/header.c | 3 +-
tools/perf/util/map.c | 4 +--
tools/perf/util/probe-event.c | 9 ++++--
tools/perf/util/probe-finder.c | 8 +++--
.../scripting-engines/trace-event-python.c | 2 +-
tools/perf/util/symbol.c | 2 +-
12 files changed, 43 insertions(+), 35 deletions(-)

diff --git a/tools/perf/builtin-buildid-cache.c b/tools/perf/builtin-buildid-cache.c
index c3cb168d546d..a25411926e48 100644
--- a/tools/perf/builtin-buildid-cache.c
+++ b/tools/perf/builtin-buildid-cache.c
@@ -186,7 +186,7 @@ static int build_id_cache__add_file(const char *filename, struct nsinfo *nsi)
return -1;
}

- build_id__sprintf(bid.data, sizeof(bid.data), sbuild_id);
+ build_id__sprintf(&bid, sbuild_id);
err = build_id_cache__add_s(sbuild_id, filename, nsi,
false, false);
pr_debug("Adding %s %s: %s\n", sbuild_id, filename,
@@ -210,7 +210,7 @@ static int build_id_cache__remove_file(const char *filename, struct nsinfo *nsi)
return -1;
}

- build_id__sprintf(bid.data, sizeof(bid.data), sbuild_id);
+ build_id__sprintf(&bid, sbuild_id);
err = build_id_cache__remove_s(sbuild_id);
pr_debug("Removing %s %s: %s\n", sbuild_id, filename,
err ? "FAIL" : "Ok");
@@ -314,7 +314,7 @@ static int build_id_cache__update_file(const char *filename, struct nsinfo *nsi)
}
err = 0;

- build_id__sprintf(bid.data, sizeof(bid.data), sbuild_id);
+ build_id__sprintf(&bid, sbuild_id);
if (build_id_cache__cached(sbuild_id))
err = build_id_cache__remove_s(sbuild_id);

diff --git a/tools/perf/tests/sdt.c b/tools/perf/tests/sdt.c
index 3ef37f739203..ed76c693f65e 100644
--- a/tools/perf/tests/sdt.c
+++ b/tools/perf/tests/sdt.c
@@ -37,7 +37,7 @@ static int build_id_cache__add_file(const char *filename)
return err;
}

- build_id__sprintf(bid.data, sizeof(bid.data), sbuild_id);
+ build_id__sprintf(&bid, sbuild_id);
err = build_id_cache__add_s(sbuild_id, filename, NULL, false, false);
if (err < 0)
pr_debug("Failed to add build id cache of %s\n", filename);
diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
index a016e1bd7b8d..6c8575e182ed 100644
--- a/tools/perf/util/annotate.c
+++ b/tools/perf/util/annotate.c
@@ -1578,8 +1578,7 @@ int symbol__strerror_disassemble(struct map_symbol *ms, int errnum, char *buf, s
char *build_id_msg = NULL;

if (dso->has_build_id) {
- build_id__sprintf(dso->bid.data,
- sizeof(dso->bid.data), bf + 15);
+ build_id__sprintf(&dso->bid, bf + 15);
build_id_msg = bf;
}
scnprintf(buf, buflen,
diff --git a/tools/perf/util/build-id.c b/tools/perf/util/build-id.c
index 1c332e78bbc3..b5648735f01f 100644
--- a/tools/perf/util/build-id.c
+++ b/tools/perf/util/build-id.c
@@ -37,6 +37,7 @@

#include <linux/ctype.h>
#include <linux/zalloc.h>
+#include <asm/bug.h>

static bool no_buildid_cache;

@@ -95,13 +96,13 @@ struct perf_tool build_id__mark_dso_hit_ops = {
.ordered_events = true,
};

-int build_id__sprintf(const u8 *build_id, int len, char *bf)
+int build_id__sprintf(const struct build_id *build_id, char *bf)
{
char *bid = bf;
- const u8 *raw = build_id;
- int i;
+ const u8 *raw = build_id->data;
+ size_t i;

- for (i = 0; i < len; ++i) {
+ for (i = 0; i < build_id->size; ++i) {
sprintf(bid, "%02x", *raw);
++raw;
bid += 2;
@@ -125,7 +126,7 @@ int sysfs__sprintf_build_id(const char *root_dir, char *sbuild_id)
if (ret < 0)
return ret;

- return build_id__sprintf(bid.data, sizeof(bid.data), sbuild_id);
+ return build_id__sprintf(&bid, sbuild_id);
}

int filename__sprintf_build_id(const char *pathname, char *sbuild_id)
@@ -137,7 +138,7 @@ int filename__sprintf_build_id(const char *pathname, char *sbuild_id)
if (ret < 0)
return ret;

- return build_id__sprintf(bid.data, sizeof(bid.data), sbuild_id);
+ return build_id__sprintf(&bid, sbuild_id);
}

/* asnprintf consolidates asprintf and snprintf */
@@ -270,7 +271,7 @@ char *dso__build_id_filename(const struct dso *dso, char *bf, size_t size,
if (!dso->has_build_id)
return NULL;

- build_id__sprintf(dso->bid.data, sizeof(dso->bid.data), sbuild_id);
+ build_id__sprintf(&dso->bid, sbuild_id);
linkname = build_id_cache__linkname(sbuild_id, NULL, 0);
if (!linkname)
return NULL;
@@ -767,13 +768,13 @@ int build_id_cache__add_s(const char *sbuild_id, const char *name,
return err;
}

-static int build_id_cache__add_b(const u8 *build_id, size_t build_id_size,
+static int build_id_cache__add_b(const struct build_id *bid,
const char *name, struct nsinfo *nsi,
bool is_kallsyms, bool is_vdso)
{
char sbuild_id[SBUILD_ID_SIZE];

- build_id__sprintf(build_id, build_id_size, sbuild_id);
+ build_id__sprintf(bid, sbuild_id);

return build_id_cache__add_s(sbuild_id, name, nsi, is_kallsyms,
is_vdso);
@@ -839,8 +840,8 @@ static int dso__cache_build_id(struct dso *dso, struct machine *machine)
is_kallsyms = true;
name = machine->mmap_name;
}
- return build_id_cache__add_b(dso->bid.data, sizeof(dso->bid.data), name,
- dso->nsinfo, is_kallsyms, is_vdso);
+ return build_id_cache__add_b(&dso->bid, name, dso->nsinfo,
+ is_kallsyms, is_vdso);
}

static int __dsos__cache_build_ids(struct list_head *head,
@@ -900,3 +901,10 @@ bool perf_session__read_build_ids(struct perf_session *session, bool with_hits)

return ret;
}
+
+void build_id__init(struct build_id *bid, const u8 *data, size_t size)
+{
+ WARN_ON(size > BUILD_ID_SIZE);
+ memcpy(bid->data, data, size);
+ bid->size = size;
+}
diff --git a/tools/perf/util/build-id.h b/tools/perf/util/build-id.h
index 5033e96d5c9d..f293f99d5dba 100644
--- a/tools/perf/util/build-id.h
+++ b/tools/perf/util/build-id.h
@@ -19,7 +19,8 @@ extern struct perf_tool build_id__mark_dso_hit_ops;
struct dso;
struct feat_fd;

-int build_id__sprintf(const u8 *build_id, int len, char *bf);
+void build_id__init(struct build_id *bid, const u8 *data, size_t size);
+int build_id__sprintf(const struct build_id *build_id, char *bf);
int sysfs__sprintf_build_id(const char *root_dir, char *sbuild_id);
int filename__sprintf_build_id(const char *pathname, char *sbuild_id);
char *build_id_cache__kallsyms_path(const char *sbuild_id, char *bf,
diff --git a/tools/perf/util/dso.c b/tools/perf/util/dso.c
index e87fa9a71d9f..2f7f01ead9a1 100644
--- a/tools/perf/util/dso.c
+++ b/tools/perf/util/dso.c
@@ -172,9 +172,7 @@ int dso__read_binary_type_filename(const struct dso *dso,
break;
}

- build_id__sprintf(dso->bid.data,
- sizeof(dso->bid.data),
- build_id_hex);
+ build_id__sprintf(&dso->bid, build_id_hex);
len = __symbol__join_symfs(filename, size, "/usr/lib/debug/.build-id/");
snprintf(filename + len, size - len, "%.2s/%s.debug",
build_id_hex, build_id_hex + 2);
@@ -1374,7 +1372,7 @@ size_t dso__fprintf_buildid(struct dso *dso, FILE *fp)
{
char sbuild_id[SBUILD_ID_SIZE];

- build_id__sprintf(dso->bid.data, sizeof(dso->bid.data), sbuild_id);
+ build_id__sprintf(&dso->bid, sbuild_id);
return fprintf(fp, "%s", sbuild_id);
}

diff --git a/tools/perf/util/header.c b/tools/perf/util/header.c
index cde8f6eba479..fe220f01fc94 100644
--- a/tools/perf/util/header.c
+++ b/tools/perf/util/header.c
@@ -2095,8 +2095,7 @@ static int __event_process_build_id(struct perf_record_header_build_id *bev,
free(m.name);
}

- build_id__sprintf(dso->bid.data, sizeof(dso->bid.data),
- sbuild_id);
+ build_id__sprintf(&dso->bid, sbuild_id);
pr_debug("build id event received for %s: %s\n",
dso->long_name, sbuild_id);
dso__put(dso);
diff --git a/tools/perf/util/map.c b/tools/perf/util/map.c
index 522df3090b1c..f44ede437dc7 100644
--- a/tools/perf/util/map.c
+++ b/tools/perf/util/map.c
@@ -331,9 +331,7 @@ int map__load(struct map *map)
if (map->dso->has_build_id) {
char sbuild_id[SBUILD_ID_SIZE];

- build_id__sprintf(map->dso->bid.data,
- sizeof(map->dso->bid.data),
- sbuild_id);
+ build_id__sprintf(&map->dso->bid, sbuild_id);
pr_debug("%s with build id %s not found", name, sbuild_id);
} else
pr_debug("Failed to open %s", name);
diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
index 2041ad849851..8eae2afff71a 100644
--- a/tools/perf/util/probe-event.c
+++ b/tools/perf/util/probe-event.c
@@ -473,7 +473,7 @@ static struct debuginfo *open_from_debuginfod(struct dso *dso, struct nsinfo *ns
if (!c)
return NULL;

- build_id__sprintf(dso->bid.data, BUILD_ID_SIZE, sbuild_id);
+ build_id__sprintf(&dso->bid, sbuild_id);
fd = debuginfod_find_debuginfo(c, (const unsigned char *)sbuild_id,
0, &path);
if (fd >= 0)
@@ -1005,6 +1005,7 @@ static int _show_one_line(FILE *fp, int l, bool skip, bool show_num)
static int __show_line_range(struct line_range *lr, const char *module,
bool user)
{
+ struct build_id bid;
int l = 1;
struct int_node *ln;
struct debuginfo *dinfo;
@@ -1025,8 +1026,10 @@ static int __show_line_range(struct line_range *lr, const char *module,
if (!ret)
ret = debuginfo__find_line_range(dinfo, lr);
}
- if (dinfo->build_id)
- build_id__sprintf(dinfo->build_id, BUILD_ID_SIZE, sbuild_id);
+ if (dinfo->build_id) {
+ build_id__init(&bid, dinfo->build_id, BUILD_ID_SIZE);
+ build_id__sprintf(&bid, sbuild_id);
+ }
debuginfo__delete(dinfo);
if (ret == 0 || ret == -ENOENT) {
pr_warning("Specified source line is not found.\n");
diff --git a/tools/perf/util/probe-finder.c b/tools/perf/util/probe-finder.c
index 6eddf7be8293..2c4061035f77 100644
--- a/tools/perf/util/probe-finder.c
+++ b/tools/perf/util/probe-finder.c
@@ -949,6 +949,7 @@ static int probe_point_lazy_walker(const char *fname, int lineno,
/* Find probe points from lazy pattern */
static int find_probe_point_lazy(Dwarf_Die *sp_die, struct probe_finder *pf)
{
+ struct build_id bid;
char sbuild_id[SBUILD_ID_SIZE] = "";
int ret = 0;
char *fpath;
@@ -957,9 +958,10 @@ static int find_probe_point_lazy(Dwarf_Die *sp_die, struct probe_finder *pf)
const char *comp_dir;

comp_dir = cu_get_comp_dir(&pf->cu_die);
- if (pf->dbg->build_id)
- build_id__sprintf(pf->dbg->build_id,
- BUILD_ID_SIZE, sbuild_id);
+ if (pf->dbg->build_id) {
+ build_id__init(&bid, pf->dbg->build_id, BUILD_ID_SIZE);
+ build_id__sprintf(&bid, sbuild_id);
+ }
ret = find_source_path(pf->fname, sbuild_id, comp_dir, &fpath);
if (ret < 0) {
pr_warning("Failed to find source file path.\n");
diff --git a/tools/perf/util/scripting-engines/trace-event-python.c b/tools/perf/util/scripting-engines/trace-event-python.c
index db3b1c275d8f..7cbd024e3e63 100644
--- a/tools/perf/util/scripting-engines/trace-event-python.c
+++ b/tools/perf/util/scripting-engines/trace-event-python.c
@@ -1064,7 +1064,7 @@ static int python_export_dso(struct db_export *dbe, struct dso *dso,
char sbuild_id[SBUILD_ID_SIZE];
PyObject *t;

- build_id__sprintf(dso->bid.data, sizeof(dso->bid.data), sbuild_id);
+ build_id__sprintf(&dso->bid, sbuild_id);

t = tuple_new(5);

diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
index dd1cf91c62fb..369cbad09f0d 100644
--- a/tools/perf/util/symbol.c
+++ b/tools/perf/util/symbol.c
@@ -2152,7 +2152,7 @@ static char *dso__find_kallsyms(struct dso *dso, struct map *map)
goto proc_kallsyms;
}

- build_id__sprintf(dso->bid.data, sizeof(dso->bid.data), sbuild_id);
+ build_id__sprintf(&dso->bid, sbuild_id);

/* Find kallsyms in build-id cache with kcore */
scnprintf(path, sizeof(path), "%s/%s/%s",
--
2.26.2

2020-10-14 02:57:10

by Jiri Olsa

[permalink] [raw]
Subject: [PATCH 5/9] perf tools: Pass build_id object to dso__set_build_id

Passing build_id object to dso__set_build_id, so it's easier
to initialize dos's build id object.

Acked-by: Ian Rogers <[email protected]>
Signed-off-by: Jiri Olsa <[email protected]>
---
tools/perf/util/dso.c | 4 ++--
tools/perf/util/dso.h | 2 +-
tools/perf/util/header.c | 4 +++-
tools/perf/util/symbol-minimal.c | 2 +-
tools/perf/util/symbol.c | 2 +-
5 files changed, 8 insertions(+), 6 deletions(-)

diff --git a/tools/perf/util/dso.c b/tools/perf/util/dso.c
index 2f7f01ead9a1..4415ce83150b 100644
--- a/tools/perf/util/dso.c
+++ b/tools/perf/util/dso.c
@@ -1326,9 +1326,9 @@ void dso__put(struct dso *dso)
dso__delete(dso);
}

-void dso__set_build_id(struct dso *dso, void *build_id)
+void dso__set_build_id(struct dso *dso, struct build_id *bid)
{
- memcpy(dso->bid.data, build_id, sizeof(dso->bid.data));
+ dso->bid = *bid;
dso->has_build_id = 1;
}

diff --git a/tools/perf/util/dso.h b/tools/perf/util/dso.h
index eac004210b47..5a5678dbdaa5 100644
--- a/tools/perf/util/dso.h
+++ b/tools/perf/util/dso.h
@@ -260,7 +260,7 @@ bool dso__sorted_by_name(const struct dso *dso);
void dso__set_sorted_by_name(struct dso *dso);
void dso__sort_by_name(struct dso *dso);

-void dso__set_build_id(struct dso *dso, void *build_id);
+void dso__set_build_id(struct dso *dso, struct build_id *bid);
bool dso__build_id_equal(const struct dso *dso, u8 *build_id);
void dso__read_running_kernel_build_id(struct dso *dso,
struct machine *machine);
diff --git a/tools/perf/util/header.c b/tools/perf/util/header.c
index fe220f01fc94..21243adbb9fd 100644
--- a/tools/perf/util/header.c
+++ b/tools/perf/util/header.c
@@ -2082,8 +2082,10 @@ static int __event_process_build_id(struct perf_record_header_build_id *bev,
dso = machine__findnew_dso(machine, filename);
if (dso != NULL) {
char sbuild_id[SBUILD_ID_SIZE];
+ struct build_id bid;

- dso__set_build_id(dso, &bev->build_id);
+ build_id__init(&bid, bev->build_id, BUILD_ID_SIZE);
+ dso__set_build_id(dso, &bid);

if (dso_space != DSO_SPACE__USER) {
struct kmod_path m = { .name = NULL, };
diff --git a/tools/perf/util/symbol-minimal.c b/tools/perf/util/symbol-minimal.c
index dba6b9e5d64e..f9eb0bee7f15 100644
--- a/tools/perf/util/symbol-minimal.c
+++ b/tools/perf/util/symbol-minimal.c
@@ -349,7 +349,7 @@ int dso__load_sym(struct dso *dso, struct map *map __maybe_unused,
dso->is_64_bit = ret;

if (filename__read_build_id(ss->name, &bid) > 0)
- dso__set_build_id(dso, bid.data);
+ dso__set_build_id(dso, &bid);
return 0;
}

diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
index 369cbad09f0d..976632d0baa0 100644
--- a/tools/perf/util/symbol.c
+++ b/tools/perf/util/symbol.c
@@ -1818,7 +1818,7 @@ int dso__load(struct dso *dso, struct map *map)
is_regular_file(dso->long_name)) {
__symbol__join_symfs(name, PATH_MAX, dso->long_name);
if (filename__read_build_id(name, &bid) > 0)
- dso__set_build_id(dso, bid.data);
+ dso__set_build_id(dso, &bid);
}

/*
--
2.26.2

2020-10-14 10:00:32

by Jiri Olsa

[permalink] [raw]
Subject: [PATCH 8/9] perf tools: Align buildid list output for short build ids

With shorter md5 build ids we need to align their
paths properly with other build ids:

$ perf buildid-list
17f4e448cc746582ea1881528deb549f7fdb3fd5 [kernel.kallsyms]
a50e350e97c43b4708d09bcd85ebfff7 .../tools/perf/buildid-ex-md5
1805c738c8f3ec0f47b7ea09080c28f34d18a82b /usr/lib64/ld-2.31.so

Acked-by: Ian Rogers <[email protected]>
Signed-off-by: Jiri Olsa <[email protected]>
---
tools/perf/util/dso.c | 2 +-
tools/perf/util/dso.h | 1 -
tools/perf/util/dsos.c | 6 ++++--
3 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/tools/perf/util/dso.c b/tools/perf/util/dso.c
index ca965845b35e..55c11e854fe4 100644
--- a/tools/perf/util/dso.c
+++ b/tools/perf/util/dso.c
@@ -1369,7 +1369,7 @@ int dso__kernel_module_get_build_id(struct dso *dso,
return 0;
}

-size_t dso__fprintf_buildid(struct dso *dso, FILE *fp)
+static size_t dso__fprintf_buildid(struct dso *dso, FILE *fp)
{
char sbuild_id[SBUILD_ID_SIZE];

diff --git a/tools/perf/util/dso.h b/tools/perf/util/dso.h
index f926c96bf230..d8cb4f5680a4 100644
--- a/tools/perf/util/dso.h
+++ b/tools/perf/util/dso.h
@@ -362,7 +362,6 @@ struct dso *machine__findnew_kernel(struct machine *machine, const char *name,

void dso__reset_find_symbol_cache(struct dso *dso);

-size_t dso__fprintf_buildid(struct dso *dso, FILE *fp);
size_t dso__fprintf_symbols_by_name(struct dso *dso, FILE *fp);
size_t dso__fprintf(struct dso *dso, FILE *fp);

diff --git a/tools/perf/util/dsos.c b/tools/perf/util/dsos.c
index 87161e431830..183a81d5b2f9 100644
--- a/tools/perf/util/dsos.c
+++ b/tools/perf/util/dsos.c
@@ -287,10 +287,12 @@ size_t __dsos__fprintf_buildid(struct list_head *head, FILE *fp,
size_t ret = 0;

list_for_each_entry(pos, head, node) {
+ char sbuild_id[SBUILD_ID_SIZE];
+
if (skip && skip(pos, parm))
continue;
- ret += dso__fprintf_buildid(pos, fp);
- ret += fprintf(fp, " %s\n", pos->long_name);
+ build_id__sprintf(&pos->bid, sbuild_id);
+ ret += fprintf(fp, "%-40s %s\n", sbuild_id, pos->long_name);
}
return ret;
}
--
2.26.2

2020-10-14 10:01:37

by Jiri Olsa

[permalink] [raw]
Subject: [PATCH 9/9] perf tools: Add build id shell test

Adding test for build id cache that adds binary
with sha1 and md5 build ids and verifies it's
added properly.

The test updates build id cache with perf record
and perf buildid-cache -a.

Acked-by: Ian Rogers <[email protected]>
Signed-off-by: Jiri Olsa <[email protected]>
---
tools/perf/tests/shell/buildid.sh | 101 ++++++++++++++++++++++++++++++
1 file changed, 101 insertions(+)
create mode 100755 tools/perf/tests/shell/buildid.sh

diff --git a/tools/perf/tests/shell/buildid.sh b/tools/perf/tests/shell/buildid.sh
new file mode 100755
index 000000000000..4861a20edee2
--- /dev/null
+++ b/tools/perf/tests/shell/buildid.sh
@@ -0,0 +1,101 @@
+#!/bin/sh
+# build id cache operations
+# SPDX-License-Identifier: GPL-2.0
+
+# skip if there's no readelf
+if ! [ -x "$(command -v readelf)" ]; then
+ echo "failed: no readelf, install binutils"
+ exit 2
+fi
+
+# skip if there's no compiler
+if ! [ -x "$(command -v cc)" ]; then
+ echo "failed: no compiler, install gcc"
+ exit 2
+fi
+
+ex_md5=$(mktemp /tmp/perf.ex.MD5.XXX)
+ex_sha1=$(mktemp /tmp/perf.ex.SHA1.XXX)
+
+echo 'int main(void) { return 0; }' | cc -Wl,--build-id=sha1 -o ${ex_sha1} -x c -
+echo 'int main(void) { return 0; }' | cc -Wl,--build-id=md5 -o ${ex_md5} -x c -
+
+echo "test binaries: ${ex_sha1} ${ex_md5}"
+
+check()
+{
+ id=`readelf -n ${1} 2>/dev/null | grep 'Build ID' | awk '{print $3}'`
+
+ echo "build id: ${id}"
+
+ link=${build_id_dir}/.build-id/${id:0:2}/${id:2}
+ echo "link: ${link}"
+
+ if [ ! -h $link ]; then
+ echo "failed: link ${link} does not exist"
+ exit 1
+ fi
+
+ file=${build_id_dir}/.build-id/${id:0:2}/`readlink ${link}`/elf
+ echo "file: ${file}"
+
+ if [ ! -x $file ]; then
+ echo "failed: file ${file} does not exist"
+ exit 1
+ fi
+
+ diff ${file} ${1}
+ if [ $? -ne 0 ]; then
+ echo "failed: ${file} do not match"
+ exit 1
+ fi
+
+ echo "OK for ${1}"
+}
+
+test_add()
+{
+ build_id_dir=$(mktemp -d /tmp/perf.debug.XXX)
+ perf="perf --buildid-dir ${build_id_dir}"
+
+ ${perf} buildid-cache -v -a ${1}
+ if [ $? -ne 0 ]; then
+ echo "failed: add ${1} to build id cache"
+ exit 1
+ fi
+
+ check ${1}
+
+ rm -rf ${build_id_dir}
+}
+
+test_record()
+{
+ data=$(mktemp /tmp/perf.data.XXX)
+ build_id_dir=$(mktemp -d /tmp/perf.debug.XXX)
+ perf="perf --buildid-dir ${build_id_dir}"
+
+ ${perf} record --buildid-all -o ${data} ${1}
+ if [ $? -ne 0 ]; then
+ echo "failed: record ${1}"
+ exit 1
+ fi
+
+ check ${1}
+
+ rm -rf ${build_id_dir}
+ rm -rf ${data}
+}
+
+# add binaries manual via perf buildid-cache -a
+test_add ${ex_sha1}
+test_add ${ex_md5}
+
+# add binaries via perf record post processing
+test_record ${ex_sha1}
+test_record ${ex_md5}
+
+# cleanup
+rm ${ex_sha1} ${ex_md5}
+
+exit ${err}
--
2.26.2

2020-10-14 16:08:40

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: Re: [PATCH 5/9] perf tools: Pass build_id object to dso__set_build_id

Em Tue, Oct 13, 2020 at 09:24:37PM +0200, Jiri Olsa escreveu:
> Passing build_id object to dso__set_build_id, so it's easier
> to initialize dos's build id object.
>
> Acked-by: Ian Rogers <[email protected]>
> Signed-off-by: Jiri Olsa <[email protected]>
> ---
> tools/perf/util/dso.c | 4 ++--
> tools/perf/util/dso.h | 2 +-
> tools/perf/util/header.c | 4 +++-
> tools/perf/util/symbol-minimal.c | 2 +-
> tools/perf/util/symbol.c | 2 +-
> 5 files changed, 8 insertions(+), 6 deletions(-)
>
> diff --git a/tools/perf/util/dso.c b/tools/perf/util/dso.c
> index 2f7f01ead9a1..4415ce83150b 100644
> --- a/tools/perf/util/dso.c
> +++ b/tools/perf/util/dso.c
> @@ -1326,9 +1326,9 @@ void dso__put(struct dso *dso)
> dso__delete(dso);
> }
>
> -void dso__set_build_id(struct dso *dso, void *build_id)
> +void dso__set_build_id(struct dso *dso, struct build_id *bid)
> {
> - memcpy(dso->bid.data, build_id, sizeof(dso->bid.data));
> + dso->bid = *bid;

Can't we use bid->size here?

dso->bid.size = bid->size;
memcpy(dso->bid.data, bid->data, bid->size));

?

Not worth it? Probably :-)

- Arnaldo

> dso->has_build_id = 1;
> }
>
> diff --git a/tools/perf/util/dso.h b/tools/perf/util/dso.h
> index eac004210b47..5a5678dbdaa5 100644
> --- a/tools/perf/util/dso.h
> +++ b/tools/perf/util/dso.h
> @@ -260,7 +260,7 @@ bool dso__sorted_by_name(const struct dso *dso);
> void dso__set_sorted_by_name(struct dso *dso);
> void dso__sort_by_name(struct dso *dso);
>
> -void dso__set_build_id(struct dso *dso, void *build_id);
> +void dso__set_build_id(struct dso *dso, struct build_id *bid);
> bool dso__build_id_equal(const struct dso *dso, u8 *build_id);
> void dso__read_running_kernel_build_id(struct dso *dso,
> struct machine *machine);
> diff --git a/tools/perf/util/header.c b/tools/perf/util/header.c
> index fe220f01fc94..21243adbb9fd 100644
> --- a/tools/perf/util/header.c
> +++ b/tools/perf/util/header.c
> @@ -2082,8 +2082,10 @@ static int __event_process_build_id(struct perf_record_header_build_id *bev,
> dso = machine__findnew_dso(machine, filename);
> if (dso != NULL) {
> char sbuild_id[SBUILD_ID_SIZE];
> + struct build_id bid;
>
> - dso__set_build_id(dso, &bev->build_id);
> + build_id__init(&bid, bev->build_id, BUILD_ID_SIZE);
> + dso__set_build_id(dso, &bid);
>
> if (dso_space != DSO_SPACE__USER) {
> struct kmod_path m = { .name = NULL, };
> diff --git a/tools/perf/util/symbol-minimal.c b/tools/perf/util/symbol-minimal.c
> index dba6b9e5d64e..f9eb0bee7f15 100644
> --- a/tools/perf/util/symbol-minimal.c
> +++ b/tools/perf/util/symbol-minimal.c
> @@ -349,7 +349,7 @@ int dso__load_sym(struct dso *dso, struct map *map __maybe_unused,
> dso->is_64_bit = ret;
>
> if (filename__read_build_id(ss->name, &bid) > 0)
> - dso__set_build_id(dso, bid.data);
> + dso__set_build_id(dso, &bid);
> return 0;
> }
>
> diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
> index 369cbad09f0d..976632d0baa0 100644
> --- a/tools/perf/util/symbol.c
> +++ b/tools/perf/util/symbol.c
> @@ -1818,7 +1818,7 @@ int dso__load(struct dso *dso, struct map *map)
> is_regular_file(dso->long_name)) {
> __symbol__join_symfs(name, PATH_MAX, dso->long_name);
> if (filename__read_build_id(name, &bid) > 0)
> - dso__set_build_id(dso, bid.data);
> + dso__set_build_id(dso, &bid);
> }
>
> /*
> --
> 2.26.2
>

--

- Arnaldo

2020-10-14 16:10:33

by Jiri Olsa

[permalink] [raw]
Subject: Re: [PATCH 5/9] perf tools: Pass build_id object to dso__set_build_id

On Wed, Oct 14, 2020 at 08:51:44AM -0300, Arnaldo Carvalho de Melo wrote:
> Em Tue, Oct 13, 2020 at 09:24:37PM +0200, Jiri Olsa escreveu:
> > Passing build_id object to dso__set_build_id, so it's easier
> > to initialize dos's build id object.
> >
> > Acked-by: Ian Rogers <[email protected]>
> > Signed-off-by: Jiri Olsa <[email protected]>
> > ---
> > tools/perf/util/dso.c | 4 ++--
> > tools/perf/util/dso.h | 2 +-
> > tools/perf/util/header.c | 4 +++-
> > tools/perf/util/symbol-minimal.c | 2 +-
> > tools/perf/util/symbol.c | 2 +-
> > 5 files changed, 8 insertions(+), 6 deletions(-)
> >
> > diff --git a/tools/perf/util/dso.c b/tools/perf/util/dso.c
> > index 2f7f01ead9a1..4415ce83150b 100644
> > --- a/tools/perf/util/dso.c
> > +++ b/tools/perf/util/dso.c
> > @@ -1326,9 +1326,9 @@ void dso__put(struct dso *dso)
> > dso__delete(dso);
> > }
> >
> > -void dso__set_build_id(struct dso *dso, void *build_id)
> > +void dso__set_build_id(struct dso *dso, struct build_id *bid)
> > {
> > - memcpy(dso->bid.data, build_id, sizeof(dso->bid.data));
> > + dso->bid = *bid;
>
> Can't we use bid->size here?
>
> dso->bid.size = bid->size;
> memcpy(dso->bid.data, bid->data, bid->size));
>
> ?
>
> Not worth it? Probably :-)

yea, I wonder compiler will do the same thing in both cases,
but I don't know ;-)

I wanted to demonstrate that it's the same object

jirka