libbpf: add support to GCC in CORE macro definitions

Commit Message

Cupertino Miranda Feb. 8, 2024, 7:58 p.m. UTC

Due to internal differences between LLVM and GCC the current
implementation for the CO-RE macros does not fit GCC parser, as it will
optimize those expressions even before those would be accessible by the
BPF backend.

As examples, the following would be optimized out with the original
definitions:
  - As enums are converted to their integer representation during
  parsing, the IR would not know how to distinguish an integer
  constant from an actual enum value.
  - Types need to be kept as temporary variables, as the existing type
  casts of the 0 address (as expanded for LLVM), are optimized away by
  the GCC C parser, never really reaching GCCs IR.

Although, the macros appear to add extra complexity, the expanded code
is removed from the compilation flow very early in the compilation
process, not really affecting the quality of the generated assembly.

Signed-off-by: Cupertino Miranda <[e-mail]>
---
 tools/lib/bpf/bpf_core_read.h | 46 ++++++++++++++++++++++++++++++-----
 1 file changed, 40 insertions(+), 6 deletions(-)

Comments

Andrii Nakryiko Feb. 12, 2024, 11:51 p.m. UTC | #1

On Thu, Feb 8, 2024 at 11:58 AM Cupertino Miranda
<[e-mail]> wrote:
>
> Due to internal differences between LLVM and GCC the current
> implementation for the CO-RE macros does not fit GCC parser, as it will
> optimize those expressions even before those would be accessible by the
> BPF backend.
>
> As examples, the following would be optimized out with the original
> definitions:
>   - As enums are converted to their integer representation during
>   parsing, the IR would not know how to distinguish an integer
>   constant from an actual enum value.
>   - Types need to be kept as temporary variables, as the existing type
>   casts of the 0 address (as expanded for LLVM), are optimized away by
>   the GCC C parser, never really reaching GCCs IR.
>
> Although, the macros appear to add extra complexity, the expanded code
> is removed from the compilation flow very early in the compilation
> process, not really affecting the quality of the generated assembly.
>
> Signed-off-by: Cupertino Miranda <[e-mail]>
> ---
>  tools/lib/bpf/bpf_core_read.h | 46 ++++++++++++++++++++++++++++++-----
>  1 file changed, 40 insertions(+), 6 deletions(-)
>
> diff --git a/tools/lib/bpf/bpf_core_read.h b/tools/lib/bpf/bpf_core_read.h
> index 0d3e88bd7d5f..074f1f4e4d2b 100644
> --- a/tools/lib/bpf/bpf_core_read.h
> +++ b/tools/lib/bpf/bpf_core_read.h
> @@ -81,6 +81,23 @@ enum bpf_enum_value_kind {
>         val;                                                                  \
>  })
>
> +/* Differentiator between compilers builtin implementations. This is a
> + * requirement due to the compiler parsing differences where GCC optimizes
> + * early in parsing those constructs of type pointers to the builtin specific
> + * type, resulting in not being possible to collect the required type
> + * information in the builtin expansion.
> + */
> +#ifdef __clang__
> +#define bpf_type_for_compiler(type) ((typeof(type) *) 0)

let's call it something with triple underscore and shorter at the same
time. ___bpf_typeof()?

> +#else
> +#define COMPOSE_VAR(t, s) t##s

we already define this as ___concat() in this file, let's reuse that one

> +#define bpf_type_for_compiler1(type, NR) ({ \
> +       extern  typeof(type) *COMPOSE_VAR(bpf_type_tmp_, NR); \

nite: double space

please also align '\' at the end to match the rest of this file

> +       COMPOSE_VAR(bpf_type_tmp_, NR); \
> +})
> +#define bpf_type_for_compiler(type) bpf_type_for_compiler1(type, __COUNTER__)
> +#endif
> +
>  /*
>   * Extract bitfield, identified by s->field, and return its value as u64.
>   * This version of macro is using direct memory reads and should be used from

[...]

>   * Convenience macro to check that provided enumerator value is defined in
> @@ -246,8 +268,14 @@ enum bpf_enum_value_kind {
>   *    kernel's BTF;
>   *    0, if no matching enum and/or enum value within that enum is found.
>   */
> +#ifdef __clang__
>  #define bpf_core_enum_value_exists(enum_type, enum_value)                  \
>         __builtin_preserve_enum_value(*(typeof(enum_type) *)enum_value, BPF_ENUMVAL_EXISTS)
> +#else
> +#define bpf_core_enum_value_exists(enum_type, enum_value)                  \
> +       __builtin_preserve_enum_value(bpf_type_for_compiler(enum_type), \
> +                                     enum_value, BPF_ENUMVAL_EXISTS)

with ___bpf_typeof() it should fit on one line

> +#endif
>
>  /*
>   * Convenience macro to get the integer value of an enumerator value in
> @@ -257,8 +285,14 @@ enum bpf_enum_value_kind {
>   *    present in target kernel's BTF;
>   *    0, if no matching enum and/or enum value within that enum is found.
>   */
> +#ifdef __clang__
>  #define bpf_core_enum_value(enum_type, enum_value)                         \
>         __builtin_preserve_enum_value(*(typeof(enum_type) *)enum_value, BPF_ENUMVAL_VALUE)
> +#else
> +#define bpf_core_enum_value(enum_type, enum_value)                         \
> +       __builtin_preserve_enum_value(bpf_type_for_compiler(enum_type), \
> +                                     enum_value, BPF_ENUMVAL_VALUE)
> +#endif
>
>  /*
>   * bpf_core_read() abstracts away bpf_probe_read_kernel() call and captures
> --
> 2.30.2
>

pw-bot: cr

Patch

diff --git a/tools/lib/bpf/bpf_core_read.h b/tools/lib/bpf/bpf_core_read.h
index 0d3e88bd7d5f..074f1f4e4d2b 100644
--- a/tools/lib/bpf/bpf_core_read.h
+++ b/tools/lib/bpf/bpf_core_read.h
@@ -81,6 +81,23 @@  enum bpf_enum_value_kind {
 	val;								      \
 })
 
+/* Differentiator between compilers builtin implementations. This is a
+ * requirement due to the compiler parsing differences where GCC optimizes
+ * early in parsing those constructs of type pointers to the builtin specific
+ * type, resulting in not being possible to collect the required type
+ * information in the builtin expansion.
+ */
+#ifdef __clang__
+#define bpf_type_for_compiler(type) ((typeof(type) *) 0)
+#else
+#define COMPOSE_VAR(t, s) t##s
+#define bpf_type_for_compiler1(type, NR) ({ \
+	extern  typeof(type) *COMPOSE_VAR(bpf_type_tmp_, NR); \
+	COMPOSE_VAR(bpf_type_tmp_, NR); \
+})
+#define bpf_type_for_compiler(type) bpf_type_for_compiler1(type, __COUNTER__)
+#endif
+
 /*
  * Extract bitfield, identified by s->field, and return its value as u64.
  * This version of macro is using direct memory reads and should be used from
@@ -145,8 +162,13 @@  enum bpf_enum_value_kind {
 	}								\
 })
 
+#ifdef __clang__
 #define ___bpf_field_ref1(field)	(field)
-#define ___bpf_field_ref2(type, field)	(((typeof(type) *)0)->field)
+#define ___bpf_field_ref2(type, field)	(bpf_type_for_compiler(type)->field)
+#else
+#define ___bpf_field_ref1(field)	(&(field))
+#define ___bpf_field_ref2(type, field)	(&(bpf_type_for_compiler(type)->field))
+#endif
 #define ___bpf_field_ref(args...)					    \
 	___bpf_apply(___bpf_field_ref, ___bpf_narg(args))(args)
 
@@ -196,7 +218,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(*bpf_type_for_compiler(type), BPF_TYPE_ID_LOCAL)
 
 /*
  * Convenience macro to get BTF type ID of a target kernel's type that matches
@@ -206,7 +228,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(*bpf_type_for_compiler(type), BPF_TYPE_ID_TARGET)
 
 /*
  * Convenience macro to check that provided named type
@@ -216,7 +238,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(*bpf_type_for_compiler(type), BPF_TYPE_EXISTS)
 
 /*
  * Convenience macro to check that provided named type
@@ -226,7 +248,7 @@  enum bpf_enum_value_kind {
  *    0, if the type does not match any in the target kernel
  */
 #define bpf_core_type_matches(type)					    \
-	__builtin_preserve_type_info(*(typeof(type) *)0, BPF_TYPE_MATCHES)
+	__builtin_preserve_type_info(*bpf_type_for_compiler(type), BPF_TYPE_MATCHES)
 
 /*
  * Convenience macro to get the byte size of a provided named type
@@ -236,7 +258,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(*bpf_type_for_compiler(type), BPF_TYPE_SIZE)
 
 /*
  * Convenience macro to check that provided enumerator value is defined in
@@ -246,8 +268,14 @@  enum bpf_enum_value_kind {
  *    kernel's BTF;
  *    0, if no matching enum and/or enum value within that enum is found.
  */
+#ifdef __clang__
 #define bpf_core_enum_value_exists(enum_type, enum_value)		    \
 	__builtin_preserve_enum_value(*(typeof(enum_type) *)enum_value, BPF_ENUMVAL_EXISTS)
+#else
+#define bpf_core_enum_value_exists(enum_type, enum_value)		    \
+	__builtin_preserve_enum_value(bpf_type_for_compiler(enum_type), \
+				      enum_value, BPF_ENUMVAL_EXISTS)
+#endif
 
 /*
  * Convenience macro to get the integer value of an enumerator value in
@@ -257,8 +285,14 @@  enum bpf_enum_value_kind {
  *    present in target kernel's BTF;
  *    0, if no matching enum and/or enum value within that enum is found.
  */
+#ifdef __clang__
 #define bpf_core_enum_value(enum_type, enum_value)			    \
 	__builtin_preserve_enum_value(*(typeof(enum_type) *)enum_value, BPF_ENUMVAL_VALUE)
+#else
+#define bpf_core_enum_value(enum_type, enum_value)			    \
+	__builtin_preserve_enum_value(bpf_type_for_compiler(enum_type), \
+				      enum_value, BPF_ENUMVAL_VALUE)
+#endif
 
 /*
  * bpf_core_read() abstracts away bpf_probe_read_kernel() call and captures