diff mbox series

libbpf: Use PRIu64 for sym->st_value to fix build on 32-bit arches

Message ID 20191127134553.GC22719@kernel.org
State Changes Requested
Delegated to: BPF Maintainers
Headers show
Series libbpf: Use PRIu64 for sym->st_value to fix build on 32-bit arches | expand

Commit Message

Arnaldo Nov. 27, 2019, 1:45 p.m. UTC
Another fix I'm carrying in my perf/core branch,

Regards,

- Arnaldo

commit 98bb09f90a0ae33125fabc8f41529345382f1498
Author: Arnaldo Carvalho de Melo <acme@redhat.com>
Date:   Wed Nov 27 09:26:54 2019 -0300

    libbpf: Use PRIu64 for sym->st_value to fix build on 32-bit arches
    
    The st_value field is a 64-bit value, so use PRIu64 to fix this error on
    32-bit arches:
    
      In file included from libbpf.c:52:
      libbpf.c: In function 'bpf_program__record_reloc':
      libbpf_internal.h:59:22: error: format '%lu' expects argument of type 'long unsigned int', but argument 3 has type 'Elf64_Addr' {aka 'const long long unsigned int'} [-Werror=format=]
        libbpf_print(level, "libbpf: " fmt, ##__VA_ARGS__); \
                            ^~~~~~~~~~
      libbpf_internal.h:62:27: note: in expansion of macro '__pr'
       #define pr_warn(fmt, ...) __pr(LIBBPF_WARN, fmt, ##__VA_ARGS__)
                                 ^~~~
      libbpf.c:1822:4: note: in expansion of macro 'pr_warn'
          pr_warn("bad call relo offset: %lu\n", sym->st_value);
          ^~~~~~~
      libbpf.c:1822:37: note: format string is defined here
          pr_warn("bad call relo offset: %lu\n", sym->st_value);
                                         ~~^
                                         %llu
    
    Fixes: 1f8e2bcb2cd5 ("libbpf: Refactor relocation handling")
    Cc: Alexei Starovoitov <ast@kernel.org>
    Cc: Andrii Nakryiko <andriin@fb.com>
    Link: https://lkml.kernel.org/n/tip-iabs1wq19c357bkk84p7blif@git.kernel.org
    Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>

Comments

Alexei Starovoitov Nov. 27, 2019, 4:39 p.m. UTC | #1
On Wed, Nov 27, 2019 at 5:45 AM Arnaldo Carvalho de Melo
<acme@kernel.org> wrote:
>
> Another fix I'm carrying in my perf/core branch,

Why in perf/core?
I very much prefer all libbpf patches to go via normal route via bpf/net trees.
We had enough conflicts in this merge window. Let's avoid them.
Arnaldo Nov. 27, 2019, 6:45 p.m. UTC | #2
Em Wed, Nov 27, 2019 at 08:39:28AM -0800, Alexei Starovoitov escreveu:
> On Wed, Nov 27, 2019 at 5:45 AM Arnaldo Carvalho de Melo
> <acme@kernel.org> wrote:
> >
> > Another fix I'm carrying in my perf/core branch,
 
> Why in perf/core?
> I very much prefer all libbpf patches to go via normal route via bpf/net trees.
> We had enough conflicts in this merge window. Let's avoid them.

Humm, if we both carry the same patch the merge process can do its magic
and nobody gets hurt? Besides these are really minor things, no?

- Arnaldo
Alexei Starovoitov Nov. 27, 2019, 6:55 p.m. UTC | #3
On Wed, Nov 27, 2019 at 10:45 AM Arnaldo Carvalho de Melo
<acme@kernel.org> wrote:
>
> Em Wed, Nov 27, 2019 at 08:39:28AM -0800, Alexei Starovoitov escreveu:
> > On Wed, Nov 27, 2019 at 5:45 AM Arnaldo Carvalho de Melo
> > <acme@kernel.org> wrote:
> > >
> > > Another fix I'm carrying in my perf/core branch,
>
> > Why in perf/core?
> > I very much prefer all libbpf patches to go via normal route via bpf/net trees.
> > We had enough conflicts in this merge window. Let's avoid them.
>
> Humm, if we both carry the same patch the merge process can do its magic
> and nobody gets hurt? Besides these are really minor things, no?

I thought so too, but learned the hard lesson recently.
We should try to avoid that as much as possible.
Andrii's is fixing stuff in the same lines:
https://patchwork.ozlabs.org/patch/1201344/
these two patches will likely conflict. I'd rather have them both in bpf tree.
What is the value for this patch in perf tree?
To fix the build on 32-bit arches, right?
But how urgent is it? Can you wait few days until this one and other
libbpf fixes
land via bpf/net trees?
Alexei Starovoitov Nov. 27, 2019, 7:33 p.m. UTC | #4
On Wed, Nov 27, 2019 at 5:45 AM Arnaldo Carvalho de Melo
<acme@kernel.org> wrote:
>
> Another fix I'm carrying in my perf/core branch,
>
> Regards,
>
> - Arnaldo
>
> commit 98bb09f90a0ae33125fabc8f41529345382f1498
> Author: Arnaldo Carvalho de Melo <acme@redhat.com>
> Date:   Wed Nov 27 09:26:54 2019 -0300
>
>     libbpf: Use PRIu64 for sym->st_value to fix build on 32-bit arches
>
>     The st_value field is a 64-bit value, so use PRIu64 to fix this error on
>     32-bit arches:
>
>       In file included from libbpf.c:52:
>       libbpf.c: In function 'bpf_program__record_reloc':
>       libbpf_internal.h:59:22: error: format '%lu' expects argument of type 'long unsigned int', but argument 3 has type 'Elf64_Addr' {aka 'const long long unsigned int'} [-Werror=format=]
>         libbpf_print(level, "libbpf: " fmt, ##__VA_ARGS__); \
>                             ^~~~~~~~~~
>       libbpf_internal.h:62:27: note: in expansion of macro '__pr'
>        #define pr_warn(fmt, ...) __pr(LIBBPF_WARN, fmt, ##__VA_ARGS__)
>                                  ^~~~
>       libbpf.c:1822:4: note: in expansion of macro 'pr_warn'
>           pr_warn("bad call relo offset: %lu\n", sym->st_value);
>           ^~~~~~~
>       libbpf.c:1822:37: note: format string is defined here
>           pr_warn("bad call relo offset: %lu\n", sym->st_value);
>                                          ~~^
>                                          %llu
>
>     Fixes: 1f8e2bcb2cd5 ("libbpf: Refactor relocation handling")
>     Cc: Alexei Starovoitov <ast@kernel.org>
>     Cc: Andrii Nakryiko <andriin@fb.com>
>     Link: https://lkml.kernel.org/n/tip-iabs1wq19c357bkk84p7blif@git.kernel.org
>     Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
>
> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> index b20f82e58989..6b0eae5c8a94 100644
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c
> @@ -1819,7 +1819,7 @@ static int bpf_program__record_reloc(struct bpf_program *prog,
>                         return -LIBBPF_ERRNO__RELOC;
>                 }
>                 if (sym->st_value % 8) {
> -                       pr_warn("bad call relo offset: %lu\n", sym->st_value);
> +                       pr_warn("bad call relo offset: %" PRIu64 "\n", sym->st_value);

Looking at this more... I never liked this PRI stuff. It makes for
such unreadable code.
How about just typecasting st_value to (long) ?
Arnaldo Nov. 27, 2019, 7:39 p.m. UTC | #5
Em Wed, Nov 27, 2019 at 10:55:31AM -0800, Alexei Starovoitov escreveu:
> On Wed, Nov 27, 2019 at 10:45 AM Arnaldo Carvalho de Melo
> <acme@kernel.org> wrote:
> >
> > Em Wed, Nov 27, 2019 at 08:39:28AM -0800, Alexei Starovoitov escreveu:
> > > On Wed, Nov 27, 2019 at 5:45 AM Arnaldo Carvalho de Melo
> > > <acme@kernel.org> wrote:
> > > >
> > > > Another fix I'm carrying in my perf/core branch,
> >
> > > Why in perf/core?
> > > I very much prefer all libbpf patches to go via normal route via bpf/net trees.
> > > We had enough conflicts in this merge window. Let's avoid them.
> >
> > Humm, if we both carry the same patch the merge process can do its magic
> > and nobody gets hurt? Besides these are really minor things, no?
> 
> I thought so too, but learned the hard lesson recently.
> We should try to avoid that as much as possible.
> Andrii's is fixing stuff in the same lines:
> https://patchwork.ozlabs.org/patch/1201344/
> these two patches will likely conflict. I'd rather have them both in bpf tree.
> What is the value for this patch in perf tree?
> To fix the build on 32-bit arches, right?
> But how urgent is it? Can you wait few days until this one and other
> libbpf fixes
> land via bpf/net trees?

Ok, I'll add a note to the pull request report about where the perf
build is clean in all containers because I added these two patches, but
that they'll go via the bpf tree, as soon as that gets merged, the
problem will go away.

And I wasn't strictly defending that I should carry this in perf/core,
just said I was, to fix something minor that I found while doing my
usual testing, patch was posted, you got notified and got the patch,
I'll remove it from perf/core now since you stated that it'll eventually
land upstream.

Thanks,

- Arnaldo
Naresh Kamboju Dec. 3, 2019, 1:50 p.m. UTC | #6
Hi Arnaldo,

FYI,

On Wed, 27 Nov 2019 at 19:15, Arnaldo Carvalho de Melo
<arnaldo.melo@gmail.com> wrote:
>
> Another fix I'm carrying in my perf/core branch,
>
> Regards,
>
> - Arnaldo
>
> commit 98bb09f90a0ae33125fabc8f41529345382f1498
> Author: Arnaldo Carvalho de Melo <acme@redhat.com>
> Date:   Wed Nov 27 09:26:54 2019 -0300
>
>     libbpf: Use PRIu64 for sym->st_value to fix build on 32-bit arches
>
>     The st_value field is a 64-bit value, so use PRIu64 to fix this error on
>     32-bit arches:
>
>       In file included from libbpf.c:52:
>       libbpf.c: In function 'bpf_program__record_reloc':
>       libbpf_internal.h:59:22: error: format '%lu' expects argument of type 'long unsigned int', but argument 3 has type 'Elf64_Addr' {aka 'const long long unsigned int'} [-Werror=format=]
>         libbpf_print(level, "libbpf: " fmt, ##__VA_ARGS__); \
>                             ^~~~~~~~~~
>       libbpf_internal.h:62:27: note: in expansion of macro '__pr'
>        #define pr_warn(fmt, ...) __pr(LIBBPF_WARN, fmt, ##__VA_ARGS__)
>                                  ^~~~
>       libbpf.c:1822:4: note: in expansion of macro 'pr_warn'
>           pr_warn("bad call relo offset: %lu\n", sym->st_value);
>           ^~~~~~~
>       libbpf.c:1822:37: note: format string is defined here
>           pr_warn("bad call relo offset: %lu\n", sym->st_value);
>                                          ~~^
>                                          %llu

This build error is been noticed on Linux mainline kernel for 32-bit
architectures from Nov 26.

Full build log,
https://ci.linaro.org/job/openembedded-lkft-linux-mainline/DISTRO=lkft,MACHINE=intel-core2-32,label=docker-lkft/2297/consoleText
https://ci.linaro.org/job/openembedded-lkft-linux-mainline/

- Naresh
Arnaldo Dec. 3, 2019, 2:41 p.m. UTC | #7
Em Tue, Dec 03, 2019 at 07:20:08PM +0530, Naresh Kamboju escreveu:
> Hi Arnaldo,
> 
> FYI,
> 
> On Wed, 27 Nov 2019 at 19:15, Arnaldo Carvalho de Melo
> <arnaldo.melo@gmail.com> wrote:
> >
> > Another fix I'm carrying in my perf/core branch,
> >
> > Regards,
> >
> > - Arnaldo
> >
> > commit 98bb09f90a0ae33125fabc8f41529345382f1498
> > Author: Arnaldo Carvalho de Melo <acme@redhat.com>
> > Date:   Wed Nov 27 09:26:54 2019 -0300
> >
> >     libbpf: Use PRIu64 for sym->st_value to fix build on 32-bit arches
> >
> >     The st_value field is a 64-bit value, so use PRIu64 to fix this error on
> >     32-bit arches:
> >
> >       In file included from libbpf.c:52:
> >       libbpf.c: In function 'bpf_program__record_reloc':
> >       libbpf_internal.h:59:22: error: format '%lu' expects argument of type 'long unsigned int', but argument 3 has type 'Elf64_Addr' {aka 'const long long unsigned int'} [-Werror=format=]
> >         libbpf_print(level, "libbpf: " fmt, ##__VA_ARGS__); \
> >                             ^~~~~~~~~~
> >       libbpf_internal.h:62:27: note: in expansion of macro '__pr'
> >        #define pr_warn(fmt, ...) __pr(LIBBPF_WARN, fmt, ##__VA_ARGS__)
> >                                  ^~~~
> >       libbpf.c:1822:4: note: in expansion of macro 'pr_warn'
> >           pr_warn("bad call relo offset: %lu\n", sym->st_value);
> >           ^~~~~~~
> >       libbpf.c:1822:37: note: format string is defined here
> >           pr_warn("bad call relo offset: %lu\n", sym->st_value);
> >                                          ~~^
> >                                          %llu
> 
> This build error is been noticed on Linux mainline kernel for 32-bit
> architectures from Nov 26.

Right, the fix is in the bpf tree:

https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf.git/commit/?id=7c3977d1e804

Should go upstream soon.

- Arnaldo
 
> Full build log,
> https://ci.linaro.org/job/openembedded-lkft-linux-mainline/DISTRO=lkft,MACHINE=intel-core2-32,label=docker-lkft/2297/consoleText
> https://ci.linaro.org/job/openembedded-lkft-linux-mainline/
> 
> - Naresh
diff mbox series

Patch

diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index b20f82e58989..6b0eae5c8a94 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -1819,7 +1819,7 @@  static int bpf_program__record_reloc(struct bpf_program *prog,
 			return -LIBBPF_ERRNO__RELOC;
 		}
 		if (sym->st_value % 8) {
-			pr_warn("bad call relo offset: %lu\n", sym->st_value);
+			pr_warn("bad call relo offset: %" PRIu64 "\n", sym->st_value);
 			return -LIBBPF_ERRNO__RELOC;
 		}
 		reloc_desc->type = RELO_CALL;