Re: Reloc changes to bfd/elf32-mips.c

[Ian Lance Taylor <ian at zembu dot com>]

   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