From: sashiko-bot@kernel.org
To: "Leo Yan" <leo.yan@arm.com>
Cc: bpf@vger.kernel.org
Subject: Re: [PATCH bpf-next v2 7/8] selftests/bpf: Use common CFLAGS for urandom_read
Date: Tue, 02 Jun 2026 17:25:59 +0000 [thread overview]
Message-ID: <20260602172559.B3A1D1F00898@smtp.kernel.org> (raw)
In-Reply-To: <20260602-tools_build_fix_zero_init_bpf_only-v2-7-c76e5250ea1c@arm.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] Replacing CFLAGS with COMMON_CFLAGS for urandom_read breaks cross-compilation with Clang.
--
commit 537d69efefb1121c195758d9362eba3f31c54f36
Author: Leo Yan <leo.yan@arm.com>
selftests/bpf: Use common CFLAGS for urandom_read
This commit introduces a COMMON_CFLAGS variable to separate general flags
from CC-specific ones that might be incompatible with CLANG when building
urandom_read and its shared library.
> diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
> index bc049620c7745..1e47e29ba4214 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 \
[Severity: Low]
Does this change break cross-compilation with Clang?
By defining COMMON_CFLAGS from scratch without including the pre-existing
CFLAGS, this drops any earlier contents from the urandom_read build.
This strips CLANG_CROSS_FLAGS (which is appended to CFLAGS earlier by
tools/scripts/Makefile.include), depriving Clang of essential
cross-compilation arguments like --sysroot and --gcc-toolchain.
Does this also inadvertently ignore any user-provided 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)) \
[Severity: Low]
When replacing CFLAGS with COMMON_CFLAGS here for the shared library build,
does this remove the required sysroot and toolchain cross-compilation flags?
[ ... ]
> @@ -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,$^) \
[Severity: Low]
Similarly, does the urandom_read build rule also lose these required
cross-compilation flags when using COMMON_CFLAGS?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260602-tools_build_fix_zero_init_bpf_only-v2-0-c76e5250ea1c@arm.com?part=7
next prev parent reply other threads:[~2026-06-02 17:26 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-06-02 14:47 [PATCH bpf-next v2 0/8] tools build: bpf: Append EXTRA_CFLAGS and HOST_EXTRACFLAGS Leo Yan
2026-06-02 14:47 ` [PATCH bpf-next v2 1/8] bpftool: Pass host flags to bootstrap libbpf Leo Yan
2026-06-03 9:21 ` Quentin Monnet
2026-06-02 14:47 ` [PATCH bpf-next v2 2/8] bpftool: Avoid adding EXTRA_CFLAGS to HOST_CFLAGS Leo Yan
2026-06-02 14:47 ` [PATCH bpf-next v2 3/8] bpftool: Append extra host flags Leo Yan
2026-06-02 14:47 ` [PATCH bpf-next v2 4/8] libbpf: Initialize CFLAGS before including Makefile.include Leo Yan
2026-06-02 15:11 ` sashiko-bot
2026-06-02 14:47 ` [PATCH bpf-next v2 5/8] tools/bpf: build: Append extra cflags Leo Yan
2026-06-02 14:47 ` [PATCH bpf-next v2 6/8] selftests/bpf: Initialize operation name before use Leo Yan
2026-06-02 14:47 ` [PATCH bpf-next v2 7/8] selftests/bpf: Use common CFLAGS for urandom_read Leo Yan
2026-06-02 15:32 ` bot+bpf-ci
2026-06-02 17:25 ` sashiko-bot [this message]
2026-06-02 14:47 ` [PATCH bpf-next v2 8/8] selftests/bpf: Avoid static LLVM linking for cross builds Leo Yan
2026-06-02 17:33 ` sashiko-bot
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20260602172559.B3A1D1F00898@smtp.kernel.org \
--to=sashiko-bot@kernel.org \
--cc=bpf@vger.kernel.org \
--cc=leo.yan@arm.com \
--cc=sashiko-reviews@lists.linux.dev \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.