Bug 22374 - Unnecessary PLT entries
Summary: Unnecessary PLT entries
Status: RESOLVED FIXED
Alias: None
Product: binutils
Classification: Unclassified
Component: ld (show other bugs)
Version: 2.30
: P2 normal
Target Milestone: ---
Assignee: Alan Modra
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2017-10-31 13:38 UTC by Alan Modra
Modified: 2023-12-15 22:48 UTC (History)
2 users (show)

See Also:
Host:
Target:
Build:
Last reconfirmed:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Alan Modra 2017-10-31 13:38:32 UTC
powerpc and a number of other architectures needlessly generate a PLT entry when function pointer initialization in a read/write section is the only reference to a given external function symbol.

alpha, hppa64, ia64, x86, x86_64 and xtensa look to be good,
Comment 1 H.J. Lu 2017-10-31 19:00:31 UTC
FWIW, x86 has

  /* Reference count of C/C++ function pointer relocations in read-write
     section which can be resolved at run-time.  */
  bfd_signed_vma func_pointer_refcount;

and

  /* Don't create the PLT entry if there are only function pointer
     relocations which can be resolved at run-time.  */
  else if (htab->elf.dynamic_sections_created
           && (h->plt.refcount > eh->func_pointer_refcount
               || eh->plt_got.refcount > 0))
    {
Comment 2 Alan Modra 2017-11-01 00:45:04 UTC
Thanks HJ.  The powerpc patch I'm testing doesn't need reference counting of function pointers now that check_relocs is running after gc_sections (and probably x86 also doesn't need ref counting nowadays).  It's quite a simple patch, making use of the fact that needs_plt is set on powerpc only when encountering branch relocs or relocs that explicitly create a plt entry.  So if needs_plt is not set I can do without a plt entry if there are no dynamic relocs in read-only sections.
Comment 3 Sourceware Commits 2017-11-01 08:19:05 UTC
The master branch has been updated by Alan Modra <amodra@sourceware.org>:

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=e48f0c8f1b9fdb195394dc7afea02ce55e4ba5e4

commit e48f0c8f1b9fdb195394dc7afea02ce55e4ba5e4
Author: Alan Modra <amodra@gmail.com>
Date:   Tue Oct 31 11:13:30 2017 +1030

    PR22374 testcase, function pointer references in .data
    
    Function pointer references in .data ought to use a dynamic reloc.
    There shouldn't be any need for a PLT entry and definitely no copy
    reloc.
    
    This test fails on quite a few targets, but isn't something that
    anyone should worry about too much.  It's really just a missed
    optimization.
    
    	PR 22374
    	* testsuite/ld-elf/pr22374a.s,
    	* testsuite/ld-elf/pr22374b.s,
    	* testsuite/ld-elf/pr22374-1.r,
    	* testsuite/ld-elf/pr22374-2.r: New test.
    	* testsuite/ld-elf/elf.exp: Run it.
Comment 4 Sourceware Commits 2017-11-01 08:19:10 UTC
The master branch has been updated by Alan Modra <amodra@sourceware.org>:

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=3988aed54acce3c682a877b51b0e09cce1079e81

commit 3988aed54acce3c682a877b51b0e09cce1079e81
Author: Alan Modra <amodra@gmail.com>
Date:   Tue Oct 31 22:13:21 2017 +1030

    PR22374, PowerPC unnecessary PLT entries
    
    We don't need a PLT entry when function pointer initialization in a
    read/write section is the only reference to a given function symbol.
    This patch prevents the unnecessary PLT entry, and ensures no dynamic
    relocs are emitted when UNDEFWEAK_NO_DYNAMIC_RELOC says so.
    
    bfd/
    	PR 22374
    	* elf32-ppc.c (ppc_elf_adjust_dynamic_symbol): Don't create a plt
    	entry when just a dynamic reloc can serve.  Ensure no dynamic
    	relocations when UNDEFWEAK_NO_DYNAMIC_RELOC by setting non_got_ref.
    	Expand and move the non_got_ref comment.
    	* elf64-ppc.c (ppc64_elf_adjust_dynamic_symbol): Likewise.
    ld/
    	* testsuite/ld-powerpc/ambiguousv2.d: Remove FIXME.
Comment 5 H.J. Lu 2017-11-01 16:33:04 UTC
(In reply to Alan Modra from comment #2)
> Thanks HJ.  The powerpc patch I'm testing doesn't need reference counting of
> function pointers now that check_relocs is running after gc_sections (and
> probably x86 also doesn't need ref counting nowadays).  It's quite a simple

You are right. I am preparing a patch.
Comment 6 Alan Modra 2018-04-11 13:04:12 UTC
All pr22374 FAILs converted to XFAILs.
Comment 7 Sourceware Commits 2018-04-27 09:26:45 UTC
The master branch has been updated by Alan Modra <amodra@sourceware.org>:

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=04383fd15b3b82d824df9c72e3ade88c43bfb5ac

commit 04383fd15b3b82d824df9c72e3ade88c43bfb5ac
Author: Alan Modra <amodra@gmail.com>
Date:   Fri Apr 27 15:46:40 2018 +0930

    PR23123, PowerPC32 ifunc regression
    
    Two of the gcc ifunc tests fail for ppc32, due to my pr22374 fix being
    a little too enthusiastic in trimming PLT entries.  ppc64 doesn't have
    the same failures because ppc64_elf_check_relocs happens to set
    needs_plt for any ifunc reloc.
    
    	PR 23123
    	PR 22374
    	* elf32-ppc.c (ppc_elf_adjust_dynamic_symbol): Don't drop plt
    	relocs for ifuncs.
    	* elf64-ppc.c (ppc64_elf_adjust_dynamic_symbol): Comment fixes.
Comment 8 Sourceware Commits 2018-04-27 09:33:16 UTC
The binutils-2_30-branch branch has been updated by Alan Modra <amodra@sourceware.org>:

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=69bc498952e7e477d6f96f3b6f900a6112e82350

commit 69bc498952e7e477d6f96f3b6f900a6112e82350
Author: Alan Modra <amodra@gmail.com>
Date:   Fri Apr 27 15:46:40 2018 +0930

    PR23123, PowerPC32 ifunc regression
    
    Two of the gcc ifunc tests fail for ppc32, due to my pr22374 fix being
    a little too enthusiastic in trimming PLT entries.  ppc64 doesn't have
    the same failures because ppc64_elf_check_relocs happens to set
    needs_plt for any ifunc reloc.
    
    	PR 23123
    	PR 22374
    	* elf32-ppc.c (ppc_elf_adjust_dynamic_symbol): Don't drop plt
    	relocs for ifuncs.
    	* elf64-ppc.c (ppc64_elf_adjust_dynamic_symbol): Comment fixes.
    
    (cherry picked from commit 04383fd15b3b82d824df9c72e3ade88c43bfb5ac)