On Tue 14 Feb 2017, Matt Turner wrote:
> Provides the ability to read the .note.gnu.build-id section of ELF
> binaries, which is inserted by the --build-id=... flag to ld.
> ---
> configure.ac | 2 +
> src/util/Makefile.sources | 2 +
> src/util/build_id.c | 109
> ++++++++++++++++++++++++++++++++++++++++++++++
> src/util/build_id.h | 56 ++++++++++++++++++++++++
> 4 files changed, 169 insertions(+)
> create mode 100644 src/util/build_id.c
> create mode 100644 src/util/build_id.h
> +AC_CHECK_FUNC([dl_iterate_phdr], [DEFINES="$DEFINES -DHAVE_DL_ITERATE_PHDR"])
Nice. I wasn't aware of dl_iterate_phdr(). My code for querying the
build-id was less slick. It used open(2) on the library, then manually
parsed the ElfW(Ehdr) and ElfW(Shdr) to find the build-id node.
> diff --git a/src/util/build_id.c b/src/util/build_id.c
> new file mode 100644
> index 0000000..a2e21b7
> --- /dev/null
> +++ b/src/util/build_id.c
> +#define ALIGN(val, align) (((val) + (align) - 1) & ~((align) - 1))
> +
> +struct note {
> + ElfW(Nhdr) nhdr;
> +
> + char name[4];
Because nothing requires ElfW(Nhdr).n_namesz to be 4, please a comment
here explaining that we hardcoded 4 because the note name is "GNU".
> + uint8_t build_id[0];
> +};
> +
> +struct callback_data {
> + const char *name;
> + struct note *note;
> +};
> +
> +static int
> +build_id_find_nhdr_callback(struct dl_phdr_info *info, size_t size, void
> *data_)
> +{
This function looks correct to me.
> + struct callback_data *data = data_;
> +
> + char *ptr = strstr(info->dlpi_name, data->name);
> + if (ptr == NULL || ptr[strlen(data->name)] != '\0')
> + return 0;
> +
> + for (unsigned i = 0; i < info->dlpi_phnum; i++) {
> + if (info->dlpi_phdr[i].p_type != PT_NOTE)
> + continue;
> +
> + struct note *note = (void *)(info->dlpi_addr +
> + info->dlpi_phdr[i].p_vaddr);
> + ptrdiff_t len = info->dlpi_phdr[i].p_filesz;
I was initially worried that the len should use p_memsz instead of
p_filesz. But the elf manpage says it's ok; if p_memsz > p_filesz, then
the excess bytes are mere padding.
> +
> + while (len >= sizeof(struct note)) {
> + if (note->nhdr.n_type == NT_GNU_BUILD_ID &&
> + note->nhdr.n_descsz != 0 &&
> + note->nhdr.n_namesz == 4 &&
> + memcmp(note->name, "GNU", 4) == 0) {
> + data->note = note;
> + return 1;
> + }
> +
> + size_t offset = sizeof(ElfW(Nhdr)) +
> + ALIGN(note->nhdr.n_namesz, 4) +
> + ALIGN(note->nhdr.n_descsz, 4);
> + note = (struct note *)((char *)note + offset);
> + len -= offset;
> + }
> + }
> +
> + return 0;
> +}
> +
> +const struct note *
> +build_id_find_nhdr(const char *name)
Please rename the 'name' parameter to be clearer. I was confused,
thinking that name was the name of ELF section for the note. It wasn't
until I read patch 2, and saw name="libvulkan.so" instead
name=".note.gnu.build-id" that I was able to understand this patch.
> +{
> + struct callback_data data = {
> + .name = name,
> + .note = NULL,
> + };
> +
> + if (dl_iterate_phdr(build_id_find_nhdr_callback, &data)) {
> + return data.note;
> + } else {
> + return NULL;
> + }
> +}
> +
> +unsigned
> +build_id_length(const struct note *note)
> +{
> + return note->nhdr.n_descsz;
> +}
> +
> +void
> +build_id_read(const struct note *note, unsigned char *build_id)
> +{
As I explain in patch 2, I think this function needs a 'size' parameter,
à la snprintf, due to Vulkan weirdness regarding restrictons on usage of
malloc.
> + memcpy(build_id, note->build_id, note->nhdr.n_descsz);
> +}
> diff --git a/src/util/build_id.h b/src/util/build_id.h
> new file mode 100644
> index 0000000..0eaecf9
> --- /dev/null
> +++ b/src/util/build_id.h
> +struct note;
Please namespace struct note. Please :)
My only hard request is the size_t parameter for build_id_read(). My
other comments are just minor nits, that you can ignore.
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
