This is the mail archive of the binutils@sourceware.org mailing list for the binutils project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[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


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]