2023-07-07 16:08:09

by James Clark

[permalink] [raw]
Subject: [PATCH] perf build: Fix library not found error when using CSLIBS

-L only specifies the search path for libraries directly provided in the
link line with -l. Because -lopencsd isn't specified, it's only linked
because it's a dependency of -lopencsd_c_api. Dependencies like this are
resolved using the default system search paths or -rpath-link=... rather
than -L. This means that compilation only works if OpenCSD is installed
to the system rather than provided with the CSLIBS (-L) option.

This could be fixed by adding -Wl,-rpath-link=$(CSLIBS) but that is less
conventional than just adding -lopencsd to the link line so that it uses
-L. -lopencsd seems to have been removed in commit ed17b1914978 ("perf
tools: Drop requirement for libstdc++.so for libopencsd check") because
it was thought that there was a chance compilation would work even if it
didn't exist, but I think that only applies to libstdc++ so there is no
harm to add it back. libopencsd.so and libopencsd_c_api.so would always
exist together.

Testing
=======

The following scenarios now all work:

* Cross build with OpenCSD installed
* Cross build using CSLIBS=...
* Native build with OpenCSD installed
* Native build using CSLIBS=...
* Static cross build with OpenCSD installed
* Static cross build with CSLIBS=...

Reported-by: Radhey Shyam Pandey <[email protected]>
Closes: https://lore.kernel.org/linux-arm-kernel/[email protected]/
Link: https://lore.kernel.org/all/[email protected]/
Fixes: ed17b1914978 ("perf tools: Drop requirement for libstdc++.so for libopencsd check")
Signed-off-by: James Clark <[email protected]>
---
tools/perf/Makefile.config | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/perf/Makefile.config b/tools/perf/Makefile.config
index 0609c19caabd..c5db0de49868 100644
--- a/tools/perf/Makefile.config
+++ b/tools/perf/Makefile.config
@@ -155,9 +155,9 @@ FEATURE_CHECK_LDFLAGS-libcrypto = -lcrypto
ifdef CSINCLUDES
LIBOPENCSD_CFLAGS := -I$(CSINCLUDES)
endif
-OPENCSDLIBS := -lopencsd_c_api
+OPENCSDLIBS := -lopencsd_c_api -lopencsd
ifeq ($(findstring -static,${LDFLAGS}),-static)
- OPENCSDLIBS += -lopencsd -lstdc++
+ OPENCSDLIBS += -lstdc++
endif
ifdef CSLIBS
LIBOPENCSD_LDFLAGS := -L$(CSLIBS)
--
2.34.1



2023-07-11 08:54:41

by Pandey, Radhey Shyam

[permalink] [raw]
Subject: RE: [PATCH] perf build: Fix library not found error when using CSLIBS

> -----Original Message-----
> From: James Clark <[email protected]>
> Sent: Friday, July 7, 2023 9:16 PM
> To: [email protected]; Pandey, Radhey Shyam
> <[email protected]>
> Cc: [email protected]; James Clark <[email protected]>; Peter
> Zijlstra <[email protected]>; Ingo Molnar <[email protected]>;
> Arnaldo Carvalho de Melo <[email protected]>; Mark Rutland
> <[email protected]>; Alexander Shishkin
> <[email protected]>; Jiri Olsa <[email protected]>;
> Namhyung Kim <[email protected]>; Ian Rogers <[email protected]>;
> Adrian Hunter <[email protected]>; Uwe Kleine-K?nig <uwe@kleine-
> koenig.org>; [email protected]
> Subject: [PATCH] perf build: Fix library not found error when using CSLIBS
>
> -L only specifies the search path for libraries directly provided in the link line
> with -l. Because -lopencsd isn't specified, it's only linked because it's a
> dependency of -lopencsd_c_api. Dependencies like this are resolved using
> the default system search paths or -rpath-link=... rather than -L. This means
> that compilation only works if OpenCSD is installed to the system rather than
> provided with the CSLIBS (-L) option.
>
> This could be fixed by adding -Wl,-rpath-link=$(CSLIBS) but that is less
> conventional than just adding -lopencsd to the link line so that it uses -L. -
> lopencsd seems to have been removed in commit ed17b1914978 ("perf
> tools: Drop requirement for libstdc++.so for libopencsd check") because it
> was thought that there was a chance compilation would work even if it didn't
> exist, but I think that only applies to libstdc++ so there is no harm to add it
> back. libopencsd.so and libopencsd_c_api.so would always exist together.
>
> Testing
> =======
>
> The following scenarios now all work:
>
> * Cross build with OpenCSD installed
> * Cross build using CSLIBS=...
> * Native build with OpenCSD installed
> * Native build using CSLIBS=...
> * Static cross build with OpenCSD installed
> * Static cross build with CSLIBS=...
>
> Reported-by: Radhey Shyam Pandey <[email protected]>
> Closes: https://lore.kernel.org/linux-arm-kernel/56905d7a-a91e-883a-b707-
> [email protected]/
> Link: https://lore.kernel.org/all/36cc4dc6-bf4b-1093-1c0a-
> [email protected]/
> Fixes: ed17b1914978 ("perf tools: Drop requirement for libstdc++.so for
> libopencsd check")
> Signed-off-by: James Clark <[email protected]>
> ---
> tools/perf/Makefile.config | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/tools/perf/Makefile.config b/tools/perf/Makefile.config index
> 0609c19caabd..c5db0de49868 100644
> --- a/tools/perf/Makefile.config
> +++ b/tools/perf/Makefile.config
> @@ -155,9 +155,9 @@ FEATURE_CHECK_LDFLAGS-libcrypto = -lcrypto ifdef
> CSINCLUDES
> LIBOPENCSD_CFLAGS := -I$(CSINCLUDES)
> endif
> -OPENCSDLIBS := -lopencsd_c_api
> +OPENCSDLIBS := -lopencsd_c_api -lopencsd
> ifeq ($(findstring -static,${LDFLAGS}),-static)
> - OPENCSDLIBS += -lopencsd -lstdc++
> + OPENCSDLIBS += -lstdc++
> endif
> ifdef CSLIBS
> LIBOPENCSD_LDFLAGS := -L$(CSLIBS)
> --
> 2.34.1

Tested-by: Radhey Shyam Pandey <[email protected]>
Cross compiled for aarch64 on x86_64.

make ARCH=arm64 NO_LIBELF=1 NO_JVMTI=1 VF=1 CORESIGHT=1 -C tools/perf

file <snip>/linux-xlnx/tools/perf/perf
perf: ELF 64-bit LSB shared object, ARM aarch64, version 1 (SYSV),
dynamically linked, interpreter /lib/ld-linux-aarch64.so.1,
BuildID[sha1]=ef7c524338577b14e7c0f914d882dec4d26e93a2,
for GNU/Linux 3.14.0, with debug_info, not stripped

Thanks,
Radhey


2023-07-11 14:59:22

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: Re: [PATCH] perf build: Fix library not found error when using CSLIBS

Em Tue, Jul 11, 2023 at 08:28:31AM +0000, Pandey, Radhey Shyam escreveu:
> > -----Original Message-----
> > From: James Clark <[email protected]>
> > Sent: Friday, July 7, 2023 9:16 PM
> > To: [email protected]; Pandey, Radhey Shyam
> > <[email protected]>
> > Cc: [email protected]; James Clark <[email protected]>; Peter
> > Zijlstra <[email protected]>; Ingo Molnar <[email protected]>;
> > Arnaldo Carvalho de Melo <[email protected]>; Mark Rutland
> > <[email protected]>; Alexander Shishkin
> > <[email protected]>; Jiri Olsa <[email protected]>;
> > Namhyung Kim <[email protected]>; Ian Rogers <[email protected]>;
> > Adrian Hunter <[email protected]>; Uwe Kleine-König <uwe@kleine-
> > koenig.org>; [email protected]
> > Subject: [PATCH] perf build: Fix library not found error when using CSLIBS
> >
> > -L only specifies the search path for libraries directly provided in the link line
> > with -l. Because -lopencsd isn't specified, it's only linked because it's a
> > dependency of -lopencsd_c_api. Dependencies like this are resolved using
> > the default system search paths or -rpath-link=... rather than -L. This means
> > that compilation only works if OpenCSD is installed to the system rather than
> > provided with the CSLIBS (-L) option.
> >
> > This could be fixed by adding -Wl,-rpath-link=$(CSLIBS) but that is less
> > conventional than just adding -lopencsd to the link line so that it uses -L. -
> > lopencsd seems to have been removed in commit ed17b1914978 ("perf
> > tools: Drop requirement for libstdc++.so for libopencsd check") because it
> > was thought that there was a chance compilation would work even if it didn't
> > exist, but I think that only applies to libstdc++ so there is no harm to add it
> > back. libopencsd.so and libopencsd_c_api.so would always exist together.
> >
> > Testing
> > =======
> >
> > The following scenarios now all work:
> >
> > * Cross build with OpenCSD installed
> > * Cross build using CSLIBS=...
> > * Native build with OpenCSD installed
> > * Native build using CSLIBS=...
> > * Static cross build with OpenCSD installed
> > * Static cross build with CSLIBS=...
> >
> > Reported-by: Radhey Shyam Pandey <[email protected]>
> > Closes: https://lore.kernel.org/linux-arm-kernel/56905d7a-a91e-883a-b707-
> > [email protected]/
> > Link: https://lore.kernel.org/all/36cc4dc6-bf4b-1093-1c0a-
> > [email protected]/
> > Fixes: ed17b1914978 ("perf tools: Drop requirement for libstdc++.so for
> > libopencsd check")
> > Signed-off-by: James Clark <[email protected]>
> > ---
> > tools/perf/Makefile.config | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/tools/perf/Makefile.config b/tools/perf/Makefile.config index
> > 0609c19caabd..c5db0de49868 100644
> > --- a/tools/perf/Makefile.config
> > +++ b/tools/perf/Makefile.config
> > @@ -155,9 +155,9 @@ FEATURE_CHECK_LDFLAGS-libcrypto = -lcrypto ifdef
> > CSINCLUDES
> > LIBOPENCSD_CFLAGS := -I$(CSINCLUDES)
> > endif
> > -OPENCSDLIBS := -lopencsd_c_api
> > +OPENCSDLIBS := -lopencsd_c_api -lopencsd
> > ifeq ($(findstring -static,${LDFLAGS}),-static)
> > - OPENCSDLIBS += -lopencsd -lstdc++
> > + OPENCSDLIBS += -lstdc++
> > endif
> > ifdef CSLIBS
> > LIBOPENCSD_LDFLAGS := -L$(CSLIBS)
> > --
> > 2.34.1
>
> Tested-by: Radhey Shyam Pandey <[email protected]>
> Cross compiled for aarch64 on x86_64.
>
> make ARCH=arm64 NO_LIBELF=1 NO_JVMTI=1 VF=1 CORESIGHT=1 -C tools/perf
>
> file <snip>/linux-xlnx/tools/perf/perf
> perf: ELF 64-bit LSB shared object, ARM aarch64, version 1 (SYSV),
> dynamically linked, interpreter /lib/ld-linux-aarch64.so.1,
> BuildID[sha1]=ef7c524338577b14e7c0f914d882dec4d26e93a2,
> for GNU/Linux 3.14.0, with debug_info, not stripped

Thanks for reporting and testing, applied to perf-tools. I see no
problems in my common case which is:

⬢[acme@toolbox perf-tools]$ alias m='make -k BUILD_BPF_SKEL=1 CORESIGHT=1 O=/tmp/build/perf-tools -C tools/perf install-bin && git status && perf test python ; perf record -o /dev/null sleep 0.01 ; perf stat --null sleep 0.01'
⬢[acme@toolbox perf-tools]$

⬢[acme@toolbox perf-tools]$ ldd ~/bin/perf | grep csd
libopencsd_c_api.so.1 => /lib64/libopencsd_c_api.so.1 (0x00007f9c012ea000)
libopencsd.so.1 => /lib64/libopencsd.so.1 (0x00007f9c00556000)
⬢[acme@toolbox perf-tools]$

- Arnaldo