[PATCH 4/6] arm: Use the WEAK assembly entry point consistently

[Pali Rohár]

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 <trini at 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 at 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.