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

[George Rimar via Phabricator via llvm-commits]

grimar added inline comments.


================
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.
----------------
ruiu wrote:
> grimar wrote:
> > grimar wrote:
> > > I would write "while .init_array.65535 is the lowest"
> > My rewording was not entirely correct. Probably it could be the following:
> > 
> > ```
> > .ctors.0 is the lowest priority while .init_array.0 is the highest.
> > ```
> I don't think that's a better comment because it doesn't say anything about what is the highest value. Is that 100, 255 or some other value?
But your version also does not say what is the lowest value for `.init_array.*`, for example.
Can it be something like the following then?

```
// Section names may include priorities, e.g. .ctors.N or .init_array.N.
// For ".ctors.N" N is a value in [0..65535], where 0 means lowest priotity and 65535 is the highest.
// For ".init_array.N" ....

```


================
Comment at: lld/ELF/SyntheticSections.cpp:112
+    return Saver.save(".fini_array." + Twine(65535 - N));
+  }
+
----------------
ruiu wrote:
> grimar wrote:
> > 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));
> > }
> > ```
> > 
> > 
> > 
> > 
> You should avoid extra allocation, but I changed the code along with that direction.
I do not think it was worth to avoid the allocation here. .ctors and .dtors are probably too rare
to care about that.

Anyways you allocate additional vector right below, what seems to be much more sensitive :)
(btw one of the way would be to create a synthetic section for that and it would just write
initial data in a reversed order without allocating the additional buffers. But again, I do not think
it would worth to do for these sections).

Your version is OK for me.



https://reviews.llvm.org/D45508