search

troglobit / sysklogd

BSD syslog daemon with syslog()/syslogp() API replacement for Linux, RFC3164 + RFC5424
https://troglobit.com/sysklogd.html
Other
93 stars 20 forks source link

Static build failure of logger tool with gcc 11 #54

Closed ffontaine closed 1 year ago

ffontaine commented 2 years ago

The following static build failure is raised when building logger tool with gcc 11 because openlog, vsyslog, syslog, closelog and setlogmask are already defined in libc:

/home/autobuild/autobuild/instance-9/output-1/per-package/sysklogd/host/bin/../lib/gcc/m68k-buildroot-linux-uclibc/11.3.0/../../../../m68k-buildroot-linux-uclibc/bin/ld: /home/autobuild/autobuild/instance-9/output-1/per-package/sysklogd/host/m68k-buildroot-linux-uclibc/sysroot/usr/lib/libc.a(syslog.os): in function `openlog':
buildroot/build/uclibc-1.0.41/libc/misc/syslog/syslog.c:169: multiple definition of `openlog'; ./.libs/libsyslog.a(libsyslog_la-syslog.o):buildroot/build/sysklogd-2.4.0/src/syslog.c:141: first defined here
/home/autobuild/autobuild/instance-9/output-1/per-package/sysklogd/host/bin/../lib/gcc/m68k-buildroot-linux-uclibc/11.3.0/../../../../m68k-buildroot-linux-uclibc/bin/ld: /home/autobuild/autobuild/instance-9/output-1/per-package/sysklogd/host/m68k-buildroot-linux-uclibc/sysroot/usr/lib/libc.a(syslog.os): in function `__vsyslog':
buildroot/build/uclibc-1.0.41/libc/misc/syslog/syslog.c:196: multiple definition of `vsyslog'; ./.libs/libsyslog.a(libsyslog_la-syslog.o):buildroot/build/sysklogd-2.4.0/src/syslog.c:115: first defined here
/home/autobuild/autobuild/instance-9/output-1/per-package/sysklogd/host/bin/../lib/gcc/m68k-buildroot-linux-uclibc/11.3.0/../../../../m68k-buildroot-linux-uclibc/bin/ld: /home/autobuild/autobuild/instance-9/output-1/per-package/sysklogd/host/m68k-buildroot-linux-uclibc/sysroot/usr/lib/libc.a(syslog.os): in function `__GI_syslog':
buildroot/build/uclibc-1.0.41/libc/misc/syslog/syslog.c:329: multiple definition of `syslog'; ./.libs/libsyslog.a(libsyslog_la-syslog.o):buildroot/build/sysklogd-2.4.0/src/syslog.c:105: first defined here
/home/autobuild/autobuild/instance-9/output-1/per-package/sysklogd/host/bin/../lib/gcc/m68k-buildroot-linux-uclibc/11.3.0/../../../../m68k-buildroot-linux-uclibc/bin/ld: /home/autobuild/autobuild/instance-9/output-1/per-package/sysklogd/host/m68k-buildroot-linux-uclibc/sysroot/usr/lib/libc.a(syslog.os): in function `closelog':
buildroot/build/uclibc-1.0.41/libc/misc/syslog/syslog.c:343: multiple definition of `closelog'; ./.libs/libsyslog.a(libsyslog_la-syslog.o):buildroot/build/sysklogd-2.4.0/src/syslog.c:147: first defined here
/home/autobuild/autobuild/instance-9/output-1/per-package/sysklogd/host/bin/../lib/gcc/m68k-buildroot-linux-uclibc/11.3.0/../../../../m68k-buildroot-linux-uclibc/bin/ld: /home/autobuild/autobuild/instance-9/output-1/per-package/sysklogd/host/m68k-buildroot-linux-uclibc/sysroot/usr/lib/libc.a(syslog.os): in function `setlogmask':
buildroot/build/uclibc-1.0.41/libc/misc/syslog/syslog.c:351: multiple definition of `setlogmask'; ./.libs/libsyslog.a(libsyslog_la-syslog.o):buildroot/build/sysklogd-2.4.0/src/syslog.c:154: first defined here

There is several options:

Full build log: http://autobuild.buildroot.org/results/c88/c88b4401f2f367791a6feb296fff8f4178f4812b/build-end.log

troglobit commented 2 years ago

Neither. It should link against the local library with support for RFC5424 that replaces the c library functions.

I'll have a look at it. Thanks for the report, Fabrice! <3

ffontaine commented 2 years ago

Thanks for your quick feedback, I'll let you handle it :+1:

troglobit commented 2 years ago

OK, this one took some time to get to grips with. Took me a good while just to understand how to reproduce the autobuilder result, and it wasn't until I had that nailed down that I realized that it was uClibc. Apparently I need glasses since it was clearly noted in the original report.

Sure enough, this seems to be a problem with uClibc-ng rather than the sysklogd project. It seems uClibc-ng does not declare all its functions as weak, only popular ones like malloc/free. I don't know why this is the case ... maybe it's very uncommon to override syslog() & C:o?

Another thing I realized is that the sysklogd package in Buildroot doesn't install to staging, so other packages cannot link against libsyslog.{a,so}, which might be another reason for this going unnoticed. I'll try to get some time over this week to correct this (in Buildroot).

Anyway, I've tested building -static (and LDFLAGS=-all-static for libtool) against both GLIBC and musl, and the only one that works as it's supposed to is musl (no one is surpised, all hail Rich Felker :tada: :) . On GLIBC we trigger lots of warnings and errors, mainly for pthread_mutex_lock() and pthread_mutex_unlock(). The reason I've not seen these before is:

  1. libtool opts to link dynamically against the c-library unless -all-static is set at build time (not configure time, that breaks configure itself), and
  2. I failed to realize that the configure flags --enable-static and --disable-shared only seem to apply to the generation of libraries in the project, not linking against glibc. So in all my tests previously both glibc (all my systems) and musl (Alpine reference build in virt-manager) have linked dynamically against the C-library, yet on those C-libraries the syslog family of APIs are decleared weak, it seems.

So, not sure how you'd like me to proceed with this one. I honestly think the fix should be done in uClibc-ng, because having exceptions for uClic/uClibc-ng in either sysklogd or Buildroot seem unnecessarily clunky.

ffontaine commented 2 years ago

Patching uclibc-ng is indeed the best long term solution. It would be great if you could send them a patch.

However, in the context of buildroot, we also have to support all uclibc-ng toolchains already deployed and used in the field. So, an other option would be to disable sysklogd with uclibc-ng in buildroot for the time being.

troglobit commented 2 years ago

I'll see what I can do about uClibc-ng. Familiarizing myself with the code base rn.

As the maintainer of sysklogd in Buildroot, disabling it completely seems a bit too radical. In particular since this issue only affects targets that a) have an MMU yet can only link statically, and b) target uClibc-ng. Yes, I'm aware uclibc is the default/preferred C library, but how many targets are really affected by this?

I'd like to propose an intermediate solution instead, to disable the sysklogd logger, since its only value is the ability to do RFC5424 logging from shell scripts. The BusyBox or util-linux logger tools are compatible with sysklogd. So I recommend --without-logger as an alternative to your suggestion. I.e.:

diff --git a/package/sysklogd/Config.in b/package/sysklogd/Config.in
index 3315a6ddc9..ab936afeb7 100644
--- a/package/sysklogd/Config.in
+++ b/package/sysklogd/Config.in
@@ -30,6 +30,7 @@ config BR2_PACKAGE_SYSKLOGD_REMOTE_DELAY

 config BR2_PACKAGE_SYSKLOGD_LOGGER
        bool "logger tool"
+       default n
        help
          Generate log messages from scripts or from the command line.
ffontaine commented 2 years ago

Unfortunately, setting a default value is not really acceptable as it will not fix the autobuilder failures and the end-user will not be warned about this issue. I'd recommend to disable static build of logger tool with uclibc-ng.

diff --git a/package/sysklogd/Config.in b/package/sysklogd/Config.in
index 3315a6ddc9..ab936afeb7 100644
--- a/package/sysklogd/Config.in
+++ b/package/sysklogd/Config.in
@@ -30,6 +30,7 @@ config BR2_PACKAGE_SYSKLOGD_REMOTE_DELAY

 config BR2_PACKAGE_SYSKLOGD_LOGGER
        bool "logger tool"
+       depends on !BR2_STATIC_LIBS || !BR2_TOOLCHAIN_USES_UCLIBC
        help
          Generate log messages from scripts or from the command line.

+comment "logger tool needs a glibc or musl toolchain or a toolchain w/ dynamic library"
+   depends on BR2_STATIC_LIBS && BR2_TOOLCHAIN_USES_UCLIBC
troglobit commented 2 years ago

I see, OK that makes sense.

ffontaine commented 2 years ago

If it's ok, I'll let you send the patch to the buildroot mailing list.

troglobit commented 2 years ago

Finally had some time to look at this again. Almost ready to submit to the mailing list ... but then I ran into a surprising discovery when running /utils/test-pkg on sysklogd. I use the following .config:

BR2_PACKAGE_BUSYBOX_SHOW_OTHERS=y
BR2_PACKAGE_SYSKLOGD=y
BR2_PACKAGE_SYSKLOGD_REMOTE_DELAY=30
BR2_PACKAGE_SYSKLOGD_LOGGER=y

With the depends patch (above) the br-arm-full-static pass is skipped. As can be expected with the patch, but then I ran the test-pkg again, this time without the patch. Lo and behold the build passes also for br-arm-full-static.

So it seems to be more than just a (possible?) limitation in uClibc-ng, but also target dependent. Naturally I'm very hesitant to submit my patch to the mailing list now. Seems wrong to limit the package for static Arm builds when the issue really seems to be limited to m68k?

ffontaine commented 2 years ago

Indeed, there is only one m68k autobuilder failure in June 26th: http://autobuild.buildroot.org/?reason=sysklogd-2.4.0 I would advise to wait a few days before next buildroot release (by Mid August) before sending a patch. We'll see if there is more failures on other architectures.

troglobit commented 2 years ago

OK, sounds good! (I have another patch too, but I'll send that separately instead of as a series, later today.)

troglobit commented 2 years ago

I've seen no further reports or problems with this. Maybe we can close it now?

troglobit commented 1 year ago

Closing due to lack of activity.