| Message ID | 20220513004117.364577-1-yosryahmed@google.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | BPF |
| Headers | show |
| Series | [bpf-next] selftests/bpf: fix building bpf selftests statically | expand |
On Thu, May 12, 2022 at 5:41 PM Yosry Ahmed <yosryahmed@google.com> wrote: > > bpf selftests can no longer be built with CFLAGS=-static with > liburandom_read.so and its dependent target. > > Filter out -static for liburandom_read.so and its dependent target. > > Signed-off-by: Yosry Ahmed <yosryahmed@google.com> > --- > tools/testing/selftests/bpf/Makefile | 9 ++++++--- > 1 file changed, 6 insertions(+), 3 deletions(-) > > diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile > index 6bbc03161544..4eaefc187d5b 100644 > --- a/tools/testing/selftests/bpf/Makefile > +++ b/tools/testing/selftests/bpf/Makefile > @@ -168,14 +168,17 @@ $(OUTPUT)/%:%.c > $(call msg,BINARY,,$@) > $(Q)$(LINK.c) $^ $(LDLIBS) -o $@ > > +# If the tests are being built statically, exclude dynamic libraries defined > +# in this Makefile and their dependencies. > +DYNAMIC_CFLAGS := $(filter-out -static,$(CFLAGS)) I don't particularly like yet another CFLAGS global variable, but also you are not filtering out -static from LDFLAGS, which would be problematic if you try to do make SAN_FLAGS=-static which otherwise would work (and is probably better than overriding all of CFLAGS How about something like this diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile index 6bbc03161544..2e8eddf240af 100644 --- a/tools/testing/selftests/bpf/Makefile +++ b/tools/testing/selftests/bpf/Makefile @@ -170,11 +170,11 @@ $(OUTPUT)/%:%.c $(OUTPUT)/liburandom_read.so: urandom_read_lib1.c urandom_read_lib2.c $(call msg,LIB,,$@) - $(Q)$(CC) $(CFLAGS) -fPIC $(LDFLAGS) $^ $(LDLIBS) --shared -o $@ + $(Q)$(CC) $(filter-out -static,$(CFLAGS) $(LDFLAGS)) $^ $(LDLIBS) -fPIC -shared -o $@ $(OUTPUT)/urandom_read: urandom_read.c urandom_read_aux.c $(OUTPUT)/liburandom_read.so $(call msg,BINARY,,$@) - $(Q)$(CC) $(CFLAGS) $(LDFLAGS) $(filter %.c,$^) \ + $(Q)$(CC) $(filter-out -static,$(CFLAGS) $(LDFLAGS)) $(filter %.c,$^) \ liburandom_read.so $(LDLIBS) \ -Wl,-rpath=. -Wl,--build-id=sha1 -o $@ ? But I also have a question, this leaves urandom_read relying on system-wide shared libraries, isn't that still a problem for you? Or you intend to just ignore urandom_read-related tests? $ ldd urandom_read linux-vdso.so.1 (0x00007ffd0d5e5000) liburandom_read.so (0x00007fc7f7d76000) libelf.so.1 => /lib64/libelf.so.1 (0x00007fc7f7937000) libz.so.1 => /lib64/libz.so.1 (0x00007fc7f7720000) librt.so.1 => /lib64/librt.so.1 (0x00007fc7f7518000) libpthread.so.0 => /lib64/libpthread.so.0 (0x00007fc7f72f8000) libc.so.6 => /lib64/libc.so.6 (0x00007fc7f6f33000) /lib64/ld-linux-x86-64.so.2 (0x00007fc7f7b50000) > $(OUTPUT)/liburandom_read.so: urandom_read_lib1.c urandom_read_lib2.c > $(call msg,LIB,,$@) > - $(Q)$(CC) $(CFLAGS) -fPIC $(LDFLAGS) $^ $(LDLIBS) --shared -o $@ > + $(Q)$(CC) $(DYNAMIC_CFLAGS) -fPIC $(LDFLAGS) $^ $(LDLIBS) --shared -o $@ > > $(OUTPUT)/urandom_read: urandom_read.c urandom_read_aux.c $(OUTPUT)/liburandom_read.so > $(call msg,BINARY,,$@) > - $(Q)$(CC) $(CFLAGS) $(LDFLAGS) $(filter %.c,$^) \ > - liburandom_read.so $(LDLIBS) \ > + $(Q)$(CC) $(DYNAMIC_CFLAGS) $(LDFLAGS) $(filter %.c,$^) \ > + liburandom_read.so $(LDLIBS) \ > -Wl,-rpath=. -Wl,--build-id=sha1 -o $@ > > $(OUTPUT)/bpf_testmod.ko: $(VMLINUX_BTF) $(wildcard bpf_testmod/Makefile bpf_testmod/*.[ch]) > -- > 2.36.0.550.gb090851708-goog >
On Fri, May 13, 2022 at 3:57 PM Andrii Nakryiko <andrii.nakryiko@gmail.com> wrote: > > On Thu, May 12, 2022 at 5:41 PM Yosry Ahmed <yosryahmed@google.com> wrote: > > > > bpf selftests can no longer be built with CFLAGS=-static with > > liburandom_read.so and its dependent target. > > > > Filter out -static for liburandom_read.so and its dependent target. > > > > Signed-off-by: Yosry Ahmed <yosryahmed@google.com> > > --- > > tools/testing/selftests/bpf/Makefile | 9 ++++++--- > > 1 file changed, 6 insertions(+), 3 deletions(-) > > > > diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile > > index 6bbc03161544..4eaefc187d5b 100644 > > --- a/tools/testing/selftests/bpf/Makefile > > +++ b/tools/testing/selftests/bpf/Makefile > > @@ -168,14 +168,17 @@ $(OUTPUT)/%:%.c > > $(call msg,BINARY,,$@) > > $(Q)$(LINK.c) $^ $(LDLIBS) -o $@ > > > > +# If the tests are being built statically, exclude dynamic libraries defined > > +# in this Makefile and their dependencies. > > +DYNAMIC_CFLAGS := $(filter-out -static,$(CFLAGS)) > > I don't particularly like yet another CFLAGS global variable, but also > you are not filtering out -static from LDFLAGS, which would be > problematic if you try to do > > make SAN_FLAGS=-static > > which otherwise would work (and is probably better than overriding all of CFLAGS > > How about something like this Yeah this looks good, thanks for pointing out the problem with LDFLAGS. > > diff --git a/tools/testing/selftests/bpf/Makefile > b/tools/testing/selftests/bpf/Makefile > index 6bbc03161544..2e8eddf240af 100644 > --- a/tools/testing/selftests/bpf/Makefile > +++ b/tools/testing/selftests/bpf/Makefile > @@ -170,11 +170,11 @@ $(OUTPUT)/%:%.c > > $(OUTPUT)/liburandom_read.so: urandom_read_lib1.c urandom_read_lib2.c > $(call msg,LIB,,$@) > - $(Q)$(CC) $(CFLAGS) -fPIC $(LDFLAGS) $^ $(LDLIBS) --shared -o $@ > + $(Q)$(CC) $(filter-out -static,$(CFLAGS) $(LDFLAGS)) $^ > $(LDLIBS) -fPIC -shared -o $@ > > $(OUTPUT)/urandom_read: urandom_read.c urandom_read_aux.c > $(OUTPUT)/liburandom_read.so > $(call msg,BINARY,,$@) > - $(Q)$(CC) $(CFLAGS) $(LDFLAGS) $(filter %.c,$^) \ > + $(Q)$(CC) $(filter-out -static,$(CFLAGS) $(LDFLAGS)) $(filter %.c,$^) \ > liburandom_read.so $(LDLIBS) \ > -Wl,-rpath=. -Wl,--build-id=sha1 -o $@ > > ? > > > But I also have a question, this leaves urandom_read relying on > system-wide shared libraries, isn't that still a problem for you? Or > you intend to just ignore urandom_read-related tests? > I wasn't running those tests, and I thought having most tests compile statically is better than nothing. Maybe this can be fixed by defining a static target for liburandom_read? I am honestly not sure, I have little experience with Makefiles/compilation. > > $ ldd urandom_read > linux-vdso.so.1 (0x00007ffd0d5e5000) > liburandom_read.so (0x00007fc7f7d76000) > libelf.so.1 => /lib64/libelf.so.1 (0x00007fc7f7937000) > libz.so.1 => /lib64/libz.so.1 (0x00007fc7f7720000) > librt.so.1 => /lib64/librt.so.1 (0x00007fc7f7518000) > libpthread.so.0 => /lib64/libpthread.so.0 (0x00007fc7f72f8000) > libc.so.6 => /lib64/libc.so.6 (0x00007fc7f6f33000) > /lib64/ld-linux-x86-64.so.2 (0x00007fc7f7b50000) > > > > > > $(OUTPUT)/liburandom_read.so: urandom_read_lib1.c urandom_read_lib2.c > > $(call msg,LIB,,$@) > > - $(Q)$(CC) $(CFLAGS) -fPIC $(LDFLAGS) $^ $(LDLIBS) --shared -o $@ > > + $(Q)$(CC) $(DYNAMIC_CFLAGS) -fPIC $(LDFLAGS) $^ $(LDLIBS) --shared -o $@ > > > > $(OUTPUT)/urandom_read: urandom_read.c urandom_read_aux.c $(OUTPUT)/liburandom_read.so > > $(call msg,BINARY,,$@) > > - $(Q)$(CC) $(CFLAGS) $(LDFLAGS) $(filter %.c,$^) \ > > - liburandom_read.so $(LDLIBS) \ > > + $(Q)$(CC) $(DYNAMIC_CFLAGS) $(LDFLAGS) $(filter %.c,$^) \ > > + liburandom_read.so $(LDLIBS) \ > > -Wl,-rpath=. -Wl,--build-id=sha1 -o $@ > > > > $(OUTPUT)/bpf_testmod.ko: $(VMLINUX_BTF) $(wildcard bpf_testmod/Makefile bpf_testmod/*.[ch]) > > -- > > 2.36.0.550.gb090851708-goog > >
On Fri, May 13, 2022 at 4:05 PM Yosry Ahmed <yosryahmed@google.com> wrote: > > On Fri, May 13, 2022 at 3:57 PM Andrii Nakryiko > <andrii.nakryiko@gmail.com> wrote: > > > > On Thu, May 12, 2022 at 5:41 PM Yosry Ahmed <yosryahmed@google.com> wrote: > > > > > > bpf selftests can no longer be built with CFLAGS=-static with > > > liburandom_read.so and its dependent target. > > > > > > Filter out -static for liburandom_read.so and its dependent target. > > > > > > Signed-off-by: Yosry Ahmed <yosryahmed@google.com> > > > --- > > > tools/testing/selftests/bpf/Makefile | 9 ++++++--- > > > 1 file changed, 6 insertions(+), 3 deletions(-) > > > > > > diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile > > > index 6bbc03161544..4eaefc187d5b 100644 > > > --- a/tools/testing/selftests/bpf/Makefile > > > +++ b/tools/testing/selftests/bpf/Makefile > > > @@ -168,14 +168,17 @@ $(OUTPUT)/%:%.c > > > $(call msg,BINARY,,$@) > > > $(Q)$(LINK.c) $^ $(LDLIBS) -o $@ > > > > > > +# If the tests are being built statically, exclude dynamic libraries defined > > > +# in this Makefile and their dependencies. > > > +DYNAMIC_CFLAGS := $(filter-out -static,$(CFLAGS)) > > > > I don't particularly like yet another CFLAGS global variable, but also > > you are not filtering out -static from LDFLAGS, which would be > > problematic if you try to do > > > > make SAN_FLAGS=-static > > > > which otherwise would work (and is probably better than overriding all of CFLAGS > > > > How about something like this > > Yeah this looks good, thanks for pointing out the problem with LDFLAGS. ok, cool, please incorporate into v2 then > > > > > diff --git a/tools/testing/selftests/bpf/Makefile > > b/tools/testing/selftests/bpf/Makefile > > index 6bbc03161544..2e8eddf240af 100644 > > --- a/tools/testing/selftests/bpf/Makefile > > +++ b/tools/testing/selftests/bpf/Makefile > > @@ -170,11 +170,11 @@ $(OUTPUT)/%:%.c > > > > $(OUTPUT)/liburandom_read.so: urandom_read_lib1.c urandom_read_lib2.c > > $(call msg,LIB,,$@) > > - $(Q)$(CC) $(CFLAGS) -fPIC $(LDFLAGS) $^ $(LDLIBS) --shared -o $@ > > + $(Q)$(CC) $(filter-out -static,$(CFLAGS) $(LDFLAGS)) $^ > > $(LDLIBS) -fPIC -shared -o $@ > > > > $(OUTPUT)/urandom_read: urandom_read.c urandom_read_aux.c > > $(OUTPUT)/liburandom_read.so > > $(call msg,BINARY,,$@) > > - $(Q)$(CC) $(CFLAGS) $(LDFLAGS) $(filter %.c,$^) \ > > + $(Q)$(CC) $(filter-out -static,$(CFLAGS) $(LDFLAGS)) $(filter %.c,$^) \ > > liburandom_read.so $(LDLIBS) \ > > -Wl,-rpath=. -Wl,--build-id=sha1 -o $@ > > > > ? > > > > > > But I also have a question, this leaves urandom_read relying on > > system-wide shared libraries, isn't that still a problem for you? Or > > you intend to just ignore urandom_read-related tests? > > > > I wasn't running those tests, and I thought having most tests compile > statically is better than nothing. Maybe this can be fixed by defining > a static target for liburandom_read? I am honestly not sure, I have > little experience with Makefiles/compilation. usdt selftest explicitly tests working with shared libraries, so we have to keep liburandom_read.so as a shared library. But yes, at least you can skip the test at runtime. > > > > > $ ldd urandom_read > > linux-vdso.so.1 (0x00007ffd0d5e5000) > > liburandom_read.so (0x00007fc7f7d76000) > > libelf.so.1 => /lib64/libelf.so.1 (0x00007fc7f7937000) > > libz.so.1 => /lib64/libz.so.1 (0x00007fc7f7720000) > > librt.so.1 => /lib64/librt.so.1 (0x00007fc7f7518000) > > libpthread.so.0 => /lib64/libpthread.so.0 (0x00007fc7f72f8000) > > libc.so.6 => /lib64/libc.so.6 (0x00007fc7f6f33000) > > /lib64/ld-linux-x86-64.so.2 (0x00007fc7f7b50000) > > > > > > > > > > > $(OUTPUT)/liburandom_read.so: urandom_read_lib1.c urandom_read_lib2.c > > > $(call msg,LIB,,$@) > > > - $(Q)$(CC) $(CFLAGS) -fPIC $(LDFLAGS) $^ $(LDLIBS) --shared -o $@ > > > + $(Q)$(CC) $(DYNAMIC_CFLAGS) -fPIC $(LDFLAGS) $^ $(LDLIBS) --shared -o $@ > > > > > > $(OUTPUT)/urandom_read: urandom_read.c urandom_read_aux.c $(OUTPUT)/liburandom_read.so > > > $(call msg,BINARY,,$@) > > > - $(Q)$(CC) $(CFLAGS) $(LDFLAGS) $(filter %.c,$^) \ > > > - liburandom_read.so $(LDLIBS) \ > > > + $(Q)$(CC) $(DYNAMIC_CFLAGS) $(LDFLAGS) $(filter %.c,$^) \ > > > + liburandom_read.so $(LDLIBS) \ > > > -Wl,-rpath=. -Wl,--build-id=sha1 -o $@ > > > > > > $(OUTPUT)/bpf_testmod.ko: $(VMLINUX_BTF) $(wildcard bpf_testmod/Makefile bpf_testmod/*.[ch]) > > > -- > > > 2.36.0.550.gb090851708-goog > > >
diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile index 6bbc03161544..4eaefc187d5b 100644 --- a/tools/testing/selftests/bpf/Makefile +++ b/tools/testing/selftests/bpf/Makefile @@ -168,14 +168,17 @@ $(OUTPUT)/%:%.c $(call msg,BINARY,,$@) $(Q)$(LINK.c) $^ $(LDLIBS) -o $@ +# If the tests are being built statically, exclude dynamic libraries defined +# in this Makefile and their dependencies. +DYNAMIC_CFLAGS := $(filter-out -static,$(CFLAGS)) $(OUTPUT)/liburandom_read.so: urandom_read_lib1.c urandom_read_lib2.c $(call msg,LIB,,$@) - $(Q)$(CC) $(CFLAGS) -fPIC $(LDFLAGS) $^ $(LDLIBS) --shared -o $@ + $(Q)$(CC) $(DYNAMIC_CFLAGS) -fPIC $(LDFLAGS) $^ $(LDLIBS) --shared -o $@ $(OUTPUT)/urandom_read: urandom_read.c urandom_read_aux.c $(OUTPUT)/liburandom_read.so $(call msg,BINARY,,$@) - $(Q)$(CC) $(CFLAGS) $(LDFLAGS) $(filter %.c,$^) \ - liburandom_read.so $(LDLIBS) \ + $(Q)$(CC) $(DYNAMIC_CFLAGS) $(LDFLAGS) $(filter %.c,$^) \ + liburandom_read.so $(LDLIBS) \ -Wl,-rpath=. -Wl,--build-id=sha1 -o $@ $(OUTPUT)/bpf_testmod.ko: $(VMLINUX_BTF) $(wildcard bpf_testmod/Makefile bpf_testmod/*.[ch])
bpf selftests can no longer be built with CFLAGS=-static with liburandom_read.so and its dependent target. Filter out -static for liburandom_read.so and its dependent target. Signed-off-by: Yosry Ahmed <yosryahmed@google.com> --- tools/testing/selftests/bpf/Makefile | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-)