[lld] 13f439a - [lld/mac] Implement support for private extern symbols

Nico Weber via llvm-commits llvm-commits at lists.llvm.org
Mon Dec 21 18:26:16 PST 2020


Author: Nico Weber
Date: 2020-12-21T21:23:33-05:00
New Revision: 13f439a1872b559d72ee2b5951395310ce4393cc

URL: https://github.com/llvm/llvm-project/commit/13f439a1872b559d72ee2b5951395310ce4393cc
DIFF: https://github.com/llvm/llvm-project/commit/13f439a1872b559d72ee2b5951395310ce4393cc.diff

LOG: [lld/mac] Implement support for private extern symbols

Private extern symbols are used for things scoped to the linkage unit.
They cause duplicate symbol errors (so they're in the symbol table,
unlike TU-scoped truly local symbols), but they don't make it into the
export trie. They are created e.g. by compiling with
-fvisibility=hidden.

If two weak symbols have differing privateness, the combined symbol is
non-private external. (Example: inline functions and some TUs that
include the header defining it were built with
-fvisibility-inlines-hidden and some weren't).

A weak private external symbol implicitly has its "weak" dropped and
behaves like a regular strong private external symbol: Weak is an export
trie concept, and private symbols are not in the export trie.

If a weak and a strong symbol have different privateness, the strong
symbol wins.

If two common symbols have differing privateness, the larger symbol
wins. If they have the same size, the privateness of the symbol seen
later during the link wins (!) -- this is a bit lame, but it matches
ld64 and this behavior takes 2 lines less to implement than the less
surprising "result is non-private external), so match ld64.
(Example: `int a` in two .c files, both built with -fcommon,
one built with -fvisibility=hidden and one without.)

This also makes `__dyld_private` a true TU-local symbol, matching ld64.
To make this work, make the `const char*` StringRefZ ctor to correctly
set `size` (without this, writing the string table crashed when calling
getName() on the __dyld_private symbol).

Mention in CommonSymbol's comment that common symbols are now disabled
by default in clang.

Mention in -keep_private_externs's HelpText that the flag only has an
effect with `-r` (which we don't implement yet -- so this patch here
doesn't regress any behavior around -r + -keep_private_externs)). ld64
doesn't explicitly document it, but the commit text of
http://reviews.llvm.org/rL216146 does, and ld64's
OutputFile::buildSymbolTable() checks `_options.outputKind() ==
Options::kObjectFile` before calling `_options.keepPrivateExterns()`
(the only reference to that function).

Fixes PR48536.

Differential Revision: https://reviews.llvm.org/D93609

Added: 
    lld/test/MachO/private-extern.s
    lld/test/MachO/weak-private-extern.s

Modified: 
    lld/MachO/Driver.cpp
    lld/MachO/InputFiles.cpp
    lld/MachO/Options.td
    lld/MachO/SymbolTable.cpp
    lld/MachO/SymbolTable.h
    lld/MachO/Symbols.h
    lld/MachO/SyntheticSections.cpp
    lld/MachO/SyntheticSections.h
    lld/test/MachO/dylink-lazy.s
    lld/test/MachO/symtab.s

Removed: 
    


################################################################################
diff  --git a/lld/MachO/Driver.cpp b/lld/MachO/Driver.cpp
index 9838e67cd4a2..82ddcf084dc0 100644
--- a/lld/MachO/Driver.cpp
+++ b/lld/MachO/Driver.cpp
@@ -524,7 +524,7 @@ static void replaceCommonSymbols() {
 
     replaceSymbol<Defined>(sym, sym->getName(), isec, /*value=*/0,
                            /*isWeakDef=*/false,
-                           /*isExternal=*/true);
+                           /*isExternal=*/true, common->privateExtern);
   }
 }
 

diff  --git a/lld/MachO/InputFiles.cpp b/lld/MachO/InputFiles.cpp
index 3a4466dd123a..e2282f1fb2bb 100644
--- a/lld/MachO/InputFiles.cpp
+++ b/lld/MachO/InputFiles.cpp
@@ -280,22 +280,33 @@ void ObjFile::parseRelocations(const section_64 &sec,
 static macho::Symbol *createDefined(const structs::nlist_64 &sym,
                                     StringRef name, InputSection *isec,
                                     uint32_t value) {
-  if (sym.n_type & N_EXT)
-    // Global defined symbol
-    return symtab->addDefined(name, isec, value, sym.n_desc & N_WEAK_DEF);
-  // Local defined symbol
+  // Symbol scope is determined by sym.n_type & (N_EXT | N_PEXT):
+  // N_EXT: Global symbols
+  // N_EXT | N_PEXT: Linkage unit (think: dylib) scoped
+  // N_PEXT: Does not occur in input files in practice,
+  //         a private extern must be external.
+  // 0: Translation-unit scoped. These are not in the symbol table.
+
+  if (sym.n_type & (N_EXT | N_PEXT)) {
+    assert((sym.n_type & N_EXT) && "invalid input");
+    return symtab->addDefined(name, isec, value, sym.n_desc & N_WEAK_DEF,
+                              sym.n_type & N_PEXT);
+  }
   return make<Defined>(name, isec, value, sym.n_desc & N_WEAK_DEF,
-                       /*isExternal=*/false);
+                       /*isExternal=*/false, /*isPrivateExtern=*/false);
 }
 
 // Absolute symbols are defined symbols that do not have an associated
 // InputSection. They cannot be weak.
 static macho::Symbol *createAbsolute(const structs::nlist_64 &sym,
                                      StringRef name) {
-  if (sym.n_type & N_EXT)
-    return symtab->addDefined(name, nullptr, sym.n_value, /*isWeakDef=*/false);
+  if (sym.n_type & (N_EXT | N_PEXT)) {
+    assert((sym.n_type & N_EXT) && "invalid input");
+    return symtab->addDefined(name, nullptr, sym.n_value, /*isWeakDef=*/false,
+                              sym.n_type & N_PEXT);
+  }
   return make<Defined>(name, nullptr, sym.n_value, /*isWeakDef=*/false,
-                       /*isExternal=*/false);
+                       /*isExternal=*/false, /*isPrivateExtern=*/false);
 }
 
 macho::Symbol *ObjFile::parseNonSectionSymbol(const structs::nlist_64 &sym,
@@ -306,7 +317,8 @@ macho::Symbol *ObjFile::parseNonSectionSymbol(const structs::nlist_64 &sym,
     return sym.n_value == 0
                ? symtab->addUndefined(name, sym.n_desc & N_WEAK_REF)
                : symtab->addCommon(name, this, sym.n_value,
-                                   1 << GET_COMM_ALIGN(sym.n_desc));
+                                   1 << GET_COMM_ALIGN(sym.n_desc),
+                                   sym.n_type & N_PEXT);
   case N_ABS:
     return createAbsolute(sym, name);
   case N_PBUD:

diff  --git a/lld/MachO/Options.td b/lld/MachO/Options.td
index 1ab2f9109ee0..52a351836a15 100644
--- a/lld/MachO/Options.td
+++ b/lld/MachO/Options.td
@@ -346,7 +346,7 @@ def bundle_loader : Separate<["-"], "bundle_loader">,
 def grp_object : OptionGroup<"object">, HelpText<"CREATING AN OBJECT FILE">;
 
 def keep_private_externs : Flag<["-"], "keep_private_externs">,
-     HelpText<"Do not convert private external symbols to static symbols">,
+     HelpText<"Do not convert private external symbols to static symbols (only valid with -r)">,
      Flags<[HelpHidden]>,
      Group<grp_object>;
 def d : Flag<["-"], "d">,

diff  --git a/lld/MachO/SymbolTable.cpp b/lld/MachO/SymbolTable.cpp
index ea231c9786e2..8a490083ebf1 100644
--- a/lld/MachO/SymbolTable.cpp
+++ b/lld/MachO/SymbolTable.cpp
@@ -38,7 +38,8 @@ std::pair<Symbol *, bool> SymbolTable::insert(StringRef name) {
 }
 
 Symbol *SymbolTable::addDefined(StringRef name, InputSection *isec,
-                                uint32_t value, bool isWeakDef) {
+                                uint32_t value, bool isWeakDef,
+                                bool isPrivateExtern) {
   Symbol *s;
   bool wasInserted;
   bool overridesWeakDef = false;
@@ -46,8 +47,13 @@ Symbol *SymbolTable::addDefined(StringRef name, InputSection *isec,
 
   if (!wasInserted) {
     if (auto *defined = dyn_cast<Defined>(s)) {
-      if (isWeakDef)
+      if (isWeakDef) {
+        // Both old and new symbol weak (e.g. inline function in two TUs):
+        // If one of them isn't private extern, the merged symbol isn't.
+        if (defined->isWeakDef())
+          defined->privateExtern &= isPrivateExtern;
         return s;
+      }
       if (!defined->isWeakDef())
         error("duplicate symbol: " + name);
     } else if (auto *dysym = dyn_cast<DylibSymbol>(s)) {
@@ -57,8 +63,9 @@ Symbol *SymbolTable::addDefined(StringRef name, InputSection *isec,
     // of a name conflict, we fall through to the replaceSymbol() call below.
   }
 
-  Defined *defined = replaceSymbol<Defined>(s, name, isec, value, isWeakDef,
-                                            /*isExternal=*/true);
+  Defined *defined =
+      replaceSymbol<Defined>(s, name, isec, value, isWeakDef,
+                             /*isExternal=*/true, isPrivateExtern);
   defined->overridesWeakDef = overridesWeakDef;
   return s;
 }
@@ -82,7 +89,7 @@ Symbol *SymbolTable::addUndefined(StringRef name, bool isWeakRef) {
 }
 
 Symbol *SymbolTable::addCommon(StringRef name, InputFile *file, uint64_t size,
-                               uint32_t align) {
+                               uint32_t align, bool isPrivateExtern) {
   Symbol *s;
   bool wasInserted;
   std::tie(s, wasInserted) = insert(name);
@@ -98,7 +105,7 @@ Symbol *SymbolTable::addCommon(StringRef name, InputFile *file, uint64_t size,
     // a name conflict, we fall through to the replaceSymbol() call below.
   }
 
-  replaceSymbol<CommonSymbol>(s, name, file, size, align);
+  replaceSymbol<CommonSymbol>(s, name, file, size, align, isPrivateExtern);
   return s;
 }
 

diff  --git a/lld/MachO/SymbolTable.h b/lld/MachO/SymbolTable.h
index 89c866594d72..871687f75eb7 100644
--- a/lld/MachO/SymbolTable.h
+++ b/lld/MachO/SymbolTable.h
@@ -33,11 +33,12 @@ class Symbol;
 class SymbolTable {
 public:
   Symbol *addDefined(StringRef name, InputSection *isec, uint32_t value,
-                     bool isWeakDef);
+                     bool isWeakDef, bool isPrivateExtern);
 
   Symbol *addUndefined(StringRef name, bool isWeakRef);
 
-  Symbol *addCommon(StringRef name, InputFile *, uint64_t size, uint32_t align);
+  Symbol *addCommon(StringRef name, InputFile *, uint64_t size, uint32_t align,
+                    bool isPrivateExtern);
 
   Symbol *addDylib(StringRef name, DylibFile *file, bool isWeakDef, bool isTlv);
 

diff  --git a/lld/MachO/Symbols.h b/lld/MachO/Symbols.h
index 80be27dd1c1f..7f987c722a1f 100644
--- a/lld/MachO/Symbols.h
+++ b/lld/MachO/Symbols.h
@@ -47,7 +47,11 @@ class Symbol {
 
   Kind kind() const { return static_cast<Kind>(symbolKind); }
 
-  StringRef getName() const { return {name.data, name.size}; }
+  StringRef getName() const {
+    if (nameSize == (uint32_t)-1)
+      nameSize = strlen(nameData);
+    return {nameData, nameSize};
+  }
 
   virtual uint64_t getVA() const { return 0; }
 
@@ -80,20 +84,26 @@ class Symbol {
   uint32_t symtabIndex = UINT32_MAX;
 
 protected:
-  Symbol(Kind k, StringRefZ name) : symbolKind(k), name(name) {}
+  Symbol(Kind k, StringRefZ name)
+      : symbolKind(k), nameData(name.data), nameSize(name.size) {}
 
   Kind symbolKind;
-  StringRefZ name;
+  const char *nameData;
+  mutable uint32_t nameSize;
 };
 
 class Defined : public Symbol {
 public:
   Defined(StringRefZ name, InputSection *isec, uint32_t value, bool isWeakDef,
-          bool isExternal)
+          bool isExternal, bool isPrivateExtern)
       : Symbol(DefinedKind, name), isec(isec), value(value),
-        overridesWeakDef(false), weakDef(isWeakDef), external(isExternal) {}
+        overridesWeakDef(false), privateExtern(isPrivateExtern),
+        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);
   }
@@ -110,6 +120,7 @@ class Defined : public Symbol {
   uint32_t value;
 
   bool overridesWeakDef : 1;
+  bool privateExtern : 1;
 
 private:
   const bool weakDef : 1;
@@ -148,14 +159,17 @@ class Undefined : public Symbol {
 //
 // The compiler creates common symbols when it sees tentative definitions.
 // (You can suppress this behavior and let the compiler create a regular
-// defined symbol by passing -fno-common.) When linking the final binary, if
-// there are remaining common symbols after name resolution is complete, the
-// linker converts them to regular defined symbols in a __common section.
+// defined symbol by passing -fno-common. -fno-common is the default in clang
+// as of LLVM 11.0.) When linking the final binary, if there are remaining
+// common symbols after name resolution is complete, the linker converts them
+// to regular defined symbols in a __common section.
 class CommonSymbol : public Symbol {
 public:
-  CommonSymbol(StringRefZ name, InputFile *file, uint64_t size, uint32_t align)
+  CommonSymbol(StringRefZ name, InputFile *file, uint64_t size, uint32_t align,
+               bool isPrivateExtern)
       : Symbol(CommonKind, name), file(file), size(size),
-        align(align != 1 ? align : llvm::PowerOf2Ceil(size)) {
+        align(align != 1 ? align : llvm::PowerOf2Ceil(size)),
+        privateExtern(isPrivateExtern) {
     // TODO: cap maximum alignment
   }
 
@@ -164,6 +178,7 @@ class CommonSymbol : public Symbol {
   InputFile *const file;
   const uint64_t size;
   const uint32_t align;
+  const bool privateExtern;
 };
 
 class DylibSymbol : public Symbol {

diff  --git a/lld/MachO/SyntheticSections.cpp b/lld/MachO/SyntheticSections.cpp
index 433e6aac0259..8b2ebd36e1ae 100644
--- a/lld/MachO/SyntheticSections.cpp
+++ b/lld/MachO/SyntheticSections.cpp
@@ -362,12 +362,10 @@ void WeakBindingSection::writeTo(uint8_t *buf) const {
 }
 
 bool macho::needsBinding(const Symbol *sym) {
-  if (isa<DylibSymbol>(sym)) {
+  if (isa<DylibSymbol>(sym))
     return true;
-  } else if (const auto *defined = dyn_cast<Defined>(sym)) {
-    if (defined->isWeakDef() && defined->isExternal())
-      return true;
-  }
+  if (const auto *defined = dyn_cast<Defined>(sym))
+    return defined->isExternalWeakDef();
   return false;
 }
 
@@ -380,7 +378,7 @@ void macho::addNonLazyBindingEntries(const Symbol *sym,
       in.weakBinding->addEntry(sym, section, offset, addend);
   } else if (auto *defined = dyn_cast<Defined>(sym)) {
     in.rebase->addEntry(section, offset);
-    if (defined->isWeakDef() && defined->isExternal())
+    if (defined->isExternalWeakDef())
       in.weakBinding->addEntry(sym, section, offset, addend);
   } else if (isa<DSOHandle>(sym)) {
     error("cannot bind to " + DSOHandle::name);
@@ -446,8 +444,10 @@ void StubHelperSection::setup() {
   in.got->addEntry(stubBinder);
 
   inputSections.push_back(in.imageLoaderCache);
-  symtab->addDefined("__dyld_private", in.imageLoaderCache, 0,
-                     /*isWeakDef=*/false);
+  dyldPrivate =
+      make<Defined>("__dyld_private", in.imageLoaderCache, 0,
+                    /*isWeakDef=*/false,
+                    /*isExternal=*/false, /*isPrivateExtern=*/false);
 }
 
 ImageLoaderCacheSection::ImageLoaderCacheSection() {
@@ -555,7 +555,7 @@ void macho::prepareBranchTarget(Symbol *sym) {
       }
     }
   } else if (auto *defined = dyn_cast<Defined>(sym)) {
-    if (defined->isWeakDef() && defined->isExternal()) {
+    if (defined->isExternalWeakDef()) {
       if (in.stubs->addEntry(sym)) {
         in.rebase->addEntry(in.lazyPointers, sym->stubsIndex * WordSize);
         in.weakBinding->addEntry(sym, in.lazyPointers,
@@ -570,9 +570,10 @@ ExportSection::ExportSection()
 
 void ExportSection::finalizeContents() {
   trieBuilder.setImageBase(in.header->addr);
-  // TODO: We should check symbol visibility.
   for (const Symbol *sym : symtab->getSymbols()) {
     if (const auto *defined = dyn_cast<Defined>(sym)) {
+      if (defined->privateExtern)
+        continue;
       trieBuilder.addSymbol(*defined);
       hasWeakSymbol = hasWeakSymbol || sym->isWeakDef();
     }
@@ -710,6 +711,13 @@ void SymtabSection::finalizeContents() {
     }
   }
 
+  // __dyld_private is a local symbol too. It's linker-created and doesn't
+  // exist in any object file.
+  if (Defined* dyldPrivate = in.stubHelper->dyldPrivate) {
+    uint32_t strx = stringTableSection.addString(dyldPrivate->getName());
+    localSymbols.push_back({dyldPrivate, strx});
+  }
+
   for (Symbol *sym : symtab->getSymbols()) {
     uint32_t strx = stringTableSection.addString(sym->getName());
     if (auto *defined = dyn_cast<Defined>(sym)) {
@@ -752,18 +760,31 @@ void SymtabSection::writeTo(uint8_t *buf) const {
     nList->n_strx = entry.strx;
     // TODO populate n_desc with more flags
     if (auto *defined = dyn_cast<Defined>(entry.sym)) {
+      uint8_t scope = 0;
+      if (defined->privateExtern) {
+        // Private external -- dylib scoped symbol.
+        // Promote to non-external at link time.
+        assert(defined->isExternal() && "invalid input file");
+        scope = MachO::N_PEXT;
+      } else if (defined->isExternal()) {
+        // Normal global symbol.
+        scope = MachO::N_EXT;
+      } else {
+        // TU-local symbol from localSymbols.
+        scope = 0;
+      }
+
       if (defined->isAbsolute()) {
-        nList->n_type = MachO::N_EXT | MachO::N_ABS;
+        nList->n_type = scope | MachO::N_ABS;
         nList->n_sect = MachO::NO_SECT;
         nList->n_value = defined->value;
       } else {
-        nList->n_type =
-            (defined->isExternal() ? MachO::N_EXT : 0) | MachO::N_SECT;
+        nList->n_type = scope | MachO::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->isWeakDef() ? MachO::N_WEAK_DEF : 0;
+      nList->n_desc |= defined->isExternalWeakDef() ? MachO::N_WEAK_DEF : 0;
     } else if (auto *dysym = dyn_cast<DylibSymbol>(entry.sym)) {
       uint16_t n_desc = nList->n_desc;
       MachO::SET_LIBRARY_ORDINAL(n_desc, dysym->file->ordinal);

diff  --git a/lld/MachO/SyntheticSections.h b/lld/MachO/SyntheticSections.h
index 9384df6481f0..7bca28de1386 100644
--- a/lld/MachO/SyntheticSections.h
+++ b/lld/MachO/SyntheticSections.h
@@ -327,6 +327,7 @@ class StubHelperSection : public SyntheticSection {
   void setup();
 
   DylibSymbol *stubBinder = nullptr;
+  Defined *dyldPrivate = nullptr;
 };
 
 // This section contains space for just a single word, and will be used by dyld

diff  --git a/lld/test/MachO/dylink-lazy.s b/lld/test/MachO/dylink-lazy.s
index 8e3e0b6ae717..a5f275e55f08 100644
--- a/lld/test/MachO/dylink-lazy.s
+++ b/lld/test/MachO/dylink-lazy.s
@@ -27,7 +27,7 @@
 # RUN: llvm-objdump --macho --rebase %t/dylink-lazy-pie | FileCheck %s --check-prefix=PIE
 
 # CHECK-LABEL: SYMBOL TABLE:
-# CHECK:       {{0*}}[[#%x, IMGLOADER:]] {{.*}} __DATA,__data __dyld_private
+# CHECK:       {{0*}}[[#%x, IMGLOADER:]] l {{.*}} __DATA,__data __dyld_private
 
 # CHECK-LABEL: Disassembly of section __TEXT,__text:
 # CHECK:         callq 0x[[#%x, HELLO_STUB:]]

diff  --git a/lld/test/MachO/private-extern.s b/lld/test/MachO/private-extern.s
new file mode 100644
index 000000000000..c8c96aa8e7b3
--- /dev/null
+++ b/lld/test/MachO/private-extern.s
@@ -0,0 +1,143 @@
+# REQUIRES: x86
+
+# RUN: split-file %s %t
+# RUN: llvm-mc -filetype=obj -triple=x86_64-apple-macos \
+# RUN:     %t/basics.s -o %t/basics.o
+
+## Check that .private_extern symbols are marked as local in the symbol table
+## and aren't in the export trie.
+# RUN: %lld -dylib %t/basics.o -o %t/basics
+# RUN: llvm-objdump --syms --exports-trie %t/basics | \
+# RUN:     FileCheck --check-prefix=EXPORTS %s
+# RUN: llvm-nm -m %t/basics | FileCheck --check-prefix=EXPORTS-NM %s
+# EXPORTS-LABEL: SYMBOL TABLE:
+# EXPORTS-DAG:   [[#%x, FOO_ADDR:]] l {{.*}} _foo
+# EXPORTS-DAG:   [[#%x, BAR_ADDR:]] g {{.*}} _bar
+# EXPORTS-LABEL: Exports trie:
+# EXPORTS-NOT:   0x{{0*}}[[#%X, FOO_ADDR]] _foo
+# EXPORTS-DAG:   0x{{0*}}[[#%X, BAR_ADDR]] _bar
+# EXPORTS-NOT:   0x{{0*}}[[#%X, FOO_ADDR]] _foo
+# EXPORTS-NM-DAG: (__TEXT,__cstring) non-external (was a private external) _foo
+# EXPORTS-NM-DAG: (__TEXT,__cstring) external _bar
+
+#--- basics.s
+.section __TEXT,__cstring
+
+.globl _foo, _bar
+.private_extern _foo
+
+_foo:
+.asciz "Foo"
+
+_bar:
+.asciz "Bar"
+
+# RUN: llvm-mc -filetype=obj -triple=x86_64-apple-macos \
+# RUN:     %t/strong-globl.s -o %t/strong-globl.o
+# RUN: llvm-mc -filetype=obj -triple=x86_64-apple-macos \
+# RUN:     %t/weak-globl.s -o %t/weak-globl.o
+
+# RUN: llvm-mc -filetype=obj -triple=x86_64-apple-macos \
+# RUN:     %t/strong-private.s -o %t/strong-private.o
+# RUN: llvm-mc -filetype=obj -triple=x86_64-apple-macos \
+# RUN:     %t/weak-private.s -o %t/weak-private.o
+
+## weak + strong symbol takes privateness from strong symbol
+## - weak private extern + strong extern = strong extern (for both .o orderings)
+# RUN: %lld -dylib %t/weak-private.o %t/strong-globl.o -o %t/wpsg
+# RUN: llvm-nm -m %t/wpsg | FileCheck --check-prefix=EXTERNAL %s
+# RUN: %lld -dylib %t/strong-globl.o %t/weak-private.o -o %t/sgwp
+# RUN: llvm-nm -m %t/sgwp | FileCheck --check-prefix=EXTERNAL %s
+# EXTERNAL: (__TEXT,__text) external _foo
+## - weak extern + strong private extern = strong private extern
+##   (for both .o orderings)
+# RUN: %lld -dylib %t/weak-globl.o %t/strong-private.o -o %t/wgsp
+# RUN: llvm-nm -m %t/wgsp | FileCheck --check-prefix=NONEXTERNAL %s
+# RUN: %lld -dylib %t/strong-private.o %t/weak-globl.o -o %t/spwg
+# RUN: llvm-nm -m %t/spwg | FileCheck --check-prefix=NONEXTERNAL %s
+# NONEXTERNAL: (__TEXT,__text) non-external (was a private external) _foo
+
+## weak + weak symbol take weaker privateness
+## - weak extern + weak private extern = weak extern (both orders)
+# RUN: %lld -dylib %t/weak-private.o %t/weak-globl.o -o %t/wpwg
+# RUN: llvm-nm -m %t/wpwg | FileCheck --check-prefix=WEAK-EXTERNAL %s
+# RUN: %lld -dylib %t/weak-globl.o %t/weak-private.o -o %t/wgwp
+# RUN: llvm-nm -m %t/wgwp | FileCheck --check-prefix=WEAK-EXTERNAL %s
+# WEAK-EXTERNAL: (__TEXT,__text) weak external _foo
+## - weak private extern + weak private extern = weak private extern
+# RUN: %lld -dylib %t/weak-private.o %t/weak-private.o -o %t/wpwp
+# RUN: llvm-nm -m %t/wpwp | FileCheck --check-prefix=NONEXTERNAL %s
+
+#--- strong-globl.s
+.globl _foo
+_foo:
+  retq
+
+#--- weak-globl.s
+.globl _foo
+.weak_definition _foo
+_foo:
+  retq
+
+#--- strong-private.s
+.private_extern _foo
+.globl _foo
+_foo:
+  retq
+
+#--- weak-private.s
+.private_extern _foo
+.globl _foo
+.weak_definition _foo
+_foo:
+  retq
+
+# RUN: llvm-mc -filetype=obj -triple=x86_64-apple-macos \
+# RUN:     %t/comm-small.s -o %t/comm-small.o
+# RUN: llvm-mc -filetype=obj -triple=x86_64-apple-macos \
+# RUN:     %t/comm-large.s -o %t/comm-large.o
+
+# RUN: llvm-mc -filetype=obj -triple=x86_64-apple-macos \
+# RUN:     %t/comm-small-private.s -o %t/comm-small-private.o
+# RUN: llvm-mc -filetype=obj -triple=x86_64-apple-macos \
+# RUN:     %t/comm-large-private.s -o %t/comm-large-private.o
+
+## For common symbols the larger one wins.
+## - smaller private extern + larger extern = larger extern
+# RUN: %lld -dylib %t/comm-small-private.o %t/comm-large.o -o %t/cspcl
+# RUN: llvm-nm -m %t/cspcl | FileCheck --check-prefix=COMMON-EXTERNAL %s
+# RUN: %lld -dylib %t/comm-large.o %t/comm-small-private.o -o %t/clcsp
+# RUN: llvm-nm -m %t/clcsp | FileCheck --check-prefix=COMMON-EXTERNAL %s
+# COMMON-EXTERNAL: (__DATA,__common) external _foo
+## - smaller extern + larger private extern = larger private extern
+# RUN: %lld -dylib %t/comm-large-private.o %t/comm-small.o -o %t/clpcs
+# RUN: llvm-nm -m %t/clpcs | FileCheck --check-prefix=COMMON-NONEXTERNAL %s
+# RUN: %lld -dylib %t/comm-small.o %t/comm-large-private.o -o %t/csclp
+# RUN: llvm-nm -m %t/csclp | FileCheck --check-prefix=COMMON-NONEXTERNAL %s
+# COMMON-NONEXTERNAL: (__DATA,__common) non-external (was a private external) _foo
+
+# For common symbols with the same size, the privateness of the symbol seen
+# later wins (!).
+## - equal private extern + equal extern = equal extern (both orders)
+# RUN: %lld -dylib %t/comm-small-private.o %t/comm-small.o -o %t/cspcs
+# RUN: llvm-nm -m %t/cspcs | FileCheck --check-prefix=COMMON-EXTERNAL %s
+## - equal extern + equal private extern = equal private extern (both orders)
+# RUN: %lld -dylib %t/comm-small.o %t/comm-small-private.o -o %t/cscsp
+# RUN: llvm-nm -m %t/cscsp | FileCheck --check-prefix=COMMON-NONEXTERNAL %s
+## - equal private extern + equal private extern = equal private extern
+# RUN: %lld -dylib %t/comm-small-private.o %t/comm-small-private.o -o %t/cspcsp
+# RUN: llvm-nm -m %t/cspcsp | FileCheck --check-prefix=COMMON-NONEXTERNAL %s
+
+#--- comm-small.s
+.comm _foo,4,2
+
+#--- comm-large.s
+.comm _foo,8,3
+
+#--- comm-small-private.s
+.private_extern _foo
+.comm _foo,4,2
+
+#--- comm-large-private.s
+.private_extern _foo
+.comm _foo,8,3

diff  --git a/lld/test/MachO/symtab.s b/lld/test/MachO/symtab.s
index 222a35798909..d18986c9d91c 100644
--- a/lld/test/MachO/symtab.s
+++ b/lld/test/MachO/symtab.s
@@ -8,7 +8,7 @@
 # RUN: llvm-readobj --syms --macho-dysymtab %t/test | FileCheck %s
 # CHECK:      Symbols [
 # CHECK-NEXT:   Symbol {
-# CHECK-NEXT:     Name: _local (2)
+# CHECK-NEXT:     Name: _local
 # CHECK-NEXT:     Type: Section (0xE)
 # CHECK-NEXT:     Section: __data (0x4)
 # CHECK-NEXT:     RefType: UndefinedNonLazy (0x0)
@@ -17,48 +17,47 @@
 # CHECK-NEXT:     Value: 0x1{{[0-9a-f]*}}
 # CHECK-NEXT:   }
 # CHECK-NEXT:   Symbol {
-# CHECK-NEXT:     Name: _main (9)
-# CHECK-NEXT:     Extern
+# CHECK-NEXT:     Name: __dyld_private
 # CHECK-NEXT:     Type: Section (0xE)
-# CHECK-NEXT:     Section: __text (0x1)
+# CHECK-NEXT:     Section: __data (0x4)
 # CHECK-NEXT:     RefType: UndefinedNonLazy (0x0)
 # CHECK-NEXT:     Flags [ (0x0)
 # CHECK-NEXT:     ]
 # CHECK-NEXT:     Value: 0x1{{[0-9a-f]*}}
 # CHECK-NEXT:   }
 # CHECK-NEXT:   Symbol {
-# CHECK-NEXT:     Name: _external (55)
+# CHECK-NEXT:     Name: _main
 # CHECK-NEXT:     Extern
 # CHECK-NEXT:     Type: Section (0xE)
-# CHECK-NEXT:     Section: __data (0x4)
+# CHECK-NEXT:     Section: __text (0x1)
 # CHECK-NEXT:     RefType: UndefinedNonLazy (0x0)
 # CHECK-NEXT:     Flags [ (0x0)
 # CHECK-NEXT:     ]
 # CHECK-NEXT:     Value: 0x1{{[0-9a-f]*}}
 # CHECK-NEXT:   }
 # CHECK-NEXT:   Symbol {
-# CHECK-NEXT:     Name: _external_weak (65)
+# CHECK-NEXT:     Name: _external
 # CHECK-NEXT:     Extern
 # CHECK-NEXT:     Type: Section (0xE)
-# CHECK-NEXT:     Section: __text (0x1)
+# CHECK-NEXT:     Section: __data (0x4)
 # CHECK-NEXT:     RefType: UndefinedNonLazy (0x0)
-# CHECK-NEXT:     Flags [ (0x80)
-# CHECK-NEXT:       WeakDef (0x80)
+# CHECK-NEXT:     Flags [ (0x0)
 # CHECK-NEXT:     ]
 # CHECK-NEXT:     Value: 0x1{{[0-9a-f]*}}
 # CHECK-NEXT:   }
 # CHECK-NEXT:   Symbol {
-# CHECK-NEXT:     Name: __dyld_private (103)
+# CHECK-NEXT:     Name: _external_weak
 # CHECK-NEXT:     Extern
 # CHECK-NEXT:     Type: Section (0xE)
-# CHECK-NEXT:     Section: __data (0x4)
+# CHECK-NEXT:     Section: __text (0x1)
 # CHECK-NEXT:     RefType: UndefinedNonLazy (0x0)
-# CHECK-NEXT:     Flags [ (0x0)
+# CHECK-NEXT:     Flags [ (0x80)
+# CHECK-NEXT:       WeakDef (0x80)
 # CHECK-NEXT:     ]
 # CHECK-NEXT:     Value: 0x1{{[0-9a-f]*}}
 # CHECK-NEXT:   }
 # CHECK-NEXT:   Symbol {
-# CHECK-NEXT:     Name: dyld_stub_binder (15)
+# CHECK-NEXT:     Name: dyld_stub_binder
 # CHECK-NEXT:     Extern
 # CHECK-NEXT:     Type: Undef (0x0)
 # CHECK-NEXT:     Section:  (0x0)
@@ -68,7 +67,7 @@
 # CHECK-NEXT:     Value: 0x0
 # CHECK-NEXT:   }
 # CHECK-NEXT:   Symbol {
-# CHECK-NEXT:     Name: _dynamic (80)
+# CHECK-NEXT:     Name: _dynamic
 # CHECK-NEXT:     Extern
 # CHECK-NEXT:     Type: Undef (0x0)
 # CHECK-NEXT:     Section:  (0x0)
@@ -81,9 +80,9 @@
 # CHECK-NEXT: ]
 # CHECK-NEXT: Dysymtab {
 # CHECK-NEXT:   ilocalsym: 0
-# CHECK-NEXT:   nlocalsym: 1
-# CHECK-NEXT:   iextdefsym: 1
-# CHECK-NEXT:   nextdefsym: 4
+# CHECK-NEXT:   nlocalsym: 2
+# CHECK-NEXT:   iextdefsym: 2
+# CHECK-NEXT:   nextdefsym: 3
 # CHECK-NEXT:   iundefsym: 5
 # CHECK-NEXT:   nundefsym: 2
 

diff  --git a/lld/test/MachO/weak-private-extern.s b/lld/test/MachO/weak-private-extern.s
new file mode 100644
index 000000000000..78fb4260999c
--- /dev/null
+++ b/lld/test/MachO/weak-private-extern.s
@@ -0,0 +1,38 @@
+# REQUIRES: x86
+
+# RUN: llvm-mc -filetype=obj -triple=x86_64-apple-macos %s -o %t.o
+# RUN: %lld -dylib %t.o -o %t.dylib -lSystem
+# RUN: llvm-objdump --macho --weak-bind %t.dylib | FileCheck %s
+# CHECK-NOT: __got
+# CHECK-NOT: __la_symbol_ptr
+
+# RUN: llvm-objdump --macho --all-headers %t.dylib | \
+# RUN:     FileCheck --check-prefix=HEADERS %s
+# HEADERS-NOT: WEAK_DEFINES
+# HEADERS-NOT: BINDS_TO_WEAK
+
+## Check that N_WEAK_DEF isn't set in the symbol table.
+## This is 
diff erent from ld64, which makes private extern weak symbols non-weak
+## for binds and relocations, but it still marks them as weak in the symbol table.
+## Since `nm -m` doesn't look at N_WEAK_DEF for N_PEXT symbols this is not
+## observable, but it feels slightly more correct.
+# RUN: llvm-readobj --syms %t.dylib | FileCheck --check-prefix=SYMS %s
+# SYMS-NOT: WeakDef (0x80)
+
+.globl _use
+_use:
+  mov _weak_private_extern_gotpcrel at GOTPCREL(%rip), %rax
+  callq _weak_private_extern
+  retq
+
+.private_extern _weak_private_extern
+.globl _weak_private_extern
+.weak_definition _weak_private_extern
+_weak_private_extern:
+  retq
+
+.private_extern _weak_private_extern_gotpcrel
+.globl _weak_private_extern_gotpcrel
+.weak_definition _weak_private_extern_gotpcrel
+_weak_private_extern_gotpcrel:
+  .quad 0x1234


        


More information about the llvm-commits mailing list