Bug 24601 - aarch64: local-exec TPREL relocations to weak undefined symbols -> assertion fail
Summary: aarch64: local-exec TPREL relocations to weak undefined symbols -> assertion ...
Status: RESOLVED FIXED
Alias: None
Product: binutils
Classification: Unclassified
Component: ld (show other bugs)
Version: unspecified
: P2 normal
Target Milestone: 2.32
Assignee: Tamar Christina
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2019-05-22 14:46 UTC by Fangrui Song
Modified: 2019-08-28 08:48 UTC (History)
1 user (show)

See Also:
Host:
Target:
Build:
Last reconfirmed: 2019-06-03 00:00:00


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Fangrui Song 2019-05-22 14:46:04 UTC
% cat a.c
__attribute__((weak,tls_model("local-exec")))
extern __thread char tls;
char *get() { return &tls; }

% clang -target aarch64 -c a.c
% readelf -r a.o
Relocation section '.rela.text' at offset 0x158 contains 2 entries:
    Offset             Info             Type               Symbol's Value  Symbol's Name + Addend
0000000000000004  0000000500000225 R_AARCH64_TLSLE_ADD_TPREL_HI12 0000000000000000 tls + 0
0000000000000008  0000000500000227 R_AARCH64_TLSLE_ADD_TPREL_LO12_NC 0000000000000000 tls + 0

% ./ld-new a.o
./ld-new: warning: cannot find entry symbol _start; defaulting to 00000000004000b0
./ld-new: BFD (GNU Binutils) 2.32.51.20190522 assertion fail ../../bfd/elfnn-aarch64.c:5059
[1]    153266 segmentation fault  ./ld-new a.o

static bfd_vma
tpoff_base (struct bfd_link_info *info)
{
  struct elf_link_hash_table *htab = elf_hash_table (info);

  /* If tls_sec is NULL, we should have signalled an error already.  */
  BFD_ASSERT (htab->tls_sec != NULL);

  bfd_vma base = align_power ((bfd_vma) TCB_SIZE,
			      htab->tls_sec->alignment_power);
  return htab->tls_sec->vma - base;
}


BTW, the formula may be incorrect if p_vaddr%p_align != 0 (the offset should be chosen so that offset%p_align = p_vaddr%p_align, but that scenario may be unrealistic.
Comment 1 Tamar Christina 2019-06-03 11:47:46 UTC
Confirmed, I'll take a look. Thanks for the report!
Comment 2 Sourceware Commits 2019-08-22 10:47:03 UTC
The master branch has been updated by Tamar Christina <tnfchris@sourceware.org>:

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

commit 652afeef247770b22c44ca292d1f4c65be40a696
Author: Tamar Christina <tamar.christina@arm.com>
Date:   Thu Aug 22 11:35:35 2019 +0100

    AArch64: Fix LD crash on weak and undefined TLS symbols. (PR/24602).
    
    This patch fixes a few linker crashes due to TLS code reaching an assert when it
    shouldn't.
    
    The first scenario is with weak TLS symbols that remain weak during linking.  In
    this case the mid-end would not have seen a TLS symbol and so wouldn't have
    allocated the TLS section.  We currently assert here and the linker crashes with
    a not very useful message.
    
    This patch changes this to return the value 0 for the TLS symbol in question
    emulating what lld and gold and other BFD targets do.  However because weak TLS
    is implementation defined and we don't define any behavior for it I also emit a
    warning to the user to inform them of such.
    
    Secondly when a strong TLS reference is undefined. The linker crashes even after
    it correctly reported that there is an undefined reference.  This changes it so
    that it gracefully exits and reports a useful error.
    
    bfd/ChangeLog:
    
    	PR ld/24601
    	* elfnn-aarch64.c (aarch64_relocate): Handle weak TLS and undefined TLS.
    	Also Pass input_bfd to _bfd_aarch64_elf_resolve_relocation.
    	* elfxx-aarch64.c (_bfd_aarch64_elf_resolve_relocation): Use it.
    	* elfxx-aarch64.h (_bfd_aarch64_elf_resolve_relocation): Emit warning
    	for weak TLS.
    
    ld/ChangeLog:
    
    	PR ld/24601
    	* testsuite/ld-aarch64/aarch64-elf.exp (undef-tls, weak-tls): New.
    	* testsuite/ld-aarch64/undef-tls.d: New test.
    	* testsuite/ld-aarch64/undef-tls.s: New test.
    	* testsuite/ld-aarch64/weak-tls.d: New test.
    	* testsuite/ld-aarch64/weak-tls.s: New test.
Comment 3 Sourceware Commits 2019-08-28 08:46:41 UTC
The binutils-2_32-branch branch has been updated by Tamar Christina <tnfchris@sourceware.org>:

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

commit 6795dba28d531a17a0800d532d555653415702bb
Author: Tamar Christina <tamar.christina@arm.com>
Date:   Thu Aug 22 11:35:35 2019 +0100

    AArch64: Fix LD crash on weak and undefined TLS symbols. (PR/24602).
    
    This patch fixes a few linker crashes due to TLS code reaching an assert when it
    shouldn't.
    
    The first scenario is with weak TLS symbols that remain weak during linking.  In
    this case the mid-end would not have seen a TLS symbol and so wouldn't have
    allocated the TLS section.  We currently assert here and the linker crashes with
    a not very useful message.
    
    This patch changes this to return the value 0 for the TLS symbol in question
    emulating what lld and gold and other BFD targets do.  However because weak TLS
    is implementation defined and we don't define any behavior for it I also emit a
    warning to the user to inform them of such.
    
    Secondly when a strong TLS reference is undefined. The linker crashes even after
    it correctly reported that there is an undefined reference.  This changes it so
    that it gracefully exits and reports a useful error.
    
    bfd/ChangeLog:
    
    	PR ld/24601
    	* elfnn-aarch64.c (aarch64_relocate): Handle weak TLS and undefined TLS.
    	Also Pass input_bfd to _bfd_aarch64_elf_resolve_relocation.
    	* elfxx-aarch64.c (_bfd_aarch64_elf_resolve_relocation): Use it.
    	* elfxx-aarch64.h (_bfd_aarch64_elf_resolve_relocation): Emit warning
    	for weak TLS.
    
    ld/ChangeLog:
    
    	PR ld/24601
    	* testsuite/ld-aarch64/aarch64-elf.exp (undef-tls, weak-tls): New.
    	* testsuite/ld-aarch64/undef-tls.d: New test.
    	* testsuite/ld-aarch64/undef-tls.s: New test.
    	* testsuite/ld-aarch64/weak-tls.d: New test.
    	* testsuite/ld-aarch64/weak-tls.s: New test.
    
    (cherry picked from commit 652afeef247770b22c44ca292d1f4c65be40a696)
Comment 4 Tamar Christina 2019-08-28 08:48:47 UTC
Fixed and backported to 2.32 branch.