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

[George Rimar via Phabricator via llvm-commits]

grimar added inline comments.


================
Comment at: lld/ELF/OutputSections.cpp:331
 
-static bool isCrtbegin(StringRef S) { return isCrtBeginEnd(S, "crtbegin"); }
-static bool isCrtend(StringRef S) { return isCrtBeginEnd(S, "crtend"); }
+bool elf::isCrtbegin(StringRef S) { return isCrtBeginEnd(S, "crtbegin"); }
+bool elf::isCrtend(StringRef S) { return isCrtBeginEnd(S, "crtend"); }
----------------
Should these two just be inlined instead?


================
Comment at: lld/ELF/SyntheticSections.cpp:92
+// Section names may include priorities, e.g. .ctors.30 or .init_array.101.
+// .ctors.65535 is the highest priority while .init_array.0 is the highest.
+// So we need to translate the number.
----------------
I would write "while .init_array.65535 is the lowest"


================
Comment at: lld/ELF/SyntheticSections.cpp:103
+  if (S.startswith(".ctors.")) {
+    int N = 0;
+    to_integer(S.substr(7), N, 10);
----------------
unsigned?


================
Comment at: lld/ELF/SyntheticSections.cpp:104
+    int N = 0;
+    to_integer(S.substr(7), N, 10);
+    return Saver.save(".init_array." + Twine(65535 - N));
----------------
I think we need to check what `to_integer` returns.


================
Comment at: lld/ELF/SyntheticSections.cpp:112
+    return Saver.save(".fini_array." + Twine(65535 - N));
+  }
+
----------------
Since you do not pass anything else than
```
".ctors" / ".ctors." / ".dtors" / ".dtors."
```
(from `createInitFiniSections`)


The code could probably be something like:

```
static StringRef toInitFiniName(StringRef S, uint32_t Type) {
  StringRef Prefix = (Type == SHT_INIT_ARRAY) ? ".init_array." : ".fini_array.";
  if (S.back() != '.')
    return Saver.save(Prefix + "65537");

  unsigned N = 0;
  if (to_integer(S.substr(7), N, 10))
    error("eeerrr");
  return Saver.save(Prefix + Twine(65535 - N));
}
```






================
Comment at: lld/ELF/SyntheticSections.cpp:128
+  // A .[cd]tors section contains function pointers that are executed at
+  // rutnime from the last one to the first one. .{init,fini}_array also
+  // contains function pointers, but the order the runtime calls them is the
----------------
ru**nt**ime


================
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]);
----------------
I would probably use

`for (InputSectionBase *&Base : InputSections)`

because the presence of `I` makes me search around where it is used as an index.


https://reviews.llvm.org/D45508