Consolidate global variables to a single place

[ruiu]

Summary

The aim of this patch is the same as https://reviews.llvm.org/D70421
but I took a different approach. I just moved all global variables
to Driver.cpp so that we don't forget to initialize them on startup.

[grimar]

What about static variables?

Like:

struct Out {
  static uint8_t *bufferStart;
  static uint8_t first;
  static PhdrEntry *tlsPhdr;
  static OutputSection *elfHeader;
  static OutputSection *programHeaders;
  static OutputSection *preinitArray;
  static OutputSection *initArray;
  static OutputSection *finiArray;
};

or SharedFile::vernauxNum

or

struct ElfSym {
  // __bss_start
  static Defined *bss;

  // etext and _etext
  static Defined *etext1;
  static Defined *etext2;

or things like

static bool isInGroup;
static uint32_t nextGroupId;

I think would be good to get rid of all such statics.
For Out we should probably have Out out, just like for InStruct in.
Some others probably can be replaced with a global variables (or may be eliminated).

That's why I've suggested a separate file for them. I think it worth to push them out and
comment them well. We should probably only allow using statics for simple situations like

if (config->andFeatures & (GNU_PROPERTY_AARCH64_FEATURE_1_BTI |
                           GNU_PROPERTY_AARCH64_FEATURE_1_PAC)) {
  static AArch64BtiPac t;
  return &t;
}
static AArch64 t;
return &t;

and restrict them for everything else in favor of globals.
We'll continue to have a mess until isolate all things that requires re-initialization I believe.

[peter.smith]

There are a couple of downsides to putting all the globals in one place. I think the LLD codebase is probably small enough for them to not be too serious:

• It couples pretty much every file to a single place. Touching this file means recompiling everything.
• It can make the code harder to reason about as any file can see all the globals, and can potentially modify them. This is a possible argument for not promoting statics to globals as the scope of a static is much easier to reason about.

I don't have a strong opinion about which is best. I can see the above patch making it easier to remember to initialize globals, and I think that there is already enough global state via config to make this only a small impact to understanding the code.

[MaskRay]

There are some static variables and static data members. Moving them to a central place will change their linkage from internal to external, and thus increase their scope. An alternative is to declare initFoobar() for files that define variables with static storage duration and call all initFoobar() in a central place.

lld/COFF/Driver.cpp

 using namespace llvm;
 using namespace llvm::object;
 using namespace llvm::COFF;
 using llvm::sys::Process;
 
 namespace lld {
 namespace coff {
 
+static void initGlobals();
 static Timer inputFileTimer("Input File Reading", Timer::root());
 
+// If you add a new global variable, make sure that you add initialization code
+// to initGlobals() too.
 Configuration *config;
 LinkerDriver *driver;
+SymbolTable *symtab;
+std::vector<OutputSection *> outputSections;
 
 bool link(ArrayRef<const char *> args, bool canExitEarly, raw_ostream &stdoutOS,
           raw_ostream &stderrOS) {
   lld::stdoutOS = &stdoutOS;
   lld::stderrOS = &stderrOS;
 
   errorHandler().logName = args::getFilenameWithoutExe(args[0]);
   errorHandler().errorLimitExceededMsg =
       "too many errors emitted, stopping now"
       " (use /errorlimit:0 to see all errors)";
   errorHandler().exitEarly = canExitEarly;
   stderrOS.enable_colors(stderrOS.has_colors());
 
+  initGlobals();
+
   config = make<Configuration>();
   symtab = make<SymbolTable>();
   driver = make<LinkerDriver>();
 
   driver->link(args);
 
   // Call exit() if we can to avoid calling destructors.
   if (canExitEarly)
     exitLld(errorCount() ? 1 : 0);
 
   freeArena();
   ObjFile::instances.clear();
   ImportFile::instances.clear();
   BitcodeFile::instances.clear();
   memset(MergeChunk::instances, 0, sizeof(MergeChunk::instances));
   return !errorCount();
 }
 
+// Reset globals. We want to make sure that on every linker invocation, we start
+// with fresh global variables, so that users who link lld as a library can call
+// link() multiple times from the same process.
+static void initGlobals() {
+  config = nullptr;
+  driver = nullptr;
+  symtab = nullptr;
+  outputSections.clear();
+}
+
 // Parse options of the form "old;new".
 static std::pair<StringRef, StringRef> getOldNewOptions(opt::InputArgList &args,
                                                         unsigned id) {
   auto *arg = args.getLastArg(id);
   if (!arg)
     return {"", ""};
 
   StringRef s = arg->getValue();

lld/COFF/SymbolTable.cpp

 
 using namespace llvm;
 
 namespace lld {
 namespace coff {
 
 static Timer ltoTimer("LTO", Timer::root());
 
-SymbolTable *symtab;
-
 void SymbolTable::addFile(InputFile *file) {
   log("Reading " + toString(file));
   file->parse();
 
   MachineTypes mt = file->getMachineType();
   if (config->machine == IMAGE_FILE_MACHINE_UNKNOWN) {
     config->machine = mt;
   } else if (mt != IMAGE_FILE_MACHINE_UNKNOWN && config->machine != mt) {

lld/COFF/Writer.h

 #include "llvm/ADT/StringRef.h"
 #include "llvm/Object/COFF.h"
 #include <chrono>
 #include <cstdint>
 #include <vector>
 
 namespace lld {
 namespace coff {
+
+class OutputSection;
+
+extern std::vector<OutputSection *> outputSections;
+
 static const int pageSize = 4096;
 
 void writeResult();
 
 class PartialSection {
 public:
   PartialSection(StringRef n, uint32_t chars)
       : name(n), characteristics(chars) {}

lld/COFF/Writer.cpp

 static_assert(sizeof(dosProgram) % 8 == 0,
               "DOSProgram size must be multiple of 8");
 
 static const int dosStubSize = sizeof(dos_header) + sizeof(dosProgram);
 static_assert(dosStubSize % 8 == 0, "DOSStub size must be multiple of 8");
 
 static const int numberOfDataDirectory = 16;
 
-// Global vector of all output sections. After output sections are finalized,
-// this can be indexed by Chunk::getOutputSection.
-static std::vector<OutputSection *> outputSections;
-
 OutputSection *Chunk::getOutputSection() const {
   return osidx == 0 ? nullptr : outputSections[osidx - 1];
 }
 
 namespace {
 
 class DebugDirectoryChunk : public NonSectionChunk {
 public:

lld/ELF/Driver.cpp

 using namespace llvm::ELF;
 using namespace llvm::object;
 using namespace llvm::sys;
 using namespace llvm::support;
 
 namespace lld {
 namespace elf {
 
+// If you add a new global variable, make sure that you add initialization code
+// to initGlobals() too.
 Configuration *config;
+InStruct in;
 LinkerDriver *driver;
+LinkerScript *script;
+Partition *mainPart;
+SymbolTable *symtab;
+const TargetInfo *target;
+std::unique_ptr<llvm::TarWriter> tar;
+std::vector<BinaryFile *> binaryFiles;
+std::vector<BitcodeFile *> bitcodeFiles;
+std::vector<InputFile *> objectFiles;
+std::vector<InputSectionBase *> inputSections;
+std::vector<LazyObjFile *> lazyObjFiles;
+std::vector<OutputSection *> outputSections;
+std::vector<Partition> partitions;
+std::vector<SharedFile *> sharedFiles;
 
+static void initGlobals();
 static void setConfigs(opt::InputArgList &args);
 static void readConfigs(opt::InputArgList &args);
 
 bool link(ArrayRef<const char *> args, bool canExitEarly, raw_ostream &stdoutOS,
           raw_ostream &stderrOS) {
   lld::stdoutOS = &stdoutOS;
   lld::stderrOS = &stderrOS;
 
   errorHandler().logName = args::getFilenameWithoutExe(args[0]);
   errorHandler().errorLimitExceededMsg =
       "too many errors emitted, stopping now (use "
       "-error-limit=0 to see all errors)";
   errorHandler().exitEarly = canExitEarly;
   stderrOS.enable_colors(stderrOS.has_colors());
 
-  inputSections.clear();
-  outputSections.clear();
-  binaryFiles.clear();
-  bitcodeFiles.clear();
-  objectFiles.clear();
-  sharedFiles.clear();
+  initGlobals();
 
   config = make<Configuration>();
   driver = make<LinkerDriver>();
   script = make<LinkerScript>();
   symtab = make<SymbolTable>();
 
   tar = nullptr;
   memset(&in, 0, sizeof(in));
bool link(ArrayRef<const char *> args, bool canExitEarly, raw_ostream &stdoutOS,
   // for all globally-allocated objects is not negligible.
   if (canExitEarly)
     exitLld(errorCount() ? 1 : 0);
 
   freeArena();
   return !errorCount();
 }
 
+// Reset globals. We want to make sure that on every linker invocation, we start
+// with fresh global variables, so that users who link lld as a library can call
+// link() multiple times from the same process.
+static void initGlobals() {
+  config = nullptr;
+  memset(&in, 0, sizeof(in));
+  driver = nullptr;
+  script = nullptr;
+  mainPart = nullptr;
+  symtab = nullptr;
+  target = nullptr;
+  tar.reset();
+  binaryFiles.clear();
+  bitcodeFiles.clear();
+  objectFiles.clear();
+  inputSections.clear();
+  lazyObjFiles.clear();
+  outputSections.clear();
+  partitions.clear();
+  sharedFiles.clear();
+}
+
 // Parses a linker -m option.
 static std::tuple<ELFKind, uint16_t, uint8_t> parseEmulation(StringRef emul) {
   uint8_t osabi = 0;
   StringRef s = emul;
   if (s.endswith("_fbsd")) {
     s = s.drop_back(5);
     osabi = ELFOSABI_FREEBSD;
   }

lld/ELF/InputFiles.cpp

else
       f->toStringCache = (f->archiveName + "(" + f->getName() + ")").str();
   }
   return f->toStringCache;
 }
 
 namespace elf {
 bool InputFile::isInGroup;
 uint32_t InputFile::nextGroupId;
-std::vector<BinaryFile *> binaryFiles;
-std::vector<BitcodeFile *> bitcodeFiles;
-std::vector<LazyObjFile *> lazyObjFiles;
-std::vector<InputFile *> objectFiles;
-std::vector<SharedFile *> sharedFiles;
-
-std::unique_ptr<TarWriter> tar;
 
 static ELFKind getELFKind(MemoryBufferRef mb, StringRef archiveName) {
   unsigned char size;
   unsigned char endian;
   std::tie(size, endian) = getElfArchType(mb.getBuffer());
 
   auto report = [&](StringRef msg) {
     StringRef filename = mb.getBufferIdentifier();

lld/ELF/InputSection.cpp

 
 namespace lld {
 // Returns a string to construct an error message.
 std::string toString(const elf::InputSectionBase *sec) {
   return (toString(sec->file) + ":(" + sec->name + ")").str();
 }
 
 namespace elf {
-std::vector<InputSectionBase *> inputSections;
 
 template <class ELFT>
 static ArrayRef<uint8_t> getSectionContents(ObjFile<ELFT> &file,
                                             const typename ELFT::Shdr &hdr) {
   if (hdr.sh_type == SHT_NOBITS)
     return makeArrayRef<uint8_t>(nullptr, hdr.sh_size);
   return check(file.getObj().getSectionContents(&hdr));
 }

lld/ELF/LinkerScript.cpp

 
 using namespace llvm;
 using namespace llvm::ELF;
 using namespace llvm::object;
 using namespace llvm::support::endian;
 
 namespace lld {
 namespace elf {
-LinkerScript *script;
 
 static uint64_t getOutputSectionVA(SectionBase *sec) {
   OutputSection *os = sec->getOutputSection();
   assert(os && "input section has no output section assigned");
   return os ? os->addr : 0;
 }
 
 uint64_t ExprValue::getValue() const {

lld/ELF/OutputSections.cpp

 uint8_t Out::first;
 PhdrEntry *Out::tlsPhdr;
 OutputSection *Out::elfHeader;
 OutputSection *Out::programHeaders;
 OutputSection *Out::preinitArray;
 OutputSection *Out::initArray;
 OutputSection *Out::finiArray;
 
-std::vector<OutputSection *> outputSections;
-
 uint32_t OutputSection::getPhdrFlags() const {
   uint32_t ret = 0;
   if (config->emachine != EM_ARM || !(flags & SHF_ARM_PURECODE))
     ret |= PF_R;
   if (flags & SHF_WRITE)
     ret |= PF_W;
   if (flags & SHF_EXECINSTR)
     ret |= PF_X;

lld/ELF/SymbolTable.cpp

 #include "llvm/ADT/STLExtras.h"
 
 using namespace llvm;
 using namespace llvm::object;
 using namespace llvm::ELF;
 
 namespace lld {
 namespace elf {
-SymbolTable *symtab;
 
 void SymbolTable::wrap(Symbol *sym, Symbol *real, Symbol *wrap) {
   // Swap symbols as instructed by -wrap.
   int &idx1 = symMap[CachedHashStringRef(sym->getName())];
   int &idx2 = symMap[CachedHashStringRef(real->getName())];
   int &idx3 = symMap[CachedHashStringRef(wrap->getName())];
 
   idx2 = idx1;

lld/ELF/SyntheticSections.cpp

SyntheticSection *next =
         i == partitions.size() - 1 ? in.partEnd : partitions[i + 1].elfHeader;
     write32(buf + 8, next->getVA() - partitions[i].elfHeader->getVA());
 
     va += 12;
     buf += 12;
   }
 }
 
-InStruct in;
-
-std::vector<Partition> partitions;
-Partition *mainPart;
-
 template GdbIndexSection *GdbIndexSection::create<ELF32LE>();
 template GdbIndexSection *GdbIndexSection::create<ELF32BE>();
 template GdbIndexSection *GdbIndexSection::create<ELF64LE>();
 template GdbIndexSection *GdbIndexSection::create<ELF64BE>();
 
 template void splitSections<ELF32LE>();
 template void splitSections<ELF32BE>();
 template void splitSections<ELF64LE>();

lld/ELF/Target.cpp

 std::string toString(elf::RelType type) {
   StringRef s = getELFRelocationTypeName(elf::config->emachine, type);
   if (s == "Unknown")
     return ("Unknown (" + Twine(type) + ")").str();
   return s;
 }
 
 namespace elf {
-const TargetInfo *target;
 
 TargetInfo *getTarget() {
   switch (config->emachine) {
   case EM_386:
   case EM_IAMCU:
     return getX86TargetInfo();
   case EM_AARCH64:
     return getAArch64TargetInfo();