[lld-macho]Allow exporting weak_def_can_be_hidden(AKA "autohide") symbols

[oontvoo]

Summary

autohide symbols behaves similarly to private_extern symbols.
However, LD64 allows exporting autohide symbols. LLD currently does not.
This patch allows LLD to export them.

[oontvoo]

typos

[oontvoo]

added test

[oontvoo]

@thakis would you like to review this too? (related to discussions in D112977)
specifically, the part where weak-def-can-be-hiddens symbols got "promoted" to privateExtern... (this's side effect is that lld currently refuses to export it ...)

[thakis]

That honestly sounds like a bug in your build config, independently of if ld64 allows it, no?

[oontvoo]

In D113167#3109188, @thakis wrote:

That honestly sounds like a bug in your build config, independently of if ld64 allows it, no?

I still think that LLD shouldn't conflate the following two cases:

// hidden.cc
extern __attribute__((weak)) __attribute__((visibility("hidden"))) void foo()  {return 5;}
int bar() { return foo(); }

VS

// can_be_hidden.cc (Must be C++, not C)
__attribute__((noinline)) inline int foo() { return 5; }
int bar() { return foo(); }

For the hidden.cc, clearly we shouldn't export foo. (current behaviour).
But for the latter, it should be fine

[smeenai]

In D113167#3109836, @oontvoo wrote:

In D113167#3109188, @thakis wrote:

That honestly sounds like a bug in your build config, independently of if ld64 allows it, no?

I still think that LLD shouldn't conflate the following two cases:

// hidden.cc
extern __attribute__((weak)) __attribute__((visibility("hidden"))) void foo()  {return 5;}
int bar() { return foo(); }

VS

// can_be_hidden.cc (Must be C++, not C)
__attribute__((noinline)) inline int foo() { return 5; }
int bar() { return foo(); }

For the hidden.cc, clearly we shouldn't export foo. (current behaviour).
But for the latter, it should be fine

In your can_be_hidden.cc example, why does LLD think foo is privateExtern to begin with? It's not marked that way in the object file (as confirmed with nm -m). That potentially seems like the actual issue here.

[oontvoo]

In D113167#3109843, @smeenai wrote:

In your can_be_hidden.cc example, why does LLD think foo is privateExtern to begin with? It's not marked that way in the object file (as confirmed with nm -m). That potentially seems like the actual issue here.

Because createDefined() in InputFiles "promote" it to privateExtern :)

P.S: link https://github.com/llvm/llvm-project/blob/48dc5c8e731b1198c4ba4b2f08b235c55e6d5aef/lld/MachO/InputFiles.cpp#L573

(it has explanation on why)

[smeenai]

In D113167#3109848, @oontvoo wrote:

In D113167#3109843, @smeenai wrote:

In your can_be_hidden.cc example, why does LLD think foo is privateExtern to begin with? It's not marked that way in the object file (as confirmed with nm -m). That potentially seems like the actual issue here.

Because createDefined() in InputFiles "promote" it to privateExtern :)

P.S: link https://github.com/llvm/llvm-project/blob/48dc5c8e731b1198c4ba4b2f08b235c55e6d5aef/lld/MachO/InputFiles.cpp#L573

(it has explanation on why)

Ah, got it. Took me a while to understand, because the combination of N_WEAK_DEF and N_WEAK_REF meaning autohide is incredibly confusing (sigh), but I get it now.

If I'm understanding your comment correctly now, you're saying that it's incorrect for LLD to conflate the autohide case with the private extern case, which I agree with you about.

After your patch, if I link your hidden.cc example with lld and add -exported_symbol __Z3foov, LLD is able to export the symbol, which seems incorrect. ld64 doesn't export the symbol, which makes sense to me.

Maybe we should be tracking autohide and private extern separately on symbols, to avoid this issue?

lld/test/MachO/exported-symbols.s

12 ↗

(On Diff #384762)

You'll need either nm -g or objdump -macho --exports-trie to only print the exported symbols.

[oontvoo]

In D113167#3110544, @smeenai wrote:

Ah, got it. Took me a while to understand, because the combination of N_WEAK_DEF and N_WEAK_REF meaning autohide is incredibly confusing (sigh), but I get it now.
If I'm understanding your comment correctly now, you're saying that it's incorrect for LLD to conflate the autohide case with the private extern case, which I agree with you about.
After your patch, if I link your hidden.cc example with lld and add -exported_symbol __Z3foov, LLD is able to export the symbol, which seems incorrect. ld64 doesn't export the symbol, which makes sense to me.

Yep, I've realised that now :)

Maybe we should be tracking autohide and private extern separately on symbols, to avoid this issue?

SG

[oontvoo]

oops - meant to "Plan changes" (not abandon)

[thakis]

In D113167#3109836, @oontvoo wrote:

In D113167#3109188, @thakis wrote:

That honestly sounds like a bug in your build config, independently of if ld64 allows it, no?

I still think that LLD shouldn't conflate the following two cases:

// hidden.cc
extern __attribute__((weak)) __attribute__((visibility("hidden"))) void foo()  {return 5;}
int bar() { return foo(); }

VS

// can_be_hidden.cc (Must be C++, not C)
__attribute__((noinline)) inline int foo() { return 5; }
int bar() { return foo(); }

For the hidden.cc, clearly we shouldn't export foo. (current behaviour).
But for the latter, it should be fine

You can make a case for this, but as far as I understand the only programs where this can make a difference in practice is one with ODR violations, right?

[oontvoo]

In D113167#3112543, @thakis wrote:

In D113167#3109836, @oontvoo wrote:

In D113167#3109188, @thakis wrote:

That honestly sounds like a bug in your build config, independently of if ld64 allows it, no?

I still think that LLD shouldn't conflate the following two cases:

// hidden.cc
extern __attribute__((weak)) __attribute__((visibility("hidden"))) void foo()  {return 5;}
int bar() { return foo(); }

VS

// can_be_hidden.cc (Must be C++, not C)
__attribute__((noinline)) inline int foo() { return 5; }
int bar() { return foo(); }

For the hidden.cc, clearly we shouldn't export foo. (current behaviour).
But for the latter, it should be fine

You can make a case for this, but as far as I understand the only programs where this can make a difference in practice is one with ODR violations, right?

If the copy is private, then I don't think it's strictly ODR violations.
But maybe two orthogonal points here:

• is it correct to make autohide symbols synonym with private-extern? (agreed that it likely has no observable difference in the output)
• is it correct to prevent autohide symbols from being exported? (From the example, there's no reason foo shouldn't be exportable - it's a bit weird, yes, but it's not wrong)

[oontvoo]

• propagated weak_def_can_be_hidden to symbols so we can distinguish it with the "real" private_extern, if needed.
• consolidated tests with existing ones

[int3]

lld/MachO/Driver.cpp

1473–1479

This is basically undoing the promotion work in InputFiles.cpp, right? Would it be simpler if we didn't attempt that promotion, and just treated these two booleans as independent values?

Also, what happens when a symbol is marked both autohide and private_extern?

lld/MachO/Symbols.h

164

hmm, this brings us to 9 bits of booleans :/

probably fine in practice, but maybe do a quick benchmark before landing this

lld/test/MachO/weak-def-can-be-hidden.s

45–46 ↗

(On Diff #385676)

could we place this test in export-options.s?

[oontvoo]

updated diff

lld/MachO/Driver.cpp

1473–1479

Well, it's a bit more convoluted than that (sorry!)
It's only undoing the promotion IF the symbol is in the exported list. (if not, it needs to stay privateExtern so we dont put it in the dynamic table).
However, if a symbol is both autohide and private-extern, then the autohide doesn't mean anything - it can't be exported either. (added the test)

We can keep the two flags independent in theory, but that means I'd have to go fix up all the places where privateExtern is treated to also take into account this extra flag, which (aside from being a bit of work) is unnecessary because as mentioned the symbol acts exactly like a privateExtern, only difference is that it can be exported.

[int3]

lld/MachO/Driver.cpp

1473–1479

I was going to say that I guess it's only a bit awkward but then I read the new code in InputFiles.cpp and I changed my mind :P

I think it's super confusing: for one, isWeakDefCanBeHidden == true means that a privateExtern symbol can be *un*hidden. Also, to prevent that un-hiding from happening when both are set, we flip isWeakDefCanBeHidden inside createDefined because it is only in that function that we know that privateExtern *really* means .private_extern. I.e. it has different semantics within that function vs everywhere else.

lld/MachO/InputFiles.cpp

587–592

what happens if there is an existing symbol that's isWeakDefCanBeHidden and another one that's isPrivateExtern? I.e. what if those two properties are defined on different symbols with the same name?

(should also add a test that covers this)

[oontvoo]

lld/MachO/InputFiles.cpp

587–592

will add a test - but I'd think the privateExtern symbol would be picked over the weakdef one, which means it won't be exported (makes sense logically).

Now the question is, what if both are weak? ....

[oontvoo]

add tests for strong-private + autohide and weak-private + autohide

[thevinster]

lld/MachO/Driver.cpp

1469–1479

The demotion/promotion is bit hard to follow unless you stare at the code for a good while. Would it better to just have createDefined check if the symbol is in the exported list and skip the whole round trip of promoting and then demoting?

lld/MachO/InputFiles.cpp

587–592

Nested ifs always is kinda making things a bit confusing to read. Not that it's significantly better, but I think hoisting the inner if and being explicit looks better (shows the actual combinations we are overriding)

if (isPrivateExtern && isWeakDefCanBeHidden)
  isWeakDefCanBeHidden = false;
else if (!isPrivateExtern && isWeakDefCanBeHidden)
  isPrivateExtern = true;

lld/MachO/MarkLive.cpp

70 ↗

(On Diff #386530)

Didn't we demote the symbol? so the || defined->weakDefCanBeHidden shouldn't be needed?

lld/test/MachO/export-options.s

138

nit: Can we move the check after all the RUN statements?

[oontvoo]

lld/MachO/Driver.cpp

1469–1479

not sure it'd be better because that would mean have to do the check for *every* single autohide symbols. (whereas right now the cost is only paid by autohide symbols that are exported)

1473–1479

Missed this earlier - right I agree it's confusing.

[oontvoo]

updated diff

[oontvoo]

reworking the flag flip to avoid confusions

[oontvoo]

• re-arrange the condition a bit.
• also parallelize scanning the symbol tables in export/unexport-ing. This shows a small improvment in our internal apps:

x ./LLD_no_parallel.txt
+ ./LLD_with_parallel.txt
+---------------------------------------------------------------------------------------------------------------------------------------------------------------------+
|      +                                                                                                                                     x                        |
|+++   +++  +      +   +                                                                                          x                     x    x  x             x  x xxx|
| |_____MA______|                                                                                                                   |________________A________M______||
+---------------------------------------------------------------------------------------------------------------------------------------------------------------------+
    N           Min           Max        Median           Avg        Stddev
x  10         84.01         89.41         88.64        87.693     1.7424061
+  10          71.9         74.29         72.63        72.753    0.77734663
Difference at 95.0% confidence
	-14.94 +/- 1.26763
	-17.0367% +/- 1.44553%
	(Student's t, pooled s = 1.34912)

[smeenai]

In D113167#3126069, @oontvoo wrote:

• re-arrange the condition a bit.
• also parallelize scanning the symbol tables in export/unexport-ing. This shows a small improvment in our internal apps:

x ./LLD_no_parallel.txt
+ ./LLD_with_parallel.txt
+---------------------------------------------------------------------------------------------------------------------------------------------------------------------+
|      +                                                                                                                                     x                        |
|+++   +++  +      +   +                                                                                          x                     x    x  x             x  x xxx|
| |_____MA______|                                                                                                                   |________________A________M______||
+---------------------------------------------------------------------------------------------------------------------------------------------------------------------+
    N           Min           Max        Median           Avg        Stddev
x  10         84.01         89.41         88.64        87.693     1.7424061
+  10          71.9         74.29         72.63        72.753    0.77734663
Difference at 95.0% confidence
	-14.94 +/- 1.26763
	-17.0367% +/- 1.44553%
	(Student's t, pooled s = 1.34912)

Am I reading that table incorrectly, or do you consider a 17% improvement to be small? :)

lld/MachO/Symbols.h

164

It won't change the size of the struct, cos of padding. Turns out these structs are laid out suboptimally with regards to padding though; I'll put up a diff to fix that.

[smeenai]

lld/MachO/Symbols.h

164

D113813

[oontvoo]

In D113167#3128638, @smeenai wrote:

x 10 84.01 89.41 88.64 87.693 1.7424061
+ 10 71.9 74.29 72.63 72.753 0.77734663
Difference at 95.0% confidence
-14.94 +/- 1.26763
-17.0367% +/- 1.44553%
(Student's t, pooled s = 1.34912)

Am I reading that table incorrectly, or do you consider a 17% improvement to be small? :)

No, 17% is correct :)

lld/MachO/Symbols.h

164

D113813

Thanks!

[int3]

Nice perf win! But please break the parallelization into a separate diff...

lld/MachO/Driver.cpp

1470–1479

use braces; block comments should be treated as turning this if into a multiline statement

1471

1474–1475

I would just say that "the former can be exported but the latter cannot". It's pretty much implied that we are following ld64's behavior unless otherwise stated

lld/MachO/SymbolTable.cpp

101–103

I forgot why we chose not to have overridesWeakDef as part of the Defined constructor... is it very ugly to add an additional optional param instead of doing this assignment?

lld/test/MachO/export-options.s

216–217

ultra nit: on line 203 above you put private_extern after weak_def*, here you put it before. would prefer to be consistent :P

[oontvoo]

lld/MachO/SymbolTable.cpp

101–103

good question - dunno why it was written this way.
so make both of them optional?

[int3]

lld/MachO/SymbolTable.cpp

101–103

yeah sounds good

[oontvoo]

addressed review comments

[oontvoo]

Thanks!

lld/MachO/ConcatOutputSection.cpp

for (Reloc &r : reverse(relocs)) {
       thunkInfo.isec->live = true;
 
       StringRef thunkName = saver.save(funcSym->getName() + ".thunk." +
                                        std::to_string(thunkInfo.sequence++));
       r.referent = thunkInfo.sym = symtab->addDefined(
           thunkName, /*file=*/nullptr, thunkInfo.isec, /*value=*/0,
           /*size=*/thunkSize, /*isWeakDef=*/false, /*isPrivateExtern=*/true,
           /*isThumb=*/false, /*isReferencedDynamically=*/false,
-          /*noDeadStrip=*/false);
+          /*noDeadStrip=*/false, /*isWeakDefCanBeHidden=*/false);
       thunkInfo.sym->used = true;
       target->populateThunk(thunkInfo.isec, funcSym);
       finalizeOne(thunkInfo.isec);
       thunks.push_back(thunkInfo.isec);
       ++thunkCount;
     }
   }
   size = isecAddr - addr;

lld/MachO/Driver.cpp

if (config->timeTraceEnabled)
     createSyntheticSections();
     createSyntheticSymbols();
 
     if (!config->exportedSymbols.empty()) {
       parallelForEach(symtab->getSymbols(), [](Symbol *sym) {
         if (auto *defined = dyn_cast<Defined>(sym)) {
           StringRef symbolName = defined->getName();
           if (config->exportedSymbols.match(symbolName)) {
             if (defined->privateExtern) {
+              if (defined->weakDefCanBeHidden) {
+                // weak_def_can_be_hidden symbols behave similarly to
+                // private_extern symbols in most cases, except for when
+                // it is explicitly exported.
+                // The former can be exported but the latter cannot.
+                defined->privateExtern = false;
+              } else {
                 warn("cannot export hidden symbol " + symbolName +
                      "\n>>> defined in " + toString(defined->getFile()));
               }
+            }
           } else {
             defined->privateExtern = true;
           }
         }
       });
     } else if (!config->unexportedSymbols.empty()) {
       parallelForEach(symtab->getSymbols(), [](Symbol *sym) {
         if (auto *defined = dyn_cast<Defined>(sym))

lld/MachO/InputFiles.cpp

if (sym.n_type & N_EXT) {
     // Similarly, merging a symbol
     // that's isPrivateExtern and not isWeakDefCanBeHidden with one
     // that's not isPrivateExtern but isWeakDefCanBeHidden technically
     // should produce one
     // that's not isPrivateExtern but isWeakDefCanBeHidden. That matters
     // with ld64's semantics, because it means the non-private-extern
     // definition will continue to take priority if more private extern
     // 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)
+    // difference between a symbol that's isWeakDefCanBeHidden(autohide) or one
+    // that's privateExtern -- neither makes it into the dynamic symbol table,
+    // unless the autohide symbol is explicitly exported.
+    // But if a symbol is both privateExtern and autohide then it can't
+    // be exported.
+    // So we nullify the autohide flag when privateExtern is present
+    // and promote the symbol to privateExtern when it is not already.
+    if (isWeakDefCanBeHidden && isPrivateExtern)
+      isWeakDefCanBeHidden = false;
+    else if (isWeakDefCanBeHidden)
       isPrivateExtern = true;
-
     return symtab->addDefined(
         name, isec->getFile(), isec, value, size, sym.n_desc & N_WEAK_DEF,
         isPrivateExtern, sym.n_desc & N_ARM_THUMB_DEF,
-        sym.n_desc & REFERENCED_DYNAMICALLY, sym.n_desc & N_NO_DEAD_STRIP);
+        sym.n_desc & REFERENCED_DYNAMICALLY, sym.n_desc & N_NO_DEAD_STRIP,
+        isWeakDefCanBeHidden);
   }
   assert(!isWeakDefCanBeHidden &&
          "weak_def_can_be_hidden on already-hidden symbol?");
   return make<Defined>(
       name, isec->getFile(), isec, value, size, sym.n_desc & N_WEAK_DEF,
       /*isExternal=*/false, /*isPrivateExtern=*/false,
       sym.n_desc & N_ARM_THUMB_DEF, sym.n_desc & REFERENCED_DYNAMICALLY,
       sym.n_desc & N_NO_DEAD_STRIP);
 }
 
 // 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) {
     return symtab->addDefined(
         name, file, nullptr, sym.n_value, /*size=*/0,
         /*isWeakDef=*/false, sym.n_type & N_PEXT, sym.n_desc & N_ARM_THUMB_DEF,
-        /*isReferencedDynamically=*/false, sym.n_desc & N_NO_DEAD_STRIP);
+        /*isReferencedDynamically=*/false, sym.n_desc & N_NO_DEAD_STRIP,
+        /*isWeakDefCanBeHidden=*/false);
   }
   return make<Defined>(name, file, nullptr, sym.n_value, /*size=*/0,
                        /*isWeakDef=*/false,
                        /*isExternal=*/false, /*isPrivateExtern=*/false,
                        sym.n_desc & N_ARM_THUMB_DEF,
                        /*isReferencedDynamically=*/false,
                        sym.n_desc & N_NO_DEAD_STRIP);
 }
static macho::Symbol *createBitcodeSymbol(const lto::InputFile::Symbol &objSym,
   if (objSym.isCommon())
     return symtab->addCommon(name, &file, objSym.getCommonSize(),
                              objSym.getCommonAlignment(), isPrivateExtern);
 
   return symtab->addDefined(name, &file, /*isec=*/nullptr, /*value=*/0,
                             /*size=*/0, objSym.isWeak(), isPrivateExtern,
                             /*isThumb=*/false,
                             /*isReferencedDynamically=*/false,
-                            /*noDeadStrip=*/false);
+                            /*noDeadStrip=*/false,
+                            /*isWeakDefCanBeHidden=*/false);
 }
 
 BitcodeFile::BitcodeFile(MemoryBufferRef mb, StringRef archiveName,
                          uint64_t offsetInArchive)
     : InputFile(BitcodeKind, mb) {
   std::string path = mb.getBufferIdentifier().str();
   // ThinLTO assumes that all MemoryBufferRefs given to it have a unique
   // name. If two members with the same name are provided, this causes a

lld/MachO/SymbolTable.h

  * 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 isThumb,
-                      bool isReferencedDynamically, bool noDeadStrip);
+                      bool isReferencedDynamically, bool noDeadStrip,
+                      bool isWeakDefCanBeHidden);
 
   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 isThumb,
-                                 bool isReferencedDynamically,
-                                 bool noDeadStrip) {
+                                 bool isReferencedDynamically, bool noDeadStrip,
+                                 bool isWeakDefCanBeHidden) {
   Symbol *s;
   bool wasInserted;
   bool overridesWeakDef = false;
   std::tie(s, wasInserted) = insert(name, file);
 
   assert(!isWeakDef || (isa<BitcodeFile>(file) && !isec) ||
          (isa<ObjFile>(file) && file == isec->getFile()));
 
   if (!wasInserted) {
     if (auto *defined = dyn_cast<Defined>(s)) {
       if (isWeakDef) {
-
         // See further comment in createDefined() in InputFiles.cpp
         if (defined->isWeakDef()) {
           defined->privateExtern &= isPrivateExtern;
+          defined->weakDefCanBeHidden &= isWeakDefCanBeHidden;
           defined->referencedDynamically |= isReferencedDynamically;
           defined->noDeadStrip |= noDeadStrip;
         }
         // FIXME: Handle this for bitcode files.
         if (auto concatIsec = dyn_cast_or_null<ConcatInputSection>(isec))
           concatIsec->wasCoalesced = true;
         return defined;
       }
if (auto *defined = dyn_cast<Defined>(s)) {
       dysym->unreference();
     }
     // 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, isThumb, isReferencedDynamically, noDeadStrip);
-  defined->overridesWeakDef = overridesWeakDef;
+      isPrivateExtern, isThumb, isReferencedDynamically, noDeadStrip,
+      overridesWeakDef, isWeakDefCanBeHidden);
   return defined;
 }
 
 Symbol *SymbolTable::addUndefined(StringRef name, InputFile *file,
                                   bool isWeakRef) {
   Symbol *s;
   bool wasInserted;
   std::tie(s, wasInserted) = insert(name, file);
 
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,
                                    bool referencedDynamically) {
-  Defined *s = addDefined(name, nullptr, isec, value, /*size=*/0,
-                          /*isWeakDef=*/false, isPrivateExtern,
-                          /*isThumb=*/false, referencedDynamically,
-                          /*noDeadStrip=*/false);
+  Defined *s =
+      addDefined(name, nullptr, isec, value, /*size=*/0,
+                 /*noDeadStrip=*/false, /*isWeakDefCanBeHidden=*/false);
   s->includeInSymtab = includeInSymtab;
   return s;
 }
 
 enum class Boundary {
   Start,
   End,
 };

lld/MachO/Symbols.h

public:
   // True if an undefined or dylib symbol is used from a live section.
   bool used : 1;
 };
 
 class Defined : public Symbol {
 public:
   Defined(StringRefZ name, InputFile *file, InputSection *isec, uint64_t value,
           uint64_t size, bool isWeakDef, bool isExternal, bool isPrivateExtern,
-          bool isThumb, bool isReferencedDynamically, bool noDeadStrip);
+          bool isThumb, bool isReferencedDynamically, bool noDeadStrip,
+          bool canOverrideWeakDef = false, bool isWeakDefCanBeHidden = false);
 
   bool isWeakDef() const override { return weakDef; }
   bool isExternalWeakDef() const {
     return isWeakDef() && isExternal() && !privateExtern;
   }
   bool isTlv() const override;
 
   bool isExternal() const { return external; }
public:
   // output file's symbol table for tools running later (such as `strip`).
   bool referencedDynamically : 1;
   // Set on symbols that should not be removed by dead code stripping.
   // Set for example on `__attribute__((used))` globals, or on some Objective-C
   // metadata. This is information only for the static linker and not written
   // to the output.
   bool noDeadStrip : 1;
 
+  bool weakDefCanBeHidden : 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
 // of referenced symbols contained within the file. If there are both weak

lld/MachO/Symbols.cpp

 
 uint64_t Symbol::getStubVA() const { return in.stubs->getVA(stubsIndex); }
 uint64_t Symbol::getGotVA() const { return in.got->getVA(gotIndex); }
 uint64_t Symbol::getTlvVA() const { return in.tlvPointers->getVA(gotIndex); }
 
 Defined::Defined(StringRefZ name, InputFile *file, InputSection *isec,
                  uint64_t value, uint64_t size, bool isWeakDef, bool isExternal,
                  bool isPrivateExtern, bool isThumb,
-                 bool isReferencedDynamically, bool noDeadStrip)
+                 bool isReferencedDynamically, bool noDeadStrip,
+                 bool canOverrideWeakDef, bool isWeakDefCanBeHidden)
     : Symbol(DefinedKind, name, file), isec(isec), value(value), size(size),
-      overridesWeakDef(false), privateExtern(isPrivateExtern),
+      overridesWeakDef(canOverrideWeakDef), privateExtern(isPrivateExtern),
       includeInSymtab(true), thumb(isThumb),
       referencedDynamically(isReferencedDynamically), noDeadStrip(noDeadStrip),
-      weakDef(isWeakDef), external(isExternal) {
+      weakDef(isWeakDef), external(isExternal),
+      weakDefCanBeHidden(isWeakDefCanBeHidden) {
   if (isec) {
     isec->symbols.push_back(this);
     // Maintain sorted order.
     for (auto it = isec->symbols.rbegin(), rend = isec->symbols.rend();
          it != rend; ++it) {
       auto next = std::next(it);
       if (next == rend)
         break;

lld/test/MachO/export-options.s

 # RUN: llvm-objdump --macho --exports-trie %t/globby | \
 # RUN:     FileCheck --check-prefix=GLOBBY %s
 
 # GLOBBY-DAG: literal_also
 # GLOBBY-DAG: globby_only
 # GLOBBY-DAG: globby_also
 # GLOBBY-NOT: literal_only
 
+# RUN: llvm-mc -filetype=obj -triple=x86_64-apple-macos \
+# RUN:     %t/autohide.s -o %t/autohide.o
+# RUN: llvm-mc -filetype=obj -triple=x86_64-apple-macos \
+# RUN:     %t/autohide-private-extern.s -o %t/autohide-private-extern.o
+# RUN: llvm-mc -filetype=obj -triple=x86_64-apple-macos \
+# RUN:     %t/glob-private-extern.s -o %t/glob-private-extern.o
+# RUN: llvm-mc -filetype=obj -triple=x86_64-apple-macos \
+# RUN:     %t/weak-private-extern.s -o %t/weak-private-extern.o
+## Test that we can export the autohide symbol but not when it's also
+## private-extern
+# RUN: %lld -dylib -exported_symbol "_foo" %t/autohide.o -o %t/exp-autohide.dylib
+# RUN: llvm-nm -g %t/exp-autohide.dylib | FileCheck %s --check-prefix=EXP-AUTOHIDE
+
+# RUN: not %lld -dylib -exported_symbol "_foo" %t/autohide-private-extern.o \
+# RUN: -o /dev/null  2>&1 | FileCheck %s --check-prefix=AUTOHIDE-PRIVATE
+
+# RUN: not %lld -dylib -exported_symbol "_foo" %t/autohide.o \
+# RUN:   %t/glob-private-extern.o -o /dev/null 2>&1 | \
+# RUN:   FileCheck %s --check-prefix=AUTOHIDE-PRIVATE
+
+# RUN: not %lld -dylib -exported_symbol "_foo" %t/autohide.o \
+# RUN:   %t/weak-private-extern.o -o /dev/null 2>&1 | \
+# RUN:   FileCheck %s --check-prefix=AUTOHIDE-PRIVATE
+
+# EXP-AUTOHIDE: T _foo        
+# AUTOHIDE-PRIVATE: error: cannot export hidden symbol _foo
+        
 #--- default.s
 
 .globl _keep_globl, _hide_globl
 _keep_globl:
   retq
 _hide_globl:
   retq
 .private_extern _private_extern
# globby_only
   globby_also
 
 #--- globbys.txt
 
 # literal_only
   l?ter[aeiou]l_*[^y] # comment
 
   *gl?bby_*
+
+#--- autohide.s
+.globl _foo
+.weak_def_can_be_hidden _foo
+_foo:
+  retq
+
+#--- autohide-private-extern.s
+.globl _foo
+.weak_def_can_be_hidden _foo
+.private_extern _foo
+_foo:
+  retq
+
+#--- glob-private-extern.s
+.global _foo
+.private_extern _foo
+_foo:
+  retq
+
+#--- weak-private-extern.s
+.global _foo
+.weak_definition _foo
+.private_extern _foo        
+_foo:
+  retq