PATCH for 64-bit MIPS ELF buglets

PATCH for 64-bit MIPS ELF buglets

[Mark Mitchell]

These patches improve the 64-bit MIPS ELF support.  They handle
DW_FORM_ref8, and handle the R_MIPS_64 relocation correctly.  (Like
R_MIPS_32 it must become a dynamic relocation if the relocation is
against an object in a shared library.)  Finally, we allow
elf_fake_sections to figure out that .rel.dyn is the dynamic
relocation section, even when using the 64-bit ABI, and even though
the 64-bit ABI uses SHT_RELA sections by default.

The only controversy I can spot in this patch is this hunk:

@@ -678,22 +679,19 @@ static reloc_howto_type elf_mips_howto_t
 	 0x000007c4,		/* dst_mask */
 	 false),		/* pcrel_offset */
 
-  /* A 64 bit relocation.  This is used in 32 bit ELF when addresses
-     are 64 bits long; the upper 32 bits are simply a sign extension.
-     The fields of the howto should be the same as for R_MIPS_32,
-     other than the type, name, and special_function.  */
+  /* A 64 bit relocation.  */
   HOWTO (R_MIPS_64,		/* type */
 	 0,			/* rightshift */
-	 2,			/* size (0 = byte, 1 = short, 2 = long) */
-	 32,			/* bitsize */
+	 4,			/* size (0 = byte, 1 = short, 2 = long) */
+	 64,			/* bitsize */
 	 false,			/* pc_relative */
 	 0,			/* bitpos */
 	 complain_overflow_bitfield, /* complain_on_overflow */
 	 mips32_64bit_reloc,	/* special_function */
 	 "R_MIPS_64",		/* name */
 	 true,			/* partial_inplace */
-	 0xffffffff,		/* src_mask */
-	 0xffffffff,		/* dst_mask */
+	 MINUS_ONE,		/* src_mask */
+	 MINUS_ONE,		/* dst_mask */
 	 false),		/* pcrel_offset */
 
   /* Displacement in the global offset table.  */

I don't understand the comment; R_MIPS_64 is clearly documented as a
64-bit relocation in the MIPS ABI.  Period.

However, if someone thinks this patch is wrong, but approves the rest
of the patch, I will check in an alternative version in which we use a
special HOWTO only for the 64-bit ABI, and retain the current version
for the 32-bit ABIs.

OK to check in?

--
Mark Mitchell                   mark@codesourcery.com
CodeSourcery, LLC               http://www.codesourcery.com

1999-07-12  Mark Mitchell  <mark@codesourcery.com>

	* dwarf2.c (read_attribute): Support DW_FORM_ref8.
	* elf32-mips.c (mips_elf_link_hash_entry): Change mips_32_relocs
	to possibly_dynamic_relocs.  Adjust usage throughout code.
	(elf_mips_howto_table): Handle R_MIPS_64 correctly.
	(elf_mips_ctor64_howto): Likewise.
	(mips_elf_calculate_relocation): Handle R_MIPS_64 like R_MIPS_32.
	Adjust indentation.
	(_bfd_mips_elf_check_relocs): Handle R_MIPS_64 like R_MIPS_32.
	Use MIPS_ELF_GOT_SIZE to calculate the size of GOT entries.
	* elf64-mips.c (elf_backend_may_use_rel_p): Define.

Index: dwarf2.c
===================================================================
RCS file: /cvs/binutils/binutils/bfd/dwarf2.c,v
retrieving revision 1.7
diff -u -p -r1.7 dwarf2.c
--- dwarf2.c	1999/07/02 21:03:50	1.7
+++ dwarf2.c	1999/07/12 19:00:02
@@ -611,6 +611,10 @@ read_attribute (attr, abbrev, unit, info
       DW_UNSND (attr) = read_4_bytes (abfd, info_ptr);
       info_ptr += 4;
       break;
+    case DW_FORM_ref8:
+      DW_UNSND (attr) = read_8_bytes (abfd, info_ptr);
+      info_ptr += 8;
+      break;
     case DW_FORM_ref_udata:
       DW_UNSND (attr) = read_unsigned_leb128 (abfd, info_ptr, &bytes_read);
       info_ptr += bytes_read;
Index: elf32-mips.c
===================================================================
RCS file: /cvs/binutils/binutils/bfd/elf32-mips.c,v
retrieving revision 1.17
diff -u -p -r1.17 elf32-mips.c
--- elf32-mips.c	1999/07/12 07:35:06	1.17
+++ elf32-mips.c	1999/07/12 19:00:06
@@ -67,8 +67,9 @@ struct mips_elf_link_hash_entry
   /* External symbol information.  */
   EXTR esym;
 
-  /* Number of MIPS_32 or MIPS_REL32 relocs against this symbol.  */
-  unsigned int mips_32_relocs;
+  /* Number of R_MIPS_32, R_MIPS_REL32, or R_MIPS_64 relocs against
+     this symbol.  */ 
+  unsigned int possibly_dynamic_relocs;
 
   /* The index of the first dynamic relocation (in the .rel.dyn
      section) against this symbol.  */
@@ -678,22 +679,19 @@ static reloc_howto_type elf_mips_howto_t
 	 0x000007c4,		/* dst_mask */
 	 false),		/* pcrel_offset */
 
-  /* A 64 bit relocation.  This is used in 32 bit ELF when addresses
-     are 64 bits long; the upper 32 bits are simply a sign extension.
-     The fields of the howto should be the same as for R_MIPS_32,
-     other than the type, name, and special_function.  */
+  /* A 64 bit relocation.  */
   HOWTO (R_MIPS_64,		/* type */
 	 0,			/* rightshift */
-	 2,			/* size (0 = byte, 1 = short, 2 = long) */
-	 32,			/* bitsize */
+	 4,			/* size (0 = byte, 1 = short, 2 = long) */
+	 64,			/* bitsize */
 	 false,			/* pc_relative */
 	 0,			/* bitpos */
 	 complain_overflow_bitfield, /* complain_on_overflow */
 	 mips32_64bit_reloc,	/* special_function */
 	 "R_MIPS_64",		/* name */
 	 true,			/* partial_inplace */
-	 0xffffffff,		/* src_mask */
-	 0xffffffff,		/* dst_mask */
+	 MINUS_ONE,		/* src_mask */
+	 MINUS_ONE,		/* dst_mask */
 	 false),		/* pcrel_offset */
 
   /* Displacement in the global offset table.  */
@@ -901,8 +899,8 @@ static reloc_howto_type elf_mips_ctor64_
 	 mips32_64bit_reloc,	/* special_function */
 	 "R_MIPS_64",		/* name */
 	 true,			/* partial_inplace */
-	 0xffffffff,		/* src_mask */
-	 0xffffffff,		/* dst_mask */
+	 MINUS_ONE,		/* src_mask */
+	 MINUS_ONE,		/* dst_mask */
 	 false);		/* pcrel_offset */
 
 /* The reloc used for the mips16 jump instruction.  */
@@ -3766,7 +3764,7 @@ mips_elf_link_hash_newfunc (entry, table
       /* We use -2 as a marker to indicate that the information has
 	 not been set.  -1 means there is no associated ifd.  */
       ret->esym.ifd = -2;
-      ret->mips_32_relocs = 0;
+      ret->possibly_dynamic_relocs = 0;
       ret->min_dyn_reloc_index = 0;
       ret->fn_stub = NULL;
       ret->need_fn_stub = false;
@@ -5850,6 +5848,7 @@ mips_elf_calculate_relocation (abfd, 
 
     case R_MIPS_32:
     case R_MIPS_REL32:
+    case R_MIPS_64:
       /* If we're creating a shared library, or this relocation is
 	 against a symbol in a shared library, then we can't know
 	 where the symbol will end up.  So, we create a relocation
@@ -5862,11 +5861,11 @@ mips_elf_calculate_relocation (abfd, 
 	  BFD_ASSERT (h != NULL);
 	  reloc_index 
 	    = mips_elf_create_dynamic_relocation (abfd, 
-						   info, 
-						   relocation,
-						   h->root.dynindx,
-						   addend,
-						   input_section);
+						  info, 
+						  relocation,
+						  h->root.dynindx,
+						  addend,
+						  input_section);
 	  if (h->min_dyn_reloc_index == 0
 	      || reloc_index < h->min_dyn_reloc_index)
 	    h->min_dyn_reloc_index = reloc_index;
@@ -5874,7 +5873,7 @@ mips_elf_calculate_relocation (abfd, 
 	}
       else
 	{
-	  if (r_type == R_MIPS_32)
+	  if (r_type != R_MIPS_REL32)
 	    value = symbol + addend;
 	  else
 	    value = addend;
@@ -5975,10 +5974,6 @@ mips_elf_calculate_relocation (abfd, 
       value = g & howto->dst_mask;
       break;
 
-    case R_MIPS_64:
-      value = (symbol + addend) & howto->dst_mask;
-      break;
-
     case R_MIPS_GOT_PAGE:
       value = mips_elf_got_page (abfd, info, symbol + addend, NULL);
       if (value == (bfd_vma) -1)
@@ -6891,6 +6886,7 @@ _bfd_mips_elf_check_relocs (abfd, info, 
 
 	    case R_MIPS_32:
 	    case R_MIPS_REL32:
+	    case R_MIPS_64:
 	      if (dynobj == NULL
 		  && (info->shared || h != NULL)
 		  && (sec->flags & SEC_ALLOC) != 0)
@@ -6918,7 +6914,7 @@ _bfd_mips_elf_check_relocs (abfd, info, 
 	     conservative, we could actually build the GOT here,
 	     rather than in relocate_section.  */
 	  g->local_gotno++;
-	  sgot->_raw_size += 4;
+	  sgot->_raw_size += MIPS_ELF_GOT_SIZE (dynobj);
 	}
 
       switch (r_type)
@@ -6959,6 +6955,7 @@ _bfd_mips_elf_check_relocs (abfd, info, 
 
 	case R_MIPS_32:
 	case R_MIPS_REL32:
+	case R_MIPS_64:
 	  if ((info->shared || h != NULL)
 	      && (sec->flags & SEC_ALLOC) != 0)
 	    {
@@ -6996,7 +6993,7 @@ _bfd_mips_elf_check_relocs (abfd, info, 
 		  /* We only need to copy this reloc if the symbol is
                      defined in a dynamic object.  */
 		  hmips = (struct mips_elf_link_hash_entry *) h;
-		  ++hmips->mips_32_relocs;
+		  ++hmips->possibly_dynamic_relocs;
 		}
 	     
 	      /* Even though we don't directly need a GOT entry for
@@ -7189,9 +7186,10 @@ _bfd_mips_elf_adjust_dynamic_symbol (inf
      file.  */
   hmips = (struct mips_elf_link_hash_entry *) h;
   if (! info->relocateable
-      && hmips->mips_32_relocs != 0
+      && hmips->possibly_dynamic_relocs != 0
       && (h->elf_link_hash_flags & ELF_LINK_HASH_DEF_REGULAR) == 0)
-    mips_elf_allocate_dynamic_relocations (dynobj, hmips->mips_32_relocs);
+    mips_elf_allocate_dynamic_relocations (dynobj, 
+					   hmips->possibly_dynamic_relocs);
 
   /* For a function, create a stub, if needed. */
   if (h->type == STT_FUNC
Index: elf64-mips.c
===================================================================
RCS file: /cvs/binutils/binutils/bfd/elf64-mips.c,v
retrieving revision 1.6
diff -u -p -r1.6 elf64-mips.c
--- elf64-mips.c	1999/07/07 19:23:21	1.6
+++ elf64-mips.c	1999/07/12 19:00:07
@@ -2148,6 +2148,7 @@ const struct elf_size_info mips_elf64_si
 #define elf_backend_gc_sweep_hook	_bfd_mips_elf_gc_sweep_hook
 #define elf_backend_got_header_size	(4*MIPS_RESERVED_GOTNO)
 #define elf_backend_plt_header_size	0
+#define elf_backend_may_use_rel_p       1
 
 /* We don't set bfd_elf64_bfd_is_local_label_name because the 32-bit 
    MIPS-specific function only applies to IRIX5, which had no 64-bit

Re: PATCH for 64-bit MIPS ELF buglets

[Ian Lance Taylor]

   From: Mark Mitchell <mark@codesourcery.com>
   Date: Mon, 12 Jul 1999 12:12:19 -0700

   The only controversy I can spot in this patch is this hunk:

   @@ -678,22 +679,19 @@ static reloc_howto_type elf_mips_howto_t
	    0x000007c4,		/* dst_mask */
	    false),		/* pcrel_offset */

   -  /* A 64 bit relocation.  This is used in 32 bit ELF when addresses
   -     are 64 bits long; the upper 32 bits are simply a sign extension.
   -     The fields of the howto should be the same as for R_MIPS_32,
   -     other than the type, name, and special_function.  */
   +  /* A 64 bit relocation.  */
      HOWTO (R_MIPS_64,		/* type */
	    0,			/* rightshift */
   -	 2,			/* size (0 = byte, 1 = short, 2 = long) */
   -	 32,			/* bitsize */
   +	 4,			/* size (0 = byte, 1 = short, 2 = long) */
   +	 64,			/* bitsize */
	    false,			/* pc_relative */
	    0,			/* bitpos */
	    complain_overflow_bitfield, /* complain_on_overflow */
	    mips32_64bit_reloc,	/* special_function */
	    "R_MIPS_64",		/* name */
	    true,			/* partial_inplace */
   -	 0xffffffff,		/* src_mask */
   -	 0xffffffff,		/* dst_mask */
   +	 MINUS_ONE,		/* src_mask */
   +	 MINUS_ONE,		/* dst_mask */
	    false),		/* pcrel_offset */

      /* Displacement in the global offset table.  */

   I don't understand the comment; R_MIPS_64 is clearly documented as a
   64-bit relocation in the MIPS ABI.  Period.

What is going on here is that some people use elf32-mips when
compiling ISA level III code.  This works fine, provided you always
load the code at an address from 0 to 0x7fffffff or 0xffffffff80000000
to 0xffffffffffffffff.  The R_MIPS_64 reloc in elf32-mips supports
this usage: it is a 64 bit relocation formed by sign extending a 32
bit address.

There used to be code in mips_elf_relocate_section which handled this
usage.  However, I see that you have removed it.

I now see that you have also removed the support for R_MIPS16_26 while
you shuffled code from mips_elf_relocate_section to
mips_elf_calculate_relocation.  That's rather disturbing.  I wish you
had explicitly pointed that out.  It's obviously very important that
BFD continue to support existing targets.  It's definitely not OK to
break that.

Cases like R_MIPS16_26 and the 32 bit version of R_MIPS_64 arise on
embedded systems.  You must not assume that the MIPS ELF code is only
used on native Unix systems.

Ian

Re: PATCH for 64-bit MIPS ELF buglets

[Mark Mitchell]

>>>>> "Ian" == Ian Lance Taylor <ian@zembu.com> writes:

    Ian> What is going on here is that some people use elf32-mips when
    Ian> compiling ISA level III code.  This works fine, provided you
    Ian> always load the code at an address from 0 to 0x7fffffff or
    Ian> 0xffffffff80000000 to 0xffffffffffffffff.  The R_MIPS_64
    Ian> reloc in elf32-mips supports this usage: it is a 64 bit
    Ian> relocation formed by sign extending a 32 bit address.

OK.  I'll do what I suggested where I use different HOWTOs in 32-bit
vs. 64-bit mode for R_MIPS_64.

    Ian> There used to be code in mips_elf_relocate_section which
    Ian> handled this usage.  However, I see that you have removed it.

I'll see what it did, and restore that behvaior.

    Ian> I now see that you have also removed the support for
    Ian> R_MIPS16_26 while you shuffled code from
    Ian> mips_elf_relocate_section to mips_elf_calculate_relocation.

That wasn't really a shuffling; it was a total gutting. :-)

The loss of R_MIPS16_26/R_MIPS16_GPREL was an oversight on my part.  I
fully intended to restore them, but forgot.  However, I cannot find
any documentation on what these relocations do.  Can you point me at
anything, other than the previous code?  If you cannot, I will try to
decipher it, and make the right thing happen, but I would feel more
comfortable working from documentation.

    Ian> You must not assume that the MIPS ELF code is only used on
    Ian> native Unix systems.

Of course.  

You occasionally point out the obvious; I'm not clueless, just a bit
new to binutils.  When big rewrites happen, things tend to get broken
for a while; now that the rewriting is done, these things will get
fixed up.  Just the usual stabilization phase.

--
Mark Mitchell                   mark@codesourcery.com
CodeSourcery, LLC               http://www.codesourcery.com

Re: PATCH for 64-bit MIPS ELF buglets

[Ian Lance Taylor]

   From: Mark Mitchell <mark@codesourcery.com>
   Date: Mon, 12 Jul 1999 12:46:25 -0700

   The loss of R_MIPS16_26/R_MIPS16_GPREL was an oversight on my part.  I
   fully intended to restore them, but forgot.  However, I cannot find
   any documentation on what these relocations do.  Can you point me at
   anything, other than the previous code?  If you cannot, I will try to
   decipher it, and make the right thing happen, but I would feel more
   comfortable working from documentation.

I'm afraid that there isn't any documentation on those relocations.  I
just wrote them so that they worked.  I can probably answer any
questions you have.

The mips16 jump instruction is weird because the address is stored in
a permuted format so that it can be picked up by the 16 bit
instruction decoder.

Some of the mips16 stub support seems to have disappeared as well.
That all needs to get restored too.

   You occasionally point out the obvious; I'm not clueless, just a bit
   new to binutils.  When big rewrites happen, things tend to get broken
   for a while; now that the rewriting is done, these things will get
   fixed up.  Just the usual stabilization phase.

I'm afraid I tend to point out the obvious to everybody on a regular
basis.  When talking over e-mail, I think it's important to make sure
that everybody is in basic agreement, and I try to accomplish that by
continually reiterating the basis for communication.  I mean no
offense, although unfortunately some people do get offended.  In
general, though, I would rather offend somebody by telling them
something that any idiot knows than I would let a bug get into the
code because somebody in fact doesn't know it, or forgot it.

I find a case like omitting mips16 support pretty scary, because there
isn't much testing for this sort of thing, so it is likely to get
caught months later by somebody who has no idea what has happened to
the code.  I assumed that you broke apart the functions without
changing the functionality; had I noticed that you were actually
changing what they did, I would not have approved the patch.

Ian

Re: PATCH for 64-bit MIPS ELF buglets

[Mark Mitchell]

>>>>> "Ian" == Ian Lance Taylor <ian@zembu.com> writes:

    Ian> I'm afraid that there isn't any documentation on those
    Ian> relocations.  I just wrote them so that they worked.  I can
    Ian> probably answer any questions you have.

Oh, dear.  OK.

    Ian> The mips16 jump instruction is weird because the address is
    Ian> stored in a permuted format so that it can be picked up by
    Ian> the 16 bit instruction decoder.

Thanks for the hint.

    Ian> Some of the mips16 stub support seems to have disappeared as
    Ian> well.  That all needs to get restored too.

I'll do my best.  In retrospect, using R_MIPS_64 for two different
relocations was probably not an ideal design choice; in the future, we
should probably try to use non-standard relocations names for
non-standard relocations.  (As you did for R_MIPS_16*.)  

Of course, sometimes we have to deal with other tools/ABIs that do
things in less than ideal ways. :-)

    Ian> I find a case like omitting mips16 support pretty scary,
    Ian> because there isn't much testing for this sort of thing, so
    Ian> it is likely to get caught months later by somebody who has
    Ian> no idea what has happened to the code.  

The scariest thing is that there is no testing of this functionality.
That's what makes it so easy to break.  The more people work on
binutils, the lower will be the percentage of people with your skill
and experience and the harder it will be for you to verify every
change.  In GCC, we tend to break these kinds of corner cases/obscure
platforms relatively often.  I don't think that's because we're bad
engineers; it's just what happens.  We use release cycles for
stabilization.  On the C++ side, we add regression tests for almost
every bug fix.  That practice has really made a huge difference, and
markedly reduces the amount of "this program used to work with 1.x but
doesn't with 2.x" kind of bug reports we see.

Perhaps it would be worthwhile for someone to produce some .o's that
could become part of the testsuite.  If ABI's are stable, then .o's
should continue to work, and there could actually be tests comparing
the output of a link to a fixed image.  (Of course, there'd have to be
some fudging for the things that *are* allowed to change, but it seems
like we could probably things up to at least test the basic
functionality.)  For example, in this case, testing that R_MIPS16_*
didn't cause an assertion failure/abort would probably have worked.
Better would be to put the relocation target at an absolute address
(via a linker script) and then verify that the relocated contents are
correct.  It should be relatively simple to right a program (using
BFD) that opens an executable image, and examines the value at a fixed
location in a particular section.  I bet that regression tests using
such a framework would do a *ton* towards avoiding problems like this
in the future.

Note that I'm not suggesting you spend your time on this; I'm sure
you're too busy.  But, perhaps Cygnus, or someone else with a strong
interest in these under-tested platforms, would be motivated to work
on such platform-specific test-suites.  We would certainly be willing
to work on this, if anyone is interested in contracting the work.

    Ian> I assumed that you broke apart the functions without changing
    Ian> the functionality; had I noticed that you were actually
    Ian> changing what they did, I would not have approved the patch.

I'm sorry the patch description was unclear.  The way in which the
relocation values was calculated was not conducive to the
multiple-relocs-at-same-location bit in the MIPS ABI, so some
considerable changes were necessitated.

--
Mark Mitchell                   mark@codesourcery.com
CodeSourcery, LLC               http://www.codesourcery.com

Re: PATCH for 64-bit MIPS ELF buglets

[Richard Henderson]

On Mon, Jul 12, 1999 at 04:01:36PM -0400, Ian Lance Taylor wrote:
> I'm afraid that there isn't any documentation on those relocations.  I
> just wrote them so that they worked.  I can probably answer any
> questions you have.
> 
> The mips16 jump instruction is weird because the address is stored in
> a permuted format so that it can be picked up by the 16 bit
> instruction decoder.

FWIW, you can find a copy of the mips16 isa at

  http://www.csrd.uiuc.edu/~ece412/isa/mips16.pdf.gz



r~

Re: PATCH for 64-bit MIPS ELF buglets

[Mark Mitchell]

>>>>> "Richard" == Richard Henderson <rth@cygnus.com> writes:

    Richard> FWIW, you can find a copy of the mips16 isa at

    Richard>   http://www.csrd.uiuc.edu/~ece412/isa/mips16.pdf.gz

Thanks!

--
Mark Mitchell                   mark@codesourcery.com
CodeSourcery, LLC               http://www.codesourcery.com

Re: PATCH for 64-bit MIPS ELF buglets

[Ian Lance Taylor]

   From: Mark Mitchell <mark@codesourcery.com>
   Date: Mon, 12 Jul 1999 13:40:03 -0700

       Ian> Some of the mips16 stub support seems to have disappeared as
       Ian> well.  That all needs to get restored too.

   I'll do my best.  In retrospect, using R_MIPS_64 for two different
   relocations was probably not an ideal design choice; in the future, we
   should probably try to use non-standard relocations names for
   non-standard relocations.  (As you did for R_MIPS_16*.)  

Actually, I think R_MIPS_64 can be handled as the same relocation, but
for the 32 MIPS ELF format you have to do the calculations using
strictly 32 bit values.  At least, that was my original thinking.

However, I admit that the existing code does not work that way.  It
does a normal 32 bit relocation, and overwrites the upper 32 bits with
32 bits of sign extension.  It should probably add the sign extension
to the upper 32 bits of the 64 bit value that it finds in the object
file.  I don't think that making that change would break any existing
object files.


I certainly agree that a better testsuite would be a significant
benefit.  As you guessed, though, I'm not likely to do much in this
regard myself.

There actually is a way to test the code, which is to use the gcc
testsuite and a simulator from gdb.  It doesn't catch corner cases,
but it does catch basic functionality.

Ian

Re: PATCH for 64-bit MIPS ELF buglets

[Mark Mitchell]

>>>>> "Ian" == Ian Lance Taylor <ian@zembu.com> writes:

    Ian> There actually is a way to test the code, which is to use the
    Ian> gcc testsuite and a simulator from gdb.  It doesn't catch
    Ian> corner cases, but it does catch basic functionality.

Clever.

Does GCC's MIPS16 support work?  Is there the appropriate GDB
simulation code?  Does anyone have recent benchmarks of GCC + MIPS16
to test against?  How do you run the GCC testsuite in this situation?
(I assume the usual "make check" doesn't cut it.)

I would be willing to give this a go, if someone gives me clear
directions as to what to do.

I do think we should encourage folks to work on the testsuite thing,
though; running the full GCC testsuite is probably a) too
time-consuming and b) too complicated and c) too dependent on GCC,
rather than purely on binutils to be practical as a regular regression
test for most developers.

--
Mark Mitchell                   mark@codesourcery.com
CodeSourcery, LLC               http://www.codesourcery.com

Re: PATCH for 64-bit MIPS ELF buglets

[Mark Mitchell]

>>>>> "Ian" == Ian Lance Taylor <ian@zembu.com> writes:

    Ian> However, I admit that the existing code does not work that
    Ian> way.  It does a normal 32 bit relocation, and overwrites the
    Ian> upper 32 bits with 32 bits of sign extension.  It should
    Ian> probably add the sign extension to the upper 32 bits of the
    Ian> 64 bit value that it finds in the object file.  I don't think
    Ian> that making that change would break any existing object
    Ian> files.

I assume by "existing" you mean "existing until recently trashed by
N32 work." :-) Your suggestion about how to implement R_MIPS_64 is
well-taken; I'll do as you suggest.  Pedantically, though, it still
requires different handling: it particular the src_mask and size are
different.  That's really what I was complaining about in terms of
using R_MIPS_64 to mean two different things.  For example, the
N32/64-bit ABIs can do *exactly* the same thing with the relocations
that they share, assuming all values are sign-extended to the proper
range.

--
Mark Mitchell                   mark@codesourcery.com
CodeSourcery, LLC               http://www.codesourcery.com

Re: PATCH for 64-bit MIPS ELF buglets

[Ian Lance Taylor]

   From: Mark Mitchell <mark@codesourcery.com>
   Date: Mon, 12 Jul 1999 14:38:00 -0700

   Does GCC's MIPS16 support work?  Is there the appropriate GDB
   simulation code?  Does anyone have recent benchmarks of GCC + MIPS16
   to test against?  How do you run the GCC testsuite in this situation?
   (I assume the usual "make check" doesn't cut it.)

   I would be willing to give this a go, if someone gives me clear
   directions as to what to do.

When I worked on the mips16 code, almost all the gcc tests passed.
That was a couple of years ago now, though.

gdb does have mips16 simulator support.  You just have to configure
for --target mips-elf.  `make check' is all you need in gcc.  In your
DejaGnu site.exp file, set target_list to mips-sim.  You may have to
frob something to get DejaGnu to find the simulator.

   I do think we should encourage folks to work on the testsuite thing,
   though; running the full GCC testsuite is probably a) too
   time-consuming and b) too complicated and c) too dependent on GCC,
   rather than purely on binutils to be practical as a regular regression
   test for most developers.

I agree.  I encourage anyone looking for a project to look into
beefing up the binutils testsuite.

Ian

Re: PATCH for 64-bit MIPS ELF buglets

[Jeffrey A Law]

  In message < 19990712143800E.mitchell@codesourcery.com >you write:
  > Does GCC's MIPS16 support work?  Is there the appropriate GDB
  > simulation code?  Does anyone have recent benchmarks of GCC + MIPS16
  > to test against?  How do you run the GCC testsuite in this situation?
  > (I assume the usual "make check" doesn't cut it.)
It should.  Yes, we have simulator support.

You'll probably need to add a multilib for mips16 to the tx39 or
some similar configuration.

You'll have to build gcc, binutils (gas, ld, binutils), gdb+sim, newlib,
libgloss and any build tools they need.

Then run the gcc testsuite with the -mips16 option.
jeff

Re: PATCH for 64-bit MIPS ELF buglets

[Mark Mitchell]

>>>>> "Jeffrey" == Jeffrey A Law <law@cygnus.com> writes:

    Jeffrey> You'll probably need to add a multilib for mips16 to the
    Jeffrey> tx39 or some similar configuration.

    Jeffrey> You'll have to build gcc, binutils (gas, ld, binutils),
    Jeffrey> gdb+sim, newlib, libgloss and any build tools they need.

    Jeffrey> Then run the gcc testsuite with the -mips16 option.  jeff

Yowsers!  We definitely need a simpler way of regression-testing `ld';
that's *way* too much work to be doing on anything approximating a
regular basis.

Thanks, though, for the details.  I will definitely save your message
for future reference.  It might even be nice to FAQ-entrify it!

--
Mark Mitchell                   mark@codesourcery.com
CodeSourcery, LLC               http://www.codesourcery.com

Re: PATCH for 64-bit MIPS ELF buglets

[Jeffrey A Law]

  In message < 19990712162135B.mitchell@codesourcery.com >you write:
  > Yowsers!  We definitely need a simpler way of regression-testing `ld';
  > that's *way* too much work to be doing on anything approximating a
  > regular basis.
Yup.  We definitely need a stronger ld testsuite.

Given a stronger ld testsuite, the kind of testing I mentioned serves as
a second level test.  ie, you run the ld testsuite regularly to make sure
nothing is totally busted, then once in a while you run the bigger test.

  > Thanks, though, for the details.  I will definitely save your message
  > for future reference.  It might even be nice to FAQ-entrify it!
The complexities involved in this kind of stuff is one of the reasons I'd
like to move towards a single CVS tree, even for the multiple projects since
it makes this kind of stuff a hell of a lot easier.

jeff

Re: PATCH for 64-bit MIPS ELF buglets

[Mark Mitchell]

>>>>> "Jeffrey" == Jeffrey A Law <law@cygnus.com> writes:

    Jeffrey> Given a stronger ld testsuite, the kind of testing I
    Jeffrey> mentioned serves as a second level test.  ie, you run the
    Jeffrey> ld testsuite regularly to make sure nothing is totally
    Jeffrey> busted, then once in a while you run the bigger test.

Right.  Just the way that we try to build GNOME with EGCS, every now
and then, even though we don't do it before every check-in.

--
Mark Mitchell                   mark@codesourcery.com
CodeSourcery, LLC               http://www.codesourcery.com

Re: PATCH for 64-bit MIPS ELF buglets

[Mark Mitchell]

I put in the attached patch.  It contains the non-controversial
aspects of the patch I submitted earlier today, as well as an attempt
to restore the behavior of R_MIPS_64 for 32-bit targets.  Ian, please
take a look at the mips_elf_relocate_section changes for the details.

I'll work on the R_MIPS16* stuff next.

--
Mark Mitchell                   mark@codesourcery.com
CodeSourcery, LLC               http://www.codesourcery.com

Index: elf32-mips.c
===================================================================
RCS file: /cvs/binutils/binutils/bfd/elf32-mips.c,v
retrieving revision 1.17
diff -u -p -r1.17 elf32-mips.c
--- elf32-mips.c	1999/07/12 07:35:06	1.17
+++ elf32-mips.c	1999/07/12 23:35:07
@@ -67,8 +67,9 @@ struct mips_elf_link_hash_entry
   /* External symbol information.  */
   EXTR esym;
 
-  /* Number of MIPS_32 or MIPS_REL32 relocs against this symbol.  */
-  unsigned int mips_32_relocs;
+  /* Number of R_MIPS_32, R_MIPS_REL32, or R_MIPS_64 relocs against
+     this symbol.  */ 
+  unsigned int possibly_dynamic_relocs;
 
   /* The index of the first dynamic relocation (in the .rel.dyn
      section) against this symbol.  */
@@ -678,22 +679,19 @@ static reloc_howto_type elf_mips_howto_t
 	 0x000007c4,		/* dst_mask */
 	 false),		/* pcrel_offset */
 
-  /* A 64 bit relocation.  This is used in 32 bit ELF when addresses
-     are 64 bits long; the upper 32 bits are simply a sign extension.
-     The fields of the howto should be the same as for R_MIPS_32,
-     other than the type, name, and special_function.  */
+  /* A 64 bit relocation.  */
   HOWTO (R_MIPS_64,		/* type */
 	 0,			/* rightshift */
-	 2,			/* size (0 = byte, 1 = short, 2 = long) */
-	 32,			/* bitsize */
+	 4,			/* size (0 = byte, 1 = short, 2 = long) */
+	 64,			/* bitsize */
 	 false,			/* pc_relative */
 	 0,			/* bitpos */
 	 complain_overflow_bitfield, /* complain_on_overflow */
 	 mips32_64bit_reloc,	/* special_function */
 	 "R_MIPS_64",		/* name */
 	 true,			/* partial_inplace */
-	 0xffffffff,		/* src_mask */
-	 0xffffffff,		/* dst_mask */
+	 MINUS_ONE,		/* src_mask */
+	 MINUS_ONE,		/* dst_mask */
 	 false),		/* pcrel_offset */
 
   /* Displacement in the global offset table.  */
@@ -3766,7 +3764,7 @@ mips_elf_link_hash_newfunc (entry, table
       /* We use -2 as a marker to indicate that the information has
 	 not been set.  -1 means there is no associated ifd.  */
       ret->esym.ifd = -2;
-      ret->mips_32_relocs = 0;
+      ret->possibly_dynamic_relocs = 0;
       ret->min_dyn_reloc_index = 0;
       ret->fn_stub = NULL;
       ret->need_fn_stub = false;
@@ -5850,6 +5848,7 @@ mips_elf_calculate_relocation (abfd, 
 
     case R_MIPS_32:
     case R_MIPS_REL32:
+    case R_MIPS_64:
       /* If we're creating a shared library, or this relocation is
 	 against a symbol in a shared library, then we can't know
 	 where the symbol will end up.  So, we create a relocation
@@ -5862,11 +5861,11 @@ mips_elf_calculate_relocation (abfd, 
 	  BFD_ASSERT (h != NULL);
 	  reloc_index 
 	    = mips_elf_create_dynamic_relocation (abfd, 
-						   info, 
-						   relocation,
-						   h->root.dynindx,
-						   addend,
-						   input_section);
+						  info, 
+						  relocation,
+						  h->root.dynindx,
+						  addend,
+						  input_section);
 	  if (h->min_dyn_reloc_index == 0
 	      || reloc_index < h->min_dyn_reloc_index)
 	    h->min_dyn_reloc_index = reloc_index;
@@ -5874,7 +5873,7 @@ mips_elf_calculate_relocation (abfd, 
 	}
       else
 	{
-	  if (r_type == R_MIPS_32)
+	  if (r_type != R_MIPS_REL32)
 	    value = symbol + addend;
 	  else
 	    value = addend;
@@ -5975,10 +5974,6 @@ mips_elf_calculate_relocation (abfd, 
       value = g & howto->dst_mask;
       break;
 
-    case R_MIPS_64:
-      value = (symbol + addend) & howto->dst_mask;
-      break;
-
     case R_MIPS_GOT_PAGE:
       value = mips_elf_got_page (abfd, info, symbol + addend, NULL);
       if (value == (bfd_vma) -1)
@@ -6178,7 +6173,17 @@ _bfd_mips_elf_relocate_section (output_b
       reloc_howto_type *howto;
 
       /* Find the relocation howto for this relocation.  */
-      howto = elf_mips_howto_table + ELF32_R_TYPE (rel->r_info);
+      if (ELF32_R_TYPE (rel->r_info) == R_MIPS_64
+	  && !ABI_64_P (output_bfd))
+	/* Some 32-bit code uses R_MIPS_64.  In particular, people use
+	   64-bit code, but make sure all their addresses are in the 
+	   lowermost or uppermost 32-bit section of the 64-bit address
+	   space.  Thus, when they use an R_MIPS_64 they mean what is
+	   usually meant by R_MIPS_32, with the exception that the
+	   stored value is sign-extended to 64 bits.  */
+	howto = elf_mips_howto_table + R_MIPS_32;
+      else
+	howto = elf_mips_howto_table + ELF32_R_TYPE (rel->r_info);
 
       if (!use_saved_addend_p)
 	{
@@ -6303,6 +6308,56 @@ _bfd_mips_elf_relocate_section (output_b
 	  continue;
 	}
 
+      if (ELF32_R_TYPE (rel->r_info) == R_MIPS_64
+	  && !ABI_64_P (output_bfd))
+	/* See the comment above about using R_MIPS_64 in the 32-bit
+	   ABI.  Until now, we've been using the HOWTO for R_MIPS_32;
+	   that calculated the right value.  Now, however, we
+	   sign-extend the 32-bit result to 64-bits, and store it as a
+	   64-bit value.  We are especially generous here in that we
+	   go to extreme lengths to support this usage on systems with
+	   only a 32-bit VMA.  */
+	{
+#ifdef BFD64
+	  /* Just sign-extend the value, and then fall through to the
+	     normal case, using the R_MIPS_64 howto.  That will store
+	     the 64-bit value into a 64-bit area.  */
+	  value = mips_elf_sign_extend (value, 64);
+	  howto = elf_mips_howto_table + R_MIPS_64;
+#else /* !BFD64 */
+	  /* In the 32-bit VMA case, we must handle sign-extension and
+	     endianness manually.  */
+	  bfd_vma sign_bits;
+	  bfd_vma low_bits;
+	  bfd_vma high_bits;
+
+	  if (value & 0x80000000)
+	    sign_bits = 0xffffffff;
+	  else
+	    sign_bits = 0;
+
+	  /* If only a 32-bit VMA is available do two separate
+	     stores.  */
+	  if (bfd_big_endian (input_bfd))
+	    {
+	      /* Store the sign-bits (which are most significant)
+		 first.  */
+	      low_bits = sign_bits;
+	      high_bits = value;
+	    }
+	  else
+	    {
+	      low_bits = value;
+	      high_bits = sign_bits;
+	    }
+	  bfd_put_32 (input_bfd, low_bits, 
+		      contents + rel->r_offset);
+	  bfd_put_32 (input_bfd, high_bits, 
+		      contents + rel->r_offset + 4);
+	  continue;
+#endif /* !BFD64 */
+	}
+
       /* Actually perform the relocation.  */
       mips_elf_perform_relocation (howto, rel, value, input_bfd, 
 				   contents);
@@ -6891,6 +6946,7 @@ _bfd_mips_elf_check_relocs (abfd, info, 
 
 	    case R_MIPS_32:
 	    case R_MIPS_REL32:
+	    case R_MIPS_64:
 	      if (dynobj == NULL
 		  && (info->shared || h != NULL)
 		  && (sec->flags & SEC_ALLOC) != 0)
@@ -6918,7 +6974,7 @@ _bfd_mips_elf_check_relocs (abfd, info, 
 	     conservative, we could actually build the GOT here,
 	     rather than in relocate_section.  */
 	  g->local_gotno++;
-	  sgot->_raw_size += 4;
+	  sgot->_raw_size += MIPS_ELF_GOT_SIZE (dynobj);
 	}
 
       switch (r_type)
@@ -6959,6 +7015,7 @@ _bfd_mips_elf_check_relocs (abfd, info, 
 
 	case R_MIPS_32:
 	case R_MIPS_REL32:
+	case R_MIPS_64:
 	  if ((info->shared || h != NULL)
 	      && (sec->flags & SEC_ALLOC) != 0)
 	    {
@@ -6996,7 +7053,7 @@ _bfd_mips_elf_check_relocs (abfd, info, 
 		  /* We only need to copy this reloc if the symbol is
                      defined in a dynamic object.  */
 		  hmips = (struct mips_elf_link_hash_entry *) h;
-		  ++hmips->mips_32_relocs;
+		  ++hmips->possibly_dynamic_relocs;
 		}
 	     
 	      /* Even though we don't directly need a GOT entry for
@@ -7189,9 +7246,10 @@ _bfd_mips_elf_adjust_dynamic_symbol (inf
      file.  */
   hmips = (struct mips_elf_link_hash_entry *) h;
   if (! info->relocateable
-      && hmips->mips_32_relocs != 0
+      && hmips->possibly_dynamic_relocs != 0
       && (h->elf_link_hash_flags & ELF_LINK_HASH_DEF_REGULAR) == 0)
-    mips_elf_allocate_dynamic_relocations (dynobj, hmips->mips_32_relocs);
+    mips_elf_allocate_dynamic_relocations (dynobj, 
+					   hmips->possibly_dynamic_relocs);
 
   /* For a function, create a stub, if needed. */
   if (h->type == STT_FUNC

Re: PATCH for 64-bit MIPS ELF buglets

[Mark Mitchell]

>>>>> "Ian" == Ian Lance Taylor <ian@zembu.com> writes:

    Ian> I'm afraid that there isn't any documentation on those
    Ian> relocations.  I just wrote them so that they worked.  I can
    Ian> probably answer any questions you have.

OK, I'll try to make this easy on myself. :-)

Can you describe, in the same sort of way used in the "official" MIPS
ABI documentation what R_MIPS16_26 and R_MIPS16_GPREL do?

In other words, what bits are relocated, and what calculation is
performed to calculate their values?  Once I know that, it should be
easy to do the right thing in mips_elf_calculate_relocation (and, if
truly odd things happen, in mips_elf_perform_relocation).

I have to admit that I find the code that was there before pretty much
impenetrable.  That goes for the 16/32-bit stub stuff as well.  I
understand what the point is (16-bit code and 32-bit code needs stubs
when calling each other), but I don't understand what the code is
doing.

Bottom line: the best way to get this fixed is going to be if you can
give me a complete roadmap.  Otherwise, I'm going to be bumbling about
in the dark, which will probably lots of both of our time, while we
iterate until we get it right.

Thanks,

--
Mark Mitchell                   mark@codesourcery.com
CodeSourcery, LLC               http://www.codesourcery.com

Re: PATCH for 64-bit MIPS ELF buglets

[Ian Lance Taylor]

   From: Mark Mitchell <mark@codesourcery.com>
   Date: Mon, 12 Jul 1999 17:30:53 -0700

   Can you describe, in the same sort of way used in the "official" MIPS
   ABI documentation what R_MIPS16_26 and R_MIPS16_GPREL do?

I'm going to make this up; if what I say clearly contradicts the code,
follow the code.

R_MIPS16_26 is used for the mips16 jal and jalx instructions.  Most
mips16 instructions are 16 bits, but these instructions are 32 bits.

The format of these instructions is:

    +--------------+--------------------------------+
    !     JALX     ! X!   Imm 20:16  !   Imm 25:21  !
    +--------------+--------------------------------+
    !	  	  Immediate  15:0		    !
    +-----------------------------------------------+

JALX is the 5-bit value 00011.  X is 0 for jal, 1 for jalx.  Note that
the immediate value in the first word is swapped.

When producing a relocateable object file, handle R_MIPS16_26 mostly
like R_MIPS_26.  In particular, store the addend as a straight 26-bit
value in a 32-bit instruction.  (gas makes life simpler for itself by
never adjusting a R_MIPS16_26 reloc to be against a section, so the
addend is always zero).  However, the 32 bit instruction is stored as
2 16-bit values, rather than a single 32-bit value.  In a big-endian
file, the result is the same; in a little-endian file, the two 16-bit
halves of the 32 bit value are swapped.  This is so that a
disassembler can recognize the jal instruction.

When doing a final link, treat it as a 32 bit instruction stored as
two 16-bit values.  The addend A is the contents of the targ26 field.
The calculation is the same as R_MIPS_26.  When storing the calculated
value, reorder the immediate value as shown above, and don't forget to
store the value as two 16-bit values.

To put it in MIPS ABI terms, the relocation field is T-targ26-16,
defined as

big-endian:
    +--------+----------------------+
    |        |                      |
    |        |    targ26-16         |
    |31    26|25                   0|
    +--------+----------------------+

little-endian:
    +----------+------+-------------+
    |          |      |             |
    |  sub1    |      |     sub2    |
    |0        9|10  15|16         31|
    +----------+--------------------+
where targ26-16 is sub1 followed by sub2 (i.e., the addend field A is
((sub1 << 16) | sub2)).

When producing a relocateable object file, the calculation is
    (((A < 2) | (P & 0xf0000000) + S) >> 2)
When producing a fully linked file, the calculation is
    let R = (((A < 2) | (P & 0xf0000000) + S) >> 2)
    ((R & 0x1f0000) << 5) | ((R & 0x3e00000) >> 5) | (R & 0xffff)


R_MIPS16_GPREL is used for GP-relative addressing in mips16 mode.  A
typical instruction will have a format like this:

    +--------------+--------------------------------+
    !    EXTEND    !     Imm 10:5    !   Imm 15:11  !
    +--------------+--------------------------------+
    !    Major     !   rx   !   ry   !   Imm  4:0   !
    +--------------+--------------------------------+

EXTEND is the five bit value 11110.  Major is the instruction opcode.

This is handled exactly like R_MIPS_GPREL16, except that the addend is
retrieved and stored as shown in this diagram; that is, the Imm fields
above replace the V-rel16 field.

   I have to admit that I find the code that was there before pretty much
   impenetrable.  That goes for the 16/32-bit stub stuff as well.  I
   understand what the point is (16-bit code and 32-bit code needs stubs
   when calling each other), but I don't understand what the code is
   doing.

The mips16 can operate in either 16-bit mode or 32-bit mode.  This is
signalled by the low-order bit of the PC; when it is 1, the chip is in
16-bit mode.  The low-order bit is forced to zero when fetching
instructions.

Thus the mips16 can switch between modes any time it does a jump to
register (jr, jalr).  When doing an absolute jump (j, jal), the low
order bit is not represented, and accordingly the mode does not
change.  A new 32-bit instruction, jalx, does a jump and also changes
the mode.  There is a similar jalx 16-bit instruction.

Define a 16-bit symbol as a text symbol at the start of a sequence of
16-bit instructions.  A 16-bit symbol is marked in an object file
using the st_other field; a value of STO_MIPS16 indicates a 16-bit
symbol.

A R_MIPS_26 relocation implies a 32-bit instruction.  When such a
relocation refers to a 16 bit symbol, the opcode is required to be
jal.  The linker automatically converts it to jalx.

Similarly, a R_MIPS16_26 relocation implies a 16-bit instruction.
When such a relocation refers to a 32-bit symbol, the opcode is
required to be jal.  The linker automatically converts it to jalx.


This is complicated by floating point parameters and return values.
On the mips16, 16-bit code can not refer to the floating point
registers.  Therefore, when calling between 32-bit code and 16-bit
code and passing or returning floating point values, stub code is
needed to adjust the values.  This stub code is automatically
generated by the compiler (see gcc/config/mips/mips.c).  The linker is
responsible for using the stub code when it is needed, and discarding
it when it is not.

I quote elf32-mips.c:

  /* The mips16 compiler uses a couple of special sections to handle
     floating point arguments.

     Section names that look like .mips16.fn.FNNAME contain stubs that
     copy floating point arguments from the fp regs to the gp regs and
     then jump to FNNAME.  If any 32 bit function calls FNNAME, the
     call should be redirected to the stub instead.  If no 32 bit
     function calls FNNAME, the stub should be discarded.  We need to
     consider any reference to the function, not just a call, because
     if the address of the function is taken we will need the stub,
     since the address might be passed to a 32 bit function.

     Section names that look like .mips16.call.FNNAME contain stubs
     that copy floating point arguments from the gp regs to the fp
     regs and then jump to FNNAME.  If FNNAME is a 32 bit function,
     then any 16 bit function that calls FNNAME should be redirected
     to the stub instead.  If FNNAME is not a 32 bit function, the
     stub should be discarded.

     .mips16.call.fp.FNNAME sections are similar, but contain stubs
     which call FNNAME and then copy the return value from the fp regs
     to the gp regs.  These stubs store the return value in $18 while
     calling FNNAME; any function which might call one of these stubs
     must arrange to save $18 around the call.  (This case is not
     needed for 32 bit functions that call 16 bit functions, because
     16 bit functions always return floating point values in both
     $f0/$f1 and $2/$3.)

     Note that in all cases FNNAME might be defined statically.
     Therefore, FNNAME is not used literally.  Instead, the relocation
     information will indicate which symbol the section is for.

     We record any stubs that we find in the symbol table.  */

You can see this in action in various places.  In the relocate_section
function, the code needs to check for calls to a function which has a
stub, and to convert the code to call the stub when switching between
16-bit and 32-bit mode.  This is a matter of changing the target of
the relocation, as though it referred to the stub rather than to the
symbol to which it actually refers.  The version of relocate_section
before your changes makes the right checks.

Ian

Re: PATCH for 64-bit MIPS ELF buglets

[Ian Lance Taylor]

   Date: 12 Jul 1999 23:10:19 -0400
   From: Ian Lance Taylor <ian@zembu.com>

   A R_MIPS_26 relocation implies a 32-bit instruction.  When such a
   relocation refers to a 16 bit symbol, the opcode is required to be
   jal.  The linker automatically converts it to jalx.

   Similarly, a R_MIPS16_26 relocation implies a 16-bit instruction.
   When such a relocation refers to a 32-bit symbol, the opcode is
   required to be jal.  The linker automatically converts it to jalx.

A small correction: in either case, the opcode is required to be
either jal or jalx.  An instance of jal is converted to jalx.

Ian

Re: PATCH for 64-bit MIPS ELF buglets

[Mark Mitchell]

Thanks for the detailed explanation.  I've not read it in detail yet,
but it looks like what I needed.  I'll also put your words into
elf32-mips.c; that effort should not be lost on future generations.

I'll try to get this stuff fixed up later this week.

--
Mark Mitchell                   mark@codesourcery.com
CodeSourcery, LLC               http://www.codesourcery.com

Re: PATCH for 64-bit MIPS ELF buglets

[Mark Mitchell]

Here's a patch (checked in) which attempts to restore R_MIPS16_26
relocation support.  (Not the stub functionality, just the relocation.)

Ian, please check this over, especially the code in
mips_elf_perform_relocation.

Thank you,

--
Mark Mitchell                   mark@codesourcery.com
CodeSourcery, LLC               http://www.codesourcery.com

1999-07-13  Mark Mitchell  <mark@codesourcery.com>

	* elf32-mips.c (mips_elf_calculate_relocation): Handle R_MIPS16_26.
	(mips_elf_relocate_section): Adjust calling sequence for 
	mips_elf_perform_relocation.
	(mips_elf_perform_relocation): Take additional argument.  Handle
	R_MIPS16_26.  Use bfd_put for convenience.

Index: elf32-mips.c
===================================================================
RCS file: /cvs/binutils/binutils/bfd/elf32-mips.c,v
retrieving revision 1.21
diff -u -p -r1.21 elf32-mips.c
--- elf32-mips.c	1999/07/13 18:21:19	1.21
+++ elf32-mips.c	1999/07/14 06:52:14
@@ -170,7 +170,8 @@ static bfd_reloc_status_type mips_elf_ca
 static bfd_vma mips_elf_obtain_contents
   PARAMS ((reloc_howto_type *, const Elf_Internal_Rela *, bfd *, bfd_byte *));
 static void mips_elf_perform_relocation
-  PARAMS ((reloc_howto_type *, const Elf_Internal_Rela *, bfd_vma,
+  PARAMS ((struct bfd_link_info *, reloc_howto_type *, 
+	   const Elf_Internal_Rela *, bfd_vma,
 	   bfd *, bfd_byte *));
 static boolean mips_elf_assign_gp PARAMS ((bfd *, bfd_vma *));
 static boolean mips_elf_sort_hash_table_f 
@@ -5883,6 +5884,12 @@ mips_elf_calculate_relocation (abfd, 
       value &= howto->dst_mask;
       break;
 
+    case R_MIPS16_26:
+      /* The calculation for R_MIPS_26 is just the same as for an
+	 R_MIPS_26.  It's only the storage of the relocated field into
+	 the output file that's different.  That's handle in
+	 mips_elf_perform_relocation.  So, we just fall through to the
+	 R_MIPS_26 case here.  */
     case R_MIPS_26:
       if (local_p)
 	value = (((addend << 2) | (p & 0xf0000000)) + symbol) >> 2;
@@ -5922,7 +5929,7 @@ mips_elf_calculate_relocation (abfd, 
 
 	     Here $t9 holds the address of the function being called,
 	     as required by the MIPS ELF ABI.  The R_MIPS_LO16
-	     relocation can easily overlfow in this situation, but the
+	     relocation can easily overflow in this situation, but the
 	     R_MIPS_HI16 relocation will handle the overflow.
 	     Therefore, we consider this a bug in the MIPS ABI, and do
 	     not check for overflow here.  */
@@ -6037,7 +6044,6 @@ mips_elf_calculate_relocation (abfd, 
       /* We don't do anything with these at present.  */
       return bfd_reloc_continue;
 
-    case R_MIPS16_26:
     case R_MIPS16_GPREL:
       /* These relocations, used for MIPS16, are not clearly
 	 documented anywhere.  What do they do?  */
@@ -6107,7 +6113,9 @@ mips_elf_obtain_contents (howto, relocat
    Returns false if anything goes wrong.  */
 
 static void
-mips_elf_perform_relocation (howto, relocation, value, input_bfd, contents)
+mips_elf_perform_relocation (info, howto, relocation, value,
+			     input_bfd, contents)
+     struct bfd_link_info *info;
      reloc_howto_type *howto;
      const Elf_Internal_Rela *relocation;
      bfd_vma value;
@@ -6115,48 +6123,105 @@ mips_elf_perform_relocation (howto, relo
      bfd_byte *contents;
 {
   bfd_vma x;
-  bfd_byte *location = contents + relocation->r_offset;
+  bfd_byte *location;
+
+  /* Figure out where the relocation is occurring.  */
+  location = contents + relocation->r_offset;
 
   /* Obtain the current value.  */
   x = mips_elf_obtain_contents (howto, relocation, input_bfd, contents);
 
   /* Clear the field we are setting.  */
   x &= ~howto->dst_mask;
-
-  /* Set the field.  */
-  x |= (value & howto->dst_mask);
-
-  /* Put the value into the output.  */
-  switch (bfd_get_reloc_size (howto))
-    {
-    case 0:
-      x = 0;
-      break;
 
-    case 1:
-      bfd_put_8 (input_bfd, x, location);
-      break;
+  /* If this is the R_MIPS16_26 relocation, we must store the
+     value in a funny way.  */
+  if (ELF32_R_TYPE (relocation->r_info) == R_MIPS16_26)
+    {
+      /* R_MIPS16_26 is used for the mips16 jal and jalx instructions.
+	 Most mips16 instructions are 16 bits, but these instructions
+	 are 32 bits.
+
+	 The format of these instructions is:
+
+	 +--------------+--------------------------------+
+	 !     JALX     ! X!   Imm 20:16  !   Imm 25:21  !
+	 +--------------+--------------------------------+
+	 !	  	  Immediate  15:0		    !
+	 +-----------------------------------------------+
+	 
+	 JALX is the 5-bit value 00011.  X is 0 for jal, 1 for jalx.
+	 Note that the immediate value in the first word is swapped.
+
+	 When producing a relocateable object file, R_MIPS16_26 is
+	 handled mostly like R_MIPS_26.  In particular, the addend is
+	 stored as a straight 26-bit value in a 32-bit instruction.
+	 (gas makes life simpler for itself by never adjusting a
+	 R_MIPS16_26 reloc to be against a section, so the addend is
+	 always zero).  However, the 32 bit instruction is stored as 2
+	 16-bit values, rather than a single 32-bit value.  In a
+	 big-endian file, the result is the same; in a little-endian
+	 file, the two 16-bit halves of the 32 bit value are swapped.
+	 This is so that a disassembler can recognize the jal
+	 instruction.
+
+	 When doing a final link, R_MIPS16_26 is treated as a 32 bit
+	 instruction stored as two 16-bit values.  The addend A is the
+	 contents of the targ26 field.  The calculation is the same as
+	 R_MIPS_26.  When storing the calculated value, reorder the
+	 immediate value as shown above, and don't forget to store the
+	 value as two 16-bit values.
+
+	 To put it in MIPS ABI terms, the relocation field is T-targ26-16,
+	 defined as
+	 
+	 big-endian:
+	 +--------+----------------------+
+	 |        |                      |
+	 |        |    targ26-16         |
+	 |31    26|25                   0|
+	 +--------+----------------------+
+	 
+	 little-endian:
+	 +----------+------+-------------+
+	 |          |      |             |
+	 |  sub1    |      |     sub2    |
+	 |0        9|10  15|16         31|
+	 +----------+--------------------+
+	 where targ26-16 is sub1 followed by sub2 (i.e., the addend field A is
+	 ((sub1 << 16) | sub2)).
+	 
+	 When producing a relocateable object file, the calculation is
+	 (((A < 2) | (P & 0xf0000000) + S) >> 2)
+	 When producing a fully linked file, the calculation is
+	 let R = (((A < 2) | (P & 0xf0000000) + S) >> 2)
+	 ((R & 0x1f0000) << 5) | ((R & 0x3e00000) >> 5) | (R & 0xffff)  */
+
+      if (!info->relocateable)
+	/* Shuffle the bits according to the formula above.  */
+	value = (((value & 0x1f0000) << 5) 
+		 | ((value & 0x3e00000) >> 5) 
+		 | (value & 0xffff));
+      
+      /* Perform the relocation.  */
+      x |= (value & howto->dst_mask);
 
-    case 2:
-      bfd_put_16 (input_bfd, x, location);
-      break;
+      /* Swap the high- and low-order 16 bits on little-endian
+         systems.  */
+      if (bfd_little_endian (input_bfd))
+	x = (((x & 0xffff) << 16)
+	     | (((x & 0xffff0000) >> 16) & 0xffff));
 
-    case 4:
+      /* Store the value.  */
       bfd_put_32 (input_bfd, x, location);
-      break;
+      return;
+    }
 
-    case 8:
-#ifdef BFD64
-      bfd_put_64 (input_bfd, x, location);
-#else
-      abort ();
-#endif
-      break;
+  /* Set the field.  */
+  x |= (value & howto->dst_mask);
 
-    default:
-      abort ();
-      break;
-    }
+  /* Put the value into the output.  */
+  bfd_put (8 * bfd_get_reloc_size (howto), input_bfd, x, location);
 }
 
 /* Relocate a MIPS ELF section.  */
@@ -6376,7 +6441,7 @@ _bfd_mips_elf_relocate_section (output_b
 	}
 
       /* Actually perform the relocation.  */
-      mips_elf_perform_relocation (howto, rel, value, input_bfd, 
+      mips_elf_perform_relocation (info, howto, rel, value, input_bfd, 
 				   contents);
     }
 

Re: PATCH for 64-bit MIPS ELF buglets

[Mark Mitchell]

Ian --

  Here's a patch which attempts to restore the MIPS16 stub handling
stuff.  OK to check in?

--
Mark Mitchell                   mark@codesourcery.com
CodeSourcery, LLC               http://www.codesourcery.com

1999-07-15  Mark Mitchell  <mark@codesourcery.com>

	* elf32-mips.c (mips_elf_stub_section_p): New function.
	(mips_elf_calculate_relocation): Handle MIPS16 stub functions.
	(mips_elf_relocate_section): Adjust calling sequence for
	mips_elf_calculate_relocation and mips_elf_perform_relocation.
	(mips_elf_perform_relocation): Turn `jal' into `jalx' where
	required.

Index: elf32-mips.c
===================================================================
RCS file: /cvs/binutils/binutils/bfd/elf32-mips.c,v
retrieving revision 1.26
diff -u -p -r1.26 elf32-mips.c
--- elf32-mips.c	1999/07/14 19:00:32	1.26
+++ elf32-mips.c	1999/07/15 17:29:25
@@ -168,13 +168,14 @@ static boolean mips_elf_next_lo16_addend
 static bfd_reloc_status_type mips_elf_calculate_relocation
   PARAMS ((bfd *, bfd *, asection *, struct bfd_link_info *,
 	   const Elf_Internal_Rela *, bfd_vma, reloc_howto_type *,
-	   Elf_Internal_Sym *, asection **, bfd_vma *, const char **));
+	   Elf_Internal_Sym *, asection **, bfd_vma *, const char **,
+	   boolean *));
 static bfd_vma mips_elf_obtain_contents
   PARAMS ((reloc_howto_type *, const Elf_Internal_Rela *, bfd *, bfd_byte *));
-static void mips_elf_perform_relocation
+static boolean mips_elf_perform_relocation
   PARAMS ((struct bfd_link_info *, reloc_howto_type *, 
 	   const Elf_Internal_Rela *, bfd_vma,
-	   bfd *, bfd_byte *));
+	   bfd *, asection *, bfd_byte *, boolean));
 static boolean mips_elf_assign_gp PARAMS ((bfd *, bfd_vma *));
 static boolean mips_elf_sort_hash_table_f 
   PARAMS ((struct mips_elf_link_hash_entry *, PTR));
@@ -192,6 +193,8 @@ static unsigned int mips_elf_create_dyna
 	   long, bfd_vma, asection *));
 static void mips_elf_allocate_dynamic_relocations 
   PARAMS ((bfd *, unsigned int));
+static boolean mips_elf_stub_section_p 
+  PARAMS ((bfd *, asection *));
 
 /* The level of IRIX compatibility we're striving for.  */
 
@@ -5662,6 +5665,8 @@ mips_elf_create_dynamic_relocation (outp
    RELOCATION; RELOCATION->R_ADDEND is ignored.
 
    The result of the relocation calculation is stored in VALUEP.
+   REQUIRE_JALXP indicates whether or not the opcode used with this
+   relocation must be JALX.
 
    This function returns bfd_reloc_continue if the caller need take no
    further action regarding this relocation, bfd_reloc_notsupported if
@@ -5679,7 +5684,8 @@ mips_elf_calculate_relocation (abfd, 
 			       local_syms,
 			       local_sections,
 			       valuep,
-			       namep) 
+			       namep,
+			       require_jalxp) 
      bfd *abfd;
      bfd *input_bfd;
      asection *input_section;
@@ -5691,6 +5697,7 @@ mips_elf_calculate_relocation (abfd, 
      asection **local_sections;
      bfd_vma *valuep;
      const char **namep;
+     boolean *require_jalxp;
 {
   /* The eventual value we will return.  */
   bfd_vma value;
@@ -5822,7 +5827,65 @@ mips_elf_calculate_relocation (abfd, 
 	  return bfd_reloc_undefined;
 	}
     }
+  
+  /* If this is a 32-bit call to a 16-bit function, we need to
+     redirect the call to the stub, unless we're already *in* a stub.  */
+  if (r_type != R_MIPS16_26 && !info->relocateable
+      && ((h != NULL && h->fn_stub != NULL)
+	  || (local_p && elf_tdata (input_bfd)->local_stubs != NULL
+	      && elf_tdata (input_bfd)->local_stubs[r_symndx] != NULL))
+      && !mips_elf_stub_section_p (input_bfd, input_section))
+    {
+      /* This is a 32-bit call to a 16-bit function.  We should
+	 have already noticed that we were going to need the
+	 stub.  */
+      if (local_p)
+	sec = elf_tdata (input_bfd)->local_stubs[r_symndx];
+      else
+	{
+	  BFD_ASSERT (h->need_fn_stub);
+	  sec = h->fn_stub;
+	}
 
+      symbol = sec->output_section->vma + sec->output_offset;
+      *require_jalxp = true;
+    }
+  /* If this is a 16-bit call to a 32-bit function, we need to
+     redirect the call to the stub.  */
+  else if (r_type == R_MIPS16_26 && !info->relocateable
+	   && h != NULL 
+	   && (h->call_stub != NULL || h->call_fp_stub != NULL)
+	   && h->root.other != STO_MIPS16)
+    {
+      /* If both call_stub and call_fp_stub are defined, we can figure
+	 out which one to use by seeing which one appears in the input
+	 file.  */
+      if (h->call_stub != NULL && h->call_fp_stub != NULL)
+	{
+	  asection *o;
+
+	  for (o = input_bfd->sections; o != NULL; o = o->next)
+	    {
+	      if (strncmp (bfd_get_section_name (input_bfd, o),
+			   CALL_FP_STUB, sizeof CALL_FP_STUB - 1) == 0)
+		{
+		  sec = h->call_fp_stub;
+		  break;
+		}
+	    }
+	  if (sec == NULL)
+	    sec = h->call_stub;
+	}
+      else if (h->call_stub != NULL)
+	sec = h->call_stub;
+      else
+	sec = h->call_fp_stub;
+
+      BFD_ASSERT (sec->_raw_size > 0);
+      symbol = sec->output_section->vma + sec->output_offset;
+      *require_jalxp = true;
+    }
+
   /* If we haven't already determined the GOT offset, or the GP value,
      and we're going to need it, get it now.  */
   switch (r_type)
@@ -6114,19 +6177,24 @@ mips_elf_obtain_contents (howto, relocat
 /* It has been determined that the result of the RELOCATION is the
    VALUE.  Use HOWTO to place VALUE into the output file at the
    appropriate position.  The SECTION is the section to which the
-   relocatin applies.
+   relocatin applies.  If REQUIRE_JALX is true, then the opcode used
+   for the relocation must be either JAL or JALX, and it is
+   unconditionally converted to JALX.
 
    Returns false if anything goes wrong.  */
 
-static void
+static boolean
 mips_elf_perform_relocation (info, howto, relocation, value,
-			     input_bfd, contents)
+			     input_bfd, input_section, 
+			     contents, require_jalx)
      struct bfd_link_info *info;
      reloc_howto_type *howto;
      const Elf_Internal_Rela *relocation;
      bfd_vma value;
      bfd *input_bfd;
+     asection *input_section;
      bfd_byte *contents;
+     boolean require_jalx;
 {
   bfd_vma x;
   bfd_byte *location;
@@ -6239,6 +6307,24 @@ mips_elf_perform_relocation (info, howto
   /* Set the field.  */
   x |= (value & howto->dst_mask);
 
+  if (require_jalx)
+    {
+      /* If the opcode is not JAL or JALX, there's a problem.  */
+      if ((x & 0xf8000000) != 0x18000000)
+	{
+	  (*_bfd_error_handler)
+	    (_("%s: %s+0x%lx: jump to stub routine which is not jal"),
+	     bfd_get_filename (input_bfd),
+	     input_section->name,
+	     (unsigned long) relocation->r_offset);
+	  bfd_set_error (bfd_error_bad_value);
+	  return false;
+	}
+
+      /* Make this the JALX opcode.  */
+      x = (x & ~0xfc000000) | 0x1c000000;
+    }
+
   /* Swap the high- and low-order 16 bits on little-endian systems
      when doing a MIPS16 relocation.  */
   if ((ELF32_R_TYPE (relocation->r_info) == R_MIPS16_GPREL
@@ -6248,6 +6334,21 @@ mips_elf_perform_relocation (info, howto
   
   /* Put the value into the output.  */
   bfd_put (8 * bfd_get_reloc_size (howto), input_bfd, x, location);
+  return true;
+}
+
+/* Returns true if SECTION is a MIPS16 stub section.  */
+
+static boolean
+mips_elf_stub_section_p (abfd, section)
+     bfd *abfd;
+     asection *section;
+{
+  const char *name = bfd_get_section_name (abfd, section);
+
+  return (strncmp (name, FN_STUB, sizeof FN_STUB - 1) == 0
+	  || strncmp (name, CALL_STUB, sizeof CALL_STUB - 1) == 0
+	  || strncmp (name, CALL_FP_STUB, sizeof CALL_FP_STUB - 1) == 0);
 }
 
 /* Relocate a MIPS ELF section.  */
@@ -6279,6 +6380,7 @@ _bfd_mips_elf_relocate_section (output_b
       const char *name;
       bfd_vma value;
       reloc_howto_type *howto;
+      boolean require_jalx;
 
       /* Find the relocation howto for this relocation.  */
       if (ELF32_R_TYPE (rel->r_info) == R_MIPS_64
@@ -6383,7 +6485,8 @@ _bfd_mips_elf_relocate_section (output_b
 					     local_syms,
 					     local_sections,
 					     &value,
-					     &name))
+					     &name,
+					     &require_jalx))
 	{
 	case bfd_reloc_continue:
 	  /* There's nothing to do.  */
@@ -6476,8 +6579,10 @@ _bfd_mips_elf_relocate_section (output_b
 	}
 
       /* Actually perform the relocation.  */
-      mips_elf_perform_relocation (info, howto, rel, value, input_bfd, 
-				   contents);
+      if (!mips_elf_perform_relocation (info, howto, rel, value, input_bfd, 
+					input_section, contents,
+					require_jalx))
+	return false;
     }
 
   return true;