Re: [PATCH] Drop $(EXEEXT) suffix from shared libraries
On Mon, 2020-11-30 at 21:35 +0100, Mark Wielaard wrote: > On Mon, Nov 30, 2020 at 08:00:00AM +, Dmitry V. Levin wrote: > > According to GNU Automake documentation [1], $(EXEEXT) is the > > suffix > > that should be used for executables, it is not applicable for > > shared libraries. > > > > [1] > > https://www.gnu.org/software/automake/manual/html_node/EXEEXT.html > > I think you are right and this patch is correct. This probably only > affects non-posix or windows setups. Ulf (on CC now) was working on > building on windows (although I think we never integrated all the > patches for that). Maybe he has an opinion on this patch? Lets assume that no objection equals patch is good (for now, we can always revert or change it later of course). Pushed. Thanks, Mark
Re: [PATCH] debuginfod: export DEBUGINFOD_SONAME macro in debuginfod.h
Hi Dmitry, On Mon, 2020-11-30 at 09:00 +, Dmitry V. Levin wrote: > Add DEBUGINFOD_SONAME macro to API for use by those of libdebuginfod > clients that would like to dlopen the library in the same way as > __libdwfl_debuginfod_init does. I can see how this is useful, but shouldn't libdwfl/debuginfod-client.c then also use this method/new constants? Don't we need both versioned and versionless names (at least dwfl tries the versioned one first, then falls back to the unversioned one). It would be nice to see documentation in doc/debuginfod_find_debuginfo.3 Finally, I am actually using the Makefile VERSION variable in a downstream (DTS) to make sure the so name of all libraries is different from the system one. This is just a minor issue though, and I should probably upstream a tweak to do this upstream so others can also more easily use this. Cheers, Mark
Re: [PATCH] debuginfod: export DEBUGINFOD_SONAME macro in debuginfod.h
Hi Mark, On Sun, Dec 06, 2020 at 01:06:42PM +0100, Mark Wielaard wrote: > Hi Dmitry, > > On Mon, 2020-11-30 at 09:00 +, Dmitry V. Levin wrote: > > Add DEBUGINFOD_SONAME macro to API for use by those of libdebuginfod > > clients that would like to dlopen the library in the same way as > > __libdwfl_debuginfod_init does. > > I can see how this is useful, but shouldn't libdwfl/debuginfod-client.c > then also use this method/new constants? Thanks, I think libdwfl/debuginfod-client.c should use the versioned name only, and it shouldn't fallback to "libdebuginfod.so" as it does now. I'll submit a separate patch to address that. > Don't we need both versioned and versionless names (at least dwfl tries > the versioned one first, then falls back to the unversioned one). I don't think the versioned name should be exported because it changes in every version while clients don't have to be rebuilt that often. > It would be nice to see documentation in > doc/debuginfod_find_debuginfo.3 Yes, it would be nice, agreed. > Finally, I am actually using the Makefile VERSION variable in a > downstream (DTS) to make sure the so name of all libraries is different > from the system one. This is just a minor issue though, and I should > probably upstream a tweak to do this upstream so others can also more > easily use this. Do you suggest to keep the Makefile VERSION variable? It would become an unused variable with the remaining part of the patch applied unless you upstream the tweak you are talking about. -- ldv
Re: [PATCH 1/3] link_map: Pull release_buffer() into file scope
Hi Timm,
On Tue, 2020-12-01 at 09:38 +0100, Timm Bäder via Elfutils-devel wrote:
> From: Timm Bäder
>
> Get rid of another nested function
It is missing a ChangeLog entry.
> diff --git a/libdwfl/link_map.c b/libdwfl/link_map.c
> index 29307c74..5c39c631 100644
> --- a/libdwfl/link_map.c
> +++ b/libdwfl/link_map.c
> @@ -225,6 +225,18 @@ addrsize (uint_fast8_t elfclass)
>return elfclass * 4;
> }
>
> +static inline int
> +release_buffer (Dwfl *dwfl,
> +Dwfl_Memory_Callback *memory_callback, void
> *memory_callback_arg,
> +void **buffer, size_t *buffer_available,
> +int result)
> +{
> + if (buffer != NULL)
> +(void) (*memory_callback) (dwfl, -1, buffer, buffer_available, 0, 0,
> + memory_callback_arg);
> + return result;
> +}
Note that this changes the semantics slightly. Because you now take the
address of the buffer variable before checking it is NULL (it now never
is). Also note that the result is not always used, so it might be
cleaner to not pass it around. It is IMHO better to fully inline it.
> @@ -249,13 +261,6 @@ report_r_debug (uint_fast8_t elfclass, uint_fast8_t
> elfdata,
>
>void *buffer = NULL;
>size_t buffer_available = 0;
> - inline int release_buffer (int result)
> - {
> -if (buffer != NULL)
> - (void) (*memory_callback) (dwfl, -1, &buffer, &buffer_available, 0, 0,
> -memory_callback_arg);
> -return result;
> - }
>
>GElf_Addr addrs[4];
>inline bool read_addrs (GElf_Addr vaddr, size_t n)
> [...]
> @@ -304,7 +310,9 @@ report_r_debug (uint_fast8_t elfclass, uint_fast8_t
> elfdata,
>}
>
>if (unlikely (read_addrs (read_vaddr, 1)))
> -return release_buffer (-1);
> +release_buffer (dwfl, memory_callback, memory_callback_arg,
> +&buffer, &buffer_available, -1);
> +
Note that here the result is used, but the change doesn't use it
anymore and so doesn't return early but just tries to carry on after an
error occurred.
Cheers,
Mark
Re: [PATCH] debuginfod: export DEBUGINFOD_SONAME macro in debuginfod.h
Hi - > Thanks, I think libdwfl/debuginfod-client.c should use the versioned name > only, and it shouldn't fallback to "libdebuginfod.so" as it does now. I believe the code falls back to the unversioned soname because only that is available in the build tree (as opposed to the install tree). - FChE
Re: [PATCH] debuginfod: export DEBUGINFOD_SONAME macro in debuginfod.h
On Sun, Dec 06, 2020 at 08:32:43AM -0500, Frank Ch. Eigler wrote: > Hi - > > > Thanks, I think libdwfl/debuginfod-client.c should use the versioned name > > only, and it shouldn't fallback to "libdebuginfod.so" as it does now. > > I believe the code falls back to the unversioned soname because only > that is available in the build tree (as opposed to the install tree). Yes, and this is exactly the thing I'm going to fix. -- ldv
Re: [PATCH 3/3] link_map: Inline consider_phdr() into only caller
Hi Timm, On Tue, 2020-12-01 at 09:38 +0100, Timm Bäder via Elfutils-devel wrote: > This gets rid of the tested function and is shorter. It is slightly shorter indeed. Although I changed a couple of lines to not exceed 80 chars. And I added a ChangeLog entry to libdwfl/ChangeLog. Pushed. Mark
Re: link_map: Remove nested functions
Hi Timm, On Tue, 2020-12-01 at 09:38 +0100, Timm Bäder via Elfutils-devel wrote: > the attached patches get rid of nested functions in > libdwfl/link_map.c. > > I wrote these a while back and just looked at them again and we could > use the same read_state struct here as well. I just quickly checked, > but it seems a bit more involved due to the > integrated_memory_callback > handling. I can look into that anyway if required. I had some comments on the first patch, the third patch seems fine and has been committed. If you have adjust for the changes in the first patch it would be nice if you could look at making the second patch read_addrs function take a bit less than 11 arguments, Note that all patches were missing ChangeLog entries and Signed-off-by lines. Cheers, Mark
[PATCH v2 1/3] debuginfod: export DEBUGINFOD_SONAME macro in debuginfod.h
Add DEBUGINFOD_SONAME macro to API for use by those of libdebuginfod
clients that would like to dlopen the library in the same way as
__libdwfl_debuginfod_init does.
Signed-off-by: Dmitry V. Levin
---
ChangeLog| 5 +
configure.ac | 5 -
debuginfod/ChangeLog | 8
debuginfod/Makefile.am | 12 ++--
debuginfod/{debuginfod.h => debuginfod.h.in} | 3 +++
doc/ChangeLog| 4
doc/debuginfod_find_debuginfo.3 | 15 +++
7 files changed, 45 insertions(+), 7 deletions(-)
rename debuginfod/{debuginfod.h => debuginfod.h.in} (97%)
diff --git a/ChangeLog b/ChangeLog
index 565d021c..71e80a25 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,8 @@
+2020-11-30 Dmitry V. Levin
+
+ * configure.ac (LIBDEBUGINFOD_SONAME): New AC_SUBST variable.
+ (AC_CONFIG_FILES): Add debuginfod/debuginfod.h.
+
2020-11-01 Érico N. Rolim
* configure.ac: Check for fts and obstack from outside libc.
diff --git a/configure.ac b/configure.ac
index c1a6954d..2f7316e8 100644
--- a/configure.ac
+++ b/configure.ac
@@ -25,6 +25,9 @@ m4_ifndef([AC_PACKAGE_URL],
[Define to home page for this package])
AC_SUBST([PACKAGE_URL], ["http://elfutils.org/";])])
+LIBDEBUGINFOD_SONAME=libdebuginfod.so.1
+AC_SUBST([LIBDEBUGINFOD_SONAME])
+
# We want eu- as default program prefix if none was given by the user.
# But if the user explicitly provided --program-prefix="" then pretend
# it wasn't set at all (NONE). We want to test this really early before
@@ -61,7 +64,7 @@ dnl The RPM spec file. We substitute a few values in the
file.
AC_CONFIG_FILES([elfutils.spec:config/elfutils.spec.in])
dnl debuginfo-server client & server parts.
-AC_CONFIG_FILES([debuginfod/Makefile])
+AC_CONFIG_FILES([debuginfod/Makefile debuginfod/debuginfod.h])
AC_CANONICAL_HOST
diff --git a/debuginfod/ChangeLog b/debuginfod/ChangeLog
index 3039371f..07899289 100644
--- a/debuginfod/ChangeLog
+++ b/debuginfod/ChangeLog
@@ -1,5 +1,13 @@
2020-11-30 Dmitry V. Levin
+ * Makefile.am (libdebuginfod.so): Replace $@.$(VERSION) with
+ $(LIBDEBUGINFOD_SONAME).
+ (install, uninstall, MOSTLYCLEANFILES): Replace
+ libdebuginfod.so.$(VERSION) with $(LIBDEBUGINFOD_SONAME).
+ * debuginfod.h: Rename to ...
+ * debuginfod.h.in ... this.
+ (DEBUGINFOD_SONAME): New macro.
+
* Makefile.am (libdebuginfod.so$(EXEEXT)): Drop $(EXEEXT) suffix.
2020-11-25 Frank Ch. Eigler
diff --git a/debuginfod/Makefile.am b/debuginfod/Makefile.am
index 352b4915..34f34acf 100644
--- a/debuginfod/Makefile.am
+++ b/debuginfod/Makefile.am
@@ -102,30 +102,30 @@ libdebuginfod_so_LDLIBS = $(libcurl_LIBS) $(fts_LIBS)
endif
libdebuginfod.so: $(srcdir)/libdebuginfod.map $(libdebuginfod_so_LIBS)
$(AM_V_CCLD)$(LINK) $(dso_LDFLAGS) -o $@ \
- -Wl,--soname,$@.$(VERSION) \
+ -Wl,--soname,$(LIBDEBUGINFOD_SONAME) \
-Wl,--version-script,$<,--no-undefined \
-Wl,--whole-archive $(libdebuginfod_so_LIBS)
-Wl,--no-whole-archive \
$(libdebuginfod_so_LDLIBS)
@$(textrel_check)
- $(AM_V_at)ln -fs $@ $@.$(VERSION)
+ $(AM_V_at)ln -fs $@ $(LIBDEBUGINFOD_SONAME)
endif
if LIBDEBUGINFOD
install: install-am libdebuginfod.so
$(mkinstalldirs) $(DESTDIR)$(libdir)
$(INSTALL_PROGRAM) libdebuginfod.so
$(DESTDIR)$(libdir)/libdebuginfod-$(PACKAGE_VERSION).so
- ln -fs libdebuginfod-$(PACKAGE_VERSION).so
$(DESTDIR)$(libdir)/libdebuginfod.so.$(VERSION)
- ln -fs libdebuginfod.so.$(VERSION) $(DESTDIR)$(libdir)/libdebuginfod.so
+ ln -fs libdebuginfod-$(PACKAGE_VERSION).so
$(DESTDIR)$(libdir)/$(LIBDEBUGINFOD_SONAME)
+ ln -fs libdebuginfod-$(PACKAGE_VERSION).so
$(DESTDIR)$(libdir)/libdebuginfod.so
uninstall: uninstall-am
rm -f $(DESTDIR)$(libdir)/libdebuginfod-$(PACKAGE_VERSION).so
- rm -f $(DESTDIR)$(libdir)/libdebuginfod.so.$(VERSION)
+ rm -f $(DESTDIR)$(libdir)/$(LIBDEBUGINFOD_SONAME)
rm -f $(DESTDIR)$(libdir)/libdebuginfod.so
rmdir --ignore-fail-on-non-empty $(DESTDIR)$(includedir)/elfutils
endif
EXTRA_DIST = libdebuginfod.map
-MOSTLYCLEANFILES = $(am_libdebuginfod_pic_a_OBJECTS)
libdebuginfod.so.$(VERSION)
+MOSTLYCLEANFILES = $(am_libdebuginfod_pic_a_OBJECTS) $(LIBDEBUGINFOD_SONAME)
CLEANFILES += $(am_libdebuginfod_pic_a_OBJECTS) libdebuginfod.so
# automake std-options override: arrange to pass LD_LIBRARY_PATH
diff --git a/debuginfod/debuginfod.h b/debuginfod/debuginfod.h.in
similarity index 97%
rename from debuginfod/debuginfod.h
rename to debuginfod/debuginfod.h.in
index 4ee86ce9..559ea947 100644
--- a/debuginfod/debuginfod.h
+++ b/debuginfod/debuginfod.h.in
@@ -36,6 +36,9 @@
#define DEBUGINFOD_PROGRESS_ENV_VAR "DEBUGINFOD_PROGRESS"
#define
[PATCH v2 2/3] debuginfod: create the fully versioned libdebuginfod file name during build
When the fully versioned file name is available, the fall back to dlopen of "libdebuginfod.so" in __libdwfl_debuginfod_init is no longer needed. Signed-off-by: Dmitry V. Levin --- debuginfod/ChangeLog | 7 +++ debuginfod/Makefile.am | 10 ++ 2 files changed, 13 insertions(+), 4 deletions(-) diff --git a/debuginfod/ChangeLog b/debuginfod/ChangeLog index 07899289..27abdfda 100644 --- a/debuginfod/ChangeLog +++ b/debuginfod/ChangeLog @@ -1,3 +1,10 @@ +2020-12-06 Dmitry V. Levin + + * Makefile.am (LIBDEBUGINFOD_FULL_NAME): New variable. + (libdebuginfod.so): Use it to create the fully versioned file name. + (install, uninstall): Replace libdebuginfod-$(PACKAGE_VERSION).so with + $(LIBDEBUGINFOD_FULL_NAME). + 2020-11-30 Dmitry V. Levin * Makefile.am (libdebuginfod.so): Replace $@.$(VERSION) with diff --git a/debuginfod/Makefile.am b/debuginfod/Makefile.am index 34f34acf..7df66125 100644 --- a/debuginfod/Makefile.am +++ b/debuginfod/Makefile.am @@ -100,6 +100,7 @@ libdebuginfod_so_LDLIBS = else libdebuginfod_so_LDLIBS = $(libcurl_LIBS) $(fts_LIBS) endif +LIBDEBUGINFOD_FULL_NAME = libdebuginfod-$(PACKAGE_VERSION).so libdebuginfod.so: $(srcdir)/libdebuginfod.map $(libdebuginfod_so_LIBS) $(AM_V_CCLD)$(LINK) $(dso_LDFLAGS) -o $@ \ -Wl,--soname,$(LIBDEBUGINFOD_SONAME) \ @@ -107,18 +108,19 @@ libdebuginfod.so: $(srcdir)/libdebuginfod.map $(libdebuginfod_so_LIBS) -Wl,--whole-archive $(libdebuginfod_so_LIBS) -Wl,--no-whole-archive \ $(libdebuginfod_so_LDLIBS) @$(textrel_check) + $(AM_V_at)ln -fs $@ $(LIBDEBUGINFOD_FULL_NAME) $(AM_V_at)ln -fs $@ $(LIBDEBUGINFOD_SONAME) endif if LIBDEBUGINFOD install: install-am libdebuginfod.so $(mkinstalldirs) $(DESTDIR)$(libdir) - $(INSTALL_PROGRAM) libdebuginfod.so $(DESTDIR)$(libdir)/libdebuginfod-$(PACKAGE_VERSION).so - ln -fs libdebuginfod-$(PACKAGE_VERSION).so $(DESTDIR)$(libdir)/$(LIBDEBUGINFOD_SONAME) - ln -fs libdebuginfod-$(PACKAGE_VERSION).so $(DESTDIR)$(libdir)/libdebuginfod.so + $(INSTALL_PROGRAM) libdebuginfod.so $(DESTDIR)$(libdir)/$(LIBDEBUGINFOD_FULL_NAME) + ln -fs $(LIBDEBUGINFOD_FULL_NAME) $(DESTDIR)$(libdir)/$(LIBDEBUGINFOD_SONAME) + ln -fs $(LIBDEBUGINFOD_FULL_NAME) $(DESTDIR)$(libdir)/libdebuginfod.so uninstall: uninstall-am - rm -f $(DESTDIR)$(libdir)/libdebuginfod-$(PACKAGE_VERSION).so + rm -f $(DESTDIR)$(libdir)/$(LIBDEBUGINFOD_FULL_NAME) rm -f $(DESTDIR)$(libdir)/$(LIBDEBUGINFOD_SONAME) rm -f $(DESTDIR)$(libdir)/libdebuginfod.so rmdir --ignore-fail-on-non-empty $(DESTDIR)$(includedir)/elfutils -- ldv
[PATCH v2 3/3] libdwfl: do not fall back to dlopen of libdebuginfod.so
The fully versioned libdebuginfod file name should always be available.
Signed-off-by: Dmitry V. Levin
---
libdwfl/ChangeLog | 5 +
libdwfl/debuginfod-client.c | 3 ---
2 files changed, 5 insertions(+), 3 deletions(-)
diff --git a/libdwfl/ChangeLog b/libdwfl/ChangeLog
index 67a4d743..2f815b54 100644
--- a/libdwfl/ChangeLog
+++ b/libdwfl/ChangeLog
@@ -1,3 +1,8 @@
+2020-12-06 Dmitry V. Levin
+
+ * debuginfod-client.c (__libdwfl_debuginfod_init): Do not fall back
+ to dlopen of "libdebuginfod.so".
+
2020-11-28 Mark Wielaard
* dwfl_segment_report_module.c (dwfl_segment_report_module):
diff --git a/libdwfl/debuginfod-client.c b/libdwfl/debuginfod-client.c
index ee604ad9..2c4e1a16 100644
--- a/libdwfl/debuginfod-client.c
+++ b/libdwfl/debuginfod-client.c
@@ -103,9 +103,6 @@ __libdwfl_debuginfod_init (void)
{
void *debuginfod_so = dlopen("libdebuginfod-" VERSION ".so", RTLD_LAZY);
- if (debuginfod_so == NULL)
-debuginfod_so = dlopen("libdebuginfod.so", RTLD_LAZY);
-
if (debuginfod_so != NULL)
{
fp_debuginfod_begin = dlsym (debuginfod_so, "debuginfod_begin");
--
ldv
[PATCH v2 4/3] debuginfod: create libdebuginfod files using a race-free method
Before this change, both the soname and the fully versioned libdebuginfod file name were created after libdebuginfod.so, opening a race for all libdebuginfod users that have Makefile dependencies on libdebuginfod.so but actually use libdebuginfod.so.1 or the fully versioned libdebuginfod file name. After this change, the libdebuginfod.so symlink is created the last of all libdebuginfod files, closing the race. --- debuginfod/ChangeLog | 4 debuginfod/Makefile.am | 12 2 files changed, 12 insertions(+), 4 deletions(-) diff --git a/debuginfod/ChangeLog b/debuginfod/ChangeLog index 27abdfda..03eabb6b 100644 --- a/debuginfod/ChangeLog +++ b/debuginfod/ChangeLog @@ -1,5 +1,9 @@ 2020-12-06 Dmitry V. Levin + * Makefile.am [LIBDEBUGINFOD]: Create the fully versioned + libdebuginfod file name first, turn the soname and libdebuginfod.so + into symlinks. + * Makefile.am (LIBDEBUGINFOD_FULL_NAME): New variable. (libdebuginfod.so): Use it to create the fully versioned file name. (install, uninstall): Replace libdebuginfod-$(PACKAGE_VERSION).so with diff --git a/debuginfod/Makefile.am b/debuginfod/Makefile.am index 7df66125..b667ad6d 100644 --- a/debuginfod/Makefile.am +++ b/debuginfod/Makefile.am @@ -101,21 +101,25 @@ else libdebuginfod_so_LDLIBS = $(libcurl_LIBS) $(fts_LIBS) endif LIBDEBUGINFOD_FULL_NAME = libdebuginfod-$(PACKAGE_VERSION).so -libdebuginfod.so: $(srcdir)/libdebuginfod.map $(libdebuginfod_so_LIBS) +$(LIBDEBUGINFOD_FULL_NAME): $(srcdir)/libdebuginfod.map $(libdebuginfod_so_LIBS) $(AM_V_CCLD)$(LINK) $(dso_LDFLAGS) -o $@ \ -Wl,--soname,$(LIBDEBUGINFOD_SONAME) \ -Wl,--version-script,$<,--no-undefined \ -Wl,--whole-archive $(libdebuginfod_so_LIBS) -Wl,--no-whole-archive \ $(libdebuginfod_so_LDLIBS) @$(textrel_check) - $(AM_V_at)ln -fs $@ $(LIBDEBUGINFOD_FULL_NAME) - $(AM_V_at)ln -fs $@ $(LIBDEBUGINFOD_SONAME) + +$(LIBDEBUGINFOD_SONAME): $(LIBDEBUGINFOD_FULL_NAME) + ln -fs $< $@ + +libdebuginfod.so: $(LIBDEBUGINFOD_SONAME) + ln -fs $< $@ endif if LIBDEBUGINFOD install: install-am libdebuginfod.so $(mkinstalldirs) $(DESTDIR)$(libdir) - $(INSTALL_PROGRAM) libdebuginfod.so $(DESTDIR)$(libdir)/$(LIBDEBUGINFOD_FULL_NAME) + $(INSTALL_PROGRAM) $(LIBDEBUGINFOD_FULL_NAME) $(DESTDIR)$(libdir)/ ln -fs $(LIBDEBUGINFOD_FULL_NAME) $(DESTDIR)$(libdir)/$(LIBDEBUGINFOD_SONAME) ln -fs $(LIBDEBUGINFOD_FULL_NAME) $(DESTDIR)$(libdir)/libdebuginfod.so -- ldv
