diff mbox series

[dwarves,v4,2/6] btf_encoder: use __weak declarations of version-dependent libbpf API

Message ID 20250228194654.1022535-3-ihor.solodrai@linux.dev (mailing list archive)
State Not Applicable
Headers show
Series btf_encoder: emit type tags for bpf_arena pointers | expand

Checks

Context Check Description
netdev/tree_selection success Guessing tree name failed - patch did not apply

Commit Message

Ihor Solodrai Feb. 28, 2025, 7:46 p.m. UTC 
Instead of compile-time checks for libbpf version, use __weak
declarations of the required API functions and do runtime checks at
the call sites. This will help with compatibility when libbpf is
dynamically linked to pahole [1].

[1] https://lore.kernel.org/dwarves/deff78f8-1f99-4c79-a302-cff8dce4d803@oracle.com/

Signed-off-by: Ihor Solodrai <ihor.solodrai@linux.dev>
---
 btf_encoder.c | 48 +++++++++++++++++++-----------------------------
 dwarves.h     | 11 ++++++++++-
 pahole.c      |  2 --
 3 files changed, 29 insertions(+), 32 deletions(-)

Comments

Ihor Solodrai Feb. 28, 2025, 7:53 p.m. UTC | #1 
On 2/28/25 11:46 AM, Ihor Solodrai wrote:
> Instead of compile-time checks for libbpf version, use __weak
> declarations of the required API functions and do runtime checks at
> the call sites. This will help with compatibility when libbpf is
> dynamically linked to pahole [1].
>
> [1] https://lore.kernel.org/dwarves/deff78f8-1f99-4c79-a302-cff8dce4d803@oracle.com/
>
> Signed-off-by: Ihor Solodrai <ihor.solodrai@linux.dev>
> ---
>  btf_encoder.c | 48 +++++++++++++++++++-----------------------------
>  dwarves.h     | 11 ++++++++++-
>  pahole.c      |  2 --
>  3 files changed, 29 insertions(+), 32 deletions(-)
>
> diff --git a/btf_encoder.c b/btf_encoder.c
> index 2bea5ee..12a040f 100644
> --- a/btf_encoder.c
> +++ b/btf_encoder.c
> @@ -34,6 +34,7 @@
>  #include <search.h> /* for tsearch(), tfind() and tdestroy() */
>  #include <pthread.h>
>  
> +#define BTF_BASE_ELF_SEC	".BTF.base"
>  #define BTF_IDS_SECTION		".BTF_ids"
>  #define BTF_ID_FUNC_PFX		"__BTF_ID__func__"
>  #define BTF_ID_SET8_PFX		"__BTF_ID__set8__"
> @@ -625,29 +626,6 @@ static int32_t btf_encoder__add_struct(struct btf_encoder *encoder, uint8_t kind
>  	return id;
>  }
>  
> -#if LIBBPF_MAJOR_VERSION < 1

There is an identical condition in btf_loader.c, however it guards
static functions, for example btf_enum64(). I decided to leave it as
is, although I find it unlikely that someone would use libbpf < 1.0.

> [...]
Alan Maguire March 6, 2025, 5:14 p.m. UTC | #2 
On 28/02/2025 19:53, Ihor Solodrai wrote:
> On 2/28/25 11:46 AM, Ihor Solodrai wrote:
>> Instead of compile-time checks for libbpf version, use __weak
>> declarations of the required API functions and do runtime checks at
>> the call sites. This will help with compatibility when libbpf is
>> dynamically linked to pahole [1].
>>
>> [1] https://lore.kernel.org/dwarves/deff78f8-1f99-4c79-a302-cff8dce4d803@oracle.com/
>>
>> Signed-off-by: Ihor Solodrai <ihor.solodrai@linux.dev>
>> ---
>>  btf_encoder.c | 48 +++++++++++++++++++-----------------------------
>>  dwarves.h     | 11 ++++++++++-
>>  pahole.c      |  2 --
>>  3 files changed, 29 insertions(+), 32 deletions(-)
>>
>> diff --git a/btf_encoder.c b/btf_encoder.c
>> index 2bea5ee..12a040f 100644
>> --- a/btf_encoder.c
>> +++ b/btf_encoder.c
>> @@ -34,6 +34,7 @@
>>  #include <search.h> /* for tsearch(), tfind() and tdestroy() */
>>  #include <pthread.h>
>>  
>> +#define BTF_BASE_ELF_SEC	".BTF.base"
>>  #define BTF_IDS_SECTION		".BTF_ids"
>>  #define BTF_ID_FUNC_PFX		"__BTF_ID__func__"
>>  #define BTF_ID_SET8_PFX		"__BTF_ID__set8__"
>> @@ -625,29 +626,6 @@ static int32_t btf_encoder__add_struct(struct btf_encoder *encoder, uint8_t kind
>>  	return id;
>>  }
>>  
>> -#if LIBBPF_MAJOR_VERSION < 1
> 
> There is an identical condition in btf_loader.c, however it guards
> static functions, for example btf_enum64(). I decided to leave it as
> is, although I find it unlikely that someone would use libbpf < 1.0.
> 
>> [...]

yeah, I just noticed we've got a minimum version requirement of 0.4:


                pkg_check_modules(LIBBPF REQUIRED libbpf>=0.4.0)

That needs to be revisited in the future to be > v1.0 I would say. To
test libbpf shared library support, I tried building with libbpf v1.2,
but hit errors around absence of struct btf_enum64. Looks like these
compilation errors resulted from not having an up-to-date
/usr/include/linux/btf.h (IIRC the libbpf repo ships with a copy, maybe
we should do the same?). The errors are not caused by your series, so
not something you need to worry about, but more work is needed to better
support shared library builds even for libbpf > 1.0 it seems. Thanks!

Alan
diff mbox series

Patch

diff --git a/btf_encoder.c b/btf_encoder.c
index 2bea5ee..12a040f 100644
--- a/btf_encoder.c
+++ b/btf_encoder.c
@@ -34,6 +34,7 @@ 
 #include <search.h> /* for tsearch(), tfind() and tdestroy() */
 #include <pthread.h>
 
+#define BTF_BASE_ELF_SEC	".BTF.base"
 #define BTF_IDS_SECTION		".BTF_ids"
 #define BTF_ID_FUNC_PFX		"__BTF_ID__func__"
 #define BTF_ID_SET8_PFX		"__BTF_ID__set8__"
@@ -625,29 +626,6 @@  static int32_t btf_encoder__add_struct(struct btf_encoder *encoder, uint8_t kind
 	return id;
 }
 
-#if LIBBPF_MAJOR_VERSION < 1
-static inline int libbpf_err(int ret)
-{
-        if (ret < 0)
-                errno = -ret;
-        return ret;
-}
-
-static
-int btf__add_enum64(struct btf *btf __maybe_unused, const char *name __maybe_unused,
-		    __u32 byte_sz __maybe_unused, bool is_signed __maybe_unused)
-{
-	return  libbpf_err(-ENOTSUP);
-}
-
-static
-int btf__add_enum64_value(struct btf *btf __maybe_unused, const char *name __maybe_unused,
-			  __u64 value __maybe_unused)
-{
-	return  libbpf_err(-ENOTSUP);
-}
-#endif
-
 static int32_t btf_encoder__add_enum(struct btf_encoder *encoder, const char *name, struct type *etype,
 				     struct conf_load *conf_load)
 {
@@ -660,8 +638,13 @@  static int32_t btf_encoder__add_enum(struct btf_encoder *encoder, const char *na
 	is_enum32 = size <= 4 || conf_load->skip_encoding_btf_enum64;
 	if (is_enum32)
 		id = btf__add_enum(btf, name, size);
-	else
+	else if (btf__add_enum64)
 		id = btf__add_enum64(btf, name, size, etype->is_signed_enum);
+	else {
+		fprintf(stderr, "btf__add_enum64 is not available, is libbpf < 1.0?\n");
+		return -ENOTSUP;
+	}
+
 	if (id > 0) {
 		t = btf__type_by_id(btf, id);
 		btf_encoder__log_type(encoder, t, false, true, "size=%u", t->size);
@@ -684,10 +667,14 @@  static int btf_encoder__add_enum_val(struct btf_encoder *encoder, const char *na
 	 */
 	if (conf_load->skip_encoding_btf_enum64)
 		err = btf__add_enum_value(encoder->btf, name, (uint32_t)value);
-	else if (etype->size > 32)
-		err = btf__add_enum64_value(encoder->btf, name, value);
-	else
+	else if (etype->size <= 32)
 		err = btf__add_enum_value(encoder->btf, name, value);
+	else if (btf__add_enum64_value)
+		err = btf__add_enum64_value(encoder->btf, name, value);
+	else {
+		fprintf(stderr, "btf__add_enum64_value is not available, is libbpf < 1.0?\n");
+		return -ENOTSUP;
+	}
 
 	if (!err) {
 		if (encoder->verbose) {
@@ -2035,10 +2022,14 @@  int btf_encoder__encode(struct btf_encoder *encoder, struct conf_load *conf)
 		 * silently ignore the feature request if libbpf does not
 		 * support it.
 		 */
-#if LIBBPF_MAJOR_VERSION >= 1 && LIBBPF_MINOR_VERSION >= 5
 		if (encoder->gen_distilled_base) {
 			struct btf *btf = NULL, *distilled_base = NULL;
 
+			if (!btf__distill_base) {
+				fprintf(stderr, "btf__distill_base is not available, is libbpf < 1.5?\n");
+				return -ENOTSUP;
+			}
+
 			if (btf__distill_base(encoder->btf, &distilled_base, &btf) < 0) {
 				fprintf(stderr, "could not generate distilled base BTF: %s\n",
 					strerror(errno));
@@ -2051,7 +2042,6 @@  int btf_encoder__encode(struct btf_encoder *encoder, struct conf_load *conf)
 			btf__free(distilled_base);
 			return err;
 		}
-#endif
 		err = btf_encoder__write_elf(encoder, encoder->btf, BTF_ELF_SEC);
 	}
 
diff --git a/dwarves.h b/dwarves.h
index 8234e1a..ab32204 100644
--- a/dwarves.h
+++ b/dwarves.h
@@ -14,6 +14,7 @@ 
 #include <obstack.h>
 #include <dwarf.h>
 #include <elfutils/libdwfl.h>
+#include <linux/types.h>
 #include <sys/types.h>
 
 #include "dutil.h"
@@ -44,13 +45,21 @@  enum load_steal_kind {
 	LSK__STOP_LOADING,
 };
 
+/*
+ * Weak declarations of libbpf APIs that are version-dependent
+ */
+#define __weak __attribute__((weak))
+struct btf;
+__weak extern int btf__add_enum64(struct btf *btf, const char *name, __u32 byte_sz, bool is_signed);
+__weak extern int btf__add_enum64_value(struct btf *btf, const char *name, __u64 value);
+__weak extern int btf__distill_base(const struct btf *src_btf, struct btf **new_base_btf, struct btf **new_split_btf);
+
 /*
  * BTF combines all the types into one big CU using btf_dedup(), so for something
  * like a allyesconfig vmlinux kernel we can get over 65535 types.
  */
 typedef uint32_t type_id_t;
 
-struct btf;
 struct conf_fprintf;
 
 /** struct conf_load - load configuration
diff --git a/pahole.c b/pahole.c
index af3e1cf..09aed1c 100644
--- a/pahole.c
+++ b/pahole.c
@@ -1206,9 +1206,7 @@  struct btf_feature {
 	BTF_DEFAULT_FEATURE(consistent_func, skip_encoding_btf_inconsistent_proto, false),
 	BTF_DEFAULT_FEATURE(decl_tag_kfuncs, btf_decl_tag_kfuncs, false),
 	BTF_NON_DEFAULT_FEATURE(reproducible_build, reproducible_build, false),
-#if LIBBPF_MAJOR_VERSION >= 1 && LIBBPF_MINOR_VERSION >= 5
 	BTF_NON_DEFAULT_FEATURE(distilled_base, btf_gen_distilled_base, false),
-#endif
 	BTF_NON_DEFAULT_FEATURE(global_var, encode_btf_global_vars, false),
 };