diff mbox series

package/Makefile.in: Use "-z text" instead of "-Wl, -z, text"

Message ID 20240805-z-linker-v1-1-8362e6645f67@gmx.net
State Accepted
Headers show
Series package/Makefile.in: Use "-z text" instead of "-Wl, -z, text" | expand

Commit Message

J. Neuschäfer Aug. 5, 2024, 9:53 p.m. UTC
After the recent addition to -Wl,-z,text to TARGET_LDFLAGS in case of
musl-libc and dynamic linking, it was found that some packages pass
TARGET_LDFLAGS directly to ld, but the -Wl syntax only works with
compiler drivers (gcc/clang). This commit changes the flag to -z text,
which the gcc and clang also understand and pass to the linker.

Reported-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
Signed-off-by: J. Neuschäfer <j.neuschaefer@gmx.net>
---
Note: This patch is inspired by gnu-efi, which passes TARGET_LDFLAGS
directly to ld. This particular case is solved by this patch, but
gnu-efi then fails differently (on ARM), with errors such as:

  /.../per-package/gnu-efi/host/bin/arm-buildroot-linux-musleabi-ld: buildroot/build/gnu-efi-3.0.18/arm/lib/../../lib/print.c:1431:(.text+0x78c): undefined reference to `__aeabi_i2d'
---
 package/Makefile.in | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)


---
base-commit: f9f2ade9bb207d2481b12a325bba7cee5a3e9cbf
change-id: 20240805-z-linker-68e22fcb3388

Best regards,
--
J. Neuschäfer <j.neuschaefer@gmx.net>

Comments

Thomas Petazzoni Aug. 6, 2024, 10:13 a.m. UTC | #1
On Mon, 05 Aug 2024 23:53:56 +0200
J. Neuschäfer via buildroot <buildroot@buildroot.org> wrote:

> After the recent addition to -Wl,-z,text to TARGET_LDFLAGS in case of
> musl-libc and dynamic linking, it was found that some packages pass
> TARGET_LDFLAGS directly to ld, but the -Wl syntax only works with
> compiler drivers (gcc/clang). This commit changes the flag to -z text,
> which the gcc and clang also understand and pass to the linker.
> 
> Reported-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
> Signed-off-by: J. Neuschäfer <j.neuschaefer@gmx.net>
> ---
> Note: This patch is inspired by gnu-efi, which passes TARGET_LDFLAGS
> directly to ld. This particular case is solved by this patch, but
> gnu-efi then fails differently (on ARM), with errors such as:

Applied to master, thanks.

Thomas
Yann E. MORIN Aug. 9, 2024, 10 a.m. UTC | #2
J., All,

On 2024-08-05 23:53 +0200, J. Neuschäfer via buildroot spake thusly:
> After the recent addition to -Wl,-z,text to TARGET_LDFLAGS in case of
> musl-libc and dynamic linking, it was found that some packages pass
> TARGET_LDFLAGS directly to ld, but the -Wl syntax only works with
> compiler drivers (gcc/clang). This commit changes the flag to -z text,
> which the gcc and clang also understand and pass to the linker.
[--SNIP--]
> +# support -Wl,[...]. -z is supported by both gcc and clang, so it probably
> +# won't cause us problems.

I laughed at that last part. ;-)

Yes, it is causing an issue with gpsd:

    $ wget -O defconfig.c0399 http://autobuild.buildroot.org/results/c03/c039989947b960ac6af17c87090366abc26dcb6d/config

    $ cat <<-_EOF_ >tst-gpsd
    #!/usr/bin/env bash
    make distclean
    rm br.log
    make BR2_DEFCONFIG=$(pwd)/defconfig.c0399 defconfig || exit 125
    brmake -j22 gpsd-depends || exit 125
    make gpsd
    _EOF_

    $ git checkout 2024.08-rc1
    $ git bisect start
    $ git bisect bad
    $ git checkout 'master@{2024-07-01}'
    $ git bisect good
    $ git bisect run ./tst-gpsd

    bd035872be9948515ee50ae034bda4a2e044ff22 is the first bad commit
    commit bd035872be9948515ee50ae034bda4a2e044ff22
    Author: J. Neuschäfer <j.neuschaefer@gmx.net>
    Date:   Mon Aug 5 23:53:56 2024 +0200

        package/Makefile.in: Use "-z text" instead of "-Wl, -z, text"

    [--SNIP--]

Of course, I suspect the real culprit is not this actual change; it
probably just triggers a latent issue in the gpsd build process, which
uses scons, so meh...

I'll try to have a look, but if someone else can also look at it, that
would be nice. Thanks!

Regards,
Yann E. MORIN.

>  ifeq ($(BR2_TOOLCHAIN_USES_MUSL):$(BR2_STATIC_LIBS),y:)
> -TARGET_LDFLAGS += -Wl,-z,text
> +TARGET_LDFLAGS += -z text
>  endif
> 
>  # By design, _FORTIFY_SOURCE requires gcc optimization to be enabled.
> 
> ---
> base-commit: f9f2ade9bb207d2481b12a325bba7cee5a3e9cbf
> change-id: 20240805-z-linker-68e22fcb3388
> 
> Best regards,
> --
> J. Neuschäfer <j.neuschaefer@gmx.net>
> 
> _______________________________________________
> buildroot mailing list
> buildroot@buildroot.org
> https://lists.buildroot.org/mailman/listinfo/buildroot
Yann E. MORIN Aug. 9, 2024, 12:52 p.m. UTC | #3
J., All,

On 2024-08-09 12:00 +0200, MORIN Yann INNOV/IT-S spake thusly:
> On 2024-08-05 23:53 +0200, J. Neuschäfer via buildroot spake thusly:
> > After the recent addition to -Wl,-z,text to TARGET_LDFLAGS in case of
> > musl-libc and dynamic linking, it was found that some packages pass
> > TARGET_LDFLAGS directly to ld, but the -Wl syntax only works with
> > compiler drivers (gcc/clang). This commit changes the flag to -z text,
> > which the gcc and clang also understand and pass to the linker.
[--SNIP--]
> Yes, it is causing an issue with gpsd:
[--SNIP--]
> Of course, I suspect the real culprit is not this actual change; it
> probably just triggers a latent issue in the gpsd build process, which
> uses scons, so meh...
> I'll try to have a look, but if someone else can also look at it, that
> would be nice. Thanks!

I now went to the bottom of it and found the real culprit (scons, as I
had thought). I'm a bit in a rabbit hole now, but I hope that I can soon
send a patch (or maybe a patchset...) to fix that...

Regards,
Yann E. MORIN.
diff mbox series

Patch

diff --git a/package/Makefile.in b/package/Makefile.in
index 1cedfefa7e..43a5c279c0 100644
--- a/package/Makefile.in
+++ b/package/Makefile.in
@@ -154,8 +154,13 @@  TARGET_LDFLAGS = $(call qstrip,$(BR2_TARGET_LDFLAGS))
 # when DT_TEXREL is used, so we capture the problem earlier.
 #
 # See also: https://www.openwall.com/lists/musl/2020/09/25/4
+#
+# NOTE: We're using "-z text" instead of "-Wl,-z,text" here, because some
+# packages pass TARGET_LDFLAGS directly to ld rather than gcc, and ld doesn't
+# support -Wl,[...]. -z is supported by both gcc and clang, so it probably
+# won't cause us problems.
 ifeq ($(BR2_TOOLCHAIN_USES_MUSL):$(BR2_STATIC_LIBS),y:)
-TARGET_LDFLAGS += -Wl,-z,text
+TARGET_LDFLAGS += -z text
 endif

 # By design, _FORTIFY_SOURCE requires gcc optimization to be enabled.