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

[Tony Ambardar <tony.ambardar@gmail.com>]

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@gmail.com> wrote:
> >
> > From: Tony Ambardar <tony.ambardar@gmail.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@gmail.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.

Out of curiosity, I saw that tools/build includes feature-detection code
(incl backtrace) and wondered if selftests/bpf ever used this facility?
> 
> > +
> >  static bool verbose(void)
> >  {
> >         return env.verbosity > VERBOSE_NONE;
> > --
> > 2.34.1
> >