Optiboot / optiboot

Small and Fast Bootloader for Arduino and other Atmel AVR chips
Other
1.09k stars 401 forks source link

EEPROM-related linker error on avr-libc 1:1.8.0+Atmel3.5.0-1 / gcc 4.9 #233

Closed matthijskooijman closed 6 years ago

matthijskooijman commented 6 years ago

When compiling master, for a BIGBOOT device, with avr-libc 1:1.8.0+Atmel3.5.0-1 (from Debian stable):

$ make atmega1280
(...)
avr-gcc -g -Wall -Os -fno-split-wide-types -mrelax -mmcu=atmega1280 -DF_CPU=16000000L  -DBAUD_RATE=115200 -DLED_START_FLASHES=3      -DBIGBOOT  -Wl,--section-start=.text=0x1fc00  -Wl,--section-start=.version=0x1fffe -Wl,--relax -nostartfiles -nostdlib -o optiboot_atmega1280.elf optiboot.o -lc
optiboot.o: In function `appStart':
/home/matthijs/docs/Electronics/Arduino/optiboot/optiboot/bootloaders/optiboot/optiboot.c:858: undefined reference to `eeprom_write_byte'
/home/matthijs/docs/Electronics/Arduino/optiboot/optiboot/bootloaders/optiboot/optiboot.c:858: undefined reference to `eeprom_read_byte'
(...)

With the version from Debian oldstable (1:1.8.0+Atmel3.4.4-1), this works as expected. Investigation shows that these functions live in libc.a (note that the header file has some magic to rename the functions and add an mcu-specific suffix):

$ avr-objdump -t /usr/lib/gcc/avr/4.8.1/../../../avr/lib/avr51/libc.a|grep eewr_byte_m1280
eewr_byte_atmega128.o:     file format elf32-avr
SYMBOL TABLE:
00000000 g     F .text.avr-libc 0000001c __eewr_byte_m1280
(...)

It seems that in the newer version, these functions were moved from libc to a device-specific library (which also removes the need for adding the mcu-specific suffix):

$ avr-objdump -t /usr/lib/gcc/avr/4.9.2/../../../avr/lib/avr51/libatmega1280.a
eerd_byte.o:     file format elf32-avr
SYMBOL TABLE:
00000000 g     F .text.avr-libc 00000010 eeprom_read_byte
(...)

This device-specific library is normally included in the link, but it is excluded because optiboot passes -nostdlib.

This issue was also identified here, and the proposed fix is to add an explicit (e.g.) -latmega1280 to the linker commandline, but that requires hardcoding these for each supported target (and also relying on an internal compiler detail - the name of this library).

One other fix would be to omit -nostdlib. A quick test for the atmega328, atmega16, atmega1280 shows that the code size is not different, regardless of whether -nostdlib is passed. This also makes sense, as these standard libraries are passed to the linker (internally) using normal -l options, which only pulls in (parts of) these libraries when they are actually needed to satisfy an undefined symbol, so I would not really expect any size gain from -nostdlib. Or is there another reason to pass -nostdlib?

@MCUDude, it seems you have a big collection of optiboot compiled for various targets. Would it be easy for you to check if there is any impact of removing -nostdlib? I would even expect hex files to be identical, which would be the strongest indication that it does not matter.

matthijskooijman commented 6 years ago

@MCUdude, never mind, I already did some testing myself (your build scripts are convenient :-D)

Using https://github.com/MCUdude/optiboot_flash, I compiled (that version of) optiboot for all targets listed in that repo (except for 324pb and 328pb which my toolchain doesn't know yet). I compiled only for 16Mhz and 115200 to save time, which should not really matter for this comparison. I compiled with and without -nostdlib and saw that leaving out this option resulted in identical .hex files for all targets. I did this test for gcc 4.8.9 (from Debian) as well as 5.4.0 (from Arduino git master).

Hence, I would propose removing -nostdlib, since it does not seem to bring any gain, and does introduce problems in the compilation.

WestfW commented 6 years ago

Wow. Is it that simple? (Also see https://github.com/Optiboot/optiboot/issues/177 ) Does it still build with older versions of avr-gcc as well? (Do we care, any more?)

matthijskooijman commented 6 years ago

Ah, missed #177, thanks for pointing it out. It indeed seems this simple, and I would expect it also works on older and newer compilers. I noticed you fixed this in https://github.com/Optiboot/optiboot/commit/31cd92f1c9869f2ee9c383ea4476ef77a02fe8bd, thanks!