[LLD] Set alignment as part of Characteristics in TLS table.

[luqmana]

Summary

Fixes https://bugs.llvm.org/show_bug.cgi?id=46473

LLD wasn't previously specifying any specific alignment in the TLS table's Characteristics field so the loader would just assume the default value (16 bytes). This works most of the time except if you have thread locals that want specific higher alignments (e.g. 32 as in the bug) *even* if they specify an alignment on the thread local. This change updates LLD to take the max alignment from tls section.

[luqmana]

Update.

[luqmana]

Fix style warnings.

[luqmana]

Update tests.

[luqmana]

Ping?

[rnk]

Thanks for the fix!

llvm/include/llvm/Object/COFF.h

592

If there is already some non-zero alignment, you would have to mask it out, right? Maybe structure this as:

• build the new alignment characteristic bits, 0 or some value
• load, mask, or in, and store them in one operation

As this setter is currently used, this bug is probably not observable, but it's best to be safe.

[luqmana]

Mask out existing alignment bits.

[rnk]

Looks good, suggestions optional.

lld/COFF/Writer.cpp

2054–2055

I have some optional nitty suggestions here. You could do something like this to return early and reduce indentation:

Defined *b = dyn_cast_or_null<Defined>(symtab->findUnderscore("_tls_used"));
if (!b)
  return;
...

Also, I'm not sure b is the most descriptive name. sym, tlsSym?

[luqmana]

Address nit: Use early return to reduce indentation.

[luqmana]

Accidentally messed up the last diff there. Update revision with final code change that was committed for visibility.

lld/COFF/Writer.cpp

private:
   void writeSections();
   void writeBuildId();
   void sortSections();
   void sortExceptionTable();
   void sortCRTSectionChunks(std::vector<Chunk *> &chunks);
   void addSyntheticIdata();
   void fixPartialSectionChars(StringRef name, uint32_t chars);
   bool fixGnuImportChunks();
+  void fixTlsAlignment();
   PartialSection *createPartialSection(StringRef name, uint32_t outChars);
   PartialSection *findPartialSection(StringRef name, uint32_t outChars);
 
   llvm::Optional<coff_symbol16> createSymbol(Defined *d);
   size_t addEntryToStringTable(StringRef str);
 
   OutputSection *findSection(StringRef name);
   void addBaserels();
private:
   uint64_t importTableSize = 0;
   Chunk *edataStart = nullptr;
   Chunk *edataEnd = nullptr;
   Chunk *iatStart = nullptr;
   uint64_t iatSize = 0;
   DelayLoadContents delayIdata;
   EdataContents edata;
   bool setNoSEHCharacteristic = false;
+  uint32_t tlsAlignment = 0;
 
   DebugDirectoryChunk *debugDirectory = nullptr;
   std::vector<std::pair<COFF::DebugType, Chunk *>> debugRecords;
   CVDebugRecordChunk *buildId = nullptr;
   ArrayRef<uint8_t> sectionTable;
 
   uint64_t fileSize;
   uint32_t pointerToSymbolTable = 0;
void Writer::run() {
   if (config->is64()) {
     writeHeader<pe32plus_header>();
   } else {
     writeHeader<pe32_header>();
   }
   writeSections();
   sortExceptionTable();
 
+  // Fix up the alignment in the TLS Directory's characteristic field,
+  // if a specific alignment value is needed
+  if (tlsAlignment)
+    fixTlsAlignment();
+
   t1.stop();
 
   if (!config->pdbPath.empty() && config->debug) {
     assert(buildId);
     createPDB(symtab, outputSections, sectionTable, buildId->buildId);
   }
   writeBuildId();
 
for (Chunk *c : symtab->getChunks()) {
     if (sc && !sc->live) {
       if (config->verbose)
         sc->printDiscardedMessage();
       continue;
     }
     StringRef name = c->getSectionName();
     if (shouldStripSectionSuffix(sc, name))
       name = name.split('$').first;
+
+    if (name.startswith(".tls"))
+      tlsAlignment = std::max(tlsAlignment, c->getAlignment());
+
     PartialSection *pSec = createPartialSection(name,
                                                 c->getOutputCharacteristics());
     pSec->chunks.push_back(c);
   }
 
   fixPartialSectionChars(".rsrc", data | r);
   fixPartialSectionChars(".edata", data | r);
   // Even in non MinGW cases, we might need to link against GNU import

 }
 
 PartialSection *Writer::findPartialSection(StringRef name, uint32_t outChars) {
   auto it = partialSections.find({name, outChars});
   if (it != partialSections.end())
     return it->second;
   return nullptr;
 }
+
+void Writer::fixTlsAlignment() {
+  Defined *tlsSym =
+      dyn_cast_or_null<Defined>(symtab->findUnderscore("_tls_used"));
+  if (!tlsSym)
+    return;
+
+  OutputSection *sec = tlsSym->getChunk()->getOutputSection();
+  assert(sec && tlsSym->getRVA() >= sec->getRVA() &&
+         "no output section for _tls_used");
+
+  uint8_t *secBuf = buffer->getBufferStart() + sec->getFileOff();
+  uint64_t tlsOffset = tlsSym->getRVA() - sec->getRVA();
+  uint64_t directorySize = config->is64()
+                               ? sizeof(object::coff_tls_directory64)
+                               : sizeof(object::coff_tls_directory32);
+
+  if (tlsOffset + directorySize > sec->getRawSize())
+    fatal("_tls_used sym is malformed");
+
+  if (config->is64()) {
+    object::coff_tls_directory64 *tlsDir =
+        reinterpret_cast<object::coff_tls_directory64 *>(&secBuf[tlsOffset]);
+    tlsDir->setAlignment(tlsAlignment);
+  } else {
+    object::coff_tls_directory32 *tlsDir =
+        reinterpret_cast<object::coff_tls_directory32 *>(&secBuf[tlsOffset]);
+    tlsDir->setAlignment(tlsAlignment);
+  }
+}

lld/test/COFF/tls-alignment-32.ll

 ; REQUIRES: x86
 
 ; This test is to make sure that the necessary alignment for thread locals
 ; gets reflected in the TLS Directory of the generated executable on x86.
 ;
 ; aligned_thread_local specifies 'align 32' and so the generated
 ; exe should reflect that with a value of IMAGE_SCN_ALIGN_32BYTES
 ; in the Characteristics field of the IMAGE_TLS_DIRECTORY
 
 ; RUN: llc -filetype=obj %S/Inputs/tlssup-32.ll -o %t.tlssup.obj
 ; RUN: llc -filetype=obj %s -o %t.obj
 ; RUN: lld-link %t.tlssup.obj %t.obj -entry:main -nodefaultlib -out:%t.exe
 ; RUN: llvm-readobj --coff-tls-directory %t.exe | FileCheck %s
 
 ; CHECK: TLSDirectory {
-; CHECK: Characteristics [ (0x0)
+; CHECK: Characteristics [ (0x600000)
+; CHECK-NEXT: IMAGE_SCN_ALIGN_32BYTES (0x600000)
 
 target triple = "i686-pc-windows-msvc"
 
 @aligned_thread_local = thread_local global i32 42, align 32
 
 define i32 @main() {
   %t = load i32, i32* @aligned_thread_local
   ret i32 %t
 }

lld/test/COFF/tls-alignment-64.ll

 ; REQUIRES: x86
 
 ; This test is to make sure that the necessary alignment for thread locals
 ; gets reflected in the TLS Directory of the generated executable on x86-64.
 ;
 ; aligned_thread_local specifies 'align 64' and so the generated
 ; exe should reflect that with a value of IMAGE_SCN_ALIGN_64BYTES
 ; in the Characteristics field of the IMAGE_TLS_DIRECTORY
 
 ; RUN: llc -filetype=obj %S/Inputs/tlssup-64.ll -o %t.tlssup.obj
 ; RUN: llc -filetype=obj %s -o %t.obj
 ; RUN: lld-link %t.tlssup.obj %t.obj -entry:main -nodefaultlib -out:%t.exe
 ; RUN: llvm-readobj --coff-tls-directory %t.exe | FileCheck %s
 
 ; CHECK: TLSDirectory {
-; CHECK: Characteristics [ (0x0)
+; CHECK: Characteristics [ (0x700000)
+; CHECK-NEXT: IMAGE_SCN_ALIGN_64BYTES (0x700000)
 
 target triple = "x86_64-pc-windows-msvc"
 
 @aligned_thread_local = thread_local global i32 42, align 64
 
 define i32 @main() {
   %t = load i32, i32* @aligned_thread_local
   ret i32 %t
 }

llvm/include/llvm/Object/COFF.h

struct coff_tls_directory {
   IntTy EndAddressOfRawData;
   IntTy AddressOfIndex;
   IntTy AddressOfCallBacks;
   support::ulittle32_t SizeOfZeroFill;
   support::ulittle32_t Characteristics;
 
   uint32_t getAlignment() const {
     // Bit [20:24] contains section alignment.
-    uint32_t Shift = (Characteristics & 0x00F00000) >> 20;
+    uint32_t Shift = (Characteristics & COFF::IMAGE_SCN_ALIGN_MASK) >> 20;
     if (Shift > 0)
       return 1U << (Shift - 1);
     return 0;
   }
+
+  void setAlignment(uint32_t Align) {
+    uint32_t AlignBits = 0;
+    if (Align) {
+      assert(llvm::isPowerOf2_32(Align) && "alignment is not a power of 2");
+      assert(llvm::Log2_32(Align) <= 13 && "alignment requested is too large");
+      AlignBits = (llvm::Log2_32(Align) + 1) << 20;
+    }
+    Characteristics =
+        (Characteristics & ~COFF::IMAGE_SCN_ALIGN_MASK) | AlignBits;
+  }
 };
 
 using coff_tls_directory32 = coff_tls_directory<support::little32_t>;
 using coff_tls_directory64 = coff_tls_directory<support::little64_t>;
 
 /// Bits in control flow guard flags as we understand them.
 enum class coff_guard_flags : uint32_t {
   CFInstrumented = 0x00000100,