Page MenuHomeFreeBSD

Implement a BSD licensed crtbegin/crtend
ClosedPublic

Authored by andrew on Oct 16 2018, 4:31 PM.

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

Repository
rS FreeBSD src repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

andrew created this revision.Oct 16 2018, 4:31 PM
andrew marked an inline comment as done.Oct 16 2018, 4:33 PM
andrew added inline comments.
gnu/lib/Makefile
5–6 ↗(On Diff #49229)

This will be fixes before committing

andrew updated this revision to Diff 49238.Oct 16 2018, 5:44 PM

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

andrew added a reviewer: kib.Oct 16 2018, 5:46 PM
kib added inline comments.Oct 17 2018, 11:07 AM
lib/csu/common/crtbegin.c
84 ↗(On Diff #49238)

It is better to use .pushsection there

86 ↗(On Diff #49238)

... 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 marked an inline comment as done.Oct 17 2018, 11:49 AM
andrew added inline comments.
lib/csu/common/crtbegin.c
86 ↗(On Diff #49238)

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

andrew updated this revision to Diff 49250.Oct 17 2018, 12:07 PM

Use .pushsection & .popsection

emaste added inline comments.Oct 17 2018, 3:25 PM
lib/csu/common/crtbegin.c
11–13 ↗(On Diff #49250)

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

andrew marked an inline comment as done.Oct 17 2018, 4:21 PM
andrew added inline comments.
lib/csu/common/crtbegin.c
11–13 ↗(On Diff #49250)

We already include it in many other csu files.

andrew updated this revision to Diff 49258.Oct 17 2018, 5:23 PM

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.

andrew updated this revision to Diff 49259.Oct 17 2018, 5:25 PM

Add the missing Makefile include for powerpc64

kib added a comment.Oct 18 2018, 11:05 AM

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.

kib added a comment.Oct 18 2018, 9:40 PM

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.

emaste added inline comments.Oct 20 2018, 6:37 PM
lib/csu/common/crtbegin.c
11–13 ↗(On Diff #49250)

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

andrew updated this revision to Diff 49509.Oct 23 2018, 3:40 PM
  • 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.

andrew updated this revision to Diff 49512.Oct 23 2018, 4:18 PM
  • Add $FreeBSD$ to header files
  • Add WITH/WITHOUT descriptions
  • Remove HAVE_INITFINI_ARRAY and replace with HAVE_CTORS and CTORS_CONSTRUCTORS
kib accepted this revision.Oct 23 2018, 4:20 PM
kib added inline comments.
lib/csu/common/crtbegin.c
86 ↗(On Diff #49509)

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

This revision is now accepted and ready to land.Oct 23 2018, 4:20 PM
andrew updated this revision to Diff 49513.Oct 23 2018, 4:31 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?

emaste added inline comments.Oct 24 2018, 2:33 PM
lib/csu/Makefile.inc
20 ↗(On Diff #49513)

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

andrew updated this revision to Diff 49555.Oct 24 2018, 3:40 PM

Add simple crtbegin/crtend tests

emaste added inline comments.Oct 24 2018, 4:32 PM
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
33 ↗(On Diff #49555)

ctors?

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

Should end with a .

andrew updated this revision to Diff 49567.Oct 24 2018, 5:02 PM

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

emaste added inline comments.Oct 24 2018, 5:05 PM
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 updated this revision to Diff 49569.Oct 24 2018, 5:14 PM

Use .Pa

andrew marked an inline comment as done.Oct 24 2018, 5:20 PM
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.

andrew updated this revision to Diff 49573.Oct 24 2018, 5:59 PM
  • Add a C++ test
  • Add missing break statements to the default case in the fini switches
emaste accepted this revision.Oct 25 2018, 1:13 PM
emaste added inline comments.
lib/csu/aarch64/crt.h
1 ↗(On Diff #49573)

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.
greg_unrelenting.technology added inline comments.
lib/csu/aarch64/crt.h
1 ↗(On Diff #49573)

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 marked an inline comment as done.Oct 25 2018, 9:10 PM
andrew added inline comments.
lib/csu/aarch64/crt.h
1 ↗(On Diff #49573)

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