This is the mail archive of the
binutils@sourceware.org
mailing list for the binutils project.
Refix MIPS GOT_PAGE counting
- From: Richard Sandiford <rdsandiford at googlemail dot com>
- To: binutils at sourceware dot org
- Cc: wong dot kwongyuan at gmail dot com
- Date: Sun, 15 May 2011 20:08:45 +0100
- Subject: Refix MIPS GOT_PAGE counting
This message:
http://sourceware.org/ml/binutils/2004-02/msg00195.html
was about a case in which we failed to allocate GOT entries for
GOT_PAGE references. The thread included exchanges such as:
Richard Sandiford wrote:
> Daniel Jacobowitz wrote:
>> Richard Sandiford wrote:
>>> I've attached a shell archive (to be run from the top level of the
>>> build directory). Before the patch, a.x needlessly had a global GOT
>>> entry for gs. After the patch, both a.x and b.x only have a global
>>> GOT entry for "us".
>>
>> Interested in adding this to the testsuite? :)
> Maybe...
and:
Richard Sandiford wrote:
> I don't think it's worth adding the attachment to dejagnu since (a) the
> current sources pass it anyway and (b) we can use the existing elf-rel-got*
> tests to make sure that locally-binding symbols use page entries. As below.
Well, the custard pie of fate has finished its seven year trajectory
and I now get to fix the test again. If a GOT_PAGE entry is against
a global symbol, we need to account for both page and global entries,
because we don't know at this stage which is needed.
For the record, the regression was introduced by:
http://sourceware.org/ml/binutils/2008-03/msg00233.html
which was itself fixing a bug I'd introduced with an earlier patch.
My patch only counted global entries, the fix only counted local entries,
and this one counts both.
I've now done what I should have done originally and added the
testcase to dejagnu.
At the moment, run_ld_link_tests postpones the decision about whether a
link failure is good or bad until it sees whether the first action is an
error-matching test. It therefore passes any test that has no actions,
regardless of whether the link fails or succeeds. That doesn't seem
particularly useful, so I've made "no actions" == "test that the
link succeeds".
Tested on mips64-linux-gnu, mipsisa32el-linux-gnu, mips64-elf and
x86_64-linux-gnu. Also tested with a GCC regression test on
mips64-linux-gnu. Applied to trunk and branch.
Many thanks to Wang Jiong for finding the bug and for tracking it down
to a regression in the original testcase.
Richard
bfd/
* elfxx-mips.c (_bfd_mips_elf_check_relocs): Record both local and
global GOT entries for GOT_PAGE relocations against global symbols.
ld/testsuite/
* lib/ld-lib.exp (run_ld_link_tests): Simplify pass/fail logic.
Fail if the link command fails and if no test rules are defined.
* ld-mips-elf/reloc-6a.s, ld-mips-elf/reloc-6b.s: New tests.
* ld-mips-elf/mips-elf.exp: Run them.
Index: bfd/elfxx-mips.c
===================================================================
--- bfd/elfxx-mips.c 2011-05-15 13:50:51.000000000 +0100
+++ bfd/elfxx-mips.c 2011-05-15 13:52:26.000000000 +0100
@@ -7743,7 +7743,6 @@ _bfd_mips_elf_check_relocs (bfd *abfd, s
if (!mips_elf_record_got_page_entry (info, abfd, r_symndx,
addend))
return FALSE;
- break;
}
/* Fall through. */
Index: ld/testsuite/lib/ld-lib.exp
===================================================================
--- ld/testsuite/lib/ld-lib.exp 2011-05-15 14:02:18.000000000 +0100
+++ ld/testsuite/lib/ld-lib.exp 2011-05-15 15:19:03.000000000 +0100
@@ -978,26 +978,21 @@ proc run_ld_link_tests { ldtests } {
}
# Catch assembler errors.
- if { $is_unresolved != 0 } {
+ if { $is_unresolved } {
unresolved $testname
continue
}
if { [regexp ".*\\.a$" $binfile] } {
if { ![ar_simple_create $ar $ld_options $binfile "$objfiles"] } {
- fail $testname
set failed 1
- } else {
- set failed 0
}
} elseif { ![ld_simple_link $ld $binfile "-L$srcdir/$subdir $ld_options $objfiles"] } {
set maybe_failed 1
set ld_output "$exec_output"
- } else {
- set failed 0
}
- if { $failed == 0 } {
+ if { !$failed } {
foreach actionlist $actions {
set action [lindex $actionlist 0]
set progopts [lindex $actionlist 1]
@@ -1034,10 +1029,7 @@ proc run_ld_link_tests { ldtests } {
break
}
set maybe_failed 0
- } elseif { $maybe_failed != 0 } {
- set failed 1
- break
- } elseif { $dump_prog != "" } {
+ } elseif { !$maybe_failed && $dump_prog != "" } {
set dumpfile [lindex $actionlist 2]
set binary $dump_prog
@@ -1079,18 +1071,14 @@ proc run_ld_link_tests { ldtests } {
remote_file host delete "dump.out"
}
}
-
- if { $failed != 0 } {
- fail $testname
- } else { if { $is_unresolved == 0 } {
- pass $testname
- } }
}
- # Catch action errors.
- if { $is_unresolved != 0 } {
+ if { $is_unresolved } {
unresolved $testname
- continue
+ } elseif { $maybe_failed || $failed } {
+ fail $testname
+ } else {
+ pass $testname
}
}
}
Index: ld/testsuite/ld-mips-elf/reloc-6a.s
===================================================================
--- /dev/null 2011-05-15 08:56:06.273397198 +0100
+++ ld/testsuite/ld-mips-elf/reloc-6a.s 2011-05-15 13:54:43.000000000 +0100
@@ -0,0 +1,11 @@
+ .globl us
+ .globl gs
+us:
+gs:
+ls:
+ lw $4,%got_page(us)($gp)
+ addiu $4,$4,%got_ofst(us)
+ lw $4,%got_page(gs)($gp)
+ addiu $4,$4,%got_ofst(gs)
+ lw $4,%got_page(ls)($gp)
+ addiu $4,$4,%got_ofst(ls)
Index: ld/testsuite/ld-mips-elf/reloc-6b.s
===================================================================
--- /dev/null 2011-05-15 08:56:06.273397198 +0100
+++ ld/testsuite/ld-mips-elf/reloc-6b.s 2011-05-15 13:54:59.000000000 +0100
@@ -0,0 +1,11 @@
+ .globl __start
+ .globl gs
+__start:
+gs:
+ls:
+ lw $4,%got_page(us)($gp)
+ addiu $4,$4,%got_ofst(us)
+ lw $4,%got_page(gs)($gp)
+ addiu $4,$4,%got_ofst(gs)
+ lw $4,%got_page(ls)($gp)
+ addiu $4,$4,%got_ofst(ls)
Index: ld/testsuite/ld-mips-elf/mips-elf.exp
===================================================================
--- ld/testsuite/ld-mips-elf/mips-elf.exp 2011-05-15 13:51:16.000000000 +0100
+++ ld/testsuite/ld-mips-elf/mips-elf.exp 2011-05-15 14:09:01.000000000 +0100
@@ -298,6 +298,18 @@ if {$has_newabi} {
}
run_dump_test "reloc-4"
run_dump_test "reloc-5"
+if { $has_newabi } {
+ run_ld_link_tests {
+ {"reloc test 6a" "-shared"
+ "-n32" "reloc-6a.s"
+ {}
+ "reloc-6a.so"}
+ {"reloc test 6b" "tmpdir/reloc-6a.so"
+ "-n32" "reloc-6b.s"
+ {}
+ "reloc-6b"}
+ }
+}
if {$has_newabi && $linux_gnu} {
run_dump_test "eh-frame1-n32"