[lld] Proposal and example for changing Atom attributes.

[kledzik]

Summary

Currently, all the nodes in the Atom graph are read only because "const Atom" is used everywhere. But there are a few cases where it would be nice to make minor modifications to atoms. For example, to implement the darwin linker option -expored_symbols_list, we need to be able to change the scope of atoms. ELF has a case where the dynamicExport attribute on an DefinedAtom may need to
be changed.

There is a couple parts to this patch:

1. Two new "notify" methods are added to LinkingContext. They are called by the global symbol table when an atom is added to the symbol table.

2. A setScope() method was added to DefinedAtom. The base implementation just asserts.

3. For mach-o, yaml, and native the setScope() method modifies the scope of the atom. For the native reader, since the atom is just a pointer into a read-only file buffer, the new scope is stored in a side table.

4. The darwin driver has support for the -exported_symbols_list option. The MachOLinkingContext notify methods watch for atoms being added to the symbol table with match the symbol names specified by the exported symbols list

and adjust the scope as appropriate.

If the above goes into lld, then for the ELF issue where particular symbols are currently not be added to the .dynsym table, the fix would be to add a new method setDynamicExport(bool) to DefinedAtom, and in ELFLinkingContext implement the "notify" methods to notice the case (DefinedAtom overriding weak shared library atom) and call setDynamicExport(true) on that DefinedAtom.

[ruiu]

include/lld/Core/DefinedAtom.h

233

I assume we will add more setXX member functions (e.g. setInterposable) when we need it. I don't like to add them now but want to make sure that we are on the same page.

include/lld/Core/LinkingContext.h

230

This function signature made me a bit sad because it's intended to mutate a given atom despite it takes const Atom *. We may probably want to make the atom graph mutable in the first place, because it's no longer actually const because of the addition of this function, so that we don't have to sprinkle const_casts in many places. I don't like to see that change in this CL but it may need to be considered later.

239

I like this function. I'll use it for PE/COFF as well -- when an undefined atom specified by "--undefined" (or "/include" on Windows) is coalesced with a defined atom, I want to copy that atom's deadStrip attribute to a new atom.

lib/Core/DefinedAtom.cpp

87

Does this have to be compared with the current value? I don't think we want to call setScope() if it's not really implemented. It seems to me that we can just put llvm_unreachable here.

lib/ReaderWriter/MachO/MachOLinkingContext.cpp

585

You can early return here if you invert the condition inside if.

602

if (currentlyGlobal && currentlyGlobal == blacklisted)?

[kledzik]

include/lld/Core/DefinedAtom.h

233

Yes. I alluded to that in the overview, that the ELF could add setExportDynamic()

include/lld/Core/LinkingContext.h

230

Yes. For now the graph is 99.9% const, so a few const_cast<> keeps it closer to what we are trying to model. But if we have Passes that need to modify atoms, or lots of places need start modifying atoms, it will probably make sense to remove the const in the zillion places.

lib/Core/DefinedAtom.cpp

87

I started with that, but thought it would be odd to case an abort if something called setScope() and was not actually changing the scope. I'll change it back.

lib/ReaderWriter/MachO/MachOLinkingContext.cpp

585

yes!

602

It reduces to:

if (currentlyGlobal && blacklisted)

[shankarke]

include/lld/Core/LinkingContext.h

230

Various flavors can go over the atoms in pass and set the appropriate attributes right, Am I missing something ? Same with the below function as well.

lib/Core/DefinedAtom.cpp

86

I think visibility and scope are being mixed. While scope needs to be a property of the atom, I am not convinced visibility has to be part of the Atom has well, as visibility is mostly user driven.

lib/ReaderWriter/MachO/MachOLinkingContext.cpp

588

Arent we mixing visibility and scope here ? I think its better to add visibility as a separate attribute.

lib/ReaderWriter/Native/ReaderNative.cpp

888–894

Is this needed anytime ? What kind of usecase is this ?

[atanasyan]

Due time zone difference I just to add my two cents - I like the the target independent part of this patch and agree with Rui that we need to make the atom graph mutable in a separate patch before or after this one. The atom graph will never be immutable from now and we should not hide this fact.

[kledzik]

include/lld/Core/LinkingContext.h

230

Shankar, Passes are run after resolving is done. So a Pass cannot tell, for instance, that a DefineAtom replaced a weak SharedLibraryAtom (which for ELF means the DefinedAtom should be dynamicExport). These hooks give the flavor's LinkingContext a change to record such flavor specific info.

lib/Core/DefinedAtom.cpp

86

What do you mean by "visibility"? The lld model does not have a visibility concept.

The ELF file format started with just two levels of scope (STB_LOCAL and STB_GLOBAL). Later the middle level of scope (linkage unit) was invented, for ELF it is represented as STB_GLOBAL plus STV_HIDDEN on the side.

lib/ReaderWriter/Native/ReaderNative.cpp

888–894

This is to show that we can add setters to the NativeReader even though the native file is mapped read-only. In the future if a flavor needs to use setters and someone stores a input file in native, we need the setters to work.

include/lld/Core/DefinedAtom.h

public:
   /// \brief The visibility of this atom to other atoms.
   ///
   /// C static functions have scope scopeTranslationUnit.  Regular C functions
   /// have scope scopeGlobal.  Functions compiled with visibility=hidden have
   /// scope scopeLinkageUnit so they can be see by other atoms being linked but
   /// not by the OS loader.
   virtual Scope scope() const = 0;
 
+  /// \brief Alter the visibility of this atom.
+  virtual void setScope(Scope);
+
   /// \brief Whether the linker should use direct or indirect access to this
   /// atom.
   virtual Interposable interposable() const = 0;
 
   /// \brief how the linker should handle if multiple atoms have the same name.
   virtual Merge merge() const = 0;
 
   /// \brief The type of this atom, such as code or data.

include/lld/Core/LinkingContext.h

const std::map<std::string, std::string> &getAliases() const {
     return _aliasSymbols;
   }
 
   void setInputGraph(std::unique_ptr<InputGraph> inputGraph) {
     _inputGraph = std::move(inputGraph);
   }
   InputGraph &getInputGraph() const { return *_inputGraph; }
 
+  /// Notify the LinkingContext when an atom is added to the symbol table.
+  /// This is an opportunity for platform specific work to be done such
+  /// as adjusting the atom's scope.
+  virtual void notifySymbolTableAdd(const Atom *atom) const {
+  }
+
+  /// Notify the LinkingContext when the symbol table found a name collision.
+  /// The useNew parameter specifies which the symbol table plans to keep,
+  /// but that can be changed by the LinkingContext.  This is also an
+  /// opportunity for the platform specific processing.
+  virtual void notifySymbolTableCoalesce(const Atom *existingAtom,
+                                         const Atom *newAtom,
+                                         bool &useNew) const {
+  }
+
   /// This method adds undefined symbols specified by the -u option to the to
   /// the list of undefined symbols known to the linker. This option essentially
   /// forces an undefined symbol to be created. You may also need to call
   /// addDeadStripRoot() for the symbol if your platform supports dead
   /// stripping, so that the symbol will not be removed from the output.
   void addInitialUndefinedSymbol(StringRef symbolName) {
     _initialUndefinedSymbols.push_back(symbolName);
   }

include/lld/ReaderWriter/MachOLinkingContext.h

 #ifndef LLD_READER_WRITER_MACHO_LINKING_CONTEXT_H
 #define LLD_READER_WRITER_MACHO_LINKING_CONTEXT_H
 
 #include "lld/Core/LinkingContext.h"
 #include "lld/ReaderWriter/Reader.h"
 #include "lld/ReaderWriter/Writer.h"
 
 #include "llvm/ADT/StringMap.h"
+#include "llvm/ADT/StringSet.h"
 #include "llvm/Support/ErrorHandling.h"
 #include "llvm/Support/MachO.h"
 
 #include <set>
 
 using llvm::MachO::HeaderFileType;
 
 namespace lld {
 
 namespace mach_o {
-class ArchHandler; 
+class ArchHandler;
 class MachODylibFile;
 }
 
 class MachOLinkingContext : public LinkingContext {
 public:
   MachOLinkingContext();
   ~MachOLinkingContext();
 
public:
 
   enum class OS {
     unknown,
     macOSX,
     iOS,
     iOS_simulator
   };
 
+  enum class ExportMode {
+    globals,    // Default, all global symbols exported.
+    whiteList,  // -exported_symbols_list, only listed symbols exported.
+    blackList   // -unexported_symbols_list, no listed symbol exported.
+  };
+
   /// Initializes the context to sane default values given the specified output
   /// file type, arch, os, and minimum os version.  This should be called before
   /// other setXXX() methods.
   void configure(HeaderFileType type, Arch arch, OS os, uint32_t minOSVersion);
 
   void addPasses(PassManager &pm) override;
   bool validateImpl(raw_ostream &diagnostics) override;
   bool createImplicitFiles(std::vector<std::unique_ptr<File>> &) const override;
+  void notifySymbolTableAdd(const Atom *atom) const override;
+  void notifySymbolTableCoalesce(const Atom *existingAtom, const Atom *newAtom,
+                                 bool &useNew) const override;
 
   uint32_t getCPUType() const;
   uint32_t getCPUSubType() const;
 
   bool addEntryPointLoadCommand() const;
   bool addUnixThreadLoadCommand() const;
   bool outputTypeHasEntry() const;
   bool is64Bit() const;
 
   virtual uint64_t pageZeroSize() const { return _pageZeroSize; }
   virtual uint64_t pageSize() const { return _pageSize; }
 
   mach_o::ArchHandler &archHandler() const;
 
   HeaderFileType outputMachOType() const { return _outputMachOType; }
 
   Arch arch() const { return _arch; }
   StringRef archName() const { return nameFromArch(_arch); }
   OS os() const { return _os; }
 
+  ExportMode exportMode() { return _exportMode; }
+  void setExportMode(ExportMode mode) { _exportMode = mode; }
+  void addExportedSymbol(StringRef sym);
+
   bool minOS(StringRef mac, StringRef iOS) const;
   void setDoNothing(bool value) { _doNothing = value; }
   bool doNothing() const { return _doNothing; }
   bool printAtoms() const { return _printAtoms; }
   bool testingFileUsage() const { return _testingFileUsage; }
   const StringRefVector &searchDirs() const { return _searchDirs; }
   const StringRefVector &frameworkDirs() const { return _frameworkDirs; }
   void setSysLibRoots(const StringRefVector &paths);
public:
 
   /// Construct 32-bit value from string "X.Y.Z" where
   /// bits are xxxx.yy.zz.  Largest number is 65535.255.255
   static bool parsePackedVersion(StringRef str, uint32_t &result);
 
 private:
   Writer &writer() const override;
   mach_o::MachODylibFile* loadIndirectDylib(StringRef path) const;
+  void checkExportWhiteList(const DefinedAtom *atom) const;
+  void checkExportBlackList(const DefinedAtom *atom) const;
 
 
   struct ArchInfo {
     StringRef                 archName;
     MachOLinkingContext::Arch arch;
     bool                      littleEndian;
     uint32_t                  cputype;
     uint32_t                  cpusubtype;
private:
   bool _testingFileUsage;
   StringRef _bundleLoader;
   mutable std::unique_ptr<mach_o::ArchHandler> _archHandler;
   mutable std::unique_ptr<Writer> _writer;
   std::vector<SectionAlign> _sectAligns;
   llvm::StringMap<mach_o::MachODylibFile*> _pathToDylibMap;
   std::set<mach_o::MachODylibFile*> _allDylibs;
   mutable std::vector<std::unique_ptr<class MachOFileNode>> _indirectDylibs;
+  ExportMode _exportMode;
+  llvm::StringSet<> _exportedSymbols;
 };
 
 } // end namespace lld
 
 #endif

lib/Core/DefinedAtom.cpp

 //===- DefinedAtom.cpp ------------------------------------------*- C++ -*-===//
 //
 //                             The LLVM Linker
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 
-#include "llvm/Support/ErrorHandling.h"
 
 #include "lld/Core/DefinedAtom.h"
 
+#include "llvm/Support/ErrorHandling.h"
+
+#include <cassert>
 
 namespace lld {
 
 
 DefinedAtom::ContentPermissions DefinedAtom::permissions() const {
   // By default base permissions on content type.
   return permissions(this->contentType());
 }
DefinedAtom::ContentPermissions DefinedAtom::permissions(ContentType type) {
   case typeUnknown:
   case typeTempLTO:
     return permUnknown;
   }
   llvm_unreachable("unknown content type");
 }
 
 
+void DefinedAtom::setScope(Scope s) {
+  assert(s == scope() && "setScope() must be overridden to alter scope");
+}
+
+
 } // namespace
 

lib/Core/SymbolTable.cpp

 }
 
 bool SymbolTable::addByName(const Atom &newAtom) {
   StringRef name = newAtom.name();
   assert(!name.empty());
   const Atom *existing = findByName(name);
   if (existing == nullptr) {
     // Name is not in symbol table yet, add it associate with this atom.
+    _context.notifySymbolTableAdd(&newAtom);
     _nameTable[name] = &newAtom;
     return true;
   }
 
   // Do nothing if the same object is added more than once.
   if (existing == &newAtom)
     return false;
 
case NCR_DupShLib: {
     break;
   }
   case NCR_Error:
     llvm::errs() << "SymbolTable: error while merging " << name << "\n";
     llvm::report_fatal_error("duplicate symbol error");
     break;
   }
 
+  // Give context a chance to change which is kept.
+  _context.notifySymbolTableCoalesce(existing, &newAtom, useNew);
+
   if (useNew) {
     // Update name table to use new atom.
     _nameTable[name] = &newAtom;
     // Add existing atom to replacement table.
     _replacedAtoms[existing] = &newAtom;
   } else {
     // New atom is not being used.  Add it to replacement table.
     _replacedAtoms[&newAtom] = existing;

lib/Driver/DarwinLdDriver.cpp

 }
 
 void addFile(StringRef path, std::unique_ptr<InputGraph> &inputGraph,
              bool forceLoad) {
    inputGraph->addInputElement(std::unique_ptr<InputElement>(
                                           new MachOFileNode(path, forceLoad)));
 }
 
+// Export lists are one symbol per line.  Blank lines are ignored.
+// Trailing comments start with #.
+std::error_code parseExportsList(StringRef exportFilePath,
+                                 MachOLinkingContext &ctx,
+                                 raw_ostream &diagnostics) {
+  // Map in export list file.
+  ErrorOr<std::unique_ptr<MemoryBuffer>> mb =
+                                   MemoryBuffer::getFileOrSTDIN(exportFilePath);
+  if (std::error_code ec = mb.getError())
+    return ec;
+  StringRef buffer = mb->get()->getBuffer();
+  while (!buffer.empty()) {
+    // Split off each line in the file.
+    std::pair<StringRef, StringRef> lineAndRest = buffer.split('\n');
+    StringRef line = lineAndRest.first;
+    // Ignore trailing # comments.
+    std::pair<StringRef, StringRef> symAndComment = line.split('#');
+    StringRef sym = symAndComment.first.trim();
+    if (!sym.empty())
+      ctx.addExportedSymbol(sym);
+    buffer = lineAndRest.second;
+  }
+  return std::error_code();
+}
+
 //
 // There are two variants of the  -filelist option:
 //
 //   -filelist <path>
 // In this variant, the path is to a text file which contains one file path
 // per line.  There are no comments or trimming of whitespace.
 //
 //   -fileList <path>,<dir>
for (auto &alignArg : parsedArgs->filtered(OPT_sectalign)) {
     ctx.addSectionAlignment(segName, sectName, align2);
   }
 
   // Handle -mllvm
   for (auto &llvmArg : parsedArgs->filtered(OPT_mllvm)) {
     ctx.appendLLVMOption(llvmArg->getValue());
   }
 
-  // Handle -print_atoms a
+  // Handle -print_atoms
   if (parsedArgs->getLastArg(OPT_print_atoms))
     ctx.setPrintAtoms();
 
   // Handle -t (trace) option.
   if (parsedArgs->getLastArg(OPT_t))
     ctx.setLogInputFiles(true);
 
   // In -test_file_usage mode, we'll be given an explicit list of paths that
for (auto path : ctx.searchDirs()) {
       diagnostics << "    " << path << '\n';
     }
     diagnostics << "Framework search paths:\n";
     for (auto path : ctx.frameworkDirs()) {
       diagnostics << "    " << path << '\n';
     }
   }
 
+  // Handle -exported_symbols_list <file>
+  for (auto expFile : parsedArgs->filtered(OPT_exported_symbols_list)) {
+    if (ctx.exportMode() == MachOLinkingContext::ExportMode::blackList) {
+      diagnostics << "error: -exported_symbols_list cannot be combined "
+                  << "with -unexported_symbols_list\n";
+       return false;
+    }
+    ctx.setExportMode(MachOLinkingContext::ExportMode::whiteList);
+    if (std::error_code ec = parseExportsList(expFile->getValue(), ctx,
+                                              diagnostics)) {
+      diagnostics << "error: " << ec.message()
+                  << ", processing '-exported_symbols_list "
+                  << expFile->getValue()
+                  << "'\n";
+      return false;
+    }
+  }
+
+  // Handle -unexported_symbols_list <file>
+  for (auto expFile : parsedArgs->filtered(OPT_unexported_symbols_list)) {
+    if (ctx.exportMode() == MachOLinkingContext::ExportMode::whiteList) {
+      diagnostics << "error: -unexported_symbols_list cannot be combined "
+                  << "with -exported_symbols_list\n";
+       return false;
+    }
+    ctx.setExportMode(MachOLinkingContext::ExportMode::blackList);
+    if (std::error_code ec = parseExportsList(expFile->getValue(), ctx,
+                                              diagnostics)) {
+      diagnostics << "error: " << ec.message()
+                  << ", processing '-unexported_symbols_list "
+                  << expFile->getValue()
+                  << "'\n";
+      return false;
+    }
+  }
+
   // Handle input files
   for (auto &arg : *parsedArgs) {
     ErrorOr<StringRef> resolvedPath = StringRef();
     switch (arg->getOption().getID()) {
     default:
       continue;
     case OPT_INPUT:
       addFile(arg->getValue(), inputGraph, globalWholeArchive);
       break;
     case OPT_l:
       resolvedPath = ctx.searchLibrary(arg->getValue());
       if (!resolvedPath) {
         diagnostics << "Unable to find library -l" << arg->getValue() << "\n";
         return false;
       } else if (ctx.testingFileUsage()) {
-       diagnostics << "Found library " << canonicalizePath(resolvedPath.get()) << '\n';
+       diagnostics << "Found library "
+                   << canonicalizePath(resolvedPath.get()) << '\n';
       }
       addFile(resolvedPath.get(), inputGraph, globalWholeArchive);
       break;
     case OPT_framework:
       resolvedPath = ctx.findPathForFramework(arg->getValue());
       if (!resolvedPath) {
         diagnostics << "Unable to find -framework " << arg->getValue() << "\n";
         return false;
       } else if (ctx.testingFileUsage()) {
-        diagnostics << "Found framework " << canonicalizePath(resolvedPath.get()) << '\n';
+        diagnostics << "Found framework "
+                    << canonicalizePath(resolvedPath.get()) << '\n';
       }
       addFile(resolvedPath.get(), inputGraph, globalWholeArchive);
       break;
     case OPT_filelist:
       if (std::error_code ec = parseFileList(arg->getValue(), inputGraph,
                                              ctx, globalWholeArchive,
                                              diagnostics)) {
         diagnostics << "error: " << ec.message()

lib/Driver/DarwinLdOptions.td

def ios_version_min : Separate<["-"], "ios_version_min">,
      MetaVarName<"<version>">,
      HelpText<"Minimum iOS version">, Group<grp_opts>;
 def ios_simulator_version_min : Separate<["-"], "ios_simulator_version_min">,
      MetaVarName<"<version>">,
      HelpText<"Minimum iOS simulator version">, Group<grp_opts>;
 def mllvm : Separate<["-"], "mllvm">,
      MetaVarName<"<option>">,
      HelpText<"Options to pass to LLVM during LTO">, Group<grp_opts>;
+def exported_symbols_list : Separate<["-"], "exported_symbols_list">,
+     MetaVarName<"<file-path>">,
+     HelpText<"Restricts which symbols will be exported">, Group<grp_opts>;
+def unexported_symbols_list : Separate<["-"], "unexported_symbols_list">,
+     MetaVarName<"<file-path>">,
+     HelpText<"Lists symbols that should not be exported">, Group<grp_opts>;
 
 // main executable options
 def grp_main : OptionGroup<"opts">, HelpText<"MAIN EXECUTABLE OPTIONS">;
 def entry : Separate<["-"], "e">,
      MetaVarName<"<entry-name>">,
      HelpText<"entry symbol name">,Group<grp_main>;
 
 // dylib executable options

lib/ReaderWriter/MachO/Atoms.h

public:
   uint64_t size() const override { return _content.size(); }
 
   ContentType contentType() const override { return _contentType; }
 
   StringRef name() const override { return _name; }
 
   Scope scope() const override { return _scope; }
 
+  void setScope(Scope s) override { _scope = s; }
+
   Merge merge() const override { return _merge; }
 
   DeadStripKind deadStrip() const override {
     if (_contentType == DefinedAtom::typeInitializerPtr)
       return deadStripNever;
     if (_contentType == DefinedAtom::typeTerminatorPtr)
       return deadStripNever;
     return deadStripNormal;
void addReference(uint32_t offsetInAtom, uint16_t relocType,
                Reference::KindNamespace ns = Reference::KindNamespace::mach_o) {
     SimpleDefinedAtom::addReference(ns, arch, relocType, offsetInAtom, target, addend);
   }
   
 private:
   const StringRef _name;
   const ArrayRef<uint8_t> _content;
   const ContentType _contentType;
-  const Scope _scope;
+        Scope _scope;
   const Merge _merge;
   const bool _thumb;
 };
 
 class MachODefinedCustomSectionAtom : public MachODefinedAtom {
 public:
   MachODefinedCustomSectionAtom(const File &f, const StringRef name, 
                                 Scope scope, ContentType type, Merge merge,

lib/ReaderWriter/MachO/MachOLinkingContext.cpp

for (const SectionAlign &entry : _sectAligns) {
     if (seg.equals(entry.segmentName) && sect.equals(entry.sectionName)) {
       align2 = entry.align2;
       return true;
     }
   }
   return false;
 }
 
+
+void MachOLinkingContext::addExportedSymbol(StringRef sym) {
+  // FIXME: Support wildcards.
+  _exportedSymbols.insert(sym);
+}
+
+void MachOLinkingContext::checkExportWhiteList(const DefinedAtom *atom) const {
+  bool currentlyGlobal = (atom->scope() == DefinedAtom::scopeGlobal);
+  bool whiteListed = _exportedSymbols.count(atom->name());
+
+  if (currentlyGlobal != whiteListed) {
+    if (currentlyGlobal) {
+      // Reduce scope of symbols not in export list down to "hidden".
+      const_cast<DefinedAtom *>(atom)->setScope(Atom::scopeLinkageUnit);
+    } else {
+      // Warn, if symbol that is supposed to be exported is hidden.
+      llvm::errs() << "warning: cannot export visibility=hidden symbol "
+                   << atom->name() << "\n";
+    }
+  }
+}
+
+void MachOLinkingContext::checkExportBlackList(const DefinedAtom *atom) const {
+  bool currentlyGlobal = (atom->scope() == DefinedAtom::scopeGlobal);
+  bool blackListed = _exportedSymbols.count(atom->name());
+
+  if (currentlyGlobal == blackListed) {
+    if (currentlyGlobal) {
+      // Reduce scope of symbols in unexport list down to "hidden".
+      const_cast<DefinedAtom *>(atom)->setScope(Atom::scopeLinkageUnit);
+    }
+  }
+}
+
+void MachOLinkingContext::notifySymbolTableAdd(const Atom *atom) const {
+  // Nothing to do if command line did not have -[un]exported_symbols_list.
+  switch (_exportMode) {
+  case ExportMode::globals:
+    // Do nothing by default.
+    break;
+  case ExportMode::whiteList:
+    // If -exported_symbols_list specified, see if scope needs adjusting.
+    if (const DefinedAtom *defAtom = dyn_cast<DefinedAtom>(atom))
+      checkExportWhiteList(defAtom);
+    break;
+  case ExportMode::blackList:
+    // If -unexported_symbols_list specified, see if scope needs adjusting.
+    if (const DefinedAtom *defAtom = dyn_cast<DefinedAtom>(atom))
+      checkExportBlackList(defAtom);
+    break;
+  }
+}
+
+void MachOLinkingContext::notifySymbolTableCoalesce(const Atom *existingAtom,
+                                                    const Atom *newAtom,
+                                                    bool &useNew) const {
+  // Nothing to do newAtom is not going to be used by symbol table.
+  if (!useNew)
+    return;
+
+  // Do same checking done when new atom added to symbol table.
+  notifySymbolTableAdd(newAtom);
+}
+
+
 } // end namespace lld

lib/ReaderWriter/Native/ReaderNative.cpp

 
 #include "lld/Core/Atom.h"
 #include "lld/Core/Error.h"
 #include "lld/Core/File.h"
 #include "lld/Core/Simple.h"
 #include "lld/ReaderWriter/Reader.h"
 
 #include "llvm/ADT/ArrayRef.h"
+#include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/Debug.h"
 #include "llvm/Support/ErrorHandling.h"
 #include "llvm/Support/Format.h"
 #include "llvm/Support/MemoryBuffer.h"
 #include "llvm/Support/raw_ostream.h"
 
 #include <memory>
public:
   uint64_t ordinal() const override;
 
   StringRef name() const override;
 
   uint64_t size() const override {
     return _ivarData->contentSize;
   }
 
-  DefinedAtom::Scope scope() const override {
-    return (DefinedAtom::Scope)(attributes().scope);
-  }
+  DefinedAtom::Scope scope() const override;
+
+  void setScope(DefinedAtom::Scope s) override;
 
   DefinedAtom::Interposable interposable() const override {
     return (DefinedAtom::Interposable)(attributes().interposable);
   }
 
   DefinedAtom::Merge merge() const override {
     return (DefinedAtom::Merge)(attributes().merge);
   }
struct IvarArray {
                         elementCount(0) { }
 
     const uint8_t*     arrayStart;
     const uint8_t*     arrayEnd;
     uint32_t           elementSize;
     uint32_t           elementCount;
   };
 
+  struct Overrides {
+    Atom::Scope             scope;
+  };
+  typedef llvm::DenseMap<const NativeDefinedAtomV1 *, Overrides> AtomAttributeOverrides;
+
   std::unique_ptr<MemoryBuffer>   _buffer;
   const NativeFileHeader*         _header;
   AtomArray<DefinedAtom>          _definedAtoms;
   AtomArray<UndefinedAtom>        _undefinedAtoms;
   AtomArray<SharedLibraryAtom>    _sharedLibraryAtoms;
   AtomArray<AbsoluteAtom>         _absoluteAtoms;
   const uint8_t*                  _absAttributes;
   uint32_t                        _absAbsoluteMaxOffset;
   const uint8_t*                  _attributes;
   uint32_t                        _attributesMaxOffset;
   IvarArray                       _referencesV1;
   IvarArray                       _referencesV2;
   const Atom**                    _targetsTable;
   uint32_t                        _targetsTableCount;
   const char*                     _strings;
   uint32_t                        _stringsMaxOffset;
   const Reference::Addend*        _addends;
   uint32_t                        _addendsMaxIndex;
   const uint8_t                  *_contentStart;
   const uint8_t                  *_contentEnd;
+  mutable AtomAttributeOverrides  _atomAttributeOverrides;
 };
 
 inline const lld::File &NativeDefinedAtomV1::file() const {
   return *_file;
 }
 
 inline uint64_t NativeDefinedAtomV1:: ordinal() const {
   const uint8_t* p = reinterpret_cast<const uint8_t*>(_ivarData);
   return p - _file->_definedAtoms._arrayStart;
 }
 
 inline StringRef NativeDefinedAtomV1::name() const {
   return _file->string(_ivarData->nameOffset);
 }
 
+inline DefinedAtom::Scope NativeDefinedAtomV1::scope() const {
+  auto pos = _file->_atomAttributeOverrides.find(this);
+  if (pos == _file->_atomAttributeOverrides.end())
+    return (DefinedAtom::Scope)(attributes().scope);
+  else
+    return pos->second.scope;
+}
+
+inline void NativeDefinedAtomV1::setScope(DefinedAtom::Scope s) {
+  // Do nothing is scope is already requested value.
+  if (scope() == s)
+    return;
+  // Changing scope is rare.  Store changes in side table.
+  _file->_atomAttributeOverrides[this].scope = s;
+}
+
 inline const NativeAtomAttributesV1& NativeDefinedAtomV1::attributes() const {
   return _file->attribute(_ivarData->attributesOffset);
 }
 
 inline ArrayRef<uint8_t> NativeDefinedAtomV1::rawContent() const {
   if (!occupiesDiskSpace())
     return ArrayRef<uint8_t>();
   const uint8_t* p = _file->content(_ivarData->contentOffset,

lib/ReaderWriter/PECOFF/Atoms.h

public:
   static bool classof(const COFFBaseDefinedAtom *atom) {
     return atom->getKind() == Kind::File;
   }
 
   void setAlignment(Alignment val) { _alignment = val; }
   SectionChoice sectionChoice() const override { return sectionCustomRequired; }
   StringRef customSectionName() const override { return _sectionName; }
   Scope scope() const override { return _scope; }
+  void setScope(Scope s) override { _scope = s; }
   ContentType contentType() const override { return _contentType; }
   ContentPermissions permissions() const override { return _permissions; }
   uint64_t ordinal() const override { return _ordinal; }
   Alignment alignment() const override { return _alignment; }
 
   void addAssociate(const DefinedAtom *other) {
     auto *ref = new COFFReference(other, 0, lld::Reference::kindAssociate,
                                   Reference::KindNamespace::all,

lib/ReaderWriter/YAML/ReaderWriterYAML.cpp

const lld::File &fileFromContext(IO &io) {
       assert(info->_file != nullptr);
       return *info->_file;
     }
 
     const lld::File &file() const override { return _file; }
     StringRef name() const override { return _name; }
     uint64_t size() const override { return _size; }
     Scope scope() const override { return _scope; }
+    void setScope(Scope s) override { _scope = s; }
     Interposable interposable() const override { return _interpose; }
     Merge merge() const override { return _merge; }
     ContentType contentType() const override { return _contentType; }
     Alignment alignment() const override { return _alignment; }
     SectionChoice sectionChoice() const override { return _sectionChoice; }
     StringRef customSectionName() const override { return _sectionName; }
     SectionPosition sectionPosition() const override { return _sectionPosition; }
     DeadStripKind deadStrip() const override { return _deadStrip; }

test/mach-o/Inputs/exported_symbols_list.exp

+#
+# For use with exported_symbols_list.yaml
+#
+_foo
+_b
+

test/mach-o/exported_symbols_list.yaml

+# RUN: lld -flavor darwin -arch x86_64 -macosx_version_min 10.8 -dylib \
+# RUN:      %s %p/Inputs/libSystem.yaml -o %t \
+# RUN:      -exported_symbols_list %p/Inputs/exported_symbols_list.exp && \
+# RUN: llvm-nm -m %t | FileCheck %s
+#
+# Test -exported_symbols_list properly changes visibility.
+#
+
+--- !mach-o
+arch:            x86_64
+file-type:       MH_OBJECT
+flags:           [ MH_SUBSECTIONS_VIA_SYMBOLS ]
+sections:
+  - segment:         __TEXT
+    section:         __text
+    type:            S_REGULAR
+    attributes:      [ S_ATTR_PURE_INSTRUCTIONS, S_ATTR_SOME_INSTRUCTIONS ]
+    address:         0x0000000000000000
+    content:         [ 0x55, 0x48, 0x89, 0xE5, 0x5D, 0xC3, 0x55, 0x48, 
+                       0x89, 0xE5, 0x5D, 0xC3 ]
+  - segment:         __DATA
+    section:         __data
+    type:            S_REGULAR
+    attributes:      [  ]
+    alignment:       2
+    address:         0x000000000000000C
+    content:         [ 0x0A, 0x00, 0x00, 0x00, 0x14, 0x00, 0x00, 0x00 ]
+
+global-symbols:
+  - name:            _a
+    type:            N_SECT
+    scope:           [ N_EXT ]
+    sect:            2
+    value:           0x000000000000000C
+  - name:            _b
+    type:            N_SECT
+    scope:           [ N_EXT ]
+    sect:            2
+    value:           0x0000000000000010
+  - name:            _bar
+    type:            N_SECT
+    scope:           [ N_EXT ]
+    sect:            1
+    value:           0x0000000000000006
+  - name:            _foo
+    type:            N_SECT
+    scope:           [ N_EXT ]
+    sect:            1
+    value:           0x0000000000000000
+
+
+...
+
+# CHECK: (__DATA,__data) private external _a
+# CHECK: (__DATA,__data) external _b
+# CHECK: (__TEXT,__text) private external _bar
+# CHECK: (__TEXT,__text) external _foo