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 trini@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 pali@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.