From: "Jose E. Marchesi" <jose.marchesi@oracle.com>
To: bpf@vger.kernel.org
Cc: "Jose E . Marchesi" <jose.marchesi@oracle.com>,
david.faust@oracle.com, cupertino.miranda@oracle.com,
Alexei Starovoitov <alexei.starovoitov@gmail.com>
Subject: [PATCH bpf-next] bpf: make list_for_each_entry portable
Date: Thu, 9 May 2024 10:46:50 +0200 [thread overview]
Message-ID: <20240509084650.17546-1-jose.marchesi@oracle.com> (raw)
The macro list_for_each_entry is defined in bpf_arena_list.h as
follows:
#define list_for_each_entry(pos, head, member) \
for (void * ___tmp = (pos = list_entry_safe((head)->first, \
typeof(*(pos)), member), \
(void *)0); \
pos && ({ ___tmp = (void *)pos->member.next; 1; }); \
cond_break, \
pos = list_entry_safe((void __arena *)___tmp, typeof(*(pos)), member))
The macro cond_break, in turn, expands to a statement expression that
contains a `break' statement. Compound statement expressions, and the
subsequent ability of placing statements in the header of a `for'
loop, are GNU extensions.
Unfortunately, clang implements this GNU extension differently than
GCC:
- In GCC the `break' statement is bound to the containing "breakable"
context in which the defining `for' appears. If there is no such
context, GCC emits a warning: break statement without enclosing `for'
o `switch' statement.
- In clang the `break' statement is bound to the defining `for'. If
the defining `for' is itself inside some breakable construct, then
clang emits a -Wgcc-compat warning.
This patch makes it possible to use the cond_break macro with GCC by
adding an outer breakable context __compat_break that expands to a
one-iteration `for' loop when compiling with GCC, and to nothing when
compiling with clang. It is important to note that the __compat_break
one-iteration loop gets optimized away by GCC with -O1 and higher.
The list_for_each_entry macro is adapted to use __compat_break, as are
the pertinent loops in the few tests that use cond_break directly.
Tested in bpf-next master.
No regressions.
Signed-off-by: Jose E. Marchesi <jose.marchesi@oracle.com>
Cc: david.faust@oracle.com
Cc: cupertino.miranda@oracle.com
Cc: Alexei Starovoitov <alexei.starovoitov@gmail.com>
---
tools/testing/selftests/bpf/bpf_arena_list.h | 2 ++
tools/testing/selftests/bpf/bpf_experimental.h | 11 +++++++++++
tools/testing/selftests/bpf/progs/arena_list.c | 1 +
.../bpf/progs/verifier_iterating_callbacks.c | 3 +++
4 files changed, 17 insertions(+)
diff --git a/tools/testing/selftests/bpf/bpf_arena_list.h b/tools/testing/selftests/bpf/bpf_arena_list.h
index b99b9f408eff..5659c715a8a0 100644
--- a/tools/testing/selftests/bpf/bpf_arena_list.h
+++ b/tools/testing/selftests/bpf/bpf_arena_list.h
@@ -29,10 +29,12 @@ static inline void *bpf_iter_num_new(struct bpf_iter_num *it, int i, int j) { re
static inline void bpf_iter_num_destroy(struct bpf_iter_num *it) {}
static inline bool bpf_iter_num_next(struct bpf_iter_num *it) { return true; }
#define cond_break ({})
+#define __compat_break
#endif
/* Safely walk link list elements. Deletion of elements is allowed. */
#define list_for_each_entry(pos, head, member) \
+ __compat_break \
for (void * ___tmp = (pos = list_entry_safe((head)->first, \
typeof(*(pos)), member), \
(void *)0); \
diff --git a/tools/testing/selftests/bpf/bpf_experimental.h b/tools/testing/selftests/bpf/bpf_experimental.h
index 8b9cc87be4c4..7f03570638a6 100644
--- a/tools/testing/selftests/bpf/bpf_experimental.h
+++ b/tools/testing/selftests/bpf/bpf_experimental.h
@@ -326,6 +326,17 @@ l_true: \
})
#endif
+/* A `break' executed in the head of a `for' loop statement is bound
+ to the current loop in clang, but it is bound to the enclosing loop
+ in GCC. Note both compilers optimize the outer loop out with -O1
+ and higher. This macro shall be used to annotate any loop that
+ uses cond_break within its header. */
+#ifdef __clang__
+#define __compat_break
+#else
+#define __compat_break for (int __control = 1; __control; --__control)
+#endif
+
#ifdef __BPF_FEATURE_MAY_GOTO
#define cond_break \
({ __label__ l_break, l_continue; \
diff --git a/tools/testing/selftests/bpf/progs/arena_list.c b/tools/testing/selftests/bpf/progs/arena_list.c
index c0422c58cee2..570c1e043257 100644
--- a/tools/testing/selftests/bpf/progs/arena_list.c
+++ b/tools/testing/selftests/bpf/progs/arena_list.c
@@ -49,6 +49,7 @@ int arena_list_add(void *ctx)
list_head = &global_head;
+ __compat_break
for (i = zero; i < cnt; cond_break, i++) {
struct elem __arena *n = bpf_alloc(sizeof(*n));
diff --git a/tools/testing/selftests/bpf/progs/verifier_iterating_callbacks.c b/tools/testing/selftests/bpf/progs/verifier_iterating_callbacks.c
index 99e561f18f9b..e0437609af21 100644
--- a/tools/testing/selftests/bpf/progs/verifier_iterating_callbacks.c
+++ b/tools/testing/selftests/bpf/progs/verifier_iterating_callbacks.c
@@ -318,6 +318,7 @@ int cond_break1(const void *ctx)
unsigned long i;
unsigned int sum = 0;
+ __compat_break
for (i = zero; i < ARR_SZ; cond_break, i++)
sum += i;
for (i = zero; i < ARR_SZ; i++) {
@@ -336,6 +337,7 @@ int cond_break2(const void *ctx)
int i, j;
int sum = 0;
+ __compat_break
for (i = zero; i < 1000; cond_break, i++)
for (j = zero; j < 1000; j++) {
sum += i + j;
@@ -349,6 +351,7 @@ static __noinline int loop(void)
{
int i, sum = 0;
+ __compat_break
for (i = zero; i <= 1000000; i++, cond_break)
sum += i;
--
2.30.2
next reply other threads:[~2024-05-09 8:47 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-05-09 8:46 Jose E. Marchesi [this message]
2024-05-09 21:48 ` [PATCH bpf-next] bpf: make list_for_each_entry portable Alexei Starovoitov
2024-05-10 8:26 ` Jose E. Marchesi
2024-05-10 17:03 ` Alexei Starovoitov
2024-05-10 17:16 ` Jose E. Marchesi
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=20240509084650.17546-1-jose.marchesi@oracle.com \
--to=jose.marchesi@oracle.com \
--cc=alexei.starovoitov@gmail.com \
--cc=bpf@vger.kernel.org \
--cc=cupertino.miranda@oracle.com \
--cc=david.faust@oracle.com \
/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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).