build system: link object files #14754

leandrolanzieri · 2020-08-13T14:08:41Z

Contribution description

This PR is another attempt to use the generated object files for the linking step instead of the archives. This makes the linking process more transparent and helps pointing out possible redefinition of symbols that may not have been caught before. It also saves a small amount of time by no dealing with the archive creation. The UNDEF list of objects is not needed anymore.

The PR also fixes the bindist example which is broken in current master. The archive seems to be malformed and the second linking is not possible. The example now copies the object files for the second linking.

Current problem

Currently we are using mostly archives in the linking step (only object files specified in the UNDEF makefile variable will be linked directly). That means that:

As we link archives and not objects the order really matters, because of the algorithm the linker uses to pick objects from those archives. We may be masking errors of multiple symbol definitions, which could be dangerous (see the example bellow).
We are generating one archive per module (even when some modules don't have any object files, e.g. sys or gnrc). This produced some issue in OSx apparently and we found a workaround to be able to link empty archives.
As archives may have circular dependencies between them, we are forced to use the --start-group and --end-group options for all our archives. According to man ld, "Using this option has a significant performance cost".

An example of the problem

As order matters, and we use alphabetical order to link the archives, we are currently not overriding the weakkly defined panic_arch function, and using always the version provided by the core. To see this, just compile some example on an architecture that overrides this (e.g. any cortexm), and inspect the binary (nm bin/samr21-xpro/gcoap_example.elf), the symbol
type will be W. This could happen with other symbols in other modules, and depends on which other modules are being used and their order in the linking process.

Testing procedure

Green CI
We should test this on different platforms. Keep in mind that binaries will change, because of the problem explained above.

Issues/PRs references

Alternative to #8711
Initial suggestion in #5757

leandrolanzieri · 2020-08-18T08:58:19Z

Many changes that are currently in the PR can and should be split. I'll do so if we agree with the change.

maribu · 2020-08-18T18:34:54Z

Could this fix the issue that randomly a weak symbol is linked in instead of a strong symbol? I had this issue when working with a local PKG and in the end resorted to using guardimg the weak symbol with #ifdef HAS_FOO, as otherwise I ended up with the weak symbol being preferred over the strong one from time to time.

leandrolanzieri · 2020-08-18T18:59:53Z

Could this fix the issue that randomly a weak symbol is linked in instead of a strong symbol? I had this issue when working with a local PKG and in the end resorted to using guardimg the weak symbol with #ifdef HAS_FOO, as otherwise I ended up with the weak symbol being preferred over the strong one from time to time.

Yes. That's because of the way the linker is picking the objects that provide needed symbols from the archives, which depends of the order of the archives on the list, and the name of the objects. Using objects it should link the strong symbol, and throw an error when more than one strong symbol is found. For examples of this, see the panic_arch example above, and the multiple fixes that were added to the PR because of multiple definitions of symbols.

leandrolanzieri · 2020-09-22T13:35:44Z

@leandrolanzieri Would you mind confirming that this commit indeed caused a regression with libhydrogen and the combination of board and compiler?

I'm on it. I could not reproduce at first but when building in Docker the error pops up.

leandrolanzieri · 2020-09-23T07:59:42Z

@leandrolanzieri Would you mind confirming that this commit indeed caused a regression with libhydrogen and the combination of board and compiler?

I'm on it. I could not reproduce at first but when building in Docker the error pops up.

The regression seems to be caused by the commit you pointed out. I'm debugging to figure out why the call to hydro_mem_ct_cmp_u32 from hydro_secretbox_decrypt is returning an error. The weird thing as I mentioned is that I can only reproduce when using the Docker build.

My clang version outside docker is:

clang version 8.0.0-3 (tags/RELEASE_800/final)
Target: x86_64-pc-linux-gnu
Thread model: posix
InstalledDir: /usr/bin

bergzand · 2020-09-23T11:36:45Z

I've opened an issue to track the regression, see #15066

benpicco · 2020-10-04T16:55:49Z

This broke the output on esp8266 #15137

(That might also be the reason why esp32 tests are failing on CI)

maribu · 2020-10-04T20:09:28Z

Interestingly, the output of elf_diff of e80bb4b and e24a64f for examples/default on the ESP8266 is huge. The number of symbols linked in and their names remained unchanged, but many symbols changed in size.

Seemingly, symbol exception_handler from cpu/esp_common/exceptions.c was never linked in for the ESP8266 previously (and some other same-named symbol likely from the SDK got linked in instead) - but it now is.

leandrolanzieri · 2020-10-05T06:00:18Z

Seemingly, symbol exception_handler from cpu/esp_common/exceptions.c was never linked in for the ESP8266 previously (and some other same-named symbol likely from the SDK got linked in instead) - but it now is.

This could be the case for other functions defined as weak, which did not get overridden before and now are. I'll look into this.

gschorcht · 2020-10-05T06:08:55Z

Seemingly, symbol exception_handler from cpu/esp_common/exceptions.c was never linked in for the ESP8266 previously (and some other same-named symbol likely from the SDK got linked in instead)

That's not true. The exception_handler function from cpu/esp_common/exceptions.c was definitely used before to print the register set as well as the running processes and to restart the system in case of crashes.

leandrolanzieri · 2020-10-05T07:06:44Z

Interestingly, the output of elf_diff of e80bb4b and e24a64f for examples/default on the ESP8266 is huge. The number of symbols linked in and their names remained unchanged, but many symbols changed in size.

I'm also getting a big output with this tool. Interestingly, it points out that one symbol disappeared and one symbol is new (main):

Symbols Disappeared
===================
_exit-0x8: 0 bytes
   388c      	beqz.n	a8, 40210017 <_irom0_text_start+0x7>
   b44023        	excw
   101b      	addi.n	a1, a0, 1
   322040        	excw
   __ED_SOURCE_START____ED_SOURCE_END__

New Symbols
===========
main-0x14: 0 bytes
   5388      	l32i.n	a8, a3, 20
   984023        	excw
   402353        	excw
   53a8      	l32i.n	a10, a3, 20
   d04023        	excw
   402353        	excw
   53d8      	l32i.n	a13, a3, 20
   214023        	excw
   __ED_SOURCE_START____ED_SOURCE_END__

maribu · 2020-10-05T07:08:53Z

That's not true. The exception_handler function from cpu/esp_common/exceptions.c was definitely used before to print the register set as well as the running processes and to restart the system in case of crashes.

Strange. It changed by 100 Byte and the machine code looks pretty different. I wonder what else could explain this :-/

Here is by the way the output of elf_diff for examples/default build with the docker toolchain: https://mari-bu.de/pr14754.html (Beware: The file is huge (as in multiple MiBs), that will take some seconds to load from my Raspberry Pi home server.)

I sadly forgot to overwrite RIOT_VERSION, but that results only in small changes in the generated binary anyway.

leandrolanzieri · 2020-10-08T10:12:53Z

Please see #15186

JannesVolkens · 2020-10-09T10:19:49Z

Because of this PR, it is somehow no longer possible to build anything on the nucleo-f207zg :/
I tried running the test under /tests/buttons and it gave me this output:

jannes@Latitude:~/RIOT/tests/buttons$ BOARD=nucleo-f207zg make all flash term
Building application "tests_buttons" for "nucleo-f207zg" with MCU "stm32".

"make" -C /home/jannes/RIOT/boards/nucleo-f207zg
"make" -C /home/jannes/RIOT/boards/common/nucleo
"make" -C /home/jannes/RIOT/core
"make" -C /home/jannes/RIOT/cpu/stm32
"make" -C /home/jannes/RIOT/cpu/cortexm_common
"make" -C /home/jannes/RIOT/cpu/cortexm_common/periph
"make" -C /home/jannes/RIOT/cpu/stm32/bootloader
"make" -C /home/jannes/RIOT/cpu/stm32/periph
"make" -C /home/jannes/RIOT/cpu/stm32/stmclk
"make" -C /home/jannes/RIOT/cpu/stm32/vectors
"make" -C /home/jannes/RIOT/drivers
"make" -C /home/jannes/RIOT/drivers/periph_common
"make" -C /home/jannes/RIOT/sys
"make" -C /home/jannes/RIOT/sys/auto_init
"make" -C /home/jannes/RIOT/sys/isrpipe
"make" -C /home/jannes/RIOT/sys/newlib_syscalls_default
"make" -C /home/jannes/RIOT/sys/pm_layered
"make" -C /home/jannes/RIOT/sys/stdio_uart
"make" -C /home/jannes/RIOT/sys/test_utils/interactive_sync
"make" -C /home/jannes/RIOT/sys/tsrb
/home/jannes/RIOT/tests/buttons/bin/nucleo-f207zg/core/kernel_init.o: In function `kernel_init':
/home/jannes/RIOT/core/kernel_init.c:72: multiple definition of `kernel_init'
/home/jannes/RIOT/tests/buttons/bin/nucleo-f207zg/core/init.o:/home/jannes/RIOT/core/init.c:78: first defined here
/home/jannes/RIOT/tests/buttons/bin/nucleo-f207zg/stm32_clk/stmclk_fx.o: In function `stmclk_init_sysclk':
/home/jannes/RIOT/cpu/stm32/stmclk/stmclk_fx.c:167: multiple definition of `stmclk_init_sysclk'
/home/jannes/RIOT/tests/buttons/bin/nucleo-f207zg/stm32_clk/stmclk_f2f4f7.o:/home/jannes/RIOT/cpu/stm32/stmclk/stmclk_f2f4f7.c:465: first defined here
/home/jannes/RIOT/tests/buttons/bin/nucleo-f207zg/stm32_vectors/STM32F207xx.o: In function `dummy_handler':
/home/jannes/RIOT/cpu/stm32/vectors/STM32F207xx.c:14: multiple definition of `dummy_handler'
/home/jannes/RIOT/tests/buttons/bin/nucleo-f207zg/cpu/vectors.o:/home/jannes/RIOT/cpu/stm32f2/vectors.c:28: first defined here
/home/jannes/RIOT/tests/buttons/bin/nucleo-f207zg/stm32_vectors/STM32F207xx.o:/home/jannes/RIOT/cpu/stm32/vectors/STM32F207xx.c:94: multiple definition of `vector_cpu'
/home/jannes/RIOT/tests/buttons/bin/nucleo-f207zg/cpu/vectors.o:/home/jannes/RIOT/cpu/stm32f2/vectors.c:109: first defined here
/home/jannes/RIOT/tests/buttons/bin/nucleo-f207zg/stm32_vectors/vectors_f2.o: In function `dummy_handler':
/home/jannes/RIOT/cpu/stm32/vectors/vectors_f2.c:28: multiple definition of `dummy_handler'
/home/jannes/RIOT/tests/buttons/bin/nucleo-f207zg/cpu/vectors.o:/home/jannes/RIOT/cpu/stm32f2/vectors.c:28: first defined here
/home/jannes/RIOT/tests/buttons/bin/nucleo-f207zg/stm32_vectors/vectors_f2.o:/home/jannes/RIOT/cpu/stm32/vectors/vectors_f2.c:109: multiple definition of `vector_cpu'
/home/jannes/RIOT/tests/buttons/bin/nucleo-f207zg/cpu/vectors.o:/home/jannes/RIOT/cpu/stm32f2/vectors.c:109: first defined here
collect2: error: ld returned 1 exit status
/home/jannes/RIOT/tests/buttons/../../Makefile.include:584: recipe for target '/home/jannes/RIOT/tests/buttons/bin/nucleo-f207zg/tests_buttons.elf' failed
make: *** [/home/jannes/RIOT/tests/buttons/bin/nucleo-f207zg/tests_buttons.elf] Error 1

aabadie · 2020-10-09T10:23:39Z

it is somehow no longer possible to build anything on the nucleo-f207zg

Can you try clean flash term targets ? You have object files from older builds.

JannesVolkens · 2020-10-09T10:31:04Z

Oh damn, I didn't see this :D.
Thank you @aabadie, everything is working now.

With the change from using archive files to using object files in PR RIOT-OS#14754, the module `esp_wifi_enterprise` was no longer compilable. The reason for this was that the file `bignum.c` was present in two different vendor sub-directories and created two identical object files, which then led to a symbol conflict when linking. This commit removes one of these identical files. The one that is used in all `esp_wifi` variants is kept, the one that is only used in `esp_wifi_enterprise` is dropped.

leandrolanzieri added Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors) Area: build system Area: Build system labels Aug 13, 2020

leandrolanzieri requested review from cgundogan, aabadie, kaspar030 and fjmolinas August 13, 2020 14:08

cgundogan added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Aug 13, 2020

leandrolanzieri force-pushed the pr/no_archives branch 2 times, most recently from 2d6358f to 9000eed Compare August 13, 2020 15:02

cgundogan added CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR and removed CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Aug 15, 2020

leandrolanzieri force-pushed the pr/no_archives branch from 6f793d2 to 6ea75e6 Compare August 18, 2020 08:29

leandrolanzieri marked this pull request as ready for review August 18, 2020 08:58

leandrolanzieri requested review from basilfx, bergzand, DipSwitch, gschorcht, jia200x, kYc0o, maribu, MichelRottleuthner, miri64, PeterKietzmann, roberthartung, smlng and vincent-d as code owners August 18, 2020 08:58

leandrolanzieri force-pushed the pr/no_archives branch from 6ea75e6 to f8d25ac Compare August 18, 2020 15:16

bergzand mentioned this pull request Sep 22, 2020

cortexm_common: Make no_thread_idle compatible with cortex-m0 #14557

Merged

bergzand mentioned this pull request Sep 23, 2020

tests/pkg_libhydrogen: test fails on master for the samr21-xpro with LLVM #15066

Open

fjmolinas mentioned this pull request Sep 30, 2020

esp32: mtd tests failing on CI #15123

Closed

benpicco mentioned this pull request Oct 4, 2020

tests: blacklist mtd using tests for esp32-wroom-32 #15124

Merged

leandrolanzieri mentioned this pull request Oct 8, 2020

cpu/esp*: fix linker scripts #15186

Merged

leandrolanzieri mentioned this pull request Oct 9, 2020

cpu/esp*: fix linker scripts [backport 2020.10] #15197

Merged

leandrolanzieri mentioned this pull request Nov 3, 2020

Makefile.include: discover object files in sub-paths while linking #15364

Merged

kaspar030 mentioned this pull request Aug 31, 2021

make: add -ffunction-sections -fdata-sections to LINKFLAGS if LTO=1 #16789

Merged

fjmolinas mentioned this pull request Oct 4, 2021

Deleted or non selected source files are linked in #16942

Closed

This was referenced Oct 4, 2021

Makefile.base: cleanup non selected source object files #16945

Merged

Makefile.base: cleanup non selected source object files [backport 2021.10] #16953

Merged

leandrolanzieri mentioned this pull request Oct 13, 2021

Makefile.base: do not clean objects of bindist modules #16981

Merged

gschorcht mentioned this pull request Dec 1, 2021

cpu/esp32: esp_wifi_enterprise compilation error #17305

Closed

gschorcht mentioned this pull request Dec 1, 2021

cpu/esp32: fix esp_wifi_enterprise compilation problem #17306

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

build system: link object files #14754

build system: link object files #14754

leandrolanzieri commented Aug 13, 2020

leandrolanzieri commented Aug 18, 2020

maribu commented Aug 18, 2020

leandrolanzieri commented Aug 18, 2020

leandrolanzieri commented Sep 22, 2020

leandrolanzieri commented Sep 23, 2020

bergzand commented Sep 23, 2020 •

edited

Loading

benpicco commented Oct 4, 2020

maribu commented Oct 4, 2020

leandrolanzieri commented Oct 5, 2020

gschorcht commented Oct 5, 2020

leandrolanzieri commented Oct 5, 2020

maribu commented Oct 5, 2020

leandrolanzieri commented Oct 8, 2020

JannesVolkens commented Oct 9, 2020

aabadie commented Oct 9, 2020 •

edited

Loading

JannesVolkens commented Oct 9, 2020

build system: link object files #14754

build system: link object files #14754

Conversation

leandrolanzieri commented Aug 13, 2020

Contribution description

Current problem

An example of the problem

Testing procedure

Issues/PRs references

leandrolanzieri commented Aug 18, 2020

maribu commented Aug 18, 2020

leandrolanzieri commented Aug 18, 2020

leandrolanzieri commented Sep 22, 2020

leandrolanzieri commented Sep 23, 2020

bergzand commented Sep 23, 2020 • edited Loading

benpicco commented Oct 4, 2020

maribu commented Oct 4, 2020

leandrolanzieri commented Oct 5, 2020

gschorcht commented Oct 5, 2020

leandrolanzieri commented Oct 5, 2020

maribu commented Oct 5, 2020

leandrolanzieri commented Oct 8, 2020

JannesVolkens commented Oct 9, 2020

aabadie commented Oct 9, 2020 • edited Loading

JannesVolkens commented Oct 9, 2020

bergzand commented Sep 23, 2020 •

edited

Loading

aabadie commented Oct 9, 2020 •

edited

Loading