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> 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 >
