| Message ID | 20240103102311.4048759-1-abrodkin@synopsys.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | arch: Set max/common-page-size for libgcc & libstdc++ | expand |
Hello Alexey, Thanks for the patch. See below some comments. On Wed, 3 Jan 2024 02:23:11 -0800 Alexey Brodkin via buildroot <buildroot@buildroot.org> wrote: > I would agree that it gets more and more ugly, but so far I cannot see > any more elegant solution. So if there're suggestions how to improve, > let's discuss. Indeed, it's not super great as you realized yourself. Also, I believe it's not the first time we identify a discrepancy in CFLAGS that are in the wrapper, but that we forget to pass when building gcc/libc. See your own commit d2ae7eb2a24c9033f8d8a9f81c5fe2ff2febdb5f from 2020, which also added -matomic in gcc.mk. Things are also a bit messy in the different C libraries: - uClibc is built with $(TARGET_ABI) and $(TARGET_DEBUGGING) - glibc is built with $(TARGET_OPTIMIZATION) and a bunch of additional flags - musl is built with the full $(TARGET_CFLAGS) Seems a bit meh to me. So I think ideally we would to distinguish in a clear manner: - CFLAGS that are passed through the wrapper, and which therefore need to be manually passed when building gcc and C libraries - CFLAGS that are passed through the environment I think at least for now what could be done is to use $(ARCH_TOOLCHAIN_WRAPPER_OPTS) in gcc.mk. But I'm surprised they are not needed also in the C library build. Why are you not seeing the same problem with the C library build, it should also need the same page size flags, doesn't it? Thomas
Thomas, Alexey, All, On 2024-01-04 22:19 +0100, Thomas Petazzoni spake thusly: > On Wed, 3 Jan 2024 02:23:11 -0800 > Alexey Brodkin via buildroot <buildroot@buildroot.org> wrote: > > I would agree that it gets more and more ugly, but so far I cannot see > > any more elegant solution. So if there're suggestions how to improve, > > let's discuss. [--SNIP--] > But I'm surprised they are not needed also in the C library build. Why > are you not seeing the same problem with the C library build, it should > also need the same page size flags, doesn't it? It is my understanding that the toolchain wrapper is installed as a post initial gcc hook as well: package/gcc/gcc-initial/gcc-initial.mk: 59 HOST_GCC_INITIAL_POST_BUILD_HOOKS += TOOLCHAIN_WRAPPER_BUILD 60 HOST_GCC_INITIAL_POST_INSTALL_HOOKS += TOOLCHAIN_WRAPPER_INSTALL 61 HOST_GCC_INITIAL_POST_INSTALL_HOOKS += HOST_GCC_INSTALL_WRAPPER_AND_SIMPLE_SYMLINKS So the C libraries are built using our wrapper. Regards, Yann E. MORIN.
On Thu, 4 Jan 2024 22:34:34 +0100 "Yann E. MORIN" <yann.morin.1998@free.fr> wrote: > > But I'm surprised they are not needed also in the C library build. Why > > are you not seeing the same problem with the C library build, it should > > also need the same page size flags, doesn't it? > > It is my understanding that the toolchain wrapper is installed as a post > initial gcc hook as well: > > package/gcc/gcc-initial/gcc-initial.mk: > 59 HOST_GCC_INITIAL_POST_BUILD_HOOKS += TOOLCHAIN_WRAPPER_BUILD > 60 HOST_GCC_INITIAL_POST_INSTALL_HOOKS += TOOLCHAIN_WRAPPER_INSTALL > 61 HOST_GCC_INITIAL_POST_INSTALL_HOOKS += HOST_GCC_INSTALL_WRAPPER_AND_SIMPLE_SYMLINKS > > So the C libraries are built using our wrapper. Hum, indeed. But then, why doesn't that apply to libgcc and libstdc++ as well? It is because libgcc/libstdc++ are built with the second stage gcc, before it is installed and wrapped by our wrapper? I guess so because the first stage gcc isn't capable of building shared libraries, or something like that. So, indeed, my proposal to Alexey is to use ARCH_TOOLCHAIN_WRAPPER_OPTS in gcc.mk. This will allow to get rid of the special -matomic addition. Thomas
Hi Thomas, > On Thu, 4 Jan 2024 22:34:34 +0100 > "Yann E. MORIN" <yann.morin.1998@free.fr> wrote: > > > > But I'm surprised they are not needed also in the C library build. Why > > > are you not seeing the same problem with the C library build, it should > > > also need the same page size flags, doesn't it? > > > > It is my understanding that the toolchain wrapper is installed as a post > > initial gcc hook as well: > > > > package/gcc/gcc-initial/gcc-initial.mk: > > 59 HOST_GCC_INITIAL_POST_BUILD_HOOKS += TOOLCHAIN_WRAPPER_BUILD > > 60 HOST_GCC_INITIAL_POST_INSTALL_HOOKS += TOOLCHAIN_WRAPPER_INSTALL > > 61 HOST_GCC_INITIAL_POST_INSTALL_HOOKS += HOST_GCC_INSTALL_WRAPPER_AND_SIMPLE_SYMLINKS > > > > So the C libraries are built using our wrapper. Right. > Hum, indeed. But then, why doesn't that apply to libgcc and libstdc++ > as well? It is because libgcc/libstdc++ are built with the second stage > gcc, before it is installed and wrapped by our wrapper? I guess so > because the first stage gcc isn't capable of building shared libraries, > or something like that. libgcc_s.so.1 gets built with: -------------------->8--------------------- .../output/build/host-gcc-final-arc-2023.09-release/build/./gcc/xgcc ... -o libgcc_s.so.1 -------------------->8--------------------- while libstdc++.so gets built with: -------------------->8--------------------- libtool: link: .../buildroot/output/build/host-gcc-final-arc-2023.09-release/build/./gcc/xgcc ... -o .libs/libstdc++.so.6.0.32 -------------------->8--------------------- And that's the reason Buildroot toolchain wrapper doesn't affect them both. Also from the snippets above you may see why they need separate env variables: CFLAGS for libgcc and LDFLAGS for libstdc++. BTW, that's the same problem I solved with adding "-matomic" flag on libgcc compilation via https://git.buildroot.net/buildroot/commit/?id=d2ae7eb2a24c9033f8d8a9f81c5fe2ff2febdb5f > So, indeed, my proposal to Alexey is to use ARCH_TOOLCHAIN_WRAPPER_OPTS > in gcc.mk. This will allow to get rid of the special -matomic addition. Do you mean something like that in gcc.mk? ----------------------->8--------------------- GCC_COMMON_TARGET_CFLAGS = $(TARGET_CFLAGS) $(ARCH_TOOLCHAIN_WRAPPER_OPTS) GCC_COMMON_TARGET_CXXFLAGS = $(TARGET_CXXFLAGS) $(ARCH_TOOLCHAIN_WRAPPER_OPTS) GCC_COMMON_TARGET_LDFLAGS = $(TARGET_LDFLAGS) $(ARCH_TOOLCHAIN_WRAPPER_OPTS) ----------------------->8--------------------- -Alexey
On Thu, 4 Jan 2024 22:22:45 +0000 Alexey Brodkin <Alexey.Brodkin@synopsys.com> wrote: > Do you mean something like that in gcc.mk? > ----------------------->8--------------------- > GCC_COMMON_TARGET_CFLAGS = $(TARGET_CFLAGS) $(ARCH_TOOLCHAIN_WRAPPER_OPTS) > GCC_COMMON_TARGET_CXXFLAGS = $(TARGET_CXXFLAGS) $(ARCH_TOOLCHAIN_WRAPPER_OPTS) > GCC_COMMON_TARGET_LDFLAGS = $(TARGET_LDFLAGS) $(ARCH_TOOLCHAIN_WRAPPER_OPTS) > ----------------------->8--------------------- Yes, exactly, and removing: # Make sure libgcc & libstdc++ always get built with -matomic on ARC700 ifeq ($(GCC_TARGET_CPU):$(BR2_ARC_ATOMIC_EXT),arc700:y) GCC_COMMON_TARGET_CFLAGS += -matomic GCC_COMMON_TARGET_CXXFLAGS += -matomic endif because this will now be taken care of by $(ARCH_TOOLCHAIN_WRAPPER_OPTS) Thomas
diff --git a/arch/arch.mk b/arch/arch.mk index 2e737b92ac..23733eb199 100644 --- a/arch/arch.mk +++ b/arch/arch.mk @@ -21,12 +21,20 @@ GCC_TARGET_MODE := $(call qstrip,$(BR2_GCC_TARGET_MODE)) # Explicitly set LD's "max-page-size" instead of relying on some defaults ifeq ($(BR2_ARC_PAGE_SIZE_4K)$(BR2_ARM64_PAGE_SIZE_4K),y) ARCH_TOOLCHAIN_WRAPPER_OPTS += -Wl,-z,max-page-size=4096 -Wl,-z,common-page-size=4096 +TARGET_CFLAGS += -Wl,-z,max-page-size=4096 -Wl,-z,common-page-size=4096 +TARGET_LDFLAGS += -Wl,-z,max-page-size=4096 -Wl,-z,common-page-size=4096 else ifeq ($(BR2_ARC_PAGE_SIZE_8K),y) ARCH_TOOLCHAIN_WRAPPER_OPTS += -Wl,-z,max-page-size=8192 -Wl,-z,common-page-size=8192 +TARGET_CFLAGS += -Wl,-z,max-page-size=8192 -Wl,-z,common-page-size=8192 +TARGET_LDFLAGS += -Wl,-z,max-page-size=8192 -Wl,-z,common-page-size=8192 else ifeq ($(BR2_ARC_PAGE_SIZE_16K),y) ARCH_TOOLCHAIN_WRAPPER_OPTS += -Wl,-z,max-page-size=16384 -Wl,-z,common-page-size=16384 +TARGET_CFLAGS += -Wl,-z,max-page-size=16384 -Wl,-z,common-page-size=16384 +TARGET_LDFLAGS += -Wl,-z,max-page-size=16384 -Wl,-z,common-page-size=16384 else ifeq ($(BR2_ARM64_PAGE_SIZE_64K),y) ARCH_TOOLCHAIN_WRAPPER_OPTS += -Wl,-z,max-page-size=65536 -Wl,-z,common-page-size=65536 +TARGET_CFLAGS += -Wl,-z,max-page-size=65536 -Wl,-z,common-page-size=65536 +TARGET_LDFLAGS += -Wl,-z,max-page-size=65536 -Wl,-z,common-page-size=65536 endif # Include any architecture specific makefiles. diff --git a/package/gcc/gcc.mk b/package/gcc/gcc.mk index 93bc46643c..d9a2164c74 100644 --- a/package/gcc/gcc.mk +++ b/package/gcc/gcc.mk @@ -100,6 +100,7 @@ HOST_GCC_COMMON_MAKE_OPTS = \ GCC_COMMON_TARGET_CFLAGS = $(TARGET_CFLAGS) GCC_COMMON_TARGET_CXXFLAGS = $(TARGET_CXXFLAGS) +GCC_COMMON_TARGET_LDFLAGS = $(TARGET_LDFLAGS) # used to fix ../../../../libsanitizer/libbacktrace/../../libbacktrace/elf.c:772:21: error: 'st.st_mode' may be used uninitialized in this function [-Werror=maybe-uninitialized] ifeq ($(BR2_ENABLE_DEBUG),y) @@ -115,6 +116,7 @@ endif # Propagate options used for target software building to GCC target libs HOST_GCC_COMMON_CONF_ENV += CFLAGS_FOR_TARGET="$(GCC_COMMON_TARGET_CFLAGS)" HOST_GCC_COMMON_CONF_ENV += CXXFLAGS_FOR_TARGET="$(GCC_COMMON_TARGET_CXXFLAGS)" +HOST_GCC_COMMON_CONF_ENV += LDFLAGS_FOR_TARGET="$(GCC_COMMON_TARGET_LDFLAGS)" HOST_GCC_COMMON_CONF_ENV += AR_FOR_TARGET=gcc-ar NM_FOR_TARGET=gcc-nm RANLIB_FOR_TARGET=gcc-ranlib # libitm needs sparc V9+
With [1], [2] & [3] we made sure Buildroot packages get built with proper MMU page size assumed. This was done nicely through insertion of required flags into the toolchain wrapper so that there's no need to pass these flags to each and every package separately - toolchain wrapper used for real building has all set internally and so proper flags are impleicitly used. But there's yet another corner case which is not handled that way - these are binaries or rather libraries which are being used as a part of GCC compilation: libgcc_s.so.1 and libstdc++.so. And so to make sure both the libraries get built properly we need to set TARGET_CFLAGS (cures libgcc_s.so) & TARGET_LDFLAGS (cures libstdc++.so). In case of ARM by defaut 64 KiB page size seems to be used, as w/o that patch I see the following for BR2_ARM64_PAGE_SIZE_4K=y: --------------------------->8---------------------------- $ ./output/host/bin/aarch64-linux-readelf -l ./output/target/lib/libgcc_s.so.1 Elf file type is DYN (Shared object file) Entry point 0x0 There are 6 program headers, starting at offset 64 Program Headers: Type Offset VirtAddr PhysAddr FileSiz MemSiz Flags Align LOAD 0x0000000000000000 0x0000000000000000 0x0000000000000000 0x0000000000013d1c 0x0000000000013d1c R E 0x10000 LOAD 0x000000000001fd98 0x000000000002fd98 0x000000000002fd98 0x0000000000000438 0x00000000000005c8 RW 0x10000 DYNAMIC 0x000000000001fdb8 0x000000000002fdb8 0x000000000002fdb8 0x0000000000000200 0x0000000000000200 RW 0x8 $ ./output/host/bin/aarch64-linux-readelf -l ./output/target/usr/lib/libstdc++.so.6.0.32 Elf file type is DYN (Shared object file) Entry point 0x0 There are 7 program headers, starting at offset 64 Program Headers: Type Offset VirtAddr PhysAddr FileSiz MemSiz Flags Align LOAD 0x0000000000000000 0x0000000000000000 0x0000000000000000 0x00000000001d3462 0x00000000001d3462 R E 0x10000 LOAD 0x00000000001d5760 0x00000000001e5760 0x00000000001e5760 0x000000000000e528 0x0000000000012de8 RW 0x10000 DYNAMIC 0x00000000001deef0 0x00000000001eeef0 0x00000000001eeef0 0x0000000000000240 0x0000000000000240 RW 0x8 --------------------------->8---------------------------- Note alignment of 0x10000 in sections marked for loading. And with the patch applied we get expected alignment of 0x1000 (4 KiB): --------------------------->8---------------------------- $ ./output/host/bin/aarch64-linux-readelf -l ./output/target/lib/libgcc_s.so.1 Elf file type is DYN (Shared object file) Entry point 0x0 There are 6 program headers, starting at offset 64 Program Headers: Type Offset VirtAddr PhysAddr FileSiz MemSiz Flags Align LOAD 0x0000000000000000 0x0000000000000000 0x0000000000000000 0x0000000000013d1c 0x0000000000013d1c R E 0x1000 LOAD 0x0000000000013d98 0x0000000000014d98 0x0000000000014d98 0x0000000000000438 0x00000000000005c8 RW 0x1000 DYNAMIC 0x0000000000013db8 0x0000000000014db8 0x0000000000014db8 0x0000000000000200 0x0000000000000200 RW 0x8 $ ./output/host/bin/aarch64-linux-readelf -l ./output/target/usr/lib/libstdc++.so.6.0.32 Elf file type is DYN (Shared object file) Entry point 0x0 There are 7 program headers, starting at offset 64 Program Headers: Type Offset VirtAddr PhysAddr FileSiz MemSiz Flags Align LOAD 0x0000000000000000 0x0000000000000000 0x0000000000000000 0x00000000001d3462 0x00000000001d3462 R E 0x1000 LOAD 0x00000000001d3760 0x00000000001d4760 0x00000000001d4760 0x000000000000e528 0x0000000000012de8 RW 0x1000 DYNAMIC 0x00000000001dcef0 0x00000000001ddef0 0x00000000001ddef0 0x0000000000000240 0x0000000000000240 RW 0x8 --------------------------->8---------------------------- I would agree that it gets more and more ugly, but so far I cannot see any more elegant solution. So if there're suggestions how to improve, let's discuss. [1] https://git.buildroot.net/buildroot/commit/?id=3cc2c6d19ab2e1bb4634f26f9318da9b07df5fff [2] https://git.buildroot.net/buildroot/commit/?id=dcb74db89e74e512e36b32cea6f574a1a1ca84c4 [3] https://git.buildroot.net/buildroot/commit/?id=5e52c28397b79f8c4c99552217cbe95202166626 Signed-off-by: Alexey Brodkin <abrodkin@synopsys.com> Signed-off-by: Vladimir Isaev <VVIsaev@gmail.com> Signed-off-by: Pavel Kozlov <kozlov@synopsys.com> Cc: Thomas Petazzoni <thomas.petazzoni@bootlin.com> Cc: Yann E. MORIN <yann.morin.1998@free.fr> Cc: Arnout Vandecappelle <arnout@mind.be> --- arch/arch.mk | 8 ++++++++ package/gcc/gcc.mk | 2 ++ 2 files changed, 10 insertions(+)