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

141

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; }
+  bool computeIsPreemptible() const;
   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;
-
+bool SymbolBody::computeIsPreemptible() const {
+  assert(!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>(this))
     return !SS->CopyRelSec && !NeedsPltAddr;
 
   // Only symbols that appear in dynsym can be preempted.
   if (!symbol()->includeInDynsym())

ELF/Writer.cpp

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 = S->body()->computeIsPreemptible();
+
   // 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();