[ELF] Remove needsPltAddr in favor of needsCopy

[MaskRay]

Summary

needsPltAddr is equivalent to needsCopy && isFunc. In many places, it is
equivalent to needsCopy because the non-STT_FUNC cases are ruled out.

[peter.smith]

LGTM, although worth waiting a bit to see what others think.

I'm still wondering if we can find a better name than needsCopy; I still haven't come up with a better one. It would need a word that captures the abstraction of a "copy relocation" and "canonical PLT entry" without being so close to one of them that it implies just that one thing yet self descriptive on what the need is for. The best I can come up with is needsCanonicalDefinition but not sure that is better than needsCopy.

[ikudrin]

needsCopy is declared as a temporary flag and, according to its comment, should not be used after postScanRelocations(). Because the usage is extended, that should be reflected in the comment.

[MaskRay]

Rebase after needsCopy has a better comment.

[ikudrin]

LGTM

lld/ELF/Arch/AArch64.cpp

const uint8_t pacBr[] = {
       0x20, 0x02, 0x1f, 0xd6   // br   x17
   };
   const uint8_t stdBr[] = {
       0x20, 0x02, 0x1f, 0xd6,  // br   x17
       0x1f, 0x20, 0x03, 0xd5   // nop
   };
   const uint8_t nopData[] = { 0x1f, 0x20, 0x03, 0xd5 }; // nop
 
-  // needsPltAddr indicates a non-ifunc canonical PLT entry whose address may
+  // needsCopy indicates a non-ifunc canonical PLT entry whose address may
   // escape to shared objects. isInIplt indicates a non-preemptible ifunc. Its
   // address may escape if referenced by a direct relocation. The condition is
   // conservative.
-  bool hasBti = btiHeader && (sym.needsPltAddr || sym.isInIplt);
+  bool hasBti = btiHeader && (sym.needsCopy || sym.isInIplt);
   if (hasBti) {
     memcpy(buf, btiData, sizeof(btiData));
     buf += sizeof(btiData);
     pltEntryAddr += sizeof(btiData);
   }
 
   uint64_t gotPltEntryAddr = sym.getGotPltVA();
   memcpy(buf, addrInst, sizeof(addrInst));

lld/ELF/MapFile.cpp

 
 // Returns a list of all symbols that we want to print out.
 static std::vector<Defined *> getSymbols() {
   std::vector<Defined *> v;
   for (InputFile *file : objectFiles)
     for (Symbol *b : file->getSymbols())
       if (auto *dr = dyn_cast<Defined>(b))
         if (!dr->isSection() && dr->section && dr->section->isLive() &&
-            (dr->file == file || dr->needsPltAddr || dr->section->bss))
+            (dr->file == file || dr->needsCopy || dr->section->bss))
           v.push_back(dr);
   return v;
 }
 
 // Returns a map from sections to their symbols.
 static SymbolMapTy getSectionSyms(ArrayRef<Defined *> syms) {
   SymbolMapTy ret;
   for (Defined *dr : syms)

lld/ELF/Relocations.cpp

if (sym.needsCopy) {
         // iterations aliases won't cause redundant copies.
         assert(!sym.needsCopy);
       } else {
         assert(sym.isFunc() && sym.needsPlt);
         if (!sym.isDefined()) {
           replaceWithDefined(
               sym, in.plt,
               target->pltHeaderSize + target->pltEntrySize * sym.pltIndex, 0);
+          sym.needsCopy = true;
           if (config->emachine == EM_PPC) {
             // PPC32 canonical PLT entries are at the beginning of .glink
             cast<Defined>(sym).value = in.plt->headerSize;
             in.plt->headerSize += 16;
             cast<PPC32GlinkSection>(in.plt)->canonical_plts.push_back(&sym);
           }
         }
-        sym.needsPltAddr = true;
       }
     }
   };
   for (Symbol *sym : symtab->symbols())
     fn(*sym);
 
   // Local symbols may need the aforementioned non-preemptible ifunc and GOT
   // handling. They don't need regular PLT.

lld/ELF/Symbols.h

 
 protected:
   Symbol(Kind k, InputFile *file, StringRefZ name, uint8_t binding,
          uint8_t stOther, uint8_t type)
       : file(file), nameData(name.data), nameSize(name.size), binding(binding),
         type(type), stOther(stOther), symbolKind(k), visibility(stOther & 3),
         isUsedInRegularObj(!file || file->kind() == InputFile::ObjKind),
         exportDynamic(isExportDynamic(k, visibility)), inDynamicList(false),
-        canInline(false), referenced(false), traced(false), needsPltAddr(false),
-        isInIplt(false), gotInIgot(false), isPreemptible(false),
-        used(!config->gcSections), needsTocRestore(false), scriptDefined(false),
-        needsCopy(false), needsGot(false), needsPlt(false),
-        hasDirectReloc(false) {}
+        canInline(false), referenced(false), traced(false), isInIplt(false),
+        gotInIgot(false), isPreemptible(false), used(!config->gcSections),
+        needsTocRestore(false), scriptDefined(false), needsCopy(false),
+        needsGot(false), needsPlt(false), hasDirectReloc(false) {}
 
 public:
-  // True the symbol should point to its PLT entry.
-  // For SharedSymbol only.
-  uint8_t needsPltAddr : 1;
-
   // True if this symbol is in the Iplt sub-section of the Plt and the Igot
   // sub-section of the .got.plt or .got.
   uint8_t isInIplt : 1;
 
   // True if this symbol needs a GOT entry and its GOT entry is actually in
   // Igot. This will be true only for certain non-preemptible ifuncs.
   uint8_t gotInIgot : 1;
 

lld/ELF/Symbols.cpp

case Symbol::DefinedKind: {
     // symbols has the `STO_MIPS_MICROMIPS` flag in the `st_other`
     // field. Unfortunately, the `MIPS::relocate()` method has
     // a symbol value only. To pass type of the symbol (regular/microMIPS)
     // to that routine as well as other places where we write
     // a symbol value as-is (.dynamic section, `Elf_Ehdr::e_entry`
     // field etc) do the same trick as compiler uses to mark microMIPS
     // for CPU - set the less-significant bit.
     if (config->emachine == EM_MIPS && isMicroMips() &&
-        ((sym.stOther & STO_MIPS_MICROMIPS) || sym.needsPltAddr))
+        ((sym.stOther & STO_MIPS_MICROMIPS) || sym.needsCopy))
       va |= 1;
 
     if (d.isTls() && !config->relocatable) {
       // Use the address of the TLS segment's first section rather than the
       // segment's address, because segment addresses aren't initialized until
       // after sections are finalized. (e.g. Measuring the size of .rela.dyn
       // for Android relocation packing requires knowing TLS symbol addresses
       // during section finalization.)

lld/ELF/SyntheticSections.cpp

if (!config->defineCommon)
     if (auto *d = dyn_cast<Defined>(sym))
       return dyn_cast_or_null<BssSection>(d->section);
   return nullptr;
 }
 
 static uint32_t getSymSectionIndex(Symbol *sym) {
   if (getCommonSec(sym))
     return SHN_COMMON;
-  if (!isa<Defined>(sym) || sym->needsPltAddr)
+  assert(!(sym->needsCopy && sym->isObject()));
+  if (!isa<Defined>(sym) || sym->needsCopy)
     return SHN_UNDEF;
   if (const OutputSection *os = sym->getOutputSection())
     return os->sectionIndex >= SHN_LORESERVE ? (uint32_t)SHN_XINDEX
                                              : os->sectionIndex;
   return SHN_ABS;
 }
 
 // Write the internal symbol table contents to the output symbol table.
template <class ELFT> void SymbolTableSection<ELFT>::writeTo(uint8_t *buf) {
   // pointer equality by STO_MIPS_PLT flag. That is necessary to help
   // dynamic linker distinguish such symbols and MIPS lazy-binding stubs.
   // https://sourceware.org/ml/binutils/2008-07/txt00000.txt
   if (config->emachine == EM_MIPS) {
     auto *eSym = reinterpret_cast<Elf_Sym *>(buf);
 
     for (SymbolTableEntry &ent : symbols) {
       Symbol *sym = ent.sym;
-      if (sym->isInPlt() && sym->needsPltAddr)
+      if (sym->isInPlt() && sym->needsCopy)
         eSym->st_other |= STO_MIPS_PLT;
       if (isMicroMips()) {
         // We already set the less-significant bit for symbols
         // marked by the `STO_MIPS_MICROMIPS` flag and for microMIPS PLT
         // records. That allows us to distinguish such symbols in
         // the `MIPS<ELFT>::relocate()` routine. Now we should
         // clear that bit for non-dynamic symbol table, so tools
         // like `objdump` will be able to deal with a correct
         // symbol position.
         if (sym->isDefined() &&
-            ((sym->stOther & STO_MIPS_MICROMIPS) || sym->needsPltAddr)) {
+            ((sym->stOther & STO_MIPS_MICROMIPS) || sym->needsCopy)) {
           if (!strTabSec.isDynamic())
             eSym->st_value &= ~1;
           eSym->st_other |= STO_MIPS_MICROMIPS;
         }
       }
       if (config->relocatable)
         if (auto *d = dyn_cast<Defined>(sym))
           if (isMipsPIC<ELFT>(d))