* [PATCH 1/4] perf buildid-cache: Don't skip 16-byte build-ids
@ 2021-02-10 19:17 Nicholas Fraser
2021-02-15 14:31 ` Jiri Olsa
2021-02-17 19:54 ` Jiri Olsa
0 siblings, 2 replies; 7+ messages in thread
From: Nicholas Fraser @ 2021-02-10 19:17 UTC (permalink / raw)
To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Mark Rutland, Alexander Shishkin, Jiri Olsa, Namhyung Kim,
Ian Rogers, Frank Ch. Eigler, Song Liu, Adrian Hunter,
Kim Phillips, Tommi Rantala, Remi Bernon, linux-kernel
Cc: Ulrich Czekalla, Huw Davies
lsdir_bid_tail_filter() ignored any build-id that wasn't exactly 20
bytes. This worked only for SHA-1 build-ids. The build-id for a PE file
is always a 16-byte GUID and ELF files can also have MD5 or UUID
build-ids.
This fix changes the filter to allow build-ids between 16 and 20 bytes.
Signed-off-by: Nicholas Fraser <nfraser@codeweavers.com>
---
tools/perf/util/build-id.c | 5 +++--
tools/perf/util/build-id.h | 4 +++-
2 files changed, 6 insertions(+), 3 deletions(-)
diff --git a/tools/perf/util/build-id.c b/tools/perf/util/build-id.c
index 02df36b30ac5..e32e8f2ff3bd 100644
--- a/tools/perf/util/build-id.c
+++ b/tools/perf/util/build-id.c
@@ -448,7 +448,8 @@ static bool lsdir_bid_tail_filter(const char *name __maybe_unused,
int i = 0;
while (isxdigit(d->d_name[i]) && i < SBUILD_ID_SIZE - 3)
i++;
- return (i == SBUILD_ID_SIZE - 3) && (d->d_name[i] == '\0');
+ return (i >= SBUILD_ID_MIN_SIZE - 3) && (i <= SBUILD_ID_SIZE - 3) &&
+ (d->d_name[i] == '\0');
}
struct strlist *build_id_cache__list_all(bool validonly)
@@ -490,7 +491,7 @@ struct strlist *build_id_cache__list_all(bool validonly)
}
strlist__for_each_entry(nd2, linklist) {
if (snprintf(sbuild_id, SBUILD_ID_SIZE, "%s%s",
- nd->s, nd2->s) != SBUILD_ID_SIZE - 1)
+ nd->s, nd2->s) > SBUILD_ID_SIZE - 1)
goto err_out;
if (validonly && !build_id_cache__valid_id(sbuild_id))
continue;
diff --git a/tools/perf/util/build-id.h b/tools/perf/util/build-id.h
index 02613f4b2c29..c19617151670 100644
--- a/tools/perf/util/build-id.h
+++ b/tools/perf/util/build-id.h
@@ -2,8 +2,10 @@
#ifndef PERF_BUILD_ID_H_
#define PERF_BUILD_ID_H_ 1
-#define BUILD_ID_SIZE 20
+#define BUILD_ID_SIZE 20 /* SHA-1 length in bytes */
+#define BUILD_ID_MIN_SIZE 16 /* MD5/UUID/GUID length in bytes */
#define SBUILD_ID_SIZE (BUILD_ID_SIZE * 2 + 1)
+#define SBUILD_ID_MIN_SIZE (BUILD_ID_MIN_SIZE * 2 + 1)
#include "machine.h"
#include "tool.h"
--
2.30.0
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 1/4] perf buildid-cache: Don't skip 16-byte build-ids
2021-02-10 19:17 [PATCH 1/4] perf buildid-cache: Don't skip 16-byte build-ids Nicholas Fraser
@ 2021-02-15 14:31 ` Jiri Olsa
2021-02-16 16:35 ` Nicholas Fraser
2021-02-17 19:54 ` Jiri Olsa
1 sibling, 1 reply; 7+ messages in thread
From: Jiri Olsa @ 2021-02-15 14:31 UTC (permalink / raw)
To: Nicholas Fraser
Cc: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Mark Rutland, Alexander Shishkin, Namhyung Kim, Ian Rogers,
Frank Ch. Eigler, Song Liu, Adrian Hunter, Kim Phillips,
Tommi Rantala, Remi Bernon, linux-kernel, Ulrich Czekalla,
Huw Davies
On Wed, Feb 10, 2021 at 02:17:25PM -0500, Nicholas Fraser wrote:
> lsdir_bid_tail_filter() ignored any build-id that wasn't exactly 20
> bytes. This worked only for SHA-1 build-ids. The build-id for a PE file
> is always a 16-byte GUID and ELF files can also have MD5 or UUID
> build-ids.
>
> This fix changes the filter to allow build-ids between 16 and 20 bytes.
hi,
there's tests/shell/buildid.sh test for this, please add
testcase for this.. looks like perf buildid-cacle -l will
end up calling this function
thanks,
jirka
>
> Signed-off-by: Nicholas Fraser <nfraser@codeweavers.com>
> ---
> tools/perf/util/build-id.c | 5 +++--
> tools/perf/util/build-id.h | 4 +++-
> 2 files changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/tools/perf/util/build-id.c b/tools/perf/util/build-id.c
> index 02df36b30ac5..e32e8f2ff3bd 100644
> --- a/tools/perf/util/build-id.c
> +++ b/tools/perf/util/build-id.c
> @@ -448,7 +448,8 @@ static bool lsdir_bid_tail_filter(const char *name __maybe_unused,
> int i = 0;
> while (isxdigit(d->d_name[i]) && i < SBUILD_ID_SIZE - 3)
> i++;
> - return (i == SBUILD_ID_SIZE - 3) && (d->d_name[i] == '\0');
> + return (i >= SBUILD_ID_MIN_SIZE - 3) && (i <= SBUILD_ID_SIZE - 3) &&
> + (d->d_name[i] == '\0');
> }
>
> struct strlist *build_id_cache__list_all(bool validonly)
> @@ -490,7 +491,7 @@ struct strlist *build_id_cache__list_all(bool validonly)
> }
> strlist__for_each_entry(nd2, linklist) {
> if (snprintf(sbuild_id, SBUILD_ID_SIZE, "%s%s",
> - nd->s, nd2->s) != SBUILD_ID_SIZE - 1)
> + nd->s, nd2->s) > SBUILD_ID_SIZE - 1)
> goto err_out;
> if (validonly && !build_id_cache__valid_id(sbuild_id))
> continue;
> diff --git a/tools/perf/util/build-id.h b/tools/perf/util/build-id.h
> index 02613f4b2c29..c19617151670 100644
> --- a/tools/perf/util/build-id.h
> +++ b/tools/perf/util/build-id.h
> @@ -2,8 +2,10 @@
> #ifndef PERF_BUILD_ID_H_
> #define PERF_BUILD_ID_H_ 1
>
> -#define BUILD_ID_SIZE 20
> +#define BUILD_ID_SIZE 20 /* SHA-1 length in bytes */
> +#define BUILD_ID_MIN_SIZE 16 /* MD5/UUID/GUID length in bytes */
> #define SBUILD_ID_SIZE (BUILD_ID_SIZE * 2 + 1)
> +#define SBUILD_ID_MIN_SIZE (BUILD_ID_MIN_SIZE * 2 + 1)
>
> #include "machine.h"
> #include "tool.h"
> --
> 2.30.0
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/4] perf buildid-cache: Don't skip 16-byte build-ids
2021-02-15 14:31 ` Jiri Olsa
@ 2021-02-16 16:35 ` Nicholas Fraser
2021-02-17 18:19 ` Arnaldo Carvalho de Melo
2021-02-17 19:24 ` Jiri Olsa
0 siblings, 2 replies; 7+ messages in thread
From: Nicholas Fraser @ 2021-02-16 16:35 UTC (permalink / raw)
To: Jiri Olsa
Cc: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Mark Rutland, Alexander Shishkin, Namhyung Kim, Ian Rogers,
Frank Ch. Eigler, Song Liu, Adrian Hunter, Kim Phillips,
Tommi Rantala, Remi Bernon, linux-kernel, Ulrich Czekalla,
Huw Davies
No problem, I've added a call to "perf buildid-cache -l" in check() to make
sure the added IDs are reported. The MD5 test fails without the previous
patch to allow 16-byte build-ids.
Do you also want PE files tested by this test script? This would make the
script depend on a number of things:
- We'd need to change "readelf" to something that can read the build-id of
PE files;
- We'd need MinGW installed to build them on the fly the same as the ELF
test cases, although we could use tests/pe-file.exe instead which is
already included;
- We'd need Wine to actually run it under perf record, which I'm sure you
don't want to depend on;
- We'd need to detect whether perf was built with libbfd support. Easily
done by calling perf version --build-options but by this point the test
case is getting pretty out of hand for what it's supposed to test.
I'll send patches for this test without PE support. It still tests 16-byte
build-ids because it tests an MD5 ELF file. Let me know if you also want me
to add PE support to this script. We can make it optionally perform PE
tests if all of the necessary tools are installed but it would take some
pretty large changes to the script.
Nick
On 2021-02-15 9:31 a.m., Jiri Olsa wrote:
> On Wed, Feb 10, 2021 at 02:17:25PM -0500, Nicholas Fraser wrote:
>> lsdir_bid_tail_filter() ignored any build-id that wasn't exactly 20
>> bytes. This worked only for SHA-1 build-ids. The build-id for a PE file
>> is always a 16-byte GUID and ELF files can also have MD5 or UUID
>> build-ids.
>>
>> This fix changes the filter to allow build-ids between 16 and 20 bytes.
>
> hi,
> there's tests/shell/buildid.sh test for this, please add
> testcase for this.. looks like perf buildid-cacle -l will
> end up calling this function
>
> thanks,
> jirka
>
>>
>> Signed-off-by: Nicholas Fraser <nfraser@codeweavers.com>
>> ---
>> tools/perf/util/build-id.c | 5 +++--
>> tools/perf/util/build-id.h | 4 +++-
>> 2 files changed, 6 insertions(+), 3 deletions(-)
>>
>> diff --git a/tools/perf/util/build-id.c b/tools/perf/util/build-id.c
>> index 02df36b30ac5..e32e8f2ff3bd 100644
>> --- a/tools/perf/util/build-id.c
>> +++ b/tools/perf/util/build-id.c
>> @@ -448,7 +448,8 @@ static bool lsdir_bid_tail_filter(const char *name __maybe_unused,
>> int i = 0;
>> while (isxdigit(d->d_name[i]) && i < SBUILD_ID_SIZE - 3)
>> i++;
>> - return (i == SBUILD_ID_SIZE - 3) && (d->d_name[i] == '\0');
>> + return (i >= SBUILD_ID_MIN_SIZE - 3) && (i <= SBUILD_ID_SIZE - 3) &&
>> + (d->d_name[i] == '\0');
>> }
>>
>> struct strlist *build_id_cache__list_all(bool validonly)
>> @@ -490,7 +491,7 @@ struct strlist *build_id_cache__list_all(bool validonly)
>> }
>> strlist__for_each_entry(nd2, linklist) {
>> if (snprintf(sbuild_id, SBUILD_ID_SIZE, "%s%s",
>> - nd->s, nd2->s) != SBUILD_ID_SIZE - 1)
>> + nd->s, nd2->s) > SBUILD_ID_SIZE - 1)
>> goto err_out;
>> if (validonly && !build_id_cache__valid_id(sbuild_id))
>> continue;
>> diff --git a/tools/perf/util/build-id.h b/tools/perf/util/build-id.h
>> index 02613f4b2c29..c19617151670 100644
>> --- a/tools/perf/util/build-id.h
>> +++ b/tools/perf/util/build-id.h
>> @@ -2,8 +2,10 @@
>> #ifndef PERF_BUILD_ID_H_
>> #define PERF_BUILD_ID_H_ 1
>>
>> -#define BUILD_ID_SIZE 20
>> +#define BUILD_ID_SIZE 20 /* SHA-1 length in bytes */
>> +#define BUILD_ID_MIN_SIZE 16 /* MD5/UUID/GUID length in bytes */
>> #define SBUILD_ID_SIZE (BUILD_ID_SIZE * 2 + 1)
>> +#define SBUILD_ID_MIN_SIZE (BUILD_ID_MIN_SIZE * 2 + 1)
>>
>> #include "machine.h"
>> #include "tool.h"
>> --
>> 2.30.0
>>
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/4] perf buildid-cache: Don't skip 16-byte build-ids
2021-02-16 16:35 ` Nicholas Fraser
@ 2021-02-17 18:19 ` Arnaldo Carvalho de Melo
2021-02-17 19:24 ` Jiri Olsa
1 sibling, 0 replies; 7+ messages in thread
From: Arnaldo Carvalho de Melo @ 2021-02-17 18:19 UTC (permalink / raw)
To: Nicholas Fraser
Cc: Jiri Olsa, Peter Zijlstra, Ingo Molnar, Mark Rutland,
Alexander Shishkin, Namhyung Kim, Ian Rogers, Frank Ch. Eigler,
Song Liu, Adrian Hunter, Kim Phillips, Tommi Rantala,
Remi Bernon, linux-kernel, Ulrich Czekalla, Huw Davies
Em Tue, Feb 16, 2021 at 11:35:06AM -0500, Nicholas Fraser escreveu:
> No problem, I've added a call to "perf buildid-cache -l" in check() to make
> sure the added IDs are reported. The MD5 test fails without the previous
> patch to allow 16-byte build-ids.
>
> Do you also want PE files tested by this test script? This would make the
> script depend on a number of things:
>
> - We'd need to change "readelf" to something that can read the build-id of
> PE files;
>
> - We'd need MinGW installed to build them on the fly the same as the ELF
> test cases, although we could use tests/pe-file.exe instead which is
> already included;
>
> - We'd need Wine to actually run it under perf record, which I'm sure you
> don't want to depend on;
We can check if wine is installed, if not, just emit a warning. I, for
one, will install wine for this purpose.
> - We'd need to detect whether perf was built with libbfd support. Easily
> done by calling perf version --build-options but by this point the test
> case is getting pretty out of hand for what it's supposed to test.
Nope, at least a group of people need to be checking if this new code
doesn't regress, and fix it if it does.
> I'll send patches for this test without PE support. It still tests 16-byte
> build-ids because it tests an MD5 ELF file. Let me know if you also want me
> to add PE support to this script. We can make it optionally perform PE
> tests if all of the necessary tools are installed but it would take some
> pretty large changes to the script.
>
> Nick
>
>
> On 2021-02-15 9:31 a.m., Jiri Olsa wrote:
> > On Wed, Feb 10, 2021 at 02:17:25PM -0500, Nicholas Fraser wrote:
> >> lsdir_bid_tail_filter() ignored any build-id that wasn't exactly 20
> >> bytes. This worked only for SHA-1 build-ids. The build-id for a PE file
> >> is always a 16-byte GUID and ELF files can also have MD5 or UUID
> >> build-ids.
> >>
> >> This fix changes the filter to allow build-ids between 16 and 20 bytes.
> >
> > hi,
> > there's tests/shell/buildid.sh test for this, please add
> > testcase for this.. looks like perf buildid-cacle -l will
> > end up calling this function
> >
> > thanks,
> > jirka
> >
> >>
> >> Signed-off-by: Nicholas Fraser <nfraser@codeweavers.com>
> >> ---
> >> tools/perf/util/build-id.c | 5 +++--
> >> tools/perf/util/build-id.h | 4 +++-
> >> 2 files changed, 6 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/tools/perf/util/build-id.c b/tools/perf/util/build-id.c
> >> index 02df36b30ac5..e32e8f2ff3bd 100644
> >> --- a/tools/perf/util/build-id.c
> >> +++ b/tools/perf/util/build-id.c
> >> @@ -448,7 +448,8 @@ static bool lsdir_bid_tail_filter(const char *name __maybe_unused,
> >> int i = 0;
> >> while (isxdigit(d->d_name[i]) && i < SBUILD_ID_SIZE - 3)
> >> i++;
> >> - return (i == SBUILD_ID_SIZE - 3) && (d->d_name[i] == '\0');
> >> + return (i >= SBUILD_ID_MIN_SIZE - 3) && (i <= SBUILD_ID_SIZE - 3) &&
> >> + (d->d_name[i] == '\0');
> >> }
> >>
> >> struct strlist *build_id_cache__list_all(bool validonly)
> >> @@ -490,7 +491,7 @@ struct strlist *build_id_cache__list_all(bool validonly)
> >> }
> >> strlist__for_each_entry(nd2, linklist) {
> >> if (snprintf(sbuild_id, SBUILD_ID_SIZE, "%s%s",
> >> - nd->s, nd2->s) != SBUILD_ID_SIZE - 1)
> >> + nd->s, nd2->s) > SBUILD_ID_SIZE - 1)
> >> goto err_out;
> >> if (validonly && !build_id_cache__valid_id(sbuild_id))
> >> continue;
> >> diff --git a/tools/perf/util/build-id.h b/tools/perf/util/build-id.h
> >> index 02613f4b2c29..c19617151670 100644
> >> --- a/tools/perf/util/build-id.h
> >> +++ b/tools/perf/util/build-id.h
> >> @@ -2,8 +2,10 @@
> >> #ifndef PERF_BUILD_ID_H_
> >> #define PERF_BUILD_ID_H_ 1
> >>
> >> -#define BUILD_ID_SIZE 20
> >> +#define BUILD_ID_SIZE 20 /* SHA-1 length in bytes */
> >> +#define BUILD_ID_MIN_SIZE 16 /* MD5/UUID/GUID length in bytes */
> >> #define SBUILD_ID_SIZE (BUILD_ID_SIZE * 2 + 1)
> >> +#define SBUILD_ID_MIN_SIZE (BUILD_ID_MIN_SIZE * 2 + 1)
> >>
> >> #include "machine.h"
> >> #include "tool.h"
> >> --
> >> 2.30.0
> >>
> >
--
- Arnaldo
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/4] perf buildid-cache: Don't skip 16-byte build-ids
2021-02-16 16:35 ` Nicholas Fraser
2021-02-17 18:19 ` Arnaldo Carvalho de Melo
@ 2021-02-17 19:24 ` Jiri Olsa
1 sibling, 0 replies; 7+ messages in thread
From: Jiri Olsa @ 2021-02-17 19:24 UTC (permalink / raw)
To: Nicholas Fraser
Cc: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Mark Rutland, Alexander Shishkin, Namhyung Kim, Ian Rogers,
Frank Ch. Eigler, Song Liu, Adrian Hunter, Kim Phillips,
Tommi Rantala, Remi Bernon, linux-kernel, Ulrich Czekalla,
Huw Davies
On Tue, Feb 16, 2021 at 11:35:06AM -0500, Nicholas Fraser wrote:
> No problem, I've added a call to "perf buildid-cache -l" in check() to make
> sure the added IDs are reported. The MD5 test fails without the previous
> patch to allow 16-byte build-ids.
>
> Do you also want PE files tested by this test script? This would make the
> script depend on a number of things:
>
> - We'd need to change "readelf" to something that can read the build-id of
> PE files;
>
> - We'd need MinGW installed to build them on the fly the same as the ELF
> test cases, although we could use tests/pe-file.exe instead which is
> already included;
>
> - We'd need Wine to actually run it under perf record, which I'm sure you
> don't want to depend on;
>
> - We'd need to detect whether perf was built with libbfd support. Easily
> done by calling perf version --build-options but by this point the test
> case is getting pretty out of hand for what it's supposed to test.
right, that's a lot of dependencies ;-) but I don't mind having
separate script (PE specific) checking on all that, and printing
'SKIP' if some dependency is not there
I guess it will take some time to configure the system, but it will
make sure we won't break anything for you in future
thanks,
jirka
>
> I'll send patches for this test without PE support. It still tests 16-byte
> build-ids because it tests an MD5 ELF file. Let me know if you also want me
> to add PE support to this script. We can make it optionally perform PE
> tests if all of the necessary tools are installed but it would take some
> pretty large changes to the script.
>
> Nick
>
>
> On 2021-02-15 9:31 a.m., Jiri Olsa wrote:
> > On Wed, Feb 10, 2021 at 02:17:25PM -0500, Nicholas Fraser wrote:
> >> lsdir_bid_tail_filter() ignored any build-id that wasn't exactly 20
> >> bytes. This worked only for SHA-1 build-ids. The build-id for a PE file
> >> is always a 16-byte GUID and ELF files can also have MD5 or UUID
> >> build-ids.
> >>
> >> This fix changes the filter to allow build-ids between 16 and 20 bytes.
> >
> > hi,
> > there's tests/shell/buildid.sh test for this, please add
> > testcase for this.. looks like perf buildid-cacle -l will
> > end up calling this function
> >
> > thanks,
> > jirka
> >
> >>
> >> Signed-off-by: Nicholas Fraser <nfraser@codeweavers.com>
> >> ---
> >> tools/perf/util/build-id.c | 5 +++--
> >> tools/perf/util/build-id.h | 4 +++-
> >> 2 files changed, 6 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/tools/perf/util/build-id.c b/tools/perf/util/build-id.c
> >> index 02df36b30ac5..e32e8f2ff3bd 100644
> >> --- a/tools/perf/util/build-id.c
> >> +++ b/tools/perf/util/build-id.c
> >> @@ -448,7 +448,8 @@ static bool lsdir_bid_tail_filter(const char *name __maybe_unused,
> >> int i = 0;
> >> while (isxdigit(d->d_name[i]) && i < SBUILD_ID_SIZE - 3)
> >> i++;
> >> - return (i == SBUILD_ID_SIZE - 3) && (d->d_name[i] == '\0');
> >> + return (i >= SBUILD_ID_MIN_SIZE - 3) && (i <= SBUILD_ID_SIZE - 3) &&
> >> + (d->d_name[i] == '\0');
> >> }
> >>
> >> struct strlist *build_id_cache__list_all(bool validonly)
> >> @@ -490,7 +491,7 @@ struct strlist *build_id_cache__list_all(bool validonly)
> >> }
> >> strlist__for_each_entry(nd2, linklist) {
> >> if (snprintf(sbuild_id, SBUILD_ID_SIZE, "%s%s",
> >> - nd->s, nd2->s) != SBUILD_ID_SIZE - 1)
> >> + nd->s, nd2->s) > SBUILD_ID_SIZE - 1)
> >> goto err_out;
> >> if (validonly && !build_id_cache__valid_id(sbuild_id))
> >> continue;
> >> diff --git a/tools/perf/util/build-id.h b/tools/perf/util/build-id.h
> >> index 02613f4b2c29..c19617151670 100644
> >> --- a/tools/perf/util/build-id.h
> >> +++ b/tools/perf/util/build-id.h
> >> @@ -2,8 +2,10 @@
> >> #ifndef PERF_BUILD_ID_H_
> >> #define PERF_BUILD_ID_H_ 1
> >>
> >> -#define BUILD_ID_SIZE 20
> >> +#define BUILD_ID_SIZE 20 /* SHA-1 length in bytes */
> >> +#define BUILD_ID_MIN_SIZE 16 /* MD5/UUID/GUID length in bytes */
> >> #define SBUILD_ID_SIZE (BUILD_ID_SIZE * 2 + 1)
> >> +#define SBUILD_ID_MIN_SIZE (BUILD_ID_MIN_SIZE * 2 + 1)
> >>
> >> #include "machine.h"
> >> #include "tool.h"
> >> --
> >> 2.30.0
> >>
> >
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/4] perf buildid-cache: Don't skip 16-byte build-ids
2021-02-10 19:17 [PATCH 1/4] perf buildid-cache: Don't skip 16-byte build-ids Nicholas Fraser
2021-02-15 14:31 ` Jiri Olsa
@ 2021-02-17 19:54 ` Jiri Olsa
2021-02-18 19:42 ` Arnaldo Carvalho de Melo
1 sibling, 1 reply; 7+ messages in thread
From: Jiri Olsa @ 2021-02-17 19:54 UTC (permalink / raw)
To: Nicholas Fraser
Cc: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Mark Rutland, Alexander Shishkin, Namhyung Kim, Ian Rogers,
Frank Ch. Eigler, Song Liu, Adrian Hunter, Kim Phillips,
Tommi Rantala, Remi Bernon, linux-kernel, Ulrich Czekalla,
Huw Davies
On Wed, Feb 10, 2021 at 02:17:25PM -0500, Nicholas Fraser wrote:
> lsdir_bid_tail_filter() ignored any build-id that wasn't exactly 20
> bytes. This worked only for SHA-1 build-ids. The build-id for a PE file
> is always a 16-byte GUID and ELF files can also have MD5 or UUID
> build-ids.
>
> This fix changes the filter to allow build-ids between 16 and 20 bytes.
>
> Signed-off-by: Nicholas Fraser <nfraser@codeweavers.com>
Acked-by: Jiri Olsa <jolsa@redhat.com>
thanks,
jirka
> ---
> tools/perf/util/build-id.c | 5 +++--
> tools/perf/util/build-id.h | 4 +++-
> 2 files changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/tools/perf/util/build-id.c b/tools/perf/util/build-id.c
> index 02df36b30ac5..e32e8f2ff3bd 100644
> --- a/tools/perf/util/build-id.c
> +++ b/tools/perf/util/build-id.c
> @@ -448,7 +448,8 @@ static bool lsdir_bid_tail_filter(const char *name __maybe_unused,
> int i = 0;
> while (isxdigit(d->d_name[i]) && i < SBUILD_ID_SIZE - 3)
> i++;
> - return (i == SBUILD_ID_SIZE - 3) && (d->d_name[i] == '\0');
> + return (i >= SBUILD_ID_MIN_SIZE - 3) && (i <= SBUILD_ID_SIZE - 3) &&
> + (d->d_name[i] == '\0');
> }
>
> struct strlist *build_id_cache__list_all(bool validonly)
> @@ -490,7 +491,7 @@ struct strlist *build_id_cache__list_all(bool validonly)
> }
> strlist__for_each_entry(nd2, linklist) {
> if (snprintf(sbuild_id, SBUILD_ID_SIZE, "%s%s",
> - nd->s, nd2->s) != SBUILD_ID_SIZE - 1)
> + nd->s, nd2->s) > SBUILD_ID_SIZE - 1)
> goto err_out;
> if (validonly && !build_id_cache__valid_id(sbuild_id))
> continue;
> diff --git a/tools/perf/util/build-id.h b/tools/perf/util/build-id.h
> index 02613f4b2c29..c19617151670 100644
> --- a/tools/perf/util/build-id.h
> +++ b/tools/perf/util/build-id.h
> @@ -2,8 +2,10 @@
> #ifndef PERF_BUILD_ID_H_
> #define PERF_BUILD_ID_H_ 1
>
> -#define BUILD_ID_SIZE 20
> +#define BUILD_ID_SIZE 20 /* SHA-1 length in bytes */
> +#define BUILD_ID_MIN_SIZE 16 /* MD5/UUID/GUID length in bytes */
> #define SBUILD_ID_SIZE (BUILD_ID_SIZE * 2 + 1)
> +#define SBUILD_ID_MIN_SIZE (BUILD_ID_MIN_SIZE * 2 + 1)
>
> #include "machine.h"
> #include "tool.h"
> --
> 2.30.0
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/4] perf buildid-cache: Don't skip 16-byte build-ids
2021-02-17 19:54 ` Jiri Olsa
@ 2021-02-18 19:42 ` Arnaldo Carvalho de Melo
0 siblings, 0 replies; 7+ messages in thread
From: Arnaldo Carvalho de Melo @ 2021-02-18 19:42 UTC (permalink / raw)
To: Jiri Olsa
Cc: Nicholas Fraser, Peter Zijlstra, Ingo Molnar, Mark Rutland,
Alexander Shishkin, Namhyung Kim, Ian Rogers, Frank Ch. Eigler,
Song Liu, Adrian Hunter, Kim Phillips, Tommi Rantala,
Remi Bernon, linux-kernel, Ulrich Czekalla, Huw Davies
Em Wed, Feb 17, 2021 at 08:54:00PM +0100, Jiri Olsa escreveu:
> On Wed, Feb 10, 2021 at 02:17:25PM -0500, Nicholas Fraser wrote:
> > lsdir_bid_tail_filter() ignored any build-id that wasn't exactly 20
> > bytes. This worked only for SHA-1 build-ids. The build-id for a PE file
> > is always a 16-byte GUID and ELF files can also have MD5 or UUID
> > build-ids.
> >
> > This fix changes the filter to allow build-ids between 16 and 20 bytes.
> >
> > Signed-off-by: Nicholas Fraser <nfraser@codeweavers.com>
>
> Acked-by: Jiri Olsa <jolsa@redhat.com>
Thanks, applied.
- Arnaldo
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2021-02-18 19:59 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-02-10 19:17 [PATCH 1/4] perf buildid-cache: Don't skip 16-byte build-ids Nicholas Fraser
2021-02-15 14:31 ` Jiri Olsa
2021-02-16 16:35 ` Nicholas Fraser
2021-02-17 18:19 ` Arnaldo Carvalho de Melo
2021-02-17 19:24 ` Jiri Olsa
2021-02-17 19:54 ` Jiri Olsa
2021-02-18 19:42 ` Arnaldo Carvalho de Melo
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).