This is the mail archive of the
binutils@sourceware.org
mailing list for the binutils project.
[RFA:] Fix ld/13990, ARM BFD_ASSERT with --shared and --gc-sections
- From: Hans-Peter Nilsson <hans-peter dot nilsson at axis dot com>
- To: binutils at sourceware dot org
- Date: Thu, 19 Apr 2012 01:07:25 +0200
- Subject: [RFA:] Fix ld/13990, ARM BFD_ASSERT with --shared and --gc-sections
This bug will strike for e.g. arm-linux-gnueabi when:
- There's a reference (call-reloc) to a symbol undefined at first
reference, preliminary requiring a PLT.
- The symbols' definition is found in the linked objects and is
hidden (or forced local).
- The referencing section (containing the call-reloc) is GC'd
(its global symbols are forced local by the version script and
found to be unused).
Then, the gc_sweep function tries to balance the books by -= 1
on the now-GC'd-away PLT (with refcount set to -1 from the
already forced-local symbol) which is guarded by a "BFD_ASSERT
(root_plt->refcount > 0)". So the assert strikes.
But, the non-positive PLT refcount is supposed to be exactly
that -1 (and there's no fallout later on) and such negative
counts are handled elsewhere in elf32-arm.c. The special PLT
and GOT accounting in elf32-arm.c doesn't allocate anything and
looking around in that file I found no other reason to add the
usual target-specific elf_backend_hide_symbol to handle the
hiding specially; just checking for a negative count seems
sufficient.
The assert is there to catch book-keeping errors, but as those
counts are balanced one item at a time, it's expected to see the
value 0 before seeing -1 on such errors: better add such an
BFD_ASSERT to not lose any error sentinels. Also adding one on
other negative values for good measure.
So *this time* the BFD_ASSERT and ignoring it was benevolent,
and the output probably still works; I haven't tested besides
checking that the exact same file is output.
Tested arm-eabi, arm-linux-gnueabi, arm-unknown-nacl (to see
that the test is skipped; it pretty much had to be, because the
addresses are all different) and a bunch of other arm-targets
but which return early in arm-elf.exp and thus uninteresting.
Ok to commit?
ld/testsuite:
PR ld/13990
* ld-arm/arm-elf.exp: Run gc-hidden-1.
* gc-hidden-1.d: New test-file.
* gcdfn.s, hideall.ld, hidfn.s, main.s: New files.
bfd:
PR ld/13990
* elf32-arm.c (elf32_arm_gc_sweep_hook): Handle a forced-local
symbol, where PLT refcount is set to -1.
diff --git a/ld/testsuite/ld-arm/arm-elf.exp b/ld/testsuite/ld-arm/arm-elf.exp
index 05dc7bb..b6eb3d3 100644
--- a/ld/testsuite/ld-arm/arm-elf.exp
+++ b/ld/testsuite/ld-arm/arm-elf.exp
@@ -759,3 +759,4 @@ run_dump_test "attr-merge-vfp-6r"
run_dump_test "attr-merge-incompatible"
run_dump_test "unresolved-1"
run_dump_test "unresolved-1-dyn"
+run_dump_test "gc-hidden-1"
diff --git a/ld/testsuite/ld-arm/gc-hidden-1.d b/ld/testsuite/ld-arm/gc-hidden-1.d
new file mode 100644
index 0000000..80c7e9e
--- /dev/null
+++ b/ld/testsuite/ld-arm/gc-hidden-1.d
@@ -0,0 +1,25 @@
+#target: arm*-*-*eabi
+#source: main.s
+#source: gcdfn.s
+#source: hidfn.s
+#ld: --gc-sections --shared --version-script hideall.ld
+#objdump: -dRT
+
+# See PR ld/13990: a forced-local PLT reference to a
+# forced-local symbol is GC'ed, trigging a BFD_ASSERT.
+
+.*: file format elf32-.*
+
+DYNAMIC SYMBOL TABLE:
+0+124 l d .text 0+ .text
+0+ g DO \*ABS\* 0+ NS NS
+
+Disassembly of section .text:
+
+0+124 <_start>:
+ 124: e52de004 push {lr} ; \(str lr, \[sp, #-4\]!\)
+ 128: eb000000 bl 130 <hidfn>
+ 12c: e8bd8000 pop {pc}
+
+0+130 <hidfn>:
+ 130: e8bd8000 pop {pc}
diff --git a/ld/testsuite/ld-arm/gcdfn.s b/ld/testsuite/ld-arm/gcdfn.s
new file mode 100644
index 0000000..f2afae7
--- /dev/null
+++ b/ld/testsuite/ld-arm/gcdfn.s
@@ -0,0 +1,8 @@
+ .text
+ .globl gcdfn
+ .type gcdfn, %function
+gcdfn:
+ str lr, [sp, #-4]!
+ bl hidfn(PLT)
+ ldmfd sp!, {pc}
+ .size gcdfn, . - gcdfn
diff --git a/ld/testsuite/ld-arm/hideall.ld b/ld/testsuite/ld-arm/hideall.ld
new file mode 100644
index 0000000..077d6b5
--- /dev/null
+++ b/ld/testsuite/ld-arm/hideall.ld
@@ -0,0 +1 @@
+NS { local: *; };
diff --git a/ld/testsuite/ld-arm/hidfn.s b/ld/testsuite/ld-arm/hidfn.s
new file mode 100644
index 0000000..a66b558
--- /dev/null
+++ b/ld/testsuite/ld-arm/hidfn.s
@@ -0,0 +1,7 @@
+ .text
+ .globl hidfn
+ .hidden hidfn
+ .type hidfn, %function
+hidfn:
+ ldmfd sp!, {pc}
+ .size hidfn, . - hidfn
diff --git a/ld/testsuite/ld-arm/main.s b/ld/testsuite/ld-arm/main.s
new file mode 100644
index 0000000..046d19d
--- /dev/null
+++ b/ld/testsuite/ld-arm/main.s
@@ -0,0 +1,8 @@
+ .text
+ .globl _start
+ .type _start, %function
+_start:
+ str lr, [sp, #-4]!
+ bl hidfn(PLT)
+ ldmfd sp!, {pc}
+ .size _start, . - _start
diff --git a/bfd/elf32-arm.c b/bfd/elf32-arm.c
index f5b5c4d..5907974 100644
--- a/bfd/elf32-arm.c
+++ b/bfd/elf32-arm.c
@@ -12256,18 +12256,34 @@ elf32_arm_gc_sweep_hook (bfd * abfd,
if (may_need_local_target_p
&& elf32_arm_get_plt_info (abfd, eh, r_symndx, &root_plt, &arm_plt))
{
- BFD_ASSERT (root_plt->refcount > 0);
- root_plt->refcount -= 1;
+ /* If PLT refcount book-keeping is wrong and too low, we'll
+ see a zero value (going to -1) for the root PLT reference
+ count. */
+ if (root_plt->refcount >= 0)
+ {
+ BFD_ASSERT (root_plt->refcount != 0);
+ root_plt->refcount -= 1;
+
+ /* No use incrementing these members as all their uses are
+ dominated by tests on the PLT refcount being
+ non-negative. */
+ if (r_type == R_ARM_THM_CALL)
+ arm_plt->maybe_thumb_refcount--;
+
+ if (r_type == R_ARM_THM_JUMP24
+ || r_type == R_ARM_THM_JUMP19)
+ arm_plt->thumb_refcount--;
+ }
+ else
+ /* A value of -1 means the symbol has become local, forced
+ or seeing a hidden definition. Any other negative value
+ is an error. */
+ BFD_ASSERT (root_plt->refcount == -1);
+ /* Use of this member (controlling the IPLT type), is not
+ fully dominated by PLT refcount tests so always update it. */
if (!call_reloc_p)
arm_plt->noncall_refcount--;
-
- if (r_type == R_ARM_THM_CALL)
- arm_plt->maybe_thumb_refcount--;
-
- if (r_type == R_ARM_THM_JUMP24
- || r_type == R_ARM_THM_JUMP19)
- arm_plt->thumb_refcount--;
}
if (may_become_dynamic_p)
brgds, H-P