On Wednesday 23 November 2022 18:15:17 Tom Rini wrote: > On Wed, Nov 23, 2022 at 11:50:59PM +0100, Pali Rohár wrote: > > On Tuesday 22 November 2022 12:31:56 Tom Rini wrote: > > > It is a bad idea, and more modern toolchains will fail, if you declare > > > an assembly function to be global and then weak, instead of declaring it > > > weak to start with. Update assorted assembly files to use the WEAK macro > > > directly. > > > > > > Signed-off-by: Tom Rini <tr...@konsulko.com> > > > > During debugging of Nokia N900 code I was looking at this and I > > originally thought that this redefinition is the issue why N900 u-boot > > did not work... (but as we now know, the n900 issue was somewhere else). > > > > So I agree with this change, feel free to add my: > > > > Reviewed-by: Pali Rohár <p...@kernel.org> > > > > ... but even after this change, linked u-boot.bin binary is > > not-so-correct. It works but has an issue: In final u-boot.bin binary > > there is both weak and non-weak version of every weak function. You can > > verify it for example by looking at "save_boot_params" code (really > > code, not just symbol) in u-boot ELF binary. > > > > The reason for this is that linker (even LTO enabled) cannot eliminate > > code for weak version of function because it does not know how to > > "drop" it from binary/assembly code. So linker just set that non-weak > > version of function is active and let non-weak version present in binary > > as probably dead code. > > > > This is affected only by assembly files, not by C files, because gcc is > > called with -ffunction-sections -fdata-sections flags which cause that > > every (weak) function is in its separate section (so C function > > "int abc() { ... }" is put into the section ".text.abc" instead of > > ".text") and linker's flag --gc-sections (or LTO optimization) then drop > > all unreferenced sections. > > > > I do not know how fix this issue in assembly files. But cannot be WEAK > > macro modified to change section to ".text.<entry_name>" to mimic C > > compiler behavior? Would this cause any issues? > > Yes, you're right about the cause, and potential solution. If you can > come up with a way to get each assembly function put in to a separate > .text.funcname section, that would be great and much appreciated. I > think I tried this at one point a long long time ago and it did work, > but I didn't have something clean, either. I think I was hoping that the > linux kernel folks would solve it in time, but they decided the > time/effort for --gc-sections wasn't worth it, in the end.
I quickly looked at this. If "as" is invoked with --sectname-subst flag then it is possible to use '.section %S.<func_name>' and '.previous' directives. See documentation where is example of that: https://sourceware.org/binutils/docs/as/Section.html#ELF-Version I experimented with adding into include/linux/linkage.h: #define WEAKSECT(name) \ .section .text.name ASM_NL \ WEAK(name) #define ENDPROCSECT(name) \ ENDPROC(name) ASM_NL \ .previous And then defining in arch/arm/cpu/armv7/start.S: WEAKSECT(save_boot_params) b save_boot_params_ret ENDPROCSECT(save_boot_params) (Note that n900 has custom non-weak save_boot_params) Then I run: make CROSS_COMPILE=arm-linux-gnueabi- nokia_rx51_defconfig u-boot.bin KBUILD_LDFLAGS="--print-gc-sections" And it showed me: ld: removing unused section '.text.save_boot_params' in file 'arch/arm/cpu/armv7/start.o' So seems that it is working. For proper integration it would be needed to integrate --sectname-subst flag support and then replace all usage by new macros.