2017-12-18 07:29:06

by Masami Hiramatsu

[permalink] [raw]
Subject: [PATCH 0/2] perf-probe: Improve warning message for buildid mismatch

Hello,

This series ensure the build-ids for target binary and debuginfo
are matched. If there is a mismatch, it warns user to check the
package versions.

To reproduce the problematic environment (on Fedora26),
I did following operations;

=====
# rpm -q glibc glibc-debuginfo
glibc-2.25-12.fc26.x86_64
package glibc-debuginfo is not installed

# dnf debuginfo-install glibc
[...]
Installed:
glibc-debuginfo.x86_64 2.25-12.fc26
glibc-debuginfo-common.x86_64 2.25-12.fc26

Complete!

# dnf downgrade glibc
[...]
Downgraded:
glibc.x86_64 2.25-6.fc26 glibc-common.x86_64 2.25-6.fc26
glibc-devel.x86_64 2.25-6.fc26 glibc-headers.x86_64 2.25-6.fc26
glibc-langpack-en.x86_64 2.25-6.fc26 libcrypt-nss.x86_64 2.25-6.fc26

Complete!

# rpm -q glibc glibc-debuginfo
glibc-2.25-6.fc26.x86_64
glibc-debuginfo-2.25-12.fc26.x86_64
=====

OK, so you can see now we have different versions of glibc and
glibc-debuginfo. Each debuginfo must be different.

With this series, defining new events also warns :)

=====
# perf probe -x /usr/lib64/libc-2.25.so -a malloc_get_state\\@GLIBC_2.2.5
WARN: There is a build-id mismatch between
/usr/lib/debug/usr/lib64/libc-2.25.so.debug
and
/usr/lib64/libc-2.25.so
Please check your system's debuginfo files for mismatches, e.g. check the versions for the target package and debuginfo package.
Added new event:
probe_libc:malloc_get_state (on malloc_get_state@GLIBC_2.2.5 in /usr/lib64/libc-2.25.so)

You can now use it in all perf tools, such as:

perf record -e probe_libc:malloc_get_state -aR sleep 1

=====

And listing up events warns.

=====
# perf probe -l | more
WARN: There is a build-id mismatch between
/usr/lib/debug/usr/lib64/libc-2.25.so.debug
and
/usr/lib64/libc-2.25.so
Please check your system's debuginfo files for mismatches, e.g. check the versions for the target package and debuginfo package.
probe_libc:malloc_get_state (on malloc_get_state@GLIBC_2.2.5 in /usr/lib64/libc-2.25.so)
=====

With [1/2], this warning messages repeated on same target (glibc). To suppress
this, [2/2] introduces no-debuginfo blacklist to skip it 2nd time as below.

=====
# perf probe -l | more
WARN: There is a build-id mismatch between
/usr/lib/debug/usr/lib64/libc-2.25.so.debug
and
/usr/lib64/libc-2.25.so
Please check your system's debuginfo files for mismatches, e.g. check the versions for the target package and debuginfo package.
probe_libc:malloc_get_state (on malloc_get_state@GLIBC_2.2.5 in /usr/lib64/libc-2.25.so)
probe_libc:malloc_get_state_1 (on malloc_get_state@GLIBC_2.2.5 in /usr/lib64/libc-2.25.so)
probe_libc:malloc_get_state_2 (on malloc_get_state@GLIBC_2.2.5 in /usr/lib64/libc-2.25.so)
=====

Arnaldo, would this match to your request?


Thank you,
---

Masami Hiramatsu (2):
perf-probe: Ensure debuginfo's build-id is correct
perf-probe: Skip searching debuginfo if we know no debuginfo


tools/perf/util/probe-finder.c | 34 +++++++++++++++++++++++++++++++++-
1 file changed, 33 insertions(+), 1 deletion(-)

--
Masami Hiramatsu (Linaro Ltd.) <[email protected]>


2017-12-18 07:29:28

by Masami Hiramatsu

[permalink] [raw]
Subject: [PATCH 1/2] perf-probe: Ensure debuginfo's build-id is correct

Ensure that the build-id of debuginfo is correctly
matched to target build-id, if not, it warns user
to check the system debuginfo package is correctly
installed.

E.g. on such environment, you will see below warning.
======
# perf probe -l
WARN: There is a build-id mismatch between
/usr/lib/debug/usr/lib64/libc-2.25.so.debug
and
/usr/lib64/libc-2.25.so
Please check your system's debuginfo files for mismatches, e.g. check
the versions for the target package and debuginfo package.
probe_libc:malloc_get_state (on malloc_get_state@GLIBC_2.2.5 in /us
r/lib64/libc-2.25.so)
======

Signed-off-by: Masami Hiramatsu <[email protected]>
---
tools/perf/util/probe-finder.c | 18 ++++++++++++++++++
1 file changed, 18 insertions(+)

diff --git a/tools/perf/util/probe-finder.c b/tools/perf/util/probe-finder.c
index a5731de0e5eb..5bb71e056b21 100644
--- a/tools/perf/util/probe-finder.c
+++ b/tools/perf/util/probe-finder.c
@@ -119,9 +119,11 @@ enum dso_binary_type distro_dwarf_types[] = {

struct debuginfo *debuginfo__new(const char *path)
{
+ u8 bid[BUILD_ID_SIZE], bid2[BUILD_ID_SIZE];
enum dso_binary_type *type;
char buf[PATH_MAX], nil = '\0';
struct dso *dso;
+ bool have_build_id = false;
struct debuginfo *dinfo = NULL;

/* Try to open distro debuginfo files */
@@ -129,12 +131,28 @@ struct debuginfo *debuginfo__new(const char *path)
if (!dso)
goto out;

+ if (filename__read_build_id(path, bid, BUILD_ID_SIZE) > 0)
+ have_build_id = true;
+
for (type = distro_dwarf_types;
!dinfo && *type != DSO_BINARY_TYPE__NOT_FOUND;
type++) {
if (dso__read_binary_type_filename(dso, *type, &nil,
buf, PATH_MAX) < 0)
continue;
+
+ if (have_build_id) {
+ /* This can be fail because the file doesn't exist */
+ if (filename__read_build_id(buf, bid2,
+ BUILD_ID_SIZE) < 0)
+ continue;
+ if (memcmp(bid, bid2, BUILD_ID_SIZE)) {
+ pr_warning("WARN: There is a build-id mismatch between\n %s\n and\n %s\n"
+ "Please check your system's debuginfo files for mismatches, e.g. check the "
+ "versions for the target package and debuginfo package.\n", buf, path);
+ continue;
+ }
+ }
dinfo = __debuginfo__new(buf);
}
dso__put(dso);

2017-12-18 07:29:57

by Masami Hiramatsu

[permalink] [raw]
Subject: [PATCH 2/2] perf-probe: Skip searching debuginfo if we know no debuginfo

Skip searching debuginfo if we have already searched and
there is no debuginfo for given file.
This also reduce debuginfo mismatch warnings for listing
events on same binary.

Signed-off-by: Masami Hiramatsu <[email protected]>
---
tools/perf/util/probe-finder.c | 16 +++++++++++++++-
1 file changed, 15 insertions(+), 1 deletion(-)

diff --git a/tools/perf/util/probe-finder.c b/tools/perf/util/probe-finder.c
index 5bb71e056b21..cafc3e5b9863 100644
--- a/tools/perf/util/probe-finder.c
+++ b/tools/perf/util/probe-finder.c
@@ -119,6 +119,8 @@ enum dso_binary_type distro_dwarf_types[] = {

struct debuginfo *debuginfo__new(const char *path)
{
+ static struct strlist *no_debuginfo_files;
+
u8 bid[BUILD_ID_SIZE], bid2[BUILD_ID_SIZE];
enum dso_binary_type *type;
char buf[PATH_MAX], nil = '\0';
@@ -126,6 +128,12 @@ struct debuginfo *debuginfo__new(const char *path)
bool have_build_id = false;
struct debuginfo *dinfo = NULL;

+ /* Skip if we already know it has no debuginfo */
+ if (!no_debuginfo_files)
+ no_debuginfo_files = strlist__new(NULL, NULL);
+ else if (strlist__find(no_debuginfo_files, path))
+ return NULL;
+
/* Try to open distro debuginfo files */
dso = dso__new(path);
if (!dso)
@@ -159,7 +167,13 @@ struct debuginfo *debuginfo__new(const char *path)

out:
/* if failed to open all distro debuginfo, open given binary */
- return dinfo ? : __debuginfo__new(path);
+ if (!dinfo) {
+ dinfo = __debuginfo__new(path);
+ if (!dinfo)
+ strlist__add(no_debuginfo_files, path);
+ }
+
+ return dinfo;
}

void debuginfo__delete(struct debuginfo *dbg)

2017-12-29 02:52:53

by Masami Hiramatsu

[permalink] [raw]
Subject: Re: [PATCH 0/2] perf-probe: Improve warning message for buildid mismatch

Hello Arnaldo,

Could you review it?

Thank you,

On Mon, 18 Dec 2017 16:28:35 +0900
Masami Hiramatsu <[email protected]> wrote:

> Hello,
>
> This series ensure the build-ids for target binary and debuginfo
> are matched. If there is a mismatch, it warns user to check the
> package versions.
>
> To reproduce the problematic environment (on Fedora26),
> I did following operations;
>
> =====
> # rpm -q glibc glibc-debuginfo
> glibc-2.25-12.fc26.x86_64
> package glibc-debuginfo is not installed
>
> # dnf debuginfo-install glibc
> [...]
> Installed:
> glibc-debuginfo.x86_64 2.25-12.fc26
> glibc-debuginfo-common.x86_64 2.25-12.fc26
>
> Complete!
>
> # dnf downgrade glibc
> [...]
> Downgraded:
> glibc.x86_64 2.25-6.fc26 glibc-common.x86_64 2.25-6.fc26
> glibc-devel.x86_64 2.25-6.fc26 glibc-headers.x86_64 2.25-6.fc26
> glibc-langpack-en.x86_64 2.25-6.fc26 libcrypt-nss.x86_64 2.25-6.fc26
>
> Complete!
>
> # rpm -q glibc glibc-debuginfo
> glibc-2.25-6.fc26.x86_64
> glibc-debuginfo-2.25-12.fc26.x86_64
> =====
>
> OK, so you can see now we have different versions of glibc and
> glibc-debuginfo. Each debuginfo must be different.
>
> With this series, defining new events also warns :)
>
> =====
> # perf probe -x /usr/lib64/libc-2.25.so -a malloc_get_state\\@GLIBC_2.2.5
> WARN: There is a build-id mismatch between
> /usr/lib/debug/usr/lib64/libc-2.25.so.debug
> and
> /usr/lib64/libc-2.25.so
> Please check your system's debuginfo files for mismatches, e.g. check the versions for the target package and debuginfo package.
> Added new event:
> probe_libc:malloc_get_state (on malloc_get_state@GLIBC_2.2.5 in /usr/lib64/libc-2.25.so)
>
> You can now use it in all perf tools, such as:
>
> perf record -e probe_libc:malloc_get_state -aR sleep 1
>
> =====
>
> And listing up events warns.
>
> =====
> # perf probe -l | more
> WARN: There is a build-id mismatch between
> /usr/lib/debug/usr/lib64/libc-2.25.so.debug
> and
> /usr/lib64/libc-2.25.so
> Please check your system's debuginfo files for mismatches, e.g. check the versions for the target package and debuginfo package.
> probe_libc:malloc_get_state (on malloc_get_state@GLIBC_2.2.5 in /usr/lib64/libc-2.25.so)
> =====
>
> With [1/2], this warning messages repeated on same target (glibc). To suppress
> this, [2/2] introduces no-debuginfo blacklist to skip it 2nd time as below.
>
> =====
> # perf probe -l | more
> WARN: There is a build-id mismatch between
> /usr/lib/debug/usr/lib64/libc-2.25.so.debug
> and
> /usr/lib64/libc-2.25.so
> Please check your system's debuginfo files for mismatches, e.g. check the versions for the target package and debuginfo package.
> probe_libc:malloc_get_state (on malloc_get_state@GLIBC_2.2.5 in /usr/lib64/libc-2.25.so)
> probe_libc:malloc_get_state_1 (on malloc_get_state@GLIBC_2.2.5 in /usr/lib64/libc-2.25.so)
> probe_libc:malloc_get_state_2 (on malloc_get_state@GLIBC_2.2.5 in /usr/lib64/libc-2.25.so)
> =====
>
> Arnaldo, would this match to your request?
>
>
> Thank you,
> ---
>
> Masami Hiramatsu (2):
> perf-probe: Ensure debuginfo's build-id is correct
> perf-probe: Skip searching debuginfo if we know no debuginfo
>
>
> tools/perf/util/probe-finder.c | 34 +++++++++++++++++++++++++++++++++-
> 1 file changed, 33 insertions(+), 1 deletion(-)
>
> --
> Masami Hiramatsu (Linaro Ltd.) <[email protected]>


--
Masami Hiramatsu <[email protected]>

2018-01-03 06:50:39

by Ravi Bangoria

[permalink] [raw]
Subject: Re: [PATCH 0/2] perf-probe: Improve warning message for buildid mismatch



On 12/18/2017 12:58 PM, Masami Hiramatsu wrote:
> Hello,
>
> This series ensure the build-ids for target binary and debuginfo
> are matched. If there is a mismatch, it warns user to check the
> package versions.

For the series,

Reviewed-by: Ravi Bangoria <[email protected]>

2018-01-04 16:17:34

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: Re: [PATCH 1/2] perf-probe: Ensure debuginfo's build-id is correct

Em Mon, Dec 18, 2017 at 04:29:03PM +0900, Masami Hiramatsu escreveu:
> Ensure that the build-id of debuginfo is correctly
> matched to target build-id, if not, it warns user
> to check the system debuginfo package is correctly
> installed.

So we look at a variety of files looking for one that has a matching
build-id, I think the warning message should state that the file with
the unmatched build-id is simply being skipped, no?

And why do this at 'perf probe -l' time? I.e. at that point whatever
probes that are in place already have all the needed debug info?

I.e. the warning should be done at probe creation time only?

- Arnaldo

> E.g. on such environment, you will see below warning.
> ======
> # perf probe -l
> WARN: There is a build-id mismatch between
> /usr/lib/debug/usr/lib64/libc-2.25.so.debug
> and
> /usr/lib64/libc-2.25.so
> Please check your system's debuginfo files for mismatches, e.g. check
> the versions for the target package and debuginfo package.
> probe_libc:malloc_get_state (on malloc_get_state@GLIBC_2.2.5 in /us
> r/lib64/libc-2.25.so)
> ======
>
> Signed-off-by: Masami Hiramatsu <[email protected]>
> ---
> tools/perf/util/probe-finder.c | 18 ++++++++++++++++++
> 1 file changed, 18 insertions(+)
>
> diff --git a/tools/perf/util/probe-finder.c b/tools/perf/util/probe-finder.c
> index a5731de0e5eb..5bb71e056b21 100644
> --- a/tools/perf/util/probe-finder.c
> +++ b/tools/perf/util/probe-finder.c
> @@ -119,9 +119,11 @@ enum dso_binary_type distro_dwarf_types[] = {
>
> struct debuginfo *debuginfo__new(const char *path)
> {
> + u8 bid[BUILD_ID_SIZE], bid2[BUILD_ID_SIZE];
> enum dso_binary_type *type;
> char buf[PATH_MAX], nil = '\0';
> struct dso *dso;
> + bool have_build_id = false;
> struct debuginfo *dinfo = NULL;
>
> /* Try to open distro debuginfo files */
> @@ -129,12 +131,28 @@ struct debuginfo *debuginfo__new(const char *path)
> if (!dso)
> goto out;
>
> + if (filename__read_build_id(path, bid, BUILD_ID_SIZE) > 0)
> + have_build_id = true;
> +
> for (type = distro_dwarf_types;
> !dinfo && *type != DSO_BINARY_TYPE__NOT_FOUND;
> type++) {
> if (dso__read_binary_type_filename(dso, *type, &nil,
> buf, PATH_MAX) < 0)
> continue;
> +
> + if (have_build_id) {
> + /* This can be fail because the file doesn't exist */
> + if (filename__read_build_id(buf, bid2,
> + BUILD_ID_SIZE) < 0)
> + continue;
> + if (memcmp(bid, bid2, BUILD_ID_SIZE)) {
> + pr_warning("WARN: There is a build-id mismatch between\n %s\n and\n %s\n"
> + "Please check your system's debuginfo files for mismatches, e.g. check the "
> + "versions for the target package and debuginfo package.\n", buf, path);
> + continue;
> + }
> + }
> dinfo = __debuginfo__new(buf);
> }
> dso__put(dso);

2018-01-06 14:11:22

by Masami Hiramatsu

[permalink] [raw]
Subject: Re: [PATCH 1/2] perf-probe: Ensure debuginfo's build-id is correct

On Thu, 4 Jan 2018 13:17:28 -0300
Arnaldo Carvalho de Melo <[email protected]> wrote:

> Em Mon, Dec 18, 2017 at 04:29:03PM +0900, Masami Hiramatsu escreveu:
> > Ensure that the build-id of debuginfo is correctly
> > matched to target build-id, if not, it warns user
> > to check the system debuginfo package is correctly
> > installed.
>
> So we look at a variety of files looking for one that has a matching
> build-id, I think the warning message should state that the file with
> the unmatched build-id is simply being skipped, no?

That is a corner case. In most cases, the debuginfo file will not be
found and silently skipped. We warn only if the debuginfo file exist,
but the version is not matched.

> And why do this at 'perf probe -l' time? I.e. at that point whatever
> probes that are in place already have all the needed debug info?

This is shown not only for perf probe -l, but also other debuginfo
usecases. So, if you installed an old debuginfo but has same filename
with new one, this message will be shown if you try perf probe -a func:line etc.


>
> I.e. the warning should be done at probe creation time only?

I think both are good, because if you see this warning, you will
incorrectly install debuginfo files or perf's debuginfo detection
doesn't work correctly on that environment (e.g. new distro).

Thank you,

>
> - Arnaldo
>
> > E.g. on such environment, you will see below warning.
> > ======
> > # perf probe -l
> > WARN: There is a build-id mismatch between
> > /usr/lib/debug/usr/lib64/libc-2.25.so.debug
> > and
> > /usr/lib64/libc-2.25.so
> > Please check your system's debuginfo files for mismatches, e.g. check
> > the versions for the target package and debuginfo package.
> > probe_libc:malloc_get_state (on malloc_get_state@GLIBC_2.2.5 in /us
> > r/lib64/libc-2.25.so)
> > ======
> >
> > Signed-off-by: Masami Hiramatsu <[email protected]>
> > ---
> > tools/perf/util/probe-finder.c | 18 ++++++++++++++++++
> > 1 file changed, 18 insertions(+)
> >
> > diff --git a/tools/perf/util/probe-finder.c b/tools/perf/util/probe-finder.c
> > index a5731de0e5eb..5bb71e056b21 100644
> > --- a/tools/perf/util/probe-finder.c
> > +++ b/tools/perf/util/probe-finder.c
> > @@ -119,9 +119,11 @@ enum dso_binary_type distro_dwarf_types[] = {
> >
> > struct debuginfo *debuginfo__new(const char *path)
> > {
> > + u8 bid[BUILD_ID_SIZE], bid2[BUILD_ID_SIZE];
> > enum dso_binary_type *type;
> > char buf[PATH_MAX], nil = '\0';
> > struct dso *dso;
> > + bool have_build_id = false;
> > struct debuginfo *dinfo = NULL;
> >
> > /* Try to open distro debuginfo files */
> > @@ -129,12 +131,28 @@ struct debuginfo *debuginfo__new(const char *path)
> > if (!dso)
> > goto out;
> >
> > + if (filename__read_build_id(path, bid, BUILD_ID_SIZE) > 0)
> > + have_build_id = true;
> > +
> > for (type = distro_dwarf_types;
> > !dinfo && *type != DSO_BINARY_TYPE__NOT_FOUND;
> > type++) {
> > if (dso__read_binary_type_filename(dso, *type, &nil,
> > buf, PATH_MAX) < 0)
> > continue;
> > +
> > + if (have_build_id) {
> > + /* This can be fail because the file doesn't exist */
> > + if (filename__read_build_id(buf, bid2,
> > + BUILD_ID_SIZE) < 0)
> > + continue;
> > + if (memcmp(bid, bid2, BUILD_ID_SIZE)) {
> > + pr_warning("WARN: There is a build-id mismatch between\n %s\n and\n %s\n"
> > + "Please check your system's debuginfo files for mismatches, e.g. check the "
> > + "versions for the target package and debuginfo package.\n", buf, path);
> > + continue;
> > + }
> > + }
> > dinfo = __debuginfo__new(buf);
> > }
> > dso__put(dso);


--
Masami Hiramatsu <[email protected]>

2018-01-11 12:31:27

by Masami Hiramatsu

[permalink] [raw]
Subject: Re: [PATCH 1/2] perf-probe: Ensure debuginfo's build-id is correct

Hi Arnaldo,

On Thu, 4 Jan 2018 13:17:28 -0300
Arnaldo Carvalho de Melo <[email protected]> wrote:

> Em Mon, Dec 18, 2017 at 04:29:03PM +0900, Masami Hiramatsu escreveu:
> > Ensure that the build-id of debuginfo is correctly
> > matched to target build-id, if not, it warns user
> > to check the system debuginfo package is correctly
> > installed.
>
> So we look at a variety of files looking for one that has a matching
> build-id, I think the warning message should state that the file with
> the unmatched build-id is simply being skipped, no?

Actually, this patch series came from your request that you faced
buildid mismatch between running binary and debuginfo binary, and
you asked me to show the message to tell user what he needs to
check.

>
> And why do this at 'perf probe -l' time? I.e. at that point whatever
> probes that are in place already have all the needed debug info?

As I pointed, this is not only for perf-probe -l but also perf-probe -a
(or any other operation which uses debuginfo, like -V, -L). If user
see this message at "perf probe -l", he might install debuginfo after
he setup probes. Otherwise, he should see this message when he tries
to setup probes.

BTW, this message is actually shown only if the binary has build-id.
If either file doesn't have build-id, it is just ignored.

Thank you,

>
> I.e. the warning should be done at probe creation time only?
>
> - Arnaldo
>
> > E.g. on such environment, you will see below warning.
> > ======
> > # perf probe -l
> > WARN: There is a build-id mismatch between
> > /usr/lib/debug/usr/lib64/libc-2.25.so.debug
> > and
> > /usr/lib64/libc-2.25.so
> > Please check your system's debuginfo files for mismatches, e.g. check
> > the versions for the target package and debuginfo package.
> > probe_libc:malloc_get_state (on malloc_get_state@GLIBC_2.2.5 in /us
> > r/lib64/libc-2.25.so)
> > ======
> >
> > Signed-off-by: Masami Hiramatsu <[email protected]>
> > ---
> > tools/perf/util/probe-finder.c | 18 ++++++++++++++++++
> > 1 file changed, 18 insertions(+)
> >
> > diff --git a/tools/perf/util/probe-finder.c b/tools/perf/util/probe-finder.c
> > index a5731de0e5eb..5bb71e056b21 100644
> > --- a/tools/perf/util/probe-finder.c
> > +++ b/tools/perf/util/probe-finder.c
> > @@ -119,9 +119,11 @@ enum dso_binary_type distro_dwarf_types[] = {
> >
> > struct debuginfo *debuginfo__new(const char *path)
> > {
> > + u8 bid[BUILD_ID_SIZE], bid2[BUILD_ID_SIZE];
> > enum dso_binary_type *type;
> > char buf[PATH_MAX], nil = '\0';
> > struct dso *dso;
> > + bool have_build_id = false;
> > struct debuginfo *dinfo = NULL;
> >
> > /* Try to open distro debuginfo files */
> > @@ -129,12 +131,28 @@ struct debuginfo *debuginfo__new(const char *path)
> > if (!dso)
> > goto out;
> >
> > + if (filename__read_build_id(path, bid, BUILD_ID_SIZE) > 0)
> > + have_build_id = true;
> > +
> > for (type = distro_dwarf_types;
> > !dinfo && *type != DSO_BINARY_TYPE__NOT_FOUND;
> > type++) {
> > if (dso__read_binary_type_filename(dso, *type, &nil,
> > buf, PATH_MAX) < 0)
> > continue;
> > +
> > + if (have_build_id) {
> > + /* This can be fail because the file doesn't exist */
> > + if (filename__read_build_id(buf, bid2,
> > + BUILD_ID_SIZE) < 0)
> > + continue;
> > + if (memcmp(bid, bid2, BUILD_ID_SIZE)) {
> > + pr_warning("WARN: There is a build-id mismatch between\n %s\n and\n %s\n"
> > + "Please check your system's debuginfo files for mismatches, e.g. check the "
> > + "versions for the target package and debuginfo package.\n", buf, path);
> > + continue;
> > + }
> > + }
> > dinfo = __debuginfo__new(buf);
> > }
> > dso__put(dso);


--
Masami Hiramatsu <[email protected]>

2018-01-11 14:59:36

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: Re: [PATCH 1/2] perf-probe: Ensure debuginfo's build-id is correct

Em Thu, Jan 11, 2018 at 09:31:19PM +0900, Masami Hiramatsu escreveu:
> Hi Arnaldo,
>
> On Thu, 4 Jan 2018 13:17:28 -0300
> Arnaldo Carvalho de Melo <[email protected]> wrote:
>
> > Em Mon, Dec 18, 2017 at 04:29:03PM +0900, Masami Hiramatsu escreveu:
> > > Ensure that the build-id of debuginfo is correctly
> > > matched to target build-id, if not, it warns user
> > > to check the system debuginfo package is correctly
> > > installed.
> >
> > So we look at a variety of files looking for one that has a matching
> > build-id, I think the warning message should state that the file with
> > the unmatched build-id is simply being skipped, no?
>
> Actually, this patch series came from your request that you faced
> buildid mismatch between running binary and debuginfo binary, and
> you asked me to show the message to tell user what he needs to
> check.
>
> >
> > And why do this at 'perf probe -l' time? I.e. at that point whatever
> > probes that are in place already have all the needed debug info?
>
> As I pointed, this is not only for perf-probe -l but also perf-probe -a

yeah, I recently noticed this while doind a gdb session, where it warned
about buildid mismatches.

I'll apply your patch, thanks for working on it!

- Arnaldo

> (or any other operation which uses debuginfo, like -V, -L). If user
> see this message at "perf probe -l", he might install debuginfo after
> he setup probes. Otherwise, he should see this message when he tries
> to setup probes.
>
> BTW, this message is actually shown only if the binary has build-id.
> If either file doesn't have build-id, it is just ignored.
>
> Thank you,
>
> >
> > I.e. the warning should be done at probe creation time only?
> >
> > - Arnaldo
> >
> > > E.g. on such environment, you will see below warning.
> > > ======
> > > # perf probe -l
> > > WARN: There is a build-id mismatch between
> > > /usr/lib/debug/usr/lib64/libc-2.25.so.debug
> > > and
> > > /usr/lib64/libc-2.25.so
> > > Please check your system's debuginfo files for mismatches, e.g. check
> > > the versions for the target package and debuginfo package.
> > > probe_libc:malloc_get_state (on malloc_get_state@GLIBC_2.2.5 in /us
> > > r/lib64/libc-2.25.so)
> > > ======
> > >
> > > Signed-off-by: Masami Hiramatsu <[email protected]>
> > > ---
> > > tools/perf/util/probe-finder.c | 18 ++++++++++++++++++
> > > 1 file changed, 18 insertions(+)
> > >
> > > diff --git a/tools/perf/util/probe-finder.c b/tools/perf/util/probe-finder.c
> > > index a5731de0e5eb..5bb71e056b21 100644
> > > --- a/tools/perf/util/probe-finder.c
> > > +++ b/tools/perf/util/probe-finder.c
> > > @@ -119,9 +119,11 @@ enum dso_binary_type distro_dwarf_types[] = {
> > >
> > > struct debuginfo *debuginfo__new(const char *path)
> > > {
> > > + u8 bid[BUILD_ID_SIZE], bid2[BUILD_ID_SIZE];
> > > enum dso_binary_type *type;
> > > char buf[PATH_MAX], nil = '\0';
> > > struct dso *dso;
> > > + bool have_build_id = false;
> > > struct debuginfo *dinfo = NULL;
> > >
> > > /* Try to open distro debuginfo files */
> > > @@ -129,12 +131,28 @@ struct debuginfo *debuginfo__new(const char *path)
> > > if (!dso)
> > > goto out;
> > >
> > > + if (filename__read_build_id(path, bid, BUILD_ID_SIZE) > 0)
> > > + have_build_id = true;
> > > +
> > > for (type = distro_dwarf_types;
> > > !dinfo && *type != DSO_BINARY_TYPE__NOT_FOUND;
> > > type++) {
> > > if (dso__read_binary_type_filename(dso, *type, &nil,
> > > buf, PATH_MAX) < 0)
> > > continue;
> > > +
> > > + if (have_build_id) {
> > > + /* This can be fail because the file doesn't exist */
> > > + if (filename__read_build_id(buf, bid2,
> > > + BUILD_ID_SIZE) < 0)
> > > + continue;
> > > + if (memcmp(bid, bid2, BUILD_ID_SIZE)) {
> > > + pr_warning("WARN: There is a build-id mismatch between\n %s\n and\n %s\n"
> > > + "Please check your system's debuginfo files for mismatches, e.g. check the "
> > > + "versions for the target package and debuginfo package.\n", buf, path);
> > > + continue;
> > > + }
> > > + }
> > > dinfo = __debuginfo__new(buf);
> > > }
> > > dso__put(dso);
>
>
> --
> Masami Hiramatsu <[email protected]>

2018-01-11 15:06:36

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: Re: [PATCH 1/2] perf-probe: Ensure debuginfo's build-id is correct

Em Thu, Jan 11, 2018 at 11:59:32AM -0300, Arnaldo Carvalho de Melo escreveu:
> Em Thu, Jan 11, 2018 at 09:31:19PM +0900, Masami Hiramatsu escreveu:
> > On Thu, 4 Jan 2018 13:17:28 -0300
> > Arnaldo Carvalho de Melo <[email protected]> wrote:
> > > Em Mon, Dec 18, 2017 at 04:29:03PM +0900, Masami Hiramatsu escreveu:
> > > > Ensure that the build-id of debuginfo is correctly matched to
> > > > target build-id, if not, it warns user to check the system
> > > > debuginfo package is correctly installed.

> > > So we look at a variety of files looking for one that has a matching
> > > build-id, I think the warning message should state that the file with
> > > the unmatched build-id is simply being skipped, no?

> > Actually, this patch series came from your request that you faced
> > buildid mismatch between running binary and debuginfo binary, and
> > you asked me to show the message to tell user what he needs to
> > check.

> > > And why do this at 'perf probe -l' time? I.e. at that point whatever
> > > probes that are in place already have all the needed debug info?

> > As I pointed, this is not only for perf-probe -l but also perf-probe -a
>
> yeah, I recently noticed this while doind a gdb session, where it warned
> about buildid mismatches.

> I'll apply your patch, thanks for working on it!

Trying to test it, got no warning for the vmlinux case, need to
investigate, but have not time right now, so, just for documenting it
here and letting you know:

[root@jouet ~]# perf probe $verbose "vfs_getname=getname_flags:72 pathname=result->name:string"
Added new event:
probe:vfs_getname (on getname_flags:72 with pathname=result->name:string)

You can now use it in all perf tools, such as:

perf record -e probe:vfs_getname -aR sleep 1

[root@jouet ~]# perf probe -l
probe:vfs_getname (on getname_flags:72@acme/git/linux/fs/namei.c with pathname)
[root@jouet ~]#
[root@jouet ~]# uname -a
Linux jouet 4.15.0-rc7+ #6 SMP Thu Jan 11 11:39:11 -03 2018 x86_64 x86_64 x86_64 GNU/Linux
[root@jouet ~]# mv /lib/modules/4.15.0-rc7+/build/vmlinux /lib/modules/4.15.0-rc7+/build/vmlinux.OFF
[root@jouet ~]# cp /lib/modules/4.14.0+/build/vmlinux /lib/modules/4.15.0-rc7+/build/vmlinux
[root@jouet ~]# perf probe -l
probe:vfs_getname (on getname_flags+147 with pathname)
[root@jouet ~]# perf probe -v -l
Opening /sys/kernel/debug/tracing//kprobe_events write=0
Opening /sys/kernel/debug/tracing//uprobe_events write=0
Parsing probe_events: p:probe/vfs_getname _text+2601267 pathname=+0(+0(%bx)):string
Group:probe Event:vfs_getname probe:p
Looking at the vmlinux_path (8 entries long)
symsrc__init: build id mismatch for /lib/modules/4.15.0-rc7+/build/vmlinux.
symsrc__init: cannot get elf header.
Using /proc/kcore for kernel object code
Using /proc/kallsyms for symbols
try to find information at 27b133 in kernel
Looking at the vmlinux_path (8 entries long)
symsrc__init: build id mismatch for /lib/modules/4.15.0-rc7+/build/vmlinux.
Failed to find the path for kernel: No such file or directory
Failed to find corresponding probes from debuginfo.
probe:vfs_getname (on getname_flags+147 with pathname)
[root@jouet ~]#

- Arnaldo

2018-01-11 16:21:24

by Masami Hiramatsu

[permalink] [raw]
Subject: Re: [PATCH 1/2] perf-probe: Ensure debuginfo's build-id is correct

On Thu, 11 Jan 2018 12:06:31 -0300
Arnaldo Carvalho de Melo <[email protected]> wrote:

> Em Thu, Jan 11, 2018 at 11:59:32AM -0300, Arnaldo Carvalho de Melo escreveu:
> > Em Thu, Jan 11, 2018 at 09:31:19PM +0900, Masami Hiramatsu escreveu:
> > > On Thu, 4 Jan 2018 13:17:28 -0300
> > > Arnaldo Carvalho de Melo <[email protected]> wrote:
> > > > Em Mon, Dec 18, 2017 at 04:29:03PM +0900, Masami Hiramatsu escreveu:
> > > > > Ensure that the build-id of debuginfo is correctly matched to
> > > > > target build-id, if not, it warns user to check the system
> > > > > debuginfo package is correctly installed.
>
> > > > So we look at a variety of files looking for one that has a matching
> > > > build-id, I think the warning message should state that the file with
> > > > the unmatched build-id is simply being skipped, no?
>
> > > Actually, this patch series came from your request that you faced
> > > buildid mismatch between running binary and debuginfo binary, and
> > > you asked me to show the message to tell user what he needs to
> > > check.
>
> > > > And why do this at 'perf probe -l' time? I.e. at that point whatever
> > > > probes that are in place already have all the needed debug info?
>
> > > As I pointed, this is not only for perf-probe -l but also perf-probe -a
> >
> > yeah, I recently noticed this while doind a gdb session, where it warned
> > about buildid mismatches.
>
> > I'll apply your patch, thanks for working on it!
>
> Trying to test it, got no warning for the vmlinux case, need to
> investigate, but have not time right now, so, just for documenting it
> here and letting you know:

Thank you for reporting! hmm, I just tested glibc. So I need to
recheck with vmlinux case. Tomorrow I'll investigate it.

Thanks!

>
> [root@jouet ~]# perf probe $verbose "vfs_getname=getname_flags:72 pathname=result->name:string"
> Added new event:
> probe:vfs_getname (on getname_flags:72 with pathname=result->name:string)
>
> You can now use it in all perf tools, such as:
>
> perf record -e probe:vfs_getname -aR sleep 1
>
> [root@jouet ~]# perf probe -l
> probe:vfs_getname (on getname_flags:72@acme/git/linux/fs/namei.c with pathname)
> [root@jouet ~]#
> [root@jouet ~]# uname -a
> Linux jouet 4.15.0-rc7+ #6 SMP Thu Jan 11 11:39:11 -03 2018 x86_64 x86_64 x86_64 GNU/Linux
> [root@jouet ~]# mv /lib/modules/4.15.0-rc7+/build/vmlinux /lib/modules/4.15.0-rc7+/build/vmlinux.OFF
> [root@jouet ~]# cp /lib/modules/4.14.0+/build/vmlinux /lib/modules/4.15.0-rc7+/build/vmlinux
> [root@jouet ~]# perf probe -l
> probe:vfs_getname (on getname_flags+147 with pathname)
> [root@jouet ~]# perf probe -v -l
> Opening /sys/kernel/debug/tracing//kprobe_events write=0
> Opening /sys/kernel/debug/tracing//uprobe_events write=0
> Parsing probe_events: p:probe/vfs_getname _text+2601267 pathname=+0(+0(%bx)):string
> Group:probe Event:vfs_getname probe:p
> Looking at the vmlinux_path (8 entries long)
> symsrc__init: build id mismatch for /lib/modules/4.15.0-rc7+/build/vmlinux.
> symsrc__init: cannot get elf header.
> Using /proc/kcore for kernel object code
> Using /proc/kallsyms for symbols
> try to find information at 27b133 in kernel
> Looking at the vmlinux_path (8 entries long)
> symsrc__init: build id mismatch for /lib/modules/4.15.0-rc7+/build/vmlinux.
> Failed to find the path for kernel: No such file or directory
> Failed to find corresponding probes from debuginfo.
> probe:vfs_getname (on getname_flags+147 with pathname)
> [root@jouet ~]#
>
> - Arnaldo


--
Masami Hiramatsu <[email protected]>