Page MenuHomeFreeBSD

Implement a BSD licensed crtbegin/crtend
ClosedPublic

Authored by andrew on Oct 16 2018, 4:31 PM.
Tags
None
Referenced Files
Unknown Object (File)
Thu, Nov 14, 9:13 AM
Unknown Object (File)
Wed, Nov 13, 6:09 AM
Unknown Object (File)
Tue, Nov 12, 12:57 AM
Unknown Object (File)
Thu, Oct 24, 1:56 PM
Unknown Object (File)
Oct 22 2024, 8:50 AM
Unknown Object (File)
Oct 22 2024, 2:46 AM
Unknown Object (File)
Oct 21 2024, 9:46 AM
Unknown Object (File)
Oct 21 2024, 8:54 AM

Details

Summary

These are needed for .ctors/.dtors and .jcr handling. The former needs
all the function pointers to be called in the correct order from the
.init/.fini section. The latter just needs to call a gcj specific function
if it exists with a pointer to the start of the .jcr section.

Tested on amd64 and arm64, other architectures still need to be
implemented.

Diff Detail

Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 20264
Build 19739: arc lint + arc unit

Event Timeline

andrew added inline comments.
gnu/lib/Makefile
5–7

This will be fixes before committing

Set the first item in .ctors/.dtors to all 1's

lib/csu/common/crtbegin.c
85

It is better to use .pushsection there

87

... and .popsection there.

That said, why do you put this for amd64 at all ? _NOINIT note is put into the binaries on all arches, meaning that rtld calls init/init_array elements for the dynamic binaries (and crt1 calls them for static),

andrew added inline comments.
lib/csu/common/crtbegin.c
87

I have a test [1] the .ctors & .dtors are run. Without this they aren't. I included this for compatibility with old toolchains that use these sections.

[1] https://github.com/zxombie/initfini

Use .pushsection & .popsection

lib/csu/common/crtbegin.c
12–14

I think we should avoid having a binary redistribution clause in these components that are linked into every output binary by the toolchain.

andrew added inline comments.
lib/csu/common/crtbegin.c
12–14

We already include it in many other csu files.

Call register_classes from .init. When called from a constructor on ppc64
it will be added to the .ctors section, but before CTOR_LIST so won't
be called.

Add the missing Makefile include for powerpc64

After some investigation, I see that crtbegin/crtend.o are shipped with compilers, at least it does with gcc. So I do not see a reason to provide the .ctor fallback for e.g. amd64 or i386: in-tree compiler does not need it, and out-of-tree gcc should cope on its own.

That said, not defining HAVE_INITFINI_ARRAY for all current architectures is highly confusing, because all arches in tree currently use init/fini arrays as the method of running user constructors. At least the name needs to be changed.

In D17587#375744, @kib wrote:

After some investigation, I see that crtbegin/crtend.o are shipped with compilers, at least it does with gcc. So I do not see a reason to provide the .ctor fallback for e.g. amd64 or i386: in-tree compiler does not need it, and out-of-tree gcc should cope on its own.

I don't think it's shipped with clang. Not supporting .ctors could be a problem when linking objects created with different compilers, e.g. a static library built with gcc, but linked through clang.

That said, not defining HAVE_INITFINI_ARRAY for all current architectures is highly confusing, because all arches in tree currently use init/fini arrays as the method of running user constructors. At least the name needs to be changed.

I can change it to a combination of HAVE_CTORS and CTORS_CONSTRUCTORS. The former is if we need to support .ctors, the latter is if the constructor attribute may be using .ctors.

Not supporting .ctors could be a problem when linking objects created with different compilers, e.g. a static library built with gcc, but linked through clang.

This is precisely the reason for maintaining .ctors/.dtors support; even if GCC uses .init_array/.fini_array on all platforms on FreeBSD now users may have existing .o files which do not.

Not supporting .ctors could be a problem when linking objects created with different compilers, e.g. a static library built with gcc, but linked through clang.

This is precisely the reason for maintaining .ctors/.dtors support; even if GCC uses .init_array/.fini_array on all platforms on FreeBSD now users may have existing .o files which do not.

I do not think that it is _reasonable_ to maintain ABI for .o's. and in fact we already do not do that. E.g. we change signatures of the versioned functions, which can be only handled for the completely linked targets.

Linux also does not guarantee ABI stability for the .o, you must recompile if you changed glibc version and intend to relink.

But I do not object against providing the call to .ctors, I am confused by the platforms selection for such compatibility, and by the names (Andrew promised to fix the later).

In D17587#375979, @kib wrote:

But I do not object against providing the call to .ctors, I am confused by the platforms selection for such compatibility, and by the names (Andrew promised to fix the later).

I've only included the platforms I can test. For the .init section we need the asm for each architecture that uses it, however I don't know this for each, and am unable to test on all platforms. The current set is based on what machines I have access to. I expect to add arm, i386, and some mips, but after that it's difficult for me to test.

lib/csu/common/crtbegin.c
12–14

Aha, we already have 1-clause BSD in the tree, we ought to use that.

SPDX link: https://spdx.org/licenses/BSD-1-Clause.html
Their reference: https://svnweb.freebsd.org/base/head/include/ifaddrs.h?revision=326823
Tag: SPDX-License-Identifier: BSD-1-Clause

  • Update the license
  • Add i386 support
  • Add a build option to enable

The build option is enabled on arm64 and powerpc64 as these are lower risc.
I would like more testing on amd64 and i386 (e.g. a ports exp run) before enabling by default there.

  • Add $FreeBSD$ to header files
  • Add WITH/WITHOUT descriptions
  • Remove HAVE_INITFINI_ARRAY and replace with HAVE_CTORS and CTORS_CONSTRUCTORS
kib added inline comments.
lib/csu/common/crtbegin.c
87

There should be \t before instruction/asm pseudo-ops.

This revision is now accepted and ready to land.Oct 23 2018, 4:20 PM
  • Add a tab before instructions in the .init section
  • Fix a logic inversion in an ifdef
This revision now requires review to proceed.Oct 23 2018, 4:31 PM

Could you add your initfini test as a (ATF?) test here?

lib/csu/Makefile.inc
20 ↗(On Diff #49513)

We'll want to make sure we test dynamically/statically linked binaries with ctors and exceptions.

Add simple crtbegin/crtend tests

lib/csu/Makefile
9–10 ↗(On Diff #49555)

Is this just the set of hosts where you've been able to validate the tests? Would be nice to enable on all archs (eventually).

lib/csu/common/crtend.c
34

ctors?

tools/build/options/WITHOUT_BSD_CRTBEGIN
2 ↗(On Diff #49513)

Should end with a .

Update based on feedback from emaste
Rework how the tests are built

tools/build/options/WITHOUT_BSD_CRTBEGIN
2 ↗(On Diff #49513)

Now I see why you might have omitted the trailing . :)

Perhaps this?

licensed
.Pa crtbegin.o
and
.Pa crtend .o .
andrew added inline comments.
lib/csu/Makefile
9–10 ↗(On Diff #49555)

This is the list of platforms with crt.h. I expect the others will have it after we commit this so we can enable the tests everwhere.

  • Add a C++ test
  • Add missing break statements to the default case in the fini switches
emaste added inline comments.
lib/csu/aarch64/crt.h
2

Perhaps put a comment here that arm64 does not use .ctors/.dtors or such? To explain why there's a seemingly-useless header.

This revision is now accepted and ready to land.Oct 25 2018, 1:13 PM
This revision was automatically updated to reflect the committed changes.
val_packett.cool added inline comments.
lib/csu/aarch64/crt.h
2

Huh, why doesn't arm64 use them?!

I've been porting ldc, the LLVM D Compiler, to FreeBSD/aarch64 — and wondering why the hell aren't ctors/dtros running…

andrew added inline comments.
lib/csu/aarch64/crt.h
2

On arm64 we use .init_array for constructors as it's defined in the ABI.