Thread (3 messages) 3 messages, 3 authors, 2020-08-27

Re: [PATCH bpf-next] libbpf: fix compilation warnings for 64-bit printf args

From: Alexei Starovoitov <hidden>
Date: 2020-08-26 16:09:00
Also in: bpf

On Tue, Aug 25, 2020 at 08:09:21PM -0700, Andrii Nakryiko wrote:
quoted hunk ↗ jump to hunk
Add __pu64 and __ps64 (sort of like "printf u64 and s64") for libbpf-internal
use only in printf-like situations to avoid compilation warnings due to
%lld/%llu mismatch with a __u64/__s64 due to some architecture defining the
latter as either `long` or `long long`. Use that on all %lld/%llu cases in
libbpf.c.

Reported-by: Naresh Kamboju <redacted>
Fixes: eacaaed784e2 ("libbpf: Implement enum value-based CO-RE relocations")
Fixes: 50e09460d9f8 ("libbpf: Skip well-known ELF sections when iterating ELF")
Signed-off-by: Andrii Nakryiko <redacted>
---
 tools/lib/bpf/libbpf.c          | 15 ++++++++-------
 tools/lib/bpf/libbpf_internal.h | 11 +++++++++++
 2 files changed, 19 insertions(+), 7 deletions(-)
diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index 2e2523d8bb6d..211eb0d9020c 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -1529,12 +1529,12 @@ static int set_kcfg_value_num(struct extern_desc *ext, void *ext_val,
 {
 	if (ext->kcfg.type != KCFG_INT && ext->kcfg.type != KCFG_CHAR) {
 		pr_warn("extern (kcfg) %s=%llu should be integer\n",
-			ext->name, (unsigned long long)value);
+			ext->name, (__pu64)value);
 		return -EINVAL;
 	}
 	if (!is_kcfg_value_in_range(ext, value)) {
 		pr_warn("extern (kcfg) %s=%llu value doesn't fit in %d bytes\n",
-			ext->name, (unsigned long long)value, ext->kcfg.sz);
+			ext->name, (__pu64)value, ext->kcfg.sz);
 		return -ERANGE;
 	}
 	switch (ext->kcfg.sz) {
@@ -2823,7 +2823,8 @@ static int bpf_object__elf_collect(struct bpf_object *obj)
 			obj->efile.bss = data;
 			obj->efile.bss_shndx = idx;
 		} else {
-			pr_info("elf: skipping section(%d) %s (size %zu)\n", idx, name, sh.sh_size);
+			pr_info("elf: skipping section(%d) %s (size %zu)\n", idx, name,
+				(size_t)sh.sh_size);
 		}
 	}
 
@@ -5244,7 +5245,7 @@ static int bpf_core_patch_insn(struct bpf_program *prog,
 		if (res->validate && imm != orig_val) {
 			pr_warn("prog '%s': relo #%d: unexpected insn #%d (LDIMM64) value: got %llu, exp %u -> %u\n",
 				bpf_program__title(prog, false), relo_idx,
-				insn_idx, imm, orig_val, new_val);
+				insn_idx, (__pu64)imm, orig_val, new_val);
 			return -EINVAL;
 		}
 
@@ -5252,7 +5253,7 @@ static int bpf_core_patch_insn(struct bpf_program *prog,
 		insn[1].imm = 0; /* currently only 32-bit values are supported */
 		pr_debug("prog '%s': relo #%d: patched insn #%d (LDIMM64) imm64 %llu -> %u\n",
 			 bpf_program__title(prog, false), relo_idx, insn_idx,
-			 imm, new_val);
+			 (__pu64)imm, new_val);
 		break;
 	}
 	default:
@@ -7782,8 +7783,8 @@ static int bpf_object__collect_st_ops_relos(struct bpf_object *obj,
 		st_ops = map->st_ops;
 		pr_debug("struct_ops reloc %s: for %lld value %lld shdr_idx %u rel.r_offset %zu map->sec_offset %zu name %d (\'%s\')\n",
 			 map->name,
-			 (long long)(rel.r_info >> 32),
-			 (long long)sym.st_value,
+			 (__ps64)(rel.r_info >> 32),
+			 (__ps64)sym.st_value,
 			 shdr_idx, (size_t)rel.r_offset,
 			 map->sec_offset, sym.st_name, name);
 
diff --git a/tools/lib/bpf/libbpf_internal.h b/tools/lib/bpf/libbpf_internal.h
index 4d1c366fca2c..7ad3c4b9917c 100644
--- a/tools/lib/bpf/libbpf_internal.h
+++ b/tools/lib/bpf/libbpf_internal.h
@@ -69,6 +69,17 @@ extern void libbpf_print(enum libbpf_print_level level,
 			 const char *format, ...)
 	__attribute__((format(printf, 2, 3)));
 
+/* These types are for casting 64-bit arguments of printf-like functions to
+ * avoid compiler warnings on various architectures that define size_t, __u64,
+ * uint64_t, etc as either unsigned long or unsigned long long (similarly for
+ * signed variants). Use these typedefs only for these purposes. Alternative
+ * is PRIu64 (and similar) macros, requiring stitching printf format strings
+ * which are extremely ugly and should be avoided in libbpf code base. With
+ * arguments casted to __pu64/__ps64, always use %llu/%lld in format string.
+ */
+typedef unsigned long long __pu64;
+typedef long long __ps64;
I think these extra typedefs will cause confusion. Original approach
of open coding type casts to long long and unsigned long long is imo cleaner.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help