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

[Nico Weber via Phabricator via llvm-commits]

thakis added inline comments.


================
Comment at: lld/MachO/OutputSection.h:70-71
   StringRef name;
+  SmallVector<Defined *, 1> sectionStartSymbols;
+  SmallVector<Defined *, 1> sectionEndSymbols;
   OutputSegment *parent = nullptr;
----------------
int3 wrote:
> when will we have more than one symbol here?
> 
> also, if we're usually going to have just one element, since a TinyPtrVector better?
> when will we have more than one symbol here?

e.g. if you have `section$start$FOO$bar` and `section$start$BAZ$quux` and `sectrename FOO bar BAZ quux` -- both symbols will refer to the same section.

> also, if we're usually going to have just one element, since a TinyPtrVector better?

There are on the order of 10 OutputSections so I don't think we _really_ need to worry about its size. But sure, done, and TIL about TinyPtrVector :) (It means we have to include Symbols.h in the header, but /shruggie)


================
Comment at: lld/MachO/SymbolTable.cpp:243
+                            : osec->sectionEndSymbols)
+      .push_back(symtab->addSynthetic(sym.getName(), /*isec=*/nullptr,
+                                      /*value=*/-1, /*isPrivateExtern=*/true,
----------------
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.


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

https://reviews.llvm.org/D106629