* [PATCH bpf-next] libbpf: Support Debian in resolve_full_path()
@ 2022-04-04 10:29 Ilya Leoshkevich
2022-04-04 21:40 ` Andrii Nakryiko
2022-04-04 21:58 ` Andrii Nakryiko
0 siblings, 2 replies; 5+ messages in thread
From: Ilya Leoshkevich @ 2022-04-04 10:29 UTC (permalink / raw)
To: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
Heiko Carstens, Vasily Gorbik, Christian Borntraeger,
Alexander Gordeev
Cc: bpf, Ilya Leoshkevich
attach_probe selftest fails on Debian-based distros with `failed to
resolve full path for 'libc.so.6'`. The reason is that these distros
embraced multiarch to the point where even for the "main" architecture
they store libc in /lib/<triple>.
This is configured in /etc/ld.so.conf and in theory it's possible to
replicate the loader's parsing and processing logic in libbpf, however
a much simpler solution is to just enumerate the known library paths.
Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
---
tools/lib/bpf/libbpf.c | 54 ++++++++++++++++++++++++++++++++++++------
1 file changed, 47 insertions(+), 7 deletions(-)
diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index 6d2be53e4ba9..4f616b11564f 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -10707,21 +10707,61 @@ static long elf_find_func_offset(const char *binary_path, const char *name)
return ret;
}
+static void add_debian_library_paths(const char **search_paths, int *n)
+{
+ /*
+ * Based on https://packages.debian.org/sid/libc6.
+ *
+ * Assume that the traced program is built for the same architecture
+ * as libbpf, which should cover the vast majority of cases.
+ */
+#if defined(__x86_64__)
+ search_paths[(*n)++] = "/lib/x86_64-linux-gnu";
+#elif defined(__i386__)
+ search_paths[(*n)++] = "/lib/i386-linux-gnu";
+#elif defined(__s390x__)
+ search_paths[(*n)++] = "/lib/s390x-linux-gnu";
+#elif defined(__s390__)
+ search_paths[(*n)++] = "/lib/s390-linux-gnu";
+#elif defined(__arm__)
+#if defined(__SOFTFP__)
+ search_paths[(*n)++] = "/lib/arm-linux-gnueabi";
+#else
+ search_paths[(*n)++] = "/lib/arm-linux-gnueabihf";
+#endif /* defined(__SOFTFP__) */
+#elif defined(__aarch64__)
+ search_paths[(*n)++] = "/lib/aarch64-linux-gnu";
+#elif defined(__mips__) && (__BYTE_ORDER__ == __ORDER_LITTLE_ENDIAN__)
+#if _MIPS_SZLONG == 64
+ search_paths[(*n)++] = "/lib/mips64el-linux-gnuabi64";
+#elif _MIPS_SZLONG == 32
+ search_paths[(*n)++] = "/lib/mipsel-linux-gnu";
+#endif
+#elif defined(__powerpc__) && (__BYTE_ORDER__ == __ORDER_LITTLE_ENDIAN__)
+ search_paths[(*n)++] = "/lib/powerpc64le-linux-gnu";
+#elif defined(__sparc__)
+ search_paths[(*n)++] = "/lib/sparc64-linux-gnu";
+#elif defined(__riscv) && __riscv_xlen == 64
+ search_paths[(*n)++] = "/lib/riscv64-linux-gnu";
+#endif
+}
+
/* Get full path to program/shared library. */
static int resolve_full_path(const char *file, char *result, size_t result_sz)
{
- const char *search_paths[2];
- int i;
+ const char *search_paths[3];
+ int i, n = 0;
if (strstr(file, ".so")) {
- search_paths[0] = getenv("LD_LIBRARY_PATH");
- search_paths[1] = "/usr/lib64:/usr/lib";
+ search_paths[n++] = getenv("LD_LIBRARY_PATH");
+ search_paths[n++] = "/usr/lib64:/usr/lib";
+ add_debian_library_paths(search_paths, &n);
} else {
- search_paths[0] = getenv("PATH");
- search_paths[1] = "/usr/bin:/usr/sbin";
+ search_paths[n++] = getenv("PATH");
+ search_paths[n++] = "/usr/bin:/usr/sbin";
}
- for (i = 0; i < ARRAY_SIZE(search_paths); i++) {
+ for (i = 0; i < n; i++) {
const char *s;
if (!search_paths[i])
--
2.35.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH bpf-next] libbpf: Support Debian in resolve_full_path()
2022-04-04 10:29 [PATCH bpf-next] libbpf: Support Debian in resolve_full_path() Ilya Leoshkevich
@ 2022-04-04 21:40 ` Andrii Nakryiko
2022-04-04 21:58 ` Andrii Nakryiko
1 sibling, 0 replies; 5+ messages in thread
From: Andrii Nakryiko @ 2022-04-04 21:40 UTC (permalink / raw)
To: Ilya Leoshkevich
Cc: Alexei Starovoitov, Daniel Borkmann, Heiko Carstens,
Vasily Gorbik, Christian Borntraeger, Alexander Gordeev, bpf
On Mon, Apr 4, 2022 at 3:29 AM Ilya Leoshkevich <iii@linux.ibm.com> wrote:
>
> attach_probe selftest fails on Debian-based distros with `failed to
> resolve full path for 'libc.so.6'`. The reason is that these distros
> embraced multiarch to the point where even for the "main" architecture
> they store libc in /lib/<triple>.
>
> This is configured in /etc/ld.so.conf and in theory it's possible to
> replicate the loader's parsing and processing logic in libbpf, however
> a much simpler solution is to just enumerate the known library paths.
>
> Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
> ---
> tools/lib/bpf/libbpf.c | 54 ++++++++++++++++++++++++++++++++++++------
> 1 file changed, 47 insertions(+), 7 deletions(-)
>
> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> index 6d2be53e4ba9..4f616b11564f 100644
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c
> @@ -10707,21 +10707,61 @@ static long elf_find_func_offset(const char *binary_path, const char *name)
> return ret;
> }
>
> +static void add_debian_library_paths(const char **search_paths, int *n)
> +{
> + /*
> + * Based on https://packages.debian.org/sid/libc6.
> + *
> + * Assume that the traced program is built for the same architecture
> + * as libbpf, which should cover the vast majority of cases.
> + */
> +#if defined(__x86_64__)
> + search_paths[(*n)++] = "/lib/x86_64-linux-gnu";
> +#elif defined(__i386__)
> + search_paths[(*n)++] = "/lib/i386-linux-gnu";
> +#elif defined(__s390x__)
> + search_paths[(*n)++] = "/lib/s390x-linux-gnu";
> +#elif defined(__s390__)
> + search_paths[(*n)++] = "/lib/s390-linux-gnu";
> +#elif defined(__arm__)
> +#if defined(__SOFTFP__)
> + search_paths[(*n)++] = "/lib/arm-linux-gnueabi";
> +#else
> + search_paths[(*n)++] = "/lib/arm-linux-gnueabihf";
> +#endif /* defined(__SOFTFP__) */
> +#elif defined(__aarch64__)
> + search_paths[(*n)++] = "/lib/aarch64-linux-gnu";
> +#elif defined(__mips__) && (__BYTE_ORDER__ == __ORDER_LITTLE_ENDIAN__)
> +#if _MIPS_SZLONG == 64
> + search_paths[(*n)++] = "/lib/mips64el-linux-gnuabi64";
> +#elif _MIPS_SZLONG == 32
> + search_paths[(*n)++] = "/lib/mipsel-linux-gnu";
> +#endif
> +#elif defined(__powerpc__) && (__BYTE_ORDER__ == __ORDER_LITTLE_ENDIAN__)
> + search_paths[(*n)++] = "/lib/powerpc64le-linux-gnu";
> +#elif defined(__sparc__)
> + search_paths[(*n)++] = "/lib/sparc64-linux-gnu";
> +#elif defined(__riscv) && __riscv_xlen == 64
> + search_paths[(*n)++] = "/lib/riscv64-linux-gnu";
> +#endif
> +}
> +
that's pretty comprehensive :)
But let's make this function return const char * instead, with NULL
for unknown/unsupported architectures. This will make the below code
simpler....
> /* Get full path to program/shared library. */
> static int resolve_full_path(const char *file, char *result, size_t result_sz)
> {
> - const char *search_paths[2];
> - int i;
> + const char *search_paths[3];
> + int i, n = 0;
>
instead of counting, we can just NULL-initialize search_paths and
teach the loop below to ignore NULL entries?
> if (strstr(file, ".so")) {
> - search_paths[0] = getenv("LD_LIBRARY_PATH");
> - search_paths[1] = "/usr/lib64:/usr/lib";
> + search_paths[n++] = getenv("LD_LIBRARY_PATH");
> + search_paths[n++] = "/usr/lib64:/usr/lib";
> + add_debian_library_paths(search_paths, &n);
so you'll just have
search_paths[2] = arch_specific_lib_paths();
> } else {
> - search_paths[0] = getenv("PATH");
> - search_paths[1] = "/usr/bin:/usr/sbin";
> + search_paths[n++] = getenv("PATH");
> + search_paths[n++] = "/usr/bin:/usr/sbin";
> }
>
> - for (i = 0; i < ARRAY_SIZE(search_paths); i++) {
> + for (i = 0; i < n; i++) {
> const char *s;
>
> if (!search_paths[i])
oh, actually we already do ignore NULLs, it seems?
> --
> 2.35.1
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH bpf-next] libbpf: Support Debian in resolve_full_path()
2022-04-04 10:29 [PATCH bpf-next] libbpf: Support Debian in resolve_full_path() Ilya Leoshkevich
2022-04-04 21:40 ` Andrii Nakryiko
@ 2022-04-04 21:58 ` Andrii Nakryiko
2022-04-04 22:22 ` Ilya Leoshkevich
1 sibling, 1 reply; 5+ messages in thread
From: Andrii Nakryiko @ 2022-04-04 21:58 UTC (permalink / raw)
To: Ilya Leoshkevich
Cc: Alexei Starovoitov, Daniel Borkmann, Heiko Carstens,
Vasily Gorbik, Christian Borntraeger, Alexander Gordeev, bpf
On Mon, Apr 4, 2022 at 3:29 AM Ilya Leoshkevich <iii@linux.ibm.com> wrote:
>
> attach_probe selftest fails on Debian-based distros with `failed to
> resolve full path for 'libc.so.6'`. The reason is that these distros
> embraced multiarch to the point where even for the "main" architecture
> they store libc in /lib/<triple>.
>
> This is configured in /etc/ld.so.conf and in theory it's possible to
> replicate the loader's parsing and processing logic in libbpf, however
> a much simpler solution is to just enumerate the known library paths.
>
> Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
> ---
> tools/lib/bpf/libbpf.c | 54 ++++++++++++++++++++++++++++++++++++------
> 1 file changed, 47 insertions(+), 7 deletions(-)
>
> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> index 6d2be53e4ba9..4f616b11564f 100644
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c
> @@ -10707,21 +10707,61 @@ static long elf_find_func_offset(const char *binary_path, const char *name)
> return ret;
> }
>
> +static void add_debian_library_paths(const char **search_paths, int *n)
> +{
> + /*
> + * Based on https://packages.debian.org/sid/libc6.
> + *
> + * Assume that the traced program is built for the same architecture
> + * as libbpf, which should cover the vast majority of cases.
> + */
> +#if defined(__x86_64__)
can you please also drop defined() where possible, it looks cleaner to me:
#if __x86_64__
vs
#if defined(__x86_64__)
> + search_paths[(*n)++] = "/lib/x86_64-linux-gnu";
> +#elif defined(__i386__)
> + search_paths[(*n)++] = "/lib/i386-linux-gnu";
> +#elif defined(__s390x__)
> + search_paths[(*n)++] = "/lib/s390x-linux-gnu";
> +#elif defined(__s390__)
> + search_paths[(*n)++] = "/lib/s390-linux-gnu";
> +#elif defined(__arm__)
> +#if defined(__SOFTFP__)
> + search_paths[(*n)++] = "/lib/arm-linux-gnueabi";
> +#else
> + search_paths[(*n)++] = "/lib/arm-linux-gnueabihf";
> +#endif /* defined(__SOFTFP__) */
> +#elif defined(__aarch64__)
> + search_paths[(*n)++] = "/lib/aarch64-linux-gnu";
> +#elif defined(__mips__) && (__BYTE_ORDER__ == __ORDER_LITTLE_ENDIAN__)
> +#if _MIPS_SZLONG == 64
> + search_paths[(*n)++] = "/lib/mips64el-linux-gnuabi64";
> +#elif _MIPS_SZLONG == 32
> + search_paths[(*n)++] = "/lib/mipsel-linux-gnu";
> +#endif
> +#elif defined(__powerpc__) && (__BYTE_ORDER__ == __ORDER_LITTLE_ENDIAN__)
> + search_paths[(*n)++] = "/lib/powerpc64le-linux-gnu";
> +#elif defined(__sparc__)
> + search_paths[(*n)++] = "/lib/sparc64-linux-gnu";
> +#elif defined(__riscv) && __riscv_xlen == 64
> + search_paths[(*n)++] = "/lib/riscv64-linux-gnu";
> +#endif
> +}
> +
> /* Get full path to program/shared library. */
> static int resolve_full_path(const char *file, char *result, size_t result_sz)
> {
> - const char *search_paths[2];
> - int i;
> + const char *search_paths[3];
> + int i, n = 0;
>
> if (strstr(file, ".so")) {
> - search_paths[0] = getenv("LD_LIBRARY_PATH");
> - search_paths[1] = "/usr/lib64:/usr/lib";
> + search_paths[n++] = getenv("LD_LIBRARY_PATH");
> + search_paths[n++] = "/usr/lib64:/usr/lib";
> + add_debian_library_paths(search_paths, &n);
> } else {
> - search_paths[0] = getenv("PATH");
> - search_paths[1] = "/usr/bin:/usr/sbin";
> + search_paths[n++] = getenv("PATH");
> + search_paths[n++] = "/usr/bin:/usr/sbin";
> }
>
> - for (i = 0; i < ARRAY_SIZE(search_paths); i++) {
> + for (i = 0; i < n; i++) {
> const char *s;
>
> if (!search_paths[i])
> --
> 2.35.1
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH bpf-next] libbpf: Support Debian in resolve_full_path()
2022-04-04 21:58 ` Andrii Nakryiko
@ 2022-04-04 22:22 ` Ilya Leoshkevich
2022-04-04 22:59 ` Andrii Nakryiko
0 siblings, 1 reply; 5+ messages in thread
From: Ilya Leoshkevich @ 2022-04-04 22:22 UTC (permalink / raw)
To: Andrii Nakryiko
Cc: Alexei Starovoitov, Daniel Borkmann, Heiko Carstens,
Vasily Gorbik, Christian Borntraeger, Alexander Gordeev, bpf
On Mon, 2022-04-04 at 14:58 -0700, Andrii Nakryiko wrote:
> On Mon, Apr 4, 2022 at 3:29 AM Ilya Leoshkevich <iii@linux.ibm.com>
> wrote:
> >
> > attach_probe selftest fails on Debian-based distros with `failed to
> > resolve full path for 'libc.so.6'`. The reason is that these
> > distros
> > embraced multiarch to the point where even for the "main"
> > architecture
> > they store libc in /lib/<triple>.
> >
> > This is configured in /etc/ld.so.conf and in theory it's possible
> > to
> > replicate the loader's parsing and processing logic in libbpf,
> > however
> > a much simpler solution is to just enumerate the known library
> > paths.
> >
> > Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
> > ---
> > tools/lib/bpf/libbpf.c | 54 ++++++++++++++++++++++++++++++++++++--
> > ----
> > 1 file changed, 47 insertions(+), 7 deletions(-)
> >
> > diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> > index 6d2be53e4ba9..4f616b11564f 100644
> > --- a/tools/lib/bpf/libbpf.c
> > +++ b/tools/lib/bpf/libbpf.c
> > @@ -10707,21 +10707,61 @@ static long elf_find_func_offset(const
> > char *binary_path, const char *name)
> > return ret;
> > }
> >
> > +static void add_debian_library_paths(const char **search_paths,
> > int *n)
> > +{
> > + /*
> > + * Based on https://packages.debian.org/sid/libc6.
> > + *
> > + * Assume that the traced program is built for the same
> > architecture
> > + * as libbpf, which should cover the vast majority of
> > cases.
> > + */
> > +#if defined(__x86_64__)
>
> can you please also drop defined() where possible, it looks cleaner
> to me:
>
> #if __x86_64__
>
> vs
>
> #if defined(__x86_64__)
The consensus in the existing kernel and tools code (including libbpf
itself) seems to be to use #if defined() or #ifdef for such macros:
$ git grep __x86_64__ | wc -l
306
$ git grep __x86_64__ | grep -v \
-e '#\s*ifdef __x86_64__' \
-e 'defined\s*(__x86_64__)' \
-e '#\s*ifndef __x86_64__' \
-e '#\s*else' \
-e '#\s*endif'
arch/x86/Makefile: CHECKFLAGS += -D__x86_64__
arch/x86/Makefile.um:CHECKFLAGS += -m64 -D__x86_64__
tools/lib/bpf/libbpf.c:#if __x86_64__
tools/testing/selftests/ipc/Makefile: CFLAGS := -DCONFIG_X86_64 -
D__x86_64__
tools/testing/selftests/rcutorture/bin/mkinitrd.sh:if echo -e "#if
__x86_64__||__i386__||__i486__||__i586__||__i686__" \
tools/testing/selftests/x86/mov_ss_trap.c:#if __x86_64__
I think `#if __x86_64__` should work in most cases, but I'd rather
stick with the existing style if you don't mind.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH bpf-next] libbpf: Support Debian in resolve_full_path()
2022-04-04 22:22 ` Ilya Leoshkevich
@ 2022-04-04 22:59 ` Andrii Nakryiko
0 siblings, 0 replies; 5+ messages in thread
From: Andrii Nakryiko @ 2022-04-04 22:59 UTC (permalink / raw)
To: Ilya Leoshkevich
Cc: Alexei Starovoitov, Daniel Borkmann, Heiko Carstens,
Vasily Gorbik, Christian Borntraeger, Alexander Gordeev, bpf
On Mon, Apr 4, 2022 at 3:22 PM Ilya Leoshkevich <iii@linux.ibm.com> wrote:
>
> On Mon, 2022-04-04 at 14:58 -0700, Andrii Nakryiko wrote:
> > On Mon, Apr 4, 2022 at 3:29 AM Ilya Leoshkevich <iii@linux.ibm.com>
> > wrote:
> > >
> > > attach_probe selftest fails on Debian-based distros with `failed to
> > > resolve full path for 'libc.so.6'`. The reason is that these
> > > distros
> > > embraced multiarch to the point where even for the "main"
> > > architecture
> > > they store libc in /lib/<triple>.
> > >
> > > This is configured in /etc/ld.so.conf and in theory it's possible
> > > to
> > > replicate the loader's parsing and processing logic in libbpf,
> > > however
> > > a much simpler solution is to just enumerate the known library
> > > paths.
> > >
> > > Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
> > > ---
> > > tools/lib/bpf/libbpf.c | 54 ++++++++++++++++++++++++++++++++++++--
> > > ----
> > > 1 file changed, 47 insertions(+), 7 deletions(-)
> > >
> > > diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> > > index 6d2be53e4ba9..4f616b11564f 100644
> > > --- a/tools/lib/bpf/libbpf.c
> > > +++ b/tools/lib/bpf/libbpf.c
> > > @@ -10707,21 +10707,61 @@ static long elf_find_func_offset(const
> > > char *binary_path, const char *name)
> > > return ret;
> > > }
> > >
> > > +static void add_debian_library_paths(const char **search_paths,
> > > int *n)
> > > +{
> > > + /*
> > > + * Based on https://packages.debian.org/sid/libc6.
> > > + *
> > > + * Assume that the traced program is built for the same
> > > architecture
> > > + * as libbpf, which should cover the vast majority of
> > > cases.
> > > + */
> > > +#if defined(__x86_64__)
> >
> > can you please also drop defined() where possible, it looks cleaner
> > to me:
> >
> > #if __x86_64__
> >
> > vs
> >
> > #if defined(__x86_64__)
>
> The consensus in the existing kernel and tools code (including libbpf
> itself) seems to be to use #if defined() or #ifdef for such macros:
>
> $ git grep __x86_64__ | wc -l
> 306
>
> $ git grep __x86_64__ | grep -v \
> -e '#\s*ifdef __x86_64__' \
> -e 'defined\s*(__x86_64__)' \
> -e '#\s*ifndef __x86_64__' \
> -e '#\s*else' \
> -e '#\s*endif'
> arch/x86/Makefile: CHECKFLAGS += -D__x86_64__
> arch/x86/Makefile.um:CHECKFLAGS += -m64 -D__x86_64__
> tools/lib/bpf/libbpf.c:#if __x86_64__
> tools/testing/selftests/ipc/Makefile: CFLAGS := -DCONFIG_X86_64 -
> D__x86_64__
> tools/testing/selftests/rcutorture/bin/mkinitrd.sh:if echo -e "#if
> __x86_64__||__i386__||__i486__||__i586__||__i686__" \
> tools/testing/selftests/x86/mov_ss_trap.c:#if __x86_64__
>
> I think `#if __x86_64__` should work in most cases, but I'd rather
> stick with the existing style if you don't mind.
Yeah, that's fine. I felt like #if defined() is unnecessarily verbose,
#ifdef would be totally fine. But looking at latest version, I think
it's fine, mostly due to more linear structure, so it's all good.
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2022-04-04 23:15 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-04-04 10:29 [PATCH bpf-next] libbpf: Support Debian in resolve_full_path() Ilya Leoshkevich
2022-04-04 21:40 ` Andrii Nakryiko
2022-04-04 21:58 ` Andrii Nakryiko
2022-04-04 22:22 ` Ilya Leoshkevich
2022-04-04 22:59 ` Andrii Nakryiko
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.