[Buildroot] [PATCH 1/1] package/containerd: fix build with toolchain without Gold linker

[Buildroot] [PATCH 1/1] package/containerd: fix build with toolchain without Gold linker

[Fiona Klute via buildroot]

From: "Fiona Klute (WIWA)" <fiona.klute@gmx.de>

Go forces use of the Gold linker on aarch64 due to a bug in BFD (the
default GNU linker) that is fixed in Binutils >= 2.41 [1]. All
toolchains provided by Buildroot have the fix, and none of them
provide Gold, so the forced setting breaks dynamic linking in
Buildroot. Set EXTLDFLAGS to use BFD instead.

Additionally disable PIE on musl to fix "read-only segment has dynamic
relocations" errors during linking [2].

[1] https://github.com/golang/go/issues/22040
[2] https://github.com/golang/go/issues/17847

Signed-off-by: Fiona Klute (WIWA) <fiona.klute@gmx.de>
---
This patch requires Yann E. Morin's patch that introduces
PACKAGE_EXTLDFLAGS:
https://patchwork.ozlabs.org/project/buildroot/patch/876f3a7bb6a2375193fc8f06ab856d2449f83727.1699547993.git.yann.morin@orange.com/

Based on the linked issue the --no-pie linker flag should be needed
only for musl, but I'd be grateful if anyone could test with glibc.

 package/containerd/containerd.mk | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/package/containerd/containerd.mk b/package/containerd/containerd.mk
index ee07c22495..cb648f87a9 100644
--- a/package/containerd/containerd.mk
+++ b/package/containerd/containerd.mk
@@ -49,6 +49,20 @@ ifneq ($(BR2_PACKAGE_CONTAINERD_CRI),y)
 CONTAINERD_TAGS += no_cri
 endif
 
+ifeq ($(BR2_aarch64),y)
+# Go forces use of the Gold linker on aarch64 due to a bug in BFD that
+# is fixed in Binutils >= 2.41 (so all versions provided by
+# Buildroot). Forcing Gold will break with toolchains that don't
+# provide it, so override the flag and use BFD.
+# See: https://github.com/golang/go/issues/22040
+CONTAINERD_EXTLDFLAGS += -fuse-ld=bfd
+ifeq ($(BR2_TOOLCHAIN_USES_MUSL),y)
+# Go exe build with PIE doesn't work with musl.
+# See: https://github.com/golang/go/issues/17847
+CONTAINERD_EXTLDFLAGS += -Wl,--no-pie
+endif
+endif
+
 define CONTAINERD_INSTALL_INIT_SYSTEMD
 	$(INSTALL) -D -m 0644 $(@D)/containerd.service \
 		$(TARGET_DIR)/usr/lib/systemd/system/containerd.service
-- 
2.47.2

_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH 1/1] package/containerd: fix build with toolchain without Gold linker

[yann.morin]

Fiona, All,

On 2025-02-04 15:32 +0100, Fiona Klute spake thusly:
> From: "Fiona Klute (WIWA)" <fiona.klute@gmx.de>
[--SNIP--]
> Additionally disable PIE on musl to fix "read-only segment has dynamic
> relocations" errors during linking [2].
[--SNIP--]
> +ifeq ($(BR2_aarch64),y)
> +# Go forces use of the Gold linker on aarch64 due to a bug in BFD that
> +# is fixed in Binutils >= 2.41 (so all versions provided by
> +# Buildroot). Forcing Gold will break with toolchains that don't
> +# provide it, so override the flag and use BFD.
> +# See: https://github.com/golang/go/issues/22040
> +CONTAINERD_EXTLDFLAGS += -fuse-ld=bfd
> +ifeq ($(BR2_TOOLCHAIN_USES_MUSL),y)
> +# Go exe build with PIE doesn't work with musl.
> +# See: https://github.com/golang/go/issues/17847
> +CONTAINERD_EXTLDFLAGS += -Wl,--no-pie
> +endif
> +endif

Is the PIE issue really only hapenning on Aarch64? The upstream issue
17847 does not make it that obvious, but at least mips64 is said to have
the issue as well, and someone even mentioned amd64 (aka x86_64).

I.e. don't we need the musl workaround out of the aarch64 condition?

Regards,
Yann E. MORIN.

-- 
                                        ____________
.-----------------.--------------------:       _    :------------------.
|  Yann E. MORIN  | Real-Time Embedded |    __/ )   | /"\ ASCII RIBBON |
|                 | Software  Designer |  _/ - /'   | \ / CAMPAIGN     |
| +33 638.411.245 '--------------------: (_    `--, |  X  AGAINST      |
| yann.morin (at) orange.com           |_="    ,--' | / \ HTML MAIL    |
'--------------------------------------:______/_____:------------------'

____________________________________________________________________________________________________________
Ce message et ses pieces jointes peuvent contenir des informations confidentielles ou privilegiees et ne doivent donc
pas etre diffuses, exploites ou copies sans autorisation. Si vous avez recu ce message par erreur, veuillez le signaler
a l'expediteur et le detruire ainsi que les pieces jointes. Les messages electroniques etant susceptibles d'alteration,
Orange decline toute responsabilite si ce message a ete altere, deforme ou falsifie. Merci.

This message and its attachments may contain confidential or privileged information that may be protected by law;
they should not be distributed, used or copied without authorisation.
If you have received this email in error, please notify the sender and delete this message and its attachments.
As emails may be altered, Orange is not liable for messages that have been modified, changed or falsified.
Thank you.
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH 1/1] package/containerd: fix build with toolchain without Gold linker

[Fiona Klute via buildroot]

Hi Yann!

Am 04.02.25 um 15:47 schrieb yann.morin@orange.com:
> Fiona, All,
>
> On 2025-02-04 15:32 +0100, Fiona Klute spake thusly:
>> From: "Fiona Klute (WIWA)" <fiona.klute@gmx.de>
> [--SNIP--]
>> Additionally disable PIE on musl to fix "read-only segment has dynamic
>> relocations" errors during linking [2].
> [--SNIP--]
>> +ifeq ($(BR2_aarch64),y)
>> +# Go forces use of the Gold linker on aarch64 due to a bug in BFD that
>> +# is fixed in Binutils >= 2.41 (so all versions provided by
>> +# Buildroot). Forcing Gold will break with toolchains that don't
>> +# provide it, so override the flag and use BFD.
>> +# See: https://github.com/golang/go/issues/22040
>> +CONTAINERD_EXTLDFLAGS += -fuse-ld=bfd
>> +ifeq ($(BR2_TOOLCHAIN_USES_MUSL),y)
>> +# Go exe build with PIE doesn't work with musl.
>> +# See: https://github.com/golang/go/issues/17847
>> +CONTAINERD_EXTLDFLAGS += -Wl,--no-pie
>> +endif
>> +endif
>
> Is the PIE issue really only hapenning on Aarch64? The upstream issue
> 17847 does not make it that obvious, but at least mips64 is said to have
> the issue as well, and someone even mentioned amd64 (aka x86_64).
>
> I.e. don't we need the musl workaround out of the aarch64 condition?
I'm not sure. The Go code has a check if PIE works with the given
compiler & linker and disables it otherwise [1], and I didn't run into
this problem when patching the forced --fuse-ld=gold out of Go, so I
*assume* the --fuse-ld=bfd override breaks that check in some way and we
don't need --no-pie it if we don't override the linker.

It definitely would be good to actually test that. I'll see if I can fit
a build in, though I'd appreciate if someone who can actually test the
result gives it a try. Note that the bug will only show with a toolchain
that builds PIE by default.

Best regards,
Fiona

[1]
https://github.com/golang/go/blob/46fd6b4e37f043b57a2d426a633f61ffd4c40931/src/cmd/link/internal/ld/lib.go#L1879-L1894

_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH 1/1] package/containerd: fix build with toolchain without Gold linker

[Fiona Klute via buildroot]

[-- Attachment #1: Type: text/plain, Size: 2434 bytes --]

Am 04.02.25 um 19:36 schrieb Fiona Klute:
> Hi Yann!
>
> Am 04.02.25 um 15:47 schrieb yann.morin@orange.com:
>> Fiona, All,
>>
>> On 2025-02-04 15:32 +0100, Fiona Klute spake thusly:
>>> From: "Fiona Klute (WIWA)" <fiona.klute@gmx.de>
>> [--SNIP--]
>>> Additionally disable PIE on musl to fix "read-only segment has dynamic
>>> relocations" errors during linking [2].
>> [--SNIP--]
>>> +ifeq ($(BR2_aarch64),y)
>>> +# Go forces use of the Gold linker on aarch64 due to a bug in BFD that
>>> +# is fixed in Binutils >= 2.41 (so all versions provided by
>>> +# Buildroot). Forcing Gold will break with toolchains that don't
>>> +# provide it, so override the flag and use BFD.
>>> +# See: https://github.com/golang/go/issues/22040
>>> +CONTAINERD_EXTLDFLAGS += -fuse-ld=bfd
>>> +ifeq ($(BR2_TOOLCHAIN_USES_MUSL),y)
>>> +# Go exe build with PIE doesn't work with musl.
>>> +# See: https://github.com/golang/go/issues/17847
>>> +CONTAINERD_EXTLDFLAGS += -Wl,--no-pie
>>> +endif
>>> +endif
>>
>> Is the PIE issue really only hapenning on Aarch64? The upstream issue
>> 17847 does not make it that obvious, but at least mips64 is said to have
>> the issue as well, and someone even mentioned amd64 (aka x86_64).
>>
>> I.e. don't we need the musl workaround out of the aarch64 condition?
> I'm not sure. The Go code has a check if PIE works with the given
> compiler & linker and disables it otherwise [1], and I didn't run into
> this problem when patching the forced --fuse-ld=gold out of Go, so I
> *assume* the --fuse-ld=bfd override breaks that check in some way and we
> don't need --no-pie it if we don't override the linker.
>
> It definitely would be good to actually test that. I'll see if I can fit
> a build in, though I'd appreciate if someone who can actually test the
> result gives it a try. Note that the bug will only show with a toolchain
> that builds PIE by default.

Update: I've tested it now with a build for x86_64 Qemu (config based on
configs/qemu_x86_64_defconfig attached). Build passes and works
(alpine:latest runs), except not with the default Qemu command line from
board/qemu/x86_64/readme.txt: I had to add "-cpu max", with the default
Docker refuses to start due to missing CPU features.

On mips64 I can't enable Docker due to
BR2_PACKAGE_HOST_GO_TARGET_CGO_LINKING_SUPPORTS=n.

Conclusion: I think the patch is good as-is. :-)

Best regards,
Fiona

[-- Attachment #2: qemu_x86_64_docker.config --]
[-- Type: text/plain, Size: 871 bytes --]

BR2_x86_64=y
BR2_TOOLCHAIN_EXTERNAL=y
BR2_TOOLCHAIN_EXTERNAL_BOOTLIN_X86_64_MUSL_STABLE=y
BR2_DL_DIR="$(HOME)/.buildroot/dl"
BR2_GLOBAL_PATCH_DIR="board/qemu/patches"
BR2_DOWNLOAD_FORCE_CHECK_HASHES=y
BR2_SYSTEM_DHCP="eth0"
BR2_ROOTFS_POST_BUILD_SCRIPT="board/qemu/x86_64/post-build.sh"
BR2_ROOTFS_POST_IMAGE_SCRIPT="board/qemu/post-image.sh"
BR2_ROOTFS_POST_SCRIPT_ARGS="$(BR2_DEFCONFIG)"
BR2_LINUX_KERNEL=y
BR2_LINUX_KERNEL_CUSTOM_VERSION=y
BR2_LINUX_KERNEL_CUSTOM_VERSION_VALUE="6.12.9"
BR2_LINUX_KERNEL_USE_CUSTOM_CONFIG=y
BR2_LINUX_KERNEL_CUSTOM_CONFIG_FILE="board/qemu/x86_64/linux.config"
BR2_LINUX_KERNEL_NEEDS_HOST_LIBELF=y
BR2_PACKAGE_CA_CERTIFICATES=y
BR2_PACKAGE_DOCKER_CLI=y
BR2_PACKAGE_DOCKER_ENGINE=y
BR2_TARGET_ROOTFS_EXT2=y
BR2_TARGET_ROOTFS_EXT2_SIZE="256M"
# BR2_TARGET_ROOTFS_TAR is not set
BR2_PACKAGE_HOST_QEMU=y
BR2_PACKAGE_HOST_QEMU_SYSTEM_MODE=y

[-- Attachment #3: Type: text/plain, Size: 150 bytes --]

_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH 1/1] package/containerd: fix build with toolchain without Gold linker

[Christian Stewart via buildroot]

[-- Attachment #1.1: Type: text/plain, Size: 2570 bytes --]

Hi Fiona, Yann, all,



On Tue, Feb 4, 2025, 6:32 AM Fiona Klute <fiona.klute@gmx.de> wrote:

> From: "Fiona Klute (WIWA)" <fiona.klute@gmx.de>
>
> Go forces use of the Gold linker on aarch64 due to a bug in BFD (the
> default GNU linker) that is fixed in Binutils >= 2.41 [1]. All
> toolchains provided by Buildroot have the fix, and none of them
> provide Gold, so the forced setting breaks dynamic linking in
> Buildroot. Set EXTLDFLAGS to use BFD instead.
>
> Additionally disable PIE on musl to fix "read-only segment has dynamic
> relocations" errors during linking [2].
>
> [1] https://github.com/golang/go/issues/22040
> [2] https://github.com/golang/go/issues/17847
>
> Signed-off-by: Fiona Klute (WIWA) <fiona.klute@gmx.de>
> ---
> This patch requires Yann E. Morin's patch that introduces
> PACKAGE_EXTLDFLAGS:
>
> https://patchwork.ozlabs.org/project/buildroot/patch/876f3a7bb6a2375193fc8f06ab856d2449f83727.1699547993.git.yann.morin@orange.com/
>
> Based on the linked issue the --no-pie linker flag should be needed
> only for musl, but I'd be grateful if anyone could test with glibc.
>
>  package/containerd/containerd.mk | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
>
> diff --git a/package/containerd/containerd.mk b/package/containerd/
> containerd.mk
> index ee07c22495..cb648f87a9 100644
> --- a/package/containerd/containerd.mk
> +++ b/package/containerd/containerd.mk
> @@ -49,6 +49,20 @@ ifneq ($(BR2_PACKAGE_CONTAINERD_CRI),y)
>  CONTAINERD_TAGS += no_cri
>  endif
>
> +ifeq ($(BR2_aarch64),y)
> +# Go forces use of the Gold linker on aarch64 due to a bug in BFD that
> +# is fixed in Binutils >= 2.41 (so all versions provided by
> +# Buildroot). Forcing Gold will break with toolchains that don't
> +# provide it, so override the flag and use BFD.
> +# See: https://github.com/golang/go/issues/22040
> +CONTAINERD_EXTLDFLAGS += -fuse-ld=bfd
> +ifeq ($(BR2_TOOLCHAIN_USES_MUSL),y)
> +# Go exe build with PIE doesn't work with musl.
> +# See: https://github.com/golang/go/issues/17847
> +CONTAINERD_EXTLDFLAGS += -Wl,--no-pie
> +endif
> +endif
> +
>  define CONTAINERD_INSTALL_INIT_SYSTEMD
>         $(INSTALL) -D -m 0644 $(@D)/containerd.service \
>                 $(TARGET_DIR)/usr/lib/systemd/system/containerd.service
> --
> 2.47.2
>


This looks good to me. But shouldn't we apply this more generally to go-src
and go-bin? It's true for all Go packages that we should pass these flags
right? Why specifically containerd?

Thanks!
Christian Stewart

>
>

[-- Attachment #1.2: Type: text/html, Size: 4540 bytes --]

[-- Attachment #2: Type: text/plain, Size: 150 bytes --]

_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH 1/1] package/containerd: fix build with toolchain without Gold linker

[Arnout Vandecappelle via buildroot]

On 04/02/2025 22:23, Christian Stewart wrote:
> Hi Fiona, Yann, all,
> 
> 
> 
> On Tue, Feb 4, 2025, 6:32 AM Fiona Klute <fiona.klute@gmx.de 
> <mailto:fiona.klute@gmx.de>> wrote:
> 
>     From: "Fiona Klute (WIWA)" <fiona.klute@gmx.de <mailto:fiona.klute@gmx.de>>
> 
>     Go forces use of the Gold linker on aarch64 due to a bug in BFD (the
>     default GNU linker) that is fixed in Binutils >= 2.41 [1]. All
>     toolchains provided by Buildroot have the fix, and none of them
>     provide Gold, so the forced setting breaks dynamic linking in
>     Buildroot. Set EXTLDFLAGS to use BFD instead.
> 
>     Additionally disable PIE on musl to fix "read-only segment has dynamic
>     relocations" errors during linking [2].
> 
>     [1] https://github.com/golang/go/issues/22040 <https://github.com/golang/go/
>     issues/22040>
>     [2] https://github.com/golang/go/issues/17847 <https://github.com/golang/go/
>     issues/17847>
> 
>     Signed-off-by: Fiona Klute (WIWA) <fiona.klute@gmx.de
>     <mailto:fiona.klute@gmx.de>>
>     ---
>     This patch requires Yann E. Morin's patch that introduces
>     PACKAGE_EXTLDFLAGS:
>     https://patchwork.ozlabs.org/project/buildroot/
>     patch/876f3a7bb6a2375193fc8f06ab856d2449f83727.1699547993.git.yann.morin@orange.com/ <https://patchwork.ozlabs.org/project/buildroot/patch/876f3a7bb6a2375193fc8f06ab856d2449f83727.1699547993.git.yann.morin@orange.com/>
> 
>     Based on the linked issue the --no-pie linker flag should be needed
>     only for musl, but I'd be grateful if anyone could test with glibc.
> 
>       package/containerd/containerd.mk <http://containerd.mk> | 14 ++++++++++++++
>       1 file changed, 14 insertions(+)
> 
>     diff --git a/package/containerd/containerd.mk <http://containerd.mk> b/
>     package/containerd/containerd.mk <http://containerd.mk>
>     index ee07c22495..cb648f87a9 100644
>     --- a/package/containerd/containerd.mk <http://containerd.mk>
>     +++ b/package/containerd/containerd.mk <http://containerd.mk>
>     @@ -49,6 +49,20 @@ ifneq ($(BR2_PACKAGE_CONTAINERD_CRI),y)
>       CONTAINERD_TAGS += no_cri
>       endif
> 
>     +ifeq ($(BR2_aarch64),y)
>     +# Go forces use of the Gold linker on aarch64 due to a bug in BFD that
>     +# is fixed in Binutils >= 2.41 (so all versions provided by
>     +# Buildroot). Forcing Gold will break with toolchains that don't
>     +# provide it, so override the flag and use BFD.
>     +# See: https://github.com/golang/go/issues/22040 <https://github.com/
>     golang/go/issues/22040>
>     +CONTAINERD_EXTLDFLAGS += -fuse-ld=bfd
>     +ifeq ($(BR2_TOOLCHAIN_USES_MUSL),y)
>     +# Go exe build with PIE doesn't work with musl.
>     +# See: https://github.com/golang/go/issues/17847 <https://github.com/
>     golang/go/issues/17847>
>     +CONTAINERD_EXTLDFLAGS += -Wl,--no-pie
>     +endif
>     +endif
>     +
>       define CONTAINERD_INSTALL_INIT_SYSTEMD
>              $(INSTALL) -D -m 0644 $(@D)/containerd.service \
>                      $(TARGET_DIR)/usr/lib/systemd/system/containerd.service
>     -- 
>     2.47.2
> 
> 
> 
> This looks good to me. But shouldn't we apply this more generally to go-src and 
> go-bin? It's true for all Go packages that we should pass these flags right? Why 
> specifically containerd?

  Yes I agree - this patch (and Yann's infrastructure patch that this patch 
depends on) should be in pkg-golang instead of an individual package.

  Regards,
  Arnout

_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH 1/1] package/containerd: fix build with toolchain without Gold linker

[yann.morin]

Arnout, Fiona, Christian, All,

On 2025-02-05 09:03 +0100, Arnout Vandecappelle spake thusly:
> On 04/02/2025 22:23, Christian Stewart wrote:
> > On Tue, Feb 4, 2025, 6:32 AM Fiona Klute <fiona.klute@gmx.de
> > <mailto:fiona.klute@gmx.de>> wrote:
[--SNIP--]
> >     +ifeq ($(BR2_aarch64),y)
> >     +# Go forces use of the Gold linker on aarch64 due to a bug in BFD that
> >     +# is fixed in Binutils >= 2.41 (so all versions provided by
> >     +# Buildroot). Forcing Gold will break with toolchains that don't
> >     +# provide it, so override the flag and use BFD.
> >     +# See: https://github.com/golang/go/issues/22040 <https://github.com/
> >     golang/go/issues/22040>
> >     +CONTAINERD_EXTLDFLAGS += -fuse-ld=bfd
> >     +ifeq ($(BR2_TOOLCHAIN_USES_MUSL),y)
> >     +# Go exe build with PIE doesn't work with musl.
> >     +# See: https://github.com/golang/go/issues/17847 <https://github.com/
> >     golang/go/issues/17847>
> >     +CONTAINERD_EXTLDFLAGS += -Wl,--no-pie
> >     +endif
> >     +endif
[--SNIP--]
> > This looks good to me. But shouldn't we apply this more generally to
> > go-src and go-bin? It's true for all Go packages that we should pass
> > these flags right? Why specifically containerd?
[--SNIP--]
>  Yes I agree - this patch (and Yann's infrastructure patch that this patch
> depends on) should be in pkg-golang instead of an individual package.

Not sure I entirely grok that. The change I submitted is in pkg-golang,
as it is infra.

So, I guess you only actually suggested moving setting -fuse-ld=bfd and
-Wl,--no-pie flags into pkg-golang, right?

When I initially needed -fuse-ld=bfd for filebeat, I did not feel very
confident that we should do it unconditionally. Indeed, none of the
{host-,}golang-package packages we had at that time needed it at all. So
I concluded that filebeat was special (it was already special in a few
other respects, so meh).

And since I am far from a go expert, I have no idea how impactful such a
change would be. So I was a bit skeptical at making it part of the infra
so that it applied to all packages.

Still today, only one package (containerd) actually needs it; it is
still an exception. Thus _I_ am still a little bit skeptical that this
should be moved into the infra.

But if Christian, our eresident go expert, says we should go for it,
then let's do it.

Still, I believe there should be a way for packages to be able to pass
arbitrary extldflags, and thus the variable should be exposed.

Regards,
Yann E. MORIN.

-- 
                                        ____________
.-----------------.--------------------:       _    :------------------.
|  Yann E. MORIN  | Real-Time Embedded |    __/ )   | /"\ ASCII RIBBON |
|                 | Software  Designer |  _/ - /'   | \ / CAMPAIGN     |
| +33 638.411.245 '--------------------: (_    `--, |  X  AGAINST      |
| yann.morin (at) orange.com           |_="    ,--' | / \ HTML MAIL    |
'--------------------------------------:______/_____:------------------'

____________________________________________________________________________________________________________
Ce message et ses pieces jointes peuvent contenir des informations confidentielles ou privilegiees et ne doivent donc
pas etre diffuses, exploites ou copies sans autorisation. Si vous avez recu ce message par erreur, veuillez le signaler
a l'expediteur et le detruire ainsi que les pieces jointes. Les messages electroniques etant susceptibles d'alteration,
Orange decline toute responsabilite si ce message a ete altere, deforme ou falsifie. Merci.

This message and its attachments may contain confidential or privileged information that may be protected by law;
they should not be distributed, used or copied without authorisation.
If you have received this email in error, please notify the sender and delete this message and its attachments.
As emails may be altered, Orange is not liable for messages that have been modified, changed or falsified.
Thank you.
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH 1/1] package/containerd: fix build with toolchain without Gold linker

[Christian Stewart via buildroot]

[-- Attachment #1.1: Type: text/plain, Size: 1839 bytes --]

Hi Yann,

On Wed, Feb 5, 2025, 12:29 AM <yann.morin@orange.com> wrote:

> >  Yes I agree - this patch (and Yann's infrastructure patch that this
> patch
> > depends on) should be in pkg-golang instead of an individual package.
>
> Not sure I entirely grok that. The change I submitted is in pkg-golang,
> as it is infra.
>
> So, I guess you only actually suggested moving setting -fuse-ld=bfd and
> -Wl,--no-pie flags into pkg-golang, right?


My thinking is that we always want Go to have fuse-ld set to the correct
"ld" we are using. So it makes sense to set this in pkg-golang.

This way any new packages that have dynamic linking (cgo) will also pick up
the fix as needed.

When I initially needed -fuse-ld=bfd for filebeat, I did not feel very
> confident that we should do it unconditionally. Indeed, none of the
> {host-,}golang-package packages we had at that time needed it at all. So
> I concluded that filebeat was special (it was already special in a few
> other respects, so meh).
>

It is just because it uses cgo, or am I wrong?

And since I am far from a go expert, I have no idea how impactful such a
> change would be. So I was a bit skeptical at making it part of the infra
> so that it applied to all packages.
>


> Still today, only one package (containerd) actually needs it; it is
> still an exception. Thus _I_ am still a little bit skeptical that this
> should be moved into the infra.
>
> But if Christian, our eresident go expert, says we should go for it,
> then let's do it.
>
> Still, I believe there should be a way for packages to be able to pass
> arbitrary extldflags, and thus the variable should be exposed.
>

Yes, that variable should be exposed, I agree. It's useful to be able to
pass extra ld flags as needed.

Best regards,
Christian Stewart

[-- Attachment #1.2: Type: text/html, Size: 3106 bytes --]

[-- Attachment #2: Type: text/plain, Size: 150 bytes --]

_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH 1/1] package/containerd: fix build with toolchain without Gold linker

[yann.morin]

Christian, All,

On 2025-02-05 01:20 -0800, Christian Stewart spake thusly:
> On Wed, Feb 5, 2025, 12:29 AM < [1]yann.morin@orange.com> wrote:
>   So, I guess you only actually suggested moving setting -fuse-ld=bfd and
>   -Wl,--no-pie flags into pkg-golang, right?
> My thinking is that we always want Go to have fuse-ld set to the correct "ld" we are using. So it makes sense to set this in
> pkg-golang.
> This way any new packages that have dynamic linking (cgo) will also pick up the fix as needed.

OK, that is for -fuse-ld=bfd. That makes sense.

What about -Wl,--no-pie, then? Does it also make sense to have in the
infra as well?

>   When I initially needed -fuse-ld=bfd for filebeat, I did not feel very
>   confident that we should do it unconditionally. Indeed, none of the
>   {host-,}golang-package packages we had at that time needed it at all. So
>   I concluded that filebeat was special (it was already special in a few
>   other respects, so meh).
> It is just because it uses cgo, or am I wrong?

TBH, I just banged on it until it build and run. If cgo is the reason we
need to pass -fuse-ld=bfd, then that should be part of the condition,
no? i.e., something like (in pkg-golang.mk):

    ifeq ($$(BR2_PACKAGE_HOST_GO_TARGET_CGO_LINKING_SUPPORTS),y)
    ifeq ($$(BR2_aarch64),y)
    $(2)_EXTLDFLAGS += -fuse-ld=bfd
    endif  # AArch64
    endif  # CGO linking

Or can we just pass it unconditionally, even if it is actually not going
to be used? I.e. do we need any condition at all, evem the AArch64 one?

If we want to introduce that assignment in the infra, we need to get as
much insights as possible to explain it, and we need to get affirmative
statements as to how we should set it.

I guess my patch adding FOO_EXTLDFLAGS would not change, but Fiona will
need that info if she is to respin a fix for containerd's build failure.

[--SNIP--]
>   Still, I believe there should be a way for packages to be able to pass
>   arbitrary extldflags, and thus the variable should be exposed.
> Yes, that variable should be exposed, I agree. It's useful to be able to pass extra ld flags as needed.

ACK, thanks for the feeback!

Regards,
Yann E. MORIN.

-- 
                                        ____________
.-----------------.--------------------:       _    :------------------.
|  Yann E. MORIN  | Real-Time Embedded |    __/ )   | /"\ ASCII RIBBON |
|                 | Software  Designer |  _/ - /'   | \ / CAMPAIGN     |
| +33 638.411.245 '--------------------: (_    `--, |  X  AGAINST      |
| yann.morin (at) orange.com           |_="    ,--' | / \ HTML MAIL    |
'--------------------------------------:______/_____:------------------'

____________________________________________________________________________________________________________
Ce message et ses pieces jointes peuvent contenir des informations confidentielles ou privilegiees et ne doivent donc
pas etre diffuses, exploites ou copies sans autorisation. Si vous avez recu ce message par erreur, veuillez le signaler
a l'expediteur et le detruire ainsi que les pieces jointes. Les messages electroniques etant susceptibles d'alteration,
Orange decline toute responsabilite si ce message a ete altere, deforme ou falsifie. Merci.

This message and its attachments may contain confidential or privileged information that may be protected by law;
they should not be distributed, used or copied without authorisation.
If you have received this email in error, please notify the sender and delete this message and its attachments.
As emails may be altered, Orange is not liable for messages that have been modified, changed or falsified.
Thank you.
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH 1/1] package/containerd: fix build with toolchain without Gold linker

[Arnout Vandecappelle via buildroot]

On 05/02/2025 10:20, Christian Stewart wrote:
> Hi Yann,
> 
> On Wed, Feb 5, 2025, 12:29 AM <yann.morin@orange.com 
> <mailto:yann.morin@orange.com>> wrote:
> 
>      >  Yes I agree - this patch (and Yann's infrastructure patch that this patch
>      > depends on) should be in pkg-golang instead of an individual package.
> 
>     Not sure I entirely grok that. The change I submitted is in pkg-golang,
>     as it is infra.

  Yes, sorry, I was talking about the part in filebeat (and I forgot that 
filebeat was a new package).


>     So, I guess you only actually suggested moving setting -fuse-ld=bfd and
>     -Wl,--no-pie flags into pkg-golang, right?
> 
> 
> My thinking is that we always want Go to have fuse-ld set to the correct "ld" we 
> are using. So it makes sense to set this in pkg-golang.
> 
> This way any new packages that have dynamic linking (cgo) will also pick up the 
> fix as needed.

  AFAIU it would be needed for any combination of
- CGO package;
- arch where Go decides it needs gold;
- toolchain that doesn't have gold.

So the only package-specific aspect is indeed that it's CGO.

  We don't have a way at the moment for a package to indicate that it's using 
CGO (there's just a Config.in depends line). But I think the extld option only 
applies to CGO builds anyway.


>     When I initially needed -fuse-ld=bfd for filebeat, I did not feel very
>     confident that we should do it unconditionally. Indeed, none of the
>     {host-,}golang-package packages we had at that time needed it at all. So
>     I concluded that filebeat was special (it was already special in a few
>     other respects, so meh).
> 
> 
> It is just because it uses cgo, or am I wrong?
> 
>     And since I am far from a go expert, I have no idea how impactful such a
>     change would be. So I was a bit skeptical at making it part of the infra
>     so that it applied to all packages.
> 
> 
> 
>     Still today, only one package (containerd) actually needs it; it is
>     still an exception. Thus _I_ am still a little bit skeptical that this
>     should be moved into the infra.

  AFAIU, the problem (that Go wants to use gold even if it is not available) 
really should exist for all packages, there is nothing package-specific about it.

  There is also the potential problem that ld.bfd doesn't actually work, but we 
don't have a solution for that and no easy way to detect at Config.in level when 
it would exist.

  Then there is the additional PIE issue that Fiona addresses with this patch - 
I have no idea if that one is package-specific or not. I _think_ the autobuild 
failure on cloudflared [1] is the same issue, but I'm not sure. It does feel 
like that one is pretty much package-independent as well though.


> 
>     But if Christian, our eresident go expert, says we should go for it,
>     then let's do it.
> 
>     Still, I believe there should be a way for packages to be able to pass
>     arbitrary extldflags, and thus the variable should be exposed.
> 
> 
> Yes, that variable should be exposed, I agree. It's useful to be able to pass 
> extra ld flags as needed.

  I also agree that the variable should be exposed. Even though I don't think it 
should be used by any package for the time being.

  But I think the extld and PIE options should be appended to it from the infra.

  Regards,
  Arnout


[1] 
http://autobuild.buildroot.net/results/694/6941f9ebbaaf4d294d66d32c06a4cd5237b8a4e8/build-end.log


_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH 1/1] package/containerd: fix build with toolchain without Gold linker

[Fiona Klute via buildroot]

Hi everyone!

Am 05.02.25 um 10:42 schrieb Arnout Vandecappelle:
>
>
> On 05/02/2025 10:20, Christian Stewart wrote:
>> Hi Yann,
>>
>> On Wed, Feb 5, 2025, 12:29 AM <yann.morin@orange.com
>> <mailto:yann.morin@orange.com>> wrote:
>>
>>      >  Yes I agree - this patch (and Yann's infrastructure patch that
>> this patch
>>      > depends on) should be in pkg-golang instead of an individual
>> package.
>>
>>     Not sure I entirely grok that. The change I submitted is in pkg-
>> golang,
>>     as it is infra.
>
>   Yes, sorry, I was talking about the part in filebeat (and I forgot
> that filebeat was a new package).
>
>
>>     So, I guess you only actually suggested moving setting -fuse-
>> ld=bfd and
>>     -Wl,--no-pie flags into pkg-golang, right?
>>
>>
>> My thinking is that we always want Go to have fuse-ld set to the
>> correct "ld" we are using. So it makes sense to set this in pkg-golang.
>>
>> This way any new packages that have dynamic linking (cgo) will also
>> pick up the fix as needed.
>
>   AFAIU it would be needed for any combination of
> - CGO package;
> - arch where Go decides it needs gold;
> - toolchain that doesn't have gold.
>
> So the only package-specific aspect is indeed that it's CGO.
>
>   We don't have a way at the moment for a package to indicate that it's
> using CGO (there's just a Config.in depends line). But I think the extld
> option only applies to CGO builds anyway.
>
>
>>     When I initially needed -fuse-ld=bfd for filebeat, I did not feel
>> very
>>     confident that we should do it unconditionally. Indeed, none of the
>>     {host-,}golang-package packages we had at that time needed it at
>> all. So
>>     I concluded that filebeat was special (it was already special in a
>> few
>>     other respects, so meh).
>>
>>
>> It is just because it uses cgo, or am I wrong?
>>
>>     And since I am far from a go expert, I have no idea how impactful
>> such a
>>     change would be. So I was a bit skeptical at making it part of the
>> infra
>>     so that it applied to all packages.
>>
>>
>>
>>     Still today, only one package (containerd) actually needs it; it is
>>     still an exception. Thus _I_ am still a little bit skeptical that
>> this
>>     should be moved into the infra.
>
>   AFAIU, the problem (that Go wants to use gold even if it is not
> available) really should exist for all packages, there is nothing
> package-specific about it.
>
>   There is also the potential problem that ld.bfd doesn't actually work,
> but we don't have a solution for that and no easy way to detect at
> Config.in level when it would exist.
>
>   Then there is the additional PIE issue that Fiona addresses with this
> patch - I have no idea if that one is package-specific or not. I _think_
> the autobuild failure on cloudflared [1] is the same issue, but I'm not
> sure. It does feel like that one is pretty much package-independent as
> well though.

That one looks different: The build is for 32 bit arm, not aarch64, Go
doesn't force Gold there. And the check if PIE is supported [1] seems to
only break with the added --fuse-ld=bfd flag. Unless it is independently
broken on arm to begin with.

>>     But if Christian, our eresident go expert, says we should go for it,
>>     then let's do it.
>>
>>     Still, I believe there should be a way for packages to be able to
>> pass
>>     arbitrary extldflags, and thus the variable should be exposed.
>>
>>
>> Yes, that variable should be exposed, I agree. It's useful to be able
>> to pass extra ld flags as needed.
>
>   I also agree that the variable should be exposed. Even though I don't
> think it should be used by any package for the time being.
>
>   But I think the extld and PIE options should be appended to it from
> the infra.
Okay, I'll look at adjusting the patch for that. Does everyone agree the
override should be limited to aarch64, plus musl for the --no-pie part?
I think it makes sense to keep the override as narrow as possible to
avoid unintended side effects.

Best regards,
Fiona

[1]
https://github.com/golang/go/blob/842e4b5207003db692d72a1aeba4f164bbeb1c13/src/cmd/link/internal/ld/lib.go#L1879-L1894

_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH 1/1] package/containerd: fix build with toolchain without Gold linker

[Arnout Vandecappelle via buildroot]

  Hi Fiona,

On 04/02/2025 15:32, Fiona Klute wrote:
> From: "Fiona Klute (WIWA)" <fiona.klute@gmx.de>
> 
> Go forces use of the Gold linker on aarch64 due to a bug in BFD (the
> default GNU linker) that is fixed in Binutils >= 2.41 [1]. All
> toolchains provided by Buildroot have the fix, and none of them
> provide Gold, so the forced setting breaks dynamic linking in
> Buildroot. Set EXTLDFLAGS to use BFD instead.
> 
> Additionally disable PIE on musl to fix "read-only segment has dynamic
> relocations" errors during linking [2].
> 
> [1] https://github.com/golang/go/issues/22040
> [2] https://github.com/golang/go/issues/17847
> 
> Signed-off-by: Fiona Klute (WIWA) <fiona.klute@gmx.de>
> ---
> This patch requires Yann E. Morin's patch that introduces
> PACKAGE_EXTLDFLAGS:
> https://patchwork.ozlabs.org/project/buildroot/patch/876f3a7bb6a2375193fc8f06ab856d2449f83727.1699547993.git.yann.morin@orange.com/

  After all this discussion, could you respin this as:

- a series where Yann's patch is 1/2. It's quite confusing for us maintainers to 
have a patch that depends on another patch, unless when it's part of a series. 
You can just apply that patch on your branch, add your own signed-off-by, and 
include it in your own series;
- add the use-ld=bfd as part of the golang infrastructure;
- I'm not sure about the PIE part yet, but you can try building any other CGO 
package with a musl toolchain and if it fails as well it needs to be in the infra.


  Regards,
  Arnout


> Based on the linked issue the --no-pie linker flag should be needed
> only for musl, but I'd be grateful if anyone could test with glibc.
> 
>   package/containerd/containerd.mk | 14 ++++++++++++++
>   1 file changed, 14 insertions(+)
> 
> diff --git a/package/containerd/containerd.mk b/package/containerd/containerd.mk
> index ee07c22495..cb648f87a9 100644
> --- a/package/containerd/containerd.mk
> +++ b/package/containerd/containerd.mk
> @@ -49,6 +49,20 @@ ifneq ($(BR2_PACKAGE_CONTAINERD_CRI),y)
>   CONTAINERD_TAGS += no_cri
>   endif
>   
> +ifeq ($(BR2_aarch64),y)
> +# Go forces use of the Gold linker on aarch64 due to a bug in BFD that
> +# is fixed in Binutils >= 2.41 (so all versions provided by
> +# Buildroot). Forcing Gold will break with toolchains that don't
> +# provide it, so override the flag and use BFD.
> +# See: https://github.com/golang/go/issues/22040
> +CONTAINERD_EXTLDFLAGS += -fuse-ld=bfd
> +ifeq ($(BR2_TOOLCHAIN_USES_MUSL),y)
> +# Go exe build with PIE doesn't work with musl.
> +# See: https://github.com/golang/go/issues/17847
> +CONTAINERD_EXTLDFLAGS += -Wl,--no-pie
> +endif
> +endif
> +
>   define CONTAINERD_INSTALL_INIT_SYSTEMD
>   	$(INSTALL) -D -m 0644 $(@D)/containerd.service \
>   		$(TARGET_DIR)/usr/lib/systemd/system/containerd.service

_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH 1/1] package/containerd: fix build with toolchain without Gold linker

[Fiona Klute via buildroot]

Hi Arnout!

Am 05.02.25 um 11:33 schrieb Arnout Vandecappelle:
>   Hi Fiona,
>
> On 04/02/2025 15:32, Fiona Klute wrote:
>> From: "Fiona Klute (WIWA)" <fiona.klute@gmx.de>
>>
>> Go forces use of the Gold linker on aarch64 due to a bug in BFD (the
>> default GNU linker) that is fixed in Binutils >= 2.41 [1]. All
>> toolchains provided by Buildroot have the fix, and none of them
>> provide Gold, so the forced setting breaks dynamic linking in
>> Buildroot. Set EXTLDFLAGS to use BFD instead.
>>
>> Additionally disable PIE on musl to fix "read-only segment has dynamic
>> relocations" errors during linking [2].
>>
>> [1] https://github.com/golang/go/issues/22040
>> [2] https://github.com/golang/go/issues/17847
>>
>> Signed-off-by: Fiona Klute (WIWA) <fiona.klute@gmx.de>
>> ---
>> This patch requires Yann E. Morin's patch that introduces
>> PACKAGE_EXTLDFLAGS:
>> https://patchwork.ozlabs.org/project/buildroot/
>> patch/876f3a7bb6a2375193fc8f06ab856d2449f83727.1699547993.git.yann.morin@orange.com/
>
>   After all this discussion, could you respin this as:
>
> - a series where Yann's patch is 1/2. It's quite confusing for us
> maintainers to have a patch that depends on another patch, unless when
> it's part of a series. You can just apply that patch on your branch, add
> your own signed-off-by, and include it in your own series;
> - add the use-ld=bfd as part of the golang infrastructure;
> - I'm not sure about the PIE part yet, but you can try building any
> other CGO package with a musl toolchain and if it fails as well it needs
> to be in the infra.

Thanks, I've just sent the updated series:
https://patchwork.ozlabs.org/project/buildroot/list/?series=442939

I've tested building docker-engine, docker-cli, and runc, and they build
fine without the --no-pie override, so I've kept that to containerd only.

Best regards,
Fiona

_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot