PR ld/16643: ARM: BFD_ASSERT when only ref to a GC'd symbol is in a stripped section

PR ld/16643: ARM: BFD_ASSERT when only ref to a GC'd symbol is in a stripped section

[Roland McGrath]

Below is a patch that adds a test case for bug 16643, which I've just
filed.  I don't have a fix.  I'm seeking advice on what the proper fix
might be.

The case is under --gc-sections --strip-all.  My test is doing -shared
mostly just because that's what the original case was doing, but it
actually doesn't matter whether it's -shared or a normal final link.

The situation is that there is a symbol, defined in a section that will be
GC'd, whose only reference is in a non-GC'd but non-allocated section that
will be stripped.  (In the real case, it was in .debug_info as part of the
real DWARF data.)  It hits this assert:
	  /* 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;
	    }
(elf32-arm.c:12445 in today's trunk).  This hits in the call to
elf32_arm_gc_sweep_hook for the input .debug_blah section (the
non-allocated section containing a reference to the symbol), which will be
stripped from the output.  Without --strip-all, the assert does not hit.

I haven't had much luck trying to figure out where the PLT refcount is
supposed to be maintained in general.  Apparently, something about
--strip-all mode affects how this bookkeeping gets done.  Can anyone point
me to where the actual bug here might be?


Thanks,
Roland


ld/testsuite/
2014-02-27  Roland McGrath  <mcgrathr@google.com>

	PR ld/16643
	* ld-arm/gc-hidden-strip.d: New file.
	* ld-arm/gc-hidden-strip-unused.s: New file.
	* ld-arm/gc-hidden-strip-main.s: New file.
	* ld-arm/arm-elf.exp: Run the new test.

--- a/ld/testsuite/ld-arm/arm-elf.exp
+++ b/ld/testsuite/ld-arm/arm-elf.exp
@@ -842,3 +842,4 @@ if { ![istarget "arm*-*-nacl*"] } {
 }
 run_dump_test "unresolved-2"
 run_dump_test "gc-hidden-1"
+run_dump_test "gc-hidden-strip"
--- /dev/null
+++ b/ld/testsuite/ld-arm/gc-hidden-strip-main.s
@@ -0,0 +1,6 @@
+	.text
+	.globl foo
+	.type foo, %function
+foo:
+	bx lr
+	.size foo, . - foo
--- /dev/null
+++ b/ld/testsuite/ld-arm/gc-hidden-strip-unused.s
@@ -0,0 +1,11 @@
+	.section .data.unused_item,"aw",%progbits
+	.p2align 2
+	.global unused_item
+	.hidden	unused_item
+	.type unused_item, %object
+	.size unused_item, 4
+unused_item:
+	.word 1
+
+	.section .debug_blah,"",%progbits
+	.word unused_item
--- /dev/null
+++ b/ld/testsuite/ld-arm/gc-hidden-strip.d
@@ -0,0 +1,15 @@
+#source: gc-hidden-strip-main.s
+#source: gc-hidden-strip-unused.s
+#ld: --gc-sections --shared --strip-all
+#objdump: -RT
+# This test is only valid on ELF based ports.
+# not-target: *-*-*coff *-*-pe *-*-wince *-*-*aout* *-*-netbsd *-*-riscix*
+
+# See PR ld/16643: the only reference to a GC'd symbol is in a stripped
+# section, trigging a BFD_ASSERT.
+
+.*:     file format elf32-.*
+
+DYNAMIC SYMBOL TABLE:
+0+[0-9a-f]+ l\s+d\s+\.text\s+0+\s+\.text
+#pass

Re: PR ld/16643: ARM: BFD_ASSERT when only ref to a GC'd symbol is in a stripped section

[Alan Modra]

On Thu, Feb 27, 2014 at 03:07:15PM -0800, Roland McGrath wrote:
> I haven't had much luck trying to figure out where the PLT refcount is
> supposed to be maintained in general.  Apparently, something about
> --strip-all mode affects how this bookkeeping gets done.  Can anyone point
> me to where the actual bug here might be?

This sort of bug is always a mismatch between check_relocs and
gc_sweep_hook.  In this case, it looks like the problem is in
elflink.c, where check_relocs is called depending on info->strip and
SEC_DEBUGGING, but gc_sweep_hook is called without those tests.

Of course, the arm backend is slightly insane in counting plt entries
for references from debug sections..  PowerPC for example does this
in check_relocs:

  /* Don't do anything special with non-loaded, non-alloced sections.
     In particular, any relocs in such sections should not affect GOT
     and PLT reference counting (ie. we don't allow them to create GOT
     or PLT entries), there's no possibility or desire to optimize TLS
     relocs, and there's not much point in propagating relocs to shared
     libs that the dynamic linker won't relocate.  */
  if ((sec->flags & SEC_ALLOC) == 0)
    return TRUE;

Here you go, this cures your testcase.

	PR ld/16643
	* elflink.c (elf_gc_sweep): Call gc_sweep_hook for exactly
	the same conditions we called check_relocs.

diff --git a/bfd/elflink.c b/bfd/elflink.c
index 28ccf53..47e4802 100644
--- a/bfd/elflink.c
+++ b/bfd/elflink.c
@@ -11992,7 +11992,9 @@ elf_gc_sweep (bfd *abfd, struct bfd_link_info *info)
 	     info we collected before.  */
 	  if (gc_sweep_hook
 	      && (o->flags & SEC_RELOC) != 0
-	      && o->reloc_count > 0
+	      && o->reloc_count != 0
+	      && !((info->strip == strip_all || info->strip == strip_debugger)
+		   && (o->flags & SEC_DEBUGGING) != 0)
 	      && !bfd_is_abs_section (o->output_section))
 	    {
 	      Elf_Internal_Rela *internal_relocs;

-- 
Alan Modra
Australia Development Lab, IBM

Re: PR ld/16643: ARM: BFD_ASSERT when only ref to a GC'd symbol is in a stripped section

[Roland McGrath]

On Thu, Feb 27, 2014 at 4:25 PM, Alan Modra <amodra@gmail.com> wrote:
> Here you go, this cures your testcase.

Great!  Thanks a lot.

OK to commit the test case now?

OK to backport both fix and test to 2.24?


Thanks,
Roland


ld/testsuite/
2014-02-28  Roland McGrath  <mcgrathr@google.com>

	PR ld/16643
	* ld-arm/gc-hidden-strip.d: New file.
	* ld-arm/gc-hidden-strip-unused.s: New file.
	* ld-arm/gc-hidden-strip-main.s: New file.
	* ld-arm/arm-elf.exp: Run the new test.

--- a/ld/testsuite/ld-arm/arm-elf.exp
+++ b/ld/testsuite/ld-arm/arm-elf.exp
@@ -842,3 +842,4 @@ if { ![istarget "arm*-*-nacl*"] } {
 }
 run_dump_test "unresolved-2"
 run_dump_test "gc-hidden-1"
+run_dump_test "gc-hidden-strip"
--- /dev/null
+++ b/ld/testsuite/ld-arm/gc-hidden-strip-main.s
@@ -0,0 +1,6 @@
+	.text
+	.globl foo
+	.type foo, %function
+foo:
+	bx lr
+	.size foo, . - foo
--- /dev/null
+++ b/ld/testsuite/ld-arm/gc-hidden-strip-unused.s
@@ -0,0 +1,11 @@
+	.section .data.unused_item,"aw",%progbits
+	.p2align 2
+	.global unused_item
+	.hidden	unused_item
+	.type unused_item, %object
+	.size unused_item, 4
+unused_item:
+	.word 1
+
+	.section .debug_blah,"",%progbits
+	.word unused_item
--- /dev/null
+++ b/ld/testsuite/ld-arm/gc-hidden-strip.d
@@ -0,0 +1,15 @@
+#source: gc-hidden-strip-main.s
+#source: gc-hidden-strip-unused.s
+#ld: --gc-sections --shared --strip-all
+#objdump: -RT
+# This test is only valid on ELF based ports.
+# not-target: *-*-*coff *-*-pe *-*-wince *-*-*aout* *-*-netbsd *-*-riscix*
+
+# See PR ld/16643: the only reference to a GC'd symbol is in a stripped
+# section, trigging a BFD_ASSERT.
+
+.*:     file format elf32-.*
+
+DYNAMIC SYMBOL TABLE:
+0+[0-9a-f]+ l\s+d\s+\.text\s+0+\s+\.text
+#pass

Re: PR ld/16643: ARM: BFD_ASSERT when only ref to a GC'd symbol is in a stripped section

[Roland McGrath]

ping.
Still looking for maintainer approval to commit my test case.
Still looking for release maintainer approval to backport fix+test to 2.24.

On Fri, Feb 28, 2014 at 9:44 AM, Roland McGrath <mcgrathr@google.com> wrote:
> On Thu, Feb 27, 2014 at 4:25 PM, Alan Modra <amodra@gmail.com> wrote:
>> Here you go, this cures your testcase.
>
> Great!  Thanks a lot.
>
> OK to commit the test case now?
>
> OK to backport both fix and test to 2.24?
>
>
> Thanks,
> Roland
>
>
> ld/testsuite/
> 2014-02-28  Roland McGrath  <mcgrathr@google.com>
>
>         PR ld/16643
>         * ld-arm/gc-hidden-strip.d: New file.
>         * ld-arm/gc-hidden-strip-unused.s: New file.
>         * ld-arm/gc-hidden-strip-main.s: New file.
>         * ld-arm/arm-elf.exp: Run the new test.
>
> --- a/ld/testsuite/ld-arm/arm-elf.exp
> +++ b/ld/testsuite/ld-arm/arm-elf.exp
> @@ -842,3 +842,4 @@ if { ![istarget "arm*-*-nacl*"] } {
>  }
>  run_dump_test "unresolved-2"
>  run_dump_test "gc-hidden-1"
> +run_dump_test "gc-hidden-strip"
> --- /dev/null
> +++ b/ld/testsuite/ld-arm/gc-hidden-strip-main.s
> @@ -0,0 +1,6 @@
> +       .text
> +       .globl foo
> +       .type foo, %function
> +foo:
> +       bx lr
> +       .size foo, . - foo
> --- /dev/null
> +++ b/ld/testsuite/ld-arm/gc-hidden-strip-unused.s
> @@ -0,0 +1,11 @@
> +       .section .data.unused_item,"aw",%progbits
> +       .p2align 2
> +       .global unused_item
> +       .hidden unused_item
> +       .type unused_item, %object
> +       .size unused_item, 4
> +unused_item:
> +       .word 1
> +
> +       .section .debug_blah,"",%progbits
> +       .word unused_item
> --- /dev/null
> +++ b/ld/testsuite/ld-arm/gc-hidden-strip.d
> @@ -0,0 +1,15 @@
> +#source: gc-hidden-strip-main.s
> +#source: gc-hidden-strip-unused.s
> +#ld: --gc-sections --shared --strip-all
> +#objdump: -RT
> +# This test is only valid on ELF based ports.
> +# not-target: *-*-*coff *-*-pe *-*-wince *-*-*aout* *-*-netbsd *-*-riscix*
> +
> +# See PR ld/16643: the only reference to a GC'd symbol is in a stripped
> +# section, trigging a BFD_ASSERT.
> +
> +.*:     file format elf32-.*
> +
> +DYNAMIC SYMBOL TABLE:
> +0+[0-9a-f]+ l\s+d\s+\.text\s+0+\s+\.text
> +#pass

Re: PR ld/16643: ARM: BFD_ASSERT when only ref to a GC'd symbol is in a stripped section

[Tristan Gingold]

On 05 Mar 2014, at 19:07, Roland McGrath <mcgrathr@google.com> wrote:

> ping.
> Still looking for maintainer approval to commit my test case.
> Still looking for release maintainer approval to backport fix+test to 2.24.

OK for 2.24 once approved on trunk.

Tristan.

> 
> On Fri, Feb 28, 2014 at 9:44 AM, Roland McGrath <mcgrathr@google.com> wrote:
>> On Thu, Feb 27, 2014 at 4:25 PM, Alan Modra <amodra@gmail.com> wrote:
>>> Here you go, this cures your testcase.
>> 
>> Great!  Thanks a lot.
>> 
>> OK to commit the test case now?
>> 
>> OK to backport both fix and test to 2.24?
>> 
>> 
>> Thanks,
>> Roland
>> 
>> 
>> ld/testsuite/
>> 2014-02-28  Roland McGrath  <mcgrathr@google.com>
>> 
>>        PR ld/16643
>>        * ld-arm/gc-hidden-strip.d: New file.
>>        * ld-arm/gc-hidden-strip-unused.s: New file.
>>        * ld-arm/gc-hidden-strip-main.s: New file.
>>        * ld-arm/arm-elf.exp: Run the new test.
>> 
>> --- a/ld/testsuite/ld-arm/arm-elf.exp
>> +++ b/ld/testsuite/ld-arm/arm-elf.exp
>> @@ -842,3 +842,4 @@ if { ![istarget "arm*-*-nacl*"] } {
>> }
>> run_dump_test "unresolved-2"
>> run_dump_test "gc-hidden-1"
>> +run_dump_test "gc-hidden-strip"
>> --- /dev/null
>> +++ b/ld/testsuite/ld-arm/gc-hidden-strip-main.s
>> @@ -0,0 +1,6 @@
>> +       .text
>> +       .globl foo
>> +       .type foo, %function
>> +foo:
>> +       bx lr
>> +       .size foo, . - foo
>> --- /dev/null
>> +++ b/ld/testsuite/ld-arm/gc-hidden-strip-unused.s
>> @@ -0,0 +1,11 @@
>> +       .section .data.unused_item,"aw",%progbits
>> +       .p2align 2
>> +       .global unused_item
>> +       .hidden unused_item
>> +       .type unused_item, %object
>> +       .size unused_item, 4
>> +unused_item:
>> +       .word 1
>> +
>> +       .section .debug_blah,"",%progbits
>> +       .word unused_item
>> --- /dev/null
>> +++ b/ld/testsuite/ld-arm/gc-hidden-strip.d
>> @@ -0,0 +1,15 @@
>> +#source: gc-hidden-strip-main.s
>> +#source: gc-hidden-strip-unused.s
>> +#ld: --gc-sections --shared --strip-all
>> +#objdump: -RT
>> +# This test is only valid on ELF based ports.
>> +# not-target: *-*-*coff *-*-pe *-*-wince *-*-*aout* *-*-netbsd *-*-riscix*
>> +
>> +# See PR ld/16643: the only reference to a GC'd symbol is in a stripped
>> +# section, trigging a BFD_ASSERT.
>> +
>> +.*:     file format elf32-.*
>> +
>> +DYNAMIC SYMBOL TABLE:
>> +0+[0-9a-f]+ l\s+d\s+\.text\s+0+\s+\.text
>> +#pass

Re: PR ld/16643: ARM: BFD_ASSERT when only ref to a GC'd symbol is in a stripped section

[Roland McGrath]

On Thu, Mar 6, 2014 at 12:18 AM, Tristan Gingold <gingold@adacore.com> wrote:
> OK for 2.24 once approved on trunk.

Committed to 2.24.

Thanks,
Roland

Re: PR ld/16643: ARM: BFD_ASSERT when only ref to a GC'd symbol is in a stripped section

[Nicholas Clifton]

Hi Roland,


>> ld/testsuite/
>> 2014-02-28  Roland McGrath  <mcgrathr@google.com>
>>
>>          PR ld/16643
>>          * ld-arm/gc-hidden-strip.d: New file.
>>          * ld-arm/gc-hidden-strip-unused.s: New file.
>>          * ld-arm/gc-hidden-strip-main.s: New file.
>>          * ld-arm/arm-elf.exp: Run the new test.

Approved - please apply.  Sorry for the delay.

Cheers
   Nick

Re: PR ld/16643: ARM: BFD_ASSERT when only ref to a GC'd symbol is in a stripped section

[Roland McGrath]

On Thu, Mar 6, 2014 at 12:54 AM, Nicholas Clifton <nickc@redhat.com> wrote:
>>> ld/testsuite/
>>> 2014-02-28  Roland McGrath  <mcgrathr@google.com>
>>>
>>>          PR ld/16643
>>>          * ld-arm/gc-hidden-strip.d: New file.
>>>          * ld-arm/gc-hidden-strip-unused.s: New file.
>>>          * ld-arm/gc-hidden-strip-main.s: New file.
>>>          * ld-arm/arm-elf.exp: Run the new test.
>
>
> Approved - please apply.  Sorry for the delay.

Commited to trunk.

Thanks,
Roland