[lld-macho] Parse & emit the N_ARM_THUMB_DEF symbol flag

[int3]

Summary

Eventually we'll use this flag to properly handle bl/blx
opcodes.

[int3]

rebase

[gkm]

LGTM ... except for the jarring introduction of an arch-specific bit into arch-independent code. Yet, N_ARM_THUMB_DEF already exists as an arch-dependent bit among the otherwise arch-independent symbol table flags, so I see no way around it.

lld/MachO/Driver.cpp

521–522

Super nit.

[int3]

LGTM ... except for the jarring introduction of an arch-specific bit into arch-independent code. Yet, N_ARM_THUMB_DEF already exists as an arch-dependent bit among the otherwise arch-independent symbol table flags, so I see no way around it.

Yup, I feel the same way, but the ABI is what it is.

lld/MachO/Driver.cpp

521–522

this isn't up to me, it's clang-format :p

lld/MachO/Driver.cpp

for (Symbol *sym : symtab->getSymbols()) {
     // Casting to size_t will truncate large values on 32-bit architectures,
     // but it's not really worth supporting the linking of 64-bit programs on
     // 32-bit archs.
     isec->data = {nullptr, static_cast<size_t>(common->size)};
     isec->flags = S_ZEROFILL;
     inputSections.push_back(isec);
 
     replaceSymbol<Defined>(sym, sym->getName(), isec->file, isec, /*value=*/0,
                            /*size=*/0,
                            /*isWeakDef=*/false,
-                           /*isExternal=*/true, common->privateExtern);
+                           /*isExternal=*/true, common->privateExtern,
+                           /*isThumb=*/false);
   }
 }
 
 static inline char toLowerDash(char x) {
   if (x >= 'A' && x <= 'Z')
     return x - 'A' + 'a';
   else if (x == ' ')
     return '-';

lld/MachO/InputFiles.cpp

if (sym.n_type & (N_EXT | N_PEXT)) {
     // definitions are encountered. With lld's semantics there's no observable
     // difference between a symbol that's isWeakDefCanBeHidden or one that's
     // privateExtern -- neither makes it into the dynamic symbol table. So just
     // promote isWeakDefCanBeHidden to isPrivateExtern here.
     if (isWeakDefCanBeHidden)
       isPrivateExtern = true;
 
     return symtab->addDefined(name, isec->file, isec, value, size,
-                              sym.n_desc & N_WEAK_DEF, isPrivateExtern);
+                              sym.n_desc & N_WEAK_DEF, isPrivateExtern,
+                              sym.n_desc & N_ARM_THUMB_DEF);
   }
 
   assert(!isWeakDefCanBeHidden &&
          "weak_def_can_be_hidden on already-hidden symbol?");
   return make<Defined>(name, isec->file, isec, value, size,
                        sym.n_desc & N_WEAK_DEF,
-                       /*isExternal=*/false, /*isPrivateExtern=*/false);
+                       /*isExternal=*/false, /*isPrivateExtern=*/false,
+                       sym.n_desc & N_ARM_THUMB_DEF);
 }
 
 // Absolute symbols are defined symbols that do not have an associated
 // InputSection. They cannot be weak.
 template <class NList>
 static macho::Symbol *createAbsolute(const NList &sym, InputFile *file,
                                      StringRef name) {
   if (sym.n_type & (N_EXT | N_PEXT)) {
     assert((sym.n_type & N_EXT) && "invalid input");
     return symtab->addDefined(name, file, nullptr, sym.n_value, /*size=*/0,
-                              /*isWeakDef=*/false, sym.n_type & N_PEXT);
+                              /*isWeakDef=*/false, sym.n_type & N_PEXT,
+                              sym.n_desc & N_ARM_THUMB_DEF);
   }
   return make<Defined>(name, file, nullptr, sym.n_value, /*size=*/0,
                        /*isWeakDef=*/false,
-                       /*isExternal=*/false, /*isPrivateExtern=*/false);
+                       /*isExternal=*/false, /*isPrivateExtern=*/false,
+                       sym.n_desc & N_ARM_THUMB_DEF);
 }
 
 template <class NList>
 macho::Symbol *ObjFile::parseNonSectionSymbol(const NList &sym,
                                               StringRef name) {
   uint8_t type = sym.n_type & N_TYPE;
   switch (type) {
   case N_UNDF:
static macho::Symbol *createBitcodeSymbol(const lto::InputFile::Symbol &objSym,
   case GlobalValue::ProtectedVisibility:
     error(name + " has protected visibility, which is not supported by Mach-O");
     break;
   case GlobalValue::DefaultVisibility:
     break;
   }
 
   return symtab->addDefined(name, &file, /*isec=*/nullptr, /*value=*/0,
-                            /*size=*/0, objSym.isWeak(), isPrivateExtern);
+                            /*size=*/0, objSym.isWeak(), isPrivateExtern,
+                            /*isThumb=*/false);
 }
 
 BitcodeFile::BitcodeFile(MemoryBufferRef mbref)
     : InputFile(BitcodeKind, mbref) {
   obj = check(lto::InputFile::create(mbref));
 
   // Convert LTO Symbols to LLD Symbols in order to perform resolution. The
   // "winning" symbol will then be marked as Prevailing at LTO compilation
   // time.
   for (const lto::InputFile::Symbol &objSym : obj->symbols())
     symbols.push_back(createBitcodeSymbol(objSym, *this));
 }
 
 template void ObjFile::parse<LP64>();
 template void DylibFile::parse<LP64>(DylibFile *umbrella);

lld/MachO/SymbolTable.h

  * replaceSymbol(), which does an in-place update of the Symbol via `placement
  * new`. Therefore, there is no need to update any relocations that hold
  * pointers the "old" Symbol -- they will automatically point to the new one.
  */
 class SymbolTable {
 public:
   Defined *addDefined(StringRef name, InputFile *, InputSection *,
                       uint64_t value, uint64_t size, bool isWeakDef,
-                      bool isPrivateExtern);
+                      bool isPrivateExtern, bool isThumb);
 
   Symbol *addUndefined(StringRef name, InputFile *, bool isWeakRef);
 
   Symbol *addCommon(StringRef name, InputFile *, uint64_t size, uint32_t align,
                     bool isPrivateExtern);
 
   Symbol *addDylib(StringRef name, DylibFile *file, bool isWeakDef, bool isTlv);
   Symbol *addDynamicLookup(StringRef name);

lld/MachO/SymbolTable.cpp

std::pair<Symbol *, bool> SymbolTable::insert(StringRef name,
 
   sym->isUsedInRegularObj |= !file || isa<ObjFile>(file);
   return {sym, p.second};
 }
 
 Defined *SymbolTable::addDefined(StringRef name, InputFile *file,
                                  InputSection *isec, uint64_t value,
                                  uint64_t size, bool isWeakDef,
-                                 bool isPrivateExtern) {
+                                 bool isPrivateExtern, bool isThumb) {
   Symbol *s;
   bool wasInserted;
   bool overridesWeakDef = false;
   std::tie(s, wasInserted) = insert(name, file);
 
   if (!wasInserted) {
     if (auto *defined = dyn_cast<Defined>(s)) {
       if (isWeakDef) {
if (auto *defined = dyn_cast<Defined>(s)) {
       overridesWeakDef = !isWeakDef && dysym->isWeakDef();
     }
     // Defined symbols take priority over other types of symbols, so in case
     // of a name conflict, we fall through to the replaceSymbol() call below.
   }
 
   Defined *defined =
       replaceSymbol<Defined>(s, name, file, isec, value, size, isWeakDef,
-                             /*isExternal=*/true, isPrivateExtern);
+                             /*isExternal=*/true, isPrivateExtern, isThumb);
   defined->overridesWeakDef = overridesWeakDef;
   return defined;
 }
 
 Symbol *SymbolTable::addUndefined(StringRef name, InputFile *file,
                                   bool isWeakRef) {
   Symbol *s;
   bool wasInserted;
else if (isa<Undefined>(s) || (isa<DylibSymbol>(s) && s->isWeakDef()))
     file->fetch(sym);
   return s;
 }
 
 Defined *SymbolTable::addSynthetic(StringRef name, InputSection *isec,
                                    uint64_t value, bool isPrivateExtern,
                                    bool includeInSymtab) {
   Defined *s = addDefined(name, nullptr, isec, value, /*size=*/0,
-                          /*isWeakDef=*/false, isPrivateExtern);
+                          /*isWeakDef=*/false, isPrivateExtern,
+                          /*isThumb=*/false);
   s->includeInSymtab = includeInSymtab;
   return s;
 }
 
 void lld::macho::treatUndefinedSymbol(const Undefined &sym) {
   auto message = [](const Undefined &sym) {
     std::string message = "undefined symbol: " + toString(sym);
     std::string fileName = toString(sym.getFile());

lld/MachO/Symbols.h

 public:
   // True if this symbol was referenced by a regular (non-bitcode) object.
   bool isUsedInRegularObj;
 };
 
 class Defined : public Symbol {
 public:
   Defined(StringRefZ name, InputFile *file, InputSection *isec, uint64_t value,
-          uint64_t size, bool isWeakDef, bool isExternal, bool isPrivateExtern)
+          uint64_t size, bool isWeakDef, bool isExternal, bool isPrivateExtern,
+          bool isThumb)
       : Symbol(DefinedKind, name, file), isec(isec), value(value), size(size),
         overridesWeakDef(false), privateExtern(isPrivateExtern),
-        includeInSymtab(true), weakDef(isWeakDef), external(isExternal) {}
+        includeInSymtab(true), thumb(isThumb), weakDef(isWeakDef),
+        external(isExternal) {}
 
   bool isWeakDef() const override { return weakDef; }
   bool isExternalWeakDef() const {
     return isWeakDef() && isExternal() && !privateExtern;
   }
   bool isTlv() const override {
     return !isAbsolute() && isThreadLocalVariables(isec->flags);
   }
public:
   // size is only calculated for regular (non-bitcode) symbols.
   uint64_t size;
 
   bool overridesWeakDef : 1;
   // Whether this symbol should appear in the output binary's export trie.
   bool privateExtern : 1;
   // Whether this symbol should appear in the output symbol table.
   bool includeInSymtab : 1;
+  // Only relevant when compiling for Thumb-supporting arm32 archs.
+  bool thumb : 1;
 
 private:
   const bool weakDef : 1;
   const bool external : 1;
 };
 
 // This enum does double-duty: as a symbol property, it indicates whether & how
 // a dylib symbol is referenced. As a DylibFile property, it indicates the kind

lld/MachO/SyntheticSections.cpp

void StubHelperSection::setup() {
   }
   stubBinder->refState = RefState::Strong;
   in.got->addEntry(stubBinder);
 
   inputSections.push_back(in.imageLoaderCache);
   dyldPrivate =
       make<Defined>("__dyld_private", nullptr, in.imageLoaderCache, 0, 0,
                     /*isWeakDef=*/false,
-                    /*isExternal=*/false, /*isPrivateExtern=*/false);
+                    /*isExternal=*/false, /*isPrivateExtern=*/false,
+                    /*isThumb=*/false);
 }
 
 ImageLoaderCacheSection::ImageLoaderCacheSection() {
   segname = segment_names::data;
   name = section_names::data;
   uint8_t *arr = bAlloc.Allocate<uint8_t>(target->wordSize);
   memset(arr, 0, target->wordSize);
   data = {arr, target->wordSize};
if (auto *defined = dyn_cast<Defined>(entry.sym)) {
         nList->n_sect = NO_SECT;
         nList->n_value = defined->value;
       } else {
         nList->n_type = scope | N_SECT;
         nList->n_sect = defined->isec->parent->index;
         // For the N_SECT symbol type, n_value is the address of the symbol
         nList->n_value = defined->getVA();
       }
+      nList->n_desc |= defined->thumb ? N_ARM_THUMB_DEF : 0;
       nList->n_desc |= defined->isExternalWeakDef() ? N_WEAK_DEF : 0;
     } else if (auto *dysym = dyn_cast<DylibSymbol>(entry.sym)) {
       uint16_t n_desc = nList->n_desc;
       int16_t ordinal = ordinalForDylibSymbol(*dysym);
       if (ordinal == BIND_SPECIAL_DYLIB_FLAT_LOOKUP)
         SET_LIBRARY_ORDINAL(n_desc, DYNAMIC_LOOKUP_ORDINAL);
       else if (ordinal == BIND_SPECIAL_DYLIB_MAIN_EXECUTABLE)
         SET_LIBRARY_ORDINAL(n_desc, EXECUTABLE_ORDINAL);

lld/MachO/UnwindInfoSection.cpp

for (Reloc &r : isec->relocs) {
     if (auto *referentIsec = r.referent.dyn_cast<InputSection *>()) {
       // Personality functions can be referenced via section relocations
       // if they live in the same object file. Create placeholder synthetic
       // symbols for them in the GOT.
       Symbol *&s = personalityTable[{referentIsec, r.addend}];
       if (s == nullptr) {
         s = make<Defined>("<internal>", /*file=*/nullptr, referentIsec,
                           r.addend, /*size=*/0, /*isWeakDef=*/false,
-                          /*isExternal=*/false, /*isPrivateExtern=*/false);
+                          /*isExternal=*/false, /*isPrivateExtern=*/false,
+                          /*isThumb=*/false);
         in.got->addEntry(s);
       }
       r.referent = s;
       r.addend = 0;
     }
   }
 }
 

lld/test/MachO/weak-def-thumb-conflict.s

+# REQUIRES: arm
+# RUN: rm -rf %t; split-file %s %t
+# RUN: llvm-mc -filetype=obj -triple=armv7-apple-watchos %t/thumb-foo.s -o %t/thumb-foo.o
+# RUN: llvm-mc -filetype=obj -triple=armv7-apple-watchos %t/arm-foo.s -o %t/arm-foo.o
+# RUN: %lld-watchos -arch armv7 -dylib %t/arm-foo.o %t/thumb-foo.o -o %t/arm-foo
+# RUN: %lld-watchos -arch armv7 -dylib %t/thumb-foo.o %t/arm-foo.o -o %t/thumb-foo
+# RUN: llvm-nm -m %t/arm-foo | FileCheck %s --check-prefix=ARM
+# RUN: llvm-nm -m %t/thumb-foo | FileCheck %s --check-prefix=THUMB
+
+## Check that we preserve the .thumb_def flag if we pick the thumb definition of
+## _foo.
+# ARM:   (__TEXT,arm)   weak external         _foo
+# THUMB: (__TEXT,thumb) weak external [Thumb] _foo
+
+#--- thumb-foo.s
+.section __TEXT,thumb
+.globl _foo
+.weak_definition _foo
+.thumb_func _foo
+.p2align 2
+_foo:
+
+#--- arm-foo.s
+.section __TEXT,arm
+.globl _foo
+.weak_definition _foo
+.p2align 2
+_foo: