| Message ID | 0907a7f51a6681bf626ab27febd4a32ddd7e617d.1723208902.git.yann.morin@orange.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | package/Makefile.in: work around buggy filesystem wrt LDFLAGS | expand |
All, On 2024-08-09 15:08 +0200, yann.morin@orange.com spake thusly: > From: "Yann E. MORIN" <yann.morin@orange.com> The commit title mentions "filesystems"; that's of course incorrect, and should mention "buildsystems". Sorry for the confusion. Regards, Yann E. MORIN. > 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 >
On Fri, Aug 09, 2024 at 03:08:22PM +0200, yann.morin@orange.com wrote: > 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): s/mines/mine/ perhaps (although Wiktionary documents[1] it as a variant) [1]: https://en.wiktionary.org/wiki/mines#Etymology_2 > > 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> > --- And wow! What a trip... Thank you for investigating this. Reviewed-by: 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
On Fri, 9 Aug 2024 15:08:22 +0200 <yann.morin@orange.com> wrote: > 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(-) Applied to master, thanks. Thomas
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.