[PATCH] D45508: Implement --ctors-in-init-array.
Rafael Avila de Espindola via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Wed Apr 11 17:35:43 PDT 2018
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
More information about the llvm-commits
mailing list