[PATCH] D29983: [LLD][ELF] Calculate sizes of SHF_ALLOC Synthetic Sections early

[Rafael Avila de Espindola via llvm-commits]

> Index: ELF/Writer.cpp
> ===================================================================
> --- ELF/Writer.cpp
> +++ ELF/Writer.cpp
> @@ -1037,6 +1037,19 @@
>    for (SyntheticSection<ELFT> *SS : Sections)
>      if (SS && SS->OutSec && !SS->empty()) {
>        SS->finalize();
> +      if (!(SS->OutSec->Flags & SHF_ALLOC)) {
> +        SS->OutSec->Size = 0;
> +        SS->OutSec->assignOffsets();
> +      }
> +    }
> +}
> +
> +template <class ELFT>
> +static void finalizeSyntheticAllocSize(
> +    const std::vector<SyntheticSection<ELFT> *> &Sections) {
> +  for (SyntheticSection<ELFT> *SS : Sections)
> +    if (SS && SS->OutSec && !SS->empty()) {
> +      SS->finalizeAllocSize();
>        SS->OutSec->Size = 0;
>        SS->OutSec->assignOffsets();

So we call assignOffsets twice on non alloc sections? Do we have to?


> +  // Now that we have defined all possible static symbols including linker-
> +  // synthesized ones. Visit all static symbols to give the finishing touches.
> +  for (Symbol *S : Symtab<ELFT>::X->getSymbols()) {
> +    SymbolBody *Body = S->body();
> +
> +    if (!includeInSymtab<ELFT>(*Body))
> +      continue;
> +    if (In<ELFT>::SymTab)
> +      In<ELFT>::SymTab->addGlobal(Body);
> +  }

This entire loop does nothing if In<ELFT>::SymTab is null. Move the if
outside.

How complicated would it be to make In<ELFT>::SymTab support adding
globals and locals in different order?

>    // Fill other section headers. The dynamic table is finalized
>    // at the end because some tags like RELSZ depend on result
>    // of finalizing other sections.

I guess this comment is out of date.

>    for (OutputSectionBase *Sec : OutputSections)
>      Sec->finalize();
>  
>    // Dynamic section must be the last one in this list and dynamic
> -  // symbol table section (DynSymTab) must be the first one.
> -  finalizeSynthetic<ELFT>(
> -      {In<ELFT>::DynSymTab, In<ELFT>::GnuHashTab, In<ELFT>::HashTab,
> -       In<ELFT>::SymTab,    In<ELFT>::ShStrTab,   In<ELFT>::StrTab,
> -       In<ELFT>::VerDef,    In<ELFT>::DynStrTab,  In<ELFT>::GdbIndex,
> -       In<ELFT>::Got,       In<ELFT>::MipsGot,    In<ELFT>::IgotPlt,
> -       In<ELFT>::GotPlt,    In<ELFT>::RelaDyn,    In<ELFT>::RelaIplt,
> -       In<ELFT>::RelaPlt,   In<ELFT>::Plt,        In<ELFT>::Iplt,
> -       In<ELFT>::Plt,       In<ELFT>::EhFrameHdr, In<ELFT>::VerSym,
> -       In<ELFT>::VerNeed,   In<ELFT>::Dynamic});
> +  // symbol table section (DynSymTab) must be the first one. The finalize
> +  // must not change the size of a SHF_ALLOC section.

This comment should probably go with the definition of the Synthetics
vector.


> +template <class ELFT> void MipsGotSection<ELFT>::finalize() {
> +  PageEntriesNum = 0;
> +  for (std::pair<const OutputSectionBase *, size_t> &P : PageIndexMap) {
> +    // For each output section referenced by GOT page relocations calculate
> +    // and save into PageIndexMap an upper bound of MIPS GOT entries required
> +    // to store page addresses of local symbols. We assume the worst case -
> +    // each 64kb page of the output section has at least one GOT relocation
> +    // against it. And take in account the case when the section intersects
> +    // page boundaries.

This comment is duplicated, no? In fact, the function body is
duplicated. Can you refactor it to a computeSize function/method and
then finalizeAllocSize calls it and sets Size and finalize calls it and
asserts that is the value of Size?


Cheers,
Rafael