On Wed, Mar 20, 2024 at 1:51 AM Jiri Olsa <olsajiri@xxxxxxxxx> wrote: > > On Wed, Mar 20, 2024 at 01:41:03AM +0000, Quentin Monnet wrote: > > Bpftool's Makefile uses $(HOST_CFLAGS) to build the bootstrap version of > > bpftool, in order to pick the flags for the host (where we run the > > bootstrap version) and not for the target system (where we plan to run > > the full bpftool binary). But we pass too much information through this > > variable. > > > > In particular, we set HOST_CFLAGS by copying most of the $(CFLAGS); but > > we do this after the feature detection for bpftool, which means that > > $(CFLAGS), hence $(HOST_CFLAGS), contain all macro definitions for using > > the different optional features. For example, -DHAVE_LLVM_SUPPORT may be > > passed to the $(HOST_CFLAGS), even though the LLVM disassembler is not > > used in the bootstrap version, and the related library may even be > > missing for the host architecture. > > > > A similar thing happens with the $(LDFLAGS), that we use unchanged for > > linking the bootstrap version even though they may contains flags to > > link against additional libraries. > > > > To address the $(HOST_CFLAGS) issue, we move the definition of > > $(HOST_CFLAGS) earlier in the Makefile, before the $(CFLAGS) update > > resulting from the feature probing - none of which being relevant to the > > bootstrap version. To clean up the $(LDFLAGS) for the bootstrap version, > > we introduce a dedicated $(HOST_LDFLAGS) variable that we base on > > $(LDFLAGS), before the feature probing as well. > > > > On my setup, the following macro and libraries are removed from the > > compiler invocation to build bpftool after this patch: > > > > -DUSE_LIBCAP > > -DHAVE_LLVM_SUPPORT > > -I/usr/lib/llvm-17/include > > -D_GNU_SOURCE > > -D__STDC_CONSTANT_MACROS > > -D__STDC_FORMAT_MACROS > > -D__STDC_LIMIT_MACROS > > -lLLVM-17 > > -L/usr/lib/llvm-17/lib > > > > Another advantage of cleaning up these flags is that displaying > > available features with "bpftool version" becomes more accurate for the > > bootstrap bpftool, and no longer reflects the features detected (and > > available only) for the final binary. > > > > Cc: Jean-Philippe Brucker <jean-philippe@xxxxxxxxxx> > > Signed-off-by: Quentin Monnet <qmo@xxxxxxxxxx> > > got some fuzz when applying it, but other than that lgtm > > Acked-by: Jiri Olsa <jolsa@xxxxxxxxxx> > this was applied, but PW bot didn't notice it. I marked it as accepted in patchworks, just FYI. > jirka > > > --- > > tools/bpf/bpftool/Makefile | 9 +++++---- > > 1 file changed, 5 insertions(+), 4 deletions(-) > > > > diff --git a/tools/bpf/bpftool/Makefile b/tools/bpf/bpftool/Makefile > > index e9154ace80ff..972f8d727130 100644 > > --- a/tools/bpf/bpftool/Makefile > > +++ b/tools/bpf/bpftool/Makefile > > @@ -89,6 +89,10 @@ ifneq ($(EXTRA_LDFLAGS),) > > LDFLAGS += $(EXTRA_LDFLAGS) > > endif > > > > +HOST_CFLAGS := $(subst -I$(LIBBPF_INCLUDE),-I$(LIBBPF_BOOTSTRAP_INCLUDE),\ > > + $(subst $(CLANG_CROSS_FLAGS),,$(CFLAGS))) > > +HOST_LDFLAGS := $(LDFLAGS) > > + > > INSTALL ?= install > > RM ?= rm -f > > > > @@ -178,9 +182,6 @@ ifeq ($(filter -DHAVE_LLVM_SUPPORT -DHAVE_LIBBFD_SUPPORT,$(CFLAGS)),) > > SRCS := $(filter-out jit_disasm.c,$(SRCS)) > > endif > > > > -HOST_CFLAGS = $(subst -I$(LIBBPF_INCLUDE),-I$(LIBBPF_BOOTSTRAP_INCLUDE),\ > > - $(subst $(CLANG_CROSS_FLAGS),,$(CFLAGS))) > > - > > BPFTOOL_BOOTSTRAP := $(BOOTSTRAP_OUTPUT)bpftool > > > > BOOTSTRAP_OBJS = $(addprefix $(BOOTSTRAP_OUTPUT),main.o common.o json_writer.o gen.o btf.o xlated_dumper.o btf_dumper.o disasm.o) > > @@ -238,7 +239,7 @@ $(OUTPUT)disasm.o: $(srctree)/kernel/bpf/disasm.c > > $(QUIET_CC)$(CC) $(CFLAGS) -c -MMD $< -o $@ > > > > $(BPFTOOL_BOOTSTRAP): $(BOOTSTRAP_OBJS) $(LIBBPF_BOOTSTRAP) > > - $(QUIET_LINK)$(HOSTCC) $(HOST_CFLAGS) $(LDFLAGS) $(BOOTSTRAP_OBJS) $(LIBS_BOOTSTRAP) -o $@ > > + $(QUIET_LINK)$(HOSTCC) $(HOST_CFLAGS) $(HOST_LDFLAGS) $(BOOTSTRAP_OBJS) $(LIBS_BOOTSTRAP) -o $@ > > > > $(OUTPUT)bpftool: $(OBJS) $(LIBBPF) > > $(QUIET_LINK)$(CC) $(CFLAGS) $(LDFLAGS) $(OBJS) $(LIBS) -o $@ > > -- > > 2.34.1 > >
