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

On Tue, May 2, 2023 at 11:30 PM Bin Meng bmeng.cn@gmail.com wrote:

Hi Simon,

On Sun, Apr 30, 2023 at 9:30 AM Simon Glass sjg@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@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-wea...
• gcc: https://gcc.gnu.org/onlinedocs/gcc/Common-Variable-Attributes.html#index-wea...

*/ +#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.