diff mbox series

[bpf-next] tools/resolve_btfids: fix setting HOSTCFLAGS

Message ID 20230530123352.1308488-1-vmalik@redhat.com (mailing list archive)
State Accepted
Commit edd75c802855271c8610f58a2fc9e54aefc49ce5
Delegated to: BPF
Headers show
Series [bpf-next] tools/resolve_btfids: fix setting HOSTCFLAGS | expand

Checks

Context Check Description
netdev/series_format success Single patches do not need cover letters
netdev/tree_selection success Clearly marked for bpf-next
netdev/fixes_present success Fixes tag not required for -next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 8 this patch: 8
netdev/cc_maintainers success CCed 13 of 13 maintainers
netdev/build_clang success Errors and warnings before: 8 this patch: 8
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success Fixes tag looks correct
netdev/build_allmodconfig_warn success Errors and warnings before: 8 this patch: 8
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 16 lines checked
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0
bpf/vmtest-bpf-next-PR success PR summary
bpf/vmtest-bpf-next-VM_Test-1 success Logs for ShellCheck
bpf/vmtest-bpf-next-VM_Test-6 success Logs for set-matrix
bpf/vmtest-bpf-next-VM_Test-2 success Logs for build for aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-4 success Logs for build for x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-5 success Logs for build for x86_64 with llvm-16
bpf/vmtest-bpf-next-VM_Test-3 success Logs for build for s390x with gcc
bpf/vmtest-bpf-next-VM_Test-8 pending Logs for test_maps on s390x with gcc
bpf/vmtest-bpf-next-VM_Test-16 pending Logs for test_progs_no_alu32 on s390x with gcc
bpf/vmtest-bpf-next-VM_Test-7 success Logs for test_maps on aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-15 success Logs for test_progs_no_alu32 on aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-19 success Logs for test_progs_no_alu32_parallel on aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-22 success Logs for test_progs_parallel on aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-25 success Logs for test_verifier on aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-9 success Logs for test_maps on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-10 success Logs for test_maps on x86_64 with llvm-16
bpf/vmtest-bpf-next-VM_Test-11 success Logs for test_progs on aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-17 success Logs for test_progs_no_alu32 on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-20 success Logs for test_progs_no_alu32_parallel on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-21 success Logs for test_progs_no_alu32_parallel on x86_64 with llvm-16
bpf/vmtest-bpf-next-VM_Test-23 success Logs for test_progs_parallel on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-24 success Logs for test_progs_parallel on x86_64 with llvm-16
bpf/vmtest-bpf-next-VM_Test-27 success Logs for test_verifier on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-28 success Logs for test_verifier on x86_64 with llvm-16
bpf/vmtest-bpf-next-VM_Test-29 success Logs for veristat
bpf/vmtest-bpf-next-VM_Test-13 success Logs for test_progs on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-14 success Logs for test_progs on x86_64 with llvm-16
bpf/vmtest-bpf-next-VM_Test-18 success Logs for test_progs_no_alu32 on x86_64 with llvm-16
bpf/vmtest-bpf-next-VM_Test-26 success Logs for test_verifier on s390x with gcc
bpf/vmtest-bpf-next-VM_Test-12 fail Logs for test_progs on s390x with gcc

Commit Message

Viktor Malik May 30, 2023, 12:33 p.m. UTC
Building BPF selftests with custom HOSTCFLAGS yields an error:

    # make HOSTCFLAGS="-O2"
    [...]
      HOSTCC  ./tools/testing/selftests/bpf/tools/build/resolve_btfids/main.o
    main.c:73:10: fatal error: linux/rbtree.h: No such file or directory
       73 | #include <linux/rbtree.h>
          |          ^~~~~~~~~~~~~~~~

The reason is that tools/bpf/resolve_btfids/Makefile passes header
include paths by extending HOSTCFLAGS which is overridden by setting
HOSTCFLAGS in the make command (because of Makefile rules [1]).

This patch fixes the above problem by passing the include paths via
`HOSTCFLAGS_resolve_btfids` which is used by tools/build/Build.include
and can be combined with overridding HOSTCFLAGS.

[1] https://www.gnu.org/software/make/manual/html_node/Overriding.html

Fixes: 56a2df7615fa ("tools/resolve_btfids: Compile resolve_btfids as host program")
Signed-off-by: Viktor Malik <vmalik@redhat.com>
---
 tools/bpf/resolve_btfids/Makefile | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Jiri Olsa May 30, 2023, 1:29 p.m. UTC | #1
On Tue, May 30, 2023 at 02:33:52PM +0200, Viktor Malik wrote:
> Building BPF selftests with custom HOSTCFLAGS yields an error:
> 
>     # make HOSTCFLAGS="-O2"
>     [...]
>       HOSTCC  ./tools/testing/selftests/bpf/tools/build/resolve_btfids/main.o
>     main.c:73:10: fatal error: linux/rbtree.h: No such file or directory
>        73 | #include <linux/rbtree.h>
>           |          ^~~~~~~~~~~~~~~~
> 
> The reason is that tools/bpf/resolve_btfids/Makefile passes header
> include paths by extending HOSTCFLAGS which is overridden by setting
> HOSTCFLAGS in the make command (because of Makefile rules [1]).
> 
> This patch fixes the above problem by passing the include paths via
> `HOSTCFLAGS_resolve_btfids` which is used by tools/build/Build.include
> and can be combined with overridding HOSTCFLAGS.
> 
> [1] https://www.gnu.org/software/make/manual/html_node/Overriding.html
> 
> Fixes: 56a2df7615fa ("tools/resolve_btfids: Compile resolve_btfids as host program")
> Signed-off-by: Viktor Malik <vmalik@redhat.com>
> ---
>  tools/bpf/resolve_btfids/Makefile | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/bpf/resolve_btfids/Makefile b/tools/bpf/resolve_btfids/Makefile
> index ac548a7baa73..4b8079f294f6 100644
> --- a/tools/bpf/resolve_btfids/Makefile
> +++ b/tools/bpf/resolve_btfids/Makefile
> @@ -67,7 +67,7 @@ $(BPFOBJ): $(wildcard $(LIBBPF_SRC)/*.[ch] $(LIBBPF_SRC)/Makefile) | $(LIBBPF_OU
>  LIBELF_FLAGS := $(shell $(HOSTPKG_CONFIG) libelf --cflags 2>/dev/null)
>  LIBELF_LIBS  := $(shell $(HOSTPKG_CONFIG) libelf --libs 2>/dev/null || echo -lelf)
>  
> -HOSTCFLAGS += -g \
> +HOSTCFLAGS_resolve_btfids += -g \
>            -I$(srctree)/tools/include \
>            -I$(srctree)/tools/include/uapi \
>            -I$(LIBBPF_INCLUDE) \
> @@ -76,7 +76,7 @@ HOSTCFLAGS += -g \
>  
>  LIBS = $(LIBELF_LIBS) -lz
>  
> -export srctree OUTPUT HOSTCFLAGS Q HOSTCC HOSTLD HOSTAR
> +export srctree OUTPUT HOSTCFLAGS_resolve_btfids Q HOSTCC HOSTLD HOSTAR

hum, AFAICS this way the spacified HOSTCFLAGS="-O2" won't be pushed
to the libbpf and libsubcmd dependencies, right?

how about we add the EXTRA_CFLAGS variable like we do in libbpf,
libsubcmd or perf

with the change below you'd need to run:

  $ make EXTRA_CFLAGS="-O2"

I'll dig up the cross build scenarious we broke last time we
touched this stuff, perhaps Ian might remember as well ;-)

jirka


---
diff --git a/tools/bpf/resolve_btfids/Makefile b/tools/bpf/resolve_btfids/Makefile
index ac548a7baa73..58cfedc9c2db 100644
--- a/tools/bpf/resolve_btfids/Makefile
+++ b/tools/bpf/resolve_btfids/Makefile
@@ -18,8 +18,8 @@ else
 endif
 
 # Overrides for the prepare step libraries.
-HOST_OVERRIDES := AR="$(HOSTAR)" CC="$(HOSTCC)" LD="$(HOSTLD)" ARCH="$(HOSTARCH)" \
-		  CROSS_COMPILE="" EXTRA_CFLAGS="$(HOSTCFLAGS)"
+HOST_OVERRIDES = AR="$(HOSTAR)" CC="$(HOSTCC)" LD="$(HOSTLD)" ARCH="$(HOSTARCH)" \
+		  CROSS_COMPILE=""
 
 RM      ?= rm
 HOSTCC  ?= gcc
@@ -67,7 +67,7 @@ $(BPFOBJ): $(wildcard $(LIBBPF_SRC)/*.[ch] $(LIBBPF_SRC)/Makefile) | $(LIBBPF_OU
 LIBELF_FLAGS := $(shell $(HOSTPKG_CONFIG) libelf --cflags 2>/dev/null)
 LIBELF_LIBS  := $(shell $(HOSTPKG_CONFIG) libelf --libs 2>/dev/null || echo -lelf)
 
-HOSTCFLAGS += -g \
+HOSTCFLAGS += $(EXTRA_CFLAGS) -g \
           -I$(srctree)/tools/include \
           -I$(srctree)/tools/include/uapi \
           -I$(LIBBPF_INCLUDE) \
@@ -76,7 +76,7 @@ HOSTCFLAGS += -g \
 
 LIBS = $(LIBELF_LIBS) -lz
 
-export srctree OUTPUT HOSTCFLAGS Q HOSTCC HOSTLD HOSTAR
+export srctree OUTPUT HOSTCFLAGS Q HOSTCC HOSTLD HOSTAR EXTRA_CFLAGS
 include $(srctree)/tools/build/Makefile.include
 
 $(BINARY_IN): fixdep FORCE prepare | $(OUTPUT)
Viktor Malik May 31, 2023, 7:20 a.m. UTC | #2
On 5/30/23 15:29, Jiri Olsa wrote:
> On Tue, May 30, 2023 at 02:33:52PM +0200, Viktor Malik wrote:
>> Building BPF selftests with custom HOSTCFLAGS yields an error:
>>
>>     # make HOSTCFLAGS="-O2"
>>     [...]
>>       HOSTCC  ./tools/testing/selftests/bpf/tools/build/resolve_btfids/main.o
>>     main.c:73:10: fatal error: linux/rbtree.h: No such file or directory
>>        73 | #include <linux/rbtree.h>
>>           |          ^~~~~~~~~~~~~~~~
>>
>> The reason is that tools/bpf/resolve_btfids/Makefile passes header
>> include paths by extending HOSTCFLAGS which is overridden by setting
>> HOSTCFLAGS in the make command (because of Makefile rules [1]).
>>
>> This patch fixes the above problem by passing the include paths via
>> `HOSTCFLAGS_resolve_btfids` which is used by tools/build/Build.include
>> and can be combined with overridding HOSTCFLAGS.
>>
>> [1] https://www.gnu.org/software/make/manual/html_node/Overriding.html
>>
>> Fixes: 56a2df7615fa ("tools/resolve_btfids: Compile resolve_btfids as host program")
>> Signed-off-by: Viktor Malik <vmalik@redhat.com>
>> ---
>>  tools/bpf/resolve_btfids/Makefile | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/tools/bpf/resolve_btfids/Makefile b/tools/bpf/resolve_btfids/Makefile
>> index ac548a7baa73..4b8079f294f6 100644
>> --- a/tools/bpf/resolve_btfids/Makefile
>> +++ b/tools/bpf/resolve_btfids/Makefile
>> @@ -67,7 +67,7 @@ $(BPFOBJ): $(wildcard $(LIBBPF_SRC)/*.[ch] $(LIBBPF_SRC)/Makefile) | $(LIBBPF_OU
>>  LIBELF_FLAGS := $(shell $(HOSTPKG_CONFIG) libelf --cflags 2>/dev/null)
>>  LIBELF_LIBS  := $(shell $(HOSTPKG_CONFIG) libelf --libs 2>/dev/null || echo -lelf)
>>  
>> -HOSTCFLAGS += -g \
>> +HOSTCFLAGS_resolve_btfids += -g \
>>            -I$(srctree)/tools/include \
>>            -I$(srctree)/tools/include/uapi \
>>            -I$(LIBBPF_INCLUDE) \
>> @@ -76,7 +76,7 @@ HOSTCFLAGS += -g \
>>  
>>  LIBS = $(LIBELF_LIBS) -lz
>>  
>> -export srctree OUTPUT HOSTCFLAGS Q HOSTCC HOSTLD HOSTAR
>> +export srctree OUTPUT HOSTCFLAGS_resolve_btfids Q HOSTCC HOSTLD HOSTAR
> 
> hum, AFAICS this way the spacified HOSTCFLAGS="-O2" won't be pushed
> to the libbpf and libsubcmd dependencies, right?

IIUC, it will, b/c we're doing:

    HOST_OVERRIDES := ... EXTRA_CFLAGS="$(HOSTCFLAGS)"

and then pass HOST_OVERRIDES to libbpf and libsubcmd builds, which will
then pick EXTRA_CFLAGS as a part of their build.

Confirmed for libsubcmd:

    $ make HOSTCFLAGS="-O2" V=1 | grep libsubcmd | grep O2 | wc -l
    14
    $ make V=1 | grep libsubcmd | grep O2 | wc -l
    0

Interestingly, I couldn't do the same for libbpf. It looks like libbpf
is not rebuilt for resolve_btfids b/c resolve_btfids/Makefile uses
$(BPFOBJ) as the libbpf target and selftests/bpf/Makefile passes
BPFOBJ=$(HOST_BPFOBJ) to the resolve_btfids build. So, an already built
libbpf is reused and that one hasn't picked HOSTCFLAGS.

> how about we add the EXTRA_CFLAGS variable like we do in libbpf,
> libsubcmd or perf
> 
> with the change below you'd need to run:
> 
>   $ make EXTRA_CFLAGS="-O2"
> 

I'd like to avoid that b/c then, we would need to issue a different make
command for the BPF selftests than for the rest of the kernel to pass
custom flags to host-built programs.

> I'll dig up the cross build scenarious we broke last time we
> touched this stuff, perhaps Ian might remember as well ;-)

That will be useful, thanks :-)

Viktor

> 
> jirka
> 
> 
> ---
> diff --git a/tools/bpf/resolve_btfids/Makefile b/tools/bpf/resolve_btfids/Makefile
> index ac548a7baa73..58cfedc9c2db 100644
> --- a/tools/bpf/resolve_btfids/Makefile
> +++ b/tools/bpf/resolve_btfids/Makefile
> @@ -18,8 +18,8 @@ else
>  endif
>  
>  # Overrides for the prepare step libraries.
> -HOST_OVERRIDES := AR="$(HOSTAR)" CC="$(HOSTCC)" LD="$(HOSTLD)" ARCH="$(HOSTARCH)" \
> -		  CROSS_COMPILE="" EXTRA_CFLAGS="$(HOSTCFLAGS)"
> +HOST_OVERRIDES = AR="$(HOSTAR)" CC="$(HOSTCC)" LD="$(HOSTLD)" ARCH="$(HOSTARCH)" \
> +		  CROSS_COMPILE=""
>  
>  RM      ?= rm
>  HOSTCC  ?= gcc
> @@ -67,7 +67,7 @@ $(BPFOBJ): $(wildcard $(LIBBPF_SRC)/*.[ch] $(LIBBPF_SRC)/Makefile) | $(LIBBPF_OU
>  LIBELF_FLAGS := $(shell $(HOSTPKG_CONFIG) libelf --cflags 2>/dev/null)
>  LIBELF_LIBS  := $(shell $(HOSTPKG_CONFIG) libelf --libs 2>/dev/null || echo -lelf)
>  
> -HOSTCFLAGS += -g \
> +HOSTCFLAGS += $(EXTRA_CFLAGS) -g \
>            -I$(srctree)/tools/include \
>            -I$(srctree)/tools/include/uapi \
>            -I$(LIBBPF_INCLUDE) \
> @@ -76,7 +76,7 @@ HOSTCFLAGS += -g \
>  
>  LIBS = $(LIBELF_LIBS) -lz
>  
> -export srctree OUTPUT HOSTCFLAGS Q HOSTCC HOSTLD HOSTAR
> +export srctree OUTPUT HOSTCFLAGS Q HOSTCC HOSTLD HOSTAR EXTRA_CFLAGS
>  include $(srctree)/tools/build/Makefile.include
>  
>  $(BINARY_IN): fixdep FORCE prepare | $(OUTPUT)
>
Jiri Olsa May 31, 2023, 8:34 a.m. UTC | #3
On Wed, May 31, 2023 at 09:20:44AM +0200, Viktor Malik wrote:
> On 5/30/23 15:29, Jiri Olsa wrote:
> > On Tue, May 30, 2023 at 02:33:52PM +0200, Viktor Malik wrote:
> >> Building BPF selftests with custom HOSTCFLAGS yields an error:
> >>
> >>     # make HOSTCFLAGS="-O2"
> >>     [...]
> >>       HOSTCC  ./tools/testing/selftests/bpf/tools/build/resolve_btfids/main.o
> >>     main.c:73:10: fatal error: linux/rbtree.h: No such file or directory
> >>        73 | #include <linux/rbtree.h>
> >>           |          ^~~~~~~~~~~~~~~~
> >>
> >> The reason is that tools/bpf/resolve_btfids/Makefile passes header
> >> include paths by extending HOSTCFLAGS which is overridden by setting
> >> HOSTCFLAGS in the make command (because of Makefile rules [1]).
> >>
> >> This patch fixes the above problem by passing the include paths via
> >> `HOSTCFLAGS_resolve_btfids` which is used by tools/build/Build.include
> >> and can be combined with overridding HOSTCFLAGS.
> >>
> >> [1] https://www.gnu.org/software/make/manual/html_node/Overriding.html
> >>
> >> Fixes: 56a2df7615fa ("tools/resolve_btfids: Compile resolve_btfids as host program")
> >> Signed-off-by: Viktor Malik <vmalik@redhat.com>
> >> ---
> >>  tools/bpf/resolve_btfids/Makefile | 4 ++--
> >>  1 file changed, 2 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/tools/bpf/resolve_btfids/Makefile b/tools/bpf/resolve_btfids/Makefile
> >> index ac548a7baa73..4b8079f294f6 100644
> >> --- a/tools/bpf/resolve_btfids/Makefile
> >> +++ b/tools/bpf/resolve_btfids/Makefile
> >> @@ -67,7 +67,7 @@ $(BPFOBJ): $(wildcard $(LIBBPF_SRC)/*.[ch] $(LIBBPF_SRC)/Makefile) | $(LIBBPF_OU
> >>  LIBELF_FLAGS := $(shell $(HOSTPKG_CONFIG) libelf --cflags 2>/dev/null)
> >>  LIBELF_LIBS  := $(shell $(HOSTPKG_CONFIG) libelf --libs 2>/dev/null || echo -lelf)
> >>  
> >> -HOSTCFLAGS += -g \
> >> +HOSTCFLAGS_resolve_btfids += -g \
> >>            -I$(srctree)/tools/include \
> >>            -I$(srctree)/tools/include/uapi \
> >>            -I$(LIBBPF_INCLUDE) \
> >> @@ -76,7 +76,7 @@ HOSTCFLAGS += -g \
> >>  
> >>  LIBS = $(LIBELF_LIBS) -lz
> >>  
> >> -export srctree OUTPUT HOSTCFLAGS Q HOSTCC HOSTLD HOSTAR
> >> +export srctree OUTPUT HOSTCFLAGS_resolve_btfids Q HOSTCC HOSTLD HOSTAR
> > 
> > hum, AFAICS this way the spacified HOSTCFLAGS="-O2" won't be pushed
> > to the libbpf and libsubcmd dependencies, right?
> 
> IIUC, it will, b/c we're doing:
> 
>     HOST_OVERRIDES := ... EXTRA_CFLAGS="$(HOSTCFLAGS)"
> 
> and then pass HOST_OVERRIDES to libbpf and libsubcmd builds, which will
> then pick EXTRA_CFLAGS as a part of their build.
> 
> Confirmed for libsubcmd:
> 
>     $ make HOSTCFLAGS="-O2" V=1 | grep libsubcmd | grep O2 | wc -l
>     14
>     $ make V=1 | grep libsubcmd | grep O2 | wc -l
>     0
> 
> Interestingly, I couldn't do the same for libbpf. It looks like libbpf
> is not rebuilt for resolve_btfids b/c resolve_btfids/Makefile uses
> $(BPFOBJ) as the libbpf target and selftests/bpf/Makefile passes
> BPFOBJ=$(HOST_BPFOBJ) to the resolve_btfids build. So, an already built
> libbpf is reused and that one hasn't picked HOSTCFLAGS.
> 
> > how about we add the EXTRA_CFLAGS variable like we do in libbpf,
> > libsubcmd or perf
> > 
> > with the change below you'd need to run:
> > 
> >   $ make EXTRA_CFLAGS="-O2"
> > 
> 
> I'd like to avoid that b/c then, we would need to issue a different make
> command for the BPF selftests than for the rest of the kernel to pass
> custom flags to host-built programs.

ok

> 
> > I'll dig up the cross build scenarious we broke last time we
> > touched this stuff, perhaps Ian might remember as well ;-)
> 
> That will be useful, thanks :-)

there's test described by Nathan in here:

https://lore.kernel.org/bpf/Y9mFVNEi5wAINARY@dev-arch.thelio-3990X/

jirka

> 
> Viktor
> 
> > 
> > jirka
> > 
> > 
> > ---
> > diff --git a/tools/bpf/resolve_btfids/Makefile b/tools/bpf/resolve_btfids/Makefile
> > index ac548a7baa73..58cfedc9c2db 100644
> > --- a/tools/bpf/resolve_btfids/Makefile
> > +++ b/tools/bpf/resolve_btfids/Makefile
> > @@ -18,8 +18,8 @@ else
> >  endif
> >  
> >  # Overrides for the prepare step libraries.
> > -HOST_OVERRIDES := AR="$(HOSTAR)" CC="$(HOSTCC)" LD="$(HOSTLD)" ARCH="$(HOSTARCH)" \
> > -		  CROSS_COMPILE="" EXTRA_CFLAGS="$(HOSTCFLAGS)"
> > +HOST_OVERRIDES = AR="$(HOSTAR)" CC="$(HOSTCC)" LD="$(HOSTLD)" ARCH="$(HOSTARCH)" \
> > +		  CROSS_COMPILE=""
> >  
> >  RM      ?= rm
> >  HOSTCC  ?= gcc
> > @@ -67,7 +67,7 @@ $(BPFOBJ): $(wildcard $(LIBBPF_SRC)/*.[ch] $(LIBBPF_SRC)/Makefile) | $(LIBBPF_OU
> >  LIBELF_FLAGS := $(shell $(HOSTPKG_CONFIG) libelf --cflags 2>/dev/null)
> >  LIBELF_LIBS  := $(shell $(HOSTPKG_CONFIG) libelf --libs 2>/dev/null || echo -lelf)
> >  
> > -HOSTCFLAGS += -g \
> > +HOSTCFLAGS += $(EXTRA_CFLAGS) -g \
> >            -I$(srctree)/tools/include \
> >            -I$(srctree)/tools/include/uapi \
> >            -I$(LIBBPF_INCLUDE) \
> > @@ -76,7 +76,7 @@ HOSTCFLAGS += -g \
> >  
> >  LIBS = $(LIBELF_LIBS) -lz
> >  
> > -export srctree OUTPUT HOSTCFLAGS Q HOSTCC HOSTLD HOSTAR
> > +export srctree OUTPUT HOSTCFLAGS Q HOSTCC HOSTLD HOSTAR EXTRA_CFLAGS
> >  include $(srctree)/tools/build/Makefile.include
> >  
> >  $(BINARY_IN): fixdep FORCE prepare | $(OUTPUT)
> > 
>
Viktor Malik May 31, 2023, 11:40 a.m. UTC | #4
On 5/31/23 10:34, Jiri Olsa wrote:
> On Wed, May 31, 2023 at 09:20:44AM +0200, Viktor Malik wrote:
>> On 5/30/23 15:29, Jiri Olsa wrote:
>>> On Tue, May 30, 2023 at 02:33:52PM +0200, Viktor Malik wrote:
>>>> Building BPF selftests with custom HOSTCFLAGS yields an error:
>>>>
>>>>     # make HOSTCFLAGS="-O2"
>>>>     [...]
>>>>       HOSTCC  ./tools/testing/selftests/bpf/tools/build/resolve_btfids/main.o
>>>>     main.c:73:10: fatal error: linux/rbtree.h: No such file or directory
>>>>        73 | #include <linux/rbtree.h>
>>>>           |          ^~~~~~~~~~~~~~~~
>>>>
>>>> The reason is that tools/bpf/resolve_btfids/Makefile passes header
>>>> include paths by extending HOSTCFLAGS which is overridden by setting
>>>> HOSTCFLAGS in the make command (because of Makefile rules [1]).
>>>>
>>>> This patch fixes the above problem by passing the include paths via
>>>> `HOSTCFLAGS_resolve_btfids` which is used by tools/build/Build.include
>>>> and can be combined with overridding HOSTCFLAGS.
>>>>
>>>> [1] https://www.gnu.org/software/make/manual/html_node/Overriding.html
>>>>
>>>> Fixes: 56a2df7615fa ("tools/resolve_btfids: Compile resolve_btfids as host program")
>>>> Signed-off-by: Viktor Malik <vmalik@redhat.com>
>>>> ---
>>>>  tools/bpf/resolve_btfids/Makefile | 4 ++--
>>>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/tools/bpf/resolve_btfids/Makefile b/tools/bpf/resolve_btfids/Makefile
>>>> index ac548a7baa73..4b8079f294f6 100644
>>>> --- a/tools/bpf/resolve_btfids/Makefile
>>>> +++ b/tools/bpf/resolve_btfids/Makefile
>>>> @@ -67,7 +67,7 @@ $(BPFOBJ): $(wildcard $(LIBBPF_SRC)/*.[ch] $(LIBBPF_SRC)/Makefile) | $(LIBBPF_OU
>>>>  LIBELF_FLAGS := $(shell $(HOSTPKG_CONFIG) libelf --cflags 2>/dev/null)
>>>>  LIBELF_LIBS  := $(shell $(HOSTPKG_CONFIG) libelf --libs 2>/dev/null || echo -lelf)
>>>>  
>>>> -HOSTCFLAGS += -g \
>>>> +HOSTCFLAGS_resolve_btfids += -g \
>>>>            -I$(srctree)/tools/include \
>>>>            -I$(srctree)/tools/include/uapi \
>>>>            -I$(LIBBPF_INCLUDE) \
>>>> @@ -76,7 +76,7 @@ HOSTCFLAGS += -g \
>>>>  
>>>>  LIBS = $(LIBELF_LIBS) -lz
>>>>  
>>>> -export srctree OUTPUT HOSTCFLAGS Q HOSTCC HOSTLD HOSTAR
>>>> +export srctree OUTPUT HOSTCFLAGS_resolve_btfids Q HOSTCC HOSTLD HOSTAR
>>>
>>> hum, AFAICS this way the spacified HOSTCFLAGS="-O2" won't be pushed
>>> to the libbpf and libsubcmd dependencies, right?
>>
>> IIUC, it will, b/c we're doing:
>>
>>     HOST_OVERRIDES := ... EXTRA_CFLAGS="$(HOSTCFLAGS)"
>>
>> and then pass HOST_OVERRIDES to libbpf and libsubcmd builds, which will
>> then pick EXTRA_CFLAGS as a part of their build.
>>
>> Confirmed for libsubcmd:
>>
>>     $ make HOSTCFLAGS="-O2" V=1 | grep libsubcmd | grep O2 | wc -l
>>     14
>>     $ make V=1 | grep libsubcmd | grep O2 | wc -l
>>     0
>>
>> Interestingly, I couldn't do the same for libbpf. It looks like libbpf
>> is not rebuilt for resolve_btfids b/c resolve_btfids/Makefile uses
>> $(BPFOBJ) as the libbpf target and selftests/bpf/Makefile passes
>> BPFOBJ=$(HOST_BPFOBJ) to the resolve_btfids build. So, an already built
>> libbpf is reused and that one hasn't picked HOSTCFLAGS.
>>
>>> how about we add the EXTRA_CFLAGS variable like we do in libbpf,
>>> libsubcmd or perf
>>>
>>> with the change below you'd need to run:
>>>
>>>   $ make EXTRA_CFLAGS="-O2"
>>>
>>
>> I'd like to avoid that b/c then, we would need to issue a different make
>> command for the BPF selftests than for the rest of the kernel to pass
>> custom flags to host-built programs.
> 
> ok
> 
>>
>>> I'll dig up the cross build scenarious we broke last time we
>>> touched this stuff, perhaps Ian might remember as well ;-)
>>
>> That will be useful, thanks :-)
> 
> there's test described by Nathan in here:
> 
> https://lore.kernel.org/bpf/Y9mFVNEi5wAINARY@dev-arch.thelio-3990X/

Thanks, I ran the commands and it builds fine.

Running:
    $ make -j4 ARCH=arm64 CROSS_COMPILE=aarch64-linux-gnu- HOSTLDFLAGS=-fuse-ld=lld LLVM=1 V=1 prepare

Current master:
    [...]
    clang -Wp,-MD,$LINUX_SRC/tools/bpf/resolve_btfids/.main.o.d -Wp,-MT,$LINUX_SRC/tools/bpf/resolve_btfids/main.o -g -I$LINUX_SRC/tools/include -I$LINUX_SRC/tools/include/uapi -I$LINUX_SRC/tools/bpf/resolve_btfids/libbpf/include -I$LINUX_SRC/tools/bpf/resolve_btfids/libsubcmd/include  -D"BUILD_STR(s)=#s"   -c -o $LINUX_SRC/tools/bpf/resolve_btfids/main.o main.c
    [...]

With this patch:
    [...]
    clang -Wp,-MD,$LINUX_SRC/tools/bpf/resolve_btfids/.main.o.d -Wp,-MT,$LINUX_SRC/tools/bpf/resolve_btfids/main.o  -D"BUILD_STR(s)=#s"  -g -I$LINUX_SRC/tools/include -I$LINUX_SRC/tools/include/uapi -I$LINUX_SRC/tools/bpf/resolve_btfids/libbpf/include -I$LINUX_SRC/tools/bpf/resolve_btfids/libsubcmd/include  -c -o $LINUX_SRC/tools/bpf/resolve_btfids/main.o main.c
    [...]

With this patch and HOSTCFLAGS=-O2:
    [...]
    clang -Wp,-MD,$LINUX_SRC/tools/bpf/resolve_btfids/.main.o.d -Wp,-MT,$LINUX_SRC/tools/bpf/resolve_btfids/main.o -O2 -D"BUILD_STR(s)=#s"  -g -I$LINUX_SRC/tools/include -I$LINUX_SRC/tools/include/uapi -I$LINUX_SRC/tools/bpf/resolve_btfids/libbpf/include -I$LINUX_SRC/tools/bpf/resolve_btfids/libsubcmd/include  -c -o $LINUX_SRC/tools/bpf/resolve_btfids/main.o main.c
    [...]

The flags in the first two are the same, they are just reordered. The
last one correctly adds -O2. Other than that, this patch only drops
resolve_btfids-specific flags (those -I...) from building
resolve_btfids/fixdep but that's, IIUC, fine.

Viktor

> 
> jirka
> 
>>
>> Viktor
>>
>>>
>>> jirka
>>>
>>>
>>> ---
>>> diff --git a/tools/bpf/resolve_btfids/Makefile b/tools/bpf/resolve_btfids/Makefile
>>> index ac548a7baa73..58cfedc9c2db 100644
>>> --- a/tools/bpf/resolve_btfids/Makefile
>>> +++ b/tools/bpf/resolve_btfids/Makefile
>>> @@ -18,8 +18,8 @@ else
>>>  endif
>>>  
>>>  # Overrides for the prepare step libraries.
>>> -HOST_OVERRIDES := AR="$(HOSTAR)" CC="$(HOSTCC)" LD="$(HOSTLD)" ARCH="$(HOSTARCH)" \
>>> -		  CROSS_COMPILE="" EXTRA_CFLAGS="$(HOSTCFLAGS)"
>>> +HOST_OVERRIDES = AR="$(HOSTAR)" CC="$(HOSTCC)" LD="$(HOSTLD)" ARCH="$(HOSTARCH)" \
>>> +		  CROSS_COMPILE=""
>>>  
>>>  RM      ?= rm
>>>  HOSTCC  ?= gcc
>>> @@ -67,7 +67,7 @@ $(BPFOBJ): $(wildcard $(LIBBPF_SRC)/*.[ch] $(LIBBPF_SRC)/Makefile) | $(LIBBPF_OU
>>>  LIBELF_FLAGS := $(shell $(HOSTPKG_CONFIG) libelf --cflags 2>/dev/null)
>>>  LIBELF_LIBS  := $(shell $(HOSTPKG_CONFIG) libelf --libs 2>/dev/null || echo -lelf)
>>>  
>>> -HOSTCFLAGS += -g \
>>> +HOSTCFLAGS += $(EXTRA_CFLAGS) -g \
>>>            -I$(srctree)/tools/include \
>>>            -I$(srctree)/tools/include/uapi \
>>>            -I$(LIBBPF_INCLUDE) \
>>> @@ -76,7 +76,7 @@ HOSTCFLAGS += -g \
>>>  
>>>  LIBS = $(LIBELF_LIBS) -lz
>>>  
>>> -export srctree OUTPUT HOSTCFLAGS Q HOSTCC HOSTLD HOSTAR
>>> +export srctree OUTPUT HOSTCFLAGS Q HOSTCC HOSTLD HOSTAR EXTRA_CFLAGS
>>>  include $(srctree)/tools/build/Makefile.include
>>>  
>>>  $(BINARY_IN): fixdep FORCE prepare | $(OUTPUT)
>>>
>>
>
Jiri Olsa June 5, 2023, 9:45 a.m. UTC | #5
On Tue, May 30, 2023 at 02:33:52PM +0200, Viktor Malik wrote:
> Building BPF selftests with custom HOSTCFLAGS yields an error:
> 
>     # make HOSTCFLAGS="-O2"
>     [...]
>       HOSTCC  ./tools/testing/selftests/bpf/tools/build/resolve_btfids/main.o
>     main.c:73:10: fatal error: linux/rbtree.h: No such file or directory
>        73 | #include <linux/rbtree.h>
>           |          ^~~~~~~~~~~~~~~~
> 
> The reason is that tools/bpf/resolve_btfids/Makefile passes header
> include paths by extending HOSTCFLAGS which is overridden by setting
> HOSTCFLAGS in the make command (because of Makefile rules [1]).
> 
> This patch fixes the above problem by passing the include paths via
> `HOSTCFLAGS_resolve_btfids` which is used by tools/build/Build.include
> and can be combined with overridding HOSTCFLAGS.
> 
> [1] https://www.gnu.org/software/make/manual/html_node/Overriding.html
> 
> Fixes: 56a2df7615fa ("tools/resolve_btfids: Compile resolve_btfids as host program")
> Signed-off-by: Viktor Malik <vmalik@redhat.com>

Acked-by: Jiri Olsa <jolsa@kernel.org>

jirka

> ---
>  tools/bpf/resolve_btfids/Makefile | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/bpf/resolve_btfids/Makefile b/tools/bpf/resolve_btfids/Makefile
> index ac548a7baa73..4b8079f294f6 100644
> --- a/tools/bpf/resolve_btfids/Makefile
> +++ b/tools/bpf/resolve_btfids/Makefile
> @@ -67,7 +67,7 @@ $(BPFOBJ): $(wildcard $(LIBBPF_SRC)/*.[ch] $(LIBBPF_SRC)/Makefile) | $(LIBBPF_OU
>  LIBELF_FLAGS := $(shell $(HOSTPKG_CONFIG) libelf --cflags 2>/dev/null)
>  LIBELF_LIBS  := $(shell $(HOSTPKG_CONFIG) libelf --libs 2>/dev/null || echo -lelf)
>  
> -HOSTCFLAGS += -g \
> +HOSTCFLAGS_resolve_btfids += -g \
>            -I$(srctree)/tools/include \
>            -I$(srctree)/tools/include/uapi \
>            -I$(LIBBPF_INCLUDE) \
> @@ -76,7 +76,7 @@ HOSTCFLAGS += -g \
>  
>  LIBS = $(LIBELF_LIBS) -lz
>  
> -export srctree OUTPUT HOSTCFLAGS Q HOSTCC HOSTLD HOSTAR
> +export srctree OUTPUT HOSTCFLAGS_resolve_btfids Q HOSTCC HOSTLD HOSTAR
>  include $(srctree)/tools/build/Makefile.include
>  
>  $(BINARY_IN): fixdep FORCE prepare | $(OUTPUT)
> -- 
> 2.40.1
>
patchwork-bot+netdevbpf@kernel.org June 5, 2023, 10:50 p.m. UTC | #6
Hello:

This patch was applied to bpf/bpf-next.git (master)
by Andrii Nakryiko <andrii@kernel.org>:

On Tue, 30 May 2023 14:33:52 +0200 you wrote:
> Building BPF selftests with custom HOSTCFLAGS yields an error:
> 
>     # make HOSTCFLAGS="-O2"
>     [...]
>       HOSTCC  ./tools/testing/selftests/bpf/tools/build/resolve_btfids/main.o
>     main.c:73:10: fatal error: linux/rbtree.h: No such file or directory
>        73 | #include <linux/rbtree.h>
>           |          ^~~~~~~~~~~~~~~~
> 
> [...]

Here is the summary with links:
  - [bpf-next] tools/resolve_btfids: fix setting HOSTCFLAGS
    https://git.kernel.org/bpf/bpf-next/c/edd75c802855

You are awesome, thank you!
diff mbox series

Patch

diff --git a/tools/bpf/resolve_btfids/Makefile b/tools/bpf/resolve_btfids/Makefile
index ac548a7baa73..4b8079f294f6 100644
--- a/tools/bpf/resolve_btfids/Makefile
+++ b/tools/bpf/resolve_btfids/Makefile
@@ -67,7 +67,7 @@  $(BPFOBJ): $(wildcard $(LIBBPF_SRC)/*.[ch] $(LIBBPF_SRC)/Makefile) | $(LIBBPF_OU
 LIBELF_FLAGS := $(shell $(HOSTPKG_CONFIG) libelf --cflags 2>/dev/null)
 LIBELF_LIBS  := $(shell $(HOSTPKG_CONFIG) libelf --libs 2>/dev/null || echo -lelf)
 
-HOSTCFLAGS += -g \
+HOSTCFLAGS_resolve_btfids += -g \
           -I$(srctree)/tools/include \
           -I$(srctree)/tools/include/uapi \
           -I$(LIBBPF_INCLUDE) \
@@ -76,7 +76,7 @@  HOSTCFLAGS += -g \
 
 LIBS = $(LIBELF_LIBS) -lz
 
-export srctree OUTPUT HOSTCFLAGS Q HOSTCC HOSTLD HOSTAR
+export srctree OUTPUT HOSTCFLAGS_resolve_btfids Q HOSTCC HOSTLD HOSTAR
 include $(srctree)/tools/build/Makefile.include
 
 $(BINARY_IN): fixdep FORCE prepare | $(OUTPUT)