[PATCH v2 26/30] build: Disable weak symbols for MSYS2

[Fangrui Song]

On Tue, May 2, 2023 at 11:30 PM Bin Meng <bmeng.cn at gmail.com> wrote:
>
> Hi Simon,
>
> On Sun, Apr 30, 2023 at 9:30 AM Simon Glass <sjg at chromium.org> wrote:
> >
> > Weak symbols are not well supported by the PE format, so disable them.
> > We need to manually ensure that only one function is present in the source
> > code.
> >
> > Add a Kconfig option to control this and enable it when building for
> > Windows.
> >
> > Signed-off-by: Simon Glass <sjg at chromium.org>
> > ---
> >
> > (no changes since v1)
> >
> >  Kconfig                             | 12 ++++++++++++
> >  include/linux/compiler_attributes.h |  4 ++++
> >  2 files changed, 16 insertions(+)
> >
> > diff --git a/Kconfig b/Kconfig
> > index 9ac816abef1c..985b09680934 100644
> > --- a/Kconfig
> > +++ b/Kconfig
> > @@ -75,6 +75,18 @@ config CLANG_VERSION
> >  config CC_IS_MSYS
> >         def_bool $(success,uname -o | grep -q Msys)
> >
> > +config WEAK_SYMBOLS
> > +       bool "Enable use of weak symbols"
> > +       default y if !CC_IS_MSYS
> > +       help
> > +         The Portable Executable (PE) format used by Windows does not support
> > +         weak symbols very well. Even where it can be made to work, the __weak
> > +         function attribute cannot be made to work with PE. Supporting weak
> > +         symbols would involve changing the source code in undesirable ways.
> > +
> > +         This option controls whether weak symbols are used, or not. When
> > +         disabled, the __weak function attribute does nothing.
> > +
> >  choice
> >         prompt "Optimization level"
> >         default CC_OPTIMIZE_FOR_SIZE
> > diff --git a/include/linux/compiler_attributes.h b/include/linux/compiler_attributes.h
> > index 44c9a08d7346..c954109a065b 100644
> > --- a/include/linux/compiler_attributes.h
> > +++ b/include/linux/compiler_attributes.h
> > @@ -268,6 +268,10 @@
> >   *   gcc: https://gcc.gnu.org/onlinedocs/gcc/Common-Function-Attributes.html#index-weak-function-attribute
> >   *   gcc: https://gcc.gnu.org/onlinedocs/gcc/Common-Variable-Attributes.html#index-weak-variable-attribute
> >   */
> > +#ifdef CONFIG_WEAK_SYMBOLS
> >  #define __weak                          __attribute__((__weak__))
> > +#else
> > +#define __weak
> > +#endif
> >
> >  #endif /* __LINUX_COMPILER_ATTRIBUTES_H */
> > --
>
> I am adding Fangrui who is a toolchain expert to this thread.
>
> I chatted with him off-line, he thought we could probably go with the
> GCC+lld-link route. GNU ld's PE/COFF support is quite out of date.
>
> Maybe switching to lld-link could also solve the linker script issue
> you are trying to resolve in patch#23 "sandbox: Augment the linker
> script for MSYS2" ?
>
> Regards,
> Bin

https://maskray.me/blog/2021-04-25-weak-symbol#pecoff has some notes
about weak symbol support for PE/COFF.
The "undefined reference" issue is like the following:

printf 'void f(); int main() { f(); }' > a.c
printf '__attribute__((weak)) void f() {} void unused() {}' > b.c
x86_64-w64-mingw32-gcc -c a.c b.c

% x86_64-w64-mingw32-gcc a.o b.o
/usr/bin/x86_64-w64-mingw32-ld: /tmp/ccyRH2ap.o:a.c:(.text+0xe):
undefined reference to `f'
collect2: error: ld returned 1 exit status

Making the __weak macro a no-op drops all the semantics about weak,
which would likely lead to some pitfalls.
I suggest that u-boot doesn't add support for a linker that doesn't
the above case.

If u-boot is willing to refactor some code to avoid the above
situation, I think it'd be fine as well.


-- 
宋方睿