[PATCH v2 08/10] perf symbol: Make a common sysfs__read_build_id

[Ian Rogers <irogers@google.com>]

The libelf version of sysfs__read_build_id would read through the
file, the symbol-minimal version would read the whole file and then
use regular build-id parsing. Given we prefer the libelf code, make
both implementations use the libelf implementation and not require
reading the whole file. As the libelf implementation uses libelf
structs, move the code to symbol-minimal and use the
style/implementation that exists there already in the in memory
read_build_id code. Note, this means we could avoid reading phdrs
where read_build_id is used and switch to read_build_id_fd to
potentially avoid some memory allocation.

Clean up how bf was being used as it bound checked bf when a build ID
note was found but not when the name wasn't "GNU". Switch to just
lseek-ing forward the file to avoid any buffer overrun problems and
unnecessary reading.

Reduce scope of NOTE_ALIGN macro in perf-libelf.h.

Signed-off-by: Ian Rogers <irogers@google.com>
---
 tools/perf/util/perf-libelf.c    |  5 ++
 tools/perf/util/perf-libelf.h    |  5 --
 tools/perf/util/symbol-elf.c     | 50 --------------------
 tools/perf/util/symbol-minimal.c | 80 +++++++++++++++++++++++---------
 tools/perf/util/symbol-minimal.h |  1 +
 tools/perf/util/symbol.c         |  6 +++
 6 files changed, 69 insertions(+), 78 deletions(-)

diff --git a/tools/perf/util/perf-libelf.c b/tools/perf/util/perf-libelf.c
index a8a8c39056da..4198f4dfad9f 100644
--- a/tools/perf/util/perf-libelf.c
+++ b/tools/perf/util/perf-libelf.c
@@ -9,6 +9,11 @@
 #include "build-id.h"
 #include "debug.h"
 
+/*
+ * Align offset to 4 bytes as needed for note name and descriptor data.
+ */
+#define NOTE_ALIGN(n) (((n) + 3) & -4U)
+
 Elf_Scn *elf_section_by_name(Elf *elf, GElf_Ehdr *ep,
 			     GElf_Shdr *shp, const char *name, size_t *idx)
 {
diff --git a/tools/perf/util/perf-libelf.h b/tools/perf/util/perf-libelf.h
index 167679f9fc9b..2118325c04e5 100644
--- a/tools/perf/util/perf-libelf.h
+++ b/tools/perf/util/perf-libelf.h
@@ -21,11 +21,6 @@ struct build_id;
 # define PERF_ELF_C_READ_MMAP ELF_C_READ
 #endif
 
-/*
- * Align offset to 4 bytes as needed for note name and descriptor data.
- */
-#define NOTE_ALIGN(n) (((n) + 3) & -4U)
-
 Elf_Scn *elf_section_by_name(Elf *elf, GElf_Ehdr *ep, GElf_Shdr *shp, const char *name,
 			     size_t *idx);
 
diff --git a/tools/perf/util/symbol-elf.c b/tools/perf/util/symbol-elf.c
index 9ceb746b6c59..cb890de31044 100644
--- a/tools/perf/util/symbol-elf.c
+++ b/tools/perf/util/symbol-elf.c
@@ -751,56 +751,6 @@ int dso__synthesize_plt_symbols(struct dso *dso, struct symsrc *ss)
 	return 0;
 }
 
-int sysfs__read_build_id(const char *filename, struct build_id *bid)
-{
-	size_t size = sizeof(bid->data);
-	int fd, err = -1;
-
-	fd = open(filename, O_RDONLY);
-	if (fd < 0)
-		goto out;
-
-	while (1) {
-		char bf[BUFSIZ];
-		GElf_Nhdr nhdr;
-		size_t namesz, descsz;
-
-		if (read(fd, &nhdr, sizeof(nhdr)) != sizeof(nhdr))
-			break;
-
-		namesz = NOTE_ALIGN(nhdr.n_namesz);
-		descsz = NOTE_ALIGN(nhdr.n_descsz);
-		if (nhdr.n_type == NT_GNU_BUILD_ID &&
-		    nhdr.n_namesz == sizeof("GNU")) {
-			if (read(fd, bf, namesz) != (ssize_t)namesz)
-				break;
-			if (memcmp(bf, "GNU", sizeof("GNU")) == 0) {
-				size_t sz = min(descsz, size);
-				if (read(fd, bid->data, sz) == (ssize_t)sz) {
-					memset(bid->data + sz, 0, size - sz);
-					bid->size = sz;
-					err = 0;
-					break;
-				}
-			} else if (read(fd, bf, descsz) != (ssize_t)descsz)
-				break;
-		} else {
-			int n = namesz + descsz;
-
-			if (n > (int)sizeof(bf)) {
-				n = sizeof(bf);
-				pr_debug("%s: truncating reading of build id in sysfs file %s: n_namesz=%u, n_descsz=%u.\n",
-					 __func__, filename, nhdr.n_namesz, nhdr.n_descsz);
-			}
-			if (read(fd, bf, n) != n)
-				break;
-		}
-	}
-	close(fd);
-out:
-	return err;
-}
-
 bool symsrc__possibly_runtime(struct symsrc *ss)
 {
 	return ss->dynsym || ss->opdsec;
diff --git a/tools/perf/util/symbol-minimal.c b/tools/perf/util/symbol-minimal.c
index 445307f230af..0e3a27dae9d6 100644
--- a/tools/perf/util/symbol-minimal.c
+++ b/tools/perf/util/symbol-minimal.c
@@ -77,6 +77,58 @@ static int read_build_id(void *note_data, size_t note_len, struct build_id *bid,
 	return -1;
 }
 
+static int read_build_id_fd(int fd, struct build_id *bid, bool need_swap)
+{
+	size_t size = sizeof(bid->data);
+	struct {
+		u32 n_namesz;
+		u32 n_descsz;
+		u32 n_type;
+	} nhdr;
+
+	while (true) {
+		size_t namesz, descsz, n;
+		ssize_t err;
+
+		err = read(fd, &nhdr, sizeof(nhdr));
+		if (err != sizeof(nhdr))
+			return err == -1 ? -errno : -EIO;
+
+		if (need_swap) {
+			nhdr.n_namesz = bswap_32(nhdr.n_namesz);
+			nhdr.n_descsz = bswap_32(nhdr.n_descsz);
+			nhdr.n_type = bswap_32(nhdr.n_type);
+		}
+		namesz = NOTE_ALIGN(nhdr.n_namesz);
+		descsz = NOTE_ALIGN(nhdr.n_descsz);
+		n = namesz + descsz; /* Remaining bytes of this note. */
+
+		if (nhdr.n_type == NT_GNU_BUILD_ID && nhdr.n_namesz == sizeof("GNU")) {
+			char name[NOTE_ALIGN(sizeof("GNU"))];
+
+			err = read(fd, name, sizeof(name));
+			if (err != sizeof(name))
+				return err == -1 ? -errno : -EIO;
+
+			n = descsz;
+			if (memcmp(name, "GNU", sizeof("GNU")) == 0) {
+				/* Successfully found build ID. */
+				ssize_t sz = min(descsz, size);
+
+				err = read(fd, bid->data, sz);
+				if (err != sz)
+					return err == -1 ? -errno : -EIO;
+
+				memset(bid->data + sz, 0, size - sz);
+				bid->size = sz;
+				return 0;
+			}
+		}
+		if (lseek(fd, n, SEEK_CUR) == -1)
+			return -errno;
+	}
+}
+
 /*
  * Just try PT_NOTE header otherwise fails
  */
@@ -194,38 +246,20 @@ int sym_min__read_build_id(int _fd, const char *filename, struct build_id *bid)
 	return ret;
 }
 
-#ifndef HAVE_LIBELF_SUPPORT
-int sysfs__read_build_id(const char *filename, struct build_id *bid)
+int sym_min_sysfs__read_build_id(const char *filename, struct build_id *bid)
 {
-	int fd;
-	int ret = -1;
-	struct stat stbuf;
-	size_t buf_size;
-	void *buf;
+	int fd, ret;
 
 	fd = open(filename, O_RDONLY);
 	if (fd < 0)
-		return -1;
-
-	if (fstat(fd, &stbuf) < 0)
-		goto out;
-
-	buf_size = stbuf.st_size;
-	buf = malloc(buf_size);
-	if (buf == NULL)
-		goto out;
-
-	if (read(fd, buf, buf_size) != (ssize_t) buf_size)
-		goto out_free;
+		return -errno;
 
-	ret = read_build_id(buf, buf_size, bid, false);
-out_free:
-	free(buf);
-out:
+	ret = read_build_id_fd(fd, bid, /*need_swap=*/false);
 	close(fd);
 	return ret;
 }
 
+#ifndef HAVE_LIBELF_SUPPORT
 int symsrc__init(struct symsrc *ss, struct dso *dso, const char *name,
 	         enum dso_binary_type type)
 {
diff --git a/tools/perf/util/symbol-minimal.h b/tools/perf/util/symbol-minimal.h
index 185d98968212..5cce1f1f0f16 100644
--- a/tools/perf/util/symbol-minimal.h
+++ b/tools/perf/util/symbol-minimal.h
@@ -5,5 +5,6 @@
 struct build_id;
 
 int sym_min__read_build_id(int _fd, const char *filename, struct build_id *bid);
+int sym_min_sysfs__read_build_id(const char *filename, struct build_id *bid);
 
 #endif /* __PERF_SYMBOL_MINIMAL_H */
diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
index da75a1c159f2..76dc5b70350a 100644
--- a/tools/perf/util/symbol.c
+++ b/tools/perf/util/symbol.c
@@ -2050,6 +2050,12 @@ int filename__read_build_id(const char *filename, struct build_id *bid)
 	return err;
 }
 
+int sysfs__read_build_id(const char *filename, struct build_id *bid)
+{
+	/* Doesn't mmap file into memory. */
+	return sym_min_sysfs__read_build_id(filename, bid);
+}
+
 int filename__read_debuglink(const char *filename, char *debuglink, size_t size)
 {
 	int err = libbfd_filename__read_debuglink(filename, debuglink, size);
-- 
2.52.0.158.g65b55ccf14-goog