Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

DynamicLoader+LibC: Link LibC into DynamicLoader --as-sane-people #24129

Merged
merged 10 commits into from May 7, 2024

Conversation

DanShaders
Copy link
Contributor

Even less globs and questionable CMake!

Draft for now since I want to make sure CI is happy before spamming Daniel with emails.

Static libc on Serenity is broken in a more than one way and requires a
lot of patches to bring it to a usable and useful state. Therefore,
instead of keeping it around (and breaking even more) during the
upcoming libc build refactor, let's just delete it.
This is similar to how it is done in sys/arch/regs.h.
@DanShaders DanShaders marked this pull request as ready for review April 27, 2024 16:29
@github-actions github-actions bot added the 👀 pr-needs-review PR needs review from a maintainer or community member label Apr 27, 2024
@DanShaders DanShaders force-pushed the globlessness branch 3 times, most recently from 4a2fe43 to 7b1f8da Compare April 28, 2024 03:49
@DanShaders
Copy link
Contributor Author

DanShaders commented Apr 28, 2024

I've found a much more elegant solution for only linking required functions from libc.

@ADKaster, I think you would love how DynamicLoader/CMakeLists.txt looks now.

Copy link
Member

@ADKaster ADKaster left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks reasonable to me! I do wonder if we could come up with a static list of LibC files or symbols we need for the loader. One concern about your Toolchain change though.

@ADKaster
Copy link
Member

Also, static LibC seems completely removed? How will this affect ports that wanted a fully static LibC? Can/should we add it back?

@supercomputer7
Copy link
Member

supercomputer7 commented Apr 28, 2024

Also, static LibC seems completely removed? How will this affect ports that wanted a fully static LibC? Can/should we add it back?

Do we have ports that require a static LibC? I don't think so.
As far as I can tell there shouldn't be such ports, because traditionally almost everyone using Linux use glibc (with the minority of people using distributions that use musl-libc or something else and then minority of them actually statically-compile with it), which doesn't really support static compilation with it for many years (I do remember me trying to do static compile of something a few years ago and that failed with glibc. the Internet doesn't tell by a Google search, but I'd assume that at least for a decade glibc doesn't support such thing).

Even if we have ports that require static compilation somehow, we might just want to get them to use shared libraries like the rest of the system.

__stack_chk_fail_local, which libssp_nonshared.a provides, is a relic of
i386 times and is not used on modern architectures.
LibSystem's source directory is included project-wide in
/CMakeLists.txt.
In particular, define a static LibC library *in LibC's CMakeLists* and
use it in DynamicLoader. This is similar to the way LibELF is included
in DynamicLoader.

Additionally, compile DynamicLoader with -ffunction-sections,
-fdata-sections, and -Wl,--gc-sections. This brings the loader size from
~2Mb to ~1Mb with debug symbols and from ~500Kb to ~150Kb without. Also,
this makes linking DynamicLoader with LibTimeZone unnecessary.
It turns out riscv64 indeed requires -fno-stack-protector for
perform_relative_relocations, oopsie :^)
Since DynamicLoader is compiled with -fdata-sections and --gc-sections,
unused thread_local variables won't create TLS section in it anymore.
@DanShaders
Copy link
Contributor Author

I do wonder if we could come up with a static list of LibC files or symbols we need for the loader.

I think this could have worked, if libc's cpp files had had a granularity of functions (like llvm's libc for linux has). But since it doesn't, trying to split a library with a lot of cross-file dependencies into two is just meh. In fact, I originally wanted to do this, and Liav even came up with a not-so-long list of files, but no. If we do that, then every other future libc contribution adding cross-file dependencies would have to figure out how to change these 2 lists. On top of that, it doesn't solve the problem with unnecessary LibTimeZone link by itself, since we need clock_gettime from time.cpp.

@bugaevc
Copy link
Member

bugaevc commented May 2, 2024

which doesn't really support static compilation with it for many years (I do remember me trying to do static compile of something a few years ago and that failed with glibc. the Internet doesn't tell by a Google search, but I'd assume that at least for a decade glibc doesn't support such thing).

This is very false. glibc supports static linking; if it didn't have to, so many things would be easier. You can grep SHARED across the glibc codebase to see (some of) the places where static linking has to be explicitly taken into account.

Think of it this way: if glibc didn't support static linking, there is no way the Hurd could boot (well, unless we made this actually work). The first few tasks that get started (the disk driver, the PCI arbiter, the fs server...) just have to be statically linked, since there's no filesystem where the dynamic dependencies could be loaded from.

As for actually building static executables with glibc as a user, it's as simple as:

$ echo 'main() { puts("Hello, static world"); }' > test.c
$ gcc test.c -static -std=c89
$ ./a.out 
Hello, static world

(-std=c89 is only there so that GCC doesn't complain about implicit function declaration and implicit return type for this tiny example). This requires libc.a (and misc files) to link, which are in glibc-static package in my distro.

It's true that static linking is discouraged (so you should not use unless you have a real good reason), and that even the static build of glibc will dlopen NSS/iconv modules if you touch any of the relevant APIs (so: don't touch them until the root filesystem is up). It's not true that glibc doesn't support static linking.

@supercomputer7
Copy link
Member

supercomputer7 commented May 2, 2024

which doesn't really support static compilation with it for many years (I do remember me trying to do static compile of something a few years ago and that failed with glibc. the Internet doesn't tell by a Google search, but I'd assume that at least for a decade glibc doesn't support such thing).

This is very false. glibc supports static linking; if it didn't have to, so many things would be easier. You can grep SHARED across the glibc codebase to see (some of) the places where static linking has to be explicitly taken into account.

Think of it this way: if glibc didn't support static linking, there is no way the Hurd could boot (well, unless we made this actually work). The first few tasks that get started (the disk driver, the PCI arbiter, the fs server...) just have to be statically linked, since there's no filesystem where the dynamic dependencies could be loaded from.

As for actually building static executables with glibc as a user, it's as simple as:

$ echo 'main() { puts("Hello, static world"); }' > test.c
$ gcc test.c -static -std=c89
$ ./a.out 
Hello, static world

(-std=c89 is only there so that GCC doesn't complain about implicit function declaration and implicit return type for this tiny example). This requires libc.a (and misc files) to link, which are in glibc-static package in my distro.

It's true that static linking is discouraged (so you should not use unless you have a real good reason), and that even the static build of glibc will dlopen NSS/iconv modules if you touch any of the relevant APIs (so: don't touch them until the root filesystem is up). It's not true that glibc doesn't support static linking.

I know you can pass the -static flag and for simple applications which don't rely on some function symbols, it will work, so I am not speaking about simple programs like hello world, etc, but many real programs, that do NSS (and iconv as you mentioned) for example, can't get statically-compiled as glibc will just yell at you - this is what I intended to say...

Anyway, it's not like we actually care enough about static compilation anyway - I actually see no benefit in our ecosystem for this, except for the fact that you could provide a small rootfs with no dynamic loader, which has no real advantage for anybody here.

So it's a non-relevant discussion about the topic, and we should just move on to delete static libc.

@DanShaders
Copy link
Contributor Author

Static libc we "had" didn't even work, it just was there.

@supercomputer7
Copy link
Member

Static libc we "had" didn't even work, it just was there.

Right. I "fondly" (actually really not) remember me trying to force the BuggieBox application to use static libc, and it kinda worked, but was terribly hacked to be this way, so yes, the file was there but nobody was able to use it properly.
And even with the BuggieBox application, even in a small initramfs environment, one could simply provide a proper dynamic loader with it :)

@ADKaster ADKaster merged commit 5f33db9 into SerenityOS:master May 7, 2024
13 checks passed
@github-actions github-actions bot removed the 👀 pr-needs-review PR needs review from a maintainer or community member label May 7, 2024
@DanShaders DanShaders deleted the globlessness branch May 7, 2024 22:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants