[PATCH] D76839: [lld-macho] Extend SyntheticSections to cover all segment load commands

[Jez Ng via Phabricator via llvm-commits]

int3 marked an inline comment as done.
int3 added inline comments.


================
Comment at: lld/MachO/Writer.cpp:259-261
+    return linkEditOffset + 1;
+  else
+    return linkEditOffset;
----------------
int3 wrote:
> int3 wrote:
> > smeenai wrote:
> > > ruiu wrote:
> > > > int3 wrote:
> > > > > ruiu wrote:
> > > > > > So it looks like the function returns only 4 possible values -- 0, 1, linkEditOffset, or linkEditOffset+1. Can't you use 0, 1, 2, 3 instead?
> > > > > The idea is to make it easily extensible -- there may be other special sections that we want to order before the __LINKEDIT sections.
> > > > Got it, but I don't think you have to prepare this tiny function for future extension. You can just make it return 0,1,2,3 and then in a later change when you actually need to use linkEditOffset, you can start returning linkEditOffset and linkEditOffset+1.
> > > Looks like this comment still needs a response.
> > > You can just make it return 0,1,2,3 and then in a later change when you actually need to use linkEditOffset, you can start returning linkEditOffset and linkEditOffset+1.
> > 
> > I'd actually forgotten to add the symbol table / string pool sections to the ordering after rebasing, oops. So we do actually have a bunch of __LINKEDIT sections to order in this diff already.
> > 
> > In any case, I've refactored things so that the ordering can be specified a bit more declaratively, though it does add a bit more code... let me know what you think, I can revert to the if-else chain if you'd prefer.
> > 
> > 
> > I'd actually forgotten to add the symbol table / string pool sections to the ordering after rebasing
> 
> Nevermind, I've instead swapped the stack position of this diff with that of D76742, so those sections are added later...
I was debugging an issue locally that boiled down to sections not actually being arranged contiguously by the sort. There's currently nothing enforcing that property, aside from relying on us to correctly update the ordering list each time a new section is created. As such, I'm changing the sorting comparator to operate on segment names first followed by section names.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D76839/new/

https://reviews.llvm.org/D76839