Received-SPF: softfail (google.com: domain of transitioning linux-kernel-owner@vger.kernel.org does not designate 23.128.96.19 as permitted sender) client-ip=23.128.96.19;
MIME-Version: 1.0
References: <20220513004117.364577-1-yosryahmed@google.com>
In-Reply-To: <20220513004117.364577-1-yosryahmed@google.com>
From:   Andrii Nakryiko <andrii.nakryiko@gmail.com>
Date:   Fri, 13 May 2022 15:57:44 -0700
Message-ID: <CAEf4BzYiuBaCBcF6T9JRJ=fB=PWtDb9p-5CJAi164F4_h5fQPw@mail.gmail.com>
Subject: Re: [PATCH bpf-next] selftests/bpf: fix building bpf selftests statically
To:     Yosry Ahmed <yosryahmed@google.com>
Cc:     Alexei Starovoitov <ast@kernel.org>,
        Daniel Borkmann <daniel@iogearbox.net>,
        Andrii Nakryiko <andrii@kernel.org>,
        Martin KaFai Lau <kafai@fb.com>,
        Song Liu <songliubraving@fb.com>, Yonghong Song <yhs@fb.com>,
        John Fastabend <john.fastabend@gmail.com>,
        KP Singh <kpsingh@kernel.org>, Hao Luo <haoluo@google.com>,
        Networking <netdev@vger.kernel.org>, bpf <bpf@vger.kernel.org>,
        open list <linux-kernel@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

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
>