[Mesa-stable] [PATCH] util/build-id: Fix address comparison for binaries with LOAD vaddr > 0

[Emil Velikov]

On 24 January 2018 at 14:13, Stephan Gerhold <stephan at gerhold.net> wrote:
> build_id_find_nhdr_for_addr() fails to find the build-id if the first LOAD
> segment has a virtual address other than 0x0.
>
> For most shared libraries, the first LOAD segment has vaddr=0x0:
>
>     Type           Offset   VirtAddr   PhysAddr   FileSiz MemSiz  Flg Align
>     LOAD           0x000000 0x00000000 0x00000000 0x2d2e26 0x2d2e26 R E 0x1000
>     LOAD           0x2d2e54 0x002d3e54 0x002d3e54 0x2e248 0x2f148 RW  0x1000
>
> However, compiling the Intel Vulkan driver as 32-bit binary on Android produces
> the following ELF header with vaddr=0x8000 instead:
>
>     Type           Offset   VirtAddr   PhysAddr   FileSiz MemSiz  Flg Align
>     PHDR           0x000034 0x00008034 0x00008034 0x00100 0x00100 R   0x4
>     LOAD           0x000000 0x00008000 0x00008000 0x224a04 0x224a04 R E 0x1000
>     LOAD           0x225710 0x0022e710 0x0022e710 0x25988 0x27364 RW  0x1000
>
> build_id_find_nhdr_callback() compares the address of dli_fbase from dladdr()
> and dlpi_addr from dl_iterate_phdr(). With vaddr > 0, these point to a
> different memory address, e.g.:
>
>     dli_fbase=0xd8395000 (offset 0x8000)
>     dlpi_addr=0xd838d000
>
> At least on glibc and bionic (Android) dli_fbase refers to the address where
> the shared object is mapped into the process space, whereas dlpi_addr is just
> the base address for the vaddrs declared in the ELF header.
>
> To compare them correctly, we need to calculate the start of the mapping
> by adding the vaddr of the first LOAD segment to the base address.
>
> Cc: Chad Versace <chadversary at chromium.org>
> Cc: Emil Velikov <emil.velikov at collabora.com>
> Cc: Tapani Pälli <tapani.palli at intel.com>
> Cc: <mesa-stable at lists.freedesktop.org>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=104642
> Fixes: 5c98d38 "util: Query build-id by symbol address, not library name"
> ---
Based on my observation of glibc code and reading at the spec, I think
this is correct.
Admittedly the man page could be improved.

FWIW I've poked the #musl people about this change last night, and
haven't heard any feedback yet.
Be that about a) our understanding of how it should work or b) musl's
implementation on the topic.

Patch looks sensible, although input from Chad/Matt would be appreciated.
Reviewed-by: Emil Velikov <emil.velikov at collabora.com>

-Emil