[LLD][ELF] Add DefinedInThunk to SymbolBody to remove need for hash lookup

[peter.smith]

Summary

When doing thunk creation, if a relocation target is defined in a thunk we need to find a way of getting to the Thunk object. This change removes the
DenseMap in ThunkCreator that stored the mapping in preference for storing a pointer to the Thunk in SymbolBody. This saves a hash lookup at the expense of adding a pointer to SymbolBody.

This was suggested in D34692

[ruiu]

ELF/Symbols.h

93–97

Please add blank lines above and below this member variable.

DefinedInThunk sounds like a boolean variable, so I'd give a different name, but obviously Thunk doesn't work. Do you have any suggestion?

[peter.smith]

Thanks for the comments. I've updated the diff and renamed to LabeledThunk which I think makes sense as the Symbol labels the Thunk.

[peter.smith]

Rebased patch, no other changes.

[peter.smith]

The use of the hash table and the proposed new member is so that, for
example, two arm relocations to a thumb function use a single thunk,
correct?

If so the hash table is only needed during the one pass when we are
mutating relocations to point to the thunk.

If the above is correct, a hash table seems better IMHO since it is only
used in a very specific point in the code.

Cheers,
Rafael

Yes it is only used within createThunks(). This review was created in response to a review comment in one of the previous range-thunks patches (still in review) https://reviews.llvm.org/D34692#809065 expressing a preference for avoiding the hash table lookup. Personally I prefer the hash table lookup as well but I've not got a strong preference. My main concern is getting the range-thunks functionality in.

[peter.smith]

I don't think that there was consensus over whether this change was a good idea. I'm abandoning it for now, it can be resurrected if needed.

ELF/Relocations.h

std::pair<Thunk *, bool> getThunk(SymbolBody &Body, uint32_t Type,
                                     uint64_t Src);
   ThunkSection *addThunkSection(OutputSection *OS,
                                 std::vector<InputSection *> *, uint64_t Off);
   bool normalizeExistingThunk(Relocation &Rel, uint64_t Src);
 
   // Record all the available Thunks for a Symbol
   llvm::DenseMap<SymbolBody *, std::vector<Thunk *>> ThunkedSymbols;
 
-  // Find a Thunk from the Thunks symbol definition, we can use this to find
-  // the Thunk from a relocation to the Thunks symbol definition.
-  llvm::DenseMap<SymbolBody *, Thunk *> Thunks;
-
   // Track InputSections that have an inline ThunkSection placed in front
   // an inline ThunkSection may have control fall through to the section below
   // so we need to make sure that there is only one of them.
   // The Mips LA25 Thunk is an example of an inline ThunkSection.
   llvm::DenseMap<InputSection *, ThunkSection *> ThunkedSections;
 
   // All the ThunkSections that we have created, organised by InputSectionRange
   // will contain a mix of ThunkSections that have been created this pass, and

ELF/Relocations.cpp

void ThunkCreator::forEachExecInputSectionRange(
   }
 }
 
 // Return true if the relocation target is an in range Thunk.
 // Return false if the relocation is not to a Thunk. If the relocation target
 // was originally to a Thunk, but is no longer in range we revert the
 // relocation back to its original non-Thunk target.
 bool ThunkCreator::normalizeExistingThunk(Relocation &Rel, uint64_t Src) {
-  if (Thunk *ET = Thunks.lookup(Rel.Sym)) {
+  if (Thunk *ET = Rel.Sym->LabeledThunk) {
     if (Target->inBranchRange(Rel.Type, Src, Rel.Sym->getVA()))
       return true;
     Rel.Sym = &ET->Destination;
     if (Rel.Sym->isInPlt())
       Rel.Expr = toPlt(Rel.Expr);
   }
   return false;
 }
forEachExecInputSectionRange(
               AddressesChanged = true;
               // Find or create a ThunkSection for the new Thunk
               ThunkSection *TS;
               if (auto *TIS = T->getTargetInputSection())
                 TS = getISThunkSec(TIS, OS);
               else
                 TS = getISRThunkSec(OS, IS, ISR, Rel.Type, Src);
               TS->addThunk(T);
-              Thunks[T->ThunkSym] = T;
+              T->ThunkSym->LabeledThunk = T;
             }
             // Redirect relocation to Thunk, we never go via the PLT to a Thunk
             Rel.Sym = T->ThunkSym;
             Rel.Expr = fromPlt(Rel.Expr);
           }
       });
   // Merge all created synthetic ThunkSections back into OutputSection
   mergeThunks();

ELF/Symbols.h

 class ArchiveFile;
 class BitcodeFile;
 class InputFile;
 class LazyObjFile;
 template <class ELFT> class ObjFile;
 class OutputSection;
 template <class ELFT> class SharedFile;
 
+class Thunk;
+
 struct Symbol;
 
 // The base class for real symbol classes.
 class SymbolBody {
 public:
   enum Kind {
     DefinedFirst,
     DefinedRegularKind = DefinedFirst,
public:
 
   uint64_t getGotOffset() const;
   uint64_t getGotVA() const;
   uint64_t getGotPltOffset() const;
   uint64_t getGotPltVA() const;
   uint64_t getPltVA() const;
   template <class ELFT> typename ELFT::uint getSize() const;
   OutputSection *getOutputSection() const;
 
+  // The Thunk that this symbol is defined in, nullptr if not defined in Thunk
+  Thunk *LabeledThunk = nullptr;
+
   uint32_t DynsymIndex = 0;
   uint32_t GotIndex = -1;
   uint32_t GotPltIndex = -1;
   uint32_t PltIndex = -1;
   uint32_t GlobalDynIndex = -1;
 
 protected:
   SymbolBody(Kind K, StringRefZ Name, bool IsLocal, uint8_t StOther,
              uint8_t Type);