netdev - Re: [PATCH bpf-next v1 6/8] selftests/bpf: Fix compile if backtrace support missing in libc

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAEf4Bzaj-YZaXco2EAsMhneqinLgukDepeN63Xt8cC9GTbRVxg@mail.gmail.com>
Date: Mon, 29 Jul 2024 10:50:01 -0700
From: Andrii Nakryiko <andrii.nakryiko@...il.com>
To: Tony Ambardar <tony.ambardar@...il.com>
Cc: bpf@...r.kernel.org, linux-kselftest@...r.kernel.org, 
	netdev@...r.kernel.org, Alexei Starovoitov <ast@...nel.org>, 
	Daniel Borkmann <daniel@...earbox.net>, Andrii Nakryiko <andrii@...nel.org>, 
	Martin KaFai Lau <martin.lau@...ux.dev>, Eduard Zingerman <eddyz87@...il.com>, Song Liu <song@...nel.org>, 
	Yonghong Song <yonghong.song@...ux.dev>, John Fastabend <john.fastabend@...il.com>, 
	KP Singh <kpsingh@...nel.org>, Stanislav Fomichev <sdf@...ichev.me>, Hao Luo <haoluo@...gle.com>, 
	Jiri Olsa <jolsa@...nel.org>, Mykola Lysenko <mykolal@...com>, Shuah Khan <shuah@...nel.org>, 
	Björn Töpel <bjorn@...nel.org>, 
	Magnus Karlsson <magnus.karlsson@...el.com>, 
	Maciej Fijalkowski <maciej.fijalkowski@...el.com>, Jonathan Lemon <jonathan.lemon@...il.com>, 
	"David S. Miller" <davem@...emloft.net>, Yan Zhai <yan@...udflare.com>
Subject: Re: [PATCH bpf-next v1 6/8] selftests/bpf: Fix compile if backtrace
 support missing in libc

On Fri, Jul 26, 2024 at 8:48 PM Tony Ambardar <tony.ambardar@...il.com> wrote:
>
> On Thu, Jul 25, 2024 at 01:22:37PM -0700, Andrii Nakryiko wrote:
> > On Thu, Jul 25, 2024 at 3:39 AM Tony Ambardar <tony.ambardar@...il.com> wrote:
> > >
> > > From: Tony Ambardar <tony.ambardar@...il.com>
> > >
> > > Use backtrace functions only with glibc and otherwise provide stubs in
> > > test_progs.c. This avoids compile errors (e.g. with musl libc) like:
> > >
> > >   test_progs.c:13:10: fatal error: execinfo.h: No such file or directory
> > >      13 | #include <execinfo.h> /* backtrace */
> > >         |          ^~~~~~~~~~~~
> > >   test_progs.c: In function 'crash_handler':
> > >   test_progs.c:1034:14: error: implicit declaration of function 'backtrace' [-Werror=implicit-function-declaration]
> > >    1034 |         sz = backtrace(bt, ARRAY_SIZE(bt));
> > >         |              ^~~~~~~~~
> > >   test_progs.c:1045:9: error: implicit declaration of function 'backtrace_symbols_fd' [-Werror=implicit-function-declaration]
> > >    1045 |         backtrace_symbols_fd(bt, sz, STDERR_FILENO);
> > >         |         ^~~~~~~~~~~~~~~~~~~~
> > >
> > > Fixes: 9fb156bb82a3 ("selftests/bpf: Print backtrace on SIGSEGV in test_progs")
> > > Signed-off-by: Tony Ambardar <tony.ambardar@...il.com>
> > > ---
> > >  tools/testing/selftests/bpf/test_progs.c | 9 ++++++++-
> > >  1 file changed, 8 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/tools/testing/selftests/bpf/test_progs.c b/tools/testing/selftests/bpf/test_progs.c
> > > index 60c5ec0f6abf..f6cfc6a8e8f0 100644
> > > --- a/tools/testing/selftests/bpf/test_progs.c
> > > +++ b/tools/testing/selftests/bpf/test_progs.c
> > > @@ -10,7 +10,6 @@
> > >  #include <sched.h>
> > >  #include <signal.h>
> > >  #include <string.h>
> > > -#include <execinfo.h> /* backtrace */
> > >  #include <sys/sysinfo.h> /* get_nprocs */
> > >  #include <netinet/in.h>
> > >  #include <sys/select.h>
> > > @@ -19,6 +18,14 @@
> > >  #include <bpf/btf.h>
> > >  #include "json_writer.h"
> > >
> > > +#ifdef __GLIBC__
> > > +#include <execinfo.h> /* backtrace */
> > > +#else
> > > +#define backtrace(...) (0)
> > > +#define backtrace_symbols_fd(bt, sz, fd) \
> > > +       dprintf(fd, "<backtrace not supported>\n", bt, sz)
> > > +#endif
> >
> > First, let's define backtrace() and backtrace_symbols_fd() as proper
> > functions, not a macro?
> >
> > And second, what if we then make those functions __weak, so they
> > provide default implementations if libc doesn't provide those
> > functions?
> >
> > This parts seems unavoidable, though:
> >
> > #ifdef __GLIBC__
> > #include <execinfo.h>
> > #endif
> >
>
> I agree that would be cleaner, will work on a v2 with this.
>

v2 looks good, thanks

> Out of curiosity, I saw that tools/build includes feature-detection code
> (incl backtrace) and wondered if selftests/bpf ever used this facility?

I don't remember, tbh, it might have at some point in the past.

> >
> > > +
> > >  static bool verbose(void)
> > >  {
> > >         return env.verbosity > VERBOSE_NONE;
> > > --
> > > 2.34.1
> > >