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

Don't use --whole-archive for libc/common and libc/picolibc files #65434

Closed

Conversation

keith-packard
Copy link
Collaborator

This moves these two libraries outside of the --whole-archive list so that only functionality explicitly referenced by the application will get included in the executable.

For example -- the malloc implementation in libc/common/source/stdlib/malloc.c includes a SYS_INIT function, malloc_prepare. If the application references malloc, that file will be included in the output, including the malloc_prepare function which will get called at startup. If the application does not reference malloc, then
the file will be excluded and so malloc_prepare will not be included in the output.

This is an alternative for #63259 and does not require per-application configuration.

@keith-packard
Copy link
Collaborator Author

This might be hard to make work -- having the linker drop code with no references (using --whole-archive --gc-sections) is very different from the traditional model where the linker only adds code with explicit references. It might be worth the effort though -- we're currently pulling in anything marked KEEP in the linker script, even if that's the only bit of a object file which is otherwise unused.

@keith-packard keith-packard force-pushed the auto-malloc branch 2 times, most recently from e961082 to 1f48784 Compare November 19, 2023 20:08
@keith-packard
Copy link
Collaborator Author

I just added code to compute all of the nested dependencies to stick between --start-group and --end-group; would be happy if someone knew how to use the cmake LINK_GROUP stuff to make that less complicated. Alternatively, figure out how to get cmake to append -Wl,--end-group after all of the dependent libraries.

@keith-packard
Copy link
Collaborator Author

ok, another kludge -- I've added z_ symbol aliases for the common malloc functions and then used --defsym to redirect
uses to the common version of malloc instead of the C library version.

@keith-packard keith-packard force-pushed the auto-malloc branch 3 times, most recently from 6b7239b to 471daef Compare November 19, 2023 23:12
@keith-packard
Copy link
Collaborator Author

and more kludges -- check each link element to see if it's a valid target before attempting to run get_property as that will fail otherwise.

@keith-packard keith-packard force-pushed the auto-malloc branch 4 times, most recently from 6a5c50a to 0c0fea3 Compare August 24, 2024 03:30
@zephyrbot zephyrbot added the area: Testsuite Testsuite label Aug 24, 2024
@keith-packard
Copy link
Collaborator Author

I had to add a kludge to get intel_adsp builds working; that's gonna need someone with cmake knowledge to help resolve, I think.

_unlink is used by newlib's 'remove' implementation, which is
part of the standard C API, not a POSIX extension.

Signed-off-by: Keith Packard <[email protected]>
This is required for newlib's remove implementation.

Signed-off-by: Keith Packard <[email protected]>
This splits the picolibc helper functions into separate files so that
code not explicitly referenced can be excluded from the result once
we stop using --whole-archive with this library.

Signed-off-by: Keith Packard <[email protected]>
This is needed for the picolibc __chk_fail implementation as that
uses it up through version 1.8.5. After that, picolibc uses puts.

Signed-off-by: Keith Packard <[email protected]>
We can't know before building the system whether there will be any data
present in the C library parition. Allow this, simply skipping any use of
it.

Signed-off-by: Keith Packard <[email protected]>
Create the kernel library object before loading in the subdirectories so
that other parts of the code (the toolchain and C library) may adjust the
kernel dependencies as needed using target_link_library(kernel, ...)

Signed-off-by: Keith Packard <[email protected]>
When built as a regular library, this file doesn't get included
automatically.

Signed-off-by: Keith Packard <[email protected]>
Support changes to Zephyr that manage the C library dependencies using
standard cmake mechanisms.

Signed-off-by: Keith Packard <[email protected]>
These aliases will be used to direct libc allocations to the common
allocator during linking using the --wrap linker option.

Signed-off-by: Keith Packard <[email protected]>
Switch from zephyr_link_libraries (which is placed before the kernel) to
zephyr_libc_link_libraries (which is placed after).

Signed-off-by: Keith Packard <[email protected]>
With zephyr_link_libraries now redirected to push libraries before the
kernel, we need to provide a mechanism to push libraries *after* the kernel
so that libc will link correctly. Do that by using what
zephyr_link_libraries used to do, which is to call target_link_libraries.

Signed-off-by: Keith Packard <[email protected]>
As many libraries used in zephyr_link_libraries depend upon C library
and/or kernel interfaces, they need to be present in the link command line
before those libaries.

Do this by adding them to the linker command ahead of the kernel by saving
them in zephyr_link_libraries and using that in toolchain_ld_link_elf.

Signed-off-by: Keith Packard <[email protected]>
Use add_library instead of zephyr_library and then add a dependency from
the kernel to the C library bits.

This moves all of this code out of the --whole-archive set so that unused
bits from these libraries are not included in the resulting executable,
even sections marked 'KEEP' in the linker script.

Because this changes the names of the C libraries, linker script fragments
in soc/xtensa, soc/riscv and boards/qemu_x86_tiny needed to be updated.

Signed-off-by: Keith Packard <[email protected]>
I added this kludge to prevent a dependency loop from forming. I have no
idea why this particular library is causing the problem.

Signed-off-by: Keith Packard <[email protected]>
Copy link

This pull request has been marked as stale because it has been open (more than) 60 days with no activity. Remove the stale label or add a comment saying that you would like to have the label removed otherwise this pull request will automatically be closed in 14 days. Note, that you can always re-open a closed pull request at any time.

@github-actions github-actions bot added the Stale label Oct 25, 2024
@github-actions github-actions bot closed this Nov 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants