This is an archive of the discontinued LLVM Phabricator instance.

Implement --ctors-in-init-array.
AbandonedPublic

Authored by MaskRay on Apr 10 2018, 7:43 PM.

Details

Summary

This patch defines InitFiniSection synthetic section to convert
.[cd]tors to .{init,fini}_array.

Unlike https://reviews.llvm.org/D35509, this patch also adds
--no-ctors-in-init-array option so that you can choose to not merge
.[cd]tors into .{init,fini}_array.

Event Timeline

ruiu created this revision.Apr 10 2018, 7:43 PM
grimar added inline comments.Apr 11 2018, 3:35 AM
lld/ELF/OutputSections.cpp
331–332

Should these two just be inlined instead?

lld/ELF/SyntheticSections.cpp
92

I would write "while .init_array.65535 is the lowest"

103

unsigned?

104

I think we need to check what to_integer returns.

112

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));
}
128

runtime

lld/ELF/Writer.cpp
403

I would probably use

for (InputSectionBase *&Base : InputSections)

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

grimar added inline comments.Apr 11 2018, 3:44 AM
lld/ELF/SyntheticSections.cpp
92

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.
ruiu marked 2 inline comments as done.Apr 11 2018, 1:00 PM
ruiu added inline comments.
lld/ELF/OutputSections.cpp
331–332

No I don't think so.

lld/ELF/SyntheticSections.cpp
92

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?

103

In general, you shouldn't use unsigned if the only reason to do that is a number cannot be negative, but in this particular case, I think uint16_t is the best choice.

112

You should avoid extra allocation, but I changed the code along with that direction.

lld/ELF/Writer.cpp
403

I disagree. A reference to a pointer to something is harder to understand than an just integer.

ruiu updated this revision to Diff 142064.Apr 11 2018, 1:04 PM
ruiu marked an inline comment as done.
  • address review comments

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

lld/ELF/OutputSections.h
124

Why in OutputSections.h?

lld/ELF/SyntheticSections.cpp
98

Could this use ".init_array"? Should elf::getPriority return 65537 for ".init_array"?

119

using make<std::vector> looks somewhat odd. Either allocate a plain buffer or add a comment that we intentionally "leak" this std::vector.

lld/ELF/Writer.cpp
402

Could this logic be in InputFile so that we don't even create an InputSection for the .ctor/.dtor sections?

403

For what it is worth I also prefer the *& version.

grimar added inline comments.Apr 12 2018, 1:47 AM
lld/ELF/SyntheticSections.cpp
92

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" ....
112

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.

ruiu added a comment.Apr 12 2018, 2:29 PM

I found that this patch isn't correct. Sorry guys. The issue is that we need to reverse the contents after applying relocations. This patch reverses it before applying relocations. I need to fix that. I'll update the patch.

I found that this patch isn't correct. Sorry guys. The issue is that we need to reverse the contents after applying relocations. This patch reverses it before applying relocations. I need to fix that. I'll update the patch.

Hi, may I ask about the detail for the new patch. I just met the problems that sections in .ctors need to be added to .init_array.

I rebased this patch, did some simple change to make it work at trunk and reverse the contents of .ctors after applying relocations as @ruiu mentioned in comments. May I ask is it ok for me to create a new diffusion? I am not familiar with the rule for the situation in the LLVM community convention. @MaskRay @grimar

haampie added a subscriber: haampie.EditedJun 15 2022, 6:08 AM

Would be nice to have this (enabled by default, like in binutils ld), since due to an oversight, GCC 12 is the first GCC to actually enable .init_array for cross-compilers targeting Linux. [1]

That leads to annoying issues where building clang with such a GCC < 12 produces a libclang_rt.profile.a with .ctors, so that clang -fprofile-generate -fuse-ld=lld simply does not work: it ends up mixing .ctors from the runtime library and .init_array from the application, glibc doesn't execute the .ctors bits, but those contain the constructor of a global that initializes profiling. [2]

This is not an issue with ld, since it generates ELF files with just .init_array by default. So clang -fprofile-generate -fuse-ld=ld works just fine.

[1] https://gcc.gnu.org/bugzilla/show_bug.cgi?id=100896
[2] https://github.com/JuliaLang/julia/pull/45641

Herald added a project: Restricted Project. · View Herald TranscriptJun 15 2022, 6:08 AM
Herald added a subscriber: StephenFan. · View Herald Transcript

Would be nice to have this (enabled by default, like in binutils ld), since due to an oversight, GCC 12 is the first GCC to actually enable .init_array for cross-compilers targeting Linux. [1]

That leads to annoying issues where building clang with such a GCC < 12 produces a libclang_rt.profile.a with .ctors, so that clang -fprofile-generate -fuse-ld=lld simply does not work: it ends up mixing .ctors from the runtime library and .init_array from the application, glibc doesn't execute the .ctors bits, but those contain the constructor of a global that initializes profiling. [2]

This is not an issue with ld, since it generates ELF files with just .init_array by default. So clang -fprofile-generate -fuse-ld=ld works just fine.

[1] https://gcc.gnu.org/bugzilla/show_bug.cgi?id=100896
[2] https://github.com/JuliaLang/julia/pull/45641

This patch may be more relevant few years ago but at this point, it mostly just adds come complexity which will need to be cleaned up soon.

One can run objcopy --rename-section .ctors=.init_array --rename-section .dtors=.fini_array a.o to fix object files using .ctors/.dtors.

Note that the upstream GCC configuration deviates considerably with what some distros use in practice.
Many specify --enable-initfini-array explicitly.
(Deviate this topic a bit: if you'd like to help, there is a default-pie thing https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103398)

glibc doesn't execute the .ctors bits.

This is not so accurate. See https://maskray.me/blog/2021-11-07-init-ctors-init-array#ctors-and-.dtors

haampie added a comment.EditedJun 16 2022, 1:05 AM

@MaskRay, not sure if I follow

This is not so accurate.

Do you mean it's gcc not libc? In any case, when using lld, constructors in object files compiled with default cross GCC < 12 configuration are not run:

$ cat foo.cc 
#include <iostream>
class Foo {
public:
    Foo() { std::cout << "foo\n"; }
};
Foo foo;

$ cat main.cc 
#include <iostream>
int main() {
    std::cout << "hello\n";
}

$ gcc --version
x86_64-linux-gnu-gcc (GCC) 8.1.0

$ gcc -c foo.cc
$ clang++ main.cc foo.o 
$ ./a.out 
foo
hello

$ clang++ -fuse-ld=lld main.cc foo.o 
$ ./a.out 
hello

Is that expected or not?

Edit: ah, I think I get it. GCC provides ctrbegin.o and crtend.o with the sentinel values, but if GCC is compiled with --enable-initfini-array, those are omitted. So you simply can't use object files with ctors if your GCC is compiled with --enable-initfini-array. And in the example above, the initial GCC was compiled without --enable-initfini-array, and the clang later uses a different system GCC under the hood which was compiled with --enable-initfini-array.

In any case, it would be much more convenient if lld had the same behavior as ld, cause this is a pain to debug.

MaskRay added a comment.EditedJun 16 2022, 10:55 AM

@MaskRay, not sure if I follow

This is not so accurate.

Do you mean it's gcc not libc? In any case, when using lld, constructors in object files compiled with default cross GCC < 12 configuration are not run:

$ cat foo.cc 
#include <iostream>
class Foo {
public:
    Foo() { std::cout << "foo\n"; }
};
Foo foo;

$ cat main.cc 
#include <iostream>
int main() {
    std::cout << "hello\n";
}

$ gcc --version
x86_64-linux-gnu-gcc (GCC) 8.1.0

$ gcc -c foo.cc
$ clang++ main.cc foo.o 
$ ./a.out 
foo
hello

$ clang++ -fuse-ld=lld main.cc foo.o 
$ ./a.out 
hello

Is that expected or not?

Edit: ah, I think I get it. GCC provides ctrbegin.o and crtend.o with the sentinel values, but if GCC is compiled with --enable-initfini-array, those are omitted. So you simply can't use object files with ctors if your GCC is compiled with --enable-initfini-array. And in the example above, the initial GCC was compiled without --enable-initfini-array, and the clang later uses a different system GCC under the hood which was compiled with --enable-initfini-array.

In any case, it would be much more convenient if lld had the same behavior as ld, cause this is a pain to debug.

Expected. And your analysis is about right. It would be convenient few years ago, but the value is significantly low nowadays, now with the GCC configure.ac fixed.

Note: even several years ago, the GCC default configure.ac behavior may not be a strong enough motivation. That is mainly a "GCC default has diverged a lot from the practice" issue.
I agree that we want to make system GCC usable with lld, but that doesn't mean a random manual build of GCC forgetting --enable-initfini-array.

Hey folks,

Is this patch still relevant to the upstream community ?

GCC12 is starting to be deployed more and more in production and this kind of issues are at scale for all kinds of companies, so there is interest to get this landed or to find a workaround that does not include rebuilding everything.

LD' s inker script has for years put all .{c,d]tors in .{init,fini}_array. Do you see any issue in implementing the same for lld ?

MaskRay commandeered this revision.Feb 17 2023, 11:45 AM
MaskRay edited reviewers, added: ruiu; removed: MaskRay.

Hey folks,

Is this patch still relevant to the upstream community ?

No, it is increasingly unnecessary, see https://maskray.me/blog/2021-11-07-init-ctors-init-array , especially the chapter ".ctors to .init_array transition".

GCC12 is starting to be deployed more and more in production and this kind of issues are at scale for all kinds of companies, so there is interest to get this landed or to find a workaround that does not include rebuilding everything.

LD' s inker script has for years put all .{c,d]tors in .{init,fini}_array. Do you see any issue in implementing the same for lld ?

I think you misunderstand things. GCC 4.7 switched from .ctors to .init_array . At the current GCC 12 time .ctors is very obsoleted now.

MaskRay abandoned this revision.Feb 17 2023, 11:45 AM

I think you misunderstand things. GCC 4.7 switched from .ctors to .init_array . At the current GCC 12 time .ctors is very obsoleted now.

I don't think I am. I was debugging this issue today. I had some binaries and libraries built and linked with gcc9, now while transitioning to gcc12 I can clearly see that that .ctors from those libraries are present but are not executed when the resulting binary is linked with lld, only .init_array.

But sure whatever, I really didn't expect anything else with (atleast) 3 attempts to fix this upstream.

I think you misunderstand things. GCC 4.7 switched from .ctors to .init_array . At the current GCC 12 time .ctors is very obsoleted now.

I don't think I am. I was debugging this issue today. I had some binaries and libraries built and linked with gcc9, now while transitioning to gcc12 I can clearly see that that .ctors from those libraries are present but are not executed when the resulting binary is linked with lld, only .init_array.

But sure whatever, I really didn't expect anything else with (atleast) 3 attempts to fix this upstream.

You might build a GCC by yourself, instead of using a distro-provided one. See https://gcc.gnu.org/bugzilla/show_bug.cgi?id=100896

If you want GCC's .ctors work without .ctors=>.init_array conversion, you can patch GCC libgcc/crtstuff.c to always support .ctors.
Alternatively, search objcopy on https://maskray.me/blog/2021-11-07-init-ctors-init-array (just updated).