[compiler-rt][crt] Simple crtbegin and crtend implementation

[phosek]

Summary

Clang relies on existence of certain symbols that are normally
provided by crtbegin.o/crtend.o. However, LLVM does not currently
provide implementation of these files, instead relying on either
libgcc or implementations provided as part of the system.

This change provides an initial implementation of crtbegin.o/crtend.o
that can be used on system that don't provide crtbegin.o/crtend.o as
part of their C library.

[beanz]

lib/crt/crtbegin.c

5–6 ↗

(On Diff #92748)

I've asked @chandlerc to chime in either himself, or someone from the foundation. My understanding of the way we handle licensing in LLVM is that only the runtime libraries are dual licensed UIUC & MIT. The reason being that the MIT license doesn't have the same attribution requirements as UIUC. LLVM, Clang, LLDB, LLD, etc. are all UIUC licensed only.

Since all of compiler-rt is under the UIUC/MIT dual license, I would be very concerned if this file needed different licensing since lib/builtins all get embedded in binaries in the same way, and are under that same license configuration.

[emaste]

lib/crt/crtbegin.c

5–6 ↗

(On Diff #92748)

Thanks, and fair enough.

The reason being that the MIT license doesn't have the same attribution requirements

I agree with you there's no need for this file to be licensed differently from the rest of the runtime components, and agree with the goal of avoiding attribution requirements on runtime components.

That said, a quick search turns up lots of opinions on MIT attribution and binary distribution from non-lawyers (and I am, of course, not a lawyer).

So, I think there's nothing to do for this file in isolation, but we should ensure the runtime components have the expected requirement (i.e., none) for binaries.

(I seem to recall being involved in a previous discussion of this with @chandlerc, but now cannot find any evidence of that discussion.)

[chandlerc]

lib/crt/crtbegin.c

5–6 ↗

(On Diff #92748)

We should not add more license diversity here. The current license structure of compiler-rt was an attempt to provide a workable license for runtime libraries in LLVM and they should continue to be used for this component of the runtime libraries.

There are many problems with the license structure we ended up with (see Chris's original emails), and indeed we are working on relicensing all of LLVM to address these concerns. We have worked *very* hard with lawyers to make sure the new license is an excellent choice for runtimes. I would encourage any further efforts to go into the new license, and I think being consistent with the rest of compiler-rt is fine in the interim.

[echristo]

Hi Joerg,

In D28791#671326, @joerg wrote:

You are wrong on (1), (2) and (3). But I am tired of this discussion. My objection stands. This is not a functional implementation for anything but a very narrow focus. There are much more complete implementations around. This creates more problems than it solves.

This is not a productive source of discussion. What path forward would you like here - saying that external projects have more complete implementations is also not a workable path forward. In particular, this could be a very good solution for platforms that aren't yours and provide a path forward for other platforms.

If you could be specific: what are you asking for this patch to do differently?

Thanks!

-eric

[chandlerc]

In addition to what Eric said, I just want to pull back up an old comment here:

In D28791#648932, @beanz wrote:

@joerg I completely disagree with you. Since libgcc provides those files for some platforms compiler-rt should as well.

I believe that one of the project goals of compiler-rt is to allow Clang + Compiler-RT to replace GCC + libGCC. We can't do that without these object files being produced, so I believe this complexity is necessary and justified.

Regardless of what happens for Fuschia, we have a real platform (Linux) with a libc that does *not* provide these bits and which expects the compiler to provide these bits. I would like LLVM to be an effective toolchain on that platform and provide the missing pieces here. Changing libc in this case isn't realistic -- these are existing platforms that may not be updating their libc.

If Fuschia wants to follow a similar design pattern (putting the CRT bits into the toolchain rather than the libc) I have no particular opinion on that being the "right" or "wrong" design. But it doesn't seem to be much of a burden on LLVM (if a burden at all) given that I think we will have to provide these components to support Linux without relying on libgcc.

[phosek]

I've rebased and updated the change. I've also included the Clang driver changes necessary to use this, but this can be landed in a separate followup change.

[echristo]

Looks like this stalled with no feedback outside of Chandler and I.

Joerg: We asked some very specific questions to which there don't appear to be answers coming from you and this patch doesn't affect any other target negatively here. Unless you come up with a compelling argument not to commit this other than "some platforms have support for this in other places" I think we need to commit this.

I'm going to ack in a week otherwise. If you feel I've overstepped here please let me know, but shutting down discussion doesn't seem to be productive here and I think Petr would like to make forward progress on support - and I think the project as a whole is better served by being able to serve as a full system compiler replacement; this is what compiler-rt is for ultimately anyhow.

Thanks!

[krytarowski]

Is there intention to support Clang/LLVM by GLIBC/GNU developers? Unlikely (knowing FSF track of reactions against LLVM projects). There are no gcc/libgcc licensing concerns for software hosted on GLIBC (LGPLv3) with libgcc. There already does exist a functional implementation for this OS. There is no need to treat it as an issue on the Clang/LLVM side (maybe except ambitions).

In my humble opinion we shall not expand the 'vendor lock' approach of libgcc/GNU to other platforms.

There are other BSD-licensed implementations and Fuchsia can borrow the code freely.

@joerg mentioned technical issues. I would add that this kind of software is implemented in rather highly OS-specific and ideally compiler-independent way. In my opinion it belongs to libc.

[phosek]

In D28791#1343013, @krytarowski wrote:

Is there intention to support Clang/LLVM by GLIBC/GNU developers? Unlikely (knowing FSF track of reactions against LLVM projects). There are no gcc/libgcc licensing concerns for software hosted on GLIBC (LGPLv3) with libgcc. There already does exist a functional implementation for this OS. There is no need to treat it as an issue on the Clang/LLVM side (maybe except ambitions).

In my humble opinion we shall not expand the 'vendor lock' approach of libgcc/GNU to other platforms.

There are other BSD-licensed implementations and Fuchsia can borrow the code freely.

@joerg mentioned technical issues. I would add this kind of software is implemented in rather highly OS-specific and ideally compiler-independent. In my opinion it belongs to libc.

I think there's a lot of confusion around this change since the discussion has been dragging for so many months, and some of it has been only carried only on the mailing list and not on Phabricator, so I'll try to summarize the state.

We don't need crtbegin.o/crtend.o on Fuchsia. Since we're building a new OS from scratch, we have a luxury of changing the design of various aspects of our system as needed and after the initial discussion on this change, we've changed the design of Fuchsia to avoid the need for crtbegin.o/crtend.o altogether.

However, we don't have the same luxury when it comes to Linux. None of the popular Linux libc implementations (I looked at glibc, musl, newlib, uclibc and dietlibc) provide crtbegin.o/crtend.o aside from Bionic. This is how crtbegin.o/crtend.o is handled on Linux. You may disagree with it, but I don't think this is the place to argue about that design. If we could go back in time, we could argue for a different design, but that's simply not feasible. There are already vendors that build Linux, including glibc, with Clang. We're currently building an entire Linux system using Clang but we still need to pull in libgcc for crtbegin.o/crtend.o, so this isn't an ambition, it's the last remaining blocker.

My goal is to ensure that Clang supports Linux without relying on libgcc as it does today, but I'm not trying to replace crtbegin.o/crtend.o on other systems. If you look at compiler-rt/cmake/config-ix.cmake in this patch, it's restricted only to Linux. This is definitely not the only OS-specific bit in compiler-rt, there are other runtimes that are only supported on a single platform.

AFAIK @joerg's comments regarding technical issues were referring to the initial version of my patch which was very limited and doesn't reflect current version. I believe that the current version should address his comments and I've asked multiple times for additional feedback in case it doesn't, but I never got any. If there are other technical issues you or anyone else is aware of, please let me know and I'd be happy to address them.

[krytarowski]

We're currently building an entire Linux system using Clang but we still need to pull in libgcc for crtbegin.o/crtend.o, so this isn't an ambition, it's the last remaining blocker.

What are the technical reasons for this to reimplement it in Clang instead of pulling it from libgcc?

[phosek]

In D28791#1343021, @krytarowski wrote:

We're currently building an entire Linux system using Clang but we still need to pull in libgcc for crtbegin.o/crtend.o, so this isn't an ambition, it's the last remaining blocker.

What are the technical reasons for this to reimplement it in Clang instead of pulling it from libgcc?

GCC's build system is quite monolithic and fragile and AFAIK it's not possible to build just crtbegin.o/crtend.o or libgcc without building the rest of GCC. crtbegin.o/crtend.o is produced by building crtstuff.c multiple times with different flags so building it outside of GCC's build system would mean replicating portion of GCC's build system and then keeping it up-to-date. I'm not saying that it's not possible to do this, but I'd like to be able to produce a fully working toolchain entirely from LLVM's build system, and neither of the two options outlined above get us any closer to that goal.

[krytarowski]

OK, if the GNU users intend to maintain copies of rather equivalent library in each compiler it's up to them/you to maintain it.. and keep tracking of future libgcc changes.

In order to prevent confusion I would add bold statements that this is for GNU only and not needed neither recommended for others.

[echristo]

In D28791#1343013, @krytarowski wrote:

Is there intention to support Clang/LLVM by GLIBC/GNU developers? Unlikely (knowing FSF track of reactions against LLVM projects). There are no gcc/libgcc licensing concerns for software hosted on GLIBC (LGPLv3) with libgcc. There already does exist a functional implementation for this OS. There is no need to treat it as an issue on the Clang/LLVM side (maybe except ambitions).

Yes. That intention is there and they've been receptive of patches.

In my humble opinion we shall not expand the 'vendor lock' approach of libgcc/GNU to other platforms.

There are other BSD-licensed implementations and Fuchsia can borrow the code freely.

This is irrelevant.

@joerg mentioned technical issues. I would add that this kind of software is implemented in rather highly OS-specific and ideally compiler-independent way. In my opinion it belongs to libc.

That's fine, it's just not the state of the world and we should support compiling appropriately.

Thanks.

-eric

[krytarowski]

This is irrelevant.

It was explained clear that Fuchsia won't follow it.

That's fine, it's just not the state of the world and we should support compiling appropriately.

I don't want to pick a fight, but there can be used exactly same argument to defend pulling libgcc here.

But the final word is rather to platform maintainers.

[echristo]

That's fine, it's just not the state of the world and we should support compiling appropriately.

I don't want to pick a fight, but there can be used exactly same argument to defend pulling libgcc here.

You have been picking a fight. And no, it's not the same argument. Please desist.

[chandlerc]

FWIW, I completely support landing this. I think the opposition is essentially "it would be better to not design a libc in that way" which is certainly a valid opinion, but given the existence of such libc designs widely deployed and widely used, it seems very good to lay groundwork for LLVM to support them.

I also think we should *not* be pointing at GNU or any other project here, and instead simply state that these exist to support using LLVM in conjunction with systems whose libc chooses this particular design pattern.

I don't think LLVM should be in the business of trying to cast some kind of "you should feel bad" judgement on such systems. If someone in the LLVM community wants to develop a libc as part of LLVM (which I would quite like to see), then that is the place to debate libc design decisions, and not anywhere else.

So, that said, does anyone have comments on the code itself? If no one is up to commenting on the code and giving Petr direct feedback there, I'll just LGTM this because I think it is long (looooooong) overdue.

Lastly, to address specific questions here:

In D28791#1343047, @krytarowski wrote:

That's fine, it's just not the state of the world and we should support compiling appropriately.

I don't want to pick a fight, but there can be used exactly same argument to defend pulling libgcc here.

That doesn't make sense to me -- the LLVM project has lots of reasons why it doesn't pull GPL-ed code into its tree.

But yes, we are going to be in the business of being 100% and fully compatible with libgcc in order to effectively support platforms based on libcs that follow the design pattern of glibc, certainly including platforms that use glibc. Is that work? Yes. Should the project do it? Absolutely yes. Just like we work hard to be compatible with the gcc commandline and other layers of compatibility, we should work to support these platforms well.

[kristina]

In D28791#1343050, @chandlerc wrote:

FWIW, I completely support landing this. I think the opposition is essentially "it would be better to not design a libc in that way" which is certainly a valid opinion, but given the existence of such libc designs widely deployed and widely used, it seems very good to lay groundwork for LLVM to support them.

I also think we should *not* be pointing at GNU or any other project here, and instead simply state that these exist to support using LLVM in conjunction with systems whose libc chooses this particular design pattern.

I don't think LLVM should be in the business of trying to cast some kind of "you should feel bad" judgement on such systems. If someone in the LLVM community wants to develop a libc as part of LLVM (which I would quite like to see), then that is the place to debate libc design decisions, and not anywhere else.

So, that said, does anyone have comments on the code itself? If no one is up to commenting on the code and giving Petr direct feedback there, I'll just LGTM this because I think it is long (looooooong) overdue.

Lastly, to address specific questions here:

In D28791#1343047, @krytarowski wrote:

That's fine, it's just not the state of the world and we should support compiling appropriately.

I don't want to pick a fight, but there can be used exactly same argument to defend pulling libgcc here.

That doesn't make sense to me -- the LLVM project has lots of reasons why it doesn't pull GPL-ed code into its tree.

But yes, we are going to be in the business of being 100% and fully compatible with libgcc in order to effectively support platforms based on libcs that follow the design pattern of glibc, certainly including platforms that use glibc. Is that work? Yes. Should the project do it? Absolutely yes. Just like we work hard to be compatible with the gcc commandline and other layers of compatibility, we should work to support these platforms well.

Definitely agree with the points here, would be nice to have an independent replacement for libgcc, with Csu being a major part of it and lay groundwork for further efforts to have an alternative compiler runtime that is suitable for general purpose use without being reliant on GNU-provided libraries or toolchain components.

[chandlerc]

(FWIW, I think what this patch is really waiting on at this point is a genuinely detailed code review of the code... But if someone has done that already and I just missed it in all the other emails, please speak up with a fresh LGTM!)

[joerg]

For the clang side, I don't understand why Driver::GetFilePath is not good enough. This shouldn't need all toolchain changes at all.

For the compiler-rt side:
(1) I would drop the const for .ctor/dtor, since the content has to be writeable anyway for function pointer relocations.
(2) I'm trying to remember what the alignment rules for .eh_frame are. There seem to be different approaches here in the wild.
(3) .ctor/.dtor should be using pointer types, not long. sizeof(void (*)(*)) == sizeof(long) is a very questionable assumption.
(4) CTOR_END and DTOR_END are unused and will just be dropped.
(5) The logic around CRT_HAS_INITFINI_ARRAY is strange. If it is not set, do_init and do_fini must be called from .init/.fini with appropiate assembler code. Setting .init_array/.fini_array can't work in that case.
(6) Putting read-only content into .eh_frame can be a problem on MIPS for many GNU ld versions. It used to bitch about mixing read-only and read-write sections and the default DWARF encoding required writable .eh_frame.

[phosek]

In D28791#1357948, @joerg wrote:

For the clang side, I don't understand why Driver::GetFilePath is not good enough. This shouldn't need all toolchain changes at all.

I split the driver changes from this patch to make it smaller.

The reason why Driver::GetFilePath is not enough is naming. These files are installed to lib/clang/<version>/lib/linux and there can be versions for multiple architectures side-by-side so I cannot use just crtbegin.o. compiler-rt CMake's build would derive e.g. clang_rt.crtbegin-x86_64.o as the file name, and ToolChain::getCompilerRT contains the logic to construct that name based on the target.

For the compiler-rt side:
(1) I would drop the const for .ctor/dtor, since the content has to be writeable anyway for function pointer relocations.

Done

(2) I'm trying to remember what the alignment rules for .eh_frame are. There seem to be different approaches here in the wild.

I went with with sizeof(void *) which is what libgcc's implementation seem to be using.

(3) .ctor/.dtor should be using pointer types, not long. sizeof(void (*)(*)) == sizeof(long) is a very questionable assumption.

Done

(4) CTOR_END and DTOR_END are unused and will just be dropped.

They're used now.

(5) The logic around CRT_HAS_INITFINI_ARRAY is strange. If it is not set, do_init and do_fini must be called from .init/.fini with appropiate assembler code. Setting .init_array/.fini_array can't work in that case.

Done

(6) Putting read-only content into .eh_frame can be a problem on MIPS for many GNU ld versions. It used to bitch about mixing read-only and read-write sections and the default DWARF encoding required writable .eh_frame.

[matthewbauer]

compiler-rt/lib/crt/crtend.c

11 ↗

(On Diff #188094)

inttypes.h is provided by the libc while stdint.h is provided by clang. Is it possible to use stdint.h here instead?

[MaskRay]

compiler-rt/lib/crt/crtbegin.c

17 ↗

(On Diff #188496)

Should there be a #ifdef macro to exclude __dso_handle? glibc libc.a also provides __dso_handle.

21 ↗

(On Diff #188496)

Remove , visibility("hidden")

The visibility attribute is ignored for static variables and GCC would emit a -Wattributes warning.

[MaskRay]

Another thing worth mentioning, either in this patch or in a follow-up, crtbegin.o

compiler-rt/lib/crt/CMakeLists.txt

81 ↗

(On Diff #188496)

Just to ask if my understanding is correct.

clang_rt.crtbegin.o: replacement of crtbegin.o
(-fno-PIC can be used here as some toolchains default to -fPIC or -fPIE, but it doesn't matter much here)

clang_rt.crtbegin_shared.o: replacement of crtbeginS.o crtbeginT.o

[matthewbauer]

I was trying to build compiler-rt with WebAssembly and forgot I still had this patched applied. But it looks like it breaks when doing this and things work if you disble crt. You probably just don't want to build crt with WebAssembly targets?

[phosek]

In D28791#1414044, @matthewbauer wrote:

I was trying to build compiler-rt with WebAssembly and forgot I still had this patched applied. But it looks like it breaks when doing this and things work if you disble crt. You probably just don't want to build crt with WebAssembly targets?

Does WebAssembly build set OS_NAME to "Linux" and architecture to one of x86, x86-64, ARM and ARM64? See config-ix.cmake, specifically this line if (CRT_SUPPORTED_ARCH AND OS_NAME MATCHES "Linux") should ensure that this is only built on Linux and set(ALL_CRT_SUPPORTED_ARCH ${X86} ${X86_64} ${ARM32} ${ARM64}) should ensure that it's only built for x86, x86-64, ARM and ARM64.

[phosek]

compiler-rt/lib/crt/CMakeLists.txt

81 ↗

(On Diff #188496)

Correct, crtbeginS.o and crtbeginT.o don't match the naming conventions of other compiler-rt artifacts, so I came up with this scheme which should be a better fit for compiler-rt. I'm open to other suggestions though.

compiler-rt/lib/crt/crtbegin.c

17 ↗

(On Diff #188496)

I don't think this should be conditional, other crtbegin.o implementations also define this symbol unconditionally. It should be also usable independently of the C library being use (i.e. it should work with both glibc and musl).

[matthewbauer]

In D28791#1414633, @phosek wrote:

In D28791#1414044, @matthewbauer wrote:

I was trying to build compiler-rt with WebAssembly and forgot I still had this patched applied. But it looks like it breaks when doing this and things work if you disble crt. You probably just don't want to build crt with WebAssembly targets?

Does WebAssembly build set OS_NAME to "Linux" and architecture to one of x86, x86-64, ARM and ARM64? See config-ix.cmake, specifically this line if (CRT_SUPPORTED_ARCH AND OS_NAME MATCHES "Linux") should ensure that this is only built on Linux and set(ALL_CRT_SUPPORTED_ARCH ${X86} ${X86_64} ${ARM32} ${ARM64}) should ensure that it's only built for x86, x86-64, ARM and ARM64.

Ah ok, yeah this was my issue. CMake defaults to CMAKE_HOST_SYSTEM_NAME when no CMAKE_SYSTEM_NAME is provided. So it assumed I was targeting "wasm32-unknown-linux-gnu" not "wasm32-unknown-unknown-wasm".

[MaskRay]

compiler-rt/lib/crt/CMakeLists.txt

81 ↗

(On Diff #188496)

Since crtbeginT.o is used by -static (gcc -dumpspecs: %{static:crtbeginT.o%s; shared|!no-pie:crtbeginS.o%s; :crtbegin.o%s}), crtbegin_shared may not be a good name. But I don't know of a better name, either...

Other than this, other parts look great! Thank you for doing this.

[phosek]

compiler-rt/lib/crt/CMakeLists.txt

81 ↗

(On Diff #188496)

Android seems t be using crtbegin_dynamic.o but I don't know if that's much better?

[MaskRay]

compiler-rt/lib/crt/CMakeLists.txt

81 ↗

(On Diff #188496)

Given a second thought. crtbegin_shared.o looks good to me now. I propose one change: adding -fPIC to the crtbegin.o rule.

While I still don't understand why ligcc seems to define void *__dso_handle = &dso_handle; in crtbeginT.o, I cannot find a relocation in my /usr/lib/gcc/x86_64-linux-gnu/8/crtbeginT.o, not sure if it has other rule to remove that...

# https://github.com/gcc-mirror/gcc/blob/master/libgcc/Makefile.in
crtbeginT$(objext): $(srcdir)/crtstuff.c
	$(crt_compile) $(CRTSTUFF_T_CFLAGS) -c $< -DCRT_BEGIN -DCRTSTUFFT_O

// https://github.com/gcc-mirror/gcc/blob/master/libgcc/crtstuff.c
#ifdef CRTSTUFFS_O
void *__dso_handle = &__dso_handle; // crtbeginS.o crtbeginT.o
#else
void *__dso_handle = 0; // crtbegin.o
#endif

If we unify 2 of the 3, I think unifying crtbegin.o and crtbeginT.o rather than unifying crtbeginS.o crtbeginT.o makes more sense, but we'd need -fPIC for crtbegin.o (-static -pie, or -fPIC object files going into -static link). This resembles the status I observed from my Linux crtbegin*.o and https://svnweb.freebsd.org/base/head/lib/csu/common/crtbegin.c?view=markup

I even think unifying all 3 is possible. __dso_handle doesn't need to be 0 if all modules (including LD_PRELOAD DSOs) are unloaded before the main executable. There would be no difference if a nullptr or another address is used as the __cxa_finalize argument in that case. There may be cases I missed, so the current approach (unifying 2) is good enough :)

[MaskRay]

The current division (crtbegin_shared.o (-shared or -pie) crtbegin.o (others)) resembles what NetBSD and OpenBSD do. They don't have a separate crtbeginT.o. I don't understand why in a 2012 change FreeBSD added crtbeginT.o...

I casually asked dalias to confirm my thoughts. He thinks crtbegin.o belongs to the compiler runtime side rather than libc ("its purely a mechanism for weird language runtimes the compiler supports; libc has no use for it or relation to it").

He also agrees that unifying all 3 crtbegin{,S,T}.o should be possible ("there is no reason for any version but the pic-compatible (S) one to exist. others just make logic of which to link complex for no reason").

(((That could expose some bugs in existing applications depending on bug behaviors, though: I believe glibc still has the bug (or at least an unexpected behavior) that for a -pie -fPIE executable, __cxa_finalizer(main's __dso_handle) is called before __cxa_finalizer(a depending DSO's __dso_handle). Making void *__dso_handle = &__dso_handle for non -pie non -shared builds may reveal some bugs..)))
(((We are better than libgcc here. Its crtbeginT.o doesn't allow clang -static -shared a.c -o a.so why we do! https://bugzilla.redhat.com/show_bug.cgi?id=214465)))

Now I more firmly believe that this patch moves towards the correct direction to make compiler-rt a better replacement of libgcc while also paying back some tech debt. Still LG!

[phosek]

@joerg is it okay with you if I go ahead and land this?

[E5ten]

compiler-rt/lib/crt/CMakeLists.txt

60 ↗

(On Diff #189526)

This causes a cmake error for me: string sub-command FIND requires 3 or 4 parameters., I guess because one of the 2 variables there are empty, quoting the 2 variables fixes the issue for me.

[joerg]

LGTM from my perspective at least.

[phillipjohnston]

compiler-rt/lib/crt/crtbegin.c

21 ↗

(On Diff #190046)

Is there an alternative to a zero-length array here?

[phosek]

compiler-rt/lib/crt/crtbegin.c

21 ↗

(On Diff #190046)

Alternative would be to use assembly but that's less portable and I'm not sure if we would gain anything by doing so.

[hfinkel]

Looks like this needs to be updated with the new license headers.

[phosek]

In D28791#1455864, @hfinkel wrote:

Looks like this needs to be updated with the new license headers.

Done

[thakis]

compiler-rt/trunk/lib/crt/crtbegin.c

7

gcc warns about this:

/b/swarming/w/ir/k/src/third_party/llvm/projects/compiler-rt/lib/crt/crtend.c:1:1: warning: C++ style comments are not allowed in ISO C90 [enabled by default]

Could we either use /**/ comments or pass -std=c99 for this file?