[lld-macho] Force higher alignment for __thread_vars

[BertalanD]

Summary

__thread_vars contains pointers to __tlv_bootstrap, which are fixed
up by dyld; however the section's alignment is not specified. This means
that the relocations might end up on odd addresses, which is not
representable by the soon to be added chained fixups.

This is arguably a bug in MC, but this behavior has been there since TLV
support was originally added.

This patch forces the __thread_vars sections to be aligned to the
target's pointer size size. This is done by ld64 as well.

[thakis]

LG (nice catch!), but should probably fix this in MC too eventually

[BertalanD]

thank you!

should probably fix this in MC too eventually

Sure. The fix is a single line change (+ some more for adjusting old tests). I'll submit it for review in a sec.

Oh, MC's layering is a bit confusing. Not sure where to put the setAlignment call.

We'll still need to have this workaround though if we want to be able to link old objects.

[thakis]

We'll still need to have this workaround though if we want to be able to link old objects.

Yes, we'll have to keep the linker-side workaround for quite a while (MC change needs to go to stable, be picked up by projects depending on LLVM like Swift and Rust, those need to be released, people need to adopt the new versions…and also, linking against binary frameworks is a thing on the platform anyways). But one day, maybe we can turn it into an error (or, first, a warning) :)

lld/MachO/Writer.cpp

for (const auto &it : concatOutputSections) {
     ConcatOutputSection *osec = it.second;
     assert(segname != segment_names::ld);
     if (osec->isNeeded()) {
       // See comment in ObjFile::splitEhFrames()
       if (osec->name == section_names::ehFrame &&
           segname == segment_names::text)
         osec->align = target->wordSize;
 
+      // MC keeps the default 1-byte alignment for __thread_vars, even though it
+      // contains pointers that are fixed up by dyld, which requires proper
+      // alignment.
+      if (isThreadLocalVariables(osec->flags))
+        osec->align = std::max<uint32_t>(osec->align, target->wordSize);
+
       getOrCreateOutputSegment(segname)->addOutputSection(osec);
     }
   }
 
   for (SyntheticSection *ssec : syntheticSections) {
     auto it = concatOutputSections.find({ssec->segname, ssec->name});
     // We add all LinkEdit sections here because we don't know if they are
     // needed until their finalizeContents() methods get called later. While

lld/test/MachO/tlv-dylib.s

 # FLAGS-NEXT:    reloff 0
 # FLAGS-NEXT:    nreloc 0
 # FLAGS-NEXT:      type S_NON_LAZY_SYMBOL_POINTERS
 # FLAGS:       sectname __thread_vars
 # FLAGS-NEXT:   segname __DATA
 # FLAGS-NEXT:      addr
 # FLAGS-NEXT:      size 0x0000000000000030
 # FLAGS-NEXT:    offset
-# FLAGS-NEXT:     align
+# FLAGS-NEXT:     align 2^3 (8)
 # FLAGS-NEXT:    reloff 0
 # FLAGS-NEXT:    nreloc 0
 # FLAGS-NEXT:      type S_THREAD_LOCAL_VARIABLES
 # FLAGS:       sectname __thread_ptrs
 # FLAGS-NEXT:   segname __DATA
 # FLAGS-NEXT:      addr
 # FLAGS-NEXT:      size 0x0000000000000010
 # FLAGS-NEXT:    offset