makefiles: introduce 'LIBS' variable

[cladmi]

Contribution description

This introduced the LIBS variables to declare a module as a static
library.

It is a requirement to allow handling linking differently between
static libraries and shared libraries in upcoming pull requests.

Co-authored-by: Joakim Nohlgård joakim.nohlgard@eistec.se

This is split of #8711 and modified as some specific handling done for USEPKG is not necessary anymore.

I took the name "static libraries" and "shared libraries" from man ld in the --whole-archive section.

Testing procedure

Compiling should sill work in the same way and the output of BASELIBS should not have changed.

To simplify testing, I comment APPDEPS in the pic32 boards as it contains the full path and is not relevant for this test:

diff --git a/boards/pic32-clicker/Makefile.include b/boards/pic32-clicker/Makefile.include
index 264f0e502..3c9c68a90 100644
--- a/boards/pic32-clicker/Makefile.include
+++ b/boards/pic32-clicker/Makefile.include
@@ -1,4 +1,4 @@
 export CPU = mips_pic32mx
 export CPU_MODEL=p32mx470f512h
-export APPDEPS += $(RIOTCPU)/$(CPU)/$(CPU_MODEL)/$(CPU_MODEL).S
+#export APPDEPS += $(RIOTCPU)/$(CPU)/$(CPU_MODEL)/$(CPU_MODEL).S
 export USE_UHI_SYSCALLS = 1
diff --git a/boards/pic32-wifire/Makefile.include b/boards/pic32-wifire/Makefile.include
index 7e4a7c738..3c1a62eca 100644
--- a/boards/pic32-wifire/Makefile.include
+++ b/boards/pic32-wifire/Makefile.include
@@ -1,4 +1,4 @@
 export CPU = mips_pic32mz
 export CPU_MODEL=p32mz2048efg100
-export APPDEPS += $(RIOTCPU)/$(CPU)/$(CPU_MODEL)/$(CPU_MODEL).S
+#export APPDEPS += $(RIOTCPU)/$(CPU)/$(CPU_MODEL)/$(CPU_MODEL).S
 export USE_UHI_SYSCALLS = 1

and compare the value of BASELIBS.

I use https://gist.github.com/cladmi/9818888da7412a9195ec0d4c3c2a8b8a to compare the output and it should see no difference:

BINDIR='/tmp/bin/$(BOARD)' ./scripts/compare_riot_repositories_variable_value.sh riot_master riot_pr BASELIBS

There are currently /bin/sh: 1: Syntax error: Missing '))' messages but they are unrelated.

Issues/PRs references

This is a split and refactored out of #8711
Depends on #11109 refactoring

[cladmi]

My test script output reports no changed values:

cat baselibs  | grep -c differ
0

[jcarrano]

I'm having trouble understanding the LIBS variable. In #8711 it is used to differentiate between modules that generate a .a (the default for everything currently) and those linked into a .o.

If --whole-archive is used instead of ld -r, The combined .o are not needed anymore and everything is compiled into .a as always, so there is no change there. It might still be necessary to not use --whole-archive for some modules and this is what I understand LIBS does.

In any case, this "static" and "dyncamic" libraries name, even if it comes from the ld manual, is very confusing, since in the end everything is statically linked.

[cladmi]

I picked the same name as in the ld -r PR but may need to be adapted and documentation adapted to work more with whole-archive only.
It is one of the reason to split it out to focus on this discussion only without the other issues :)

It should point to archives that must be linked as "libraries" without --whole-archive.
There are modules which are real libraries that cannot be linked by considering all objects.

The issue is that the documentation does not name them but only describe a usage:

      --whole-archive
           For each archive mentioned on the command line after the --whole-archive option, include every object file in the archive in
           the link, rather than searching the archive for the required object files.  This is normally used to turn an archive file into
           a shared library, forcing every object to be included in the resulting shared library.  This option may be used more than
           once.

           Two notes when using this option from gcc: First, gcc doesn't know about this option, so you have to use -Wl,-whole-archive.
           Second, don't forget to use -Wl,-no-whole-archive after your list of archives, because gcc will add its own list of archives
           to your link and you may not want this flag to affect those as well.

All help on naming these with references is welcome.

[jcarrano]

What about "NO_WHOLE_ARCHIVE" and explain that:

"by default, modules and packages produce archives that are then linked with the --whole-archive option. This may expose some bugs (for example, symbols not being discarded when they should.) This option is provided as a workaround and causes the module to be linked with the application as a regular static archive."

or something like that.

[cladmi]

If I need to read the documentation to understand what should go in a variable, then the name is wrong. And I find no_whole_archive to be really really implementation specific.

There should be a "high level" concept that would describe what type of modules should go in without thinking about the exact link arguments.

And currently, the things that should go in are not bugs, just modules that are implemented as "archives libraries" relying on the normal ld behavior for linking archives.

What I consider a bug, is needing to rely on this for some of the things we do that should not require it in the first place, and that we link as "archives libraries" things when they should not.

But I lack the high level concept.

[cladmi]

It goes somehow with "use all objects from archive" or "link only what is used".

I find some infos that talk about "shared object and static library" and the other archives are handled as shared objects.
OBJECTS vs LIBS somehow.

[cladmi]

dynamic libraries/objects vs static libraries somehow. Another variable talking about it:

      --no-as-needed
           This option affects ELF DT_NEEDED tags for dynamic libraries mentioned on the command line after the --as-needed option.
           Normally the linker will add a DT_NEEDED tag for each dynamic library mentioned on the command line, regardless of whether
           the library is actually needed or not.  --as-needed causes a DT_NEEDED tag to only be emitted for a library that at that
           point in the link satisfies a non-weak undefined symbol reference from a regular object file or, if the library is not found
           in the DT_NEEDED lists of other needed libraries, a non-weak undefined symbol reference from another needed dynamic library.
           Object files or libraries appearing on the command line after the library in question do not affect whether the library is
           seen as needed.  This is similar to the rules for extraction of object files from archives.  --no-as-needed restores the
           default behaviour.

[cladmi]

I updated the documentation and internal handling for OBJECTS and ARCHIVE terminology

With this LIBS could also be updated to another name but I keep it until the description is indeed what we want.

[jcarrano]

Squash, then we run on CI and then we merge.

[cladmi]

I still question the name LIBS (taken from the original PR) as with this new name:

• LIBS are module names
• BASELIBS are archives. But also sometime -lsomething and other unrelated things that should not be there.

I would expect somehow that LIBS is given almost directly to the linker and not a list of special modules that will be processed.

With the naming I would be more maybe to a ARCHIVE_MODULES or LIBRARY_MODULES… as mentioned in #8711 (comment)

---

The output of the test is still "as expected":

BINDIR='/tmp/bin/$(BOARD)' ./compare_riot_repositories_variable_value.sh ../worktree/riot_master/ ../RIOT BASELIBS | tee output_baselibs                                                                                                                                              
/home/harter/work/git/worktree/riot_master/Makefile.dep:86: nordic_softdevice_ble: Disabling router modules: gnrc_ipv6_router gnrc_ipv6_router_default                                                                                                                                    
/home/harter/work/git/worktree/riot_master/Makefile.dep:86: nordic_softdevice_ble: Disabling router modules: gnrc_ipv6_router gnrc_ipv6_router_default                                                                                                                                    
/home/harter/work/git/RIOT/Makefile.dep:86: nordic_softdevice_ble: Disabling router modules: gnrc_ipv6_router gnrc_ipv6_router_default                                                                                                                                                    
/home/harter/work/git/worktree/riot_master/Makefile.dep:86: nordic_softdevice_ble: Disabling router modules: gnrc_ipv6_router_default gnrc_rpl auto_init_gnrc_rpl netstats_rpl                                                                                                            
/home/harter/work/git/worktree/riot_master/Makefile.dep:86: nordic_softdevice_ble: Disabling router modules: gnrc_ipv6_router_default gnrc_rpl auto_init_gnrc_rpl netstats_rpl                                                                                                            
/home/harter/work/git/RIOT/Makefile.dep:86: nordic_softdevice_ble: Disabling router modules: gnrc_ipv6_router_default gnrc_rpl auto_init_gnrc_rpl netstats_rpl                                                                                                                            
/home/harter/work/git/worktree/riot_master/Makefile.dep:86: nordic_softdevice_ble: Disabling router modules: gnrc_ipv6_router_default gnrc_rpl                                                                                                                                            
/home/harter/work/git/worktree/riot_master/Makefile.dep:86: nordic_softdevice_ble: Disabling router modules: gnrc_ipv6_router_default gnrc_rpl                                                                                                                                            
/home/harter/work/git/RIOT/Makefile.dep:86: nordic_softdevice_ble: Disabling router modules: gnrc_ipv6_router_default gnrc_rpl                                                                                                                                                            
/home/harter/work/git/worktree/riot_master/Makefile.dep:86: nordic_softdevice_ble: Disabling router modules: gnrc_ipv6_router_default gnrc_rpl auto_init_gnrc_rpl                                                                                                                         
/home/harter/work/git/worktree/riot_master/Makefile.dep:86: nordic_softdevice_ble: Disabling router modules: gnrc_ipv6_router_default gnrc_rpl auto_init_gnrc_rpl                                                                                                                         
/home/harter/work/git/RIOT/Makefile.dep:86: nordic_softdevice_ble: Disabling router modules: gnrc_ipv6_router_default gnrc_rpl auto_init_gnrc_rpl                                                                                                                                         
/home/harter/work/git/worktree/riot_master/Makefile.dep:86: nordic_softdevice_ble: Disabling router modules: gnrc_ipv6_router_default                                                                                                                                                     
/home/harter/work/git/worktree/riot_master/Makefile.dep:86: nordic_softdevice_ble: Disabling router modules: gnrc_ipv6_router_default                                                                                                                                                     
/home/harter/work/git/RIOT/Makefile.dep:86: nordic_softdevice_ble: Disabling router modules: gnrc_ipv6_router_default                                                                                                                                                                     
/bin/sh: 1: arithmetic expression: expecting primary: " + "                                                                                                                                                                                                                               
/bin/sh: 1: arithmetic expression: expecting primary: " + "                                                                                                                                                                                                                               
/bin/sh: 1: arithmetic expression: expecting primary: " + "                                                                                                                                                                                                                               
/bin/sh: 1: arithmetic expression: expecting primary: " + "                                                                                                                                                                                                                               
/bin/sh: 1: arithmetic expression: expecting primary: " + "                                                                                                                                                                                                                               
/bin/sh: 1: arithmetic expression: expecting primary: " + "                                                                                                                                                                                                                               
/bin/sh: 1: arithmetic expression: expecting primary: " + "                                                                                                                                                                                                                               
/bin/sh: 1: arithmetic expression: expecting primary: " + "                                                                                                                                                                                                                               
/bin/sh: 1: arithmetic expression: expecting primary: " + "                                                                                                                                                                                                                               
/bin/sh: 1: arithmetic expression: expecting primary: " + "                                                                                                                                                                                                                               
/bin/sh: 1: arithmetic expression: expecting primary: " + "                                                                                                                                                                                                                               
/bin/sh: 1: arithmetic expression: expecting primary: " + "                                                                                                                                                                                                                               
/bin/sh: 1: arithmetic expression: expecting primary: " + "                                                                                                                                                                                                                               
/bin/sh: 1: arithmetic expression: expecting primary: " + "                                                                                                                                                                                                                               
/bin/sh: 1: arithmetic expression: expecting primary: " + "                                                                                                                                                                                                                               
/bin/sh: 1: arithmetic expression: expecting primary: " + "                                                                                                                                                                                                                               
/bin/sh: 1: arithmetic expression: expecting primary: " + "                                                                                                                                                                                                                               
/bin/sh: 1: arithmetic expression: expecting primary: " + "        

[tcschmidt]

Is naming still the problem? Maybe @jcarrano or @MrKevinWeiss can help with it?

[jcarrano]

Meh, I have seen worse naming. I'd say lets move forward - as long as it is documented it will be OK.

[jcarrano]

@cladmi, squash this. Let's get over all this naming stuff and get the functionality in.

[kaspar030]

@cladmi, squash this.

yup, please do!

[kaspar030]

This is split of #8711 and modified as some specific handling done for USEPKG is not necessary anymore.

We cannot go (AFAIK) with #8711 as it is incompatible with LTO, an important use-case.
Is in that case the distinction still necessary?

[cladmi]

I somehow still do not like LIBS, I just used it as a placeholder but for me it does not reflect the fact that it is modules whose archive will need to be linked the same way as ld links libraries by default.
But as we did not get a way to describe this directly, @jcarrano initial idea of something like MODULES_NO_WHOLE_ARCHIVE would reflect the issue.

We cannot go (AFAIK) with #8711 as it is incompatible with LTO, an important use-case.
Is in that case the distinction still necessary?

If we ignore ld -r globally, then the naming does not need to be generic and could say no_whole_archive.

For me, this list has two goal, we know that some libraries cannot be linked with --whole-archive, so it is required to prevent a module to be linked this way.

But the main one, is to more easily enable linking with whole archive in RIOT without fixing anything.
Disable the new linking for all modules that behave differently and add the new linking.
Then, each module can be after investigating/migration/specific testing be fixed and removed from the no_whole_archive list.

I would even first list all modules that define a UNDEF in it.

Including the new linking should get the same binary size and symbols for everything by removing everything that could change it.

[leandrolanzieri]

@kaspar030 opinions on this?

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. If you want me to ignore this issue, please mark it with the "State: don't stale" label. Thank you for your contributions.

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. If you want me to ignore this issue, please mark it with the "State: don't stale" label. Thank you for your contributions.