[PATCH] D45508: Implement --ctors-in-init-array.

[Rafael Avila de Espindola via Phabricator via llvm-commits]

espindola added a comment.

I like the general idea. These days it is hopefully uncommon to encounter .ctor/.dtor sections so having something simple is better.



================
Comment at: lld/ELF/OutputSections.h:124
+bool isCrtbegin(StringRef S);
+bool isCrtend(StringRef S);
 
----------------
Why in OutputSections.h?


================
Comment at: lld/ELF/SyntheticSections.cpp:98
+  if (S == ".ctors")
+    return ".init_array.65537";
+  if (S == ".dtors")
----------------
Could this use ".init_array"? Should elf::getPriority return 65537 for ".init_array"?


================
Comment at: lld/ELF/SyntheticSections.cpp:119
+InputSection *elf::createInitFiniSection(InputSection *Sec) {
+  auto *Contents = make<std::vector<uint8_t>>(Sec->Data.size());
+
----------------
using make<std::vector> looks somewhat odd. Either allocate a plain buffer or add a comment that we intentionally "leak" this std::vector.


================
Comment at: lld/ELF/Writer.cpp:403
+static void createInitFiniSections() {
+  for (size_t I = 0; I < InputSections.size(); ++I) {
+    InputSection *Sec = dyn_cast<InputSection>(InputSections[I]);
----------------
ruiu wrote:
> grimar wrote:
> > I would probably use
> > 
> > `for (InputSectionBase *&Base : InputSections)`
> > 
> > because the presence of `I` makes me search around where it is used as an index.
> I disagree. A reference to a pointer to something is harder to understand than an just integer.
For what it is worth I also prefer the *& version.


================
Comment at: lld/ELF/Writer.cpp:402
+// Replace .[cd]tors sections with .{init,fini}_array sections.
+static void createInitFiniSections() {
+  for (size_t I = 0; I < InputSections.size(); ++I) {
----------------
Could this logic be in InputFile so that we don't even create an InputSection for the .ctor/.dtor sections?


https://reviews.llvm.org/D45508