This is the mail archive of the
binutils@sources.redhat.com
mailing list for the binutils project.
Patch to extend explicit reloc support in MIPS gas (part 2)
- From: Richard Sandiford <rsandifo at redhat dot com>
- To: binutils at sources dot redhat dot com
- Cc: echristo at redhat dot com
- Date: 04 Feb 2003 20:27:02 +0000
- Subject: Patch to extend explicit reloc support in MIPS gas (part 2)
- References: <m3n0loay5w.fsf@localhost.localdomain>
This patch follows on from:
http://sources.redhat.com/ml/binutils/2003-01/msg00449.html
If an R_MIPS_GOT16 relocation is applied to a local symbol, it must be
immediately followed by a R_MIPS_LO16 reloc. As a GNU extension,
several R_MIPS_GOT16 relocs can be attached to the same R_MIPS_LO16.
We already match R_MIPS_HI16 relocs to R_MIPS_LO16s, so all we need
to do is extend that code to work with R_MIPS_GOT16 as well. The only
tricky part seems to be dealing with this code in append_insn():
/* We must ensure that a fixup associated with an unmatched %hi
reloc does not become a variant frag. Otherwise, the
rearrangement of %hi relocs in frob_file may confuse
tc_gen_reloc. */
if (unmatched_reloc_p)
{
frag_wane (frag_now);
frag_new (0);
}
It took me a _long_ time to figure out why this was a problem,
although I guess it's pretty obvious when you know. To quote
a comment from the patch:
/* Umatched fixups should not be put in the same frag as a relaxable
macro. For example, suppose we have:
lui $4,%hi(l1) # 1
la $5,l2 # 2
addiu $4,$4,%lo(l1) # 3
If instructions 1 and 2 were put in the same frag, md_frob_file would
move the fixup for #1 after the fixups for the "unrelaxed" version of
#2. This would confuse tc_gen_reloc, which expects the relocations
for #2 to be the last for that frag.
The existing frag_wane() can't stay where it is because BFD_RELOC_GOT16s
(unlike BFD_RELOC_HI16s) are generated in the "unrelaxed" version of
relaxable macros. In other words, the code would trigger while we're
in the middle of generating a variant frag.
I suggest moving the code to the beginning of macro() and checking
whether the last unmatched HI relocation belongs to the same frag
as the macro: if it doesn't, no action is needed. This should
use less memory than the current scheme, in which we create a new
frag after every explicit %hi().
As a further memory optimisation, if the last "high" reloc ended up
having a matching "low", we can reuse its entry in mips_hi_fixup_list.
Note that removing the frag_wane() altogether doesn't cause any
testsuite regressions. I've included the example above as a new
test case.
Also, pic_need_relax seems pretty generic. Is it worth making it
a target-independent function? If so, what should it be called?
Tested on the same targets as before, no regressions. OK to install?
Richard
gas/
* config/tc-mips.c (reloc_needs_lo_p): New function.
(fixup_has_matching_lo_p): New function.
(append_insn): Use reloc_needs_lo_p to check whether a relocation
might need a matching %lo(). Reuse the head of mips_hi_fixup_list
if that fixup already has a matching %lo(). Don't call frag_wane here.
(macro): Call frag_wane here if the last unmatched hi was in the
current frag.
(pic_need_relax): New function, split out from...
(md_estimate_size_before_relax): ...here.
(mips_frob_file): Use reloc_needs_lo_p. Use pic_need_relax to test
whether BFD_RELOC_MIPS_GOT16 fixups refer to global symbols.
gas/testsuite/
* gas/mips/rel12.[sd], gas/mips/rel13.[sd]: New tests.
* gas/mips/mips.exp: Run them.
Index: config/tc-mips.c
===================================================================
RCS file: /cvs/src/src/gas/config/tc-mips.c,v
retrieving revision 1.190
diff -c -d -p -F^\([(a-zA-Z0-9_]\|#define\) -r1.190 tc-mips.c
*** config/tc-mips.c 2 Feb 2003 19:37:20 -0000 1.190
--- config/tc-mips.c 2 Feb 2003 20:58:19 -0000
*************** #define internalError() as_fatal (_("MIP
*** 801,806 ****
--- 801,810 ----
enum mips_regclass { MIPS_GR_REG, MIPS_FP_REG, MIPS16_REG };
+ static inline bfd_boolean reloc_needs_lo_p
+ PARAMS ((bfd_reloc_code_real_type));
+ static inline bfd_boolean fixup_has_matching_lo_p
+ PARAMS ((fixS *));
static int insn_uses_reg
PARAMS ((struct mips_cl_insn *ip, unsigned int reg,
enum mips_regclass class));
*************** static void s_mips_file
*** 932,937 ****
--- 936,943 ----
PARAMS ((int));
static void s_mips_loc
PARAMS ((int));
+ static bfd_boolean pic_need_relax
+ PARAMS ((symbolS *, asection *));
static int mips16_extended_frag
PARAMS ((fragS *, asection *, long));
static int relaxed_branch_length (fragS *, asection *, int);
*************** md_assemble (str)
*** 1439,1444 ****
--- 1445,1475 ----
}
}
+ /* Return true if the given relocation might need a matching %lo().
+ Note that R_MIPS_GOT16 relocations only need a matching %lo() when
+ applied to local symbols. */
+
+ static inline bfd_boolean
+ reloc_needs_lo_p (reloc)
+ bfd_reloc_code_real_type reloc;
+ {
+ return (reloc == BFD_RELOC_HI16_S
+ || reloc == BFD_RELOC_MIPS_GOT16);
+ }
+
+ /* Return true if the given fixup is followed by a matching R_MIPS_LO16
+ relocation. */
+
+ static inline bfd_boolean
+ fixup_has_matching_lo_p (fixp)
+ fixS *fixp;
+ {
+ return (fixp->fx_next != NULL
+ && fixp->fx_next->fx_r_type == BFD_RELOC_LO16
+ && fixp->fx_addsy == fixp->fx_next->fx_addsy
+ && fixp->fx_offset == fixp->fx_next->fx_offset);
+ }
+
/* See whether instruction IP reads register REG. CLASS is the type
of register. */
*************** append_insn (place, ip, address_expr, re
*** 1593,1606 ****
char *f;
fixS *fixp[3];
int nops = 0;
- bfd_boolean unmatched_reloc_p;
/* Mark instruction labels in mips16 mode. */
mips16_mark_labels ();
prev_pinfo = prev_insn.insn_mo->pinfo;
pinfo = ip->insn_mo->pinfo;
- unmatched_reloc_p = FALSE;
if (place == NULL && (! mips_opts.noreorder || prev_nop_frag != NULL))
{
--- 1624,1635 ----
*************** #define emit_nop() \
*** 2144,2160 ****
|| *reloc_type == BFD_RELOC_MIPS_RELGOT))
fixp[0]->fx_no_overflow = 1;
! if (reloc_type[0] == BFD_RELOC_HI16_S)
{
struct mips_hi_fixup *hi_fixup;
! hi_fixup = ((struct mips_hi_fixup *)
! xmalloc (sizeof (struct mips_hi_fixup)));
hi_fixup->fixp = fixp[0];
hi_fixup->seg = now_seg;
- hi_fixup->next = mips_hi_fixup_list;
- mips_hi_fixup_list = hi_fixup;
- unmatched_reloc_p = TRUE;
}
if (reloc_type[1] != BFD_RELOC_UNUSED)
--- 2173,2194 ----
|| *reloc_type == BFD_RELOC_MIPS_RELGOT))
fixp[0]->fx_no_overflow = 1;
! if (reloc_needs_lo_p (*reloc_type))
{
struct mips_hi_fixup *hi_fixup;
! /* Reuse the last entry if it already has a matching %lo. */
! hi_fixup = mips_hi_fixup_list;
! if (hi_fixup == 0
! || !fixup_has_matching_lo_p (hi_fixup->fixp))
! {
! hi_fixup = ((struct mips_hi_fixup *)
! xmalloc (sizeof (struct mips_hi_fixup)));
! hi_fixup->next = mips_hi_fixup_list;
! mips_hi_fixup_list = hi_fixup;
! }
hi_fixup->fixp = fixp[0];
hi_fixup->seg = now_seg;
}
if (reloc_type[1] != BFD_RELOC_UNUSED)
*************** #define emit_nop() \
*** 2706,2721 ****
/* We just output an insn, so the next one doesn't have a label. */
mips_clear_insn_labels ();
-
- /* We must ensure that a fixup associated with an unmatched %hi
- reloc does not become a variant frag. Otherwise, the
- rearrangement of %hi relocs in frob_file may confuse
- tc_gen_reloc. */
- if (unmatched_reloc_p)
- {
- frag_wane (frag_now);
- frag_new (0);
- }
}
/* This function forgets that there was any previous instruction or
--- 2740,2745 ----
*************** macro (ip)
*** 4070,4075 ****
--- 4094,4120 ----
expr1.X_add_symbol = NULL;
expr1.X_add_number = 1;
+ /* Umatched fixups should not be put in the same frag as a relaxable
+ macro. For example, suppose we have:
+
+ lui $4,%hi(l1) # 1
+ la $5,l2 # 2
+ addiu $4,$4,%lo(l1) # 3
+
+ If instructions 1 and 2 were put in the same frag, md_frob_file would
+ move the fixup for #1 after the fixups for the "unrelaxed" version of
+ #2. This would confuse tc_gen_reloc, which expects the relocations
+ for #2 to be the last for that frag.
+
+ If it looks like this situation could happen, put the macro
+ in a new frag. */
+ if (mips_hi_fixup_list != 0
+ && mips_hi_fixup_list->fixp->fx_frag == frag_now)
+ {
+ frag_wane (frag_now);
+ frag_new (0);
+ }
+
switch (mask)
{
case M_DABS:
*************** mips_frob_file ()
*** 10854,10867 ****
segment_info_type *seginfo;
int pass;
! assert (l->fixp->fx_r_type == BFD_RELOC_HI16_S);
! /* Check quickly whether the next fixup happens to be a matching
! %lo. */
! if (l->fixp->fx_next != NULL
! && l->fixp->fx_next->fx_r_type == BFD_RELOC_LO16
! && l->fixp->fx_addsy == l->fixp->fx_next->fx_addsy
! && l->fixp->fx_offset == l->fixp->fx_next->fx_offset)
continue;
/* Look through the fixups for this segment for a matching %lo.
--- 10899,10914 ----
segment_info_type *seginfo;
int pass;
! assert (reloc_needs_lo_p (l->fixp->fx_r_type));
! /* If a GOT16 relocation turns out to be against a global symbol,
! there isn't supposed to be a matching LO. */
! if (l->fixp->fx_r_type == BFD_RELOC_MIPS_GOT16
! && !pic_need_relax (l->fixp->fx_addsy, l->seg))
! continue;
!
! /* Check quickly whether the next fixup happens to be a matching %lo. */
! if (fixup_has_matching_lo_p (l->fixp))
continue;
/* Look through the fixups for this segment for a matching %lo.
*************** mips_frob_file ()
*** 10883,10891 ****
&& f->fx_offset == l->fixp->fx_offset
&& (pass == 1
|| prev == NULL
! || prev->fx_r_type != BFD_RELOC_HI16_S
! || prev->fx_addsy != f->fx_addsy
! || prev->fx_offset != f->fx_offset))
{
fixS **pf;
--- 10930,10937 ----
&& f->fx_offset == l->fixp->fx_offset
&& (pass == 1
|| prev == NULL
! || !reloc_needs_lo_p (prev->fx_r_type)
! || !fixup_has_matching_lo_p (prev)))
{
fixS **pf;
*************** nopic_need_relax (sym, before_relaxing)
*** 12668,12673 ****
--- 12714,12777 ----
return 1;
}
+
+ /* Return true if the given symbol should be considered local for SVR4 PIC. */
+
+ static bfd_boolean
+ pic_need_relax (sym, segtype)
+ symbolS *sym;
+ asection *segtype;
+ {
+ asection *symsec;
+ bfd_boolean linkonce;
+
+ /* Handle the case of a symbol equated to another symbol. */
+ while (symbol_equated_reloc_p (sym))
+ {
+ symbolS *n;
+
+ /* It's possible to get a loop here in a badly written
+ program. */
+ n = symbol_get_value_expression (sym)->X_add_symbol;
+ if (n == sym)
+ break;
+ sym = n;
+ }
+
+ symsec = S_GET_SEGMENT (sym);
+
+ /* duplicate the test for LINK_ONCE sections as in adjust_reloc_syms */
+ linkonce = FALSE;
+ if (symsec != segtype && ! S_IS_LOCAL (sym))
+ {
+ if ((bfd_get_section_flags (stdoutput, symsec) & SEC_LINK_ONCE)
+ != 0)
+ linkonce = TRUE;
+
+ /* The GNU toolchain uses an extension for ELF: a section
+ beginning with the magic string .gnu.linkonce is a linkonce
+ section. */
+ if (strncmp (segment_name (symsec), ".gnu.linkonce",
+ sizeof ".gnu.linkonce" - 1) == 0)
+ linkonce = TRUE;
+ }
+
+ /* This must duplicate the test in adjust_reloc_syms. */
+ return (symsec != &bfd_und_section
+ && symsec != &bfd_abs_section
+ && ! bfd_is_com_section (symsec)
+ && !linkonce
+ #ifdef OBJ_ELF
+ /* A global or weak symbol is treated as external. */
+ && (OUTPUT_FLAVOR != bfd_target_elf_flavour
+ || (! S_IS_WEAK (sym)
+ && (! S_IS_EXTERNAL (sym)
+ || mips_pic == EMBEDDED_PIC)))
+ #endif
+ );
+ }
+
+
/* Given a mips16 variant frag FRAGP, return non-zero if it needs an
extended opcode. SEC is the section the frag is in. */
*************** md_estimate_size_before_relax (fragp, se
*** 12946,12953 ****
fragS *fragp;
asection *segtype;
{
! int change = 0;
! bfd_boolean linkonce = FALSE;
if (RELAX_BRANCH_P (fragp->fr_subtype))
{
--- 13050,13056 ----
fragS *fragp;
asection *segtype;
{
! int change;
if (RELAX_BRANCH_P (fragp->fr_subtype))
{
*************** md_estimate_size_before_relax (fragp, se
*** 12963,13022 ****
return (RELAX_MIPS16_EXTENDED (fragp->fr_subtype) ? 4 : 2);
if (mips_pic == NO_PIC)
! {
! change = nopic_need_relax (fragp->fr_symbol, 0);
! }
else if (mips_pic == SVR4_PIC)
! {
! symbolS *sym;
! asection *symsec;
!
! sym = fragp->fr_symbol;
!
! /* Handle the case of a symbol equated to another symbol. */
! while (symbol_equated_reloc_p (sym))
! {
! symbolS *n;
!
! /* It's possible to get a loop here in a badly written
! program. */
! n = symbol_get_value_expression (sym)->X_add_symbol;
! if (n == sym)
! break;
! sym = n;
! }
!
! symsec = S_GET_SEGMENT (sym);
!
! /* duplicate the test for LINK_ONCE sections as in adjust_reloc_syms */
! if (symsec != segtype && ! S_IS_LOCAL (sym))
! {
! if ((bfd_get_section_flags (stdoutput, symsec) & SEC_LINK_ONCE)
! != 0)
! linkonce = TRUE;
!
! /* The GNU toolchain uses an extension for ELF: a section
! beginning with the magic string .gnu.linkonce is a linkonce
! section. */
! if (strncmp (segment_name (symsec), ".gnu.linkonce",
! sizeof ".gnu.linkonce" - 1) == 0)
! linkonce = TRUE;
! }
!
! /* This must duplicate the test in adjust_reloc_syms. */
! change = (symsec != &bfd_und_section
! && symsec != &bfd_abs_section
! && ! bfd_is_com_section (symsec)
! && !linkonce
! #ifdef OBJ_ELF
! /* A global or weak symbol is treated as external. */
! && (OUTPUT_FLAVOR != bfd_target_elf_flavour
! || (! S_IS_WEAK (sym)
! && (! S_IS_EXTERNAL (sym)
! || mips_pic == EMBEDDED_PIC)))
! #endif
! );
! }
else
abort ();
--- 13066,13074 ----
return (RELAX_MIPS16_EXTENDED (fragp->fr_subtype) ? 4 : 2);
if (mips_pic == NO_PIC)
! change = nopic_need_relax (fragp->fr_symbol, 0);
else if (mips_pic == SVR4_PIC)
! change = pic_need_relax (fragp->fr_symbol, segtype);
else
abort ();
Index: testsuite/gas/mips/mips.exp
===================================================================
RCS file: /cvs/src/src/gas/testsuite/gas/mips/mips.exp,v
retrieving revision 1.59
diff -c -d -p -F^\([(a-zA-Z0-9_]\|#define\) -r1.59 mips.exp
*** testsuite/gas/mips/mips.exp 2 Feb 2003 19:37:20 -0000 1.59
--- testsuite/gas/mips/mips.exp 2 Feb 2003 20:58:19 -0000
*************** if { [istarget mips*-*-*] } then {
*** 608,613 ****
--- 608,615 ----
run_dump_test "elf-rel10"
run_dump_test "elf-rel11"
}
+ run_dump_test "elf-rel12"
+ run_dump_test "elf-rel13"
run_dump_test "${tmips}${el}empic"
run_dump_test "empic2"
run_dump_test "empic3_e"
*** /dev/null Thu Apr 11 15:25:15 2002
--- testsuite/gas/mips/elf-rel12.d Sun Feb 2 19:42:53 2003
***************
*** 0 ****
--- 1,11 ----
+ #as: -march=mips1 -mabi=32
+ #readelf: --relocs
+ #name: MIPS ELF reloc 12
+
+ Relocation section '\.rel\.text' at offset .* contains 4 entries:
+ *Offset * Info * Type * Sym\.Value * Sym\. Name
+ 0+0004 * 0+..05 * R_MIPS_HI16 * 0+0000 * l1
+ 0+0008 * 0+..06 * R_MIPS_LO16 * 0+0000 * l1
+ 0+0000 * 0+..05 * R_MIPS_HI16 * 0+0004 * l2
+ 0+000c * 0+..06 * R_MIPS_LO16 * 0+0004 * l2
+ #pass
*** /dev/null Thu Apr 11 15:25:15 2002
--- testsuite/gas/mips/elf-rel12.s Sun Feb 2 19:42:53 2003
***************
*** 0 ****
--- 1,14 ----
+ .ent foo
+ foo:
+ lui $4,%hi(l2)
+ la $3,l1
+ addiu $4,$4,%lo(l2)
+
+ .space 64
+ .end foo
+
+ .globl l1
+ .globl l2
+ .data
+ l1: .word 1
+ l2: .word 2
*** /dev/null Thu Apr 11 15:25:15 2002
--- testsuite/gas/mips/elf-rel13.d Sun Feb 2 19:46:19 2003
***************
*** 0 ****
--- 1,18 ----
+ #as: -march=mips2 -mabi=32 -KPIC
+ #readelf: --relocs
+ #name: MIPS ELF reloc 13
+
+ Relocation section '\.rel\.text' at offset .* contains 10 entries:
+ *Offset * Info * Type * Sym\.Value * Sym\. Name
+ 0+0000 * 0+..09 * R_MIPS_GOT16 * 0+0000 * \.data
+ 0+0014 * 0+..06 * R_MIPS_LO16 * 0+0000 * \.data
+ 0+0010 * 0+..09 * R_MIPS_GOT16 * 0+0000 * \.data
+ 0+0018 * 0+..06 * R_MIPS_LO16 * 0+0000 * \.data
+ # The next two lines could be in either order.
+ 0+000c * 0+..09 * R_MIPS_GOT16 * 0+0000 * \.rodata
+ 0+0008 * 0+..09 * R_MIPS_GOT16 * 0+0000 * \.rodata
+ 0+001c * 0+..06 * R_MIPS_LO16 * 0+0000 * \.rodata
+ 0+0004 * 0+..09 * R_MIPS_GOT16 * 0+0000 * \.bss
+ 0+0020 * 0+..06 * R_MIPS_LO16 * 0+0000 * \.bss
+ 0+0024 * 0+..06 * R_MIPS_LO16 * 0+0000 * \.data
+ #pass
*** /dev/null Thu Apr 11 15:25:15 2002
--- testsuite/gas/mips/elf-rel13.s Sun Feb 2 19:42:53 2003
***************
*** 0 ****
--- 1,23 ----
+ .ent foo
+ foo:
+ lw $4,%got(l1)($gp)
+ lw $4,%got(l2)($gp)
+ lw $4,%got(l3)($gp)
+ lw $4,%got(l3)($gp)
+ lw $4,%got(l1+0x400)($gp)
+ addiu $4,$4,%lo(l1)
+ addiu $4,$4,%lo(l1+0x400)
+ addiu $4,$4,%lo(l3)
+ addiu $4,$4,%lo(l2)
+ addiu $4,$4,%lo(l1+0x400) # Orphaned
+ .space 64
+ .end foo
+
+ .data
+ l1: .word 1
+
+ .lcomm l2, 4
+
+ .rdata
+ .word 1
+ l3: .word 2