| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <CAP-5=fXeykYoqLJ8t6Gq31cO8eYGOnppgc86PHfWnBoz4xgw-g@mail.gmail.com>
Date: Thu, 17 Apr 2025 08:49:35 -0700
From: Ian Rogers <irogers@...gle.com>
To: arnaldo.melo@...il.com
Cc: Namhyung Kim <namhyung@...nel.org>, Adrian Hunter <adrian.hunter@...el.com>,
James Clark <james.clark@...aro.org>, Jiri Olsa <jolsa@...nel.org>,
Kan Liang <kan.liang@...ux.intel.com>, Quentin Monnet <qmo@...nel.org>,
Linux Kernel Mailing List <linux-kernel@...r.kernel.org>, linux-perf-users@...r.kernel.org
Subject: Re: [PATCH 1/1] tools build: Remove libbfd from the set of expected
libraries to build perf
On Wed, Apr 16, 2025 at 7:18 PM <arnaldo.melo@...il.com> wrote:
>
> The tools/build/feature/test-all.c file tries to build with the most
> common set of libraries expected to be present to build perf, and these
> days libbfd (binutils) isn't one since it was made opt-in via
> BUILD_NONDISTRO=1 on the make command line as it has license issues.
>
> Fix this by removing the tests from there.
>
> Fixes: dd317df072071903 ("perf build: Make binutil libraries opt in")
> Cc: Adrian Hunter <adrian.hunter@...el.com>
> Cc: Ian Rogers <irogers@...gle.com>
> Cc: James Clark <james.clark@...aro.org>
> Cc: Jiri Olsa <jolsa@...nel.org>
> Cc: Kan Liang <kan.liang@...ux.intel.com>
> Cc: Namhyung Kim <namhyung@...nel.org>
> Cc: Quentin Monnet <qmo@...nel.org>
> Signed-off-by: Arnaldo Carvalho de Melo <acme@...hat.com>
Reviewed-by: Ian Rogers <irogers@...gle.com>
There is a wider set of cleanups that remove BUILD_NONDISTRO and
libbfd that I posted back in January:
https://lore.kernel.org/lkml/20250122174308.350350-1-irogers@google.com/
These changes are carried in:
https://github.com/googleprodkernel/linux-perf/commits/google_tools_master/
The remaining use of libbfd for BPF JIT code disassembly is converted
to using either capstone or LLVM:
https://lore.kernel.org/lkml/20250122174308.350350-11-irogers@google.com/
Namhyung had concerns over code like:
https://github.com/googleprodkernel/linux-perf/blob/google_tools_master/tools/perf/util/capstone.c#L23-L132
where structs and enums derived from pahole are declared rather than
gathered from a #include. Doing things this way was deliberate in the
patch series so that the code could assume capstone or llvm support
may be present, falling back when not, and that the build wouldn't
need to have support for a no header file option that could do nothing
(this option would always be to try to fallback on dlopen and nobody
could create a less enabled build by forgetting to have a header
file). Theoretically the structs and defines, incorporated by way of
pahole, could change and a header file dependency would be robust to
this. In practice this would be an ABI incompatible change just as
changing a function name looked up by dlsym would be. Namhyung took
onboard my suggestion that we could reduce the set of structs/enums/..
for capstone by disabling the `print_insn_x86` when using dlopen, but
I think such a change should warn the user of reduced functionality,
cleaning up the warning would just bring back the code as I had
proposed:
https://lore.kernel.org/lkml/CAP-5=fXL0hXFT+t6gHp2RFd4dKnebSkd+rewudpmdentKGPURw@mail.gmail.com/
I think the patch series should be a priority to land as:
1) there is substantial cleanup wrt libbfd, libiberty, .. with
dependencies being factored out into their our C files;
2) the dependencies on libcapstone and libllvm are broken at build
time, allowing distributions to ship perf with a more minimal set of
dependencies and then later get the faster code or better support by
installing the libraries - I think ideally we'd do the same for the
libpython dependency as Namhyung has done in his uftrace;
3) the series adds BPF JIT disassembly.
Maybe this can be an occasion we respect the opinions of the patch
author and land what may be just a good patch series, but not quite
perfect to someone else's definition of perfect. We can always put
patches on top to make things perfect and discuss the merits at that
moment.
Thanks,
Ian
> ---
> tools/build/Makefile.feature | 12 ------------
> tools/build/feature/Makefile | 2 +-
> tools/build/feature/test-all.c | 19 -------------------
> tools/perf/Makefile.config | 1 +
> 4 files changed, 2 insertions(+), 32 deletions(-)
>
> diff --git a/tools/build/Makefile.feature b/tools/build/Makefile.feature
> index 57bd995ce6afa318..da025a8040a9a154 100644
> --- a/tools/build/Makefile.feature
> +++ b/tools/build/Makefile.feature
> @@ -42,17 +42,12 @@ endef
> #
> # All the others should have lines in tools/build/feature/test-all.c like:
> #
> -# #define main main_test_disassembler_init_styled
> -# # include "test-disassembler-init-styled.c"
> -# #undef main
> -#
> # #define main main_test_libzstd
> # # include "test-libzstd.c"
> # #undef main
> #
> # int main(int argc, char *argv[])
> # {
> -# main_test_disassembler_four_args();
> # main_test_libzstd();
> # return 0;
> # }
> @@ -60,7 +55,6 @@ endef
> # If the sample above works, then we end up with these lines in the FEATURE-DUMP
> # file:
> #
> -# feature-disassembler-four-args=1
> # feature-libzstd=1
> #
> FEATURE_TESTS_BASIC := \
> @@ -71,8 +65,6 @@ FEATURE_TESTS_BASIC := \
> get_current_dir_name \
> gettid \
> glibc \
> - libbfd \
> - libbfd-buildid \
> libelf \
> libelf-getphdrnum \
> libelf-gelf_getnote \
> @@ -102,8 +94,6 @@ FEATURE_TESTS_BASIC := \
> setns \
> libaio \
> libzstd \
> - disassembler-four-args \
> - disassembler-init-styled \
> file-handle
>
> # FEATURE_TESTS_BASIC + FEATURE_TESTS_EXTRA is the complete list
> @@ -119,8 +109,6 @@ FEATURE_TESTS_EXTRA := \
> hello \
> libbabeltrace \
> libcapstone \
> - libbfd-liberty \
> - libbfd-liberty-z \
> libopencsd \
> cxx \
> llvm \
> diff --git a/tools/build/feature/Makefile b/tools/build/feature/Makefile
> index b8b5fb183dd40693..76724931f68e1b92 100644
> --- a/tools/build/feature/Makefile
> +++ b/tools/build/feature/Makefile
> @@ -110,7 +110,7 @@ all: $(FILES)
> __BUILD = $(CC) $(CFLAGS) -MD -Wall -Werror -o $@ $(patsubst %.bin,%.c,$(@F)) $(LDFLAGS)
> BUILD = $(__BUILD) > $(@:.bin=.make.output) 2>&1
> BUILD_BFD = $(BUILD) -DPACKAGE='"perf"' -lbfd -ldl
> - BUILD_ALL = $(BUILD) -fstack-protector-all -O2 -D_FORTIFY_SOURCE=2 -ldw -lelf -lnuma -lelf -lslang $(FLAGS_PERL_EMBED) $(FLAGS_PYTHON_EMBED) -DPACKAGE='"perf"' -lbfd -ldl -lz -llzma -lzstd
> + BUILD_ALL = $(BUILD) -fstack-protector-all -O2 -D_FORTIFY_SOURCE=2 -ldw -lelf -lnuma -lelf -lslang $(FLAGS_PERL_EMBED) $(FLAGS_PYTHON_EMBED) -lz -llzma -lzstd
>
> __BUILDXX = $(CXX) $(CXXFLAGS) -MD -Wall -Werror -o $@ $(patsubst %.bin,%.cpp,$(@F)) $(LDFLAGS)
> BUILDXX = $(__BUILDXX) > $(@:.bin=.make.output) 2>&1
> diff --git a/tools/build/feature/test-all.c b/tools/build/feature/test-all.c
> index 03ddaac6f4c4dfa2..1010f233d9c1ad49 100644
> --- a/tools/build/feature/test-all.c
> +++ b/tools/build/feature/test-all.c
> @@ -66,14 +66,6 @@
> # include "test-libslang.c"
> #undef main
>
> -#define main main_test_libbfd
> -# include "test-libbfd.c"
> -#undef main
> -
> -#define main main_test_libbfd_buildid
> -# include "test-libbfd-buildid.c"
> -#undef main
> -
> #define main main_test_backtrace
> # include "test-backtrace.c"
> #undef main
> @@ -158,14 +150,6 @@
> # include "test-reallocarray.c"
> #undef main
>
> -#define main main_test_disassembler_four_args
> -# include "test-disassembler-four-args.c"
> -#undef main
> -
> -#define main main_test_disassembler_init_styled
> -# include "test-disassembler-init-styled.c"
> -#undef main
> -
> #define main main_test_libzstd
> # include "test-libzstd.c"
> #undef main
> @@ -193,8 +177,6 @@ int main(int argc, char *argv[])
> main_test_libelf_gelf_getnote();
> main_test_libelf_getshdrstrndx();
> main_test_libslang();
> - main_test_libbfd();
> - main_test_libbfd_buildid();
> main_test_backtrace();
> main_test_libnuma();
> main_test_numa_num_possible_cpus();
> @@ -213,7 +195,6 @@ int main(int argc, char *argv[])
> main_test_setns();
> main_test_libaio();
> main_test_reallocarray();
> - main_test_disassembler_four_args();
> main_test_libzstd();
> main_test_libtraceevent();
> main_test_libtracefs();
> diff --git a/tools/perf/Makefile.config b/tools/perf/Makefile.config
> index 9f08a6e96b351707..7e9aa3d910c2cdcc 100644
> --- a/tools/perf/Makefile.config
> +++ b/tools/perf/Makefile.config
> @@ -917,6 +917,7 @@ ifneq ($(NO_JEVENTS),1)
> endif
>
> ifdef BUILD_NONDISTRO
> + $(call feature_check,libbfd)
> ifeq ($(feature-libbfd), 1)
> EXTLIBS += -lbfd -lopcodes
> else
> --
> 2.49.0
>
Powered by blists - more mailing lists