Re: [PATCH] perf: fix issue with binaries using 16-bytes buildids (v2)

[Stephane Eranian]

Arnaldo,

To my surprise, this patch from Last October is STILL not in.
People are reporting issues with perf related to md5 buildids..


On Fri, Oct 22, 2010 at 5:25 PM, Stephane Eranian <eranian@xxxxxxxxxx> wrote:
> Buildid can vary in size. According to the man page of ld, buildid can
> be 160 bits (sha1) or 128 bits (md5, uuid). Perf assumes buildid size
> of 20 bytes (160 bits) regardless. When dealing with md5 buildids, it
> would thus read more than needed and that would cause mismatches and
> samples without symbols.
>
> This patch fixes this by taking into account the actual buildid size
> as encoded int he section header. The leftover bytes are also cleared.
>
> This second version fixes a minor issue with the memset() base position.
>
> Signed-off-by: Stephane Eranian <eranian@xxxxxxxxxx>
>
> ---
>
> diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
> index b39f499..b4fdb91 100644
> --- a/tools/perf/util/symbol.c
> +++ b/tools/perf/util/symbol.c
> @@ -1027,8 +1027,7 @@ static int dso__load_sym(struct dso *self, struct map *map, const char *name,
> Â Â Â Âif (self->has_build_id) {
> Â Â Â Â Â Â Â Âu8 build_id[BUILD_ID_SIZE];
>
> - Â Â Â Â Â Â Â if (elf_read_build_id(elf, build_id,
> - Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â BUILD_ID_SIZE) != BUILD_ID_SIZE)
> + Â Â Â Â Â Â Â if (elf_read_build_id(elf, build_id, BUILD_ID_SIZE) < 0)
> Â Â Â Â Â Â Â Â Â Â Â Âgoto out_elf_end;
>
> Â Â Â Â Â Â Â Âif (!dso__build_id_equal(self, build_id))
> @@ -1287,8 +1286,8 @@ static int elf_read_build_id(Elf *elf, void *bf, size_t size)
> Â Â Â Âptr = data->d_buf;
> Â Â Â Âwhile (ptr < (data->d_buf + data->d_size)) {
> Â Â Â Â Â Â Â ÂGElf_Nhdr *nhdr = ptr;
> - Â Â Â Â Â Â Â int namesz = NOTE_ALIGN(nhdr->n_namesz),
> - Â Â Â Â Â Â Â Â Â descsz = NOTE_ALIGN(nhdr->n_descsz);
> + Â Â Â Â Â Â Â size_t namesz = NOTE_ALIGN(nhdr->n_namesz),
> + Â Â Â Â Â Â Â Â Â Â Âdescsz = NOTE_ALIGN(nhdr->n_descsz);
> Â Â Â Â Â Â Â Âconst char *name;
>
> Â Â Â Â Â Â Â Âptr += sizeof(*nhdr);
> @@ -1297,8 +1296,10 @@ static int elf_read_build_id(Elf *elf, void *bf, size_t size)
> Â Â Â Â Â Â Â Âif (nhdr->n_type == NT_GNU_BUILD_ID &&
> Â Â Â Â Â Â Â Â Â Ânhdr->n_namesz == sizeof("GNU")) {
> Â Â Â Â Â Â Â Â Â Â Â Âif (memcmp(name, "GNU", sizeof("GNU")) == 0) {
> - Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â memcpy(bf, ptr, BUILD_ID_SIZE);
> - Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â err = BUILD_ID_SIZE;
> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â size_t sz = min(size, descsz);
> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â memcpy(bf, ptr, sz);
> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â memset(bf + sz, 0, size - sz);
> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â err = descsz;
> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Âbreak;
> Â Â Â Â Â Â Â Â Â Â Â Â}
> Â Â Â Â Â Â Â Â}
> @@ -1350,7 +1351,7 @@ int sysfs__read_build_id(const char *filename, void *build_id, size_t size)
> Â Â Â Âwhile (1) {
> Â Â Â Â Â Â Â Âchar bf[BUFSIZ];
> Â Â Â Â Â Â Â ÂGElf_Nhdr nhdr;
> - Â Â Â Â Â Â Â int namesz, descsz;
> + Â Â Â Â Â Â Â size_t namesz, descsz;
>
> Â Â Â Â Â Â Â Âif (read(fd, &nhdr, sizeof(nhdr)) != sizeof(nhdr))
> Â Â Â Â Â Â Â Â Â Â Â Âbreak;
> @@ -1359,15 +1360,16 @@ int sysfs__read_build_id(const char *filename, void *build_id, size_t size)
> Â Â Â Â Â Â Â Âdescsz = NOTE_ALIGN(nhdr.n_descsz);
> Â Â Â Â Â Â Â Âif (nhdr.n_type == NT_GNU_BUILD_ID &&
> Â Â Â Â Â Â Â Â Â Ânhdr.n_namesz == sizeof("GNU")) {
> - Â Â Â Â Â Â Â Â Â Â Â if (read(fd, bf, namesz) != namesz)
> + Â Â Â Â Â Â Â Â Â Â Â if (read(fd, bf, namesz) != (ssize_t)namesz)
> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Âbreak;
> Â Â Â Â Â Â Â Â Â Â Â Âif (memcmp(bf, "GNU", sizeof("GNU")) == 0) {
> - Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â if (read(fd, build_id,
> - Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â BUILD_ID_SIZE) == BUILD_ID_SIZE) {
> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â size_t sz = min(descsz, size);
> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â if (read(fd, build_id, sz) == (ssize_t)sz) {
> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â memset(build_id + sz, 0, size - sz);
> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Âerr = 0;
> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Âbreak;
> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â}
> - Â Â Â Â Â Â Â Â Â Â Â } else if (read(fd, bf, descsz) != descsz)
> + Â Â Â Â Â Â Â Â Â Â Â } else if (read(fd, bf, descsz) != (ssize_t)descsz)
> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Âbreak;
> Â Â Â Â Â Â Â Â} else {
> Â Â Â Â Â Â Â Â Â Â Â Âint n = namesz + descsz;
>
N‹§²æìr¸›yúèšØb²X¬¶ÇvØ^–)Þ{.nÇ+‰·¥Š{±‘êçzX§¶›¡Ü}©ž²ÆzÚ&j:+v‰¨¾«‘êçzZ+€Ê+zf£¢·hšˆ§~†­†Ûiÿûàz¹®w¥¢¸?™¨è­Ú&¢)ßf”ù^jÇy§m…á@A«a¶Úÿ0¶ìh®å’i