This is the mail archive of the
binutils@sources.redhat.com
mailing list for the binutils project.
Re: MIPS embedded PIC bug.
A couple of days ago I wrote:
> > I think there are only two cases:
> >
> > - The address is a constant and less than +/- 2^15, in which case you
> > can generate one instruction; or
> > - you don't know the address, or it's too big, in which case you must
> > generate two.
>
> Right.
>
> OK, well, now at least I think I understand how the bug needs to be
> fixed.
Since that message, I implemented code to do this properly. For
instance, for the case of:
<op> $treg, l1-l2($breg)
where <op> is e.g. lw, sw, etc., and where l2 is defined and in the
current segment, you can do one of two things:
lui $tempreg, l1-C-. (BFD_RELOC_PCREL_HI16_S)
addiu $tempreg,$tempreg,$breg
<op> $treg,l1-C-.($tempreg) (BFD_RELOC_PCREL_LO16)
(where C = l2 - .), or, if l1-C-. fits into 16 bits:
<op> $treg,l1-C-.($breg) (BFD_RELOC_PCREL_LO16)
Flushed with my success, I also:
(1) converted 'la $treg, l1-l2' (where l2 is in local segment) to emit
only one instruction if it'll fit, and
(2) added support for 'la $treg, l1-l2($breg)'. This is kinda useful
for getting the address of of a table of functions or variables, and
it's better than open-coding:
la $treg, l1-l2
addu $treg, $treg, $breg
because in the case where the difference fits in 16 bits, it can be
done in one instruction. (If you're just going to 'lw' from it
immediately, you're best doing the lw directly. but you might lw
multiple things from different offsets from the calculated table, in
which case it wins.)
I've run into one significant snag: ECOFF. My changes don't work
properly with ECOFF on MIPS. In particular, it seg-faults when
handling the:
l1:
# ...
la $3, d1-l1
# ...
.data
d1:
case. tc_gen_reloc() seems to expect that the fixS being worked on
will have an fx_subsy... and fixup_segment() in write.c seems to go
out of its way to keep that from being true. 8-)
However, as far as I can tell, MIPS ECOFF support is Just Broken. In
particular:
* MIPS ECOFF wouldn't compile before I got to it this morning.
* In a clean source tree (with my mods to make --target=mips-ecoff
compile), I tweaked the 'o'-argument checking so that it properly
rejects the case described above, causing the M_LA_AB macro code to be
used. The embedded-PIC code in in the M_LA_AB macro code has been
there for a long time, and which should work properly. It generates
the same kinds of relocations that I now generate more often. I then
tried to assemble the test case above with -membedded-pic. It
crashed.
I.e., it seems that not only was MIPS ECOFF generally in disrepair,
but any attempt to generate proper code for the sequence above with
the code that's been in the tree for a long time, would also cause the
assembler to crash.
I suspect that any fix to that crash will also fix the problem that my
new code has, but I don't have the inclination to hunt ECOFF bugs for
sport, and it really looks like the code was already broken.
Anyway, I can't actually submit this patch right now for inclusion
because of assignment issues. (I'm working on them, really. 8-)
However, I'd like comments on it, etc., if possible.
I've verified that it doesn't change the testsuite results for
mips-elf or mips-ecoff, with the exception of the MIPS ELF empic test.
This changes, because of the optimization of 'la' that i now permit
(see '(1)' above).
Once I do get around to submitting it, I'll include a testcase, of
course. (I'll also nuke the XXXCGD... #ifs -- those are for my
testing. 8-)
Thoughts? Guidance about the chunk containing 'XXX how' would be
appreciated.
chris
===================================================================
Index: gas/config/tc-mips.c
===================================================================
RCS file: /cvs/src/src/gas/config/tc-mips.c,v
retrieving revision 1.35
diff -c -r1.35 tc-mips.c
*** tc-mips.c 2001/02/10 21:41:12 1.35
--- tc-mips.c 2001/02/11 03:52:54
***************
*** 553,566 ****
external symbols and local symbols must be handled differently.
We handle these issues by actually generating both possible
! instruction sequences. The longer one is put in a frag_var with
! type rs_machine_dependent. We encode what to do with the frag in
the subtype field. We encode (1) the number of existing bytes to
replace, (2) the number of new bytes to use, (3) the offset from
the start of the existing bytes to the first reloc we must generate
(that is, the offset is applied from the start of the existing
bytes after they are replaced by the new bytes, if any), (4) the
! offset from the start of the existing bytes to the second reloc,
(5) whether a third reloc is needed (the third reloc is always four
bytes after the second reloc), and (6) whether to warn if this
variant is used (this is sometimes needed if .set nomacro or .set
--- 553,568 ----
external symbols and local symbols must be handled differently.
We handle these issues by actually generating both possible
! instruction sequences. In most cases, the longer one is put in
! a frag_var with type rs_machine_dependent. (The exception is for
! certain embedded PIC code.) We encode what to do with the frag in
the subtype field. We encode (1) the number of existing bytes to
replace, (2) the number of new bytes to use, (3) the offset from
the start of the existing bytes to the first reloc we must generate
(that is, the offset is applied from the start of the existing
bytes after they are replaced by the new bytes, if any), (4) the
! offset from the start of the existing bytes to the second reloc
! (if the same as the offset to the first reloc, only one is performed),
(5) whether a third reloc is needed (the third reloc is always four
bytes after the second reloc), and (6) whether to warn if this
variant is used (this is sometimes needed if .set nomacro or .set
***************
*** 4125,4133 ****
/* Load the address of a symbol into a register. If breg is not
zero, we then add a base register to it. */
/* When generating embedded PIC code, we permit expressions of
the form
! la $4,foo-bar
where bar is an address in the current section. These are used
when getting the addresses of functions. We don't permit
X_add_number to be non-zero, because if the symbol is
--- 4127,4147 ----
/* Load the address of a symbol into a register. If breg is not
zero, we then add a base register to it. */
+ if (treg == breg)
+ {
+ tempreg = AT;
+ used_at = 1;
+ }
+ else
+ {
+ tempreg = treg;
+ used_at = 0;
+ }
+
/* When generating embedded PIC code, we permit expressions of
the form
! la $treg,foo-bar
! la $treg,foo-bar($breg)
where bar is an address in the current section. These are used
when getting the addresses of functions. We don't permit
X_add_number to be non-zero, because if the symbol is
***************
*** 4142,4159 ****
(symbol_get_value_expression (offset_expr.X_op_symbol)
->X_add_symbol)
== now_seg)))
- && breg == 0
&& (offset_expr.X_add_number == 0
|| OUTPUT_FLAVOR == bfd_target_elf_flavour))
{
! macro_build ((char *) NULL, &icnt, &offset_expr, "lui", "t,u",
! treg, (int) BFD_RELOC_PCREL_HI16_S);
! macro_build ((char *) NULL, &icnt, &offset_expr,
((bfd_arch_bits_per_address (stdoutput) == 32
! || ! ISA_HAS_64BIT_REGS (mips_opts.isa))
? "addiu" : "daddiu"),
! "t,r,j", treg, treg, (int) BFD_RELOC_PCREL_LO16);
! return;
}
if (offset_expr.X_op != O_symbol
--- 4156,4206 ----
(symbol_get_value_expression (offset_expr.X_op_symbol)
->X_add_symbol)
== now_seg)))
&& (offset_expr.X_add_number == 0
|| OUTPUT_FLAVOR == bfd_target_elf_flavour))
{
! #define XXXCGD_USE_VAR_FRAG 1
! #if XXXCGD_USE_VAR_FRAG == 0
! frag_grow (12);
! #else
! frag_grow (16);
! #endif
! if (breg == 0)
! {
! tempreg = treg;
! used_at = 0;
! macro_build ((char *) NULL, &icnt, &offset_expr, "lui", "t,u",
! treg, (int) BFD_RELOC_PCREL_HI16_S);
! }
! else
! {
! macro_build ((char *) NULL, &icnt, &offset_expr, "lui", "t,u",
! tempreg, (int) BFD_RELOC_PCREL_HI16_S);
! macro_build ((char *) NULL, &icnt, (expressionS *) NULL,
! ((bfd_arch_bits_per_address (stdoutput) == 32
! || ! ISA_HAS_64BIT_REGS (mips_opts.isa))
! ? "addu" : "daddu"),
! "d,v,t", tempreg, tempreg, breg);
! }
! macro_build ((char *)NULL, &icnt, &offset_expr,
((bfd_arch_bits_per_address (stdoutput) == 32
! || ! ISA_HAS_64BIT_REGS (mips_opts.isa))
? "addiu" : "daddiu"),
! "t,r,j", treg, tempreg, (int) BFD_RELOC_PCREL_LO16);
! p = frag_var (rs_machine_dependent, 4, 0,
! RELAX_ENCODE ((breg == 0) ? 8 : 12, 4, 0, 0, 0, 0),
! offset_expr.X_add_symbol, (offsetT) 0,
! (char *) NULL);
! macro_build (p, &icnt, &offset_expr,
! ((bfd_arch_bits_per_address (stdoutput) == 32
! || ! ISA_HAS_64BIT_REGS (mips_opts.isa))
! ? "addiu" : "daddiu"),
! "t,r,j", treg, breg, (int) BFD_RELOC_PCREL_LO16);
!
! if (! used_at)
! return;
!
! break;
}
if (offset_expr.X_op != O_symbol
***************
*** 4163,4179 ****
offset_expr.X_op = O_constant;
}
- if (treg == breg)
- {
- tempreg = AT;
- used_at = 1;
- }
- else
- {
- tempreg = treg;
- used_at = 0;
- }
-
if (offset_expr.X_op == O_constant)
load_register (&icnt, tempreg, &offset_expr, dbl);
else if (mips_pic == NO_PIC)
--- 4210,4215 ----
***************
*** 4941,4946 ****
--- 4977,5036 ----
else
fmt = "t,o(b)";
+ /* For embedded PIC, we allow loads where the offset is calculated
+ by subtracting a symbol in the current segment from an unknown
+ symbol, relative to a base register. This is used by the
+ compiler for switch statements. */
+ if (mips_pic == EMBEDDED_PIC
+ && offset_expr.X_op == O_subtract
+ && (symbol_constant_p (offset_expr.X_op_symbol)
+ ? S_GET_SEGMENT (offset_expr.X_op_symbol) == now_seg
+ : (symbol_equated_p (offset_expr.X_op_symbol)
+ && (S_GET_SEGMENT
+ (symbol_get_value_expression (offset_expr.X_op_symbol)
+ ->X_add_symbol)
+ == now_seg)))
+ && breg != 0
+ && (offset_expr.X_add_number == 0
+ || OUTPUT_FLAVOR == bfd_target_elf_flavour))
+ {
+ /* For this case, we output the instructions:
+ lui $tempreg,<sym> (BFD_RELOC_PCREL_HI16_S)
+ addiu $tempreg,$tempreg,$breg
+ <op> $treg,<sym>($tempreg) (BFD_RELOC_PCREL_LO16)
+ If the relocation would fit entirely in 16 bits, it would be
+ nice to emit:
+ <op> $treg,<sym>($breg) (BFD_RELOC_PCREL_LO16)
+ instead, but that seems quite difficult. */
+ #if XXXCGD_USE_VAR_FRAG == 0
+ frag_grow (12);
+ #else
+ frag_grow (16);
+ #endif
+ macro_build ((char *) NULL, &icnt, &offset_expr, "lui", "t,u",
+ tempreg, (int) BFD_RELOC_PCREL_HI16_S);
+ macro_build ((char *) NULL, &icnt, (expressionS *) NULL,
+ ((bfd_arch_bits_per_address (stdoutput) == 32
+ || ! ISA_HAS_64BIT_REGS (mips_opts.isa))
+ ? "addu" : "daddu"),
+ "d,v,t", tempreg, tempreg, breg);
+ macro_build ((char *) NULL, &icnt, &offset_expr, s, fmt, treg,
+ (int) BFD_RELOC_PCREL_LO16, tempreg);
+ #if XXXCGD_USE_VAR_FRAG
+ p = frag_var (rs_machine_dependent, 4, 0,
+ RELAX_ENCODE (12, 4, 0, 0, 0,
+ (mips_opts.warn_about_macros
+ || (used_at && mips_opts.noat))),
+ offset_expr.X_add_symbol, (offsetT) 0,
+ (char *) NULL);
+ macro_build (p, &icnt, &offset_expr, s, fmt, treg,
+ (int) BFD_RELOC_PCREL_LO16, breg);
+ #endif
+ if (! used_at)
+ return;
+ break;
+ }
+
if (offset_expr.X_op != O_constant
&& offset_expr.X_op != O_symbol)
{
***************
*** 5487,5492 ****
--- 5577,5585 ----
fmt = "t,o(b)";
ldd_std:
+ /* We do _not_ allow embedded PIC (symbol-local_symbol) loads
+ for the case of doing a pair of loads to simulate an 'ld'. */
+
if (offset_expr.X_op != O_symbol
&& offset_expr.X_op != O_constant)
{
***************
*** 7796,7818 ****
/* If this value won't fit into a 16 bit offset, then go
find a macro that will generate the 32 bit offset
! code pattern. As a special hack, we accept the
! difference of two local symbols as a constant. This
! is required to suppose embedded PIC switches, which
! use an instruction which looks like
! lw $4,$L12-$LS12($4)
! The problem with handling this in a more general
! fashion is that the macro function doesn't expect to
! see anything which can be handled in a single
! constant instruction. */
if (c == 0
&& (offset_expr.X_op != O_constant
|| offset_expr.X_add_number >= 0x8000
! || offset_expr.X_add_number < -0x8000)
! && (mips_pic != EMBEDDED_PIC
! || offset_expr.X_op != O_subtract
! || (S_GET_SEGMENT (offset_expr.X_add_symbol)
! != S_GET_SEGMENT (offset_expr.X_op_symbol))))
break;
if (c == 'h' || c == 'H')
--- 7889,7899 ----
/* If this value won't fit into a 16 bit offset, then go
find a macro that will generate the 32 bit offset
! code pattern. */
if (c == 0
&& (offset_expr.X_op != O_constant
|| offset_expr.X_add_number >= 0x8000
! || offset_expr.X_add_number < -0x8000))
break;
if (c == 'h' || c == 'H')
***************
*** 11007,11012 ****
--- 11088,11131 ----
#endif
);
}
+ #if XXXCGD_USE_VAR_FRAG
+ else if (mips_pic == EMBEDDED_PIC)
+ {
+ symbolS *sym;
+ asection *symsec;
+
+ sym = fragp->fr_symbol;
+
+ /* Handle the case of a symbol equated to another symbol. */
+ while (symbol_equated_p (sym)
+ && (! S_IS_DEFINED (sym) || S_IS_COMMON (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);
+
+ /* This must duplicate the test in adjust_reloc_syms. */
+ /* XXX how can we be sure it'll actually fit?! */
+ change = (symsec == segtype
+ && symsec != &bfd_und_section
+ && symsec != &bfd_abs_section
+ && ! bfd_is_com_section (symsec)
+ && !linkonce
+ #ifdef OBJ_ELF
+ /* A weak symbol is treated as external. */
+ && ! S_IS_WEAK (sym)
+ #endif
+ );
+ }
+ #endif /* XXXCGD_USE_VAR_FRAG */
else
abort ();
***************
*** 11127,11132 ****
--- 11246,11255 ----
reloc and generate a new one. */
if (fixp->fx_frag->fr_opcode != NULL
&& (fixp->fx_r_type == BFD_RELOC_MIPS_GPREL
+ #if XXXCGD_USE_VAR_FRAG
+ || fixp->fx_r_type == BFD_RELOC_PCREL_HI16_S
+ || fixp->fx_r_type == BFD_RELOC_PCREL_LO16
+ #endif
|| fixp->fx_r_type == BFD_RELOC_MIPS_GOT16
|| fixp->fx_r_type == BFD_RELOC_MIPS_CALL16
|| fixp->fx_r_type == BFD_RELOC_MIPS_GOT_HI16
***************
*** 11134,11152 ****
|| fixp->fx_r_type == BFD_RELOC_MIPS_CALL_HI16
|| fixp->fx_r_type == BFD_RELOC_MIPS_CALL_LO16))
{
! arelent *reloc2;
assert (! RELAX_MIPS16_P (fixp->fx_frag->fr_subtype));
/* If this is not the last reloc in this frag, then we have two
GPREL relocs, or a GOT_HI16/GOT_LO16 pair, or a
! CALL_HI16/CALL_LO16, both of which are being replaced. Let
! the second one handle all of them. */
if (fixp->fx_next != NULL
&& fixp->fx_frag == fixp->fx_next->fx_frag)
{
assert ((fixp->fx_r_type == BFD_RELOC_MIPS_GPREL
&& fixp->fx_next->fx_r_type == BFD_RELOC_MIPS_GPREL)
|| (fixp->fx_r_type == BFD_RELOC_MIPS_GOT_HI16
&& (fixp->fx_next->fx_r_type
== BFD_RELOC_MIPS_GOT_LO16))
--- 11257,11280 ----
|| fixp->fx_r_type == BFD_RELOC_MIPS_CALL_HI16
|| fixp->fx_r_type == BFD_RELOC_MIPS_CALL_LO16))
{
! arelent *reloc2 = NULL;
! long orig_where;
assert (! RELAX_MIPS16_P (fixp->fx_frag->fr_subtype));
/* If this is not the last reloc in this frag, then we have two
GPREL relocs, or a GOT_HI16/GOT_LO16 pair, or a
! PCREL_HI16_S/PCREL_LO16 pair, or a CALL_HI16/CALL_LO16 pair,
! both of which are being replaced. Let the second one handle
! all of them. */
if (fixp->fx_next != NULL
&& fixp->fx_frag == fixp->fx_next->fx_frag)
{
assert ((fixp->fx_r_type == BFD_RELOC_MIPS_GPREL
&& fixp->fx_next->fx_r_type == BFD_RELOC_MIPS_GPREL)
+ || (fixp->fx_r_type == BFD_RELOC_PCREL_HI16_S
+ && (fixp->fx_next->fx_r_type
+ == BFD_RELOC_PCREL_LO16))
|| (fixp->fx_r_type == BFD_RELOC_MIPS_GOT_HI16
&& (fixp->fx_next->fx_r_type
== BFD_RELOC_MIPS_GOT_LO16))
***************
*** 11157,11179 ****
return retval;
}
fixp->fx_where = fixp->fx_frag->fr_opcode - fixp->fx_frag->fr_literal;
reloc->address = fixp->fx_frag->fr_address + fixp->fx_where;
! reloc2 = retval[1] = (arelent *) xmalloc (sizeof (arelent));
! retval[2] = NULL;
! reloc2->sym_ptr_ptr = (asymbol **) xmalloc (sizeof (asymbol *));
! *reloc2->sym_ptr_ptr = symbol_get_bfdsym (fixp->fx_addsy);
! reloc2->address = (reloc->address
! + (RELAX_RELOC2 (fixp->fx_frag->fr_subtype)
! - RELAX_RELOC1 (fixp->fx_frag->fr_subtype)));
! reloc2->addend = fixp->fx_addnumber;
! reloc2->howto = bfd_reloc_type_lookup (stdoutput, BFD_RELOC_LO16);
! assert (reloc2->howto != NULL);
if (RELAX_RELOC3 (fixp->fx_frag->fr_subtype))
{
arelent *reloc3;
reloc3 = retval[2] = (arelent *) xmalloc (sizeof (arelent));
retval[3] = NULL;
*reloc3 = *reloc2;
--- 11285,11315 ----
return retval;
}
+ orig_where = fixp->fx_where;
fixp->fx_where = fixp->fx_frag->fr_opcode - fixp->fx_frag->fr_literal;
reloc->address = fixp->fx_frag->fr_address + fixp->fx_where;
!
! /* If second reloc info is same as first, there is no second one. */
! if (RELAX_RELOC1 (fixp->fx_frag->fr_subtype)
! != RELAX_RELOC2 (fixp->fx_frag->fr_subtype))
! {
! reloc2 = retval[1] = (arelent *) xmalloc (sizeof (arelent));
! retval[2] = NULL;
! reloc2->sym_ptr_ptr = (asymbol **) xmalloc (sizeof (asymbol *));
! *reloc2->sym_ptr_ptr = symbol_get_bfdsym (fixp->fx_addsy);
! reloc2->address = (reloc->address
! + (RELAX_RELOC2 (fixp->fx_frag->fr_subtype)
! - RELAX_RELOC1 (fixp->fx_frag->fr_subtype)));
! reloc2->addend = fixp->fx_addnumber;
! reloc2->howto = bfd_reloc_type_lookup (stdoutput, BFD_RELOC_LO16);
! assert (reloc2->howto != NULL);
! }
if (RELAX_RELOC3 (fixp->fx_frag->fr_subtype))
{
arelent *reloc3;
+ assert (reloc2 != NULL);
reloc3 = retval[2] = (arelent *) xmalloc (sizeof (arelent));
retval[3] = NULL;
*reloc3 = *reloc2;
***************
*** 11200,11205 ****
--- 11336,11366 ----
break;
}
}
+ #if XXXCGD_USE_VAR_FRAG
+ else if (mips_pic == EMBEDDED_PIC)
+ {
+ assert (fixp->fx_r_type == BFD_RELOC_PCREL_LO16);
+ /* It stays that way. */
+
+ if (OUTPUT_FLAVOR == bfd_target_elf_flavour)
+ {
+ /* Fix up addend for change in location. This offsets the
+ addition of the 'where' value by the code marked "fragile
+ and not trustworthy" above. */
+ reloc->addend = fixp->fx_addnumber
+ - 2 * (orig_where - fixp->fx_where);
+ }
+ else
+ {
+ /* This code copied from the BFD_RELOC_PCREL_LO16 handling
+ near the top of this function. */
+ if (symbol_section_p (fixp->fx_addsy))
+ reloc->addend = reloc->address - S_GET_VALUE (fixp->fx_subsy);
+ else
+ reloc->addend = fixp->fx_addnumber + reloc->address;
+ }
+ }
+ #endif
else
abort ();
}