[patch] Code cleanup: Simplify __libdwfl_report_elf

[Jan Kratochvil <jan dot kratochvil at redhat dot com>]

Hi Roland,

jankratochvil/basecleanup

./libdwfl/dwfl_report_elf.c
__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;
              break;
            }
        }
-      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:

./libdwfl/libdwfl.h
/* 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
here.

This base address <-> bias confusion may also come from:
./libdwfl/link_map.c
          mod = INTUSE(dwfl_report_elf) (dwfl, basename (name),
                                         name, -1, l_addr);
possibly due to:
glibc/elf/link.h
    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
	http://sourceware.org/ml/libc-alpha/2011-09/msg00151.html

Also the current meaning of dwfl->offline_next_address is not clear to me:
./libdwfl/offline.c
process_elf
  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:
./src/readelf.c
process_file
    /* Let 0 be the logical address of the file (or first in archive).  */
    dwfl->offline_next_address = 0;
./libdwfl/argp-std.c
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.


Thanks,
Jan


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

    libdwfl/
    	* dwfl_report_elf.c (__libdwfl_report_elf): Simplify START and BIAS
    	calculation.
    
    Signed-off-by: Jan Kratochvil <jan.kratochvil@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@redhat.com>
+
+	* dwfl_report_elf.c (__libdwfl_report_elf): Simplify START and BIAS
+	calculation.
+
 2012-10-17  Jan Kratochvil  <jan.kratochvil@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;
 	      break;
 	    }
 	}
-      bias = start - vaddr;
+      bias = base;
 
       for (size_t i = phnum; i-- > 0;)
 	{