public inbox for binutils@sourceware.org
 help / color / mirror / Atom feed
* Target-specific FDE pointer sizes (1/3)
@ 2005-01-29  9:02 Richard Sandiford
  2005-01-29  9:31 ` Target-specific FDE pointer sizes (2/3) Richard Sandiford
  2005-01-31 11:12 ` Target-specific FDE pointer sizes (1/3) Nick Clifton
  0 siblings, 2 replies; 10+ messages in thread
From: Richard Sandiford @ 2005-01-29  9:02 UTC (permalink / raw)
  To: binutils

Yet another MIPS oddity, sorry.  This time it's that the 64-bit
EABI uses a combination of 32-bit ELF and 64-bit FDE addresses.
This confuses bfd and readelf, both of which assume that 32-bit
ELF objects use 32-bit addresses.

This first patch adds a new elf_backend_data hook to specify the
size of an FDE address.  For reasons explained in the patch comments,
it isn't always possible to tell for certain what the right size of
an EABI address is, so a return value of 0 is treated as "don't know".

Hopefully the target-independent bits are very straightforward.
The main complication is the MIPS-specific decision.

Tested on mips64-elf, mips64-linux-gnu and i686-linux-gnu.  OK to install?

I've included the testcases here, but they'll only pass if the upcoming
readelf patch is also applied.

Richard


bfd/
	* elf-bfd.h (elf_backend_data): Add elf_backend_eh_frame_address_size.
	(_bfd_elf_eh_frame_address_size): Declare.
	* elfxx-target.h (elf_backend_eh_frame_address_size): Define a default.
	(elfNN_bed): Initialize elf_backend_eh_frame_address_size.
	* elfxx-mips.h (_bfd_mips_elf_eh_frame_address_size): Declare.
	(elf_backend_eh_frame_address_size): Define.
	* elfxx-mips.c (_bfd_mips_elf_eh_frame_address_size): New function.
	* elf-eh-frame.c (_bfd_elf_discard_section_eh_frame): Get the address
	size from the new backend hook.
	(_bfd_elf_write_section_eh_frame): Likewise.
	(_bfd_elf_eh_frame_address_size): New function.

ld/testsuite/
	* ld-mips-elf/eh-frame1.s: Create a .gcc_compiled_long32 if using
	32-bit pointers.
	* ld-mips-elf/eh-frame1.d: Link in .gcc_compiled_long32 sections.
	* ld-mips-elf/eh-frame[34].d: New tests.
	* ld-mips-elf/mips-elf.exp: Run them.

Index: bfd/elf-bfd.h
===================================================================
RCS file: /cvs/src/src/bfd/elf-bfd.h,v
retrieving revision 1.167
diff -u -p -F^\([(a-zA-Z0-9_]\|#define\) -r1.167 elf-bfd.h
--- bfd/elf-bfd.h	17 Jan 2005 17:41:46 -0000	1.167
+++ bfd/elf-bfd.h	28 Jan 2005 13:30:06 -0000
@@ -901,6 +901,12 @@ struct elf_backend_data
   bfd_boolean (*elf_backend_ignore_discarded_relocs)
     (asection *);
 
+  /* This function returns the width of FDE pointers in bytes, or 0 if
+     that can't be determined for some reason.  The default definition
+     goes by the bfd's EI_CLASS.  */
+  unsigned int (*elf_backend_eh_frame_address_size)
+    (bfd *, asection *);
+
   /* These functions tell elf-eh-frame whether to attempt to turn
      absolute or lsda encodings into pc-relative ones.  The default
      definition enables these transformations.  */
@@ -1376,6 +1382,8 @@ extern void _bfd_elf_sprintf_vma
 extern void _bfd_elf_fprintf_vma
   (bfd *, void *, bfd_vma);
 
+extern unsigned int _bfd_elf_eh_frame_address_size
+  (bfd *, asection *);
 extern bfd_byte _bfd_elf_encode_eh_address
   (bfd *abfd, struct bfd_link_info *info, asection *osec, bfd_vma offset,
    asection *loc_sec, bfd_vma loc_offset, bfd_vma *encoded);
Index: bfd/elfxx-target.h
===================================================================
RCS file: /cvs/src/src/bfd/elfxx-target.h,v
retrieving revision 1.73
diff -u -p -F^\([(a-zA-Z0-9_]\|#define\) -r1.73 elfxx-target.h
--- bfd/elfxx-target.h	8 Oct 2004 14:53:59 -0000	1.73
+++ bfd/elfxx-target.h	28 Jan 2005 13:30:07 -0000
@@ -438,6 +438,9 @@ #define elf_backend_discard_info		NULL
 #ifndef elf_backend_ignore_discarded_relocs
 #define elf_backend_ignore_discarded_relocs	NULL
 #endif
+#ifndef elf_backend_eh_frame_address_size
+#define elf_backend_eh_frame_address_size _bfd_elf_eh_frame_address_size
+#endif
 #ifndef elf_backend_can_make_relative_eh_frame
 #define elf_backend_can_make_relative_eh_frame	_bfd_elf_can_make_relative
 #endif
@@ -568,6 +571,7 @@ static const struct elf_backend_data elf
   elf_backend_reloc_type_class,
   elf_backend_discard_info,
   elf_backend_ignore_discarded_relocs,
+  elf_backend_eh_frame_address_size,
   elf_backend_can_make_relative_eh_frame,
   elf_backend_can_make_lsda_relative_eh_frame,
   elf_backend_encode_eh_address,
Index: bfd/elfxx-mips.h
===================================================================
RCS file: /cvs/src/src/bfd/elfxx-mips.h,v
retrieving revision 1.19
diff -u -p -F^\([(a-zA-Z0-9_]\|#define\) -r1.19 elfxx-mips.h
--- bfd/elfxx-mips.h	26 Mar 2004 06:13:40 -0000	1.19
+++ bfd/elfxx-mips.h	28 Jan 2005 13:30:07 -0000
@@ -24,6 +24,8 @@ extern bfd_boolean _bfd_mips_elf_new_sec
   (bfd *, asection *);
 extern void _bfd_mips_elf_symbol_processing
   (bfd *, asymbol *);
+extern unsigned int _bfd_mips_elf_eh_frame_address_size
+  (bfd *, asection *);
 extern bfd_boolean _bfd_mips_elf_name_local_section_symbols
   (bfd *);
 extern bfd_boolean _bfd_mips_elf_section_processing
@@ -124,3 +126,4 @@ extern struct bfd_elf_special_section co
 #define elf_backend_name_local_section_symbols \
   _bfd_mips_elf_name_local_section_symbols
 #define elf_backend_special_sections _bfd_mips_elf_special_sections
+#define elf_backend_eh_frame_address_size _bfd_mips_elf_eh_frame_address_size
Index: bfd/elfxx-mips.c
===================================================================
RCS file: /cvs/src/src/bfd/elfxx-mips.c,v
retrieving revision 1.117
diff -u -p -F^\([(a-zA-Z0-9_]\|#define\) -r1.117 elfxx-mips.c
--- bfd/elfxx-mips.c	14 Dec 2004 09:48:10 -0000	1.117
+++ bfd/elfxx-mips.c	28 Jan 2005 13:30:16 -0000
@@ -4248,6 +4248,72 @@ _bfd_mips_elf_symbol_processing (bfd *ab
     }
 }
 \f
+/* Implement elf_backend_eh_frame_address_size.  This differs from
+   the default in the way it handles EABI64.
+
+   EABI64 was originally specified as an LP64 ABI, and that is what
+   -mabi=eabi normally gives on a 64-bit target.  However, gcc has
+   historically accepted the combination of -mabi=eabi and -mlong32,
+   and this ILP32 variation has become semi-official over time.
+   Both forms use elf32 and have pointer-sized FDE addresses.
+
+   If an EABI object was generated by GCC 4.0 or above, it will have
+   an empty .gcc_compiled_longXX section, where XX is the size of longs
+   in bits.  Unfortunately, ILP32 objects generated by earlier compilers
+   have no special marking to distinguish them from LP64 objects.
+
+   We don't want users of the official LP64 ABI to be punished for the
+   existence of the ILP32 variant, but at the same time, we don't want
+   to mistakenly interpret pre-4.0 ILP32 objects as being LP64 objects.
+   We therefore take the following approach:
+
+      - If ABFD contains a .gcc_compiled_longXX section, use it to
+        determine the pointer size.
+
+      - Otherwise check the type of the first relocation.  Assume that
+        the LP64 ABI is being used if the relocation is of type R_MIPS_64.
+
+      - Otherwise punt.
+
+   The second check is enough to detect LP64 objects generated by pre-4.0
+   compilers because, in the kind of output generated by those compilers,
+   the first relocation will be associated with either a CIE personality
+   routine or an FDE start address.  Furthermore, the compilers never
+   used a special (non-pointer) encoding for this ABI.
+
+   Checking the relocation type should also be safe because there is no
+   reason to use R_MIPS_64 in an ILP32 object.  Pre-4.0 compilers never
+   did so.  */
+
+unsigned int
+_bfd_mips_elf_eh_frame_address_size (bfd *abfd, asection *sec)
+{
+  if (elf_elfheader (abfd)->e_ident[EI_CLASS] == ELFCLASS64)
+    return 8;
+  if ((elf_elfheader (abfd)->e_flags & EF_MIPS_ABI) == E_MIPS_ABI_EABI64)
+    {
+      bfd_boolean long32_p, long64_p;
+
+      long32_p = bfd_get_section_by_name (abfd, ".gcc_compiled_long32") != 0;
+      long64_p = bfd_get_section_by_name (abfd, ".gcc_compiled_long64") != 0;
+      if (long32_p && long64_p)
+	return 0;
+      if (long32_p)
+	return 4;
+      if (long64_p)
+	return 8;
+
+      if (sec->reloc_count > 0
+	  && elf_section_data (sec)->relocs != NULL
+	  && (ELF32_R_TYPE (elf_section_data (sec)->relocs[0].r_info)
+	      == R_MIPS_64))
+	return 8;
+
+      return 0;
+    }
+  return 4;
+}
+\f
 /* There appears to be a bug in the MIPSpro linker that causes GOT_DISP
    relocations against two unnamed section symbols to resolve to the
    same address.  For example, if we have code like:
Index: bfd/elf-eh-frame.c
===================================================================
RCS file: /cvs/src/src/bfd/elf-eh-frame.c,v
retrieving revision 1.41
diff -u -p -F^\([(a-zA-Z0-9_]\|#define\) -r1.41 elf-eh-frame.c
--- bfd/elf-eh-frame.c	17 Jan 2005 17:44:35 -0000	1.41
+++ bfd/elf-eh-frame.c	28 Jan 2005 13:30:18 -0000
@@ -418,8 +418,10 @@ #define REQUIRE(COND)					\
      it (it would need to use 64-bit .eh_frame format anyway).  */
   REQUIRE (sec->size == (unsigned int) sec->size);
 
-  ptr_size = (elf_elfheader (abfd)->e_ident[EI_CLASS]
-	      == ELFCLASS64) ? 8 : 4;
+  ptr_size = (get_elf_backend_data (abfd)
+	      ->elf_backend_eh_frame_address_size (abfd, sec));
+  REQUIRE (ptr_size != 0);
+
   buf = ehbuf;
   last_cie = NULL;
   last_cie_inf = NULL;
@@ -987,12 +989,14 @@ _bfd_elf_write_section_eh_frame (bfd *ab
   unsigned int ptr_size;
   struct eh_cie_fde *ent;
 
-  ptr_size = (elf_elfheader (sec->owner)->e_ident[EI_CLASS]
-	      == ELFCLASS64) ? 8 : 4;
-
   if (sec->sec_info_type != ELF_INFO_TYPE_EH_FRAME)
     return bfd_set_section_contents (abfd, sec->output_section, contents,
 				     sec->output_offset, sec->size);
+
+  ptr_size = (get_elf_backend_data (abfd)
+	      ->elf_backend_eh_frame_address_size (abfd, sec));
+  BFD_ASSERT (ptr_size != 0);
+
   sec_info = elf_section_data (sec)->sec_info;
   htab = elf_hash_table (info);
   hdr_info = &htab->eh_info;
@@ -1407,6 +1411,14 @@ _bfd_elf_write_section_eh_frame_hdr (bfd
   return retval;
 }
 
+/* Return the width of FDE addresses.  This is the default implementation.  */
+
+unsigned int
+_bfd_elf_eh_frame_address_size (bfd *abfd, asection *sec ATTRIBUTE_UNUSED)
+{
+  return elf_elfheader (abfd)->e_ident[EI_CLASS] == ELFCLASS64 ? 8 : 4;
+}
+
 /* Decide whether we can use a PC-relative encoding within the given
    EH frame section.  This is the default implementation.  */
 
Index: ld/testsuite/ld-mips-elf/eh-frame1.ld
===================================================================
RCS file: /cvs/src/src/ld/testsuite/ld-mips-elf/eh-frame1.ld,v
retrieving revision 1.1
diff -u -p -F^\([(a-zA-Z0-9_]\|#define\) -r1.1 eh-frame1.ld
--- ld/testsuite/ld-mips-elf/eh-frame1.ld	16 Nov 2004 10:16:30 -0000	1.1
+++ ld/testsuite/ld-mips-elf/eh-frame1.ld	28 Jan 2005 13:30:18 -0000
@@ -13,6 +13,7 @@ SECTIONS
   . = 0x30000;
   .eh_frame : { *(.eh_frame) }
   .got : { *(.got) }
+  .gcc_compiled_long32 : { *(.gcc_compiled_long32) }
 
   /DISCARD/ : { *(*) }
 }
Index: ld/testsuite/ld-mips-elf/eh-frame1.s
===================================================================
RCS file: /cvs/src/src/ld/testsuite/ld-mips-elf/eh-frame1.s,v
retrieving revision 1.1
diff -u -p -F^\([(a-zA-Z0-9_]\|#define\) -r1.1 eh-frame1.s
--- ld/testsuite/ld-mips-elf/eh-frame1.s	16 Nov 2004 10:16:30 -0000	1.1
+++ ld/testsuite/ld-mips-elf/eh-frame1.s	28 Jan 2005 13:30:18 -0000
@@ -146,3 +146,7 @@ 2:
 
 	cie_basic	basic5
 	fde_basic	basic5,.text,0x10
+
+	.if alignment == 2
+	.section	.gcc_compiled_long32
+	.endif
Index: ld/testsuite/ld-mips-elf/mips-elf.exp
===================================================================
RCS file: /cvs/src/src/ld/testsuite/ld-mips-elf/mips-elf.exp,v
retrieving revision 1.24
diff -u -p -F^\([(a-zA-Z0-9_]\|#define\) -r1.24 mips-elf.exp
--- ld/testsuite/ld-mips-elf/mips-elf.exp	17 Jan 2005 17:44:41 -0000	1.24
+++ ld/testsuite/ld-mips-elf/mips-elf.exp	28 Jan 2005 13:30:18 -0000
@@ -81,6 +81,10 @@ if {$has_newabi && $linux_gnu} {
     run_dump_test "eh-frame2-n32"
     run_dump_test "eh-frame2-n64"
 }
+if {$embedded_elf} {
+    run_dump_test "eh-frame3"
+    run_dump_test "eh-frame4"
+}
 
 run_dump_test "jaloverflow"
 run_dump_test "jaloverflow-2"
diff -u /dev/null ld/testsuite/ld-mips-elf/eh-frame3.d
--- /dev/null	2005-01-19 11:38:20.753181944 +0000
+++ ld/testsuite/ld-mips-elf/eh-frame3.d	2005-01-28 13:28:58.336891717 +0000
@@ -0,0 +1,280 @@
+# This test is for the official LP64 version of EABI64, which uses a
+# combination of 32-bit objects and 64-bit FDE addresses.
+#
+#name: MIPS eh-frame 3
+#source: eh-frame1.s
+#source: eh-frame1.s
+#as: -EB -mips3 -mabi=eabi --defsym alignment=3 --defsym fill=0 --defsym foo=0x1020304050607080
+#readelf: -wf
+#ld: -melf32ebmip -Teh-frame1.ld
+
+The section \.eh_frame contains:
+
+00000000 0000000c 00000000 CIE
+  Version:               1
+  Augmentation:          ""
+  Code alignment factor: 1
+  Data alignment factor: 4
+  Return address column: 31
+
+  DW_CFA_nop
+  DW_CFA_nop
+  DW_CFA_nop
+
+00000010 00000014 00000014 FDE cie=00000000 pc=00020000..00020010
+
+00000028 00000014 0000002c FDE cie=00000000 pc=00020010..00020030
+
+# basic2 removed
+00000040 00000014 00000044 FDE cie=00000000 pc=00020030..00020060
+
+# basic3 removed
+00000058 00000014 0000005c FDE cie=00000000 pc=00020060..000200a0
+
+# basic4 removed
+00000070 00000014 00000074 FDE cie=00000000 pc=000200a0..000200f0
+
+00000088 0000001c 00000000 CIE
+  Version:               1
+  Augmentation:          "zP"
+  Code alignment factor: 1
+  Data alignment factor: 4
+  Return address column: 31
+  Augmentation data:     00 10 20 30 40 50 60 70 80
+
+  DW_CFA_nop
+  DW_CFA_nop
+  DW_CFA_nop
+  DW_CFA_nop
+  DW_CFA_nop
+  DW_CFA_nop
+  DW_CFA_nop
+
+000000a8 0000001c 00000024 FDE cie=00000088 pc=000200f0..00020100
+  DW_CFA_nop
+  DW_CFA_nop
+  DW_CFA_nop
+  DW_CFA_nop
+  DW_CFA_nop
+  DW_CFA_nop
+  DW_CFA_nop
+
+000000c8 0000001c 00000044 FDE cie=00000088 pc=00020100..00020120
+  DW_CFA_nop
+  DW_CFA_nop
+  DW_CFA_nop
+  DW_CFA_nop
+  DW_CFA_nop
+  DW_CFA_nop
+  DW_CFA_nop
+
+000000e8 0000001c 00000000 CIE
+  Version:               1
+  Augmentation:          "zP"
+  Code alignment factor: 1
+  Data alignment factor: 4
+  Return address column: 31
+  Augmentation data:     50 00 00 00 00 00 00 00 10 20 30 40 50 60 70 80
+
+
+00000108 0000001c 00000024 FDE cie=000000e8 pc=00020120..00020130
+  DW_CFA_nop
+  DW_CFA_nop
+  DW_CFA_nop
+  DW_CFA_nop
+  DW_CFA_nop
+  DW_CFA_nop
+  DW_CFA_nop
+
+00000128 0000001c 00000044 FDE cie=000000e8 pc=00020130..00020150
+  DW_CFA_nop
+  DW_CFA_nop
+  DW_CFA_nop
+  DW_CFA_nop
+  DW_CFA_nop
+  DW_CFA_nop
+  DW_CFA_nop
+
+00000148 0000001c 00000000 CIE
+  Version:               1
+  Augmentation:          "zPR"
+  Code alignment factor: 1
+  Data alignment factor: 4
+  Return address column: 31
+  Augmentation data:     00 10 20 30 40 50 60 70 80 00
+
+  DW_CFA_nop
+  DW_CFA_nop
+  DW_CFA_nop
+  DW_CFA_nop
+  DW_CFA_nop
+
+00000168 0000001c 00000024 FDE cie=00000148 pc=00020150..00020160
+  DW_CFA_nop
+  DW_CFA_nop
+  DW_CFA_nop
+  DW_CFA_nop
+  DW_CFA_nop
+  DW_CFA_nop
+  DW_CFA_nop
+
+# FDE for .discard removed
+# zPR2 removed
+00000188 0000001c 00000044 FDE cie=00000148 pc=00020160..00020190
+  DW_CFA_nop
+  DW_CFA_nop
+  DW_CFA_nop
+  DW_CFA_nop
+  DW_CFA_nop
+  DW_CFA_nop
+  DW_CFA_nop
+
+000001a8 0000001c 00000064 FDE cie=00000148 pc=00020190..000201d0
+  DW_CFA_nop
+  DW_CFA_nop
+  DW_CFA_nop
+  DW_CFA_nop
+  DW_CFA_nop
+  DW_CFA_nop
+  DW_CFA_nop
+
+000001c8 0000000c 00000000 CIE
+  Version:               1
+  Augmentation:          ""
+  Code alignment factor: 1
+  Data alignment factor: 4
+  Return address column: 31
+
+  DW_CFA_nop
+  DW_CFA_nop
+  DW_CFA_nop
+
+000001d8 00000014 00000014 FDE cie=000001c8 pc=000201d0..000201e0
+
+# basic1 removed, followed by repeat of above
+000001f0 00000014 0000002c FDE cie=000001c8 pc=000201e0..000201f0
+
+00000208 00000014 00000044 FDE cie=000001c8 pc=000201f0..00020210
+
+00000220 00000014 0000005c FDE cie=000001c8 pc=00020210..00020240
+
+00000238 00000014 00000074 FDE cie=000001c8 pc=00020240..00020280
+
+00000250 00000014 0000008c FDE cie=000001c8 pc=00020280..000202d0
+
+00000268 0000001c 00000000 CIE
+  Version:               1
+  Augmentation:          "zP"
+  Code alignment factor: 1
+  Data alignment factor: 4
+  Return address column: 31
+  Augmentation data:     00 10 20 30 40 50 60 70 80
+
+  DW_CFA_nop
+  DW_CFA_nop
+  DW_CFA_nop
+  DW_CFA_nop
+  DW_CFA_nop
+  DW_CFA_nop
+  DW_CFA_nop
+
+00000288 0000001c 00000024 FDE cie=00000268 pc=000202d0..000202e0
+  DW_CFA_nop
+  DW_CFA_nop
+  DW_CFA_nop
+  DW_CFA_nop
+  DW_CFA_nop
+  DW_CFA_nop
+  DW_CFA_nop
+
+000002a8 0000001c 00000044 FDE cie=00000268 pc=000202e0..00020300
+  DW_CFA_nop
+  DW_CFA_nop
+  DW_CFA_nop
+  DW_CFA_nop
+  DW_CFA_nop
+  DW_CFA_nop
+  DW_CFA_nop
+
+000002c8 0000001c 00000000 CIE
+  Version:               1
+  Augmentation:          "zP"
+  Code alignment factor: 1
+  Data alignment factor: 4
+  Return address column: 31
+  Augmentation data:     50 00 00 00 00 00 00 00 10 20 30 40 50 60 70 80
+
+
+000002e8 0000001c 00000024 FDE cie=000002c8 pc=00020300..00020310
+  DW_CFA_nop
+  DW_CFA_nop
+  DW_CFA_nop
+  DW_CFA_nop
+  DW_CFA_nop
+  DW_CFA_nop
+  DW_CFA_nop
+
+00000308 0000001c 00000044 FDE cie=000002c8 pc=00020310..00020330
+  DW_CFA_nop
+  DW_CFA_nop
+  DW_CFA_nop
+  DW_CFA_nop
+  DW_CFA_nop
+  DW_CFA_nop
+  DW_CFA_nop
+
+00000328 0000001c 00000000 CIE
+  Version:               1
+  Augmentation:          "zPR"
+  Code alignment factor: 1
+  Data alignment factor: 4
+  Return address column: 31
+  Augmentation data:     00 10 20 30 40 50 60 70 80 00
+
+  DW_CFA_nop
+  DW_CFA_nop
+  DW_CFA_nop
+  DW_CFA_nop
+  DW_CFA_nop
+
+00000348 0000001c 00000024 FDE cie=00000328 pc=00020330..00020340
+  DW_CFA_nop
+  DW_CFA_nop
+  DW_CFA_nop
+  DW_CFA_nop
+  DW_CFA_nop
+  DW_CFA_nop
+  DW_CFA_nop
+
+# FDE for .discard removed
+# zPR2 removed
+00000368 0000001c 00000044 FDE cie=00000328 pc=00020340..00020370
+  DW_CFA_nop
+  DW_CFA_nop
+  DW_CFA_nop
+  DW_CFA_nop
+  DW_CFA_nop
+  DW_CFA_nop
+  DW_CFA_nop
+
+00000388 0000001c 00000064 FDE cie=00000328 pc=00020370..000203b0
+  DW_CFA_nop
+  DW_CFA_nop
+  DW_CFA_nop
+  DW_CFA_nop
+  DW_CFA_nop
+  DW_CFA_nop
+  DW_CFA_nop
+
+000003a8 0000000c 00000000 CIE
+  Version:               1
+  Augmentation:          ""
+  Code alignment factor: 1
+  Data alignment factor: 4
+  Return address column: 31
+
+  DW_CFA_nop
+  DW_CFA_nop
+  DW_CFA_nop
+
+000003b8 00000014 00000014 FDE cie=000003a8 pc=000203b0..000203c0
diff -u /dev/null ld/testsuite/ld-mips-elf/eh-frame4.d
--- /dev/null	2005-01-19 11:38:20.753181944 +0000
+++ ld/testsuite/ld-mips-elf/eh-frame4.d	2005-01-28 13:29:52.231896752 +0000
@@ -0,0 +1,207 @@
+# This test is for the semi-official ILP32 variation of EABI64.
+#
+#name: MIPS eh-frame 4
+#source: eh-frame1.s
+#source: eh-frame1.s
+#as: -EB -mips3 -mabi=eabi --defsym alignment=2 --defsym fill=0 --defsym foo=0x50607080
+#readelf: -wf
+#ld: -melf32ebmip -Teh-frame1.ld
+
+The section \.eh_frame contains:
+
+00000000 0000000c 00000000 CIE
+  Version:               1
+  Augmentation:          ""
+  Code alignment factor: 1
+  Data alignment factor: 4
+  Return address column: 31
+
+  DW_CFA_nop
+  DW_CFA_nop
+  DW_CFA_nop
+
+00000010 0000000c 00000014 FDE cie=00000000 pc=00020000..00020010
+
+00000020 0000000c 00000024 FDE cie=00000000 pc=00020010..00020030
+
+# basic2 removed
+00000030 0000000c 00000034 FDE cie=00000000 pc=00020030..00020060
+
+# basic3 removed
+00000040 0000000c 00000044 FDE cie=00000000 pc=00020060..000200a0
+
+# basic4 removed
+00000050 0000000c 00000054 FDE cie=00000000 pc=000200a0..000200f0
+
+00000060 00000014 00000000 CIE
+  Version:               1
+  Augmentation:          "zP"
+  Code alignment factor: 1
+  Data alignment factor: 4
+  Return address column: 31
+  Augmentation data:     00 50 60 70 80
+
+  DW_CFA_nop
+  DW_CFA_nop
+  DW_CFA_nop
+
+00000078 00000010 0000001c FDE cie=00000060 pc=000200f0..00020100
+  DW_CFA_nop
+  DW_CFA_nop
+  DW_CFA_nop
+
+0000008c 00000010 00000030 FDE cie=00000060 pc=00020100..00020120
+  DW_CFA_nop
+  DW_CFA_nop
+  DW_CFA_nop
+
+000000a0 00000014 00000000 CIE
+  Version:               1
+  Augmentation:          "zP"
+  Code alignment factor: 1
+  Data alignment factor: 4
+  Return address column: 31
+  Augmentation data:     50 00 00 00 50 60 70 80
+
+
+000000b8 00000010 0000001c FDE cie=000000a0 pc=00020120..00020130
+  DW_CFA_nop
+  DW_CFA_nop
+  DW_CFA_nop
+
+000000cc 00000010 00000030 FDE cie=000000a0 pc=00020130..00020150
+  DW_CFA_nop
+  DW_CFA_nop
+  DW_CFA_nop
+
+000000e0 00000014 00000000 CIE
+  Version:               1
+  Augmentation:          "zPR"
+  Code alignment factor: 1
+  Data alignment factor: 4
+  Return address column: 31
+  Augmentation data:     00 50 60 70 80 00
+
+  DW_CFA_nop
+
+000000f8 00000010 0000001c FDE cie=000000e0 pc=00020150..00020160
+  DW_CFA_nop
+  DW_CFA_nop
+  DW_CFA_nop
+
+# FDE for .discard removed
+# zPR2 removed
+0000010c 00000010 00000030 FDE cie=000000e0 pc=00020160..00020190
+  DW_CFA_nop
+  DW_CFA_nop
+  DW_CFA_nop
+
+00000120 00000010 00000044 FDE cie=000000e0 pc=00020190..000201d0
+  DW_CFA_nop
+  DW_CFA_nop
+  DW_CFA_nop
+
+00000134 0000000c 00000000 CIE
+  Version:               1
+  Augmentation:          ""
+  Code alignment factor: 1
+  Data alignment factor: 4
+  Return address column: 31
+
+  DW_CFA_nop
+  DW_CFA_nop
+  DW_CFA_nop
+
+00000144 0000000c 00000014 FDE cie=00000134 pc=000201d0..000201e0
+
+# basic1 removed, followed by repeat of above
+00000154 0000000c 00000024 FDE cie=00000134 pc=000201e0..000201f0
+
+00000164 0000000c 00000034 FDE cie=00000134 pc=000201f0..00020210
+
+00000174 0000000c 00000044 FDE cie=00000134 pc=00020210..00020240
+
+00000184 0000000c 00000054 FDE cie=00000134 pc=00020240..00020280
+
+00000194 0000000c 00000064 FDE cie=00000134 pc=00020280..000202d0
+
+000001a4 00000014 00000000 CIE
+  Version:               1
+  Augmentation:          "zP"
+  Code alignment factor: 1
+  Data alignment factor: 4
+  Return address column: 31
+  Augmentation data:     00 50 60 70 80
+
+  DW_CFA_nop
+  DW_CFA_nop
+  DW_CFA_nop
+
+000001bc 00000010 0000001c FDE cie=000001a4 pc=000202d0..000202e0
+  DW_CFA_nop
+  DW_CFA_nop
+  DW_CFA_nop
+
+000001d0 00000010 00000030 FDE cie=000001a4 pc=000202e0..00020300
+  DW_CFA_nop
+  DW_CFA_nop
+  DW_CFA_nop
+
+000001e4 00000014 00000000 CIE
+  Version:               1
+  Augmentation:          "zP"
+  Code alignment factor: 1
+  Data alignment factor: 4
+  Return address column: 31
+  Augmentation data:     50 00 00 00 50 60 70 80
+
+
+000001fc 00000010 0000001c FDE cie=000001e4 pc=00020300..00020310
+  DW_CFA_nop
+  DW_CFA_nop
+  DW_CFA_nop
+
+00000210 00000010 00000030 FDE cie=000001e4 pc=00020310..00020330
+  DW_CFA_nop
+  DW_CFA_nop
+  DW_CFA_nop
+
+00000224 00000014 00000000 CIE
+  Version:               1
+  Augmentation:          "zPR"
+  Code alignment factor: 1
+  Data alignment factor: 4
+  Return address column: 31
+  Augmentation data:     00 50 60 70 80 00
+
+  DW_CFA_nop
+
+0000023c 00000010 0000001c FDE cie=00000224 pc=00020330..00020340
+  DW_CFA_nop
+  DW_CFA_nop
+  DW_CFA_nop
+
+# FDE for .discard removed
+# zPR2 removed
+00000250 00000010 00000030 FDE cie=00000224 pc=00020340..00020370
+  DW_CFA_nop
+  DW_CFA_nop
+  DW_CFA_nop
+
+00000264 00000010 00000044 FDE cie=00000224 pc=00020370..000203b0
+  DW_CFA_nop
+  DW_CFA_nop
+  DW_CFA_nop
+
+00000278 0000000c 00000000 CIE
+  Version:               1
+  Augmentation:          ""
+  Code alignment factor: 1
+  Data alignment factor: 4
+  Return address column: 31
+
+  DW_CFA_nop
+  DW_CFA_nop
+  DW_CFA_nop
+
+00000288 0000000c 00000014 FDE cie=00000278 pc=000203b0..000203c0

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Target-specific FDE pointer sizes (2/3)
  2005-01-29  9:02 Target-specific FDE pointer sizes (1/3) Richard Sandiford
@ 2005-01-29  9:31 ` Richard Sandiford
  2005-01-29  9:58   ` Target-specific FDE pointer sizes (3/3) Richard Sandiford
  2005-01-31 11:17   ` Target-specific FDE pointer sizes (2/3) Nick Clifton
  2005-01-31 11:12 ` Target-specific FDE pointer sizes (1/3) Nick Clifton
  1 sibling, 2 replies; 10+ messages in thread
From: Richard Sandiford @ 2005-01-29  9:31 UTC (permalink / raw)
  To: binutils

Following up from:

    http://sources.redhat.com/ml/binutils/2005-01/msg00504.html

this patch teaches readelf about the unusal MIPS EABI64 treatment
of FDE addresses.  Specifically, it replaces various:

    is_32bit_elf ? 4 : 8

address size calculations with a global eh_addr_size variable.

As with the bfd patch, we need to distinguish between the official
LP64 ABI and the not-so-official ILP32 variant.  In the case of bfd,
it was important that we get the size right, and we had to punt if
we weren't sure.  In the case of readelf, I think we just want a
"best guess", since we have to output _something_.

The guess depends on the section headers, so eh_addr_size is
initialised in process_section_headers, after the point at
which the section headers have been read.

The patch also changes find_section so that it returns zero-length
sections.  This doesn't affect the existing callers because they are
all followed by calls to get_data, which returns null for zero-length
sections.

I suppose there might be scope for a command-line option to
specify the address size, but I'd rather not add that unless
there's a specific need.  Unmarked ILP32 objects are very much
a legacy here.

Tested on mips64-elf, mips64-linux-gnu and i686-linux-gnu.
The testcase was included in the first message.  OK to install?

Richard


binutils/
	* readelf.c (eh_addr_size): New variable.
	(find_section): Move earlier in file.  Return empty sections too.
	(process_program_headers): Use find_section to find .dynamic.
	(process_section_headers): Initialize eh_addr_size.
	(dump_ia64_unwind, slurp_ia64_unwind_table, ia64_process_unwind)
	(dump_hppa_unwind, slurp_hppa_unwind_table, hppa_process_unwind)
	(display_debug_frames): Use it instead of local addr_size variable.
	(size_of_encoded_value): Get pointer size from eh_addr_size rather
	than is_32bit_elf.

Index: binutils/readelf.c
===================================================================
RCS file: /cvs/src/src/binutils/readelf.c,v
retrieving revision 1.274
diff -u -p -F^\([(a-zA-Z0-9_]\|#define\) -r1.274 readelf.c
--- binutils/readelf.c	10 Jan 2005 17:28:37 -0000	1.274
+++ binutils/readelf.c	29 Jan 2005 09:17:40 -0000
@@ -170,6 +170,7 @@ int do_notes;
 int is_32bit_elf;
 int have_frame_base;
 int need_base_address;
+bfd_vma eh_addr_size;
 
 struct group_list
 {
@@ -643,6 +644,20 @@ byte_put_big_endian (unsigned char *fiel
     }
 }
 
+/* Return a pointer to section NAME, or NULL if no such section exists.  */
+
+static Elf_Internal_Shdr *
+find_section (const char *name)
+{
+  unsigned int i;
+
+  for (i = 0; i < elf_header.e_shnum; i++)
+    if (streq (SECTION_NAME (section_headers + i), name))
+      return section_headers + i;
+
+  return NULL;
+}
+
 /* Guess the relocation size commonly used by the specific machines.  */
 
 static int
@@ -3284,15 +3299,9 @@ process_program_headers (FILE *file)
 	  if (section_headers != NULL)
 	    {
 	      Elf_Internal_Shdr *sec;
-	      unsigned int j;
 
-	      for (j = 0, sec = section_headers;
-		   j < elf_header.e_shnum;
-		   j++, sec++)
-		if (streq (SECTION_NAME (sec), ".dynamic"))
-		  break;
-
-	      if (j == elf_header.e_shnum || sec->sh_size == 0)
+	      sec = find_section (".dynamic");
+	      if (sec == NULL || sec->sh_size == 0)
 		{
 		  error (_("no .dynamic section in the dynamic segment"));
 		  break;
@@ -3716,6 +3725,26 @@ process_section_headers (FILE *file)
   dynamic_syminfo = NULL;
   symtab_shndx_hdr = NULL;
 
+  eh_addr_size = is_32bit_elf ? 4 : 8;
+  switch (elf_header.e_machine)
+    {
+    case EM_MIPS:
+    case EM_MIPS_RS3_LE:
+      /* The 64-bit MIPS EABI uses a combination of 32-bit ELF and 64-bit
+	 FDE addresses.  However, the ABI also has a semi-official ILP32
+	 variant for which the normal FDE address size rules apply.
+
+	 GCC 4.0 marks EABI64 objects with a dummy .gcc_compiled_longXX
+	 section, where XX is the size of longs in bits.  Unfortunately,
+	 earlier compilers provided no way of distinguishing ILP32 objects
+	 from LP64 objects, so if there's any doubt, we should assume that
+	 the official LP64 form is being used.  */
+      if ((elf_header.e_flags & EF_MIPS_ABI) == E_MIPS_ABI_EABI64
+	  && find_section (".gcc_compiled_long32") == NULL)
+	eh_addr_size = 8;
+      break;
+    }
+
   for (i = 0, section = section_headers;
        i < elf_header.e_shnum;
        i++, section++)
@@ -4301,12 +4330,9 @@ find_symbol_for_address (Elf_Internal_Sy
 static void
 dump_ia64_unwind (struct ia64_unw_aux_info *aux)
 {
-  bfd_vma addr_size;
   struct ia64_unw_table_entry *tp;
   int in_body;
 
-  addr_size = is_32bit_elf ? 4 : 8;
-
   for (tp = aux->table; tp < aux->table + aux->table_len; ++tp)
     {
       bfd_vma stamp;
@@ -4343,7 +4369,7 @@ dump_ia64_unwind (struct ia64_unw_aux_in
 	      (unsigned long) ((stamp & UNW_FLAG_MASK) >> 32),
 	      UNW_FLAG_EHANDLER (stamp) ? " ehandler" : "",
 	      UNW_FLAG_UHANDLER (stamp) ? " uhandler" : "",
-	      (unsigned long) (addr_size * UNW_LENGTH (stamp)));
+	      (unsigned long) (eh_addr_size * UNW_LENGTH (stamp)));
 
       if (UNW_VER (stamp) != 1)
 	{
@@ -4352,7 +4378,7 @@ dump_ia64_unwind (struct ia64_unw_aux_in
 	}
 
       in_body = 0;
-      for (dp = head + 8; dp < head + 8 + addr_size * UNW_LENGTH (stamp);)
+      for (dp = head + 8; dp < head + 8 + eh_addr_size * UNW_LENGTH (stamp);)
 	dp = unw_decode (dp, in_body, & in_body);
     }
 }
@@ -4362,7 +4388,7 @@ slurp_ia64_unwind_table (FILE *file,
 			 struct ia64_unw_aux_info *aux,
 			 Elf_Internal_Shdr *sec)
 {
-  unsigned long size, addr_size, nrelas, i;
+  unsigned long size, nrelas, i;
   Elf_Internal_Phdr *seg;
   struct ia64_unw_table_entry *tep;
   Elf_Internal_Shdr *relsec;
@@ -4371,8 +4397,6 @@ slurp_ia64_unwind_table (FILE *file,
   Elf_Internal_Sym *sym;
   const char *relname;
 
-  addr_size = is_32bit_elf ? 4 : 8;
-
   /* First, find the starting address of the segment that includes
      this section: */
 
@@ -4403,8 +4427,9 @@ slurp_ia64_unwind_table (FILE *file,
   if (!table)
     return 0;
 
-  tep = aux->table = xmalloc (size / (3 * addr_size) * sizeof (aux->table[0]));
-  for (tp = table; tp < table + size; tp += 3 * addr_size, ++tep)
+  aux->table = xmalloc (size / (3 * eh_addr_size) * sizeof (aux->table[0]));
+  tep = aux->table;
+  for (tp = table; tp < table + size; tp += 3 * eh_addr_size, ++tep)
     {
       tep->start.section = SHN_UNDEF;
       tep->end.section   = SHN_UNDEF;
@@ -4460,9 +4485,9 @@ slurp_ia64_unwind_table (FILE *file,
 	      continue;
 	    }
 
-	  i = rp->r_offset / (3 * addr_size);
+	  i = rp->r_offset / (3 * eh_addr_size);
 
-	  switch (rp->r_offset/addr_size % 3)
+	  switch (rp->r_offset/eh_addr_size % 3)
 	    {
 	    case 0:
 	      aux->table[i].start.section = sym->st_shndx;
@@ -4484,7 +4509,7 @@ slurp_ia64_unwind_table (FILE *file,
       free (rela);
     }
 
-  aux->table_len = size / (3 * addr_size);
+  aux->table_len = size / (3 * eh_addr_size);
   return 1;
 }
 
@@ -4492,13 +4517,11 @@ static int
 ia64_process_unwind (FILE *file)
 {
   Elf_Internal_Shdr *sec, *unwsec = NULL, *strsec;
-  unsigned long i, addr_size, unwcount = 0, unwstart = 0;
+  unsigned long i, unwcount = 0, unwstart = 0;
   struct ia64_unw_aux_info aux;
 
   memset (& aux, 0, sizeof (aux));
 
-  addr_size = is_32bit_elf ? 4 : 8;
-
   for (i = 0, sec = section_headers; i < elf_header.e_shnum; ++i, ++sec)
     {
       if (sec->sh_type == SHT_SYMTAB)
@@ -4602,7 +4625,7 @@ ia64_process_unwind (FILE *file)
 
 	  printf (_(" at offset 0x%lx contains %lu entries:\n"),
 		  (unsigned long) unwsec->sh_offset,
-		  (unsigned long) (unwsec->sh_size / (3 * addr_size)));
+		  (unsigned long) (unwsec->sh_size / (3 * eh_addr_size)));
 
 	  (void) slurp_ia64_unwind_table (file, & aux, unwsec);
 
@@ -4676,10 +4699,8 @@ struct hppa_unw_aux_info
 static void
 dump_hppa_unwind (struct hppa_unw_aux_info *aux)
 {
-  bfd_vma addr_size;
   struct hppa_unw_table_entry *tp;
 
-  addr_size = is_32bit_elf ? 4 : 8;
   for (tp = aux->table; tp < aux->table + aux->table_len; ++tp)
     {
       bfd_vma offset;
@@ -4746,7 +4767,7 @@ slurp_hppa_unwind_table (FILE *file,
 			 struct hppa_unw_aux_info *aux,
 			 Elf_Internal_Shdr *sec)
 {
-  unsigned long size, unw_ent_size, addr_size, nrelas, i;
+  unsigned long size, unw_ent_size, nrelas, i;
   Elf_Internal_Phdr *seg;
   struct hppa_unw_table_entry *tep;
   Elf_Internal_Shdr *relsec;
@@ -4755,8 +4776,6 @@ slurp_hppa_unwind_table (FILE *file,
   Elf_Internal_Sym *sym;
   const char *relname;
 
-  addr_size = is_32bit_elf ? 4 : 8;
-
   /* First, find the starting address of the segment that includes
      this section.  */
 
@@ -4788,11 +4807,11 @@ slurp_hppa_unwind_table (FILE *file,
   if (!table)
     return 0;
 
-  unw_ent_size = 2 * addr_size + 8;
+  unw_ent_size = 2 * eh_addr_size + 8;
 
   tep = aux->table = xmalloc (size / unw_ent_size * sizeof (aux->table[0]));
 
-  for (tp = table; tp < table + size; tp += (2 * addr_size + 8), ++tep)
+  for (tp = table; tp < table + size; tp += (2 * eh_addr_size + 8), ++tep)
     {
       unsigned int tmp1, tmp2;
 
@@ -4887,7 +4906,7 @@ slurp_hppa_unwind_table (FILE *file,
 
 	  i = rp->r_offset / unw_ent_size;
 
-	  switch ((rp->r_offset % unw_ent_size) / addr_size)
+	  switch ((rp->r_offset % unw_ent_size) / eh_addr_size)
 	    {
 	    case 0:
 	      aux->table[i].start.section = sym->st_shndx;
@@ -4917,13 +4936,11 @@ hppa_process_unwind (FILE *file)
   Elf_Internal_Shdr *unwsec = NULL;
   Elf_Internal_Shdr *strsec;
   Elf_Internal_Shdr *sec;
-  unsigned long addr_size;
   unsigned long i;
 
   memset (& aux, 0, sizeof (aux));
 
   assert (string_table != NULL);
-  addr_size = is_32bit_elf ? 4 : 8;
 
   for (i = 0, sec = section_headers; i < elf_header.e_shnum; ++i, ++sec)
     {
@@ -4953,7 +4970,7 @@ hppa_process_unwind (FILE *file)
 
 	  printf (_(" at offset 0x%lx contains %lu entries:\n"),
 		  (unsigned long) sec->sh_offset,
-		  (unsigned long) (sec->sh_size / (2 * addr_size + 8)));
+		  (unsigned long) (sec->sh_size / (2 * eh_addr_size + 8)));
 
           slurp_hppa_unwind_table (file, &aux, sec);
 	  if (aux.table_len > 0)
@@ -7050,26 +7067,6 @@ process_extended_line_op (unsigned char 
   return len;
 }
 
-/* Finds section NAME inside FILE and returns a
-   pointer to it, or NULL upon failure.  */
-
-static Elf_Internal_Shdr *
-find_section (const char * name)
-{
-  Elf_Internal_Shdr *sec;
-  unsigned int i;
-
-  for (i = elf_header.e_shnum, sec = section_headers + i - 1;
-       i; --i, --sec)
-    if (streq (SECTION_NAME (sec), name))
-      break;
-
-  if (i && sec && sec->sh_size != 0)
-    return sec;
-
-  return NULL;
-}
-
 static const char *debug_str_contents;
 static bfd_vma debug_str_size;
 
@@ -9913,7 +9910,7 @@ size_of_encoded_value (int encoding)
   switch (encoding & 0x7)
     {
     default:	/* ??? */
-    case 0:	return is_32bit_elf ? 4 : 8;
+    case 0:	return eh_addr_size;
     case 2:	return 2;
     case 3:	return 4;
     case 4:	return 8;
@@ -9947,7 +9944,6 @@ display_debug_frames (Elf_Internal_Shdr 
   int is_eh = streq (SECTION_NAME (section), ".eh_frame");
   int length_return;
   int max_regs = 0;
-  int addr_size = is_32bit_elf ? 4 : 8;
 
   printf (_("The section %s contains:\n"), SECTION_NAME (section));
 
@@ -9962,7 +9958,7 @@ display_debug_frames (Elf_Internal_Shdr 
       int need_col_headers = 1;
       unsigned char *augmentation_data = NULL;
       unsigned long augmentation_data_len = 0;
-      int encoded_ptr_size = addr_size;
+      int encoded_ptr_size = eh_addr_size;
       int offset_size;
       int initial_length_size;
 
@@ -10035,7 +10031,7 @@ display_debug_frames (Elf_Internal_Shdr 
 	    }
 	  else if (streq (fc->augmentation, "eh"))
 	    {
-	      start += addr_size;
+	      start += eh_addr_size;
 	      fc->code_factor = LEB ();
 	      fc->data_factor = SLEB ();
 	      if (version == 1)
@@ -10515,7 +10511,7 @@ display_debug_frames (Elf_Internal_Shdr 
 	      if (! do_debug_frames_interp)
 		{
 		  printf ("  DW_CFA_def_cfa_expression (");
-		  decode_location_expression (start, addr_size, ul, 0);
+		  decode_location_expression (start, eh_addr_size, ul, 0);
 		  printf (")\n");
 		}
 	      fc->cfa_exp = 1;
@@ -10528,7 +10524,7 @@ display_debug_frames (Elf_Internal_Shdr 
 	      if (! do_debug_frames_interp)
 		{
 		  printf ("  DW_CFA_expression: r%ld (", reg);
-		  decode_location_expression (start, addr_size, ul, 0);
+		  decode_location_expression (start, eh_addr_size, ul, 0);
 		  printf (")\n");
 		}
 	      fc->col_type[reg] = DW_CFA_expression;

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Target-specific FDE pointer sizes (3/3)
  2005-01-29  9:31 ` Target-specific FDE pointer sizes (2/3) Richard Sandiford
@ 2005-01-29  9:58   ` Richard Sandiford
  2005-01-31 11:17   ` Target-specific FDE pointer sizes (2/3) Nick Clifton
  1 sibling, 0 replies; 10+ messages in thread
From: Richard Sandiford @ 2005-01-29  9:58 UTC (permalink / raw)
  To: binutils

Although the address size problem mentioned in the previous posts:

    http://sources.redhat.com/ml/binutils/2005-01/msg00504.html
    http://sources.redhat.com/ml/binutils/2005-01/msg00505.html

was a latent bug it only seems to have caused problems after my
recent padding optimisation:

    http://sources.redhat.com/ml/binutils/2005-01/msg00135.html

The idea is that we shrink this_inf->size to remove any padding:

      /* Try to interpret the CFA instructions and find the first
         padding nop.  Shrink this_inf's size so that it doesn't
         including the padding.  */
      length = get_DW_EH_PE_width (cie.fde_encoding, ptr_size);
      insns = skip_non_nops (insns, end, length);
      if (insns != 0)
        this_inf->size -= end - insns;

and then round it up to the required alignment boundary in
size_of_output_cie_fde:

  return (entry->size
          + extra_augmentation_string_bytes (entry)
          + extra_augmentation_data_bytes (entry)
          + alignment - 1) & -alignment;

This should be correct in theory, but it only works if we've got the
right value of "alignment" (the number of bytes in a pointer).  I wonder
if there are other targets for which we underestimate the pointer size,
and for which we might therefore end up with misaligned sizes?

If this worries anyone, the size shrinking code could be guarded with
something like:

        if (size_of_output_cie_fde (this_inf, ptr_size) > this_inf->size)

so that it only triggers when growing the entry.  A bit of defensive
programming, if you like.

My preference would be not to do this.  I think it's just another
case of "garbage in, garbage out", and I think we could still lose
in other ways if we've got the wrong address size.  Stiil, I thought
I'd better mention it, and I'm certainly happy to make the change if
there's demand.

Richard

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: Target-specific FDE pointer sizes (1/3)
  2005-01-29  9:02 Target-specific FDE pointer sizes (1/3) Richard Sandiford
  2005-01-29  9:31 ` Target-specific FDE pointer sizes (2/3) Richard Sandiford
@ 2005-01-31 11:12 ` Nick Clifton
  1 sibling, 0 replies; 10+ messages in thread
From: Nick Clifton @ 2005-01-31 11:12 UTC (permalink / raw)
  To: Richard Sandiford; +Cc: binutils

Hi Richard,

> bfd/
> 	* elf-bfd.h (elf_backend_data): Add elf_backend_eh_frame_address_size.
> 	(_bfd_elf_eh_frame_address_size): Declare.
> 	* elfxx-target.h (elf_backend_eh_frame_address_size): Define a default.
> 	(elfNN_bed): Initialize elf_backend_eh_frame_address_size.
> 	* elfxx-mips.h (_bfd_mips_elf_eh_frame_address_size): Declare.
> 	(elf_backend_eh_frame_address_size): Define.
> 	* elfxx-mips.c (_bfd_mips_elf_eh_frame_address_size): New function.
> 	* elf-eh-frame.c (_bfd_elf_discard_section_eh_frame): Get the address
> 	size from the new backend hook.
> 	(_bfd_elf_write_section_eh_frame): Likewise.
> 	(_bfd_elf_eh_frame_address_size): New function.
> 
> ld/testsuite/
> 	* ld-mips-elf/eh-frame1.s: Create a .gcc_compiled_long32 if using
> 	32-bit pointers.
> 	* ld-mips-elf/eh-frame1.d: Link in .gcc_compiled_long32 sections.
> 	* ld-mips-elf/eh-frame[34].d: New tests.
> 	* ld-mips-elf/mips-elf.exp: Run them.

Approved - please apply.

Cheers
   Nick


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: Target-specific FDE pointer sizes (2/3)
  2005-01-29  9:31 ` Target-specific FDE pointer sizes (2/3) Richard Sandiford
  2005-01-29  9:58   ` Target-specific FDE pointer sizes (3/3) Richard Sandiford
@ 2005-01-31 11:17   ` Nick Clifton
  2005-01-31 11:37     ` Richard Sandiford
  1 sibling, 1 reply; 10+ messages in thread
From: Nick Clifton @ 2005-01-31 11:17 UTC (permalink / raw)
  To: Richard Sandiford; +Cc: binutils

Hi Richard,

> As with the bfd patch, we need to distinguish between the official
> LP64 ABI and the not-so-official ILP32 variant.  In the case of bfd,
> it was important that we get the size right, and we had to punt if
> we weren't sure.  In the case of readelf, I think we just want a
> "best guess", since we have to output _something_.

I think that if readelf has to guess it should inform the user that it 
is doing so.  That way the user has a chance to realise that readelf 
might have guessed incorrectly and that is why its output does not match 
their expectations.

> binutils/
> 	* readelf.c (eh_addr_size): New variable.
> 	(find_section): Move earlier in file.  Return empty sections too.
> 	(process_program_headers): Use find_section to find .dynamic.
> 	(process_section_headers): Initialize eh_addr_size.
> 	(dump_ia64_unwind, slurp_ia64_unwind_table, ia64_process_unwind)
> 	(dump_hppa_unwind, slurp_hppa_unwind_table, hppa_process_unwind)
> 	(display_debug_frames): Use it instead of local addr_size variable.
> 	(size_of_encoded_value): Get pointer size from eh_addr_size rather
> 	than is_32bit_elf.

Approved - with the addition of a warning message when readelf is forced 
to guess.

Cheers
   Nick

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: Target-specific FDE pointer sizes (2/3)
  2005-01-31 11:17   ` Target-specific FDE pointer sizes (2/3) Nick Clifton
@ 2005-01-31 11:37     ` Richard Sandiford
  2005-01-31 11:47       ` Nick Clifton
  0 siblings, 1 reply; 10+ messages in thread
From: Richard Sandiford @ 2005-01-31 11:37 UTC (permalink / raw)
  To: Nick Clifton; +Cc: binutils

Nick Clifton <nickc@redhat.com> writes:
> Hi Richard,
>> As with the bfd patch, we need to distinguish between the official
>> LP64 ABI and the not-so-official ILP32 variant.  In the case of bfd,
>> it was important that we get the size right, and we had to punt if
>> we weren't sure.  In the case of readelf, I think we just want a
>> "best guess", since we have to output _something_.
>
> I think that if readelf has to guess it should inform the user that it
> is doing so.  That way the user has a chance to realise that readelf
> might have guessed incorrectly and that is why its output does not match
> their expectations.

Any chance of persuading you otherwise? ;)  Three reasons:

  - It doesn't seem very useful to print a warning if the user has no
    way of overriding the guess.  If we do warn about this, I suppose
    we'd also have to add a MIPS-EABI64-specific command-line option
    to readelf, and like I said in my original posting, I'd really
    rather not do that.  There's certainly no precedent with the
    existing options.

  - I don't want to warn about unmarked LP64 objects since there's
    nothing suspect about them.  They do exactly what the official
    ABI said they should do.

  - People only ended up with unmarked ILP32 objects through using an
    undocumented combination of gcc command-line options.  I think 
    it's reasonable for tools like readelf (and gdb, etc.) to treat
    EABI objects as being ABI-conforming without any evidence to
    the contrary.

Richard

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: Target-specific FDE pointer sizes (2/3)
  2005-01-31 11:37     ` Richard Sandiford
@ 2005-01-31 11:47       ` Nick Clifton
  2005-01-31 12:15         ` Richard Sandiford
  0 siblings, 1 reply; 10+ messages in thread
From: Nick Clifton @ 2005-01-31 11:47 UTC (permalink / raw)
  To: Richard Sandiford; +Cc: binutils

Hi Richard,

>>I think that if readelf has to guess it should inform the user that it
>>is doing so.  That way the user has a chance to realise that readelf
>>might have guessed incorrectly and that is why its output does not match
>>their expectations.

> Any chance of persuading you otherwise? ;)  

You can always try. :)

Three reasons:
> 
>   - It doesn't seem very useful to print a warning if the user has no
>     way of overriding the guess. 

But without a warning the user will never know that the guess has been 
made.  Even if the user can do nothing about it, at least they will know 
that it has been made.  They could then even get hold of the readelf 
sources, modify them to change the guess and then re-run readlef, if 
they think that that will help.

>     If we do warn about this, I suppose
>     we'd also have to add a MIPS-EABI64-specific command-line option
>     to readelf, and like I said in my original posting, I'd really
>     rather not do that.  There's certainly no precedent with the
>     existing options.

I agree that there is no precedent for such an option, but that does not 
mean that it cannot be added.  I would be happy for it to be omitted for 
now however.  We could wait to see if there is a demand for such an 
option from users of readelf.

>   - I don't want to warn about unmarked LP64 objects since there's
>     nothing suspect about them.  They do exactly what the official
>     ABI said they should do.

OK.

>   - People only ended up with unmarked ILP32 objects through using an
>     undocumented combination of gcc command-line options.  I think 
>     it's reasonable for tools like readelf (and gdb, etc.) to treat
>     EABI objects as being ABI-conforming without any evidence to
>     the contrary.

Well no.  One of readelf's main purposes is to be used as a tool to 
investigate broken binaries.  Therefore it should never assume that any 
file is compliant with an ABI, just because it is supposed to be.  It 
should always be paranoid and check for compliance, and alert the user 
to any discrepancies.  Furthermore it should not seg fault or break when 
it is given a broken binary to investigate and it should gracefully 
handle any corruption that it encounters.

Cheers
   Nick

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: Target-specific FDE pointer sizes (2/3)
  2005-01-31 11:47       ` Nick Clifton
@ 2005-01-31 12:15         ` Richard Sandiford
  2005-01-31 16:20           ` Nick Clifton
  0 siblings, 1 reply; 10+ messages in thread
From: Richard Sandiford @ 2005-01-31 12:15 UTC (permalink / raw)
  To: Nick Clifton; +Cc: binutils

Nick Clifton <nickc@redhat.com> writes:
>>   - People only ended up with unmarked ILP32 objects through using an
>>     undocumented combination of gcc command-line options.  I think
>> it's reasonable for tools like readelf (and gdb, etc.) to treat
>>     EABI objects as being ABI-conforming without any evidence to
>>     the contrary.
>
> Well no.  One of readelf's main purposes is to be used as a tool to
> investigate broken binaries.  Therefore it should never assume that any
> file is compliant with an ABI, just because it is supposed to be.  It
> should always be paranoid and check for compliance, and alert the user
> to any discrepancies.

But in the case of LP64 objects, there are no discrepancies.  That's my
point.  As far as the ABI spec is concerned, these objects have exactly
the form they're supposed to have.  They were never supposed to be marked
with a .gcc_compiled_long64 section.

In other words, if we do have the warning, we'll be warning about
objects that do exactly what the ABI says they should do.  Objects
that IMO we have little reason to believe might be invalid.

The ILP32 variation is one of those things that "just happened".
I talked about it being "semi-official", but I should really have
said that it's only semi-official in versions of gcc that emit
.gcc_compiled_long32 sections.  As far as I'm concerned, it's
just not supported in earlier toolchains.  I only handled it in
the bfd patch because (in that context) it was easy to do without
any detrimental affect to objects that follow the real ABI.

> Furthermore it should not seg fault or break when it is given a broken
> binary to investigate and it should gracefully handle any corruption
> that it encounters.

FWIW, I've not seen the wrong address size cause readelf to segfault.

Anyway, I don't want this thread to degenerate into a long and pointless
flamewar.  My objections are on record, and I've made then as best I can.
If you still insist on the warning after the above, just say so, and I'll
add it without any more whinging.

Richard

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: Target-specific FDE pointer sizes (2/3)
  2005-01-31 12:15         ` Richard Sandiford
@ 2005-01-31 16:20           ` Nick Clifton
  2005-01-31 20:42             ` Richard Sandiford
  0 siblings, 1 reply; 10+ messages in thread
From: Nick Clifton @ 2005-01-31 16:20 UTC (permalink / raw)
  To: Richard Sandiford; +Cc: binutils

Hi Richard,

> But in the case of LP64 objects, there are no discrepancies.  That's my
> point.  As far as the ABI spec is concerned, these objects have exactly
> the form they're supposed to have.  They were never supposed to be marked
> with a .gcc_compiled_long64 section.
> 
> In other words, if we do have the warning, we'll be warning about
> objects that do exactly what the ABI says they should do.  Objects
> that IMO we have little reason to believe might be invalid.

I see.  In which case I will agree that the warning message would be wrong.

> Anyway, I don't want this thread to degenerate into a long and pointless
> flamewar.  My objections are on record, and I've made then as best I can.
> If you still insist on the warning after the above, just say so, and I'll
> add it without any more whinging.

No, no it's OK.  You have persuaded me.  I just needed a kick in the 
right direction.

Cheers
   Nick


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: Target-specific FDE pointer sizes (2/3)
  2005-01-31 16:20           ` Nick Clifton
@ 2005-01-31 20:42             ` Richard Sandiford
  0 siblings, 0 replies; 10+ messages in thread
From: Richard Sandiford @ 2005-01-31 20:42 UTC (permalink / raw)
  To: Nick Clifton; +Cc: binutils

Nick Clifton <nickc@redhat.com> writes:
> I see.  In which case I will agree that the warning message would be wrong.

Thanks, and sorry for all the hullabaloo.  I did a poor job of
explaining the situation.

Both patches now applied.

Richard

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2005-01-31 20:42 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2005-01-29  9:02 Target-specific FDE pointer sizes (1/3) Richard Sandiford
2005-01-29  9:31 ` Target-specific FDE pointer sizes (2/3) Richard Sandiford
2005-01-29  9:58   ` Target-specific FDE pointer sizes (3/3) Richard Sandiford
2005-01-31 11:17   ` Target-specific FDE pointer sizes (2/3) Nick Clifton
2005-01-31 11:37     ` Richard Sandiford
2005-01-31 11:47       ` Nick Clifton
2005-01-31 12:15         ` Richard Sandiford
2005-01-31 16:20           ` Nick Clifton
2005-01-31 20:42             ` Richard Sandiford
2005-01-31 11:12 ` Target-specific FDE pointer sizes (1/3) Nick Clifton

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).