[patch] Code cleanup: Simplify __libdwfl_report_elf

Jan Kratochvil jan.kratochvil at redhat.com
Sat Oct 27 21:03:37 UTC 2012

Hi Roland,


__libdwfl_report_elf <ET_DYN>
              vaddr = ph->p_vaddr & -ph->p_align;
              address_sync = ph->p_vaddr + ph->p_memsz;
              if ((base & (ph->p_align - 1)) != 0)
                base = (base + ph->p_align - 1) & -ph->p_align;
-             start = base + (ph->p_vaddr & -ph->p_align);
+             start = base + vaddr;
-      bias = start - vaddr;
+      bias = base;

This change has no functionality effect.

It is more interesting whether dwfl_report_elf and __libdwfl_report_elf
parameter BASE was intended to be "base address" or "bias".

Currently it works as bias and it is also documented so:

/* Report a module with start and end addresses computed from the ELF
   program headers in the given file, plus BASE.  For an ET_REL file,

But this means an application cannot specify base address as application does
not know where the ELF is prelinked to.  An application also cannot open an
ELF file beforehand to find out the prelinked address by hand as
__libdw_open_file is private.  Application could only parse ELF completely on
its own which does not seem right to me.  Therefore I will propose some new
API function for specifying the base address later; this paragraph is offtopic

This base address <-> bias confusion may also come from:
          mod = INTUSE(dwfl_report_elf) (dwfl, basename (name),
                                         name, -1, l_addr);
possibly due to:
    ElfW(Addr) l_addr;          /* Base address shared object is loaded at.  */

which is not "Base address" but it is BIAS; fix of the glibc comment failed
with Jakub Jelinek in:
	[patch] Fix l_addr comment

Also the current meaning of dwfl->offline_next_address is not clear to me:
  Dwfl_Module *mod = __libdwfl_report_elf (dwfl, name, file_name, fd, elf,
                                           dwfl->offline_next_address, false);

as it means BIAS, not "next_address" here, which is for example correct for:
    /* Let 0 be the logical address of the file (or first in archive).  */
    dwfl->offline_next_address = 0;
parse_opt <'e'>
            /* Start at zero so if there is just one -e foo.so,
               the DSO is shown without address bias.  */
            dwfl->offline_next_address = 0;

but it seems incorrect to me for:
      /* If this is an ET_EXEC file with fixed addresses, the address range
         it consumed may or may not intersect with the arbitrary range we
         will use for relocatable modules.  Make sure we always use a free
         range for the offline allocations.  If this module did use
         offline_next_address, it may have rounded it up for the module's
         alignment requirements.  */
      if ((dwfl->offline_next_address >= mod->low_addr 
           || mod->low_addr - dwfl->offline_next_address < OFFLINE_REDZONE)
          && dwfl->offline_next_address < mod->high_addr + OFFLINE_REDZONE)
        dwfl->offline_next_address = mod->high_addr + OFFLINE_REDZONE;

where the code IMO expects an address and not a bias.  Maybe it is in practice
"next_address" for ET_REL (which is never prelinked) and it is BIAS for ET_DYN.

I guess the current API/ABI should keep this BASE parameter to be BIAS, so
one/I can rather clarify more the code while keeping the functionality as is.


commit 10a94132fb2e669f4bc7adcce80844bd761ae3b0
Author: Jan Kratochvil <jan.kratochvil at redhat.com>
Date:   Sat Oct 27 20:45:27 2012 +0200

    	* dwfl_report_elf.c (__libdwfl_report_elf): Simplify START and BIAS
    Signed-off-by: Jan Kratochvil <jan.kratochvil at redhat.com>

diff --git a/libdwfl/ChangeLog b/libdwfl/ChangeLog
index bdd9440..83bdf46 100644
--- a/libdwfl/ChangeLog
+++ b/libdwfl/ChangeLog
@@ -1,3 +1,8 @@
+2012-10-27  Jan Kratochvil  <jan.kratochvil at redhat.com>
+	* dwfl_report_elf.c (__libdwfl_report_elf): Simplify START and BIAS
+	calculation.
 2012-10-17  Jan Kratochvil  <jan.kratochvil at redhat.com>
 	* dwfl_module_getdwarf.c (mod_verify_build_id): New function with code
diff --git a/libdwfl/dwfl_report_elf.c b/libdwfl/dwfl_report_elf.c
index 35d9f48..174ce7c 100644
--- a/libdwfl/dwfl_report_elf.c
+++ b/libdwfl/dwfl_report_elf.c
@@ -183,11 +183,11 @@ __libdwfl_report_elf (Dwfl *dwfl, const char *name, const char *file_name,
 	      address_sync = ph->p_vaddr + ph->p_memsz;
 	      if ((base & (ph->p_align - 1)) != 0)
 		base = (base + ph->p_align - 1) & -ph->p_align;
-	      start = base + (ph->p_vaddr & -ph->p_align);
+	      start = base + vaddr;
-      bias = start - vaddr;
+      bias = base;
       for (size_t i = phnum; i-- > 0;)

More information about the elfutils-devel mailing list