Re: [PATCH bpf-next v2 7/8] selftests/bpf: Use common CFLAGS for urandom_read

[bot+bpf-ci]

> diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
> index bc049620c774..1e47e29ba421 100644
> --- a/tools/testing/selftests/bpf/Makefile
> +++ b/tools/testing/selftests/bpf/Makefile
> @@ -51,7 +51,7 @@ srctree := $(patsubst %/,%,$(dir $(srctree)))
>  srctree := $(patsubst %/,%,$(dir $(srctree)))
>  endif
>
> -CFLAGS += -g $(OPT_FLAGS) -rdynamic -std=gnu11				\
> +COMMON_CFLAGS = -g $(OPT_FLAGS) -rdynamic -std=gnu11				\
>  	  -Wall -Werror -fno-omit-frame-pointer				\
>  	  -Wno-unused-but-set-variable					\
>  	  $(GENFLAGS) $(SAN_CFLAGS) $(LIBELF_CFLAGS)			\
> @@ -64,7 +64,7 @@ LDLIBS += $(LIBELF_LIBS) -lz -lrt -lpthread
>  PCAP_CFLAGS	:= $(shell $(PKG_CONFIG) --cflags libpcap 2>/dev/null && echo "-DTRAFFIC_MONITOR=1")
>  PCAP_LIBS	:= $(shell $(PKG_CONFIG) --libs libpcap 2>/dev/null)
>  LDLIBS += $(PCAP_LIBS)
> -CFLAGS += $(PCAP_CFLAGS)
> +CFLAGS += $(COMMON_CFLAGS) $(PCAP_CFLAGS)

Ian Rogers raised a concern in v4 that COMMON_CFLAGS is being assigned
before 'include ../lib.mk' (which happens later around line 165).

The ../lib.mk include appends important flags to CFLAGS, such as
-D_GNU_SOURCE=. Since COMMON_CFLAGS uses recursive assignment (=), these
flags added by lib.mk should be available when COMMON_CFLAGS expands.
However, COMMON_CFLAGS references $(GENFLAGS), $(SAN_CFLAGS), and
$(LIBELF_CFLAGS) which are defined before lib.mk, but lib.mk may also
modify CFLAGS directly with additional flags.

Will urandom_read miss any flags that lib.mk adds to CFLAGS but that
aren't captured in the variables referenced by COMMON_CFLAGS? Ian
suggested moving these assignments below the include to ensure all
lib.mk-added flags are captured.

Reference:
https://lore.kernel.org/all/CAP-5=fXjd15e5YpCM9KMjEhpjLxvxc6qUk0g7BprCfkzcRZ5_g@xxxxxxxxxxxxxx/

>
>  # Some utility functions use LLVM libraries
>  jit_disasm_helpers.c-CFLAGS = $(LLVM_CFLAGS)

[ ... ]

> @@ -261,7 +261,7 @@ endif
>  $(OUTPUT)/liburandom_read.so: urandom_read_lib1.c urandom_read_lib2.c liburandom_read.map
>  	$(call msg,LIB,,$@)
>  	$(Q)$(CLANG) $(CLANG_TARGET_ARCH) \
> -		     $(filter-out -static,$(CFLAGS) $(LDFLAGS)) \
> +		     $(filter-out -static,$(COMMON_CFLAGS) $(LDFLAGS)) \
>  		     $(filter %.c,$^) $(filter-out -static,$(LDLIBS)) \
>  		     -Wno-unused-command-line-argument \
>  		     -fuse-ld=$(LLD) -Wl,-znoseparate-code -Wl,--build-id=sha1 \

[ ... ]

> @@ -271,7 +271,7 @@ $(OUTPUT)/liburandom_read.so: urandom_read_lib1.c urandom_read_lib2.c liburandom
>  $(OUTPUT)/urandom_read: urandom_read.c urandom_read_aux.c $(OUTPUT)/liburandom_read.so
>  	$(call msg,BINARY,,$@)
>  	$(Q)$(CLANG) $(CLANG_TARGET_ARCH) \
> -		     $(filter-out -static,$(CFLAGS) $(LDFLAGS)) $(filter %.c,$^) \
> +		     $(filter-out -static,$(COMMON_CFLAGS) $(LDFLAGS)) $(filter %.c,$^) \
>  		     -Wno-unused-command-line-argument \
>  		     -lurandom_read $(filter-out -static,$(LDLIBS)) -L$(OUTPUT) \
>  		     -fuse-ld=$(LLD) -Wl,-znoseparate-code -Wl,--build-id=sha1 \


---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md

CI run summary: https://github.com/kernel-patches/bpf/actions/runs/26828477669