Re: [PATCH bpf-next] bpftool: Clean up HOST_CFLAGS, HOST_LDFLAGS for bootstrap bpftool

[Quentin Monnet <qmo@kernel.org>]

2024-03-21 18:55 UTC+0000 ~ Andrii Nakryiko <andrii.nakryiko@gmail.com>
> On Wed, Mar 20, 2024 at 1:51 AM Jiri Olsa <olsajiri@gmail.com> 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@linaro.org>
>>> Signed-off-by: Quentin Monnet <qmo@kernel.org>
>>
>> got some fuzz when applying it, but other than that lgtm
>>
>> Acked-by: Jiri Olsa <jolsa@kernel.org>
>>
> 
> this was applied, but PW bot didn't notice it. I marked it as accepted
> in patchworks, just FYI.

Thanks for the notice!
Quentin