On 12/3/24 19:22, Eduard Zingerman wrote:
> Commit [0] breaks samples/bpf build:
>
> $ make M=samples/bpf
> ...
> make -C /path/to/kernel/samples/bpf/../../tools/lib/bpf \
> ...
> EXTRA_CFLAGS=" \
> ...
> -fsanitize=bounds \
> -I/path/to/kernel/usr/include \
> ...
> /path/to/kernel/samples/bpf/libbpf/libbpf.a install_headers
> CC /path/to/kernel/samples/bpf/libbpf/staticobjs/libbpf.o
> In file included from libbpf.c:29:
> /path/to/kernel/tools/include/linux/err.h:35:8: error: 'inline' can only appear on functions
> 35 | static inline void * __must_check ERR_PTR(long error_)
> | ^
>
> The error is caused by `objtree` variable changing definition from `.`
> (dot) to an absolute path:
> - The variable TPROGS_CFLAGS is constructed as follows:
> ...
> TPROGS_CFLAGS += -I$(objtree)/usr/include
> - It is passed as EXTRA_CFLAGS for libbpf compilation:
> $(LIBBPF): ...
> ...
> $(MAKE) -C $(LIBBPF_SRC) RM='rm -rf' EXTRA_CFLAGS="$(TPROGS_CFLAGS)"
> - Before commit [0], the line passed to libbpf makefile was
> '-I./usr/include', where '.' referred to LIBBPF_SRC due to -C flag.
> The directory $(LIBBPF_SRC)/usr/include does not exist and thus
> was never resolved by C compiler.
> - After commit [0], the line passed to libbpf makefile became:
> '<output-dir>/usr/include', this directory exists and is resolved by
> C compiler.
> - Both 'tools/include' and 'usr/include' define files err.h and types.h.
> - libbpf expects headers like 'linux/err.h' and 'linux/types.h'
> defined in 'tools/include', not 'usr/include', hence the compilation
> error.
>
> This commit removes unnecessary -I flags from libbpf compilation.
> (libbpf sets up the necessary includes at lib/bpf/Makefile:63).
>
> Changes v1 [1] -> v2:
> - dropped unnecessary replacement of KBUILD_OUTPUT with $(objtree)
> (Andrii)
> Changes v2 [2] -> v3:
> - make sure --sysroot option is set for libbpf's EXTRA_CFLAGS,
> if $(SYSROOT) is set (Stanislav)
>
> [0] commit 13b25489b6f8 ("kbuild: change working directory to external module directory with M=")
> [1] https://lore.kernel.org/bpf/20241202212154.3174402-1-eddyz87@xxxxxxxxx/
> [2] https://lore.kernel.org/bpf/20241202234741.3492084-1-eddyz87@xxxxxxxxx/
>
> Fixes: 13b25489b6f8 ("kbuild: change working directory to external module directory with M=")
> Acked-by: Stanislav Fomichev <sdf@xxxxxxxxxxx>
> Signed-off-by: Eduard Zingerman <eddyz87@xxxxxxxxx>
> ---
> samples/bpf/Makefile | 13 +++++++------
> 1 file changed, 7 insertions(+), 6 deletions(-)
>
> diff --git a/samples/bpf/Makefile b/samples/bpf/Makefile
> index bcf103a4c14f..96a05e70ace3 100644
> --- a/samples/bpf/Makefile
> +++ b/samples/bpf/Makefile
> @@ -146,13 +146,14 @@ ifeq ($(ARCH), x86)
> BPF_EXTRA_CFLAGS += -fcf-protection
> endif
>
> -TPROGS_CFLAGS += -Wall -O2
> -TPROGS_CFLAGS += -Wmissing-prototypes
> -TPROGS_CFLAGS += -Wstrict-prototypes
> -TPROGS_CFLAGS += $(call try-run,\
> +COMMON_CFLAGS += -Wall -O2
> +COMMON_CFLAGS += -Wmissing-prototypes
> +COMMON_CFLAGS += -Wstrict-prototypes
> +COMMON_CFLAGS += $(call try-run,\
> printf "int main() { return 0; }" |\
> $(CC) -Werror -fsanitize=bounds -x c - -o "$$TMP",-fsanitize=bounds,)
>
> +TPROGS_CFLAGS += $(COMMON_CFLAGS)
> TPROGS_CFLAGS += -I$(objtree)/usr/include
> TPROGS_CFLAGS += -I$(srctree)/tools/testing/selftests/bpf/
> TPROGS_CFLAGS += -I$(LIBBPF_INCLUDE)
> @@ -162,7 +163,7 @@ TPROGS_CFLAGS += -I$(srctree)/tools/lib
> TPROGS_CFLAGS += -DHAVE_ATTR_TEST=0
>
> ifdef SYSROOT
> -TPROGS_CFLAGS += --sysroot=$(SYSROOT)
> +COMMON_CFLAGS += --sysroot=$(SYSROOT)
> TPROGS_LDFLAGS := -L$(SYSROOT)/usr/lib
> endif
>
> @@ -229,7 +230,7 @@ clean:
>
> $(LIBBPF): $(wildcard $(LIBBPF_SRC)/*.[ch] $(LIBBPF_SRC)/Makefile) | $(LIBBPF_OUTPUT)
> # Fix up variables inherited from Kbuild that tools/ build system won't like
> - $(MAKE) -C $(LIBBPF_SRC) RM='rm -rf' EXTRA_CFLAGS="$(TPROGS_CFLAGS)" \
> + $(MAKE) -C $(LIBBPF_SRC) RM='rm -rf' EXTRA_CFLAGS="$(COMMON_CFLAGS)" \
I was not quick enough to reply before this got merged, sorry about that.
This will break situations when we want to pass extra flags to the
libbpf sub-make from the command line, e.g. to build samples as PIE:
$ make TPROGS_USER_CFLAGS="-fpie" TPROGS_USER_LDFLAGS="-pie"
[...]
/usr/bin/ld: /bpf/samples/bpf/libbpf/libbpf.a(libbpf-in.o):
relocation R_X86_64_32 against `.rodata.str1.1' can not be used when
making a PIE object; recompile with -fPIE
/usr/bin/ld: failed to set dynamic section sizes: bad value
I think that we should add
COMMON_CFLAGS = $(TPROGS_USER_CFLAGS)
somewhere to the top of the Makefile.
Viktor
> LDFLAGS="$(TPROGS_LDFLAGS)" srctree=$(BPF_SAMPLES_PATH)/../../ \
> O= OUTPUT=$(LIBBPF_OUTPUT)/ DESTDIR=$(LIBBPF_DESTDIR) prefix= \
> $@ install_headers
