This is the mail archive of the
binutils@sourceware.org
mailing list for the binutils project.
Re: .reloc improvement
- From: Tristan Gingold <gingold at adacore dot com>
- To: Maciej W. Rozycki <macro at codesourcery dot com>
- Cc: Alan Modra <amodra at gmail dot com>, <binutils at sourceware dot org>, Richard Sandiford <rdsandiford at googlemail dot com>
- Date: Mon, 14 Nov 2011 15:53:31 +0100
- Subject: Re: .reloc improvement
- References: <20110818140817.GA14709@bubble.grove.modra.org> <87hb5c53pn.fsf@firetop.home> <20110821041645.GA18554@bubble.grove.modra.org> <alpine.DEB.1.10.1108210541070.5068@tp.orcam.me.uk> <20110822050332.GB18554@bubble.grove.modra.org> <alpine.DEB.1.10.1109060149280.20410@tp.orcam.me.uk> <alpine.DEB.1.10.1110271858540.28657@tp.orcam.me.uk> <alpine.DEB.1.10.1111141412370.4191@tp.orcam.me.uk>
On Nov 14, 2011, at 3:22 PM, Maciej W. Rozycki wrote:
> On Thu, 27 Oct 2011, Maciej W. Rozycki wrote:
>
>> On Tue, 6 Sep 2011, Maciej W. Rozycki wrote:
>>
>>> On Mon, 22 Aug 2011, Alan Modra wrote:
>>>
>>>>> Please note that on REL MIPS targets local symbols have to be retained in
>>>>> several cases where crucial information is lost if a relocation against
>>>>> such a symbol is converted to one against the containing section's symbol.
>>>>> This applies to all the PC-relative relocations, where the relocatable
>>>>> field is too narrow to hold arbitrary addends, and also, more recently, to
>>>>> microMIPS targets, where linker relaxation has to know local symbol
>>>>> (label) addresses to perform branch displacement recalculations and to
>>>>> make the LUI/ADDIU->ADDIUPC relaxation. The latter is a linker's internal
>>>>> limitation and may possibly be lifted (possibly by using the RELA
>>>>> representation internally even on REL targets), but the former is an
>>>>> inherent problem of the object file format used.
>>>>>
>>>>> Just making sure these issues are not missed -- I can't have a look at
>>>>> your change at the moment and see what exact implications it has, sorry.
>>>>
>>>> The implication of my change is that the programmer will need to be
>>>> careful in choosing symbols used with .reloc. While that isn't ideal,
>>>> .reloc is such a low-level interface that you need to know what you're
>>>> doing if you use it.
>>>
>>> Problem is on REL targets some MIPS relocation types can never ever be
>>> safely converted to refer to a section symbol + addend instead of the
>>> intended symbol. You'd therefore make .reloc useless for these types.
>>>
>>> Yes, REL shouldn't have been chosen for the MIPS ABI in the first place,
>>> with the complication observed here being the tip of an iceberg only, but
>>> there you go. The choice was later fixed with the new ABIs made for
>>> 64-bit MIPS processors, but the old ABI still remains for 32-bit ones.
>>>
>>>> Alternatively, I'm quite willing to disable the symbol to section symbol
>>>> conversion for REL targets.
>>>
>>> Good idea, that sounds like a plan to me. That could be made platform
>>> specific if that made anybody's life easier.
>>
>> Where are we with this problem? Your change regressed this program (an
>> excerpt from a test case I prepared a while ago and was about to integrate
>> with our test suite):
>>
>> $ cat reloc.s
>> .text
>>
>> .fill 0x1000000
>>
>> .set micromips
>> foo:
>> nop32
>> .reloc ., R_MICROMIPS_PC23_S2, 0f
>> .fill 0
>> addiu $2, $pc, 8
>> nop32
>> 0:
>>
>> from this:
>>
>> $ mips-sde-elf-as -32 -o reloc.o reloc.s
>> $ mips-sde-elf-objdump -dr reloc.o
>>
>> reloc.o: file format elf32-tradbigmips
>>
>>
>> Disassembly of section .text:
>>
>> 00000000 <foo-0x1000000>:
>> ...
>>
>> 01000000 <foo>:
>> 1000000: 0000 0000 nop
>> 1000004: 7900 0002 addiu v0,$pc,8
>> 1000004: R_MICROMIPS_PC23_S2 .L1
>> 1000008: 0000 0000 nop
>>
>> to this:
>>
>> $ mips-sde-elf-as -o reloc.o reloc.s
>> reloc.s: Assembler messages:
>> reloc.s:8: Error: relocation overflow
>
> Long time, no response, so I went ahead and implemented this fix. It
> unbreaks the test case for me and causes no regressions for mips-sde-elf
> (although I have no slightest idea how much coverage in the test suite
> .reloc has).
>
> OK to apply? Since this is a regression OK for 2.22 too?
It potentially affects all targets.
Ok for the branch if you have the OK for mainline.
Tristan.
>
> 2011-11-14 Maciej W. Rozycki <macro@codesourcery.com>
>
> gas/
> write.c (dump_section_relocs): Don't convert PC-relative relocs
> that have an in-place addend narrower than the addresses used.
>
> Maciej
>
> binutils-gas-reloc-pcrel-inplace.diff
> Index: binutils-fsf-trunk-quilt/gas/write.c
> ===================================================================
> --- binutils-fsf-trunk-quilt.orig/gas/write.c 2011-11-14 13:50:51.000000000 +0000
> +++ binutils-fsf-trunk-quilt/gas/write.c 2011-11-14 14:19:07.795860362 +0000
> @@ -654,15 +654,21 @@ dump_section_relocs (bfd *abfd ATTRIBUTE
> static void
> resolve_reloc_expr_symbols (void)
> {
> + bfd_vma addr_mask = 1;
> struct reloc_list *r;
>
> + /* Avoid a shift by the width of type. */
> + addr_mask <<= bfd_arch_bits_per_address (stdoutput) - 1;
> + addr_mask <<= 1;
> + addr_mask -= 1;
> +
> for (r = reloc_list; r; r = r->next)
> {
> + reloc_howto_type *howto = r->u.a.howto;
> expressionS *symval;
> symbolS *sym;
> bfd_vma offset, addend;
> asection *sec;
> - reloc_howto_type *howto;
>
> resolve_symbol_value (r->u.a.offset_sym);
> symval = symbol_get_value_expression (r->u.a.offset_sym);
> @@ -709,7 +715,16 @@ resolve_reloc_expr_symbols (void)
> }
> else if (sym != NULL)
> {
> - if (S_IS_LOCAL (sym) && !symbol_section_p (sym))
> + /* Convert relocs against local symbols to refer to the
> + corresponding section symbol plus offset instead. Keep
> + PC-relative relocs of the REL variety intact though to
> + prevent the offset from overflowing the relocated field,
> + unless it has enough bits to cover the whole address
> + space. */
> + if (S_IS_LOCAL (sym) && !symbol_section_p (sym)
> + && !(howto->partial_inplace
> + && howto->pc_relative
> + && howto->src_mask != addr_mask))
> {
> asection *symsec = S_GET_SEGMENT (sym);
> if (!(((symsec->flags & SEC_MERGE) != 0
> @@ -730,8 +745,6 @@ resolve_reloc_expr_symbols (void)
> sym = abs_section_sym;
> }
>
> - howto = r->u.a.howto;
> -
> r->u.b.sec = sec;
> r->u.b.s = symbol_get_bfdsym (sym);
> r->u.b.r.sym_ptr_ptr = &r->u.b.s;