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

[Nico Weber via Phabricator via llvm-commits]

thakis marked an inline comment as done.
thakis added a comment.

Thanks!



================
Comment at: lld/MachO/SymbolTable.cpp:243
+                            : osec->sectionEndSymbols)
+      .push_back(symtab->addSynthetic(sym.getName(), /*isec=*/nullptr,
+                                      /*value=*/-1, /*isPrivateExtern=*/true,
----------------
int3 wrote:
> 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?
That's an excellent question! I wrote the comment when there was still a backing isec. Absolute defineds get their liveness from the `used` bit on the superclass, so https://reviews.llvm.org/D106565 made that part work. We still need an explicitly-live isec to make sure that the OutputSection the new symbol is added to is emitted. I adjusted the comment.


================
Comment at: lld/test/MachO/start-end.s:260
+.fill 1
+
+.subsections_via_symbols
----------------
int3 wrote:
> would it be worthwhile to add a zerofill section to the test, to make sure the `$end` symbol occurs after the zeroes?
Sure, done. Seems to Just Work, but yes, that's a good thing to test.


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

https://reviews.llvm.org/D106629