[PATCH] D106629: [lld/mac] Implement for section$start and section$ end symbols

[Jez Ng via Phabricator via llvm-commits]

int3 accepted this revision.
int3 added a comment.
This revision is now accepted and ready to land.

lgtm, though I'm a bit confused about how the symbol liveness is working out



================
Comment at: lld/MachO/SymbolTable.cpp:243
+                            : osec->sectionEndSymbols)
+      .push_back(symtab->addSynthetic(sym.getName(), /*isec=*/nullptr,
+                                      /*value=*/-1, /*isPrivateExtern=*/true,
----------------
thakis wrote:
> int3 wrote:
> > shouldn't we be passing in a non-null `isec` here?
> To me this semantically seems like an absolute symbol, and absolute symbols don't have isec. Defined::getVA() has an early exit for absolute symbols which is attractive since I don't want any isec-based adjustments for the address.
hmm, but the comment above says that "The replacing Defined gets its liveness from the underlying isec" -- how does that work if the isec isn't set here?


================
Comment at: lld/MachO/SymbolTable.cpp:219
+  OutputSection *osec = nullptr;
+  // This looks for __TEXT/__cstring etc.
+  for (SyntheticSection *ssec : syntheticSections)
----------------
thakis wrote:
> oontvoo wrote:
> > What about `section$start$__DATA$__<....>` ? 
> That's usually a ConcatInputSection.  The "create new used" branch below works for those. (We could walk through the existing isecs and not alloc a new one if we find one with the right name, but that's more code and more chance or something to go wrong, with little benefit, so I didn't do that. An earlier version of this patch on the linked PR did that, but it's not necessary so I got rid of it.) __DATA is covered by the test below, too.



================
Comment at: lld/test/MachO/start-end.s:141
+
+## Secton-rename tests.
+## Input section __FOO/__bar is renamed to output section
----------------



================
Comment at: lld/test/MachO/start-end.s:260
+.fill 1
+
+.subsections_via_symbols
----------------
would it be worthwhile to add a zerofill section to the test, to make sure the `$end` symbol occurs after the zeroes?


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

https://reviews.llvm.org/D106629