public inbox for binutils@sourceware.org
 help / color / mirror / Atom feed
* Reloc changes to bfd/elf32-mips.c
@ 1999-09-27  4:30 Geoff Keating
  1999-09-27 12:37 ` Mark Mitchell
  1999-09-28  0:54 ` Mark Mitchell
  0 siblings, 2 replies; 19+ messages in thread
From: Geoff Keating @ 1999-09-27  4:30 UTC (permalink / raw)
  To: gavin, Mark Mitchell, binutils, brendan

This all seems to have broken recently.

Mark, this is the correct fix for the bug Brendan reported to you.

I'm not sure exactly who can approve changes to the sourceware
binutils; if Gavin or Mark could do this (at least for MIPS), it'd be
good to know as the current sequence of patches will not be the last.
I'm also not sure who can check stuff in.

Anyway, is the patch OK?

-- 
Geoffrey Keating <geoffk@cygnus.com>


===File ~/patches/cygnus/tx49-bin-literal.patch=============
md5sum: 701810431d1e9329 132c34cee8105e0f 195224
Index: binutils/bfd/ChangeLog
0a
Mon Sep 27 14:50:05 1999  Geoffrey Keating  <geoffk@cygnus.com>

	* elf32-mips.c (mips_elf_calculate_relocation): R_MIPS_LITERAL
	relocs also need the GP value.
	(_bfd_mips_elf_relocate_section): Handle unpaired LO16 relocs
 	properly.  Always use the R_MIPS_64 HOWTO when a 64-bit BFD and
 	processing a R_MIPS_64 relocation.  Handle sign-extension for
 	R_MIPS_64 correctly.  Correct the GP value for R_MIPS_LITERAL
 	relocs too.
	(mips_elf_sign_extend): Behave properly with 'long long'.

.
Changed files:
binutils/bfd/ChangeLog
binutils/bfd/elf32-mips.c
md5sum: 69a8865364de884b 0b609a637fed2fec 269102
--- /sloth/disk0/co/binutils-mainline/binutils/bfd/elf32-mips.c	Fri Aug 27 10:08:49 1999
+++ binutils/bfd/elf32-mips.c	Mon Sep 27 20:39:58 1999
@@ -5195,7 +5195,7 @@ mips_elf_sign_extend (value, bits)
      bfd_vma value;
      int bits;
 {
-  if (value & (1 << (bits - 1)))
+  if (value & ((bfd_vma)1 << (bits - 1)))
     /* VALUE is negative.  */
     value |= ((bfd_vma) - 1) << bits;      
   
@@ -6072,6 +6072,7 @@ mips_elf_calculate_relocation (abfd, 
     case R_MIPS_LO16:
     case R_MIPS_GPREL16:
     case R_MIPS_GPREL32:
+    case R_MIPS_LITERAL:
       gp0 = _bfd_get_gp_value (input_bfd);
       gp = _bfd_get_gp_value (abfd);
       break;
@@ -6538,9 +6539,7 @@ _bfd_mips_elf_relocate_section (output_b
   Elf_Internal_Rela *rel;
   const Elf_Internal_Rela *relend;
   bfd_vma addend;
-  bfd_vma last_hi16_addend;
   boolean use_saved_addend_p = false;
-  boolean last_hi16_addend_valid_p = false;
   struct elf_backend_data *bed;
 
   bed = get_elf_backend_data (output_bfd);
@@ -6557,6 +6556,7 @@ _bfd_mips_elf_relocate_section (output_b
       int r_type = ELF32_R_TYPE (rel->r_info);
 
       /* Find the relocation howto for this relocation.  */
+#ifndef BFD64
       if (r_type == 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 
@@ -6566,6 +6566,7 @@ _bfd_mips_elf_relocate_section (output_b
 	   stored value is sign-extended to 64 bits.  */
 	howto = elf_mips_howto_table + R_MIPS_32;
       else
+#endif
 	howto = mips_rtype_to_howto (r_type);
 
       if (!use_saved_addend_p)
@@ -6628,26 +6629,11 @@ _bfd_mips_elf_relocate_section (output_b
 		  l &= lo16_howto->src_mask;
 		  l = mips_elf_sign_extend (l, 16);
 
-		  /* Save the high-order bit for later.  When we
-		     encounter the R_MIPS_LO16 relocation we will need
-		     them again.  */
 		  addend <<= 16;
-		  last_hi16_addend = addend;
-		  last_hi16_addend_valid_p = true;
 
 		  /* Compute the combined addend.  */
 		  addend += l;
 		}
-	      else if (r_type == R_MIPS_LO16) 
-		{
-		  /* Used the saved HI16 addend.  */
-		  if (!last_hi16_addend_valid_p)
-		    {
-		      bfd_set_error (bfd_error_bad_value);
-		      return false;
-		    }
-		  addend |= last_hi16_addend;
-		}
 	      else if (r_type == R_MIPS16_GPREL)
 		{
 		  /* The addend is scrambled in the object file.  See
@@ -6680,7 +6666,8 @@ _bfd_mips_elf_relocate_section (output_b
 
 	  if (r_type == R_MIPS16_GPREL 
 	      || r_type == R_MIPS_GPREL16
-	      || r_type == R_MIPS_GPREL32)
+	      || r_type == R_MIPS_GPREL32
+	      || r_type == R_MIPS_LITERAL)
 	    addend -= (_bfd_get_gp_value (output_bfd)
 		       - _bfd_get_gp_value (input_bfd));
 	  else if (r_type == R_MIPS_26 || r_type == R_MIPS16_26)
@@ -6813,8 +6800,7 @@ _bfd_mips_elf_relocate_section (output_b
 	  /* 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;
+	  value = mips_elf_sign_extend (value, 32);
 #else /* !BFD64 */
 	  /* In the 32-bit VMA case, we must handle sign-extension and
 	     endianness manually.  */
============================================================

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

* Re: Reloc changes to bfd/elf32-mips.c
  1999-09-27  4:30 Reloc changes to bfd/elf32-mips.c Geoff Keating
@ 1999-09-27 12:37 ` Mark Mitchell
  1999-09-28  0:54 ` Mark Mitchell
  1 sibling, 0 replies; 19+ messages in thread
From: Mark Mitchell @ 1999-09-27 12:37 UTC (permalink / raw)
  To: geoffk; +Cc: gavin, binutils, brendan

This one I'll look at, since it's my fault.  I'll get back to you
later today.  I'm not sure I can approve it as such, but I can at
least give you my thoughts and hope to lessen Ian's burden; there's no
reason for him to deal with my bugs.

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

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

* Re: Reloc changes to bfd/elf32-mips.c
  1999-09-27  4:30 Reloc changes to bfd/elf32-mips.c Geoff Keating
  1999-09-27 12:37 ` Mark Mitchell
@ 1999-09-28  0:54 ` Mark Mitchell
  1999-09-28  6:57   ` Ian Lance Taylor
  1999-09-28 20:32   ` Geoff Keating
  1 sibling, 2 replies; 19+ messages in thread
From: Mark Mitchell @ 1999-09-28  0:54 UTC (permalink / raw)
  To: geoffk; +Cc: gavin, binutils, brendan

I'm confused on a couple of points.

  @@ -6557,6 +6556,7 @@ _bfd_mips_elf_relocate_section (output_b
	 int r_type = ELF32_R_TYPE (rel->r_info);

	 /* Find the relocation howto for this relocation.  */
  +#ifndef BFD64
	 if (r_type == 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 
  @@ -6566,6 +6566,7 @@ _bfd_mips_elf_relocate_section (output_b
	     stored value is sign-extended to 64 bits.  */
	  howto = elf_mips_howto_table + R_MIPS_32;
	 else
  +#endif
	  howto = mips_rtype_to_howto (r_type);

That's fine by me, in that I never understood exactly what was going
on there.  But, I don't think it's right.  The idea here is that
32-bit object code can still use R_MIPS_64, according to Ian.  So, I
don't think BFD64 (which says something about how many bits you have
when you're compiling BFD) is the right thing.

  @@ -6628,26 +6629,11 @@ _bfd_mips_elf_relocate_section (output_b
		    l &= lo16_howto->src_mask;
		    l = mips_elf_sign_extend (l, 16);

  -		  /* Save the high-order bit for later.  When we
  -		     encounter the R_MIPS_LO16 relocation we will need
  -		     them again.  */
		    addend <<= 16;
  -		  last_hi16_addend = addend;
  -		  last_hi16_addend_valid_p = true;

		    /* Compute the combined addend.  */
		    addend += l;
		  }
  -	      else if (r_type == R_MIPS_LO16) 
  -		{
  -		  /* Used the saved HI16 addend.  */
  -		  if (!last_hi16_addend_valid_p)
  -		    {
  -		      bfd_set_error (bfd_error_bad_value);
  -		      return false;
  -		    }
  -		  addend |= last_hi16_addend;
  -		}
		else if (r_type == R_MIPS16_GPREL)
		  {
		    /* The addend is scrambled in the object file.  See

Why is all this code going away?  This looks like it will make
R_MIPS_LO16 not work in .rel sections.  The addend for the LO16
relocation is the combination of bits in the HI16 and LO16 relocs, I
think.

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

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

* Re: Reloc changes to bfd/elf32-mips.c
  1999-09-28  0:54 ` Mark Mitchell
@ 1999-09-28  6:57   ` Ian Lance Taylor
  1999-09-28 20:32   ` Geoff Keating
  1 sibling, 0 replies; 19+ messages in thread
From: Ian Lance Taylor @ 1999-09-28  6:57 UTC (permalink / raw)
  To: mark; +Cc: geoffk, gavin, binutils, brendan

   From: Mark Mitchell <mark@codesourcery.com>
   Date: Tue, 28 Sep 1999 00:59:56 -0700

     @@ -6628,26 +6629,11 @@ _bfd_mips_elf_relocate_section (output_b
		       l &= lo16_howto->src_mask;
		       l = mips_elf_sign_extend (l, 16);

     -		  /* Save the high-order bit for later.  When we
     -		     encounter the R_MIPS_LO16 relocation we will need
     -		     them again.  */
		       addend <<= 16;
     -		  last_hi16_addend = addend;
     -		  last_hi16_addend_valid_p = true;

		       /* Compute the combined addend.  */
		       addend += l;
		     }
     -	      else if (r_type == R_MIPS_LO16) 
     -		{
     -		  /* Used the saved HI16 addend.  */
     -		  if (!last_hi16_addend_valid_p)
     -		    {
     -		      bfd_set_error (bfd_error_bad_value);
     -		      return false;
     -		    }
     -		  addend |= last_hi16_addend;
     -		}
		   else if (r_type == R_MIPS16_GPREL)
		     {
		       /* The addend is scrambled in the object file.  See

   Why is all this code going away?  This looks like it will make
   R_MIPS_LO16 not work in .rel sections.  The addend for the LO16
   relocation is the combination of bits in the HI16 and LO16 relocs, I
   think.

When computing the addend for a LO16 relocation, you don't need any
information from the HI16 relocation.  That's just math: the HI16
relocation can only contribute to the upper 16 bits, which the LO16
relocation doesn't use.  (The reverse is not true, because the LO16
relocation bits can carry into the HI16 relocation bits).

I think Geoff is trying to restore an extension we used to have, which
is to permit a LO16 relocation without a HI16 relocation.

Ian

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

* Re: Reloc changes to bfd/elf32-mips.c
  1999-09-28  0:54 ` Mark Mitchell
  1999-09-28  6:57   ` Ian Lance Taylor
@ 1999-09-28 20:32   ` Geoff Keating
  1999-09-28 20:57     ` Ian Lance Taylor
  1999-09-28 22:16     ` Mark Mitchell
  1 sibling, 2 replies; 19+ messages in thread
From: Geoff Keating @ 1999-09-28 20:32 UTC (permalink / raw)
  To: mark; +Cc: gavin, binutils, brendan

> Cc: gavin@cygnus.com, binutils@sourceware.cygnus.com, brendan@cygnus.com
> X-URL: http://www.codesourcery.com
> Organization: CodeSourcery, LLC
> From: Mark Mitchell <mark@codesourcery.com>
> Date: Tue, 28 Sep 1999 00:59:56 -0700
> X-Dispatcher: imput version 990425(IM115)
> 
> 
> I'm confused on a couple of points.
> 
>   @@ -6557,6 +6556,7 @@ _bfd_mips_elf_relocate_section (output_b
> 	 int r_type = ELF32_R_TYPE (rel->r_info);
> 
> 	 /* Find the relocation howto for this relocation.  */
>   +#ifndef BFD64
> 	 if (r_type == 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 
>   @@ -6566,6 +6566,7 @@ _bfd_mips_elf_relocate_section (output_b
> 	     stored value is sign-extended to 64 bits.  */
> 	  howto = elf_mips_howto_table + R_MIPS_32;
> 	 else
>   +#endif
> 	  howto = mips_rtype_to_howto (r_type);
> 
> That's fine by me, in that I never understood exactly what was going
> on there.  But, I don't think it's right.  The idea here is that
> 32-bit object code can still use R_MIPS_64, according to Ian.  So, I
> don't think BFD64 (which says something about how many bits you have
> when you're compiling BFD) is the right thing.

What I think this code was trying to do was make a R_MIPS_64 reloc
work when you have a 32-bit BFD.  If you have a 64-bit BFD, none of
this is necessary, and you can just do the right thing.  The object
format size is irrelevant, you'd have to do a similar thing to make a
R_MIPS_64 reloc work in an elf64 file with a 32-bit BFD (but you'd have
to do many many other things too).

The original code used the R_MIPS_32 howto for a R_MIPS_64 reloc, but
if you do that the addend gets computed wrong in big-endian files
because BFD thinks the addend is 32 bits long and only sees the high
32 bits which are not usually helpful.  It's also possible to usefully
have a R_MIPS_64 reloc with a full 64-bit addend, eg if you write

0:   .dword 0f+0xa000000000000000

I don't know why anyone bothered to do this in the first place, there
aren't that many compilers left that don't support 64-bit values; but
it must have been useful for someone so I left the code in.

>   @@ -6628,26 +6629,11 @@ _bfd_mips_elf_relocate_section (output_b
> 		    l &= lo16_howto->src_mask;
> 		    l = mips_elf_sign_extend (l, 16);
> 
>   -		  /* Save the high-order bit for later.  When we
>   -		     encounter the R_MIPS_LO16 relocation we will need
>   -		     them again.  */
> 		    addend <<= 16;
>   -		  last_hi16_addend = addend;
>   -		  last_hi16_addend_valid_p = true;
> 
> 		    /* Compute the combined addend.  */
> 		    addend += l;
> 		  }
>   -	      else if (r_type == R_MIPS_LO16) 
>   -		{
>   -		  /* Used the saved HI16 addend.  */
>   -		  if (!last_hi16_addend_valid_p)
>   -		    {
>   -		      bfd_set_error (bfd_error_bad_value);
>   -		      return false;
>   -		    }
>   -		  addend |= last_hi16_addend;
>   -		}
> 		else if (r_type == R_MIPS16_GPREL)
> 		  {
> 		    /* The addend is scrambled in the object file.  See
> 
> Why is all this code going away?  This looks like it will make
> R_MIPS_LO16 not work in .rel sections.  The addend for the LO16
> relocation is the combination of bits in the HI16 and LO16 relocs, I
> think.

The addend for the LO16 reloc can be considered as just the bits in
the LO16 reloc; we only care about the least-significant 16 bits.
HI16 relocs are different because there can be a carry from the lower
16 bits.

The problem it fixes is that the assembler does not sort LO16 relocs
so that they all come after some HI16 reloc that matches them; it
sorts HI16 relocs so that they all come before some LO16 reloc that
matches.

For instance, the following .s file:

l1:     b       l2
l3:     lw      $5,%lo(l1+0x10004)($2)
        b       l4
l2:     lui     $2,%hi(l1+0x10004)
        add     $3,$2,$3
        lw      $4,%lo(l1+0x10004)($3)
        b       l3
l4:

produces the following relocs from gas:

RELOCATION RECORDS FOR [.text]:
OFFSET           TYPE              VALUE 
000000000000000c R_MIPS_LO16       .text
0000000000000010 R_MIPS_HI16       .text
000000000000001c R_MIPS_LO16       .text

Contents of section .text:
 0000 10000003 00000000 10000005 8c450004  .............E..
 0010 3c020001 00431820 1000fffb 8c640004  <....C. .....d..

which used to cause ld to fail with an error.  gcc can generate
control flows like this (although some of the branches have to be
conditional).

-- 
Geoffrey Keating <geoffk@cygnus.com>

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

* Re: Reloc changes to bfd/elf32-mips.c
  1999-09-28 20:32   ` Geoff Keating
@ 1999-09-28 20:57     ` Ian Lance Taylor
  1999-09-28 21:52       ` Geoff Keating
  1999-09-28 22:16     ` Mark Mitchell
  1 sibling, 1 reply; 19+ messages in thread
From: Ian Lance Taylor @ 1999-09-28 20:57 UTC (permalink / raw)
  To: geoffk; +Cc: mark, gavin, binutils, brendan

   Date: Wed, 29 Sep 1999 13:30:48 +1000
   From: Geoff Keating <geoffk@ozemail.com.au>

   > From: Mark Mitchell <mark@codesourcery.com>
   > Date: Tue, 28 Sep 1999 00:59:56 -0700
   > 
   > I'm confused on a couple of points.
   > 
   >   @@ -6557,6 +6556,7 @@ _bfd_mips_elf_relocate_section (output_b
   > 	 int r_type = ELF32_R_TYPE (rel->r_info);
   > 
   > 	 /* Find the relocation howto for this relocation.  */
   >   +#ifndef BFD64
   > 	 if (r_type == 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 
   >   @@ -6566,6 +6566,7 @@ _bfd_mips_elf_relocate_section (output_b
   > 	     stored value is sign-extended to 64 bits.  */
   > 	  howto = elf_mips_howto_table + R_MIPS_32;
   > 	 else
   >   +#endif
   > 	  howto = mips_rtype_to_howto (r_type);
   > 
   > That's fine by me, in that I never understood exactly what was going
   > on there.  But, I don't think it's right.  The idea here is that
   > 32-bit object code can still use R_MIPS_64, according to Ian.  So, I
   > don't think BFD64 (which says something about how many bits you have
   > when you're compiling BFD) is the right thing.

   What I think this code was trying to do was make a R_MIPS_64 reloc
   work when you have a 32-bit BFD.

To be precise, this code was trying to make a R_MIPS_64 reloc work
when using 32 bit MIPS ELF.

I invented it because we had 64 bit MIPS chips, but we only supported
the 32 bit MIPS ELF format.  Since the compiler was generating 64 bit
address loads, we needed to have some way to get the right value in a
relocation.  Of course, since we were using a 32 bit object file
format, the correct value was always 32 bits.  The relocation made
sure that it was sign extended correctly.

   If you have a 64-bit BFD, none of
   this is necessary, and you can just do the right thing.  The object
   format size is irrelevant, you'd have to do a similar thing to make a
   R_MIPS_64 reloc work in an elf64 file with a 32-bit BFD (but you'd have
   to do many many other things too).

I don't see why it matters whether you have a 64 bit BFD.  What
matters is how you compute the relocation.

Since this is a 32 bit object file format, all the values going into
this computation are 32 bit values.  The correct way to compute the
value is to compute the 32 bit relocation, and to sign extend the 32
bit result to 64 bits when storing it in memory.

This is a target calculation.  It's not immediately obvious why it
should depend upon the host facilities in any way.

   The original code used the R_MIPS_32 howto for a R_MIPS_64 reloc, but
   if you do that the addend gets computed wrong in big-endian files
   because BFD thinks the addend is 32 bits long and only sees the high
   32 bits which are not usually helpful.

Before Mark's rewrite, the code handled this correctly in
mips_elf_relocate_section:

		  if (r_type == R_MIPS_64 && bfd_big_endian (input_bfd))
		    r = _bfd_relocate_contents (howto, input_bfd,
						addend,
						contents + rel->r_offset + 4);

Perhaps this got dropped somewhere along the way.

Ian

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

* Re: Reloc changes to bfd/elf32-mips.c
  1999-09-28 20:57     ` Ian Lance Taylor
@ 1999-09-28 21:52       ` Geoff Keating
  1999-09-28 22:03         ` Ian Lance Taylor
  0 siblings, 1 reply; 19+ messages in thread
From: Geoff Keating @ 1999-09-28 21:52 UTC (permalink / raw)
  To: ian; +Cc: mark, gavin, binutils, brendan

> Mailing-List: contact binutils-help@sourceware.cygnus.com; run by ezmlm
> List-Unsubscribe: < mailto:binutils-unsubscribe-geoffk=cygnus.com@sourceware.cygnus.com >
> List-Subscribe: < mailto:binutils-subscribe@sourceware.cygnus.com >
> List-Archive: < http://sourceware.cygnus.com/ml/binutils/ >
> List-Post: < mailto:binutils@sourceware.cygnus.com >
> List-Help: < mailto:binutils-help@sourceware.cygnus.com >, < http://sourceware.cygnus.com/ml/#faqs >
> Date: 28 Sep 1999 23:55:39 -0400
> From: Ian Lance Taylor <ian@zembu.com>
> CC: mark@codesourcery.com, gavin@cygnus.com, binutils@sourceware.cygnus.com,
>         brendan@cygnus.com
> 
>    Date: Wed, 29 Sep 1999 13:30:48 +1000
>    From: Geoff Keating <geoffk@ozemail.com.au>
> 
>    > From: Mark Mitchell <mark@codesourcery.com>
>    > Date: Tue, 28 Sep 1999 00:59:56 -0700
>    > 
>    > I'm confused on a couple of points.
>    > 
>    >   @@ -6557,6 +6556,7 @@ _bfd_mips_elf_relocate_section (output_b
>    > 	 int r_type = ELF32_R_TYPE (rel->r_info);
>    > 
>    > 	 /* Find the relocation howto for this relocation.  */
>    >   +#ifndef BFD64
>    > 	 if (r_type == 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 
>    >   @@ -6566,6 +6566,7 @@ _bfd_mips_elf_relocate_section (output_b
>    > 	     stored value is sign-extended to 64 bits.  */
>    > 	  howto = elf_mips_howto_table + R_MIPS_32;
>    > 	 else
>    >   +#endif
>    > 	  howto = mips_rtype_to_howto (r_type);
>    > 
>    > That's fine by me, in that I never understood exactly what was going
>    > on there.  But, I don't think it's right.  The idea here is that
>    > 32-bit object code can still use R_MIPS_64, according to Ian.  So, I
>    > don't think BFD64 (which says something about how many bits you have
>    > when you're compiling BFD) is the right thing.
> 
>    What I think this code was trying to do was make a R_MIPS_64 reloc
>    work when you have a 32-bit BFD.
> 
> To be precise, this code was trying to make a R_MIPS_64 reloc work
> when using 32 bit MIPS ELF.
> 
> I invented it because we had 64 bit MIPS chips, but we only supported
> the 32 bit MIPS ELF format.  Since the compiler was generating 64 bit
> address loads, we needed to have some way to get the right value in a
> relocation.  Of course, since we were using a 32 bit object file
> format, the correct value was always 32 bits.  The relocation made
> sure that it was sign extended correctly.

Yes.

>    If you have a 64-bit BFD, none of
>    this is necessary, and you can just do the right thing.  The object
>    format size is irrelevant, you'd have to do a similar thing to make a
>    R_MIPS_64 reloc work in an elf64 file with a 32-bit BFD (but you'd have
>    to do many many other things too).
> 
> I don't see why it matters whether you have a 64 bit BFD.  What
> matters is how you compute the relocation.

If you have a 32-bit BFD, you can't ask it to load a 64-bit value out
of memory into a bfd_vma; it will abort.

> Since this is a 32 bit object file format, all the values going into
> this computation are 32 bit values.  The correct way to compute the
> value is to compute the 32 bit relocation, and to sign extend the 32
> bit result to 64 bits when storing it in memory.

The addend is stored as a 64-bit value.  I don't know if it should be
considered to have all 64 bits significant (at present it isn't), but
it's still 8 bytes long.

> This is a target calculation.  It's not immediately obvious why it
> should depend upon the host facilities in any way.

Certainly, you should get the same result.  You may need to get the
result in a different way in a 32-bit BFD.

>    The original code used the R_MIPS_32 howto for a R_MIPS_64 reloc, but
>    if you do that the addend gets computed wrong in big-endian files
>    because BFD thinks the addend is 32 bits long and only sees the high
>    32 bits which are not usually helpful.
> 
> Before Mark's rewrite, the code handled this correctly in
> mips_elf_relocate_section:
> 
> 		  if (r_type == R_MIPS_64 && bfd_big_endian (input_bfd))
> 		    r = _bfd_relocate_contents (howto, input_bfd,
> 						addend,
> 						contents + rel->r_offset + 4);
> 
> Perhaps this got dropped somewhere along the way.

It seems so.

-- 
Geoffrey Keating <geoffk@cygnus.com>

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

* Re: Reloc changes to bfd/elf32-mips.c
  1999-09-28 21:52       ` Geoff Keating
@ 1999-09-28 22:03         ` Ian Lance Taylor
  1999-10-06 19:21           ` Geoff Keating
  0 siblings, 1 reply; 19+ messages in thread
From: Ian Lance Taylor @ 1999-09-28 22:03 UTC (permalink / raw)
  To: geoffk; +Cc: mark, gavin, binutils, brendan

   Date: Wed, 29 Sep 1999 14:50:20 +1000
   From: Geoff Keating <geoffk@ozemail.com.au>

We are somehow talking at cross purposes.  I don't see why what you
are saying has much to do with what I am saying.

   >    If you have a 64-bit BFD, none of
   >    this is necessary, and you can just do the right thing.  The object
   >    format size is irrelevant, you'd have to do a similar thing to make a
   >    R_MIPS_64 reloc work in an elf64 file with a 32-bit BFD (but you'd have
   >    to do many many other things too).
   > 
   > I don't see why it matters whether you have a 64 bit BFD.  What
   > matters is how you compute the relocation.

   If you have a 32-bit BFD, you can't ask it to load a 64-bit value out
   of memory into a bfd_vma; it will abort.

Well, yes.  That's why we compute a 32 bit value, and then manually
sign extend it to 64 bits.

   > Since this is a 32 bit object file format, all the values going into
   > this computation are 32 bit values.  The correct way to compute the
   > value is to compute the 32 bit relocation, and to sign extend the 32
   > bit result to 64 bits when storing it in memory.

   The addend is stored as a 64-bit value.  I don't know if it should be
   considered to have all 64 bits significant (at present it isn't), but
   it's still 8 bytes long.

Well, yes.  All the values we can manipulate in a 32 bit object file
format are 32 bits long.  We can't even talk about a longer value.
There is no reason to consider the upper 32 bits of the 64 bit
relocation.  We just need to sign extend correctly, so that the
processor will load the address correctly with a 64 load instruction.

It's true that you can construct a case which uses the upper 32 bits
of a 64 bit relocation.  But since we are using a 32 bit object file
format, such a case is not supported.

The only reason to support R_MIPS_64 is to support the processor's 64
bit load instructions when loading an address, because gcc wants to
generate such instructions.  It's not because the linker can actually
produce 64 bit addresses; it can't.

   > This is a target calculation.  It's not immediately obvious why it
   > should depend upon the host facilities in any way.

   Certainly, you should get the same result.  You may need to get the
   result in a different way in a 32-bit BFD.

If the code gets identical results either way, then the change is OK.

   >    The original code used the R_MIPS_32 howto for a R_MIPS_64 reloc, but
   >    if you do that the addend gets computed wrong in big-endian files
   >    because BFD thinks the addend is 32 bits long and only sees the high
   >    32 bits which are not usually helpful.
   > 
   > Before Mark's rewrite, the code handled this correctly in
   > mips_elf_relocate_section:
   > 
   > 		  if (r_type == R_MIPS_64 && bfd_big_endian (input_bfd))
   > 		    r = _bfd_relocate_contents (howto, input_bfd,
   > 						addend,
   > 						contents + rel->r_offset + 4);
   > 
   > Perhaps this got dropped somewhere along the way.

   It seems so.

Actually, I still some code that seems relevant, under the comment

	  /* In the 32-bit VMA case, we must handle sign-extension and
	     endianness manually.  */

Ian

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

* Re: Reloc changes to bfd/elf32-mips.c
  1999-09-28 20:32   ` Geoff Keating
  1999-09-28 20:57     ` Ian Lance Taylor
@ 1999-09-28 22:16     ` Mark Mitchell
  1999-09-29  8:25       ` Ian Lance Taylor
  1 sibling, 1 reply; 19+ messages in thread
From: Mark Mitchell @ 1999-09-28 22:16 UTC (permalink / raw)
  To: geoffk; +Cc: gavin, binutils, brendan

>>>>> "Geoff" == Geoff Keating <geoffk@ozemail.com.au> writes:

    Geoff> For instance, the following .s file:

    Geoff> l1: b l2 l3: lw $5,%lo(l1+0x10004)($2) b l4 l2: lui
    Geoff> $2,%hi(l1+0x10004) add $3,$2,$3 lw $4,%lo(l1+0x10004)($3) b
    Geoff> l3 l4:

    Geoff> produces the following relocs from gas:

    Geoff> RELOCATION RECORDS FOR [.text]: OFFSET TYPE VALUE
    Geoff> 000000000000000c R_MIPS_LO16 .text 0000000000000010
    Geoff> R_MIPS_HI16 .text 000000000000001c R_MIPS_LO16 .text

    Geoff> Contents of section .text: 0000 10000003 00000000 10000005
    Geoff> 8c450004 .............E..  0010 3c020001 00431820 1000fffb
    Geoff> 8c640004 <....C. .....d..

    Geoff> which used to cause ld to fail with an error.  gcc can
    Geoff> generate control flows like this (although some of the
    Geoff> branches have to be conditional).

I think it's OK to have `ld' (as an extension) handle this case.  But,
we really should fix GAS.  It would be useful (I know this because
customers have told me so :-)) to be able to assemble things with GAS,
but link them with the MIPS linker.  The .o files that GAS creates are
not ABI-compliant, and not for any particularly good reason.  (In
other words, I don't see a real performance improvement/useful feature
that the "incorrect" ordering buys you.)

That's an irrelevant paragraph, from the point of view of the goodness
of your patch.

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

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

* Re: Reloc changes to bfd/elf32-mips.c
  1999-09-28 22:16     ` Mark Mitchell
@ 1999-09-29  8:25       ` Ian Lance Taylor
  0 siblings, 0 replies; 19+ messages in thread
From: Ian Lance Taylor @ 1999-09-29  8:25 UTC (permalink / raw)
  To: mark; +Cc: geoffk, gavin, binutils, brendan

   From: Mark Mitchell <mark@codesourcery.com>
   Date: Tue, 28 Sep 1999 22:21:37 -0700

   I think it's OK to have `ld' (as an extension) handle this case.  But,
   we really should fix GAS.  It would be useful (I know this because
   customers have told me so :-)) to be able to assemble things with GAS,
   but link them with the MIPS linker.  The .o files that GAS creates are
   not ABI-compliant, and not for any particularly good reason.  (In
   other words, I don't see a real performance improvement/useful feature
   that the "incorrect" ordering buys you.)

Yeah, I guess we fix up unmatched %hi relocs, but we don't bother to
ensure that we don't have a leading %lo reloc.  If anybody wants to
fix this, the code is in mips_frob_file in gas/config/tc-mips.c.

Ian

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

* Re: Reloc changes to bfd/elf32-mips.c
  1999-09-28 22:03         ` Ian Lance Taylor
@ 1999-10-06 19:21           ` Geoff Keating
  1999-10-06 19:35             ` Ian Lance Taylor
  0 siblings, 1 reply; 19+ messages in thread
From: Geoff Keating @ 1999-10-06 19:21 UTC (permalink / raw)
  To: ian; +Cc: mark, gavin, binutils, brendan

> Date: 29 Sep 1999 01:02:38 -0400
> From: Ian Lance Taylor <ian@zembu.com>

> We are somehow talking at cross purposes.  I don't see why what you
> are saying has much to do with what I am saying.

That happens a lot.

Perhaps you could explain what you think the code should be doing?
This is often much more helpful than simply saying `I think this is
wrong', since the usual response is `well, I think it is right'.

My test case for this is the following:

configure --target=mipstx39-unknown-elf
make
cat > test.s <<EOF
   .text
l1:
   .dword l1+16
EOF
gas/as-new test.s -o test.o
ld/ld-new -Ttext 0x12345678 test.o -o test.out
binutils/objdump -j .text -s test.out

and without my patch I see

Contents of section .text:
 12345678 00000000 12345678                    .....4Vx        

when I expect to see

Contents of section .text:
 12345678 00000000 12345688                    .....4V.        

-- 
Geoffrey Keating <geoffk@cygnus.com>

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

* Re: Reloc changes to bfd/elf32-mips.c
  1999-10-06 19:21           ` Geoff Keating
@ 1999-10-06 19:35             ` Ian Lance Taylor
  1999-10-06 20:39               ` Geoff Keating
  0 siblings, 1 reply; 19+ messages in thread
From: Ian Lance Taylor @ 1999-10-06 19:35 UTC (permalink / raw)
  To: geoffk; +Cc: mark, gavin, binutils, brendan

   Date: Thu, 7 Oct 1999 11:51:21 +1000
   From: Geoff Keating <geoffk@ozemail.com.au>

   Perhaps you could explain what you think the code should be doing?
   This is often much more helpful than simply saying `I think this is
   wrong', since the usual response is `well, I think it is right'.

Sorry, I thought I had explained it.

I think that the 32 bit MIPS ELF code should compute a 64 bit reloc by
computing a 32 bit reloc in the least significant 32 bits.  The most
significant bit of the result of that computation should then be sign
extended into the most significant 32 bits.

This behaviour is independent of whether BFD64 is defined or not.
BFD64 is a host macro, and I am concerned with target behaviour.

   My test case for this is the following:

   configure --target=mipstx39-unknown-elf
   make
   cat > test.s <<EOF
      .text
   l1:
      .dword l1+16
   EOF
   gas/as-new test.s -o test.o
   ld/ld-new -Ttext 0x12345678 test.o -o test.out
   binutils/objdump -j .text -s test.out

   and without my patch I see

   Contents of section .text:
    12345678 00000000 12345678                    .....4Vx        

   when I expect to see

   Contents of section .text:
    12345678 00000000 12345688                    .....4V.        

I agree that the code should produce what you expect to see.  I don't
know whether this build used BFD64 or not.

Your patch changed several different things at once.  I don't know
which are necessary to make this result.  Most of your patch looked
fine.  The odd parts were the two hunks which added #ifndef BFD64 and
#endif, and the last hunk which didn't set howto.  I don't understand
all of this code.  But especially adding the #ifndef/#endif seems
strange.

As I've tried to say before, if the three hunks I mentioned don't
change what happens, then they are fine.  If they do change what
happens, then I would like to understand how they change it.  Can you
explain that?

Ian

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

* Re: Reloc changes to bfd/elf32-mips.c
  1999-10-06 19:35             ` Ian Lance Taylor
@ 1999-10-06 20:39               ` Geoff Keating
  1999-10-06 20:53                 ` Ian Lance Taylor
  0 siblings, 1 reply; 19+ messages in thread
From: Geoff Keating @ 1999-10-06 20:39 UTC (permalink / raw)
  To: ian; +Cc: mark, gavin, binutils, brendan

> Mailing-List: contact binutils-help@sourceware.cygnus.com; run by ezmlm
> List-Unsubscribe: < mailto:binutils-unsubscribe-geoffk=cygnus.com@sourceware.cygnus.com >
> List-Subscribe: < mailto:binutils-subscribe@sourceware.cygnus.com >
> List-Archive: < http://sourceware.cygnus.com/ml/binutils/ >
> List-Post: < mailto:binutils@sourceware.cygnus.com >
> List-Help: < mailto:binutils-help@sourceware.cygnus.com >, < http://sourceware.cygnus.com/ml/#faqs >
> Date: 6 Oct 1999 22:34:47 -0400
> From: Ian Lance Taylor <ian@zembu.com>
> CC: mark@codesourcery.com, gavin@cygnus.com, binutils@sourceware.cygnus.com,
>         brendan@cygnus.com
> 
>    Date: Thu, 7 Oct 1999 11:51:21 +1000
>    From: Geoff Keating <geoffk@ozemail.com.au>
> 
>    Perhaps you could explain what you think the code should be doing?
>    This is often much more helpful than simply saying `I think this is
>    wrong', since the usual response is `well, I think it is right'.
> 
> Sorry, I thought I had explained it.
> 
> I think that the 32 bit MIPS ELF code should compute a 64 bit reloc by
> computing a 32 bit reloc in the least significant 32 bits.  The most
> significant bit of the result of that computation should then be sign
> extended into the most significant 32 bits.
> 
> This behaviour is independent of whether BFD64 is defined or not.
> BFD64 is a host macro, and I am concerned with target behaviour.
> 
>    My test case for this is the following:
> 
>    configure --target=mipstx39-unknown-elf
>    make
>    cat > test.s <<EOF
>       .text
>    l1:
>       .dword l1+16
>    EOF
>    gas/as-new test.s -o test.o
>    ld/ld-new -Ttext 0x12345678 test.o -o test.out
>    binutils/objdump -j .text -s test.out
> 
>    and without my patch I see
> 
>    Contents of section .text:
>     12345678 00000000 12345678                    .....4Vx        
> 
>    when I expect to see
> 
>    Contents of section .text:
>     12345678 00000000 12345688                    .....4V.        
> 
> I agree that the code should produce what you expect to see.  I don't
> know whether this build used BFD64 or not.

I expect it did use BFD64.  I think the only MIPS targets that don't
are the mips-*-sysv and mips-*-netbsd ones.

> Your patch changed several different things at once.  I don't know
> which are necessary to make this result.  Most of your patch looked
> fine.  The odd parts were the two hunks which added #ifndef BFD64 and
> #endif, and the last hunk which didn't set howto.  I don't understand
> all of this code.  But especially adding the #ifndef/#endif seems
> strange.
> 
> As I've tried to say before, if the three hunks I mentioned don't
> change what happens, then they are fine.  If they do change what
> happens, then I would like to understand how they change it.  Can you
> explain that?

OK.  The code affected looks like this:

      if (r_type == 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 = mips_rtype_to_howto (r_type);

then it determines the addend by reading from the input section, based
on 'howto', then:

      if (r_type == 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 */
...
#endif /* !BFD64 */
	}

Now, this has these problems:

1. It's wrong to determine the addend as if it was a R_MIPS_32 reloc.
   The addend is 8 bytes long, and if you only look at the first 4
   bytes you will naturally get the wrong result, particularly on
   a big-endian machine.
2. 'mips_elf_sign_extend (value, 64)' is a no-op and didn't work
   properly anyway.

To fix (2), I changed the '64' to '32'.

To fix (1), I #ifed out the special-case handling code for R_MIPS_64.
It's not necessary on BFD64.

I didn't try to look at the !BFD64 code at all.  I assume that the
person who wrote it has tested it and found it to work, so I didn't
want to touch it in case I broke it.  I have no configurations in
which it's used that I can test.

I'm not enthusiastic about this 'let's try to work around not having a
64-bit BFD' approach at all.  It breaks down completely if you add a
pc-relative 64bit relocation:

Name			Number	Size	Calculation
R_MIPS_PC64		249	T-64	S + A - P

because you now can't change the starting address of the relocation
without changing the addend, and the addend is stored in the input
section which you shouldn't be modifying.

-- 
Geoffrey Keating <geoffk@cygnus.com>

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

* Re: Reloc changes to bfd/elf32-mips.c
  1999-10-06 20:39               ` Geoff Keating
@ 1999-10-06 20:53                 ` Ian Lance Taylor
  1999-10-07  4:47                   ` Geoff Keating
  0 siblings, 1 reply; 19+ messages in thread
From: Ian Lance Taylor @ 1999-10-06 20:53 UTC (permalink / raw)
  To: geoffk; +Cc: mark, gavin, binutils, brendan

   Date: Thu, 7 Oct 1999 13:33:38 +1000
   From: Geoff Keating <geoffk@ozemail.com.au>

   > I agree that the code should produce what you expect to see.  I don't
   > know whether this build used BFD64 or not.

   I expect it did use BFD64.  I think the only MIPS targets that don't
   are the mips-*-sysv and mips-*-netbsd ones.

Hmmm, you're right, there do seem to be bfd_elf64 vectors for the
embedded MIPS targets.

   > Your patch changed several different things at once.  I don't know
   > which are necessary to make this result.  Most of your patch looked
   > fine.  The odd parts were the two hunks which added #ifndef BFD64 and
   > #endif, and the last hunk which didn't set howto.  I don't understand
   > all of this code.  But especially adding the #ifndef/#endif seems
   > strange.
   > 
   > As I've tried to say before, if the three hunks I mentioned don't
   > change what happens, then they are fine.  If they do change what
   > happens, then I would like to understand how they change it.  Can you
   > explain that?

   OK.  The code affected looks like this:

	 if (r_type == 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 = mips_rtype_to_howto (r_type);

   then it determines the addend by reading from the input section, based
   on 'howto', then:

	 if (r_type == 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 */
   ...
   #endif /* !BFD64 */
	   }

   Now, this has these problems:

   1. It's wrong to determine the addend as if it was a R_MIPS_32 reloc.
      The addend is 8 bytes long, and if you only look at the first 4
      bytes you will naturally get the wrong result, particularly on
      a big-endian machine.
   2. 'mips_elf_sign_extend (value, 64)' is a no-op and didn't work
      properly anyway.

   To fix (2), I changed the '64' to '32'.

   To fix (1), I #ifed out the special-case handling code for R_MIPS_64.
   It's not necessary on BFD64.

   I didn't try to look at the !BFD64 code at all.  I assume that the
   person who wrote it has tested it and found it to work, so I didn't
   want to touch it in case I broke it.  I have no configurations in
   which it's used that I can test.

From your description, the code doesn't work for the non BFD64 case.

As it happens, there is no particular reason to assume that it does
work.  Mark Mitchell completely rewrote all of this code recently, and
I doubt he tested every case.

Your change (2) sounds correct.

I think we need a different version of your change (1) which also
fixes the non-BFD64 case.  Clearly on a big-endian machine the addend
needs to be extracted from the least significant word.

However, I think the addend really is only 4 bytes long.  Remember
that this is not actually a 64 bit target.  The upper 32 bits are set
purely from the sign extension.  There is no addend.

At least, that's how the code worked before Mark's rewrite, and I
think it was arguably correct.

   I'm not enthusiastic about this 'let's try to work around not having a
   64-bit BFD' approach at all.  It breaks down completely if you add a
   pc-relative 64bit relocation:

   Name			Number	Size	Calculation
   R_MIPS_PC64		249	T-64	S + A - P

   because you now can't change the starting address of the relocation
   without changing the addend, and the addend is stored in the input
   section which you shouldn't be modifying.

The weird R_MIPS_64 support was added before there was any 64 bit MIPS
ELF support.  It was added to solve the particular problem of
compiling 64 bit code for a 32 bit MIPS ELF target.  64 bit code
wanted to load 64 bit addresses, so it wanted to generate 64 bit
values.  Now, since we were using a 32 bit target, those values could
only be filled by 32 bit values.  However, we had to set the right end
of the 64 bit value, and we had to sign extend the 64 bit value.

We now have 64 bit MIPS ELF support.  We will never have to worry
about this problem again.  I just want to make sure that the existing
support doesn't break.

In particular, we will never have a R_MIPS_PC64 relocation for the 32
bit target.  Why would we want one?  Even the 64 bit target doesn't
have such a relocation (although it can construct it using
R_MIPS_SUB).

Ian

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

* Re: Reloc changes to bfd/elf32-mips.c
  1999-10-06 20:53                 ` Ian Lance Taylor
@ 1999-10-07  4:47                   ` Geoff Keating
  1999-10-07  7:28                     ` Ian Lance Taylor
  1999-10-07  9:17                     ` Mark Mitchell
  0 siblings, 2 replies; 19+ messages in thread
From: Geoff Keating @ 1999-10-07  4:47 UTC (permalink / raw)
  To: ian; +Cc: mark, gavin, binutils, brendan

OK, then, how about this?

Note also the mips_elf_highest correction.

-- 
Geoffrey Keating <geoffk@cygnus.com>

===File ~/patches/cygnus/tx49-bin-literal-2.patch===========
md5sum: a6297c4d85f373cc de4c3a99fcc4f1be 197060
Index: binutils/bfd/ChangeLog
0a
Thu Oct  7 21:31:03 1999  Geoffrey Keating  <geoffk@cygnus.com>

	* elf32-mips.c (mips_elf_calculate_relocation): R_MIPS_LITERAL
	relocs also need the GP value.
	(_bfd_mips_elf_relocate_section): Handle unpaired LO16 relocs
 	properly.  Handle sign-extension for R_MIPS_64 correctly.  Correct
 	the GP value for R_MIPS_LITERAL relocs too.  Handle
	R_MIPS_64 relocs properly on big-endian MIPS.
	(mips_elf_sign_extend): Behave properly with 'long long'.
	(mips_elf_highest): Correct typo.

.
Changed files:
binutils/bfd/ChangeLog
binutils/bfd/elf32-mips.c
md5sum: 965e5323396f23af 0cc7822e898c842b 265823
--- /sloth/disk0/co/binutils-mainline/binutils/bfd/elf32-mips.c	Tue Sep 28 14:05:32 1999
+++ binutils/bfd/elf32-mips.c	Thu Oct  7 21:37:28 1999
@@ -5069,7 +5069,7 @@ mips_elf_sign_extend (value, bits)
      bfd_vma value;
      int bits;
 {
-  if (value & (1 << (bits - 1)))
+  if (value & ((bfd_vma)1 << (bits - 1)))
     /* VALUE is negative.  */
     value |= ((bfd_vma) - 1) << bits;      
   
@@ -5128,7 +5128,7 @@ mips_elf_highest (value)
      bfd_vma value ATTRIBUTE_UNUSED;
 {
 #ifdef BFD64
-  return ((value + (bfd_vma) 0x800080008000) > 48) & 0xffff;
+  return ((value + (bfd_vma) 0x800080008000) >> 48) & 0xffff;
 #else
   abort ();
   return (bfd_vma) -1;
@@ -5946,6 +5946,7 @@ mips_elf_calculate_relocation (abfd, 
     case R_MIPS_LO16:
     case R_MIPS_GPREL16:
     case R_MIPS_GPREL32:
+    case R_MIPS_LITERAL:
       gp0 = _bfd_get_gp_value (input_bfd);
       gp = _bfd_get_gp_value (abfd);
       break;
@@ -6412,9 +6413,7 @@ _bfd_mips_elf_relocate_section (output_b
   Elf_Internal_Rela *rel;
   const Elf_Internal_Rela *relend;
   bfd_vma addend;
-  bfd_vma last_hi16_addend;
   boolean use_saved_addend_p = false;
-  boolean last_hi16_addend_valid_p = false;
   struct elf_backend_data *bed;
 
   bed = get_elf_backend_data (output_bfd);
@@ -6432,13 +6431,20 @@ _bfd_mips_elf_relocate_section (output_b
 
       /* Find the relocation howto for this relocation.  */
       if (r_type == 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;
+	{
+	  /* 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;
+
+	  /* On big-endian systems, we need to lie about the position
+	     of the reloc.  */
+	  if (bfd_big_endian (input_bfd))
+	      rel->r_offset += 4;
+	}
       else
 	howto = mips_rtype_to_howto (r_type);
 
@@ -6502,26 +6508,11 @@ _bfd_mips_elf_relocate_section (output_b
 		  l &= lo16_howto->src_mask;
 		  l = mips_elf_sign_extend (l, 16);
 
-		  /* Save the high-order bit for later.  When we
-		     encounter the R_MIPS_LO16 relocation we will need
-		     them again.  */
 		  addend <<= 16;
-		  last_hi16_addend = addend;
-		  last_hi16_addend_valid_p = true;
 
 		  /* Compute the combined addend.  */
 		  addend += l;
 		}
-	      else if (r_type == R_MIPS_LO16) 
-		{
-		  /* Used the saved HI16 addend.  */
-		  if (!last_hi16_addend_valid_p)
-		    {
-		      bfd_set_error (bfd_error_bad_value);
-		      return false;
-		    }
-		  addend |= last_hi16_addend;
-		}
 	      else if (r_type == R_MIPS16_GPREL)
 		{
 		  /* The addend is scrambled in the object file.  See
@@ -6554,7 +6545,8 @@ _bfd_mips_elf_relocate_section (output_b
 
 	  if (r_type == R_MIPS16_GPREL 
 	      || r_type == R_MIPS_GPREL16
-	      || r_type == R_MIPS_GPREL32)
+	      || r_type == R_MIPS_GPREL32
+	      || r_type == R_MIPS_LITERAL)
 	    addend -= (_bfd_get_gp_value (output_bfd)
 		       - _bfd_get_gp_value (input_bfd));
 	  else if (r_type == R_MIPS_26 || r_type == R_MIPS16_26)
@@ -6683,15 +6675,6 @@ _bfd_mips_elf_relocate_section (output_b
 	   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;
@@ -6705,6 +6688,8 @@ _bfd_mips_elf_relocate_section (output_b
 	     stores.  */
 	  if (bfd_big_endian (input_bfd))
 	    {
+	      /* Undo what we did above.  */
+	      rel->r_offset -= 4;
 	      /* Store the sign-bits (which are most significant)
 		 first.  */
 	      low_bits = sign_bits;
@@ -6720,7 +6705,6 @@ _bfd_mips_elf_relocate_section (output_b
 	  bfd_put_32 (input_bfd, high_bits, 
 		      contents + rel->r_offset + 4);
 	  continue;
-#endif /* !BFD64 */
 	}
 
       /* Actually perform the relocation.  */
============================================================

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

* Re: Reloc changes to bfd/elf32-mips.c
  1999-10-07  4:47                   ` Geoff Keating
@ 1999-10-07  7:28                     ` Ian Lance Taylor
  1999-10-07  9:17                     ` Mark Mitchell
  1 sibling, 0 replies; 19+ messages in thread
From: Ian Lance Taylor @ 1999-10-07  7:28 UTC (permalink / raw)
  To: geoffk; +Cc: mark, gavin, binutils, brendan

   Date: Thu, 7 Oct 1999 21:47:20 +1000
   From: Geoff Keating <geoffk@ozemail.com.au>

   OK, then, how about this?

   Note also the mips_elf_highest correction.

This looks fine to me.

Thanks.

Ian

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

* Re: Reloc changes to bfd/elf32-mips.c
  1999-10-07  4:47                   ` Geoff Keating
  1999-10-07  7:28                     ` Ian Lance Taylor
@ 1999-10-07  9:17                     ` Mark Mitchell
  1 sibling, 0 replies; 19+ messages in thread
From: Mark Mitchell @ 1999-10-07  9:17 UTC (permalink / raw)
  To: geoffk; +Cc: ian, gavin, binutils, brendan

>>>>> "Geoff" == Geoff Keating <geoffk@ozemail.com.au> writes:

    Geoff> OK, then, how about this?

FWIW, that looks right to me.  The mips_elf_highest fix is certainly
100% correct; that's just a typo.  And the bfd_big_endian handling for
the R_MIPS_64 looks right too.

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

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

* Re: Reloc changes to bfd/elf32-mips.c
  1999-09-27  4:24 Geoff Keating
@ 1999-09-27 18:25 ` Ian Lance Taylor
  0 siblings, 0 replies; 19+ messages in thread
From: Ian Lance Taylor @ 1999-09-27 18:25 UTC (permalink / raw)
  To: geoffk; +Cc: gavin, mark, binutils

   Date: Mon, 27 Sep 1999 21:19:26 +1000
   From: Geoff Keating <geoffk@ozemail.com.au>

   This has most impact on C++.

   I'm not sure exactly who can approve changes to the sourceware
   binutils; if Gavin or Mark could do this (at least for MIPS), it'd be
   good to know as the current sequence of patches will not be the last.
   I'm also not sure who can check stuff in.

   Anyway, is the patch OK?

This patch is OK with me.

Ian

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

* Reloc changes to bfd/elf32-mips.c
@ 1999-09-27  4:24 Geoff Keating
  1999-09-27 18:25 ` Ian Lance Taylor
  0 siblings, 1 reply; 19+ messages in thread
From: Geoff Keating @ 1999-09-27  4:24 UTC (permalink / raw)
  To: gavin, Mark Mitchell, binutils

This has most impact on C++.

I'm not sure exactly who can approve changes to the sourceware
binutils; if Gavin or Mark could do this (at least for MIPS), it'd be
good to know as the current sequence of patches will not be the last.
I'm also not sure who can check stuff in.

Anyway, is the patch OK?

-- 
Geoffrey Keating <geoffk@cygnus.com>

===File ~/patches/cygnus/tx49-bin-relax.patch===============
md5sum: 5e8722c506533dd1 fa69a2dbb712c6f2 162776
Index: binutils/gas/ChangeLog
0a
Mon Sep 20 18:17:41 1999  Geoffrey Keating  <geoffk@cygnus.com>

	* config/tc-mips.c (nopic_need_relax): Allow for the
	.sdata.foo sections generated by -fdata-sections,
	and for the .gnu.linkonce.s sections generated by C++.

.
Changed files:
binutils/gas/ChangeLog
binutils/gas/config/tc-mips.c
md5sum: 1badbcc38e94c005 36b310b7eac9fea2 333029
--- /sloth/disk0/co/binutils-mainline/binutils/gas/config/tc-mips.c	Wed Aug  4 01:22:38 1999
+++ binutils/gas/config/tc-mips.c	Mon Sep 27 20:15:59 1999
@@ -294,8 +294,9 @@ static int g_switch_seen = 0;
    better.
 
    This function can only provide a guess, but it seems to work for
-   gcc output.  If it guesses wrong, the only loss should be in
-   efficiency; it shouldn't introduce any bugs.
+   gcc output.  It needs to guess right for gcc, otherwise gcc
+   will put what it thinks is a GP-relative instruction in a branch
+   delay slot.
 
    I don't know if a fix is needed for the SVR4_PIC mode.  I've only
    fixed it for the non-PIC mode.  KR 95/04/07  */
@@ -10625,7 +10626,9 @@ nopic_need_relax (sym, before_relaxing)
 	  assert (strcmp (segname, ".lit8") != 0
 		  && strcmp (segname, ".lit4") != 0);
 	  change = (strcmp (segname, ".sdata") != 0
-		    && strcmp (segname, ".sbss") != 0);
+		    && strcmp (segname, ".sbss") != 0
+		    && strncmp (segname, ".sdata.", 7) != 0
+		    && strncmp (segname, ".gnu.linkonce.s.", 16) != 0);
 	}
       return change;
     }
============================================================

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

end of thread, other threads:[~1999-10-07  9:17 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
1999-09-27  4:30 Reloc changes to bfd/elf32-mips.c Geoff Keating
1999-09-27 12:37 ` Mark Mitchell
1999-09-28  0:54 ` Mark Mitchell
1999-09-28  6:57   ` Ian Lance Taylor
1999-09-28 20:32   ` Geoff Keating
1999-09-28 20:57     ` Ian Lance Taylor
1999-09-28 21:52       ` Geoff Keating
1999-09-28 22:03         ` Ian Lance Taylor
1999-10-06 19:21           ` Geoff Keating
1999-10-06 19:35             ` Ian Lance Taylor
1999-10-06 20:39               ` Geoff Keating
1999-10-06 20:53                 ` Ian Lance Taylor
1999-10-07  4:47                   ` Geoff Keating
1999-10-07  7:28                     ` Ian Lance Taylor
1999-10-07  9:17                     ` Mark Mitchell
1999-09-28 22:16     ` Mark Mitchell
1999-09-29  8:25       ` Ian Lance Taylor
  -- strict thread matches above, loose matches on Subject: below --
1999-09-27  4:24 Geoff Keating
1999-09-27 18:25 ` Ian Lance Taylor

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).