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

[Geoff Keating <geoffk@ozemail.com.au>]

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