[PATCH] D34034: [LLD][ELF] Make createThunks() iterate until no more thunks added

[Rafael Avila de Espindola via llvm-commits]

LGTM

Peter Smith via Phabricator <reviews at reviews.llvm.org> writes:

> peter.smith updated this revision to Diff 102644.
> peter.smith added a comment.
>
>> The code still has
>>  TS = make<ThunkSection>(TOS, IS->OutSecOff);
>>  so now we create an extra thunk, no?
>
> Yes you are right. My apologies for not catching that earlier. I've updated the diff to remove it.
>
>
> https://reviews.llvm.org/D34034
>
> Files:
>   ELF/Relocations.cpp
>   ELF/Relocations.h
>   ELF/Writer.cpp
>
> Index: ELF/Writer.cpp
> ===================================================================
> --- ELF/Writer.cpp
> +++ ELF/Writer.cpp
> @@ -1205,9 +1205,12 @@
>      // are out of range. This will need to turn into a loop that converges
>      // when no more Thunks are added
>      ThunkCreator TC;
> -    if (TC.createThunks(OutputSectionCommands))
> +    if (TC.createThunks(OutputSectionCommands)) {
>        applySynthetic({InX::MipsGot},
>                       [](SyntheticSection *SS) { SS->updateAllocSize(); });
> +      if (TC.createThunks(OutputSectionCommands))
> +        fatal("All non-range thunks should be created in first call");
> +    }
>    }
>  
>    // Fill other section headers. The dynamic table is finalized
> Index: ELF/Relocations.h
> ===================================================================
> --- ELF/Relocations.h
> +++ ELF/Relocations.h
> @@ -126,6 +126,11 @@
>    // Return true if Thunks have been added to OutputSections
>    bool createThunks(ArrayRef<OutputSectionCommand *> OutputSections);
>  
> +  // The number of completed passes of createThunks this permits us
> +  // to do one time initialization on Pass 0 and put a limit on the
> +  // number of times it can be called to prevent infinite loops.
> +  uint32_t Pass = 0;
> +
>  private:
>    void mergeThunks();
>    ThunkSection *getOSThunkSec(OutputSection *OS,
> @@ -137,14 +142,22 @@
>                           InputSection *)>
>            Fn);
>    std::pair<Thunk *, bool> getThunk(SymbolBody &Body, uint32_t Type);
> -
> +  ThunkSection *addThunkSection(OutputSection *OS,
> +                                std::vector<InputSection *> *, uint64_t Off);
>    // Track Symbols that already have a Thunk
>    llvm::DenseMap<SymbolBody *, 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 a ThunkSection placed in front
>    llvm::DenseMap<InputSection *, ThunkSection *> ThunkedSections;
>  
> -  // Track the ThunksSections that need to be inserted into an OutputSection
> +  // All the ThunkSections that we have created, organised by OutputSection
> +  // will contain a mix of ThunkSections that have been created this pass, and
> +  // ThunkSections that have been merged into the OutputSection on previous
> +  // passes
>    std::map<std::vector<InputSection *> *, std::vector<ThunkSection *>>
>        ThunkSections;
>  
> Index: ELF/Relocations.cpp
> ===================================================================
> --- ELF/Relocations.cpp
> +++ ELF/Relocations.cpp
> @@ -1009,8 +1009,7 @@
>        if ((IS->Flags & SHF_EXECINSTR) == 0)
>          break;
>      }
> -    CurTS = make<ThunkSection>(OS, Off);
> -    ThunkSections[ISR].push_back(CurTS);
> +    CurTS = addThunkSection(OS, ISR, Off);
>    }
>    return CurTS;
>  }
> @@ -1020,7 +1019,6 @@
>    if (TS)
>      return TS;
>    auto *TOS = IS->getParent();
> -  TS = make<ThunkSection>(TOS, IS->OutSecOff);
>  
>    // Find InputSectionRange within TOS that IS is in
>    OutputSectionCommand *C = Script->getCmd(TOS);
> @@ -1035,11 +1033,20 @@
>          break;
>        }
>      }
> -  ThunkSections[Range].push_back(TS);
> +  TS = addThunkSection(TOS, Range, IS->OutSecOff);
>    ThunkedSections[IS] = TS;
>    return TS;
>  }
>  
> +ThunkSection *ThunkCreator::addThunkSection(OutputSection *OS,
> +                                            std::vector<InputSection *> *ISR,
> +                                            uint64_t Off) {
> +  auto *TS = make<ThunkSection>(OS, Off);
> +  ThunkSections[ISR].push_back(TS);
> +  return TS;
> +}
> +
> +
>  std::pair<Thunk *, bool> ThunkCreator::getThunk(SymbolBody &Body,
>                                                  uint32_t Type) {
>    auto res = ThunkedSymbols.insert({&Body, nullptr});
> @@ -1081,18 +1088,22 @@
>  // extension Thunks are not yet supported.
>  bool ThunkCreator::createThunks(
>      ArrayRef<OutputSectionCommand *> OutputSections) {
> +  if (Pass > 0)
> +    ThunkSections.clear();
> +
>    // Create all the Thunks and insert them into synthetic ThunkSections. The
>    // ThunkSections are later inserted back into the OutputSection.
>  
>    // We separate the creation of ThunkSections from the insertion of the
>    // ThunkSections back into the OutputSection as ThunkSections are not always
>    // inserted into the same OutputSection as the caller.
>    forEachExecInputSection(
> -      OutputSections, [=](OutputSection *OS,  std::vector<InputSection*> *ISR,
> +      OutputSections, [&](OutputSection *OS,  std::vector<InputSection*> *ISR,
>                            InputSection *IS) {
>          for (Relocation &Rel : IS->Relocations) {
>            SymbolBody &Body = *Rel.Sym;
> -          if (!Target->needsThunk(Rel.Expr, Rel.Type, IS->File, Body))
> +          if (Thunks.find(&Body) != Thunks.end() ||
> +              !Target->needsThunk(Rel.Expr, Rel.Type, IS->File, Body))
>              continue;
>            Thunk *T;
>            bool IsNew;
> @@ -1105,15 +1116,16 @@
>              else
>                TS = getOSThunkSec(OS, ISR);
>              TS->addThunk(T);
> +            Thunks[T->ThunkSym] = 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();
> +  ++Pass;
>    return !ThunkSections.empty();
>  }
>