Add -lc -lgcc from picolibc toolchain library after module libraries #51949

keith-packard · 2022-11-03T20:12:34Z

Hacks like

zephyr/modules/openthread/CMakeLists.txt

Lines 444 to 448 in 29047f1

    
           # Make sure C library, in case of newlib, is linked after OpenThread libraries 
        
           # (to prevent linker errors) 
        
           if(CONFIG_NEWLIB_LIBC) 
        
             target_link_libraries(ot-config INTERFACE -lc) 
        
           endif()

shouldn't be required -- instead, the C library should be added to the end of the link command, rather than some place in the middle. Create a special set of link libraries which are to be added after the rest of the system and then use that for picolibc.

stephanosio

This looks reasonable to me, but @tejlmand might have a better idea on how to handle this more generically.

keith-packard · 2022-11-07T20:46:50Z

This looks reasonable to me, but @tejlmand might have a better idea on how to handle this more generically.

sounds good. I'm no cmake expert and welcome advice from others more skilled in this area.

tejlmand

Thanks for raising this.

This should clearly be cleaned up.

No immediate changes requested, but I just need a little time to carefully think this through.

Regarding the picolibc changes, those requires a toolchain with pre-built picolibc, but if I try to build with Zephyr SDK 0.15.1 and -DCONFIG_PICOLIBC_USE_MODULE=n then I get following error:

arm-zephyr-eabi-gcc: fatal error: cannot read spec file 'picolibc.specs': No such file or directory
compilation terminated.

@keith-packard which toolchain build are you using that is shipped with pre-built picolibc library to verify this code path ?

tejlmand · 2022-11-14T13:16:20Z

cmake/modules/extensions.cmake

+function(zephyr_late_link_libraries)
+  set_property(TARGET zephyr_interface APPEND PROPERTY LATE_LINK_LIBRARIES ${ARGV})
+endfunction()


a bit unsure on this.

It's one of those functions which solves an immediate problem but once it's available people will start to use it in various places, thus it has a high built-in risk of being used wrongly if people randomly starts to use this function.

yes, this function is definitely easy to abuse. I wonder if zephyr_link_libraries should just check for system libraries (-lc -lgcc et al) and do something internally with them?

I like the option to handle -lc and -lgcc (if applicable) internally.

Alternatively, if the toolchain interface were to have some abstraction / function to just name it's late link libraries, then it would be slightly more adaptable.

tejlmand · 2022-11-14T13:22:36Z

lib/libc/picolibc/CMakeLists.txt

@@ -13,7 +13,7 @@ if(NOT CONFIG_PICOLIBC_USE_MODULE)

  zephyr_compile_options(--specs=picolibc.specs)
  zephyr_compile_definitions(_POSIX_C_SOURCE=200809)
-  zephyr_link_libraries(-T/dev/null --specs=picolibc.specs c -lgcc)
+  zephyr_late_link_libraries(-T/dev/null --specs=picolibc.specs c -lgcc)


this could be one of those situation where link_libraries() is actually acceptable to use.
https://cmake.org/cmake/help/latest/command/link_libraries.html

as we actually want to ensure -lc and -lgcc are linked last.
But using link_libraries() at this location will not work, cause it needs to be on a higher level.

Moving this to higher level would require some changes to how library infrastructure is sourced in the build system.
Some libc impl also creates a library which must be linked, whereas this purely set flags.

keith-packard · 2022-11-14T17:02:35Z

@keith-packard which toolchain build are you using that is shipped with pre-built picolibc library to verify this code path ?

@stephanosio helped create https://github.com/zephyrproject-rtos/sdk-ng/tree/topic-picolibc which builds the toolchain using patches that enable picolibc. There are complete toolchains available from each successful run, like https://github.com/zephyrproject-rtos/sdk-ng/actions/runs/3331003572

stephanosio · 2022-12-12T10:12:56Z

FYI, there is Zephyr SDK 0.16.0-beta1 with picolibc available now for testing.

stephanosio · 2023-01-26T16:40:42Z

@tejlmand Do you have any updates on this?

stephanosio · 2023-03-15T11:17:35Z

@tejlmand ping

keith-packard · 2023-03-15T19:07:38Z

Maybe use a different name for zephyr_late_link_libraries? Something that reflects the lower-level nature of the libraries in question? zephyr_system_link_libraries? zephyr_toolchain_link_libraries?

keith-packard · 2023-04-04T21:09:21Z

I've renamed the function to zephyr_libc_link_libraries to try and make it clear where this is intended to be used. Perhaps this is better? Alternate suggestions welcome.

This function allows subsystems to define libraries which get added to the link command after all other libraries and modules. It's useful when using a toolchain library, like libc or libgcc, as those can get added when processing the 'lib' directory, before any module libraries and hence might not get used to resolve symbols from modules. Signed-off-by: Keith Packard <[email protected]>

When using the toolchain C library, that must be added to the link command after all other libraries and modules in the system to resolve undefined symbols. Signed-off-by: Keith Packard <[email protected]>

tejlmand

Approved, as this seems to be improvement over current solution, but as it doesn't solve the issue fully I will create a follow up enhancement issue.

tejlmand · 2023-04-17T09:02:40Z

CMakeLists.txt

+get_property(LIBC_LINK_LIBRARIES TARGET zephyr_interface PROPERTY LIBC_LINK_LIBRARIES)
+zephyr_link_libraries(${LIBC_LINK_LIBRARIES})


in most cases this will work, but technically we are not guaranteed that there will not be added extra link libraries to the Zephyr interface library.

If the app adds extra libraries to explicitly be linked, then those might a appear later, or even some Zephyr libs could have extra libs.
However, as this still seems to ba an improvement, this comment is mostly in order for future references.

Will create followup enhancement issue.

tejlmand · 2023-04-17T09:17:16Z

Approved, as this seems to be improvement over current solution, but as it doesn't solve the issue fully I will create a follow up enhancement issue.

Enhancement issue: #56924

keith-packard requested review from nashif and tejlmand as code owners November 3, 2022 20:12

zephyrbot added area: Build System area: C Library C Standard Library labels Nov 3, 2022

zephyrbot requested review from cfriedt, enjiamai, jeremybettis, KangJianX and stephanosio November 3, 2022 20:13

zephyrbot assigned tejlmand Nov 3, 2022

stephanosio reviewed Nov 7, 2022

View reviewed changes

tejlmand requested changes Nov 14, 2022

View reviewed changes

keith-packard force-pushed the late-link branch from 5ff4981 to f8501fe Compare December 26, 2022 00:55

zephyrbot requested a review from nordicjm December 26, 2022 00:55

keith-packard force-pushed the late-link branch from f8501fe to 9e5a9ba Compare January 7, 2023 09:02

keith-packard mentioned this pull request Jan 18, 2023

Make picolibc the default C library for Zephyr #49922

Closed

60 tasks

keith-packard added the area: picolibc Picolibc C Standard Library label Jan 20, 2023

keith-packard force-pushed the late-link branch from 9e5a9ba to 4a4dec0 Compare March 15, 2023 19:05

keith-packard force-pushed the late-link branch from 4a4dec0 to 8f5a57b Compare April 4, 2023 21:08

keith-packard added 2 commits April 14, 2023 08:52

lib/picolibc: Use zephyr_libc_link_libraries to find toolchain lib

bac902c

When using the toolchain C library, that must be added to the link command after all other libraries and modules in the system to resolve undefined symbols. Signed-off-by: Keith Packard <[email protected]>

keith-packard force-pushed the late-link branch from 8f5a57b to bac902c Compare April 14, 2023 15:52

tejlmand approved these changes Apr 17, 2023

View reviewed changes

carlescufi requested a review from stephanosio April 17, 2023 09:12

carlescufi approved these changes Apr 17, 2023

View reviewed changes

carlescufi merged commit b5caccb into zephyrproject-rtos:main Apr 17, 2023

tejlmand mentioned this pull request Apr 17, 2023

Improve C library linking when using gcc #56924

Open

keith-packard deleted the late-link branch April 17, 2023 18:00

tejlmand mentioned this pull request Jan 24, 2024

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

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add -lc -lgcc from picolibc toolchain library after module libraries #51949

Add -lc -lgcc from picolibc toolchain library after module libraries #51949

keith-packard commented Nov 3, 2022 •

edited by stephanosio

Loading

stephanosio left a comment

keith-packard commented Nov 7, 2022

tejlmand left a comment

tejlmand Nov 14, 2022

keith-packard Nov 14, 2022

cfriedt Mar 16, 2023

tejlmand Nov 14, 2022

keith-packard commented Nov 14, 2022

stephanosio commented Dec 12, 2022

stephanosio commented Jan 26, 2023

stephanosio commented Mar 15, 2023

keith-packard commented Mar 15, 2023

keith-packard commented Apr 4, 2023

tejlmand left a comment

tejlmand Apr 17, 2023

tejlmand commented Apr 17, 2023

	# Make sure C library, in case of newlib, is linked after OpenThread libraries
	# (to prevent linker errors)
	if(CONFIG_NEWLIB_LIBC)
	target_link_libraries(ot-config INTERFACE -lc)
	endif()

		get_property(LIBC_LINK_LIBRARIES TARGET zephyr_interface PROPERTY LIBC_LINK_LIBRARIES)
		zephyr_link_libraries(${LIBC_LINK_LIBRARIES})

Add -lc -lgcc from picolibc toolchain library after module libraries #51949

Add -lc -lgcc from picolibc toolchain library after module libraries #51949

Conversation

keith-packard commented Nov 3, 2022 • edited by stephanosio Loading

stephanosio left a comment

Choose a reason for hiding this comment

keith-packard commented Nov 7, 2022

tejlmand left a comment

Choose a reason for hiding this comment

tejlmand Nov 14, 2022

Choose a reason for hiding this comment

keith-packard Nov 14, 2022

Choose a reason for hiding this comment

cfriedt Mar 16, 2023

Choose a reason for hiding this comment

tejlmand Nov 14, 2022

Choose a reason for hiding this comment

keith-packard commented Nov 14, 2022

stephanosio commented Dec 12, 2022

stephanosio commented Jan 26, 2023

stephanosio commented Mar 15, 2023

keith-packard commented Mar 15, 2023

keith-packard commented Apr 4, 2023

tejlmand left a comment

Choose a reason for hiding this comment

tejlmand Apr 17, 2023

Choose a reason for hiding this comment

tejlmand commented Apr 17, 2023

keith-packard commented Nov 3, 2022 •

edited by stephanosio

Loading