2024-03-21 18:55 UTC+0000 ~ Andrii Nakryiko <andrii.nakryiko@xxxxxxxxx> > 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. Thanks for the notice! Quentin
