public inbox for elfutils@sourceware.org
 help / color / mirror / Atom feed
* [PATCH] Introduce public dwfl_get_debuginfod_client API
@ 2022-07-07 14:46 Milian Wolff
  2022-07-07 16:40 ` Aaron Merey
  2022-07-07 17:33 ` Milian Wolff
  0 siblings, 2 replies; 11+ messages in thread
From: Milian Wolff @ 2022-07-07 14:46 UTC (permalink / raw)
  To: elfutils-devel

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

Dwfl can use debuginfod internally, which was so far totally opaque
to the outside. While the functionality is great for users of the
dwfl API, the long wait times induced by downloading of data over
debuginfod lead to complaints by endusers. To offer them a bit more
insight into the internal ongoings, one can now use e.g.
`debuginfod_set_progressfn` on the handle returned by
`dwfl_get_debuginfod_client` to report download progress.

Rename get_client to dwfl_get_debuginfod_client and make it public.
Unconditionally compile debuginfod-client.c and stub the new public
function and always return NULL when debuginfod integration was
disabled.

Signed-off-by: Milian Wolff <mail@milianw.de>
---
 libdw/libdw.map             |  5 +++++
 libdwfl/ChangeLog           |  5 +++++
 libdwfl/Makefile.am         |  5 +----
 libdwfl/debuginfod-client.c | 23 ++++++++++++++++++-----
 libdwfl/libdwfl.h           | 10 ++++++++++
 libdwfl/libdwflP.h          |  1 +
 6 files changed, 40 insertions(+), 9 deletions(-)

diff --git a/libdw/libdw.map b/libdw/libdw.map
index 4f530378..3fdf3f93 100644
--- a/libdw/libdw.map
+++ b/libdw/libdw.map
@@ -366,3 +366,8 @@ ELFUTILS_0.186 {
     dwarf_linecontext;
     dwarf_linefunctionname;
 } ELFUTILS_0.177;
+
+ELFUTILS_0.187 {
+  global:
+    dwfl_get_debuginfod_client;
+} ELFUTILS_0.186;
diff --git a/libdwfl/ChangeLog b/libdwfl/ChangeLog
index b3ca56cb..890df156 100644
--- a/libdwfl/ChangeLog
+++ b/libdwfl/ChangeLog
@@ -1,3 +1,8 @@
+2022-06-22  Milian Wolff <mail@milianw.de>
+
+	* libdwfl.h, debuginfod-client.c (dwfl_get_debuginfod_client):
+	Rename get_client to dwfl_get_debuginfod_client and make it public.
+
 2022-05-15  Mark Wielaard  <mark@klomp.org>
 
 	* libdwfl.h (dwfl_module_addrinfo): Update docs and nonnull
diff --git a/libdwfl/Makefile.am b/libdwfl/Makefile.am
index a0013e41..3278358d 100644
--- a/libdwfl/Makefile.am
+++ b/libdwfl/Makefile.am
@@ -70,7 +70,7 @@ libdwfl_a_SOURCES = dwfl_begin.c dwfl_end.c dwfl_error.c 
dwfl_version.c \
 		    link_map.c core-file.c open.c image-header.c \
 		    dwfl_frame.c frame_unwind.c dwfl_frame_pc.c \
 		    linux-pid-attach.c linux-core-attach.c dwfl_frame_regs.c \
-		    gzip.c
+		    gzip.c debuginfod-client.c
 
 if BZLIB
 libdwfl_a_SOURCES += bzip2.c
@@ -81,9 +81,6 @@ endif
 if ZSTD
 libdwfl_a_SOURCES += zstd.c
 endif
-if LIBDEBUGINFOD
-libdwfl_a_SOURCES += debuginfod-client.c
-endif
 
 libdwfl = $(libdw)
 libdw = ../libdw/libdw.so
diff --git a/libdwfl/debuginfod-client.c b/libdwfl/debuginfod-client.c
index 153260c3..813043b1 100644
--- a/libdwfl/debuginfod-client.c
+++ b/libdwfl/debuginfod-client.c
@@ -32,6 +32,9 @@
 #endif
 
 #include "libdwflP.h"
+
+#ifdef ENABLE_LIBDEBUGINFOD
+
 #include <pthread.h>
 #include <dlfcn.h>
 
@@ -46,8 +49,8 @@ static pthread_once_t init_control = PTHREAD_ONCE_INIT;
 
 /* NB: this is slightly thread-unsafe */
 
-static debuginfod_client *
-get_client (Dwfl *dwfl)
+debuginfod_client *
+dwfl_get_debuginfod_client (Dwfl *dwfl)
 {
   if (dwfl->debuginfod != NULL)
     return dwfl->debuginfod;
@@ -71,7 +74,7 @@ __libdwfl_debuginfod_find_executable (Dwfl *dwfl,
   int fd = -1;
   if (build_id_len > 0)
     {
-      debuginfod_client *c = get_client (dwfl);
+      debuginfod_client *c = dwfl_get_debuginfod_client (dwfl);
       if (c != NULL)
 	fd = (*fp_debuginfod_find_executable) (c, build_id_bits,
 					       build_id_len, NULL);
@@ -88,7 +91,7 @@ __libdwfl_debuginfod_find_debuginfo (Dwfl *dwfl,
   int fd = -1;
   if (build_id_len > 0)
     {
-      debuginfod_client *c = get_client (dwfl);
+      debuginfod_client *c = dwfl_get_debuginfod_client (dwfl);
       if (c != NULL)
 	fd = (*fp_debuginfod_find_debuginfo) (c, build_id_bits,
 					      build_id_len, NULL);
@@ -105,7 +108,7 @@ __libdwfl_debuginfod_end (debuginfod_client *c)
 }
 
 /* Try to get the libdebuginfod library functions.
-   Only needs to be called once from get_client.  */
+   Only needs to be called once from dwfl_get_debuginfod_client.  */
 static void
 __libdwfl_debuginfod_init (void)
 {
@@ -134,3 +137,13 @@ __libdwfl_debuginfod_init (void)
 	}
     }
 }
+
+#else // ENABLE_LIBDEBUGINFOD
+
+debuginfod_client *
+dwfl_get_debuginfod_client (Dwfl *)
+{
+  return NULL;
+}
+
+#endif // ENABLE_LIBDEBUGINFOD
diff --git a/libdwfl/libdwfl.h b/libdwfl/libdwfl.h
index c55a8eaa..b323e8fb 100644
--- a/libdwfl/libdwfl.h
+++ b/libdwfl/libdwfl.h
@@ -49,6 +49,9 @@ typedef struct Dwfl_Thread Dwfl_Thread;
    PC location described by an FDE belonging to Dwfl_Thread.  */
 typedef struct Dwfl_Frame Dwfl_Frame;
 
+/* Handle for debuginfod-client connection.  */
+typedef struct debuginfod_client debuginfod_client;
+
 /* Callbacks.  */
 typedef struct
 {
@@ -795,6 +798,13 @@ int dwfl_getthread_frames (Dwfl *dwfl, pid_t tid,
 bool dwfl_frame_pc (Dwfl_Frame *state, Dwarf_Addr *pc, bool *isactivation)
   __nonnull_attribute__ (1, 2);
 
+/* Return the internal debuginfod-client connection handle for the DWFL 
session.
+   When the client connection has not yet been initialized, it will be done 
on the
+   first call to this function. If elfutils is compiled without support for 
debuginfod,
+   NULL will be returned.
+ */
+extern debuginfod_client *dwfl_get_debuginfod_client (Dwfl *dwfl);
+
 #ifdef __cplusplus
 }
 #endif
diff --git a/libdwfl/libdwflP.h b/libdwfl/libdwflP.h
index 7503a627..9f598370 100644
--- a/libdwfl/libdwflP.h
+++ b/libdwfl/libdwflP.h
@@ -786,6 +786,7 @@ INTDECL (dwfl_getthread_frames)
 INTDECL (dwfl_getthreads)
 INTDECL (dwfl_thread_getframes)
 INTDECL (dwfl_frame_pc)
+INTDECL (dwfl_get_debuginfod_client)
 
 /* Leading arguments standard to callbacks passed a Dwfl_Module.  */
 #define MODCB_ARGS(mod)	(mod), &(mod)->userdata, (mod)->name, (mod)->low_addr
-- 
2.37.0

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] Introduce public dwfl_get_debuginfod_client API
  2022-07-07 14:46 [PATCH] Introduce public dwfl_get_debuginfod_client API Milian Wolff
@ 2022-07-07 16:40 ` Aaron Merey
  2022-07-07 16:58   ` Milian Wolff
  2022-07-07 17:33 ` Milian Wolff
  1 sibling, 1 reply; 11+ messages in thread
From: Aaron Merey @ 2022-07-07 16:40 UTC (permalink / raw)
  To: Milian Wolff; +Cc: elfutils-devel

Hi Milian,

On Thu, Jul 7, 2022 at 10:47 AM Milian Wolff <mail@milianw.de> wrote:
>
> Dwfl can use debuginfod internally, which was so far totally opaque
> to the outside. While the functionality is great for users of the
> dwfl API, the long wait times induced by downloading of data over
> debuginfod lead to complaints by endusers. To offer them a bit more
> insight into the internal ongoings, one can now use e.g.
> `debuginfod_set_progressfn` on the handle returned by
> `dwfl_get_debuginfod_client` to report download progress.
>
> Rename get_client to dwfl_get_debuginfod_client and make it public.
> Unconditionally compile debuginfod-client.c and stub the new public
> function and always return NULL when debuginfod integration was
> disabled.

Thanks for the patch. This looks ok and I was able to successfully
run the testsuite with and without debuginfod enabled.

> @@ -70,7 +70,7 @@ libdwfl_a_SOURCES = dwfl_begin.c dwfl_end.c dwfl_error.c
> dwfl_version.c \

Some line breaks may have accidentally snuck into the patch. I had to
manually remove the line break right after "dwfl_error.c" for git to
apply the patch without error.

> +/* Return the internal debuginfod-client connection handle for the DWFL
> session.
> +   When the client connection has not yet been initialized, it will be done
> on the
> +   first call to this function. If elfutils is compiled without support for
> debuginfod,

Same goes for these line breaks.

Aaron


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] Introduce public dwfl_get_debuginfod_client API
  2022-07-07 16:40 ` Aaron Merey
@ 2022-07-07 16:58   ` Milian Wolff
  2022-07-07 17:09     ` Aaron Merey
  0 siblings, 1 reply; 11+ messages in thread
From: Milian Wolff @ 2022-07-07 16:58 UTC (permalink / raw)
  To: Aaron Merey; +Cc: elfutils-devel

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

On Donnerstag, 7. Juli 2022 18:40:05 CEST Aaron Merey wrote:
> Hi Milian,
> 
> On Thu, Jul 7, 2022 at 10:47 AM Milian Wolff <mail@milianw.de> wrote:
> > Dwfl can use debuginfod internally, which was so far totally opaque
> > to the outside. While the functionality is great for users of the
> > dwfl API, the long wait times induced by downloading of data over
> > debuginfod lead to complaints by endusers. To offer them a bit more
> > insight into the internal ongoings, one can now use e.g.
> > `debuginfod_set_progressfn` on the handle returned by
> > `dwfl_get_debuginfod_client` to report download progress.
> > 
> > Rename get_client to dwfl_get_debuginfod_client and make it public.
> > Unconditionally compile debuginfod-client.c and stub the new public
> > function and always return NULL when debuginfod integration was
> > disabled.
> 
> Thanks for the patch. This looks ok and I was able to successfully
> run the testsuite with and without debuginfod enabled.
> 
> > @@ -70,7 +70,7 @@ libdwfl_a_SOURCES = dwfl_begin.c dwfl_end.c dwfl_error.c
> > dwfl_version.c \
> 
> Some line breaks may have accidentally snuck into the patch. I had to
> manually remove the line break right after "dwfl_error.c" for git to
> apply the patch without error.

Ah yes sorry, I did not think about that when copy'n'pasting the git format-
patch output into my mail client. Should I resend with the line breaks fixed?

Cheers
-- 
Milian Wolff
mail@milianw.de
http://milianw.de

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] Introduce public dwfl_get_debuginfod_client API
  2022-07-07 16:58   ` Milian Wolff
@ 2022-07-07 17:09     ` Aaron Merey
  0 siblings, 0 replies; 11+ messages in thread
From: Aaron Merey @ 2022-07-07 17:09 UTC (permalink / raw)
  To: Milian Wolff; +Cc: elfutils-devel

On Thu, Jul 7, 2022 at 12:59 PM Milian Wolff <mail@milianw.de> wrote:
>
> On Donnerstag, 7. Juli 2022 18:40:05 CEST Aaron Merey wrote:
>>
> > Some line breaks may have accidentally snuck into the patch. I had to
> > manually remove the line break right after "dwfl_error.c" for git to
> > apply the patch without error.
>
> Ah yes sorry, I did not think about that when copy'n'pasting the git format-
> patch output into my mail client. Should I resend with the line breaks fixed?

Sure might as well resend.

Aaron


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH] Introduce public dwfl_get_debuginfod_client API
  2022-07-07 14:46 [PATCH] Introduce public dwfl_get_debuginfod_client API Milian Wolff
  2022-07-07 16:40 ` Aaron Merey
@ 2022-07-07 17:33 ` Milian Wolff
  2022-07-07 17:42   ` Aaron Merey
  1 sibling, 1 reply; 11+ messages in thread
From: Milian Wolff @ 2022-07-07 17:33 UTC (permalink / raw)
  To: elfutils-devel

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

Dwfl can use debuginfod internally, which was so far totally opaque
to the outside. While the functionality is great for users of the
dwfl API, the long wait times induced by downloading of data over
debuginfod lead to complaints by endusers. To offer them a bit more
insight into the internal ongoings, one can now use e.g.
`debuginfod_set_progressfn` on the handle returned by
`dwfl_get_debuginfod_client` to report download progress.

Rename get_client to dwfl_get_debuginfod_client and make it public.
Unconditionally compile debuginfod-client.c and stub the new public
function and always return NULL when debuginfod integration was
disabled.

Signed-off-by: Milian Wolff <mail@milianw.de>
---
 libdw/libdw.map             |  5 +++++
 libdwfl/ChangeLog           |  5 +++++
 libdwfl/Makefile.am         |  5 +----
 libdwfl/debuginfod-client.c | 23 ++++++++++++++++++-----
 libdwfl/libdwfl.h           | 10 ++++++++++
 libdwfl/libdwflP.h          |  1 +
 6 files changed, 40 insertions(+), 9 deletions(-)

diff --git a/libdw/libdw.map b/libdw/libdw.map
index 4f530378..3fdf3f93 100644
--- a/libdw/libdw.map
+++ b/libdw/libdw.map
@@ -366,3 +366,8 @@ ELFUTILS_0.186 {
     dwarf_linecontext;
     dwarf_linefunctionname;
 } ELFUTILS_0.177;
+
+ELFUTILS_0.187 {
+  global:
+    dwfl_get_debuginfod_client;
+} ELFUTILS_0.186;
diff --git a/libdwfl/ChangeLog b/libdwfl/ChangeLog
index b3ca56cb..890df156 100644
--- a/libdwfl/ChangeLog
+++ b/libdwfl/ChangeLog
@@ -1,3 +1,8 @@
+2022-06-22  Milian Wolff <mail@milianw.de>
+
+	* libdwfl.h, debuginfod-client.c (dwfl_get_debuginfod_client):
+	Rename get_client to dwfl_get_debuginfod_client and make it public.
+
 2022-05-15  Mark Wielaard  <mark@klomp.org>
 
 	* libdwfl.h (dwfl_module_addrinfo): Update docs and nonnull
diff --git a/libdwfl/Makefile.am b/libdwfl/Makefile.am
index a0013e41..3278358d 100644
--- a/libdwfl/Makefile.am
+++ b/libdwfl/Makefile.am
@@ -70,7 +70,7 @@ libdwfl_a_SOURCES = dwfl_begin.c dwfl_end.c dwfl_error.c dwfl_version.c \
 		    link_map.c core-file.c open.c image-header.c \
 		    dwfl_frame.c frame_unwind.c dwfl_frame_pc.c \
 		    linux-pid-attach.c linux-core-attach.c dwfl_frame_regs.c \
-		    gzip.c
+		    gzip.c debuginfod-client.c
 
 if BZLIB
 libdwfl_a_SOURCES += bzip2.c
@@ -81,9 +81,6 @@ endif
 if ZSTD
 libdwfl_a_SOURCES += zstd.c
 endif
-if LIBDEBUGINFOD
-libdwfl_a_SOURCES += debuginfod-client.c
-endif
 
 libdwfl = $(libdw)
 libdw = ../libdw/libdw.so
diff --git a/libdwfl/debuginfod-client.c b/libdwfl/debuginfod-client.c
index 153260c3..813043b1 100644
--- a/libdwfl/debuginfod-client.c
+++ b/libdwfl/debuginfod-client.c
@@ -32,6 +32,9 @@
 #endif
 
 #include "libdwflP.h"
+
+#ifdef ENABLE_LIBDEBUGINFOD
+
 #include <pthread.h>
 #include <dlfcn.h>
 
@@ -46,8 +49,8 @@ static pthread_once_t init_control = PTHREAD_ONCE_INIT;
 
 /* NB: this is slightly thread-unsafe */
 
-static debuginfod_client *
-get_client (Dwfl *dwfl)
+debuginfod_client *
+dwfl_get_debuginfod_client (Dwfl *dwfl)
 {
   if (dwfl->debuginfod != NULL)
     return dwfl->debuginfod;
@@ -71,7 +74,7 @@ __libdwfl_debuginfod_find_executable (Dwfl *dwfl,
   int fd = -1;
   if (build_id_len > 0)
     {
-      debuginfod_client *c = get_client (dwfl);
+      debuginfod_client *c = dwfl_get_debuginfod_client (dwfl);
       if (c != NULL)
 	fd = (*fp_debuginfod_find_executable) (c, build_id_bits,
 					       build_id_len, NULL);
@@ -88,7 +91,7 @@ __libdwfl_debuginfod_find_debuginfo (Dwfl *dwfl,
   int fd = -1;
   if (build_id_len > 0)
     {
-      debuginfod_client *c = get_client (dwfl);
+      debuginfod_client *c = dwfl_get_debuginfod_client (dwfl);
       if (c != NULL)
 	fd = (*fp_debuginfod_find_debuginfo) (c, build_id_bits,
 					      build_id_len, NULL);
@@ -105,7 +108,7 @@ __libdwfl_debuginfod_end (debuginfod_client *c)
 }
 
 /* Try to get the libdebuginfod library functions.
-   Only needs to be called once from get_client.  */
+   Only needs to be called once from dwfl_get_debuginfod_client.  */
 static void
 __libdwfl_debuginfod_init (void)
 {
@@ -134,3 +137,13 @@ __libdwfl_debuginfod_init (void)
 	}
     }
 }
+
+#else // ENABLE_LIBDEBUGINFOD
+
+debuginfod_client *
+dwfl_get_debuginfod_client (Dwfl *)
+{
+  return NULL;
+}
+
+#endif // ENABLE_LIBDEBUGINFOD
diff --git a/libdwfl/libdwfl.h b/libdwfl/libdwfl.h
index c55a8eaa..b323e8fb 100644
--- a/libdwfl/libdwfl.h
+++ b/libdwfl/libdwfl.h
@@ -49,6 +49,9 @@ typedef struct Dwfl_Thread Dwfl_Thread;
    PC location described by an FDE belonging to Dwfl_Thread.  */
 typedef struct Dwfl_Frame Dwfl_Frame;
 
+/* Handle for debuginfod-client connection.  */
+typedef struct debuginfod_client debuginfod_client;
+
 /* Callbacks.  */
 typedef struct
 {
@@ -795,6 +798,13 @@ int dwfl_getthread_frames (Dwfl *dwfl, pid_t tid,
 bool dwfl_frame_pc (Dwfl_Frame *state, Dwarf_Addr *pc, bool *isactivation)
   __nonnull_attribute__ (1, 2);
 
+/* Return the internal debuginfod-client connection handle for the DWFL session.
+   When the client connection has not yet been initialized, it will be done on the
+   first call to this function. If elfutils is compiled without support for debuginfod,
+   NULL will be returned.
+ */
+extern debuginfod_client *dwfl_get_debuginfod_client (Dwfl *dwfl);
+
 #ifdef __cplusplus
 }
 #endif
diff --git a/libdwfl/libdwflP.h b/libdwfl/libdwflP.h
index 7503a627..9f598370 100644
--- a/libdwfl/libdwflP.h
+++ b/libdwfl/libdwflP.h
@@ -786,6 +786,7 @@ INTDECL (dwfl_getthread_frames)
 INTDECL (dwfl_getthreads)
 INTDECL (dwfl_thread_getframes)
 INTDECL (dwfl_frame_pc)
+INTDECL (dwfl_get_debuginfod_client)
 
 /* Leading arguments standard to callbacks passed a Dwfl_Module.  */
 #define MODCB_ARGS(mod)	(mod), &(mod)->userdata, (mod)->name, (mod)->low_addr
-- 
2.37.0


[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] Introduce public dwfl_get_debuginfod_client API
  2022-07-07 17:33 ` Milian Wolff
@ 2022-07-07 17:42   ` Aaron Merey
  2022-07-13 18:20     ` Aaron Merey
  0 siblings, 1 reply; 11+ messages in thread
From: Aaron Merey @ 2022-07-07 17:42 UTC (permalink / raw)
  To: Milian Wolff; +Cc: elfutils-devel

Thanks. Let's wait for a maintainer to give it the ok before merging.

Aaron


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] Introduce public dwfl_get_debuginfod_client API
  2022-07-07 17:42   ` Aaron Merey
@ 2022-07-13 18:20     ` Aaron Merey
  2022-07-13 19:36       ` Milian Wolff
  0 siblings, 1 reply; 11+ messages in thread
From: Aaron Merey @ 2022-07-13 18:20 UTC (permalink / raw)
  To: Milian Wolff; +Cc: elfutils-devel

Hi Milian,

There weren't any concerns with the patch so I've gone ahead and merged
it as commit a4b1839c3c46.

Thanks,
Aaron


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] Introduce public dwfl_get_debuginfod_client API
  2022-07-13 18:20     ` Aaron Merey
@ 2022-07-13 19:36       ` Milian Wolff
  2022-07-13 20:35         ` Aaron Merey
  2022-07-13 20:38         ` Mark Wielaard
  0 siblings, 2 replies; 11+ messages in thread
From: Milian Wolff @ 2022-07-13 19:36 UTC (permalink / raw)
  To: Aaron Merey; +Cc: elfutils-devel

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

On Mittwoch, 13. Juli 2022 20:20:04 CEST Aaron Merey wrote:
> Hi Milian,
> 
> There weren't any concerns with the patch so I've gone ahead and merged
> it as commit a4b1839c3c46.

Thank you Aaron!

I got spammed by a flood of mails from buildbot, but from what I can tell the 
issues are all unrelated to my patch? Or did I break something? See e.g.:

[1]: https://builder.sourceware.org/buildbot/#/builders/39/builds/41

Thanks

-- 
Milian Wolff
mail@milianw.de
http://milianw.de

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] Introduce public dwfl_get_debuginfod_client API
  2022-07-13 19:36       ` Milian Wolff
@ 2022-07-13 20:35         ` Aaron Merey
  2022-07-13 20:38         ` Mark Wielaard
  1 sibling, 0 replies; 11+ messages in thread
From: Aaron Merey @ 2022-07-13 20:35 UTC (permalink / raw)
  To: Milian Wolff; +Cc: elfutils-devel

On Wed, Jul 13, 2022 at 3:36 PM Milian Wolff <mail@milianw.de> wrote:
> I got spammed by a flood of mails from buildbot, but from what I can tell the
> issues are all unrelated to my patch? Or did I break something? See e.g.:
>
> [1]: https://builder.sourceware.org/buildbot/#/builders/39/builds/41

These issues are unrelated to your patch. One has been fixed and we
are looking into the ppc/s390x failures.

Aaron


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] Introduce public dwfl_get_debuginfod_client API
  2022-07-13 19:36       ` Milian Wolff
  2022-07-13 20:35         ` Aaron Merey
@ 2022-07-13 20:38         ` Mark Wielaard
  2022-07-13 21:15           ` Milian Wolff
  1 sibling, 1 reply; 11+ messages in thread
From: Mark Wielaard @ 2022-07-13 20:38 UTC (permalink / raw)
  To: Milian Wolff; +Cc: Aaron Merey, elfutils-devel

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

Hi Milian,

On Wed, Jul 13, 2022 at 09:36:45PM +0200, Milian Wolff wrote:
> On Mittwoch, 13. Juli 2022 20:20:04 CEST Aaron Merey wrote:
> > There weren't any concerns with the patch so I've gone ahead and merged
> > it as commit a4b1839c3c46.

Yeah, sorry, I should have spoken up on the list not just mumbled
something on irc. My only concern was that I was afraid to pull in
libdebuginfod.h api by default. But you handled to by making
debuginfod_client handle an opaque pointer, which it of course is. So
my worry was unfounded.

But I did just now see it has a typo in libdw.map:

ELFUTILS_0.187 {
  global:
    dwfl_get_debuginfod_client;
} ELFUTILS_0.186;

That should be ELFUTILS_0.188 we already released 0.187.
Also (micro optimazition to prevent a PLT call) the internal calls
should use INTUSE (dwfl_get_debuginfod_client).

I made both changes and added a NEWS entry. See attached.

> I got spammed by a flood of mails from buildbot, but from what I can tell the 
> issues are all unrelated to my patch? Or did I break something? See e.g.:
> 
> [1]: https://builder.sourceware.org/buildbot/#/builders/39/builds/41

Those are unrelated to your patch, but a typo in the buildbot step:
https://sourceware.org/git/?p=builder.git;a=commitdiff;h=cc655e64acb45b2904cb4ce3169c8be92422e2c4;hp=7de05787acb62998c0fbbeec7ee6489700910962

Most builders are now green again:
https://builder.sourceware.org/buildbot/#/builders?tags=elfutils

There is however a new failure because of a new glibc/gcc
fortification on ppc64le and s390x:
https://builder.sourceware.org/buildbot/#/builders/43/builds/41
https://builder.sourceware.org/buildbot/#/builders/55/builds/40

These are also unrelated to your patch. The error is:

In file included from /usr/include/ar.h:22,
                 from ../libelf/libelfP.h:33,
                 from core-file.c:31:
In function ‘pread’,
    inlined from ‘pread_retry’ at ../lib/system.h:188:21,
    inlined from ‘elf_begin_rand’ at core-file.c:86:16,
    inlined from ‘core_file_read_eagerly’ at core-file.c:205:15:
/usr/include/bits/unistd.h:74:10: error: ‘__pread_alias’ writing 58 or more bytes into a region of size 10 overflows the destination [-Werror=stringop-overflow=]
   74 |   return __glibc_fortify (pread, __nbytes, sizeof (char),
      |          ^~~~~~~~~~~~~~~
/usr/include/ar.h: In function ‘core_file_read_eagerly’:
/usr/include/ar.h:41:10: note: destination object ‘ar_size’ of size 10
   41 |     char ar_size[10];           /* File size, in ASCII decimal.  */
      |          ^~~~~~~
/usr/include/bits/unistd.h:50:16: note: in a call to function ‘__pread_alias’ declared with attribute ‘access (write_only, 2, 3)’
   50 | extern ssize_t __REDIRECT (__pread_alias,
      |                ^~~~~~~~~~
cc1: all warnings being treated as errors

That needs investigation. The ar file format is a little odd, those
fields are in (non-zero terminated) ASCII decimal notation. Which
probably confuses the fortification.

Cheers,

Mark

[-- Attachment #2: 0001-Move-dwfl_get_debuginfod_client-to-ELFUTILS_0.188.patch --]
[-- Type: text/x-diff, Size: 3564 bytes --]

From 2fb8571e78852cc71c0128705f500490d79f8a94 Mon Sep 17 00:00:00 2001
From: Mark Wielaard <mark@klomp.org>
Date: Wed, 13 Jul 2022 22:34:36 +0200
Subject: [PATCH] Move dwfl_get_debuginfod_client to ELFUTILS_0.188

0.187 was already released, so add new function to 0.188. Also add
NEWS entry and INTUSE.

Signed-off-by: Mark Wielaard <mark@klomp.org>
---
 ChangeLog                   | 4 ++++
 NEWS                        | 2 ++
 libdw/ChangeLog             | 5 +++++
 libdw/libdw.map             | 2 +-
 libdwfl/ChangeLog           | 7 +++++++
 libdwfl/debuginfod-client.c | 5 +++--
 6 files changed, 22 insertions(+), 3 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index c5e43e8c..eb2a35f5 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,7 @@
+2022-07-13  Mark Wielaard  <mark@klomp.org>
+
+	* NEWS: Add dwfl_get_debuginfod_client.
+
 2022-06-02  Mark Wielaard  <mark@klomp.org>
 
 	* configure.ac (OLD_LIBMICROHTTPD): New AM_CONDITIONAL based on
diff --git a/NEWS b/NEWS
index c133fcbe..392f2edc 100644
--- a/NEWS
+++ b/NEWS
@@ -2,6 +2,8 @@ Version 0.188 some time after 0.187
 
 debuginfod: Add --disable-source-scan option.
 
+libdwfl: Add new function dwfl_get_debuginfod_client.
+
 Version 0.187
 
 debuginfod: Support -C option for connection thread pooling.
diff --git a/libdw/ChangeLog b/libdw/ChangeLog
index b254d9cd..6a8f7e51 100644
--- a/libdw/ChangeLog
+++ b/libdw/ChangeLog
@@ -1,3 +1,8 @@
+2022-07-13  Mark Wielaard  <mark@klomp.org>
+
+	* libdw.map (ELFUTILS_0.187): Renamed to...
+	(ELFUTILS_0.188): ... this.
+
 2022-05-09  Mark Wielaard  <mark@klomp.org>
 
 	* dwarf_getlocation.c (store_implicit_value): Check block length.
diff --git a/libdw/libdw.map b/libdw/libdw.map
index 3fdf3f93..6da25561 100644
--- a/libdw/libdw.map
+++ b/libdw/libdw.map
@@ -367,7 +367,7 @@ ELFUTILS_0.186 {
     dwarf_linefunctionname;
 } ELFUTILS_0.177;
 
-ELFUTILS_0.187 {
+ELFUTILS_0.188 {
   global:
     dwfl_get_debuginfod_client;
 } ELFUTILS_0.186;
diff --git a/libdwfl/ChangeLog b/libdwfl/ChangeLog
index 890df156..6ade1afc 100644
--- a/libdwfl/ChangeLog
+++ b/libdwfl/ChangeLog
@@ -1,3 +1,10 @@
+2022-07-13  Mark Wielaard  <mark@klomp.org>
+
+	* debuginfod-client.c (dwfl_get_debuginfod_client): Add INTDEF.
+	(__libdwfl_debuginfod_find_executable): Use
+	INTUSE (dwfl_get_debuginfod_client).
+	(__libdwfl_debuginfod_find_debuginfo): Likewise.
+
 2022-06-22  Milian Wolff <mail@milianw.de>
 
 	* libdwfl.h, debuginfod-client.c (dwfl_get_debuginfod_client):
diff --git a/libdwfl/debuginfod-client.c b/libdwfl/debuginfod-client.c
index 813043b1..87af73a7 100644
--- a/libdwfl/debuginfod-client.c
+++ b/libdwfl/debuginfod-client.c
@@ -65,6 +65,7 @@ dwfl_get_debuginfod_client (Dwfl *dwfl)
 
   return NULL;
 }
+INTDEF(dwfl_get_debuginfod_client)
 
 int
 __libdwfl_debuginfod_find_executable (Dwfl *dwfl,
@@ -74,7 +75,7 @@ __libdwfl_debuginfod_find_executable (Dwfl *dwfl,
   int fd = -1;
   if (build_id_len > 0)
     {
-      debuginfod_client *c = dwfl_get_debuginfod_client (dwfl);
+      debuginfod_client *c = INTUSE (dwfl_get_debuginfod_client) (dwfl);
       if (c != NULL)
 	fd = (*fp_debuginfod_find_executable) (c, build_id_bits,
 					       build_id_len, NULL);
@@ -91,7 +92,7 @@ __libdwfl_debuginfod_find_debuginfo (Dwfl *dwfl,
   int fd = -1;
   if (build_id_len > 0)
     {
-      debuginfod_client *c = dwfl_get_debuginfod_client (dwfl);
+      debuginfod_client *c = INTUSE (dwfl_get_debuginfod_client) (dwfl);
       if (c != NULL)
 	fd = (*fp_debuginfod_find_debuginfo) (c, build_id_bits,
 					      build_id_len, NULL);
-- 
2.30.2


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] Introduce public dwfl_get_debuginfod_client API
  2022-07-13 20:38         ` Mark Wielaard
@ 2022-07-13 21:15           ` Milian Wolff
  0 siblings, 0 replies; 11+ messages in thread
From: Milian Wolff @ 2022-07-13 21:15 UTC (permalink / raw)
  To: Mark Wielaard; +Cc: Aaron Merey, elfutils-devel

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

On Mittwoch, 13. Juli 2022 22:38:51 CEST Mark Wielaard wrote:
> Hi Milian,
> 
> On Wed, Jul 13, 2022 at 09:36:45PM +0200, Milian Wolff wrote:
> > On Mittwoch, 13. Juli 2022 20:20:04 CEST Aaron Merey wrote:
> > > There weren't any concerns with the patch so I've gone ahead and merged
> > > it as commit a4b1839c3c46.
> 
> Yeah, sorry, I should have spoken up on the list not just mumbled
> something on irc. My only concern was that I was afraid to pull in
> libdebuginfod.h api by default. But you handled to by making
> debuginfod_client handle an opaque pointer, which it of course is. So
> my worry was unfounded.
> 
> But I did just now see it has a typo in libdw.map:
> 
> ELFUTILS_0.187 {
>   global:
>     dwfl_get_debuginfod_client;
> } ELFUTILS_0.186;
> 
> That should be ELFUTILS_0.188 we already released 0.187.

Ah, thanks for spotting that.

> Also (micro optimazition to prevent a PLT call) the internal calls
> should use INTUSE (dwfl_get_debuginfod_client).

TIL, thanks. I probably will never feel at home in C code bases with all these 
arcane macros :D

> I made both changes and added a NEWS entry. See attached.

LGTM.

> > I got spammed by a flood of mails from buildbot, but from what I can tell
> > the issues are all unrelated to my patch? Or did I break something? See
> > e.g.:
> > 
> > [1]: https://builder.sourceware.org/buildbot/#/builders/39/builds/41
> 
> Those are unrelated to your patch

OK, thanks.

-- 
Milian Wolff
mail@milianw.de
http://milianw.de

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2022-07-13 21:15 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-07-07 14:46 [PATCH] Introduce public dwfl_get_debuginfod_client API Milian Wolff
2022-07-07 16:40 ` Aaron Merey
2022-07-07 16:58   ` Milian Wolff
2022-07-07 17:09     ` Aaron Merey
2022-07-07 17:33 ` Milian Wolff
2022-07-07 17:42   ` Aaron Merey
2022-07-13 18:20     ` Aaron Merey
2022-07-13 19:36       ` Milian Wolff
2022-07-13 20:35         ` Aaron Merey
2022-07-13 20:38         ` Mark Wielaard
2022-07-13 21:15           ` Milian Wolff

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).