2022-06-09 07:09:31

by James Hilliard

[permalink] [raw]
Subject: [PATCH 1/1] libbpf: replace typeof with __typeof__ for -std=c17 compatibility

Fixes errors like:
error: expected specifier-qualifier-list before 'typeof'
14 | #define __type(name, val) typeof(val) *name
| ^~~~~~
../src/core/bpf/socket_bind/socket-bind.bpf.c:25:9: note: in expansion of macro '__type'
25 | __type(key, __u32);
| ^~~~~~

Signed-off-by: James Hilliard <[email protected]>
---
tools/lib/bpf/bpf_core_read.h | 16 ++++++++--------
tools/lib/bpf/bpf_helpers.h | 4 ++--
tools/lib/bpf/bpf_tracing.h | 24 ++++++++++++------------
tools/lib/bpf/btf.h | 4 ++--
tools/lib/bpf/libbpf_internal.h | 6 +++---
tools/lib/bpf/usdt.bpf.h | 6 +++---
tools/lib/bpf/xsk.h | 12 ++++++------
7 files changed, 36 insertions(+), 36 deletions(-)

diff --git a/tools/lib/bpf/bpf_core_read.h b/tools/lib/bpf/bpf_core_read.h
index fd48b1ff59ca..d3a88721c9e7 100644
--- a/tools/lib/bpf/bpf_core_read.h
+++ b/tools/lib/bpf/bpf_core_read.h
@@ -111,7 +111,7 @@ enum bpf_enum_value_kind {
})

#define ___bpf_field_ref1(field) (field)
-#define ___bpf_field_ref2(type, field) (((typeof(type) *)0)->field)
+#define ___bpf_field_ref2(type, field) (((__typeof__(type) *)0)->field)
#define ___bpf_field_ref(args...) \
___bpf_apply(___bpf_field_ref, ___bpf_narg(args))(args)

@@ -161,7 +161,7 @@ enum bpf_enum_value_kind {
* BTF. Always succeeds.
*/
#define bpf_core_type_id_local(type) \
- __builtin_btf_type_id(*(typeof(type) *)0, BPF_TYPE_ID_LOCAL)
+ __builtin_btf_type_id(*(__typeof__(type) *)0, BPF_TYPE_ID_LOCAL)

/*
* Convenience macro to get BTF type ID of a target kernel's type that matches
@@ -171,7 +171,7 @@ enum bpf_enum_value_kind {
* - 0, if no matching type was found in a target kernel BTF.
*/
#define bpf_core_type_id_kernel(type) \
- __builtin_btf_type_id(*(typeof(type) *)0, BPF_TYPE_ID_TARGET)
+ __builtin_btf_type_id(*(__typeof__(type) *)0, BPF_TYPE_ID_TARGET)

/*
* Convenience macro to check that provided named type
@@ -181,7 +181,7 @@ enum bpf_enum_value_kind {
* 0, if no matching type is found.
*/
#define bpf_core_type_exists(type) \
- __builtin_preserve_type_info(*(typeof(type) *)0, BPF_TYPE_EXISTS)
+ __builtin_preserve_type_info(*(__typeof__(type) *)0, BPF_TYPE_EXISTS)

/*
* Convenience macro to get the byte size of a provided named type
@@ -191,7 +191,7 @@ enum bpf_enum_value_kind {
* 0, if no matching type is found.
*/
#define bpf_core_type_size(type) \
- __builtin_preserve_type_info(*(typeof(type) *)0, BPF_TYPE_SIZE)
+ __builtin_preserve_type_info(*(__typeof__(type) *)0, BPF_TYPE_SIZE)

/*
* Convenience macro to check that provided enumerator value is defined in
@@ -202,7 +202,7 @@ enum bpf_enum_value_kind {
* 0, if no matching enum and/or enum value within that enum is found.
*/
#define bpf_core_enum_value_exists(enum_type, enum_value) \
- __builtin_preserve_enum_value(*(typeof(enum_type) *)enum_value, BPF_ENUMVAL_EXISTS)
+ __builtin_preserve_enum_value(*(__typeof__(enum_type) *)enum_value, BPF_ENUMVAL_EXISTS)

/*
* Convenience macro to get the integer value of an enumerator value in
@@ -213,7 +213,7 @@ enum bpf_enum_value_kind {
* 0, if no matching enum and/or enum value within that enum is found.
*/
#define bpf_core_enum_value(enum_type, enum_value) \
- __builtin_preserve_enum_value(*(typeof(enum_type) *)enum_value, BPF_ENUMVAL_VALUE)
+ __builtin_preserve_enum_value(*(__typeof__(enum_type) *)enum_value, BPF_ENUMVAL_VALUE)

/*
* bpf_core_read() abstracts away bpf_probe_read_kernel() call and captures
@@ -300,7 +300,7 @@ enum bpf_enum_value_kind {
#define ___arrow10(a, b, c, d, e, f, g, h, i, j) a->b->c->d->e->f->g->h->i->j
#define ___arrow(...) ___apply(___arrow, ___narg(__VA_ARGS__))(__VA_ARGS__)

-#define ___type(...) typeof(___arrow(__VA_ARGS__))
+#define ___type(...) __typeof__(___arrow(__VA_ARGS__))

#define ___read(read_fn, dst, src_type, src, accessor) \
read_fn((void *)(dst), sizeof(*(dst)), &((src_type)(src))->accessor)
diff --git a/tools/lib/bpf/bpf_helpers.h b/tools/lib/bpf/bpf_helpers.h
index fb04eaf367f1..859604345e03 100644
--- a/tools/lib/bpf/bpf_helpers.h
+++ b/tools/lib/bpf/bpf_helpers.h
@@ -11,8 +11,8 @@
#include "bpf_helper_defs.h"

#define __uint(name, val) int (*name)[val]
-#define __type(name, val) typeof(val) *name
-#define __array(name, val) typeof(val) *name[]
+#define __type(name, val) __typeof__(val) *name
+#define __array(name, val) __typeof__(val) *name[]

/*
* Helper macro to place programs, maps, license in
diff --git a/tools/lib/bpf/bpf_tracing.h b/tools/lib/bpf/bpf_tracing.h
index 01ce121c302d..d64fcf01ea20 100644
--- a/tools/lib/bpf/bpf_tracing.h
+++ b/tools/lib/bpf/bpf_tracing.h
@@ -424,16 +424,16 @@ struct pt_regs;
*/
#define BPF_PROG(name, args...) \
name(unsigned long long *ctx); \
-static __attribute__((always_inline)) typeof(name(0)) \
+static __attribute__((always_inline)) __typeof__(name(0)) \
____##name(unsigned long long *ctx, ##args); \
-typeof(name(0)) name(unsigned long long *ctx) \
+__typeof__(name(0)) name(unsigned long long *ctx) \
{ \
_Pragma("GCC diagnostic push") \
_Pragma("GCC diagnostic ignored \"-Wint-conversion\"") \
return ____##name(___bpf_ctx_cast(args)); \
_Pragma("GCC diagnostic pop") \
} \
-static __attribute__((always_inline)) typeof(name(0)) \
+static __attribute__((always_inline)) __typeof__(name(0)) \
____##name(unsigned long long *ctx, ##args)

struct pt_regs;
@@ -458,16 +458,16 @@ struct pt_regs;
*/
#define BPF_KPROBE(name, args...) \
name(struct pt_regs *ctx); \
-static __attribute__((always_inline)) typeof(name(0)) \
+static __attribute__((always_inline)) __typeof__(name(0)) \
____##name(struct pt_regs *ctx, ##args); \
-typeof(name(0)) name(struct pt_regs *ctx) \
+__typeof__(name(0)) name(struct pt_regs *ctx) \
{ \
_Pragma("GCC diagnostic push") \
_Pragma("GCC diagnostic ignored \"-Wint-conversion\"") \
return ____##name(___bpf_kprobe_args(args)); \
_Pragma("GCC diagnostic pop") \
} \
-static __attribute__((always_inline)) typeof(name(0)) \
+static __attribute__((always_inline)) __typeof__(name(0)) \
____##name(struct pt_regs *ctx, ##args)

#define ___bpf_kretprobe_args0() ctx
@@ -482,16 +482,16 @@ ____##name(struct pt_regs *ctx, ##args)
*/
#define BPF_KRETPROBE(name, args...) \
name(struct pt_regs *ctx); \
-static __attribute__((always_inline)) typeof(name(0)) \
+static __attribute__((always_inline)) __typeof__(name(0)) \
____##name(struct pt_regs *ctx, ##args); \
-typeof(name(0)) name(struct pt_regs *ctx) \
+__typeof__(name(0)) name(struct pt_regs *ctx) \
{ \
_Pragma("GCC diagnostic push") \
_Pragma("GCC diagnostic ignored \"-Wint-conversion\"") \
return ____##name(___bpf_kretprobe_args(args)); \
_Pragma("GCC diagnostic pop") \
} \
-static __always_inline typeof(name(0)) ____##name(struct pt_regs *ctx, ##args)
+static __always_inline __typeof__(name(0)) ____##name(struct pt_regs *ctx, ##args)

#define ___bpf_syscall_args0() ctx
#define ___bpf_syscall_args1(x) ___bpf_syscall_args0(), (void *)PT_REGS_PARM1_CORE_SYSCALL(regs)
@@ -515,9 +515,9 @@ static __always_inline typeof(name(0)) ____##name(struct pt_regs *ctx, ##args)
*/
#define BPF_KPROBE_SYSCALL(name, args...) \
name(struct pt_regs *ctx); \
-static __attribute__((always_inline)) typeof(name(0)) \
+static __attribute__((always_inline)) __typeof__(name(0)) \
____##name(struct pt_regs *ctx, ##args); \
-typeof(name(0)) name(struct pt_regs *ctx) \
+__typeof__(name(0)) name(struct pt_regs *ctx) \
{ \
struct pt_regs *regs = PT_REGS_SYSCALL_REGS(ctx); \
_Pragma("GCC diagnostic push") \
@@ -525,7 +525,7 @@ typeof(name(0)) name(struct pt_regs *ctx) \
return ____##name(___bpf_syscall_args(args)); \
_Pragma("GCC diagnostic pop") \
} \
-static __attribute__((always_inline)) typeof(name(0)) \
+static __attribute__((always_inline)) __typeof__(name(0)) \
____##name(struct pt_regs *ctx, ##args)

#endif
diff --git a/tools/lib/bpf/btf.h b/tools/lib/bpf/btf.h
index 9fb416eb5644..c39ef51c361b 100644
--- a/tools/lib/bpf/btf.h
+++ b/tools/lib/bpf/btf.h
@@ -322,8 +322,8 @@ LIBBPF_API struct btf_dump *btf_dump__new_deprecated(const struct btf *btf,
*/
#ifndef __cplusplus
#define btf_dump__new(a1, a2, a3, a4) __builtin_choose_expr( \
- __builtin_types_compatible_p(typeof(a4), btf_dump_printf_fn_t) || \
- __builtin_types_compatible_p(typeof(a4), void(void *, const char *, va_list)), \
+ __builtin_types_compatible_p(__typeof__(a4), btf_dump_printf_fn_t) || \
+ __builtin_types_compatible_p(__typeof__(a4), void(void *, const char *, va_list)), \
btf_dump__new_deprecated((void *)a1, (void *)a2, (void *)a3, (void *)a4), \
btf_dump__new((void *)a1, (void *)a2, (void *)a3, (void *)a4))
#endif
diff --git a/tools/lib/bpf/libbpf_internal.h b/tools/lib/bpf/libbpf_internal.h
index a1ad145ffa74..259664c1dedb 100644
--- a/tools/lib/bpf/libbpf_internal.h
+++ b/tools/lib/bpf/libbpf_internal.h
@@ -138,7 +138,7 @@ static inline bool str_has_sfx(const char *str, const char *sfx)

#define COMPAT_VERSION(internal_name, api_name, version)
#define DEFAULT_VERSION(internal_name, api_name, version) \
- extern typeof(internal_name) api_name \
+ extern __typeof__(internal_name) api_name \
__attribute__((alias(#internal_name)));

#endif
@@ -300,7 +300,7 @@ static inline bool libbpf_validate_opts(const char *opts,
type##__last_field), \
(opts)->sz, #type))
#define OPTS_HAS(opts, field) \
- ((opts) && opts->sz >= offsetofend(typeof(*(opts)), field))
+ ((opts) && opts->sz >= offsetofend(__typeof__(*(opts)), field))
#define OPTS_GET(opts, field, fallback_value) \
(OPTS_HAS(opts, field) ? (opts)->field : fallback_value)
#define OPTS_SET(opts, field, value) \
@@ -311,7 +311,7 @@ static inline bool libbpf_validate_opts(const char *opts,

#define OPTS_ZEROED(opts, last_nonzero_field) \
({ \
- ssize_t __off = offsetofend(typeof(*(opts)), last_nonzero_field); \
+ ssize_t __off = offsetofend(__typeof__(*(opts)), last_nonzero_field); \
!(opts) || libbpf_is_mem_zeroed((const void *)opts + __off, \
(opts)->sz - __off); \
})
diff --git a/tools/lib/bpf/usdt.bpf.h b/tools/lib/bpf/usdt.bpf.h
index 4181fddb3687..a822e181371c 100644
--- a/tools/lib/bpf/usdt.bpf.h
+++ b/tools/lib/bpf/usdt.bpf.h
@@ -244,16 +244,16 @@ long bpf_usdt_cookie(struct pt_regs *ctx)
*/
#define BPF_USDT(name, args...) \
name(struct pt_regs *ctx); \
-static __attribute__((always_inline)) typeof(name(0)) \
+static __attribute__((always_inline)) __typeof__(name(0)) \
____##name(struct pt_regs *ctx, ##args); \
-typeof(name(0)) name(struct pt_regs *ctx) \
+__typeof__(name(0)) name(struct pt_regs *ctx) \
{ \
_Pragma("GCC diagnostic push") \
_Pragma("GCC diagnostic ignored \"-Wint-conversion\"") \
return ____##name(___bpf_usdt_args(args)); \
_Pragma("GCC diagnostic pop") \
} \
-static __attribute__((always_inline)) typeof(name(0)) \
+static __attribute__((always_inline)) __typeof__(name(0)) \
____##name(struct pt_regs *ctx, ##args)

#endif /* __USDT_BPF_H__ */
diff --git a/tools/lib/bpf/xsk.h b/tools/lib/bpf/xsk.h
index 64e9c57fd792..631549817976 100644
--- a/tools/lib/bpf/xsk.h
+++ b/tools/lib/bpf/xsk.h
@@ -36,8 +36,8 @@ extern "C" {
* LIBRARY INTERNAL
*/

-#define __XSK_READ_ONCE(x) (*(volatile typeof(x) *)&x)
-#define __XSK_WRITE_ONCE(x, v) (*(volatile typeof(x) *)&x) = (v)
+#define __XSK_READ_ONCE(x) (*(volatile __typeof__(x) *)&x)
+#define __XSK_WRITE_ONCE(x, v) (*(volatile __typeof__(x) *)&x) = (v)

#if defined(__i386__) || defined(__x86_64__)
# define libbpf_smp_store_release(p, v) \
@@ -47,7 +47,7 @@ extern "C" {
} while (0)
# define libbpf_smp_load_acquire(p) \
({ \
- typeof(*p) ___p1 = __XSK_READ_ONCE(*p); \
+ __typeof__(*p) ___p1 = __XSK_READ_ONCE(*p); \
asm volatile("" : : : "memory"); \
___p1; \
})
@@ -56,7 +56,7 @@ extern "C" {
asm volatile ("stlr %w1, %0" : "=Q" (*p) : "r" (v) : "memory")
# define libbpf_smp_load_acquire(p) \
({ \
- typeof(*p) ___p1; \
+ __typeof__(*p) ___p1; \
asm volatile ("ldar %w0, %1" \
: "=r" (___p1) : "Q" (*p) : "memory"); \
___p1; \
@@ -69,7 +69,7 @@ extern "C" {
} while (0)
# define libbpf_smp_load_acquire(p) \
({ \
- typeof(*p) ___p1 = __XSK_READ_ONCE(*p); \
+ __typeof__(*p) ___p1 = __XSK_READ_ONCE(*p); \
asm volatile ("fence r,rw" : : : "memory"); \
___p1; \
})
@@ -86,7 +86,7 @@ extern "C" {
#ifndef libbpf_smp_load_acquire
#define libbpf_smp_load_acquire(p) \
({ \
- typeof(*p) ___p1 = __XSK_READ_ONCE(*p); \
+ __typeof__(*p) ___p1 = __XSK_READ_ONCE(*p); \
__sync_synchronize(); \
___p1; \
})
--
2.25.1


2022-06-09 18:44:02

by Andrii Nakryiko

[permalink] [raw]
Subject: Re: [PATCH 1/1] libbpf: replace typeof with __typeof__ for -std=c17 compatibility

On Wed, Jun 8, 2022 at 11:28 PM James Hilliard
<[email protected]> wrote:
>
> Fixes errors like:
> error: expected specifier-qualifier-list before 'typeof'
> 14 | #define __type(name, val) typeof(val) *name
> | ^~~~~~
> ../src/core/bpf/socket_bind/socket-bind.bpf.c:25:9: note: in expansion of macro '__type'
> 25 | __type(key, __u32);
> | ^~~~~~
>
> Signed-off-by: James Hilliard <[email protected]>
> ---

If you follow DPDK link you gave me ([0]), you'll see that they ended up doing

#ifndef typeof
#define typeof __typeof__
#endif

It's way more localized. Let's do that.

But also I tried to build libbpf-bootstrap with -std=c17 and
immediately ran into issue with asm, so we need to do the same with
asm -> __asm__. Can you please update your patch and fix both issues?

[0] https://patches.dpdk.org/project/dpdk/patch/2601191342CEEE43887BDE71AB977258213F3012@irsmsx105.ger.corp.intel.com/
[1] https://github.com/libbpf/libbpf-bootstrap


> tools/lib/bpf/bpf_core_read.h | 16 ++++++++--------
> tools/lib/bpf/bpf_helpers.h | 4 ++--
> tools/lib/bpf/bpf_tracing.h | 24 ++++++++++++------------
> tools/lib/bpf/btf.h | 4 ++--
> tools/lib/bpf/libbpf_internal.h | 6 +++---
> tools/lib/bpf/usdt.bpf.h | 6 +++---
> tools/lib/bpf/xsk.h | 12 ++++++------
> 7 files changed, 36 insertions(+), 36 deletions(-)
>

[...]

2022-06-10 00:16:36

by James Hilliard

[permalink] [raw]
Subject: Re: [PATCH 1/1] libbpf: replace typeof with __typeof__ for -std=c17 compatibility

On Thu, Jun 9, 2022 at 12:11 PM Andrii Nakryiko
<[email protected]> wrote:
>
> On Wed, Jun 8, 2022 at 11:28 PM James Hilliard
> <[email protected]> wrote:
> >
> > Fixes errors like:
> > error: expected specifier-qualifier-list before 'typeof'
> > 14 | #define __type(name, val) typeof(val) *name
> > | ^~~~~~
> > ../src/core/bpf/socket_bind/socket-bind.bpf.c:25:9: note: in expansion of macro '__type'
> > 25 | __type(key, __u32);
> > | ^~~~~~
> >
> > Signed-off-by: James Hilliard <[email protected]>
> > ---
>
> If you follow DPDK link you gave me ([0]), you'll see that they ended up doing
>
> #ifndef typeof
> #define typeof __typeof__
> #endif
>
> It's way more localized. Let's do that.

Won't that potentially leak the redefinition into external code including the
header file?

I don't see a redefinition of typeof like that used elsewhere in the kernel.

However I do see __typeof__ used in many headers already so that approach
seems to follow normal conventions and seems less risky.

FYI using -std=gnu17 instead of -std=c17 works around this issue with bpf-gcc
at least so this issue isn't really a blocker like the SEC macro
issue, I had just
accidentally mixed the two issues up due to accidentally not clearing out some
header files when testing, they seem to be entirely separate.

>
> But also I tried to build libbpf-bootstrap with -std=c17 and
> immediately ran into issue with asm, so we need to do the same with
> asm -> __asm__. Can you please update your patch and fix both issues?

Are you hitting that with clang/llvm or bpf-gcc? It doesn't appear that the
libbpf-bootstrap build system is set up to build with bpf-gcc yet.

>
> [0] https://patches.dpdk.org/project/dpdk/patch/2601191342CEEE43887BDE71AB977258213F3012@irsmsx105.ger.corp.intel.com/
> [1] https://github.com/libbpf/libbpf-bootstrap
>
>
> > tools/lib/bpf/bpf_core_read.h | 16 ++++++++--------
> > tools/lib/bpf/bpf_helpers.h | 4 ++--
> > tools/lib/bpf/bpf_tracing.h | 24 ++++++++++++------------
> > tools/lib/bpf/btf.h | 4 ++--
> > tools/lib/bpf/libbpf_internal.h | 6 +++---
> > tools/lib/bpf/usdt.bpf.h | 6 +++---
> > tools/lib/bpf/xsk.h | 12 ++++++------
> > 7 files changed, 36 insertions(+), 36 deletions(-)
> >
>
> [...]

2022-06-27 21:53:23

by Andrii Nakryiko

[permalink] [raw]
Subject: Re: [PATCH 1/1] libbpf: replace typeof with __typeof__ for -std=c17 compatibility

On Thu, Jun 9, 2022 at 4:46 PM James Hilliard <[email protected]> wrote:
>
> On Thu, Jun 9, 2022 at 12:11 PM Andrii Nakryiko
> <[email protected]> wrote:
> >
> > On Wed, Jun 8, 2022 at 11:28 PM James Hilliard
> > <[email protected]> wrote:
> > >
> > > Fixes errors like:
> > > error: expected specifier-qualifier-list before 'typeof'
> > > 14 | #define __type(name, val) typeof(val) *name
> > > | ^~~~~~
> > > ../src/core/bpf/socket_bind/socket-bind.bpf.c:25:9: note: in expansion of macro '__type'
> > > 25 | __type(key, __u32);
> > > | ^~~~~~
> > >
> > > Signed-off-by: James Hilliard <[email protected]>
> > > ---
> >
> > If you follow DPDK link you gave me ([0]), you'll see that they ended up doing
> >
> > #ifndef typeof
> > #define typeof __typeof__
> > #endif
> >
> > It's way more localized. Let's do that.
>
> Won't that potentially leak the redefinition into external code including the
> header file?

all this is in BPF-side helpers and we add a bunch of other "common"
definitions like barrier(), offsetof(), etc. So I think this is
acceptable, at least for now (and users will be able to override this
due to #ifndef check, right?)

>
> I don't see a redefinition of typeof like that used elsewhere in the kernel.

kernel is just happily using much cleaner looking typeof() almost
universally, though

$ rg -w typeof ~/linux/include | wc -l
401
$ rg -w __typeof__ ~/linux/include | wc -l
28

>
> However I do see __typeof__ used in many headers already so that approach
> seems to follow normal conventions and seems less risky.
>
> FYI using -std=gnu17 instead of -std=c17 works around this issue with bpf-gcc
> at least so this issue isn't really a blocker like the SEC macro
> issue, I had just
> accidentally mixed the two issues up due to accidentally not clearing out some
> header files when testing, they seem to be entirely separate.
>
> >
> > But also I tried to build libbpf-bootstrap with -std=c17 and
> > immediately ran into issue with asm, so we need to do the same with
> > asm -> __asm__. Can you please update your patch and fix both issues?
>
> Are you hitting that with clang/llvm or bpf-gcc? It doesn't appear that the
> libbpf-bootstrap build system is set up to build with bpf-gcc yet.
>

I didn't realize you were using bpf-gcc, sorry. I was testing with clang.

> >
> > [0] https://patches.dpdk.org/project/dpdk/patch/2601191342CEEE43887BDE71AB977258213F3012@irsmsx105.ger.corp.intel.com/
> > [1] https://github.com/libbpf/libbpf-bootstrap
> >
> >
> > > tools/lib/bpf/bpf_core_read.h | 16 ++++++++--------
> > > tools/lib/bpf/bpf_helpers.h | 4 ++--
> > > tools/lib/bpf/bpf_tracing.h | 24 ++++++++++++------------
> > > tools/lib/bpf/btf.h | 4 ++--
> > > tools/lib/bpf/libbpf_internal.h | 6 +++---
> > > tools/lib/bpf/usdt.bpf.h | 6 +++---
> > > tools/lib/bpf/xsk.h | 12 ++++++------
> > > 7 files changed, 36 insertions(+), 36 deletions(-)
> > >
> >
> > [...]