[PATCH] tests: Add -ldl to dwfl_proc_attach_LDFLAGS

[PATCH] tests: Add -ldl to dwfl_proc_attach_LDFLAGS

[Mark Wielaard]

dwfl-proc-attach uses (overrides) dlopen (so it does nothing).  This
seems to cause a versioned dlopen symbol to be pulled in when building
with LTO. Resulting in a link failure (when dlopen isn't integrated
into libc):

/usr/bin/ld: dwfl-proc-attach.o (symbol from plugin): undefined
reference to symbol 'dlopen@@GLIBC_2.2.5'
/usr/bin/ld: /usr/lib64/libdl.so.2: error adding symbols: DSO missing
from command line collect2: error: ld returned 1 exit status

So simply explicitly add -ldl to the LDFLAGS.

Signed-off-by: Mark Wielaard <mark@klomp.org>
---
 tests/ChangeLog   | 4 ++++
 tests/Makefile.am | 2 +-
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/tests/ChangeLog b/tests/ChangeLog
index a59cdd51..97cb3fa3 100644
--- a/tests/ChangeLog
+++ b/tests/ChangeLog
@@ -1,3 +1,7 @@
+2021-11-18  Mark Wielaard  <mark@klomp.org>
+
+	* Makefile.am (dwfl_proc_attach_LDFLAGS): Add -ldl.
+
 2021-11-05  Frank Ch. Eigler  <fche@redhat.com>
 
 	PR28430
diff --git a/tests/Makefile.am b/tests/Makefile.am
index bfb8b13a..f212f9fb 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -717,7 +717,7 @@ strptr_LDADD = $(libelf)
 newdata_LDADD = $(libelf)
 elfstrtab_LDADD = $(libelf)
 dwfl_proc_attach_LDADD = $(libdw)
-dwfl_proc_attach_LDFLAGS = -pthread $(AM_LDFLAGS)
+dwfl_proc_attach_LDFLAGS = -pthread -ldl $(AM_LDFLAGS)
 elfshphehdr_LDADD =$(libelf)
 elfstrmerge_LDADD = $(libdw) $(libelf)
 dwelfgnucompressed_LDADD = $(libelf) $(libdw)
-- 
2.30.2

Re: [PATCH] tests: Add -ldl to dwfl_proc_attach_LDFLAGS

[Dmitry V. Levin]

On Thu, Nov 18, 2021 at 10:23:41PM +0100, Mark Wielaard wrote:
> dwfl-proc-attach uses (overrides) dlopen (so it does nothing).  This
> seems to cause a versioned dlopen symbol to be pulled in when building
> with LTO. Resulting in a link failure (when dlopen isn't integrated
> into libc):
> 
> /usr/bin/ld: dwfl-proc-attach.o (symbol from plugin): undefined
> reference to symbol 'dlopen@@GLIBC_2.2.5'
> /usr/bin/ld: /usr/lib64/libdl.so.2: error adding symbols: DSO missing
> from command line collect2: error: ld returned 1 exit status
> 
> So simply explicitly add -ldl to the LDFLAGS.
> 
> Signed-off-by: Mark Wielaard <mark@klomp.org>
> ---
>  tests/ChangeLog   | 4 ++++
>  tests/Makefile.am | 2 +-
>  2 files changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/tests/ChangeLog b/tests/ChangeLog
> index a59cdd51..97cb3fa3 100644
> --- a/tests/ChangeLog
> +++ b/tests/ChangeLog
> @@ -1,3 +1,7 @@
> +2021-11-18  Mark Wielaard  <mark@klomp.org>
> +
> +	* Makefile.am (dwfl_proc_attach_LDFLAGS): Add -ldl.
> +
>  2021-11-05  Frank Ch. Eigler  <fche@redhat.com>
>  
>  	PR28430
> diff --git a/tests/Makefile.am b/tests/Makefile.am
> index bfb8b13a..f212f9fb 100644
> --- a/tests/Makefile.am
> +++ b/tests/Makefile.am
> @@ -717,7 +717,7 @@ strptr_LDADD = $(libelf)
>  newdata_LDADD = $(libelf)
>  elfstrtab_LDADD = $(libelf)
>  dwfl_proc_attach_LDADD = $(libdw)
> -dwfl_proc_attach_LDFLAGS = -pthread $(AM_LDFLAGS)
> +dwfl_proc_attach_LDFLAGS = -pthread -ldl $(AM_LDFLAGS)

Let's make clear what's going on here.  First of all, dwfl-proc-attach.c
does not use dlopen so it doesn't pull it in and doesn't need -ldl.
In regular builds, dwfl-proc-attach.o is linked with ../libdw/libdw.so
which in turn uses dlopen and is already linked with -ldl.
When elfutils is configured with --enable-gprof or --enable-gcov,
BUILD_STATIC is enabled and dwfl-proc-attach.o is linked with
../libdw/libdw.a -lz $(zip_LIBS) $(libelf) $(libebl) -ldl -lpthread
which already contains -ldl.
In any case, I fail to understand why dwfl-proc-attach might need
an extra -ldl, especially in LDFLAGS which goes before LDADD
in the linking command.


-- 
ldv

Re: [PATCH] tests: Add -ldl to dwfl_proc_attach_LDFLAGS

[Mark Wielaard]

Hi Dmitry,

On Fri, Nov 19, 2021 at 01:20:26AM +0300, Dmitry V. Levin wrote:
> On Thu, Nov 18, 2021 at 10:23:41PM +0100, Mark Wielaard wrote:
> > dwfl-proc-attach uses (overrides) dlopen (so it does nothing).  This
> > seems to cause a versioned dlopen symbol to be pulled in when building
> > with LTO. Resulting in a link failure (when dlopen isn't integrated
> > into libc):
> >
> > /usr/bin/ld: dwfl-proc-attach.o (symbol from plugin): undefined
> > reference to symbol 'dlopen@@GLIBC_2.2.5'
> > /usr/bin/ld: /usr/lib64/libdl.so.2: error adding symbols: DSO missing
> > from command line collect2: error: ld returned 1 exit status
> >
> > So simply explicitly add -ldl to the LDFLAGS.
> [...]
> Let's make clear what's going on here.  First of all, dwfl-proc-attach.c
> does not use dlopen so it doesn't pull it in and doesn't need -ldl.
> In regular builds, dwfl-proc-attach.o is linked with ../libdw/libdw.so
> which in turn uses dlopen and is already linked with -ldl.
> When elfutils is configured with --enable-gprof or --enable-gcov,
> BUILD_STATIC is enabled and dwfl-proc-attach.o is linked with
> ../libdw/libdw.a -lz $(zip_LIBS) $(libelf) $(libebl) -ldl -lpthread
> which already contains -ldl.
> In any case, I fail to understand why dwfl-proc-attach might need
> an extra -ldl, especially in LDFLAGS which goes before LDADD
> in the linking command.

Agreed. It isn't totally clear why lto causes dlopen to be "pulled in"
(which is probably the wrong technical term). But it really
does. Instead of adding -ldl to LDFLAGS you can also remove the dlopen
from the testcase:

diff --git a/tests/dwfl-proc-attach.c b/tests/dwfl-proc-attach.c
index d02e9fc0..0c5e8622 100644
--- a/tests/dwfl-proc-attach.c
+++ b/tests/dwfl-proc-attach.c
@@ -114,9 +114,5 @@ main (int argc __attribute__ ((unused)),
    So simply override dlopen and always return NULL so libdebuginfod
    (and libcurl) are never loaded.  This test doesn't rely on
    libdebuginfod anyway.  */
-void *dlopen (void)
-{
-  return NULL;
-}
 
 #endif /* __linux__ */

Which also makes the linker happy, but removes the valgrind workaround.

Cheers,

Mark

Re: [PATCH] tests: Add -ldl to dwfl_proc_attach_LDFLAGS

[Florian Weimer]

* Dmitry V. Levin:

> Let's make clear what's going on here.  First of all, dwfl-proc-attach.c
> does not use dlopen so it doesn't pull it in and doesn't need -ldl.
> In regular builds, dwfl-proc-attach.o is linked with ../libdw/libdw.so
> which in turn uses dlopen and is already linked with -ldl.
> When elfutils is configured with --enable-gprof or --enable-gcov,
> BUILD_STATIC is enabled and dwfl-proc-attach.o is linked with
> ../libdw/libdw.a -lz $(zip_LIBS) $(libelf) $(libebl) -ldl -lpthread
> which already contains -ldl.
> In any case, I fail to understand why dwfl-proc-attach might need
> an extra -ldl, especially in LDFLAGS which goes before LDADD
> in the linking command.

It may have to do with --as-needed that some builds use.  If there are
no pending undefined references, some linkers drop earlier shared object
references with --as-needed (similar to what happens with static
archives).

The GCC LTO plugin results in ld looking at more objects in greater
detail for some reason.  Without LTO and --as-needed, you probably don't
get a dlopen export (if you do not link with -E) because indirect
dependencies are not consulted, breaking the valgrind workaround because
there is no interposition.

Thanks,
Florian

Re: [PATCH] tests: Add -ldl to dwfl_proc_attach_LDFLAGS

[Dmitry V. Levin]

On Fri, Nov 19, 2021 at 05:58:19PM +0100, Florian Weimer wrote:
> * Dmitry V. Levin:
> 
> > Let's make clear what's going on here.  First of all, dwfl-proc-attach.c
> > does not use dlopen so it doesn't pull it in and doesn't need -ldl.
> > In regular builds, dwfl-proc-attach.o is linked with ../libdw/libdw.so
> > which in turn uses dlopen and is already linked with -ldl.
> > When elfutils is configured with --enable-gprof or --enable-gcov,
> > BUILD_STATIC is enabled and dwfl-proc-attach.o is linked with
> > ../libdw/libdw.a -lz $(zip_LIBS) $(libelf) $(libebl) -ldl -lpthread
> > which already contains -ldl.
> > In any case, I fail to understand why dwfl-proc-attach might need
> > an extra -ldl, especially in LDFLAGS which goes before LDADD
> > in the linking command.
> 
> It may have to do with --as-needed that some builds use.  If there are
> no pending undefined references, some linkers drop earlier shared object
> references with --as-needed (similar to what happens with static
> archives).
> 
> The GCC LTO plugin results in ld looking at more objects in greater
> detail for some reason.  Without LTO and --as-needed, you probably don't
> get a dlopen export (if you do not link with -E) because indirect
> dependencies are not consulted, breaking the valgrind workaround because
> there is no interposition.

Thanks.  I suppose adding -rdynamic to dwfl_proc_attach_LDFLAGS should be
a more correct fix.


-- 
ldv

Re: [PATCH] tests: Add -ldl to dwfl_proc_attach_LDFLAGS

[Mark Wielaard]

[-- Attachment #1: Type: text/plain, Size: 907 bytes --]

Hi,

On Sat, Nov 20, 2021 at 01:12:17AM +0300, Dmitry V. Levin wrote:
> On Fri, Nov 19, 2021 at 05:58:19PM +0100, Florian Weimer wrote:
> > It may have to do with --as-needed that some builds use.  If there are
> > no pending undefined references, some linkers drop earlier shared object
> > references with --as-needed (similar to what happens with static
> > archives).
> > 
> > The GCC LTO plugin results in ld looking at more objects in greater
> > detail for some reason.  Without LTO and --as-needed, you probably don't
> > get a dlopen export (if you do not link with -E) because indirect
> > dependencies are not consulted, breaking the valgrind workaround because
> > there is no interposition.
> 
> Thanks.  I suppose adding -rdynamic to dwfl_proc_attach_LDFLAGS should be
> a more correct fix.

That works. But I don't really understand why.
Does the attached patch look OK to you?

Thanks,

Mark

[-- Attachment #2: 0001-tests-Add-rdynamic-to-dwfl_proc_attach_LDFLAGS.patch --]
[-- Type: text/x-diff, Size: 1741 bytes --]

From 311b42279d42187540e776648fd19c1e897fbacd Mon Sep 17 00:00:00 2001
From: Mark Wielaard <mark@klomp.org>
Date: Thu, 18 Nov 2021 21:34:57 +0100
Subject: [PATCH] tests: Add -rdynamic to dwfl_proc_attach_LDFLAGS

dwfl-proc-attach uses (overrides) dlopen (so it does nothing).  This
seems to cause a versioned dlopen symbol to be pulled in when building
with LTO. Resulting in a link failure (when dlopen isn't integrated
into libc):

/usr/bin/ld: dwfl-proc-attach.o (symbol from plugin): undefined
reference to symbol 'dlopen@@GLIBC_2.2.5'
/usr/bin/ld: /usr/lib64/libdl.so.2: error adding symbols: DSO missing
from command line collect2: error: ld returned 1 exit status

Add -rdynamic to the LDFLAGS to add all symbols to the dynamic symbol
table for dwfl-proc-attach.

Signed-off-by: Mark Wielaard <mark@klomp.org>
---
 tests/ChangeLog   | 4 ++++
 tests/Makefile.am | 2 +-
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/tests/ChangeLog b/tests/ChangeLog
index a59cdd51..d0fc3fb0 100644
--- a/tests/ChangeLog
+++ b/tests/ChangeLog
@@ -1,3 +1,7 @@
+2021-11-18  Mark Wielaard  <mark@klomp.org>
+
+	* Makefile.am (dwfl_proc_attach_LDFLAGS): Add -rdynamic.
+
 2021-11-05  Frank Ch. Eigler  <fche@redhat.com>
 
 	PR28430
diff --git a/tests/Makefile.am b/tests/Makefile.am
index bfb8b13a..2298a5ae 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -717,7 +717,7 @@ strptr_LDADD = $(libelf)
 newdata_LDADD = $(libelf)
 elfstrtab_LDADD = $(libelf)
 dwfl_proc_attach_LDADD = $(libdw)
-dwfl_proc_attach_LDFLAGS = -pthread $(AM_LDFLAGS)
+dwfl_proc_attach_LDFLAGS = -pthread -rdynamic $(AM_LDFLAGS)
 elfshphehdr_LDADD =$(libelf)
 elfstrmerge_LDADD = $(libdw) $(libelf)
 dwelfgnucompressed_LDADD = $(libelf) $(libdw)
-- 
2.30.2

Re: [PATCH] tests: Add -ldl to dwfl_proc_attach_LDFLAGS

[Mark Wielaard]

[-- Attachment #1: Type: text/plain, Size: 1120 bytes --]

Hi,

On Sat, 2021-11-20 at 15:18 +0100, Mark Wielaard wrote:
> On Sat, Nov 20, 2021 at 01:12:17AM +0300, Dmitry V. Levin wrote:
> > On Fri, Nov 19, 2021 at 05:58:19PM +0100, Florian Weimer wrote:
> > > It may have to do with --as-needed that some builds use.  If there are
> > > no pending undefined references, some linkers drop earlier shared object
> > > references with --as-needed (similar to what happens with static
> > > archives).
> > > 
> > > The GCC LTO plugin results in ld looking at more objects in greater
> > > detail for some reason.  Without LTO and --as-needed, you probably don't
> > > get a dlopen export (if you do not link with -E) because indirect
> > > dependencies are not consulted, breaking the valgrind workaround because
> > > there is no interposition.
> > 
> > Thanks.  I suppose adding -rdynamic to dwfl_proc_attach_LDFLAGS should be
> > a more correct fix.
> 
> That works. But I don't really understand why.
> Does the attached patch look OK to you?

I decided to simply disable LTO on Fedora 34, but still would like
feedback on this patch.

Thanks,

Mark

[-- Attachment #2: 0001-tests-Add-rdynamic-to-dwfl_proc_attach_LDFLAGS.patch --]
[-- Type: text/x-patch, Size: 1741 bytes --]

From 311b42279d42187540e776648fd19c1e897fbacd Mon Sep 17 00:00:00 2001
From: Mark Wielaard <mark@klomp.org>
Date: Thu, 18 Nov 2021 21:34:57 +0100
Subject: [PATCH] tests: Add -rdynamic to dwfl_proc_attach_LDFLAGS

dwfl-proc-attach uses (overrides) dlopen (so it does nothing).  This
seems to cause a versioned dlopen symbol to be pulled in when building
with LTO. Resulting in a link failure (when dlopen isn't integrated
into libc):

/usr/bin/ld: dwfl-proc-attach.o (symbol from plugin): undefined
reference to symbol 'dlopen@@GLIBC_2.2.5'
/usr/bin/ld: /usr/lib64/libdl.so.2: error adding symbols: DSO missing
from command line collect2: error: ld returned 1 exit status

Add -rdynamic to the LDFLAGS to add all symbols to the dynamic symbol
table for dwfl-proc-attach.

Signed-off-by: Mark Wielaard <mark@klomp.org>
---
 tests/ChangeLog   | 4 ++++
 tests/Makefile.am | 2 +-
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/tests/ChangeLog b/tests/ChangeLog
index a59cdd51..d0fc3fb0 100644
--- a/tests/ChangeLog
+++ b/tests/ChangeLog
@@ -1,3 +1,7 @@
+2021-11-18  Mark Wielaard  <mark@klomp.org>
+
+	* Makefile.am (dwfl_proc_attach_LDFLAGS): Add -rdynamic.
+
 2021-11-05  Frank Ch. Eigler  <fche@redhat.com>
 
 	PR28430
diff --git a/tests/Makefile.am b/tests/Makefile.am
index bfb8b13a..2298a5ae 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -717,7 +717,7 @@ strptr_LDADD = $(libelf)
 newdata_LDADD = $(libelf)
 elfstrtab_LDADD = $(libelf)
 dwfl_proc_attach_LDADD = $(libdw)
-dwfl_proc_attach_LDFLAGS = -pthread $(AM_LDFLAGS)
+dwfl_proc_attach_LDFLAGS = -pthread -rdynamic $(AM_LDFLAGS)
 elfshphehdr_LDADD =$(libelf)
 elfstrmerge_LDADD = $(libdw) $(libelf)
 dwelfgnucompressed_LDADD = $(libelf) $(libdw)
-- 
2.30.2

Re: [PATCH] tests: Add -ldl to dwfl_proc_attach_LDFLAGS

[Dmitry V. Levin]

On Sat, Nov 20, 2021 at 03:18:27PM +0100, Mark Wielaard wrote:
> Hi,
> 
> On Sat, Nov 20, 2021 at 01:12:17AM +0300, Dmitry V. Levin wrote:
> > On Fri, Nov 19, 2021 at 05:58:19PM +0100, Florian Weimer wrote:
> > > It may have to do with --as-needed that some builds use.  If there are
> > > no pending undefined references, some linkers drop earlier shared object
> > > references with --as-needed (similar to what happens with static
> > > archives).
> > > 
> > > The GCC LTO plugin results in ld looking at more objects in greater
> > > detail for some reason.  Without LTO and --as-needed, you probably don't
> > > get a dlopen export (if you do not link with -E) because indirect
> > > dependencies are not consulted, breaking the valgrind workaround because
> > > there is no interposition.
> > 
> > Thanks.  I suppose adding -rdynamic to dwfl_proc_attach_LDFLAGS should be
> > a more correct fix.
> 
> That works. But I don't really understand why.

If I understand correctly, -rdynamic makes sure the dlopen symbol
is exported by dwfl-proc-attach executable and interposes other dlopen
symbols exported by libdl or libc.  Without -rdynamic, there is a change
that dlopen defined in dwfl-proc-attach.c is optimized out.

> Does the attached patch look OK to you?

Yes, thanks.


-- 
ldv