| Message ID | 472c94bd42cda20154a26ef384b73488abf026c0.1721903630.git.tony.ambardar@gmail.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | selftests/bpf: Improve libc portability / musl support (part 2) | expand |
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 > + > static bool verbose(void) > { > return env.verbosity > VERBOSE_NONE; > -- > 2.34.1 >
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 > >
On Fri, Jul 26, 2024 at 8:48 PM Tony Ambardar <tony.ambardar@gmail.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@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. > 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 > > >
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 + static bool verbose(void) { return env.verbosity > VERBOSE_NONE;