Re: [PATCH] readelf: Adjust initial FDE address if pcrel before printing.

[Mark Wielaard <mjw at redhat dot com>]

On Fri, 2012-12-21 at 14:13 -0800, Roland McGrath wrote:
> > +  /* Needed if we find pc relative addresses.  */
> 
> "PC-relative"

Fixed.

> > +  GElf_Addr bias;
> > +  dwfl_module_getelf (dwflmod, &bias);
> 
> Check for errors?

Yes, it should, just in case.

> > +	  /* pcrel for an FDE address really means data relative. */
> 
> Two spaces at the end of a sentence.  "data-relative".
> Or explain better: "relative to the runtime address of the start_address
> field itself".

Added your better description.

> > +	  Dwarf_Addr pc_start = initial_location;
> > +	  if ((fde_encoding & 0x70) == DW_EH_PE_pcrel)
> > +	    pc_start += ((uint64_t) shdr->sh_addr
> > +			 + (base - (const unsigned char *) data->d_buf))
> > +			 - bias;
> 
> Put it inside the parens so auto-indentation always works.

Yeah, brackets were wrong.

> > +	  pc_start &= (ptr_size == 4
> > +		       ? UINT64_C (0xffffffff)
> > +		       : UINT64_C (0xffffffffffffffff));
> 
> Why is this necessary?  It needs a comment.

That was a little obscure sorry. What I should have done and do now in
the updated patch is to sign extend the possible truncated pointer value
before the calculation. Hopefully that way, with an extra comment, it is
clearer what is going on.

Thanks,

Mark

>From e4e205445f8318bec1b145fed9ad1c6e0588fa06 Mon Sep 17 00:00:00 2001
From: Mark Wielaard <mjw@redhat.com>
Date: Fri, 21 Dec 2012 22:11:44 +0100
Subject: [PATCH] readelf: Adjust initial FDE address if pcrel before printing.

The FDE initial_location is printed as start address with format_dwarf_addr.
Which does the right thing for .debug_frame addresses, but in .eh_frame
this is encoded as DW_EH_PE_pcrel and so needs to be adjusted before
formatting.

Signed-off-by: Mark Wielaard <mjw@redhat.com>
---
 src/ChangeLog |    7 ++++++-
 src/readelf.c |   23 ++++++++++++++++++++++-
 2 files changed, 28 insertions(+), 2 deletions(-)

diff --git a/src/ChangeLog b/src/ChangeLog
index 547605d..404ef0b 100644
--- a/src/ChangeLog
+++ b/src/ChangeLog
@@ -1,6 +1,11 @@
 2012-12-21  Mark Wielaard  <mjw@redhat.com>
 
-	* addr2line (main): Call dwfl_end.
+	* readelf.c (print_debug_frame_section): Adjust FDE start address
+	if pcrel before feeding it to format_dwarf_addr.
+
+2012-12-21  Mark Wielaard  <mjw@redhat.com>
+
+	* addr2line.c (main): Call dwfl_end.
 
 2012-12-11  Roland McGrath  <roland@hack.frob.com>
 
diff --git a/src/readelf.c b/src/readelf.c
index 7f6f31c..aab9141 100644
--- a/src/readelf.c
+++ b/src/readelf.c
@@ -4710,6 +4710,14 @@ print_debug_frame_section (Dwfl_Module *dwflmod, Ebl *ebl, GElf_Ehdr *ehdr,
   (void) elf_getshdrstrndx (ebl->elf, &shstrndx);
   const char *scnname = elf_strptr (ebl->elf, shstrndx, shdr->sh_name);
 
+  /* Needed if we find PC-relative addresses.  */
+  GElf_Addr bias;
+  if (dwfl_module_getelf (dwflmod, &bias) == NULL)
+    {
+      error (0, 0, gettext ("cannot get ELF: %s"), dwfl_errmsg (-1));
+      return;
+    }
+
   Elf_Data *data = elf_rawdata (scn, NULL);
 
   if (unlikely (data == NULL))
@@ -4956,8 +4964,21 @@ print_debug_frame_section (Dwfl_Module *dwflmod, Ebl *ebl, GElf_Ehdr *ehdr,
 	  Dwarf_Word address_range
 	    = read_ubyte_unaligned_inc (ptr_size, dbg, readp);
 
+	  /* pcrel for an FDE address relative to the runtime address
+	     of the start_address field itself.  Sign extend if
+	     necessary to make sure the calculation is done on the
+	     full 64 bit address even when initial_location only holds
+	     the lower 32 bits.  */
+	  Dwarf_Addr pc_start = initial_location;
+	  if (ptr_size == 4)
+	    pc_start = (uint64_t) (int32_t) pc_start;
+	  if ((fde_encoding & 0x70) == DW_EH_PE_pcrel)
+	    pc_start += ((uint64_t) shdr->sh_addr
+			 + (base - (const unsigned char *) data->d_buf)
+			 - bias);
+
 	  char *a = format_dwarf_addr (dwflmod, cie->address_size,
-				       initial_location);
+				       pc_start);
 	  printf ("\n [%6tx] FDE length=%" PRIu64 " cie=[%6tx]\n"
 		  "   CIE_pointer:              %" PRIu64 "\n"
 		  "   initial_location:         %s",
-- 
1.7.1