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

From: "Fiona Klute (WIWA)" <redacted>

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) <redacted>
---
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

Fiona, All,

On 2025-02-04 15:32 +0100, Fiona Klute spake thusly:

From: "Fiona Klute (WIWA)" <redacted>

[--SNIP--]

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

[--SNIP--]

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

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:

quoted

From: "Fiona Klute (WIWA)" <redacted>

[--SNIP--]

quoted

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

[--SNIP--]

quoted

+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

Am 04.02.25 um 19:36 schrieb Fiona Klute:

Hi Yann!

Am 04.02.25 um 15:47 schrieb yann.morin@orange.com:

quoted

Fiona, All,

On 2025-02-04 15:32 +0100, Fiona Klute spake thusly:

quoted

From: "Fiona Klute (WIWA)" <redacted>

[--SNIP--]

quoted

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

[--SNIP--]

quoted

+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

Hi Fiona, Yann, all,



On Tue, Feb 4, 2025, 6:32 AM Fiona Klute [off-list ref] wrote:

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

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

Arnout, Fiona, Christian, All,

On 2025-02-05 09:03 +0100, Arnout Vandecappelle spake thusly:

On 04/02/2025 22:23, Christian Stewart wrote:

quoted

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

[--SNIP--]

quoted

    +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--]

quoted

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

Hi Yann,

On Wed, Feb 5, 2025, 12:29 AM [off-list ref] wrote:

quoted

 Yes I agree - this patch (and Yann's infrastructure patch that this

patch

quoted

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

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

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

Hi everyone!

Am 05.02.25 um 10:42 schrieb Arnout Vandecappelle:

On 05/02/2025 10:20, Christian Stewart wrote:

quoted

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).

quoted

    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.

quoted

    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.

quoted

    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

 Hi Fiona,

On 04/02/2025 15:32, Fiona Klute wrote:

From: "Fiona Klute (WIWA)" <redacted>

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) <redacted>
---
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

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

Hi Arnout!

Am 05.02.25 um 11:33 schrieb Arnout Vandecappelle:

  Hi Fiona,

On 04/02/2025 15:32, Fiona Klute wrote:

quoted

From: "Fiona Klute (WIWA)" <redacted>

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) <redacted>
---
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