From: "Maciej W. Rozycki" <macro@codesourcery.com>
To: "Fu, Chao-Ying" <fu@mips.com>
Cc: Richard Sandiford <rdsandiford@googlemail.com>,
"binutils@sourceware.org" <binutils@sourceware.org>,
"Fuhler, Rich" <rich@mips.com>,
"Lau, David" <davidlau@mips.com>,
"Mills, Kevin" <kevinm@mips.com>,
"Garbacea, Ilie" <ilie@mips.com>,
Catherine Moore <clm@codesourcery.com>,
Nathan Sidwell <nathan@codesourcery.com>,
Joseph Myers <joseph@codesourcery.com>,
Nathan Froyd <froydnj@codesourcery.com>
Subject: RE: [PATCH] MIPS: microMIPS ASE support
Date: Thu, 24 Feb 2011 10:46:00 -0000 [thread overview]
Message-ID: <alpine.DEB.1.10.1102230034330.20460@tp.orcam.me.uk> (raw)
In-Reply-To: <7C6479EB2BF52547AC332FD6034646DA8495CBDC@exchdb01.mips.com>
On Tue, 22 Feb 2011, Fu, Chao-Ying wrote:
> > > > > + /* For microMIPS PC relative relocations, we cannot
> > > convert it to
> > > > > + against a section. If we do, it will mess up the
> > > fixp->fx_offset. */
> > > > > if (fixp->fx_r_type == BFD_RELOC_VTABLE_INHERIT
> > > > > - || fixp->fx_r_type == BFD_RELOC_VTABLE_ENTRY)
> > > > > + || fixp->fx_r_type == BFD_RELOC_VTABLE_ENTRY
> > > > > + || fixp->fx_r_type == BFD_RELOC_MICROMIPS_7_PCREL_S1
> > > > > + || fixp->fx_r_type == BFD_RELOC_MICROMIPS_10_PCREL_S1
> > > > > + || fixp->fx_r_type == BFD_RELOC_MICROMIPS_16_PCREL_S1)
> > > >
> > > > "to be against a section". That's not a helpful comment though.
> > > > _How_ will it mess up fixp->fx_offset? Give the reader a clue why
> > > > the problem applies to BFD_RELOC_MICROMIPS_16_PCREL_S1 but not
> > > > to something like BFD_RELOC_16_PCREL_S2.
>
> GAS resolves all MIPS pc-relative relocation types inside assembling, so there are no issues for
> BFD_RELOC_16_PCREL_S2. For microMIPS, GAS leaves them to the linker, so we hit new issues for
> microMIPS pc-relative relocation types. Thanks!
Thanks for the hint -- now I can see what's going on.
The problem is for REL targets we have a limited number of relocatable
bits in the instruction to store the in-place addend. The space does not
cover the whole address space and because these are PC-relative
relocations, if converted to section-relative ones, then they can overflow
even for legitimate symbol references.
As such the change belongs next to the check for BFD_RELOC_MIPS_JALR
relocations and likewise it does not apply to RELA targets. I have
updated the change accordingly.
Contrary to what you say the problem does apply to standard MIPS code
too, because BFD_RELOC_16_PCREL_S2 relocs against defined symbols are not
always resolved internally by GAS and R_MIPS_PC16 relocs are produced as
appropriate. Consider the following program:
$ cat bsec.s
.text
.space 0x40000
.Lbar:
addu $2, $3, $4
.section .init, "ax", @progbits
foo:
b .Lbar
As .Lbar is a local defined symbol from another section, a relocation is
placed into the output file for the static linker to resolve based on the
final section layout. The program does not build though:
$ mips-sde-elf-as -o bsec.o bsec.s
bsec.s: Assembler messages:
bsec.s:8: Error: relocation overflow
This is because .Lbar is too far into .text for the PC-relative offset to
fit into the relocatable field of the R_MIPS_PC16 reloc. Therefore this
reference shouldn't be made section-relative either. This observation
makes this change not specific to microMIPS relocations at all, so I have
separated it and I'm providing it below. I have removed the original hunk
from the microMIPS patch altogether now.
And for the record, MIPS16 branches produce no reloc at all (hence
excluded from the test case), even though they probably should.
2011-02-24 Maciej W. Rozycki <macro@codesourcery.com>
gas/
* config/tc-mips.c (mips_fix_adjustable): On REL targets also
reject PC-relative relocations.
gas/testsuite/
* gas/mips/branch-misc-2.d: Adjust for relocation change.
* gas/mips/branch-misc-2pic.d: Likewise.
* gas/mips/branch-misc-4.d: New test for PC-relative relocation
overflow.
* gas/mips/branch-misc-4-64.d: Likewise.
* gas/mips/branch-misc-4.s: Source for the new tests.
* testsuite/gas/mips/mips.exp: Run the new tests.
This change has been regression-tested for the mips-sde-elf and
mips-linux-gnu targets. OK to apply?
Maciej
binutils-gas-mips-fix-pc-rel.diff
Index: binutils-fsf-trunk-quilt/gas/config/tc-mips.c
===================================================================
--- binutils-fsf-trunk-quilt.orig/gas/config/tc-mips.c 2011-02-24 10:14:39.000000000 +0000
+++ binutils-fsf-trunk-quilt/gas/config/tc-mips.c 2011-02-24 10:22:22.000000000 +0000
@@ -14229,8 +14229,12 @@ mips_fix_adjustable (fixS *fixp)
&& (S_GET_SEGMENT (fixp->fx_addsy)->flags & SEC_MERGE) != 0)
return 0;
- /* There is no place to store an in-place offset for JALR relocations. */
- if (fixp->fx_r_type == BFD_RELOC_MIPS_JALR && HAVE_IN_PLACE_ADDENDS)
+ /* There is no place to store an in-place offset for JALR relocations.
+ Likewise an in-range offset of PC-relative relocations may overflow
+ the in-place relocatable field if recalculated against the start
+ address of the symbol's containing section. */
+ if (HAVE_IN_PLACE_ADDENDS
+ && (fixp->fx_pcrel || fixp->fx_r_type == BFD_RELOC_MIPS_JALR))
return 0;
#ifdef OBJ_ELF
Index: binutils-fsf-trunk-quilt/gas/testsuite/gas/mips/branch-misc-2.d
===================================================================
--- binutils-fsf-trunk-quilt.orig/gas/testsuite/gas/mips/branch-misc-2.d 2011-02-24 10:14:33.000000000 +0000
+++ binutils-fsf-trunk-quilt/gas/testsuite/gas/mips/branch-misc-2.d 2011-02-24 10:20:45.000000000 +0000
@@ -39,6 +39,6 @@
[ ]*b0: R_MIPS_PC16 x2
0+00b4 <[^>]*> 00000000 nop
0+00b8 <[^>]*> 1000ffff b 000000b8 <g6\+0x10>
-[ ]*b8: R_MIPS_PC16 \.data
+[ ]*b8: R_MIPS_PC16 \.Ldata
0+00bc <[^>]*> 00000000 nop
\.\.\.
Index: binutils-fsf-trunk-quilt/gas/testsuite/gas/mips/branch-misc-2pic.d
===================================================================
--- binutils-fsf-trunk-quilt.orig/gas/testsuite/gas/mips/branch-misc-2pic.d 2011-02-24 10:14:33.000000000 +0000
+++ binutils-fsf-trunk-quilt/gas/testsuite/gas/mips/branch-misc-2pic.d 2011-02-24 10:20:45.000000000 +0000
@@ -40,6 +40,6 @@
[ ]*b0: R_MIPS_PC16 x2
0+00b4 <[^>]*> 00000000 nop
0+00b8 <[^>]*> 1000ffff b 000000b8 <g6\+0x10>
-[ ]*b8: R_MIPS_PC16 \.data
+[ ]*b8: R_MIPS_PC16 \.Ldata
0+00bc <[^>]*> 00000000 nop
\.\.\.
Index: binutils-fsf-trunk-quilt/gas/testsuite/gas/mips/branch-misc-4.d
===================================================================
--- /dev/null 1970-01-01 00:00:00.000000000 +0000
+++ binutils-fsf-trunk-quilt/gas/testsuite/gas/mips/branch-misc-4.d 2011-02-24 10:22:59.000000000 +0000
@@ -0,0 +1,26 @@
+#objdump: -dr --prefix-addresses --show-raw-insn
+#name: MIPS branch-misc-4
+#as: -32
+
+# Verify PC-relative relocations do not overflow.
+
+.*: +file format .*mips.*
+
+Disassembly of section \.text:
+ \.\.\.
+([0-9a-f]+) <[^>]*> 1000ffff b \1 <foo>
+[ ]*[0-9a-f]+: R_MIPS_PC16 bar
+[0-9a-f]+ <[^>]*> 00000000 nop
+([0-9a-f]+) <[^>]*> 1000ffff b \1 <\.Lfoo>
+[ ]*[0-9a-f]+: R_MIPS_PC16 \.Lbar
+[0-9a-f]+ <[^>]*> 00000000 nop
+ \.\.\.
+
+Disassembly of section \.init:
+([0-9a-f]+) <[^>]*> 1000ffff b \1 <bar>
+[ ]*[0-9a-f]+: R_MIPS_PC16 foo
+[0-9a-f]+ <[^>]*> 00000000 nop
+([0-9a-f]+) <[^>]*> 1000ffff b \1 <\.Lbar>
+[ ]*[0-9a-f]+: R_MIPS_PC16 \.Lfoo
+[0-9a-f]+ <[^>]*> 00000000 nop
+ \.\.\.
Index: binutils-fsf-trunk-quilt/gas/testsuite/gas/mips/branch-misc-4.s
===================================================================
--- /dev/null 1970-01-01 00:00:00.000000000 +0000
+++ binutils-fsf-trunk-quilt/gas/testsuite/gas/mips/branch-misc-4.s 2011-02-24 10:14:40.000000000 +0000
@@ -0,0 +1,28 @@
+# Source file to verify PC-relative relocations do not overflow.
+
+ .text
+ .space 0x40000
+ .globl foo
+ .ent foo
+foo:
+ b bar
+.Lfoo:
+ b .Lbar
+ .end foo
+
+# Force at least 8 (non-delay-slot) zero bytes, to make 'objdump' print ...
+ .align 2
+ .space 8
+
+ .section .init, "ax", @progbits
+ .globl bar
+ .ent bar
+bar:
+ b foo
+.Lbar:
+ b .Lfoo
+ .end bar
+
+# Force at least 8 (non-delay-slot) zero bytes, to make 'objdump' print ...
+ .align 2
+ .space 8
Index: binutils-fsf-trunk-quilt/gas/testsuite/gas/mips/mips.exp
===================================================================
--- binutils-fsf-trunk-quilt.orig/gas/testsuite/gas/mips/mips.exp 2011-02-24 10:14:39.000000000 +0000
+++ binutils-fsf-trunk-quilt/gas/testsuite/gas/mips/mips.exp 2011-02-24 10:20:46.000000000 +0000
@@ -838,6 +838,10 @@ if { [istarget mips*-*-vxworks*] } {
run_dump_test_arches "loc-swap" [mips_arch_list_all]
run_dump_test_arches "loc-swap-dis" \
[mips_arch_list_all]
+ run_dump_test_arches "branch-misc-4" \
+ [mips_arch_list_matching mips1]
+ run_dump_test_arches "branch-misc-4-64" \
+ [mips_arch_list_matching mips3]
}
if $has_newabi {
Index: binutils-fsf-trunk-quilt/gas/testsuite/gas/mips/branch-misc-4-64.d
===================================================================
--- /dev/null 1970-01-01 00:00:00.000000000 +0000
+++ binutils-fsf-trunk-quilt/gas/testsuite/gas/mips/branch-misc-4-64.d 2011-02-24 10:14:40.000000000 +0000
@@ -0,0 +1,35 @@
+#objdump: -dr --prefix-addresses --show-raw-insn
+#name: MIPS branch-misc-4-64
+#as: -64
+#source: branch-misc-4.s
+
+# Verify PC-relative relocations do not overflow.
+
+.*: +file format .*mips.*
+
+Disassembly of section \.text:
+ \.\.\.
+[0-9a-f]+ <[^>]*> 10000000 b [0-9a-f]+ <foo\+0x[0-9a-f]+>
+[ ]*[0-9a-f]+: R_MIPS_PC16 bar\+0xf+fffc
+[ ]*[0-9a-f]+: R_MIPS_NONE \*ABS\*\+0xf+fffc
+[ ]*[0-9a-f]+: R_MIPS_NONE \*ABS\*\+0xf+fffc
+[0-9a-f]+ <[^>]*> 00000000 nop
+[0-9a-f]+ <[^>]*> 10000000 b [0-9a-f]+ <foo\+0x[0-9a-f]+>
+[ ]*[0-9a-f]+: R_MIPS_PC16 \.init\+0x4
+[ ]*[0-9a-f]+: R_MIPS_NONE \*ABS\*\+0x4
+[ ]*[0-9a-f]+: R_MIPS_NONE \*ABS\*\+0x4
+[0-9a-f]+ <[^>]*> 00000000 nop
+ \.\.\.
+
+Disassembly of section \.init:
+[0-9a-f]+ <[^>]*> 10000000 b [0-9a-f]+ <bar\+0x[0-9a-f]+>
+[ ]*[0-9a-f]+: R_MIPS_PC16 foo\+0xf+fffc
+[ ]*[0-9a-f]+: R_MIPS_NONE \*ABS\*\+0xf+fffc
+[ ]*[0-9a-f]+: R_MIPS_NONE \*ABS\*\+0xf+fffc
+[0-9a-f]+ <[^>]*> 00000000 nop
+[0-9a-f]+ <[^>]*> 10000000 b [0-9a-f]+ <bar\+0x[0-9a-f]+>
+[ ]*[0-9a-f]+: R_MIPS_PC16 \.text\+0x40004
+[ ]*[0-9a-f]+: R_MIPS_NONE \*ABS\*\+0x40004
+[ ]*[0-9a-f]+: R_MIPS_NONE \*ABS\*\+0x40004
+[0-9a-f]+ <[^>]*> 00000000 nop
+ \.\.\.
next prev parent reply other threads:[~2011-02-24 10:46 UTC|newest]
Thread overview: 41+ messages / expand[flat|nested] mbox.gz Atom feed top
2010-05-18 18:19 [PATCH] MIPS: microMIPS and MCU ASE instruction set support Maciej W. Rozycki
2010-05-23 21:38 ` Richard Sandiford
2010-05-24 22:25 ` Fu, Chao-Ying
2010-05-26 19:47 ` Richard Sandiford
2010-06-01 14:21 ` Maciej W. Rozycki
2010-06-01 14:39 ` Catherine Moore
2010-06-01 22:04 ` Richard Sandiford
2010-06-01 22:47 ` Fu, Chao-Ying
2010-06-05 9:17 ` Richard Sandiford
2010-07-26 10:56 ` [PATCH] MIPS: microMIPS ASE support Maciej W. Rozycki
2010-07-26 13:25 ` Nathan Froyd
2010-07-26 13:53 ` Maciej W. Rozycki
2010-07-26 19:03 ` Richard Sandiford
2010-12-07 1:13 ` Maciej W. Rozycki
2010-12-12 14:59 ` Richard Sandiford
2010-12-14 13:30 ` Maciej W. Rozycki
2010-12-14 14:51 ` Richard Sandiford
2010-12-16 11:54 ` Maciej W. Rozycki
2010-12-18 10:26 ` Richard Sandiford
2010-12-14 17:56 ` Joseph S. Myers
2010-12-16 15:28 ` Maciej W. Rozycki
2010-12-17 20:56 ` Fu, Chao-Ying
2010-12-18 10:09 ` Richard Sandiford
2011-01-02 11:36 ` Richard Sandiford
2011-02-21 15:35 ` Maciej W. Rozycki
2011-02-22 20:12 ` Fu, Chao-Ying
2011-02-22 20:19 ` Fu, Chao-Ying
2011-02-24 10:46 ` Maciej W. Rozycki [this message]
2011-02-26 11:41 ` Richard Sandiford
2011-02-28 16:41 ` Maciej W. Rozycki
2011-02-26 0:00 ` Maciej W. Rozycki
2011-03-13 9:23 ` Richard Sandiford
2011-07-25 7:49 ` Richard Sandiford
2011-07-26 2:01 ` Maciej W. Rozycki
2011-07-29 0:58 ` Maciej W. Rozycki
2011-07-29 11:30 ` Richard Sandiford
2011-07-29 22:52 ` Maciej W. Rozycki
2011-02-26 11:36 ` Richard Sandiford
2011-07-26 14:00 ` Maciej W. Rozycki
2010-05-26 20:19 ` [PATCH] MIPS: microMIPS and MCU ASE instruction set support Richard Sandiford
2010-05-27 21:39 ` Richard Sandiford
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=alpine.DEB.1.10.1102230034330.20460@tp.orcam.me.uk \
--to=macro@codesourcery.com \
--cc=binutils@sourceware.org \
--cc=clm@codesourcery.com \
--cc=davidlau@mips.com \
--cc=froydnj@codesourcery.com \
--cc=fu@mips.com \
--cc=ilie@mips.com \
--cc=joseph@codesourcery.com \
--cc=kevinm@mips.com \
--cc=nathan@codesourcery.com \
--cc=rdsandiford@googlemail.com \
--cc=rich@mips.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).