compute isPreemtible only once

[• rafael]

Summary

This is probably a small optimization, but the main motivation is having a way of fixing pr34053 that doesn't require a hash lookup in isPreempitible.

[ruiu]

ELF/Symbols.cpp

138–139

Is it possible to define this function as a non-member static function? if it is doable, it is preferable, because it makes clear that this function does not depend on private members.

[• rafael]

Make computeIsPreemptible a static function.

[ruiu]

LGTM

[smeenai]

SVN r310602.

ELF/Relocations.cpp

 // The pair of GOT entries created are of the form
 // GOT[e0] Module Index (Used to find pointer to TLS block at run-time)
 // GOT[e1] Offset of symbol in TLS block
 template <class ELFT>
 static unsigned handleARMTlsRelocation(uint32_t Type, SymbolBody &Body,
                                        InputSectionBase &C, uint64_t Offset,
                                        int64_t Addend, RelExpr Expr) {
   // The Dynamic TLS Module Index Relocation for a symbol defined in an
-  // executable is always 1. If the target Symbol is not preemtible then
+  // executable is always 1. If the target Symbol is not preemptible then
   // we know the offset into the TLS block at static link time.
   bool NeedDynId = Body.isPreemptible() || Config->Shared;
   bool NeedDynOff = Body.isPreemptible();
 
   auto AddTlsReloc = [&](uint64_t Off, uint32_t Type, SymbolBody *Dest,
                          bool Dyn) {
     if (Dyn)
       In<ELFT>::RelaDyn->addReloc({Type, InX::Got, Off, false, Dest, 0});
template <class ELFT> static void addCopyRelSymbol(SharedSymbol *SS) {
   BssSection *Sec = IsReadOnly ? InX::BssRelRo : InX::Bss;
   uint64_t Off = Sec->reserveSpace(SymSize, SS->getAlignment<ELFT>());
 
   // Look through the DSO's dynamic symbol table for aliases and create a
   // dynamic symbol for each one. This causes the copy relocation to correctly
   // interpose any aliases.
   for (SharedSymbol *Sym : getSymbolsAt<ELFT>(SS)) {
     Sym->CopyRelSec = Sec;
+    Sym->IsPreemptible = false;
     Sym->CopyRelSecOff = Off;
     Sym->symbol()->IsUsedInRegularObj = true;
   }
 
   In<ELFT>::RelaDyn->addReloc({Target->CopyRel, Sec, Off, false, SS, 0});
 }
 
 static void errorOrWarn(const Twine &Msg) {
if (Body.isFunc()) {
     // a direct reference to a so symbol by creating an undefined symbol with a
     // non zero st_value. Seeing that, the dynamic linker resolves the symbol to
     // the value of the symbol we created. This is true even for got entries, so
     // pointer equality is maintained. To avoid an infinite loop, the only entry
     // that points to the real function is a dedicated got entry used by the
     // plt. That is identified by special relocation types (R_X86_64_JUMP_SLOT,
     // R_386_JMP_SLOT, etc).
     Body.NeedsPltAddr = true;
+    Body.IsPreemptible = false;
     return toPlt(Expr);
   }
 
   errorOrWarn("symbol '" + toString(Body) + "' defined in " +
               toString(Body.getFile()) + " has no type");
   return Expr;
 }
 

ELF/Symbols.h

bool isLazy() const {
     return SymbolKind == LazyArchiveKind || SymbolKind == LazyObjectKind;
   }
   bool isShared() const { return SymbolKind == SharedKind; }
   bool isInCurrentDSO() const {
     return !isUndefined() && !isShared() && !isLazy();
   }
   bool isLocal() const { return IsLocal; }
   InputFile *getFile() const;
-  bool isPreemptible() const;
+  bool isPreemptible() const { return IsPreemptible; }
   StringRef getName() const { return Name; }
   uint8_t getVisibility() const { return StOther & 0x3; }
   void parseSymbolVersion();
   void copy(SymbolBody *Other);
 
   bool isInGot() const { return GotIndex != -1U; }
   bool isInPlt() const { return PltIndex != -1U; }
 
public:
   unsigned Is32BitMipsGot : 1;
 
   // True if this symbol is in the Iplt sub-section of the Plt.
   unsigned IsInIplt : 1;
 
   // True if this symbol is in the Igot sub-section of the .got.plt or .got.
   unsigned IsInIgot : 1;
 
+  unsigned IsPreemptible : 1;
+
   // The following fields have the same meaning as the ELF symbol attributes.
   uint8_t Type;    // symbol type
   uint8_t StOther; // st_other field value
 
   // The Type field may also have this value. It means that we have not yet seen
   // a non-Lazy symbol with this name, so we don't know what its type is. The
   // Type field is normally set to this value for Lazy symbols unless we saw a
   // weak undefined symbol first, in which case we need to remember the original

ELF/Symbols.cpp

static uint64_t getSymVA(const SymbolBody &Body, int64_t &Addend) {
   }
   llvm_unreachable("invalid symbol kind");
 }
 
 SymbolBody::SymbolBody(Kind K, StringRefZ Name, bool IsLocal, uint8_t StOther,
                        uint8_t Type)
     : SymbolKind(K), NeedsPltAddr(false), IsLocal(IsLocal),
       IsInGlobalMipsGot(false), Is32BitMipsGot(false), IsInIplt(false),
-      IsInIgot(false), Type(Type), StOther(StOther), Name(Name) {}
+      IsInIgot(false), IsPreemptible(false), Type(Type), StOther(StOther),
+      Name(Name) {}
 
 InputFile *SymbolBody::getFile() const {
   if (isLocal())
     return cast<InputSectionBase>(cast<DefinedRegular>(this)->Section)->File;
   return symbol()->File;
 }
 
-// Returns true if a symbol can be replaced at load-time by a symbol
-// with the same name defined in other ELF executable or DSO.
-bool SymbolBody::isPreemptible() const {
-  if (isLocal())
-    return false;
-
-  // Shared symbols resolve to the definition in the DSO. The exceptions are
-  // symbols with copy relocations (which resolve to .bss) or preempt plt
-  // entries (which resolve to that plt entry).
-  if (auto *SS = dyn_cast<SharedSymbol>(this))
-    return !SS->CopyRelSec && !NeedsPltAddr;
-
-  // Only symbols that appear in dynsym can be preempted.
-  if (!symbol()->includeInDynsym())
-    return false;
-
-  // Only default visibility symbols can be preempted.
-  if (symbol()->Visibility != STV_DEFAULT)
-    return false;
-
-  // Undefined symbols in non-DSOs are usually just an error, so it
-  // doesn't matter whether we return true or false here. However, if
-  // -unresolved-symbols=ignore-all is specified, undefined symbols in
-  // executables are automatically exported so that the runtime linker
-  // can try to resolve them. In that case, they is preemptible. So, we
-  // return true for an undefined symbol in case the option is specified.
-  if (!Config->Shared)
-    return isUndefined();
-
-  // -Bsymbolic means that definitions are not preempted.
-  if (Config->Bsymbolic || (Config->BsymbolicFunctions && isFunc()))
-    return !isDefined();
-  return true;
-}
-
 // Overwrites all attributes with Other's so that this symbol becomes
 // an alias to Other. This is useful for handling some options such as
 // --wrap.
 void SymbolBody::copy(SymbolBody *Other) {
   memcpy(symbol()->Body.buffer, Other->symbol()->Body.buffer,
          sizeof(Symbol::Body));
 }
 
 uint64_t SymbolBody::getVA(int64_t Addend) const {

ELF/Writer.cpp

if (OS->Commands.empty()) {
           return Sec == OS;
         return false;
       });
       Cmds.erase(I, Cmds.end());
     }
   }
 }
 
+// Returns true if a symbol can be replaced at load-time by a symbol
+// with the same name defined in other ELF executable or DSO.
+static bool computeIsPreemptible(const SymbolBody &B) {
+  assert(!B.isLocal());
+  // Shared symbols resolve to the definition in the DSO. The exceptions are
+  // symbols with copy relocations (which resolve to .bss) or preempt plt
+  // entries (which resolve to that plt entry).
+  if (auto *SS = dyn_cast<SharedSymbol>(&B))
+    return !SS->CopyRelSec && !SS->NeedsPltAddr;
+
+  // Only symbols that appear in dynsym can be preempted.
+  if (!B.symbol()->includeInDynsym())
+    return false;
+
+  // Only default visibility symbols can be preempted.
+  if (B.symbol()->Visibility != STV_DEFAULT)
+    return false;
+
+  // Undefined symbols in non-DSOs are usually just an error, so it
+  // doesn't matter whether we return true or false here. However, if
+  // -unresolved-symbols=ignore-all is specified, undefined symbols in
+  // executables are automatically exported so that the runtime linker
+  // can try to resolve them. In that case, they is preemptible. So, we
+  // return true for an undefined symbol in case the option is specified.
+  if (!Config->Shared)
+    return B.isUndefined();
+
+  // -Bsymbolic means that definitions are not preempted.
+  if (Config->Bsymbolic || (Config->BsymbolicFunctions && B.isFunc()))
+    return !B.isDefined();
+  return true;
+}
+
 // Create output section objects and add them to OutputSections.
 template <class ELFT> void Writer<ELFT>::finalizeSections() {
   Out::DebugInfo = findSection(".debug_info");
   Out::PreinitArray = findSection(".preinit_array");
   Out::InitArray = findSection(".init_array");
   Out::FiniArray = findSection(".fini_array");
 
   // The linker needs to define SECNAME_start, SECNAME_end and SECNAME_stop
template <class ELFT> void Writer<ELFT>::finalizeSections() {
   addRelIpltSymbols();
 
   // This responsible for splitting up .eh_frame section into
   // pieces. The relocation scan uses those pieces, so this has to be
   // earlier.
   applySynthetic({In<ELFT>::EhFrame},
                  [](SyntheticSection *SS) { SS->finalizeContents(); });
 
+  for (Symbol *S : Symtab->getSymbols())
+    S->body()->IsPreemptible = computeIsPreemptible(*S->body());
+
   // Scan relocations. This must be done after every symbol is declared so that
   // we can correctly decide if a dynamic relocation is needed.
   forEachRelSec(scanRelocations<ELFT>);
 
   if (InX::Plt && !InX::Plt->empty())
     InX::Plt->addSymbols();
   if (InX::Iplt && !InX::Iplt->empty())
     InX::Iplt->addSymbols();