[patch] Fix readelf display of 8-byte values

[patch] Fix readelf display of 8-byte values

[Cary Coutant]

[-- Attachment #1: Type: text/plain, Size: 2001 bytes --]

In binutils/dwarf.c, read_and_display_attr_value(), the handling of
ref8 and data8 forms uses dwarf_vmatoa for the first 4 bytes, but uses
printf to format the second 4 bytes:

    case DW_FORM_ref8:
    case DW_FORM_data8:
      if (!do_loc)
	{
	  uvalue = byte_get (data, 4);
	  printf (" 0x%s", dwarf_vmatoa ("x", uvalue));
	  printf (" 0x%lx", (unsigned long) byte_get (data + 4, 4));
	}
      if ((do_loc || do_debug_loc || do_debug_ranges)
	  && num_debug_info_entries == 0)
	{
	  if (sizeof (uvalue) == 8)
	    uvalue = byte_get (data, 8);
	  else
	    error (_("DW_FORM_data8 is unsupported when sizeof (dwarf_vma) != 8\n"));
	}
      data += 8;
      break;

Is this an oversight, or intentional? If simply an oversight, I'd like
to fix it; if it's intentional, I'd like to understand what the
rationale is so I don't break it.

What I'd like to do here is print something that at least looks like
an 8-byte value rather than two 4-byte values (in the wrong order for
little-endian machines), and do the same for ref_sig8 instead of what
it does now, which is print an array of 8 one-byte values. It seems
that the only mechanism dwarf.c code has to determine the ELF file's
byte order is to call byte_get, but byte_get is unable to return a
full 8-byte value when compiled in 32-bit mode. What would you think
about adding two new functions, byte_get_high and byte_get_low, to
elfcomm.c, which would return the high 32 bits and the low 32 bits,
respectively, of a 64-bit value? Then perhaps a dwarf_vmatoa64() could
format the two halves as a single hex number.

A patch for what I've suggested is attached. Comments?

-cary

binutils/:

2012-02-08  Cary Coutant  <ccoutant@google.com>

	* dwarf.c (dwarf_vmatoa64): New function.
	(read_and_display_attr_value): Print 8-byte forms as single hex
	numbers.
	(process_debug_info): Print type signatures as single hex numbers.
	* elfcomm.c (byte_get_high, byte_get_low): New functions.
	* elfcomm.h (byte_get_high, byte_get_low): New functions.

[-- Attachment #2: binutils-data8-patch.txt --]
[-- Type: text/plain, Size: 5462 bytes --]

2012-02-08  Cary Coutant  <ccoutant@google.com>

	* dwarf.c (dwarf_vmatoa64): New function.
	(read_and_display_attr_value): Print 8-byte forms as single hex
	numbers.
	(process_debug_info): Print type signatures as single hex numbers.
	* elfcomm.c (byte_get_high, byte_get_low): New functions.
	* elfcomm.h (byte_get_high, byte_get_low): New functions.


commit 902844cb815565594ae42a5f1a42b3522bfaeeb9
Author: Cary Coutant <ccoutant@google.com>
Date:   Wed Feb 8 18:08:20 2012 -0800

    Print data8 and ref_sig8 forms with proper byte order.

diff --git a/binutils/dwarf.c b/binutils/dwarf.c
index 72cad36..06bc2b8 100644
--- a/binutils/dwarf.c
+++ b/binutils/dwarf.c
@@ -169,6 +169,24 @@ dwarf_vmatoa (const char *fmtch, dwarf_vma value)
   return ret;
 }
 
+static const char *
+dwarf_vmatoa64 (dwarf_vma hvalue, dwarf_vma lvalue)
+{
+  static char buf[64];
+  int len = 0;
+
+  if (hvalue == 0)
+    snprintf (buf, sizeof (buf), "%" DWARF_VMA_FMT "x", lvalue);
+  else
+    {
+      len = snprintf (buf, sizeof (buf), "%" DWARF_VMA_FMT "x", hvalue);
+      snprintf (buf + len, sizeof (buf) - len,
+		"%08" DWARF_VMA_FMT "x", lvalue);
+    }
+
+  return buf;
+}
+
 dwarf_vma
 read_leb128 (unsigned char *data, unsigned int *length_return, int sign)
 {
@@ -1457,9 +1475,11 @@ read_and_display_attr_value (unsigned long attribute,
     case DW_FORM_data8:
       if (!do_loc)
 	{
-	  uvalue = byte_get (data, 4);
-	  printf (" 0x%s", dwarf_vmatoa ("x", uvalue));
-	  printf (" 0x%lx", (unsigned long) byte_get (data + 4, 4));
+	  dwarf_vma lvalue;
+
+	  uvalue = byte_get_high (data, 8);
+	  lvalue = byte_get_low (data, 8);
+	  printf (" 0x%s", dwarf_vmatoa64 (uvalue, lvalue));
 	}
       if ((do_loc || do_debug_loc || do_debug_ranges)
 	  && num_debug_info_entries == 0)
@@ -1541,16 +1561,13 @@ read_and_display_attr_value (unsigned long attribute,
     case DW_FORM_ref_sig8:
       if (!do_loc)
 	{
-	  int i;
-	  printf (" signature: ");
-	  for (i = 0; i < 8; i++)
-	    {
-	      printf ("%02x", (unsigned) byte_get (data, 1));
-	      data += 1;
-	    }
+	  dwarf_vma lvalue;
+
+	  uvalue = byte_get_high (data, 8);
+	  lvalue = byte_get_low (data, 8);
+	  printf (" signature: 0x%s", dwarf_vmatoa64 (uvalue, lvalue));
 	}
-      else
-        data += 8;
+      data += 8;
       break;
 
     case DW_FORM_GNU_ref_index:
@@ -2224,7 +2241,8 @@ process_debug_info (struct dwarf_section *section,
       dwarf_vma cu_offset;
       int offset_size;
       int initial_length_size;
-      unsigned char signature[8] = { 0 };
+      dwarf_vma signature_high = 0;
+      dwarf_vma signature_low = 0;
       dwarf_vma type_offset = 0;
 
       hdrptr = start;
@@ -2258,14 +2276,9 @@ process_debug_info (struct dwarf_section *section,
 
       if (do_types)
         {
-          int i;
-
-          for (i = 0; i < 8; i++)
-            {
-              signature[i] = byte_get (hdrptr, 1);
-              hdrptr += 1;
-            }
-
+          signature_high = byte_get_high (hdrptr, 8);
+          signature_low = byte_get_low (hdrptr, 8);
+          hdrptr += 8;
           type_offset = byte_get (hdrptr, offset_size);
           hdrptr += offset_size;
         }
@@ -2302,13 +2315,10 @@ process_debug_info (struct dwarf_section *section,
 	  printf (_("   Pointer Size:  %d\n"), compunit.cu_pointer_size);
 	  if (do_types)
 	    {
-	      int i;
-	      printf (_("   Signature:     "));
-	      for (i = 0; i < 8; i++)
-	        printf ("%02x", signature[i]);
-	      printf ("\n");
-             printf (_("   Type Offset:   0x%s\n"),
-                     dwarf_vmatoa ("x", type_offset));
+	      printf (_("   Signature:     0x%s\n"),
+		      dwarf_vmatoa64 (signature_high, signature_low));
+	      printf (_("   Type Offset:   0x%s\n"),
+		      dwarf_vmatoa ("x", type_offset));
 	    }
 	}
 
diff --git a/binutils/elfcomm.c b/binutils/elfcomm.c
index e44dee8..229755e 100644
--- a/binutils/elfcomm.c
+++ b/binutils/elfcomm.c
@@ -238,6 +238,30 @@ byte_get_signed (unsigned char *field, int size)
     }
 }
 
+/* Return the high-order 32-bits of a large value.  */
+
+elf_vma
+byte_get_high (unsigned char *field, int size)
+{
+  if (size <= 4)
+    return 0;
+  if (byte_get == byte_get_big_endian)
+    return byte_get_big_endian (field, size - 4);
+  return byte_get_little_endian (field + 4, size - 4);
+}
+
+/* Return the low-order 32-bits of a large value.  */
+
+elf_vma
+byte_get_low (unsigned char *field, int size)
+{
+  if (size <= 4)
+    return byte_get (field, size);
+  if (byte_get == byte_get_big_endian)
+    return byte_get_big_endian (field + size - 4, 4);
+  return byte_get_little_endian (field, 4);
+}
+
 /* Return the path name for a proxy entry in a thin archive, adjusted
    relative to the path name of the thin archive itself if necessary.
    Always returns a pointer to malloc'ed memory.  */
diff --git a/binutils/elfcomm.h b/binutils/elfcomm.h
index 3f9727e..3a1271f 100644
--- a/binutils/elfcomm.h
+++ b/binutils/elfcomm.h
@@ -47,6 +47,8 @@ extern elf_vma (*byte_get) (unsigned char *, int);
 extern elf_vma byte_get_signed (unsigned char *, int);
 extern elf_vma byte_get_little_endian (unsigned char *, int);
 extern elf_vma byte_get_big_endian (unsigned char *, int);
+extern elf_vma byte_get_high (unsigned char *, int);
+extern elf_vma byte_get_low (unsigned char *, int);
 
 #define BYTE_PUT(field, val)	byte_put (field, val, sizeof (field))
 #define BYTE_GET(field)		byte_get (field, sizeof (field))

Re: [patch] Fix readelf display of 8-byte values

[nick clifton]

Hi Cary,

> In binutils/dwarf.c, read_and_display_attr_value(), the handling of
> ref8 and data8 forms uses dwarf_vmatoa for the first 4 bytes, but uses
> printf to format the second 4 bytes:

> Is this an oversight, or intentional?

I think that this is an oversight.

> A patch for what I've suggested is attached. Comments?

I like the patch apart from two small issues:

  * The new function dwarf_vmatoa64() returns a pointer to a statically 
scoped buffer.  This is a dangerous practice and will lead to problems 
if it is invoked more than once within a given statement.  It would be 
better to fill in a buffer provided to the function, or to have multiple 
buffers (ala dwarf_vmatoa), or at least to have a warning comment at the 
start of the function.

* Since byte_get_high() and byte_get_low() are always called as a pair, 
and in sequence, why not just replace them with a single function, (eg 
byte_get64) which fills in both values ?

Cheers
   Nick

Re: [patch] Fix readelf display of 8-byte values

[Cary Coutant]

I seem to have accidentally dropped binutils from the cc list, and
Nick's reply went only to me. Revised patch below...

-cary

On Tue, Feb 14, 2012 at 1:05 AM, nick clifton <nickc@redhat.com> wrote:
> Hi Cary,
>
>> Done.
>
>
> One small point:
>
>
>> +/* Return the high-order 32-bits and the low-order 32-bits
>> +   of a large value separately.  */
>> +
>> +void
>> +byte_get_64 (unsigned char *field, int size, elf_vma *high, elf_vma *low)
>> +{
>> +  if (size<= 4)
>> +    {
>> +      *high = 0;
>> +      *low = byte_get (field, size);
>> +    }
>
>
> What is the purpose of the 'size' parameter ?  Currently size is always 8,
> which matches the purpose implied by the function's name.  Do you envisage a
> future use where the function might be used to get either 4 byte or 8 byte
> values ?  If so then this ought to be mentioned in the comment at the start
> of the function.  Or if you have no such future use in mind, then the
> parameter ought to be dropped.
>
> Cheers
>  Nick


2012-02-14  Cary Coutant  <ccoutant@google.com>

	* dwarf.c (dwarf_vmatoa64): New function.
	(read_and_display_attr_value): Print 8-byte forms as single hex
	numbers.
	(process_debug_info): Print type signatures as single hex numbers.
	* elfcomm.c (byte_get_64): New function.
	* elfcomm.h (byte_get_64): New function.


commit 7716cd59d3136dea27787e94b68d9b0e3a303a79
Author: Cary Coutant <ccoutant@google.com>
Date:   Wed Feb 8 18:08:20 2012 -0800

    Print data8 and ref_sig8 forms with proper byte order.

diff --git a/binutils/dwarf.c b/binutils/dwarf.c
index 2258c67..5ad9b93 100644
--- a/binutils/dwarf.c
+++ b/binutils/dwarf.c
@@ -169,6 +169,27 @@ dwarf_vmatoa (const char *fmtch, dwarf_vma value)
   return ret;
 }

+/* Format a 64-bit value, given as two 32-bit values, in hex.
+   For reentrancy, this uses a buffer provided by the caller.  */
+
+static const char *
+dwarf_vmatoa64 (dwarf_vma hvalue, dwarf_vma lvalue, char *buf,
+		unsigned int buf_len)
+{
+  int len = 0;
+
+  if (hvalue == 0)
+    snprintf (buf, buf_len, "%" DWARF_VMA_FMT "x", lvalue);
+  else
+    {
+      len = snprintf (buf, buf_len, "%" DWARF_VMA_FMT "x", hvalue);
+      snprintf (buf + len, buf_len - len,
+		"%08" DWARF_VMA_FMT "x", lvalue);
+    }
+
+  return buf;
+}
+
 dwarf_vma
 read_leb128 (unsigned char *data, unsigned int *length_return, int sign)
 {
@@ -1381,9 +1402,12 @@ read_and_display_attr_value (unsigned long attribute,
     case DW_FORM_data8:
       if (!do_loc)
 	{
-	  uvalue = byte_get (data, 4);
-	  printf (" 0x%s", dwarf_vmatoa ("x", uvalue));
-	  printf (" 0x%lx", (unsigned long) byte_get (data + 4, 4));
+	  dwarf_vma high_bits;
+	  char buf[64];
+
+	  byte_get_64 (data, &high_bits, &uvalue);
+	  printf (" 0x%s",
+		  dwarf_vmatoa64 (high_bits, uvalue, buf, sizeof (buf)));
 	}
       if ((do_loc || do_debug_loc || do_debug_ranges)
 	  && num_debug_info_entries == 0)
@@ -1453,16 +1477,14 @@ read_and_display_attr_value (unsigned long attribute,
     case DW_FORM_ref_sig8:
       if (!do_loc)
 	{
-	  int i;
-	  printf (" signature: ");
-	  for (i = 0; i < 8; i++)
-	    {
-	      printf ("%02x", (unsigned) byte_get (data, 1));
-	      data += 1;
-	    }
+	  dwarf_vma high_bits;
+	  char buf[64];
+
+	  byte_get_64 (data, &high_bits, &uvalue);
+	  printf (" signature: 0x%s",
+		  dwarf_vmatoa64 (high_bits, uvalue, buf, sizeof (buf)));
 	}
-      else
-        data += 8;
+      data += 8;
       break;

     default:
@@ -2113,7 +2135,8 @@ process_debug_info (struct dwarf_section *section,
       dwarf_vma cu_offset;
       int offset_size;
       int initial_length_size;
-      unsigned char signature[8] = { 0 };
+      dwarf_vma signature_high = 0;
+      dwarf_vma signature_low = 0;
       dwarf_vma type_offset = 0;

       hdrptr = start;
@@ -2147,14 +2170,8 @@ process_debug_info (struct dwarf_section *section,

       if (do_types)
         {
-          int i;
-
-          for (i = 0; i < 8; i++)
-            {
-              signature[i] = byte_get (hdrptr, 1);
-              hdrptr += 1;
-            }
-
+          byte_get_64 (hdrptr, &signature_high, &signature_low);
+          hdrptr += 8;
           type_offset = byte_get (hdrptr, offset_size);
           hdrptr += offset_size;
         }
@@ -2191,13 +2208,13 @@ process_debug_info (struct dwarf_section *section,
 	  printf (_("   Pointer Size:  %d\n"), compunit.cu_pointer_size);
 	  if (do_types)
 	    {
-	      int i;
-	      printf (_("   Signature:     "));
-	      for (i = 0; i < 8; i++)
-	        printf ("%02x", signature[i]);
-	      printf ("\n");
-             printf (_("   Type Offset:   0x%s\n"),
-                     dwarf_vmatoa ("x", type_offset));
+	      char buf[64];
+
+	      printf (_("   Signature:     0x%s\n"),
+		      dwarf_vmatoa64 (signature_high, signature_low,
+				      buf, sizeof (buf)));
+	      printf (_("   Type Offset:   0x%s\n"),
+		      dwarf_vmatoa ("x", type_offset));
 	    }
 	}

diff --git a/binutils/elfcomm.c b/binutils/elfcomm.c
index e44dee8..4224f82 100644
--- a/binutils/elfcomm.c
+++ b/binutils/elfcomm.c
@@ -238,6 +238,25 @@ byte_get_signed (unsigned char *field, int size)
     }
 }

+/* Return the high-order 32-bits and the low-order 32-bits
+   of an 8-byte value separately.  */
+
+void
+byte_get_64 (unsigned char *field, elf_vma *high, elf_vma *low)
+{
+  if (byte_get == byte_get_big_endian)
+    {
+      *high = byte_get_big_endian (field, 4);
+      *low = byte_get_big_endian (field + 4, 4);
+    }
+  else
+    {
+      *high = byte_get_little_endian (field + 4, 4);
+      *low = byte_get_little_endian (field, 4);
+    }
+  return;
+}
+
 /* Return the path name for a proxy entry in a thin archive, adjusted
    relative to the path name of the thin archive itself if necessary.
    Always returns a pointer to malloc'ed memory.  */
diff --git a/binutils/elfcomm.h b/binutils/elfcomm.h
index 3f9727e..2a3c913 100644
--- a/binutils/elfcomm.h
+++ b/binutils/elfcomm.h
@@ -47,6 +47,7 @@ extern elf_vma (*byte_get) (unsigned char *, int);
 extern elf_vma byte_get_signed (unsigned char *, int);
 extern elf_vma byte_get_little_endian (unsigned char *, int);
 extern elf_vma byte_get_big_endian (unsigned char *, int);
+extern void byte_get_64 (unsigned char *, elf_vma *, elf_vma *);

 #define BYTE_PUT(field, val)	byte_put (field, val, sizeof (field))
 #define BYTE_GET(field)		byte_get (field, sizeof (field))

Re: [patch] Fix readelf display of 8-byte values

[nick clifton]

Hi Cary,

> 2012-02-14  Cary Coutant<ccoutant@google.com>
>
> 	* dwarf.c (dwarf_vmatoa64): New function.
> 	(read_and_display_attr_value): Print 8-byte forms as single hex
> 	numbers.
> 	(process_debug_info): Print type signatures as single hex numbers.
> 	* elfcomm.c (byte_get_64): New function.
> 	* elfcomm.h (byte_get_64): New function.
>

Approved - please apply.

Cheers
   Nick