[Buildroot] [PATCH] package/Makefile.in: work around buggy filesystem wrt LDFLAGS

* [Buildroot] [PATCH] package/Makefile.in: work around buggy filesystem wrt LDFLAGS
@ 2024-08-09 13:08 yann.morin
  2024-08-09 13:14 ` yann.morin
                   ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: yann.morin @ 2024-08-09 13:08 UTC (permalink / raw)
  To: buildroot; +Cc: yann.morin, J . Neuschäfer

From: "Yann E. MORIN" <yann.morin@orange.com>

Some buildsystems (or their use of it by packages) will cause flags in
LDFLAGS to be re-orderded, or even dropped, causing some incompremsible
mayhem... For example, gpsd [0] has this in its SConscript [1](typoes
not mines, for once):

    591 # scons uses gcc, or clang, to link. Thus LDFLAGS does not serve its
    592 # traditional function of providing arguments to ln. LDFLAGS set in the
    593 # environment before running scons get moved into CCFLAGS by scons.
    594 # LDFLAGS set while running scons get ignored.
    [--SNIP--]
    611 for i in ["ARFLAGS",
    612           "CCFLAGS",
    613           "CFLAGS",
    614           "CPPFLAGS",
    615           "CXXFLAGS",
    616           "LDFLAGS",
    617           "LINKFLAGS",
    618           "SHLINKFLAGS",
    619           ]:
    620     if i in os.environ:
    621         # MergeFlags() puts the options where scons wants them, not
    622         # where you asked them to go.
    623         env.MergeFlags(Split(os.getenv(i)))

So, when LDFLAGS (our TARGET_LDFLAGS) contains "-z text"  (without the
quotes), that gets turned into a command line like (line-splitted for
readability):

    [...]/buildroot/output/host/bin/aarch64_be-buildroot-linux-musl-gcc \
        -o gpsd-3.25/drivers/driver_rtcm2.os \
        -c \
        --sysroot=[...]/buildroot/output/host/aarch64_be-buildroot-linux-musl/sysroot \
        -O3 -g0 \
        -z \
        -fPIC -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE -D_FILE_OFFSET_BITS=64 \
        gpsd-3.25/drivers/driver_rtcm2.c

Notice how there is a lone "-z" without any following keyword.

This then causes a build failure that looks totally unrelated [2]:

    In file included from gpsd-3.25/drivers/../include/gpsd.h:36,
                     from gpsd-3.25/drivers/driver_rtcm2.c:65:
    gpsd-3.25/drivers/../include/os_compat.h:40:8: error: redefinition of ‘struct timespec’
       40 | struct timespec {
          |        ^~~~~~~~
    In file included from [...]/buildroot/output/host/aarch64_be-buildroot-linux-musl/sysroot/usr/include/sys/select.h:16,
                     from gpsd-3.25/drivers/../include/gpsd.h:31:
    [...]/buildroot/output/host/aarch64_be-buildroot-linux-musl/sysroot/usr/include/bits/alltypes.h:237:8: note: originally defined here
      237 | struct timespec { time_t tv_sec; int :8*(sizeof(time_t)-sizeof(long))*(__BYTE_ORDER==4321); long tv_nsec; int :8*(sizeof(time_t)-sizeof(long))*(__BYTE_ORDER!=4321); };
          |        ^~~~~~~~
    gpsd-3.25/drivers/../include/os_compat.h:48:5: error: conflicting types for ‘clock_gettime’; have ‘int(clockid_t,  struct timespec *)’ {aka ‘int(int,  struct timespec *)’}
       48 | int clock_gettime(clockid_t, struct timespec *);
          |     ^~~~~~~~~~~~~
    In file included from gpsd-3.25/drivers/../include/gpsd.h:33:
    [...]/buildroot/output/host/aarch64_be-buildroot-linux-musl/sysroot/usr/include/time.h:104:5: note: previous declaration of ‘clock_gettime’ with type ‘int(clockid_t,  struct timespec *)’ {aka ‘int(int,  struct timespec *)’}
      104 | int clock_gettime (clockid_t, struct timespec *);
          |     ^~~~~~~~~~~~~
    scons: *** [gpsd-3.25/drivers/driver_rtcm2.os] Error 1
    scons: building terminated because of errors.
    make[1]: *** [package/pkg-generic.mk:289: [...]/buildroot/output/build/gpsd-3.25/.stamp_built] Error 2
    make: *** [Makefile:83: _all] Error 2

Although undocumented, neither in gcc not ld (clang unchecked by lack of
a clang toolchain here, and by lack of clang knowledge), -z accepts the
keyword to be snatch-glued onto it, like -zkeyword, rather than be
spearated with a space. So, use that to pass -ztext.

Fixes:
    http://autobuild.buildroot.org/results/c03/c039989947b960ac6af17c87090366abc26dcb6d/
    http://autobuild.buildroot.net/results/bc35d3e7b0e8c59c776652070650af3c749250ee/

[0] Our other scons-based package, mongodb, does not build for another
reason that probably hides the same issue as seen with gpsd.

[1] As explained in gpsd's SConscript (se above), Scons does play tricks
with variables:
    https://scons.org/doc/production/HTML/scons-man.html#f-MergeFlags
    https://scons.org/doc/production/HTML/scons-man.html#f-ParseFlags

Quoting:
    Flag values are translated according to the prefix found, and added
    to the following construction variables:
    [...]
    -Wl,                    LINKFLAGS
    [...]
    -                       CCFLAGS
    [...]
    Any other strings not associated with options are assumed to be the
    names of libraries and added to the $LIBS construction variable.

So in our case, it finds that -z is an unknown option that matches the
'-' prefix, so it is added to CFLAGS, while 'text' is a string on its
own, so added to LIBS, and thus it would try to link with -ltext
(supposedly, because we do not even go that far). Funnily enough, we can
se that "-Wl," is a known option prefix, that is added to LINKFLAGS (to
properly be used with gcc or clang, not ld).

As a consequence, gpsd's buildsystem does drop -ztext from the link
flags, and only passes it to the compile flags, which brings us back to
before we banned textrels in a1a2f498d7ec (package/Makefile.in: ban
textrels on musl toolchains). Fixing gpsd is a task for another,
separate patch...

[2] I spent quite some time to look at recent, time-related changes in
Buildroot, especially due to the infamous time64_t issues... Alas, that
was not related, and only a git-bisect pinpointed the actual issue. Poor
polar bears...

Signed-off-by: Yann E. MORIN <yann.morin@orange.com>
Cc: J. Neuschäfer <j.neuschaefer@gmx.net>
---
 package/Makefile.in | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/package/Makefile.in b/package/Makefile.in
index 43a5c279c0..808b71a93e 100644
--- a/package/Makefile.in
+++ b/package/Makefile.in
@@ -155,12 +155,17 @@ TARGET_LDFLAGS = $(call qstrip,$(BR2_TARGET_LDFLAGS))
 #
 # 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
+# NOTE: We're using "-ztext" 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.
+#
+# We're using "-ztext" instead of "-z text" here, because some buildsystems
+# (like scons, for gpsd) will reorder and/or drop LDFLAGS, causing a lone
+# "-z" to be passed and the "text" keyword to be dropped otherwise. Both
+# gcc and ld supports that, so it probably won't cause us problems.
 ifeq ($(BR2_TOOLCHAIN_USES_MUSL):$(BR2_STATIC_LIBS),y:)
-TARGET_LDFLAGS += -z text
+TARGET_LDFLAGS += -ztext
 endif
 
 # By design, _FORTIFY_SOURCE requires gcc optimization to be enabled.
-- 
2.34.1

____________________________________________________________________________________________________________
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

^ permalink raw reply related	[flat|nested] 4+ messages in thread