Bug 25056

Summary: ld fails to bind DTPMOD at link time for pie on arm
Product: binutils Reporter: Rich Felker <bugdal>
Component: ldAssignee: Not yet assigned to anyone <unassigned>
Status: RESOLVED FIXED    
Severity: normal CC: amodra, nsz
Priority: P2    
Version: unspecified   
Target Milestone: 2.34   
Host: Target:
Build: Last reconfirmed: 2019-10-03 00:00:00
Attachments: patch to fix

Description Rich Felker 2019-10-02 14:54:44 UTC
Created attachment 12015 [details]
patch to fix

Identical to #22570 for mips.
Comment 1 Rich Felker 2019-10-02 15:02:07 UTC
Link for convenience: https://sourceware.org/bugzilla/show_bug.cgi?id=22570

I suspect there may be other targets with exactly the same issue. There should probably be a generic test for all targets that asserts no DTPMOD type relocations in static PIE output (also, none for locally defined TLS symbols in regular dynamic PIE output).
Comment 2 Alan Modra 2019-10-03 00:59:14 UTC
(In reply to Rich Felker from comment #1)
> I suspect there may be other targets with exactly the same issue.

I know there are..  Note that this is not a correctness issue unless the unnecessary dynamic relocation is wrong or is misapplied by ld.so.

Note also that I suspect your patch changes one unnecessary dynamic relocation for another.  You'll be getting R_ARM_NONE I expect.  To fix that you should adjust the count of dynamic relocs too.
Comment 3 Rich Felker 2019-10-03 01:37:00 UTC
It is a correctness issue for static linking, where my intent (as the one who introduced static pie) was that presence of any unresolved relocations except load-address-relative ones is an error. If you think that's a bad policy and that dtpmod relocations should be accepted (filled in as "1" in self-relocation process), it'd be mildly disappointing but something we could move forward with.
Comment 4 Alan Modra 2019-10-03 03:13:59 UTC
I hadn't considered static pies.  Yes, I agree that it is a correctness issue there.
Comment 5 Alan Modra 2019-10-03 03:15:48 UTC
BTW, I've been working through fixing this for powerpc64.
Comment 6 Sourceware Commits 2019-10-04 09:58:42 UTC
The master branch has been updated by Szabolcs Nagy <nsz@sourceware.org>:

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

commit 9cb09e33e04feb12df2aaa6e81d61b82ad609ce5
Author: Szabolcs Nagy <szabolcs.nagy@arm.com>
Date:   Wed Oct 2 19:46:46 2019 +0100

    [PR ld/22263][PR ld/25056] arm: Avoid dynamic TLS relocs in PIE
    
    Dynamic relocs are only needed in an executable for TLS symbols if
    those are defined in an external module and even then TLS access
    can be relaxed to use IE model instead of GD.
    
    Several bfd_link_pic checks are turned into bfd_link_dll checks
    to fix TLS handling in PIE, for the same fix some other targets
    used !bfd_link_executable checks, but that includes relocatable
    objects so dll seems safer (in most cases either should work, since
    dynamic relocations are not applied in relocatable objects).
    
    On arm* fixes
    FAIL: Build pr22263-1
    
    bfd/
    
    	PR ld/22263
    	PR ld/25056
    	* elf32-arm.c (elf32_arm_tls_transition): Use bfd_link_dll instead of
    	bfd_link_pic for TLS checks.
    	(elf32_arm_final_link_relocate): Likewise.
    	(allocate_dynrelocs_for_symbol): Likewise.
Comment 7 Sourceware Commits 2019-10-10 11:29:10 UTC
The binutils-2_33-branch branch has been updated by Szabolcs Nagy <nsz@sourceware.org>:

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

commit b094948c0943c996460cbc9ab3c14207dc520445
Author: Szabolcs Nagy <szabolcs.nagy@arm.com>
Date:   Wed Oct 2 19:46:46 2019 +0100

    [PR ld/22263][PR ld/25056] arm: Avoid dynamic TLS relocs in PIE
    
    Dynamic relocs are only needed in an executable for TLS symbols if
    those are defined in an external module and even then TLS access
    can be relaxed to use IE model instead of GD.
    
    Several bfd_link_pic checks are turned into bfd_link_dll checks
    to fix TLS handling in PIE, for the same fix some other targets
    used !bfd_link_executable checks, but that includes relocatable
    objects so dll seems safer (in most cases either should work, since
    dynamic relocations are not applied in relocatable objects).
    
    On arm* fixes
    FAIL: Build pr22263-1
    
    bfd/
    
    	PR ld/22263
    	PR ld/25056
    	* elf32-arm.c (elf32_arm_tls_transition): Use bfd_link_dll instead of
    	bfd_link_pic for TLS checks.
    	(elf32_arm_final_link_relocate): Likewise.
    	(allocate_dynrelocs_for_symbol): Likewise.
Comment 8 Szabolcs Nagy 2019-10-11 10:55:22 UTC
fixed for 2.34 and on the 2.33 branch.