[LLD][ELF] Calculate sizes of SHF_ALLOC Synthetic Sections early

[peter.smith]

Summary

This change splits out the finalizing of SyntheticSections size and the overall finalizing. This permits us to move createThunks() to the point where the addresses and sizes will be stable. It involves:

• Separating out the addition of GlobalSymbols to the dynamic symbol table and the static symbol table. The existing symbol table is built to assume that all local symbols are added before globals.
• Added a new member function finalizeAllocSize() to set the SyntheticSections size.
• Call the finalizeAllocSize() prior to createThunks(), which has been moved as late as it can.

I chose the name finalizeAllocSize to emphasize that only SHF_ALLOC sizes need to be finalized. We only need to worry about SyntheticSections that can disrupt thunk creation.

There is an argument that the DynamicSection and GOT sections don't need their size finalized early as they are in RW and well behaved applications will have all their code in RO, and all RO comes before RW. However since it wasn't much more effort to do so I thought it better to be consistent and to remove the possibility of a linker script breaking the assumption that they don't matter.

I initially tried to just move some of the existing finalize() calls before createThunks() but ended up in a somewhat tangled web of ordering dependencies. I think it would be less fragile to explicitly separate out the size finalization.

In the patch I've left the size calculations in the finalize() as asserts to check that createThunks() does not affect the size calculation.

The motivation is to allow addresses to be calculated prior to createThunks so that the information can be used in determining whether calls are within range. By finalizing the size of the SyntheticSections that have the SHF_ALLOC we remove a potential source of errors where range calculations made in createThunks are broken by changes in the sizes of SyntheticSections.

My next step is to allow address assignment prior to, and immediately after createThunks. At present the linker script address assignment is designed to only be run once so we can't just use the existing Script<ELFT>::X->assignAddresses(Phdrs). Any suggestions of how to approach that task would be most welcome as I don't have a lot of experience with that code.

[ruiu]

ELF/SyntheticSections.cpp

858–872

Is this computing the same value just for the last assertion? If so it seems a bit too much.

1085

I think I prefer a more functional style than this. You could change this function to return std::vector<Entry> and call that twice, once in finalizeAllocSize and the other in finalize.

1086

nit: For short lambdas like this probably just [=] is cognitively lightweight than explicit variable lists.

1324–1326

The reason why you need this is because if it is a dynamic symbol table, finalizing it may change (or fix) the size of a .gnu.hash section, right? Please add a comment.

ELF/Writer.cpp

1202

What is this?

[peter.smith]

Thank you both for the comments. I'm thinking that it may be better to rework this a little bit and resubmit as the MipsGot::PageIndex calculation breaks my assertion that adding range extension thunks can't affect the size of the SyntheticSections. The size of the MipsGot is proportional to the size of the OutputSections size (modulo 64k), while the amount of Thunks added is likely to be smaller than 64k adding a Thunk could potentially extend over a page boundary.

Instead of adding a finalizeAllocSize() I'm thinking about the following pseudo-code:

// if need thunks ...
bool AddressesChanged = true;
do {
  if (AddressesChanged)
    updateSyntheticSectionAllocSizes();
  AssignAddresses();
  AddressesChanged = createThunks();
} while (AddressesChanged)

I'll have a go at implementing using this model to see if it comes out a bit cleaner. Let me know if this sounds like a bad idea?

Some specific follow-up:

• The OS->size += 8 after the ARMExidxSentinelSection is best removed. The OS->Size of the .ARM.exidx Output Section isn't updated until assignOffsets() is called. This caused a problem when I experimentally assigned addresses beforehand, correcting the size fixed the problem with the early address assignment but we don't need it right now.
• I think it would be possible to change the SymbolTableSection to accept the addition of locals and globals in any order, I'm happy to have a go at that but it will be worth submitting as a separate patch. Would you prefer that to be done before doing this work or after?

As an aside I've experimented a bit with LinkerScript address assignment and it is possible to re-run the address assignment if all the state in the class members such as LMAOffset, AlreadyOutputOS is reset to the initial value.

[ruiu]

In D29983#678530, @peter.smith wrote:

Thank you both for the comments. I'm thinking that it may be better to rework this a little bit and resubmit as the MipsGot::PageIndex calculation breaks my assertion that adding range extension thunks can't affect the size of the SyntheticSections. The size of the MipsGot is proportional to the size of the OutputSections size (modulo 64k), while the amount of Thunks added is likely to be smaller than 64k adding a Thunk could potentially extend over a page boundary.

Instead of adding a finalizeAllocSize() I'm thinking about the following pseudo-code:

// if need thunks ...
bool AddressesChanged = true;
do {
  if (AddressesChanged)
    updateSyntheticSectionAllocSizes();
  AssignAddresses();
  AddressesChanged = createThunks();
} while (AddressesChanged)

I'll have a go at implementing using this model to see if it comes out a bit cleaner. Let me know if this sounds like a bad idea?

This is what I expected. We need a for-loop somewhere which adds thunks until a convergence is obtained (all addresses are fixed), so that seems overall fine.

Some specific follow-up:

• The OS->size += 8 after the ARMExidxSentinelSection is best removed. The OS->Size of the .ARM.exidx Output Section isn't updated until assignOffsets() is called. This caused a problem when I experimentally assigned addresses beforehand, correcting the size fixed the problem with the early address assignment but we don't need it right now.

• I think it would be possible to change the SymbolTableSection to accept the addition of locals and globals in any order, I'm happy to have a go at that but it will be worth submitting as a separate patch. Would you prefer that to be done before doing this work or after?

Maybe before, if that makes it easier for you to make other changes?

As an aside I've experimented a bit with LinkerScript address assignment and it is possible to re-run the address assignment if all the state in the class members such as LMAOffset, AlreadyOutputOS is reset to the initial value.

[peter.smith]

Updated diff to:

• Rebased on top of D30085 (not reviewed yet) which allows us to add local symbols after globals
• Moved the addition of symbols to the GNU Hash Table to a separate member function that can be called immediately after the dynamic symbols have been added.
• Change finalizeAllocSize() to updateAllocSize(). The intention is that this will be called each time thunks are added, and immediately prior to thunks being added, to prevent the MipsGot missing an additional page.
• Hopefully addressed any review comments that still apply

I've not put a loop in for createThunks() yet as they need some work to make sure that they don't create Thunks to Thunks and loop into infinity. This would be best addressed in a later patch.

[peter.smith]

Thank you very much for the comments, I've updated the diff to incorporate the suggestions.

I will be on vacation with limited computer access till the 1st March, I'll pick this back up when I return.

I like the suggestion of splitting up the finalize into 3 separate parts. I've had a go at implementing that in this diff. This has a nice side-effect that the dynamic section finalize only has to run once as the dynamic symbol table early finalize means we don't need to update the entries. I think that this could be hidden inside a single finalize member but I think we might have to track the state to make sure finalize can be called more than once.

In the general case we will need to assign addresses multiple times as it is possible to write linker scripts where adding thunks causes address expressions in the script to change in ways that breaks the address calculations.

I've copied Rafael's comments into Phabricator for reference:

+ if (In<ELFT>::GnuHashTab && In<ELFT>::DynSymTab)
+ In<ELFT>::DynSymTab->addSymbolsToGnuHashTab();

Having the gnu hash implies that we also have a dynamic symbol table,
no?

// Dynamic section must be the last one in this list and dynamic
// symbol table section (DynSymTab) must be the first one.

This comment should stay with the definition of the Synthetics array.

+ Add remaining entries to complete .dynamic contents.
+template <class ELFT> void DynamicSection<ELFT>::updateAllocSize() {
+ std::vector<Entry> LateEntries = addLateEntries();
+ +1 for DT_NULL
+ this->Size = (Entries.size() + LateEntries.size() + 1) * this->Entsize;
+}
+
+// Add remaining entries to complete .dynamic contents.
+template <class ELFT> void DynamicSection<ELFT>::finalize() {
+ this->Link = In<ELFT>::DynStrTab->OutSec->SectionIndex;
+ std::vector<Entry> LateEntries = addLateEntries();
+ Entries.insert(Entries.end(), LateEntries.begin(), LateEntries.end());

this->OutSec->Entsize = this->Entsize;
this->OutSec->Link = this->Link;

• // +1 for DT_NULL this->Size = (Entries.size() + 1) * this->Entsize;

Can you call updateAllocSize instead? Why do you need to split
addLateEntries btw? The actual number of entries doesn't change when
adding thunks, right? I looks like it should be sufficient to just
populate Entries earlier.

-template <class ELFT> void GnuHashTableSection<ELFT>::finalize() {
+template <class ELFT> void GnuHashTableSection<ELFT>::updateAllocSize() {

unsigned NumHashed = Symbols.size();
NBuckets = calcNBuckets(NumHashed);
MaskWords = calcMaskWords(NumHashed);

Do you need to do this in every updateAllocSize?

If I understand it correctly, this patch is in an odd middle ground. I
think I would prefer to either call the full finalize multiple times if
thunks are added or split finalize is 3 methods:

• One that is called early and only once. It would do most of the work that finalize currently does.
• updateAllocSize, which is empty for most sections and just updates whatever can change the size in response to other sections changing size.
• A real finalize method, that is called only once and can set values that depend on the final address of the sections.

Cheers,
Rafael

[ruiu]

ELF/SyntheticSections.h

47

We are probably using the term finalize too much in the linker. It used to be that one class, OutputSection, has a finalizer function that fix the output section layout. Now many classes have finalizers functions and they do different things at different stages. That is hard to read. I'd like to remove all uses of finalize from the linker.

[peter.smith]

Thanks for the comments, I'll pick this back up when I get back to the office next week. It is helpful to know what the intentions are with respect to naming.

[peter.smith]

Rebased and updated the patch.

I had a go at Rafael's suggestion of renaming the split of finalizeContent into startLayout, updateAllocSize and endLayout. After moving as much of I could into startLayout we aren't left with much in updateAllocSize or endLayout. With this in mind I've tried a different idea that keeps finalizeContent and instead adds updateAllocSize and postThunkContent.

In Writer.cpp I've called updateAllocSize and postThunkContent only on the sections that need changing.

Rafael's email comments for the benefit of Phabriacator:

+ applySynthetic<ELFT>(Synthetics,
+ [](SyntheticSection<ELFT> *SS) { SS->finalize(); });

For the names, how about:

• startLayout
• updateAllocSize
• endLayout

A more precise set of names would be atLayoutStart, updateAllocSize and
atLayoutEnd, but not sure if it is actually better.

}

template <class ELFT> void Writer<ELFT>::addPredefinedSections() {

Index: ELF/SyntheticSections.h

• ELF/SyntheticSections.h

+++ ELF/SyntheticSections.h
@@ -47,6 +47,12 @@

virtual ~SyntheticSection() = default;
virtual void writeTo(uint8_t *Buf) = 0;
virtual size_t getSize() const = 0;

+ Finalize is split up into 3 parts to accommodate Thunks, early finalize
+ should do as much as possible including finalizing the section size

"finalizing" is a bit strong since we have updateAllocSize. It should
compute as good a lower bound as possible.

+ // If the size calculation needs to be updated during Thunk creation

incomplete sentence.

+ virtual void updateAllocSize() {}
+ // Runs after all content has been added, must not alter section
size

and all addresses are know?

+template <class ELFT> void DynamicSection<ELFT>::earlyFinalize() {

if (In<ELFT>::RelaDyn->OutSec->Size > 0) {
  bool IsRela = Config->Rela;
  add({IsRela ? DT_RELA : DT_REL, In<ELFT>::RelaDyn});

@@ -941,13 +941,17 @@

    add({DT_MIPS_RLD_MAP, In<ELFT>::MipsRldMap});
}

• this->OutSec->Entsize = this->Entsize;
• this->OutSec->Link = this->Link;

Why does setting the link has to be delayed too?

+template <class ELFT> void SymbolTableSection<ELFT>::finalize() {
+ this->OutSec->Link = this->Link = StrTabSec.OutSec->SectionIndex;
+ this->OutSec->Entsize = this->Entsize;
+
+ if (StrTabSec.isDynamic()) {
+ this->OutSec->Info = this->Info = 1;

This can be set early, no?

Cheers,
Rafael

[rui314]

This is looking good. LGTM.

[peter.smith]

Apologies, I'm a bit confused about the status of this patch after the last comment/status change, I'll assume this is still waiting for further comments for now.

[grimar]

In D29983#694160, @peter.smith wrote:

Apologies, I'm a bit confused about the status of this patch after the last comment/status change, I'll assume this is still waiting for further comments for now.

@rui314 and @ruiu are the same person. You still have LGTM from Rui I believe.

[ruiu]

Yes, both are my accounts. I'm using ruiu as the main account though. Sorry for the confusion. It's still LGTM.

ELF/Relocations.h

struct Relocation {
   uint64_t Offset;
   int64_t Addend;
   SymbolBody *Sym;
 };
 
 template <class ELFT> void scanRelocations(InputSectionBase &);
 
 template <class ELFT>
-void createThunks(ArrayRef<OutputSection *> OutputSections);
+bool createThunks(ArrayRef<OutputSection *> OutputSections);
 
 // Return a int64_t to make sure we get the sign extension out of the way as
 // early as possible.
 template <class ELFT>
 static inline int64_t getAddend(const typename ELFT::Rel &Rel) {
   return 0;
 }
 template <class ELFT>
 static inline int64_t getAddend(const typename ELFT::Rela &Rel) {
   return Rel.r_addend;
 }
 }
 }
 
 #endif

ELF/Relocations.cpp

 // createThunks must be called after scanRelocs has created the Relocations for
 // each InputSection. It must be called before the static symbol table is
 // finalized. If any Thunks are added to an OutputSection the output section
 // offsets of the InputSections will change.
 //
 // FIXME: All Thunks are assumed to be in range of the relocation. Range
 // extension Thunks are not yet supported.
 template <class ELFT>
-void createThunks(ArrayRef<OutputSection *> OutputSections) {
+bool createThunks(ArrayRef<OutputSection *> OutputSections) {
   // Track Symbols that already have a Thunk
   DenseMap<SymbolBody *, Thunk<ELFT> *> ThunkedSymbols;
   // Track InputSections that have a ThunkSection placed in front
   DenseMap<InputSection *, ThunkSection<ELFT> *> ThunkedSections;
   // Track the ThunksSections that need to be inserted into an OutputSection
   std::map<OutputSection *, std::vector<ThunkSection<ELFT> *>> ThunkSections;
 
   // Find or create a Thunk for Body for relocation Type
for (InputSection *IS : OS->Sections) {
         }
       }
     }
   }
 
   // Merge all created synthetic ThunkSections back into OutputSection
   for (auto &KV : ThunkSections)
     mergeThunks<ELFT>(KV.first, KV.second);
+  return !ThunkSections.empty();
 }
 
 template void scanRelocations<ELF32LE>(InputSectionBase &);
 template void scanRelocations<ELF32BE>(InputSectionBase &);
 template void scanRelocations<ELF64LE>(InputSectionBase &);
 template void scanRelocations<ELF64BE>(InputSectionBase &);
 
-template void createThunks<ELF32LE>(ArrayRef<OutputSection *>);
-template void createThunks<ELF32BE>(ArrayRef<OutputSection *>);
-template void createThunks<ELF64LE>(ArrayRef<OutputSection *>);
-template void createThunks<ELF64BE>(ArrayRef<OutputSection *>);
+template bool createThunks<ELF32LE>(ArrayRef<OutputSection *>);
+template bool createThunks<ELF32BE>(ArrayRef<OutputSection *>);
+template bool createThunks<ELF64LE>(ArrayRef<OutputSection *>);
+template bool createThunks<ELF64BE>(ArrayRef<OutputSection *>);
 }
 }

ELF/SyntheticSections.h

SyntheticSection(uint64_t Flags, uint32_t Type, uint64_t Addralign,
                      InputSectionBase::Synthetic) {
     this->Live = true;
   }
 
   virtual ~SyntheticSection() = default;
   virtual void writeTo(uint8_t *Buf) = 0;
   virtual size_t getSize() const = 0;
   virtual void finalizeContents() {}
+  // If the section has the SHF_ALLOC flag and the size may be changed if
+  // thunks are added, update the section size.
+  virtual void updateAllocSize() {}
+  // If any additional finalization of contents are needed post thunk creation.
+  virtual void postThunkContents() {}
   virtual bool empty() const { return false; }
   uint64_t getVA() const;
 
   static bool classof(const InputSectionBase *D) {
     return D->kind() == InputSectionBase::Synthetic;
   }
 };
 

 
 template <class ELFT> class MipsGotSection final : public SyntheticSection {
   typedef typename ELFT::uint uintX_t;
 
 public:
   MipsGotSection();
   void writeTo(uint8_t *Buf) override;
   size_t getSize() const override { return Size; }
+  void updateAllocSize() override;
   void finalizeContents() override;
   bool empty() const override;
   void addEntry(SymbolBody &Sym, int64_t Addend, RelExpr Expr);
   bool addDynTlsEntry(SymbolBody &Sym);
   bool addTlsIndex();
   uintX_t getPageEntryOffset(const SymbolBody &B, int64_t Addend) const;
   uintX_t getBodyEntryOffset(const SymbolBody &B, int64_t Addend) const;
   uintX_t getGlobalDynOffset(const SymbolBody &B) const;
struct Entry {
     Entry(int32_t Tag, InputSection *Sec)
         : Tag(Tag), InSec(Sec), Kind(InSecAddr) {}
     Entry(int32_t Tag, uint64_t Val) : Tag(Tag), Val(Val), Kind(PlainInt) {}
     Entry(int32_t Tag, const SymbolBody *Sym)
         : Tag(Tag), Sym(Sym), Kind(SymAddr) {}
   };
 
   // finalizeContents() fills this vector with the section contents.
-  // finalizeContents()cannot directly create final section contents because
-  // when the function is called, symbol or section addresses are not fixed yet.
   std::vector<Entry> Entries;
 
 public:
   DynamicSection();
   void finalizeContents() override;
   void writeTo(uint8_t *Buf) override;
   size_t getSize() const override { return Size; }
 

 template <class ELFT> class SymbolTableSection final : public SyntheticSection {
 public:
   typedef typename ELFT::Sym Elf_Sym;
   typedef typename ELFT::uint uintX_t;
 
   SymbolTableSection(StringTableSection<ELFT> &StrTabSec);
 
   void finalizeContents() override;
+  void postThunkContents() override;
   void writeTo(uint8_t *Buf) override;
   size_t getSize() const override { return getNumSymbols() * sizeof(Elf_Sym); }
   void addSymbol(SymbolBody *Body);
   unsigned getNumSymbols() const { return Symbols.size() + 1; }
   size_t getSymbolIndex(SymbolBody *Body);
   ArrayRef<SymbolTableEntry> getSymbols() const { return Symbols; }
 
 private:

ELF/SyntheticSections.cpp

 
 template <class ELFT>
 unsigned MipsGotSection<ELFT>::getLocalEntriesNum() const {
   return HeaderEntriesNum + PageEntriesNum + LocalEntries.size() +
          LocalEntries32.size();
 }
 
 template <class ELFT> void MipsGotSection<ELFT>::finalizeContents() {
+  updateAllocSize();
+}
+
+template <class ELFT> void MipsGotSection<ELFT>::updateAllocSize() {
   PageEntriesNum = 0;
   for (std::pair<const OutputSection *, 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.
     P.second = PageEntriesNum;
     PageEntriesNum += getMipsPageCount(P.first->Size);
   }
   Size = (getLocalEntriesNum() + GlobalEntries.size() + TlsEntries.size()) *
          sizeof(uintX_t);
 }
 
 template <class ELFT> bool MipsGotSection<ELFT>::empty() const {
   // We add the .got section to the result for dynamic MIPS target because
   // its address and properties are mentioned in the .dynamic section.
   return Config->Relocatable;
 }
 
 template <class ELFT>
 typename MipsGotSection<ELFT>::uintX_t MipsGotSection<ELFT>::getGp() const {
   return ElfSym::MipsGp->template getVA<ELFT>(0);
 }
 
 template <class ELFT>
 static void writeUint(uint8_t *Buf, typename ELFT::uint Val) {
   typedef typename ELFT::uint uintX_t;
   write<uintX_t, ELFT::TargetEndianness, sizeof(uintX_t)>(Buf, Val);
 }
 
 template <class ELFT> void MipsGotSection<ELFT>::writeTo(uint8_t *Buf) {
   // Set the MSB of the second GOT slot. This is not required by any
   // MIPS ABI documentation, though.
   //
   // There is a comment in glibc saying that "The MSB of got[1] of a
   // gnu object is set to identify gnu objects," and in GNU gold it

 }
 
 // Add remaining entries to complete .dynamic contents.
 template <class ELFT> void DynamicSection<ELFT>::finalizeContents() {
   if (this->Size)
     return; // Already finalized.
 
   this->Link = In<ELFT>::DynStrTab->OutSec->SectionIndex;
   if (In<ELFT>::RelaDyn->OutSec->Size > 0) {
     bool IsRela = Config->Rela;
     add({IsRela ? DT_RELA : DT_REL, In<ELFT>::RelaDyn});
     add({IsRela ? DT_RELASZ : DT_RELSZ, In<ELFT>::RelaDyn->OutSec->Size});
     add({IsRela ? DT_RELAENT : DT_RELENT,
          uintX_t(IsRela ? sizeof(Elf_Rela) : sizeof(Elf_Rel))});
 
     // MIPS dynamic loader does not support RELCOUNT tag.
     // The problem is in the tight relation between dynamic
     // relocations and GOT. So do not emit this tag on MIPS.
if (this->Type == SHT_DYNSYM) {
 
     if (In<ELFT>::GnuHashTab) {
       // NB: It also sorts Symbols to meet the GNU hash table requirements.
       In<ELFT>::GnuHashTab->addSymbols(Symbols);
     } else if (Config->EMachine == EM_MIPS) {
       std::stable_sort(Symbols.begin(), Symbols.end(), sortMipsSymbols);
     }
 
     size_t I = 0;
     for (const SymbolTableEntry &S : Symbols)
       S.Symbol->DynsymIndex = ++I;
     return;
   }
+}
 
-  // If it is a .symtab, move all local symbols before global symbols.
+template <class ELFT> void SymbolTableSection<ELFT>::postThunkContents() {
+  if (this->Type == SHT_DYNSYM)
+    return;
+  // move all local symbols before global symbols.
   auto It = std::stable_partition(
       Symbols.begin(), Symbols.end(), [](const SymbolTableEntry &S) {
         return S.Symbol->isLocal() ||
                S.Symbol->symbol()->computeBinding() == STB_LOCAL;
       });
   size_t NumLocals = It - Symbols.begin();
   this->OutSec->Info = NumLocals + 1;
 }

ELF/Writer.cpp

while (NonScriptI != E) {
     std::rotate(BestPos, NonScriptI, NonScriptI + 1);
     ++NonScriptI;
   }
 
   Script<ELFT>::X->adjustSectionsAfterSorting();
 }
 
 template <class ELFT>
-static void finalizeSynthetic(const std::vector<SyntheticSection *> &Sections) {
+static void applySynthetic(const std::vector<SyntheticSection *> &Sections,
+                           std::function<void(SyntheticSection *)> Fn) {
   for (SyntheticSection *SS : Sections)
     if (SS && SS->OutSec && !SS->empty()) {
-      SS->finalizeContents();
+      Fn(SS);
       SS->OutSec->template assignOffsets<ELFT>();
     }
 }
 
 // We need to add input synthetic sections early in createSyntheticSections()
 // to make them visible from linkescript side. But not all sections are always
 // required to be in output. For example we don't need dynamic section content
 // sometimes. This function filters out such unused sections from output.
template <class ELFT> void Writer<ELFT>::finalizeSections() {
   // https://sourceware.org/ml/binutils/2002-03/msg00360.html
   if (In<ELFT>::DynSymTab)
     addRegular<ELFT>("_DYNAMIC", In<ELFT>::Dynamic, 0);
 
   // Define __rel[a]_iplt_{start,end} symbols if needed.
   addRelIpltSymbols();
 
   // This responsible for splitting up .eh_frame section into
-  // pieces. The relocation scan uses those peaces, so this has to be
+  // pieces. The relocation scan uses those pieces, so this has to be
   // earlier.
-  finalizeSynthetic<ELFT>({In<ELFT>::EhFrame});
+  applySynthetic<ELFT>({In<ELFT>::EhFrame},
+                       [](SyntheticSection *SS) { SS->finalizeContents(); });
 
   // 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 (In<ELFT>::Plt && !In<ELFT>::Plt->empty())
     In<ELFT>::Plt->addSymbols();
   if (In<ELFT>::Iplt && !In<ELFT>::Iplt->empty())
template <class ELFT> void Writer<ELFT>::finalizeSections() {
   // image base and the dynamic section on mips includes the image base.
   if (!Config->Relocatable && !Config->OFormatBinary) {
     Phdrs = Script<ELFT>::X->hasPhdrsCommands() ? Script<ELFT>::X->createPhdrs()
                                                 : createPhdrs();
     addPtArmExid(Phdrs);
     fixHeaders();
   }
 
-  // Some architectures use small displacements for jump instructions.
-  // It is linker's responsibility to create thunks containing long
-  // jump instructions if jump targets are too far. Create thunks.
-  if (Target->NeedsThunks)
-    createThunks<ELFT>(OutputSections);
-
-  // Fill other section headers. The dynamic table is finalized
-  // at the end because some tags like RELSZ depend on result
-  // of finalizing other sections.
-  for (OutputSection *Sec : OutputSections)
-    Sec->finalize<ELFT>();
-
   // Dynamic section must be the last one in this list and dynamic
   // symbol table section (DynSymTab) must be the first one.
-  finalizeSynthetic<ELFT>(
+  applySynthetic<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});
+       In<ELFT>::VerNeed,   In<ELFT>::Dynamic},
+      [](SyntheticSection *SS) { SS->finalizeContents(); });
+
+  // Some architectures use small displacements for jump instructions.
+  // It is linker's responsibility to create thunks containing long
+  // jump instructions if jump targets are too far. Create thunks.
+  if (Target->NeedsThunks) {
+    // FIXME: only ARM Interworking and Mips LA25 Thunks are implemented, these
+    // do not require address information. To support range extension Thunks
+    // we need to assign addresses so that we can tell if jump instructions
+    // are out of range. This will need to turn into a loop that converges
+    // when no more Thunks are added
+    if (createThunks<ELFT>(OutputSections))
+      applySynthetic<ELFT>({In<ELFT>::MipsGot},
+                           [](SyntheticSection *SS) { SS->updateAllocSize(); });
+  }
+  // Fill other section headers. The dynamic table is finalized
+  // at the end because some tags like RELSZ depend on result
+  // of finalizing other sections.
+  for (OutputSection *Sec : OutputSections)
+    Sec->finalize<ELFT>();
+
+  // createThunks may have added local symbols to the static symbol table
+  applySynthetic<ELFT>({In<ELFT>::SymTab, In<ELFT>::ShStrTab, In<ELFT>::StrTab},
+                       [](SyntheticSection *SS) { SS->postThunkContents(); });
 }
 
 template <class ELFT> void Writer<ELFT>::addPredefinedSections() {
   // Add BSS sections.
   auto Add = [=](OutputSection *Sec) {
     if (!Sec->Sections.empty()) {
       Sec->assignOffsets<ELFT>();
       OutputSections.push_back(Sec);
     }
   };
   Add(Out::Bss);
   Add(Out::BssRelRo);
 
   // ARM ABI requires .ARM.exidx to be terminated by some piece of data.
   // We have the terminater synthetic section class. Add that at the end.
   auto *OS = dyn_cast_or_null<OutputSection>(findSection(".ARM.exidx"));
   if (OS && !OS->Sections.empty() && !Config->Relocatable)
     OS->addSection(make<ARMExidxSentinelSection<ELFT>>());
 }
 
 // The linker is expected to define SECNAME_start and SECNAME_end
 // symbols for a few sections. This function defines them.
 template <class ELFT> void Writer<ELFT>::addStartEndSymbols() {
   auto Define = [&](StringRef Start, StringRef End, OutputSection *OS) {
     // These symbols resolve to the image base if the section does not exist.
     // A special value -1 indicates end of the section.
     addOptionalSynthetic<ELFT>(Start, OS, 0);