* [PATCH bpf-next] libbpf: fix compilation warnings for 64-bit printf args
@ 2020-08-26 3:09 Andrii Nakryiko
2020-08-26 16:08 ` Alexei Starovoitov
0 siblings, 1 reply; 3+ messages in thread
From: Andrii Nakryiko @ 2020-08-26 3:09 UTC (permalink / raw)
To: bpf, netdev, ast, daniel
Cc: andrii.nakryiko, kernel-team, Andrii Nakryiko, Naresh Kamboju
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 <naresh.kamboju@linaro.org>
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 <andriin@fb.com>
---
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;
+
#define __pr(level, fmt, ...) \
do { \
libbpf_print(level, "libbpf: " fmt, ##__VA_ARGS__); \
--
2.24.1
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH bpf-next] libbpf: fix compilation warnings for 64-bit printf args
2020-08-26 3:09 [PATCH bpf-next] libbpf: fix compilation warnings for 64-bit printf args Andrii Nakryiko
@ 2020-08-26 16:08 ` Alexei Starovoitov
2020-08-27 4:12 ` Andrii Nakryiko
0 siblings, 1 reply; 3+ messages in thread
From: Alexei Starovoitov @ 2020-08-26 16:08 UTC (permalink / raw)
To: Andrii Nakryiko
Cc: bpf, netdev, ast, daniel, andrii.nakryiko, kernel-team, Naresh Kamboju
On Tue, Aug 25, 2020 at 08:09:21PM -0700, Andrii Nakryiko wrote:
> 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 <naresh.kamboju@linaro.org>
> 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 <andriin@fb.com>
> ---
> 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.
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH bpf-next] libbpf: fix compilation warnings for 64-bit printf args
2020-08-26 16:08 ` Alexei Starovoitov
@ 2020-08-27 4:12 ` Andrii Nakryiko
0 siblings, 0 replies; 3+ messages in thread
From: Andrii Nakryiko @ 2020-08-27 4:12 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: Andrii Nakryiko, bpf, Networking, Alexei Starovoitov,
Daniel Borkmann, Kernel Team, Naresh Kamboju
On Wed, Aug 26, 2020 at 9:08 AM Alexei Starovoitov
<alexei.starovoitov@gmail.com> wrote:
>
> On Tue, Aug 25, 2020 at 08:09:21PM -0700, Andrii Nakryiko wrote:
> > 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 <naresh.kamboju@linaro.org>
> > 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 <andriin@fb.com>
> > ---
> > tools/lib/bpf/libbpf.c | 15 ++++++++-------
> > tools/lib/bpf/libbpf_internal.h | 11 +++++++++++
> > 2 files changed, 19 insertions(+), 7 deletions(-)
> >
[...]
> >
> > +/* 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.
Fair enough. Sent v2 with just direct "unsigned long long" casts.
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2020-08-27 4:12 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-08-26 3:09 [PATCH bpf-next] libbpf: fix compilation warnings for 64-bit printf args Andrii Nakryiko
2020-08-26 16:08 ` Alexei Starovoitov
2020-08-27 4:12 ` Andrii Nakryiko
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.