[ELF/AArch64] Fix correct TCB aligment calculation

[zatrazz]

Summary

This patch fixes the TLS local relocations alignment done by @238258.
As pointed out, the TLS size should not be considered, but rather the
TCB size based on maximum output segment alignment. Although it has
not shown in the TLS simple cases for test-suite, more comprehensible
tests with more local TLS variable showed wrong relocations values
being generated.

The local TLS testcase is expanded to add more tls variable (both
exported and static) initialized or not.

[ruiu]

LGTM

lib/ReaderWriter/ELF/AArch64/AArch64TargetHandler.h

29

I'd invert the condition to return early.

30

Could you use the actual type instead of auto because the type is not obvious?

[shankarke]

lib/ReaderWriter/ELF/AArch64/AArch64TargetHandler.h

31–35

This is wrong. You shouldnt be using maximum alignment of all sections in the output.

[zatrazz]

Address review comments:

• Changed 'auto' for the explicit type name
• Consider just TLS segments for maximum alignment.

0001-Fix-correct-TCB-aligment-calculation.patch36 KBDownload

lib/ReaderWriter/ELF/AArch64/AArch64RelocationHandler.cpp

case ADD_AARCH64_GOTRELINDEX:
     break;
   case R_AARCH64_TLSIE_ADR_GOTTPREL_PAGE21:
     return relocR_AARCH64_TLSIE_ADR_GOTTPREL_PAGE21(loc, reloc, target, addend);
   case R_AARCH64_TLSIE_LD64_GOTTPREL_LO12_NC:
     relocR_AARCH64_TLSIE_LD64_GOTTPREL_LO12_NC(loc, reloc, target, addend);
     break;
   case R_AARCH64_TLSLE_ADD_TPREL_HI12:
   case R_AARCH64_TLSLE_ADD_TPREL_LO12_NC: {
-    _tlsSize = _layout.getAlignedTLSSize();
+    _tlsSize = _layout.getAlignedTCBSize();
     if (ref.kindValue() == R_AARCH64_TLSLE_ADD_TPREL_HI12)
       return relocR_AARCH64_TLSLE_ADD_TPREL_HI12(loc, reloc, target + _tlsSize,
                                                  addend);
     else 
       relocR_AARCH64_TLSLE_ADD_TPREL_LO12_NC(loc, reloc, target + _tlsSize,
                                              addend);
   }  break;
   default:
     return make_unhandled_reloc_error();
   }
   return std::error_code();
 }

lib/ReaderWriter/ELF/AArch64/AArch64TargetHandler.h

 #include "TargetLayout.h"
 #include "lld/Core/Simple.h"
 
 namespace lld {
 namespace elf {
 class AArch64LinkingContext;
 
 class AArch64TargetLayout final : public TargetLayout<ELF64LE> {
+  typedef llvm::object::Elf_Shdr_Impl<ELF64LE> Elf_Shdr;
+
 public:
   AArch64TargetLayout(ELFLinkingContext &ctx) : TargetLayout(ctx) {}
 
-  uint64_t getAlignedTLSSize() const {
-    return llvm::RoundUpToAlignment(TCB_ALIGNMENT, this->getTLSSize());
+  uint64_t maxAlignment() {
+    if (_maxAlignment == 0) {
+      auto sections = this->sections();
+      auto ret = std::max_element(sections.begin(), sections.end(),
+        [](Chunk<ELF64LE> *x, Chunk<ELF64LE> *y)
+          { return x->alignment() < y->alignment(); });
+      _maxAlignment = (*ret)->alignment();
+    }
+    return _maxAlignment;
+  }
+
+  uint64_t getAlignedTCBSize() {
+    return llvm::RoundUpToAlignment(TCB_SIZE, maxAlignment());
   }
 
 private:
   // Alignment requirement for TCB.
-  enum { TCB_ALIGNMENT = 0x10 };
+  enum {
+    TCB_SIZE = 16,
+  };
+  uint64_t _maxAlignment = 0;
 };
 
 class AArch64TargetHandler final : public TargetHandler {
 public:
   AArch64TargetHandler(AArch64LinkingContext &ctx);
 
   const TargetRelocationHandler &getRelocationHandler() const override {
     return *_relocationHandler;

test/elf/AArch64/local-tls.test

 # Check for correct offsets when handling relocations for local TLS
 # access (R_AARCH64_TLSLE_ADD_TPREL_HI12, R_AARCH64_TLSLE_ADD_TPREL_LO12_NC)
 # RUN: yaml2obj -format=elf %s > %t-obj
 # RUN: lld -flavor gnu -target arm64 --noinhibit-exec -o %t-exe %t-obj
 # RUN: llvm-objdump -s -t %t-exe | FileCheck %s
 
 # CHECK: Contents of section .text:
-# CHECK-NEXT: 4001dc 48d03bd5 08010091 08410091 090140b9  H.;......A....@.
-#                              \_      |        add  x8, x8, #0x0   (R_AARCH64_TLSLE_ADD_TPREL_HI12)
-#                                      \_       add  x8, x8, #0x10  (R_AARCH64_TLSLE_ADD_TPREL_LO12_NC)
+# CHECK-NEXT: 4001dc 48d03bd5 08010091 08610091 090140b9  H.;......a....@.
+#                     \_       |       |                  <foo1>:
+#                              \_      |                  add  x8, x8, #0x0   (R_AARCH64_TLSLE_ADD_TPREL_HI12)
+#                                      \_                 add  x8, x8, #0x18  (R_AARCH64_TLSLE_ADD_TPREL_LO12_NC)
+# CHECK-NEXT: 4001ec e003092a c0035fd6 48d03bd5 08010091  ...*.._.H.;.....
+#                                      \_       |         <foo2>:
+#                                               \_        add  x8, x8, #0x0   (R_AARCH64_TLSLE_ADD_TPREL_HI12)
+# CHECK-NEXT: 4001fc 08710091 090140b9 e003092a c0035fd6  .q....@....*.._.
+#                    \_                                   add  x8, x8, #0x1c  (R_AARCH64_TLSLE_ADD_TPREL_LO12_NC)
+# CHECK-NEXT: 40020c 48d03bd5 08010091 08410091 090140b9  H.;......A....@.
+#                    \_       |        |                  <foo3>:
+#                             \_       |                  add  x8, x8, #0x0   (R_AARCH64_TLSLE_ADD_TPREL_HI12)
+#                                      \_                 add  x8, x8, #0x10  (R_AARCH64_TLSLE_ADD_TPREL_LO12_NC)
+# CHECK-NEXT: 40021c e003092a c0035fd6 48d03bd5 08010091  ...*.._.H.;.....
+#                                      \_       |         <foo3>:
+#                                               \_        add  x8, x8, #0x0   (R_AARCH64_TLSLE_ADD_TPREL_HI12)
+# CHECK-NEXT: 40022c 08510091 090140b9 e003092a c0035fd6  .Q....@....*.._.
+#                    \_                                   add  x8, x8, #0x14  (R_AARCH64_TLSLE_ADD_TPREL_LO12_NC)
 
 ---
 FileHeader:      
   Class:           ELFCLASS64
   Data:            ELFDATA2LSB
   OSABI:           ELFOSABI_GNU
   Type:            ET_REL
   Machine:         EM_AARCH64
 Sections:        
   - Name:            .text
     Type:            SHT_PROGBITS
     Flags:           [ SHF_ALLOC, SHF_EXECINSTR ]
     AddressAlign:    0x0000000000000004
-    Content:         48D03BD50801009108010091090140B9E003092AC0035FD6FD7BBFA9FD030091FF4300D1BFC31FB8F6FFFF97BF030091FD7BC1A8C0035FD6
+    Content:         48D03BD50801009108010091090140B9E003092AC0035FD648D03BD50801009108010091090140B9E003092AC0035FD648D03BD50801009108010091090140B9E003092AC0035FD648D03BD50801009108010091090140B9E003092AC0035FD6FD7BBFA9FD030091FF8300D1BFC31FB8E4FFFF97A0831FB8E8FFFF97A0431FB8ECFFFF97E01300B9F0FFFF970800009008010091E00F00B9E00308AAA1835FB8A2435FB8E31340B9E40F40B900000094E1031F2AE00B00B9E003012ABF030091FD7BC1A8C0035FD6
   - Name:            .rela.text
     Type:            SHT_RELA
     Link:            .symtab
     AddressAlign:    0x0000000000000008
     Info:            .text
     Relocations:     
       - Offset:          0x0000000000000004
-        Symbol:          a
+        Symbol:          v1
         Type:            R_AARCH64_TLSLE_ADD_TPREL_HI12
       - Offset:          0x0000000000000008
-        Symbol:          a
+        Symbol:          v1
+        Type:            R_AARCH64_TLSLE_ADD_TPREL_LO12_NC
+      - Offset:          0x000000000000001C
+        Symbol:          v2
+        Type:            R_AARCH64_TLSLE_ADD_TPREL_HI12
+      - Offset:          0x0000000000000020
+        Symbol:          v2
+        Type:            R_AARCH64_TLSLE_ADD_TPREL_LO12_NC
+      - Offset:          0x0000000000000034
+        Symbol:          v3
+        Type:            R_AARCH64_TLSLE_ADD_TPREL_HI12
+      - Offset:          0x0000000000000038
+        Symbol:          v3
         Type:            R_AARCH64_TLSLE_ADD_TPREL_LO12_NC
+      - Offset:          0x000000000000004C
+        Symbol:          v4
+        Type:            R_AARCH64_TLSLE_ADD_TPREL_HI12
+      - Offset:          0x0000000000000050
+        Symbol:          v4
+        Type:            R_AARCH64_TLSLE_ADD_TPREL_LO12_NC
+      - Offset:          0x000000000000008C
+        Symbol:          .rodata.str1.1
+        Type:            R_AARCH64_ADR_PREL_PG_HI21
+      - Offset:          0x0000000000000090
+        Symbol:          .rodata.str1.1
+        Type:            R_AARCH64_ADD_ABS_LO12_NC
+      - Offset:          0x00000000000000AC
+        Symbol:          printf
+        Type:            R_AARCH64_CALL26
   - Name:            .data
     Type:            SHT_PROGBITS
     Flags:           [ SHF_WRITE, SHF_ALLOC ]
     AddressAlign:    0x0000000000000004
     Content:         ''
   - Name:            .bss
     Type:            SHT_NOBITS
     Flags:           [ SHF_WRITE, SHF_ALLOC ]
     AddressAlign:    0x0000000000000004
     Content:         ''
   - Name:            .tdata
     Type:            SHT_PROGBITS
     Flags:           [ SHF_WRITE, SHF_ALLOC, SHF_TLS ]
     AddressAlign:    0x0000000000000004
-    Content:         '04000000'
-  - Name:            .comment
+    Content:         '0300000004000000'
+  - Name:            .tbss
+    Type:            SHT_NOBITS
+    Flags:           [ SHF_WRITE, SHF_ALLOC, SHF_TLS ]
+    AddressAlign:    0x0000000000000004
+    Content:         '2569202569202569'
+  - Name:            .rodata.str1.1
     Type:            SHT_PROGBITS
-    Flags:           [ SHF_MERGE, SHF_STRINGS ]
+    Flags:           [ SHF_ALLOC, SHF_MERGE, SHF_STRINGS ]
     AddressAlign:    0x0000000000000001
-    Content:         00636C616E672076657273696F6E20332E372E302028687474703A2F2F6C6C766D2E6F72672F6769742F636C616E672E6769742039636664333732343263346635623866393362636462343536366163636565373962306563653666292028687474703A2F2F6C6C766D2E6F72672F6769742F6C6C766D2E67697420653262383764656531656531393835396561383334313565616365333533666130323362646462302900
+    Content:         25692025692025692025690A00
   - Name:            .note.GNU-stack
     Type:            SHT_PROGBITS
     AddressAlign:    0x0000000000000001
     Content:         ''
 Symbols:         
   Local:           
     - Name:            '$d.1'
       Type:            STT_TLS
       Section:         .tdata
     - Name:            '$d.2'
-      Section:         .comment
+      Type:            STT_TLS
+      Section:         .tbss
+    - Name:            '$d.3'
+      Section:         .rodata.str1.1
     - Name:            '$x.0'
       Section:         .text
+    - Name:            v2
+      Type:            STT_TLS
+      Section:         .tbss
+      Value:           0x0000000000000004
+      Size:            0x0000000000000004
+    - Name:            v4
+      Type:            STT_TLS
+      Section:         .tdata
+      Value:           0x0000000000000004
+      Size:            0x0000000000000004
     - Name:            .text
       Type:            STT_SECTION
       Section:         .text
     - Name:            .data
       Type:            STT_SECTION
       Section:         .data
     - Name:            .bss
       Type:            STT_SECTION
       Section:         .bss
     - Name:            .tdata
       Type:            STT_SECTION
       Section:         .tdata
-    - Name:            .comment
+    - Name:            .tbss
       Type:            STT_SECTION
-      Section:         .comment
+      Section:         .tbss
+    - Name:            .rodata.str1.1
+      Type:            STT_SECTION
+      Section:         .rodata.str1.1
     - Name:            .note.GNU-stack
       Type:            STT_SECTION
       Section:         .note.GNU-stack
   Global:          
-    - Name:            a
-      Type:            STT_TLS
-      Section:         .tdata
-      Size:            0x0000000000000004
-    - Name:            foo
+    - Name:            foo1
       Type:            STT_FUNC
       Section:         .text
       Size:            0x0000000000000018
-    - Name:            main
+    - Name:            foo2
       Type:            STT_FUNC
       Section:         .text
       Value:           0x0000000000000018
-      Size:            0x0000000000000020
+      Size:            0x0000000000000018
+    - Name:            foo3
+      Type:            STT_FUNC
+      Section:         .text
+      Value:           0x0000000000000030
+      Size:            0x0000000000000018
+    - Name:            foo4
+      Type:            STT_FUNC
+      Section:         .text
+      Value:           0x0000000000000048
+      Size:            0x0000000000000018
+    - Name:            main
+      Type:            STT_FUNC
+      Section:         .text
+      Value:           0x0000000000000060
+      Size:            0x0000000000000068
+    - Name:            v1
+      Type:            STT_TLS
+      Section:         .tbss
+      Size:            0x0000000000000004
+    - Name:            v3
+      Type:            STT_TLS
+      Section:         .tdata
+      Size:            0x0000000000000004
+    - Name:            printf
 ...