Re: [patch 1/4] unwinder: New base address based dwfl_report_elf_baseaddr

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

On Fri, 15 Mar 2013 17:13:13 +0100, Mark Wielaard wrote:
> On Tue, 2012-11-13 at 21:30 +0100, Jan Kratochvil wrote:
> > Current dwfl_report_elf function has parameter "GElf_Addr base" but it means
> > bias, not the base address.
> 
> If the documentation is wrong/misleading in libdwfl.h we really should
> also fix/clarify that.

The documentation is correct but I added there the prelink caveat.


> I admit that I always get a little confused myself. If I got it right
> then the bias should refer to the offset from the p_vaddr of an ELF
> segment.

yes


> And the base should refer the low address of the Dwfl_Module,
> which is fixed for the main ET_EXEC and ET_CORE at the lowest p_vaddr in
> the ELF file, but dynamic for ET_DYN (and the kernel).

yes


> Even if I got the above right I might still get confused about the
> meaning of the terms for ET_REL (there are no p_hdrs, so the terms don't
> really make sense there do they?).

ET_REL does not make sense to map into memory... For real .o elfutils does
whatever may be considered as user convenient.  For Linux kernel ET_REL files
in fact I do not know much, you definitely know more.


But as a summary the dwfl_report_elf <-> dwfl_report_elf_base difference is
only for prelinked files and prelink affects (in VMA way) only ET_DYN files.


> >   Some applications (such as Linux kernel perf)
> > cannot easily find out the bias but they know the base address.
> > I tried to implement it without this function but I could not.
> 
> Is using dwfl_report_elf () vs dwfl_report_elf_baseaddr () only
> different for ET_DYN files, or does it also change things for reporting
> other ELF files?

yes, only ET_DYN and only if they are prelinked.  Written it therefore now in
the comment.


Thanks,
Jan

[ this updated version is not present in any GIT branch ]



commit 15ce3ffbdf8005e85891d994aa9e2d9578767eee
Author: Jan Kratochvil <jan.kratochvil@redhat.com>
Date:   Sun Mar 17 14:07:27 2013 +0100

    libdwfl/
    	* dwfl_report_elf.c (__libdwfl_report_elf): Add parameter base_is_bias.
    	Set BASE_IS_BIAS to true for ET_EXEC and ET_CORE.  Provide alternative
    	setup of START and BIAS if !BASE_IS_BIAS.  Set END from BIAS, not BASE.
    	(dwfl_report_elf): Rename to ...
    	(__libdwfl_report_elf_open): ... here.  Add parameter base_is_bias,
    	pass it to __libdwfl_report_elf.
    	(dwfl_report_elf, dwfl_report_elf_baseaddr): New wrappers.
    	* libdwfl.h (dwfl_report_elf_baseaddr): New declaration.
    	* libdwflP.h (__libdwfl_report_elf): Add parameter base_is_bias.
    	* offline.c (process_elf): Pass true as base_is_bias to
    	__libdwfl_report_elf.

diff --git a/libdwfl/ChangeLog b/libdwfl/ChangeLog
index f55625c..4427a08 100644
--- a/libdwfl/ChangeLog
+++ b/libdwfl/ChangeLog
@@ -1,3 +1,17 @@
+2013-03-17  Jan Kratochvil  <jan.kratochvil@redhat.com>
+
+	* dwfl_report_elf.c (__libdwfl_report_elf): Add parameter base_is_bias.
+	Set BASE_IS_BIAS to true for ET_EXEC and ET_CORE.  Provide alternative
+	setup of START and BIAS if !BASE_IS_BIAS.  Set END from BIAS, not BASE.
+	(dwfl_report_elf): Rename to ...
+	(__libdwfl_report_elf_open): ... here.  Add parameter base_is_bias,
+	pass it to __libdwfl_report_elf.
+	(dwfl_report_elf, dwfl_report_elf_baseaddr): New wrappers.
+	* libdwfl.h (dwfl_report_elf_baseaddr): New declaration.
+	* libdwflP.h (__libdwfl_report_elf): Add parameter base_is_bias.
+	* offline.c (process_elf): Pass true as base_is_bias to
+	__libdwfl_report_elf.
+
 2013-03-15  Jan Kratochvil  <jan.kratochvil@redhat.com>
 
 	* dwfl_report_elf.c (__libdwfl_report_elf): Remove BASE aligning.
diff --git a/libdwfl/dwfl_report_elf.c b/libdwfl/dwfl_report_elf.c
index d706170..7a6970e 100644
--- a/libdwfl/dwfl_report_elf.c
+++ b/libdwfl/dwfl_report_elf.c
@@ -41,7 +41,8 @@
 Dwfl_Module *
 internal_function
 __libdwfl_report_elf (Dwfl *dwfl, const char *name, const char *file_name,
-		      int fd, Elf *elf, GElf_Addr base, bool sanity)
+		      int fd, Elf *elf, GElf_Addr base, bool base_is_bias,
+		      bool sanity)
 {
   GElf_Ehdr ehdr_mem, *ehdr = gelf_getehdr (elf, &ehdr_mem);
   if (ehdr == NULL)
@@ -166,6 +167,7 @@ __libdwfl_report_elf (Dwfl *dwfl, const char *name, const char *file_name,
     case ET_CORE:
       /* An assigned base address is meaningless for these.  */
       base = 0;
+      base_is_bias = true;
 
     case ET_DYN:
     default:;
@@ -181,11 +183,19 @@ __libdwfl_report_elf (Dwfl *dwfl, const char *name, const char *file_name,
 	    {
 	      vaddr = ph->p_vaddr & -ph->p_align;
 	      address_sync = ph->p_vaddr + ph->p_memsz;
-	      start = base + vaddr;
 	      break;
 	    }
 	}
-      bias = base;
+      if (base_is_bias)
+	{
+	  start = base + vaddr;
+	  bias = base;
+	}
+      else
+	{
+	  start = base;
+	  bias = base - vaddr;
+	}
 
       for (size_t i = phnum; i-- > 0;)
 	{
@@ -195,7 +205,7 @@ __libdwfl_report_elf (Dwfl *dwfl, const char *name, const char *file_name,
 	  if (ph->p_type == PT_LOAD
 	      && ph->p_vaddr + ph->p_memsz > 0)
 	    {
-	      end = base + (ph->p_vaddr + ph->p_memsz);
+	      end = bias + (ph->p_vaddr + ph->p_memsz);
 	      break;
 	    }
 	}
@@ -246,8 +256,9 @@ __libdwfl_report_elf (Dwfl *dwfl, const char *name, const char *file_name,
 }
 
 Dwfl_Module *
-dwfl_report_elf (Dwfl *dwfl, const char *name,
-		 const char *file_name, int fd, GElf_Addr base)
+internal_function
+__libdwfl_report_elf_open (Dwfl *dwfl, const char *name, const char *file_name,
+			   int fd, GElf_Addr base, bool base_is_bias)
 {
   bool closefd = false;
   if (fd < 0)
@@ -270,7 +281,7 @@ dwfl_report_elf (Dwfl *dwfl, const char *name,
     }
 
   Dwfl_Module *mod = __libdwfl_report_elf (dwfl, name, file_name,
-					   fd, elf, base, true);
+					   fd, elf, base, base_is_bias, true);
   if (mod == NULL)
     {
       elf_end (elf);
@@ -280,4 +291,19 @@ dwfl_report_elf (Dwfl *dwfl, const char *name,
 
   return mod;
 }
+
+Dwfl_Module *
+dwfl_report_elf (Dwfl *dwfl, const char *name,
+		 const char *file_name, int fd, GElf_Addr base)
+{
+  return __libdwfl_report_elf_open (dwfl, name, file_name, fd, base, true);
+}
 INTDEF (dwfl_report_elf)
+
+Dwfl_Module *
+dwfl_report_elf_baseaddr (Dwfl *dwfl, const char *name,
+			  const char *file_name, int fd, GElf_Addr base)
+{
+  return __libdwfl_report_elf_open (dwfl, name, file_name, fd, base, false);
+}
+INTDEF (dwfl_report_elf_baseaddr)
diff --git a/libdwfl/libdwfl.h b/libdwfl/libdwfl.h
index 000d582..7102bd8 100644
--- a/libdwfl/libdwfl.h
+++ b/libdwfl/libdwfl.h
@@ -139,13 +139,22 @@ extern Dwfl_Module *dwfl_report_module (Dwfl *dwfl, const char *name,
 
 /* 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,
-   does a simple absolute section layout starting at BASE.
+   does a simple absolute section layout starting at BASE.  For prelinked
+   ET_DYN files BASE is a difference between mapped VMA and disk file VMA.
    FD may be -1 to open FILE_NAME.  On success, FD is consumed by the
    library, and the `find_elf' callback will not be used for this module.  */
 extern Dwfl_Module *dwfl_report_elf (Dwfl *dwfl, const char *name,
 				     const char *file_name, int fd,
 				     GElf_Addr base);
 
+/* See dwfl_report_elf except that BASE is the page-aligned absolute VMA
+   address where the ELF file should start.  Any possible file prelinking of
+   the disk file is compensated.  The difference happens only for prelinked
+   ET_DYN files - otherwise the function is equivalent to dwfl_report_elf.  */
+extern Dwfl_Module *dwfl_report_elf_baseaddr (Dwfl *dwfl, const char *name,
+					      const char *file_name, int fd,
+					      GElf_Addr base);
+
 /* Similar, but report the module for offline use.  All ET_EXEC files
    being reported must be reported before any relocatable objects.
    If this is used, dwfl_report_module and dwfl_report_elf may not be
diff --git a/libdwfl/libdwflP.h b/libdwfl/libdwflP.h
index 5aaa778..204444f 100644
--- a/libdwfl/libdwflP.h
+++ b/libdwfl/libdwflP.h
@@ -382,7 +382,8 @@ extern int __libdwfl_crc32_file (int fd, uint32_t *resp) attribute_hidden;
    Consumes ELF on success, not on failure.  */
 extern Dwfl_Module *__libdwfl_report_elf (Dwfl *dwfl, const char *name,
 					  const char *file_name, int fd,
-					  Elf *elf, GElf_Addr base, bool sanity)
+					  Elf *elf, GElf_Addr base,
+					  bool base_is_bias, bool sanity)
   internal_function;
 
 /* Meat of dwfl_report_offline.  */
diff --git a/libdwfl/offline.c b/libdwfl/offline.c
index 26a6bd6..28d2782 100644
--- a/libdwfl/offline.c
+++ b/libdwfl/offline.c
@@ -127,7 +127,8 @@ process_elf (Dwfl *dwfl, const char *name, const char *file_name, int fd,
 	     Elf *elf)
 {
   Dwfl_Module *mod = __libdwfl_report_elf (dwfl, name, file_name, fd, elf,
-					   dwfl->offline_next_address, false);
+					   dwfl->offline_next_address, true,
+					   false);
   if (mod != NULL)
     {
       /* If this is an ET_EXEC file with fixed addresses, the address range