diff mbox series

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

Message ID 472c94bd42cda20154a26ef384b73488abf026c0.1721903630.git.tony.ambardar@gmail.com (mailing list archive)
State Superseded
Delegated to: BPF
Headers show
Series selftests/bpf: Improve libc portability / musl support (part 2) | expand

Checks

Context Check Description
bpf/vmtest-bpf-next-PR success PR summary
netdev/series_format success Posting correctly formatted
netdev/tree_selection success Clearly marked for bpf-next
netdev/ynl success Generated files up to date; no warnings/errors; no diff in generated;
netdev/fixes_present success Fixes tag not required for -next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 7 this patch: 7
netdev/build_tools success Errors and warnings before: 0 this patch: 0
netdev/cc_maintainers success CCed 16 of 16 maintainers
netdev/build_clang success Errors and warnings before: 7 this patch: 7
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success Fixes tag looks correct
netdev/build_allmodconfig_warn success Errors and warnings before: 7 this patch: 7
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 21 lines checked
netdev/build_clang_rust success No Rust files in patch. Skipping build
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0
bpf/vmtest-bpf-next-VM_Test-0 success Logs for Lint
bpf/vmtest-bpf-next-VM_Test-2 success Logs for Unittests
bpf/vmtest-bpf-next-VM_Test-3 success Logs for Validate matrix.py
bpf/vmtest-bpf-next-VM_Test-5 success Logs for aarch64-gcc / build-release
bpf/vmtest-bpf-next-VM_Test-1 success Logs for ShellCheck
bpf/vmtest-bpf-next-VM_Test-11 success Logs for s390x-gcc / build / build for s390x with gcc
bpf/vmtest-bpf-next-VM_Test-12 success Logs for s390x-gcc / build-release
bpf/vmtest-bpf-next-VM_Test-16 success Logs for s390x-gcc / test (test_verifier, false, 360) / test_verifier on s390x with gcc
bpf/vmtest-bpf-next-VM_Test-17 success Logs for s390x-gcc / veristat
bpf/vmtest-bpf-next-VM_Test-18 success Logs for set-matrix
bpf/vmtest-bpf-next-VM_Test-19 success Logs for x86_64-gcc / build / build for x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-20 success Logs for x86_64-gcc / build-release
bpf/vmtest-bpf-next-VM_Test-4 success Logs for aarch64-gcc / build / build for aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-10 success Logs for aarch64-gcc / veristat
bpf/vmtest-bpf-next-VM_Test-28 success Logs for x86_64-llvm-17 / build / build for x86_64 with llvm-17
bpf/vmtest-bpf-next-VM_Test-29 success Logs for x86_64-llvm-17 / build-release / build for x86_64 with llvm-17-O2
bpf/vmtest-bpf-next-VM_Test-34 success Logs for x86_64-llvm-17 / veristat
bpf/vmtest-bpf-next-VM_Test-35 success Logs for x86_64-llvm-18 / build / build for x86_64 with llvm-18
bpf/vmtest-bpf-next-VM_Test-36 success Logs for x86_64-llvm-18 / build-release / build for x86_64 with llvm-18-O2
bpf/vmtest-bpf-next-VM_Test-42 success Logs for x86_64-llvm-18 / veristat
bpf/vmtest-bpf-next-VM_Test-9 success Logs for aarch64-gcc / test (test_verifier, false, 360) / test_verifier on aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-6 success Logs for aarch64-gcc / test (test_maps, false, 360) / test_maps on aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-30 success Logs for x86_64-llvm-17 / test (test_maps, false, 360) / test_maps on x86_64 with llvm-17
bpf/vmtest-bpf-next-VM_Test-33 success Logs for x86_64-llvm-17 / test (test_verifier, false, 360) / test_verifier on x86_64 with llvm-17
bpf/vmtest-bpf-next-VM_Test-7 success Logs for aarch64-gcc / test (test_progs, false, 360) / test_progs on aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-13 success Logs for s390x-gcc / test (test_maps, false, 360) / test_maps on s390x with gcc
bpf/vmtest-bpf-next-VM_Test-8 success Logs for aarch64-gcc / test (test_progs_no_alu32, false, 360) / test_progs_no_alu32 on aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-21 success Logs for x86_64-gcc / test (test_maps, false, 360) / test_maps on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-26 success Logs for x86_64-gcc / test (test_verifier, false, 360) / test_verifier on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-31 success Logs for x86_64-llvm-17 / test (test_progs, false, 360) / test_progs on x86_64 with llvm-17
bpf/vmtest-bpf-next-VM_Test-32 success Logs for x86_64-llvm-17 / test (test_progs_no_alu32, false, 360) / test_progs_no_alu32 on x86_64 with llvm-17
bpf/vmtest-bpf-next-VM_Test-37 success Logs for x86_64-llvm-18 / test (test_maps, false, 360) / test_maps on x86_64 with llvm-18
bpf/vmtest-bpf-next-VM_Test-41 success Logs for x86_64-llvm-18 / test (test_verifier, false, 360) / test_verifier on x86_64 with llvm-18
bpf/vmtest-bpf-next-VM_Test-22 success Logs for x86_64-gcc / test (test_progs, false, 360) / test_progs on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-23 success Logs for x86_64-gcc / test (test_progs_no_alu32, false, 360) / test_progs_no_alu32 on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-24 success Logs for x86_64-gcc / test (test_progs_no_alu32_parallel, true, 30) / test_progs_no_alu32_parallel on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-25 success Logs for x86_64-gcc / test (test_progs_parallel, true, 30) / test_progs_parallel on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-27 success Logs for x86_64-gcc / veristat / veristat on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-38 success Logs for x86_64-llvm-18 / test (test_progs, false, 360) / test_progs on x86_64 with llvm-18
bpf/vmtest-bpf-next-VM_Test-39 success Logs for x86_64-llvm-18 / test (test_progs_cpuv4, false, 360) / test_progs_cpuv4 on x86_64 with llvm-18
bpf/vmtest-bpf-next-VM_Test-40 success Logs for x86_64-llvm-18 / test (test_progs_no_alu32, false, 360) / test_progs_no_alu32 on x86_64 with llvm-18
bpf/vmtest-bpf-next-VM_Test-14 success Logs for s390x-gcc / test (test_progs, false, 360) / test_progs on s390x with gcc
bpf/vmtest-bpf-next-VM_Test-15 success Logs for s390x-gcc / test (test_progs_no_alu32, false, 360) / test_progs_no_alu32 on s390x with gcc

Commit Message

Tony Ambardar July 25, 2024, 10:35 a.m. UTC 
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(-)

Comments

Andrii Nakryiko July 25, 2024, 8:22 p.m. UTC | #1 
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
>
Tony Ambardar July 27, 2024, 3:48 a.m. UTC | #2 
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
> >
Andrii Nakryiko July 29, 2024, 5:50 p.m. UTC | #3 
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 mbox series

Patch

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;