Remove nested functions from libdwfl V2

2020-11-23 Thread Timm Bäder via Elfutils-devel
Hi again,

version 2 of this patch set. I removed segmend_read() entirely now,
which meant modifying a bunch of later patches. Other than that, they
are the same.

Hope the --from to git send-email worked out, too.


Thanks,
Timm





[PATCH 01/12] segment_report_module: Get rid of segment_read()

2020-11-23 Thread Timm Bäder via Elfutils-devel
From: Timm Bäder 

Just inline the memory_callback call everywhere segmenty_read was used.

Signed-off-by: Timm Bäder 
---
 libdwfl/dwfl_segment_report_module.c | 24 +---
 1 file changed, 9 insertions(+), 15 deletions(-)

diff --git a/libdwfl/dwfl_segment_report_module.c 
b/libdwfl/dwfl_segment_report_module.c
index c7725002..c587efd7 100644
--- a/libdwfl/dwfl_segment_report_module.c
+++ b/libdwfl/dwfl_segment_report_module.c
@@ -257,18 +257,11 @@ dwfl_segment_report_module (Dwfl *dwfl, int ndx, const 
char *name,
 
   GElf_Addr start = dwfl->lookup_addr[segment];
 
-  inline bool segment_read (int segndx,
-   void **buffer, size_t *buffer_available,
-   GElf_Addr addr, size_t minread)
-  {
-return ! (*memory_callback) (dwfl, segndx, buffer, buffer_available,
-addr, minread, memory_callback_arg);
-  }
-
   inline void release_buffer (void **buffer, size_t *buffer_available)
   {
 if (*buffer != NULL)
-  (void) segment_read (-1, buffer, buffer_available, 0, 0);
+  (*memory_callback) (dwfl, -1, buffer, buffer_available, 0, 0,
+  memory_callback_arg);
   }
 
   /* First read in the file header and check its sanity.  */
@@ -282,8 +275,8 @@ dwfl_segment_report_module (Dwfl *dwfl, int ndx, const char 
*name,
  here so we can always safely free it.  */
   void *phdrsp = NULL;
 
-  if (segment_read (ndx, &buffer, &buffer_available,
-   start, sizeof (Elf64_Ehdr))
+  if (! (*memory_callback) (dwfl, ndx, &buffer, &buffer_available,
+start, sizeof (Elf64_Ehdr), memory_callback_arg)
   || memcmp (buffer, ELFMAG, SELFMAG) != 0)
 goto out;
 
@@ -301,8 +294,8 @@ dwfl_segment_report_module (Dwfl *dwfl, int ndx, const char 
*name,
   {
*data = NULL;
*data_size = filesz;
-   return segment_read (addr_segndx (dwfl, segment, vaddr, false),
-data, data_size, vaddr, filesz);
+return ! (*memory_callback) (dwfl, addr_segndx (dwfl, segment, vaddr, 
false),
+ data, data_size, vaddr, filesz, 
memory_callback_arg);
   }
 
 /* We already have this whole note segment from our initial read.  */
@@ -908,8 +901,9 @@ dwfl_segment_report_module (Dwfl *dwfl, int ndx, const char 
*name,
   {
void *into = contents + offset;
size_t read_size = size;
-   (void) segment_read (addr_segndx (dwfl, segment, vaddr, false),
-&into, &read_size, vaddr, size);
+(*memory_callback) (dwfl, addr_segndx (dwfl, segment, vaddr, false),
+&into, &read_size, vaddr, size,
+memory_callback_arg);
   }
 
   if (contiguous < file_trimmed_end)
-- 
2.26.2



[PATCH 02/12] segment_report_module: Remove nested release_buffer() function

2020-11-23 Thread Timm Bäder via Elfutils-devel
From: Timm Bäder 

Signed-off-by: Timm Bäder 
---
 libdwfl/dwfl_segment_report_module.c | 16 ++--
 1 file changed, 6 insertions(+), 10 deletions(-)

diff --git a/libdwfl/dwfl_segment_report_module.c 
b/libdwfl/dwfl_segment_report_module.c
index c587efd7..848c3bec 100644
--- a/libdwfl/dwfl_segment_report_module.c
+++ b/libdwfl/dwfl_segment_report_module.c
@@ -257,13 +257,6 @@ dwfl_segment_report_module (Dwfl *dwfl, int ndx, const 
char *name,
 
   GElf_Addr start = dwfl->lookup_addr[segment];
 
-  inline void release_buffer (void **buffer, size_t *buffer_available)
-  {
-if (*buffer != NULL)
-  (*memory_callback) (dwfl, -1, buffer, buffer_available, 0, 0,
-  memory_callback_arg);
-  }
-
   /* First read in the file header and check its sanity.  */
 
   void *buffer = NULL;
@@ -306,8 +299,8 @@ dwfl_segment_report_module (Dwfl *dwfl, int ndx, const char 
*name,
 
   inline void finish_portion (void **data, size_t *data_size)
   {
-if (*data_size != 0)
-  release_buffer (data, data_size);
+if (*data_size != 0 && *data != NULL)
+  (*memory_callback) (dwfl, -1, data, data_size, 0, 0, 
memory_callback_arg);
   }
 
   /* Extract the information we need from the file header.  */
@@ -960,7 +953,10 @@ dwfl_segment_report_module (Dwfl *dwfl, int ndx, const 
char *name,
 
 out:
   free (phdrsp);
-  release_buffer (&buffer, &buffer_available);
+  if (buffer != NULL)
+(*memory_callback) (dwfl, -1, &buffer, &buffer_available, 0, 0,
+memory_callback_arg);
+
   if (elf != NULL)
 elf_end (elf);
   if (fd != -1)
-- 
2.26.2



[PATCH 05/12] segment_report_module: Use a struct for build id information

2020-11-23 Thread Timm Bäder via Elfutils-devel
From: Timm Bäder 

Keep the three build id fields in a struct. This will be an important
clean up later.

Signed-off-by: Timm Bäder 
---
 libdwfl/dwfl_segment_report_module.c | 54 
 1 file changed, 31 insertions(+), 23 deletions(-)

diff --git a/libdwfl/dwfl_segment_report_module.c 
b/libdwfl/dwfl_segment_report_module.c
index 01adfe9e..6abbf992 100644
--- a/libdwfl/dwfl_segment_report_module.c
+++ b/libdwfl/dwfl_segment_report_module.c
@@ -54,6 +54,13 @@
 # define MY_ELFDATAELFDATA2MSB
 #endif
 
+struct elf_build_id
+{
+  void *memory;
+  size_t len;
+  GElf_Addr vaddr;
+};
+
 
 /* Return user segment index closest to ADDR but not above it.
If NEXT, return the closest to ADDR but not below it.  */
@@ -206,16 +213,16 @@ handle_file_note (GElf_Addr module_start, GElf_Addr 
module_end,
 
 static bool
 invalid_elf (Elf *elf, bool disk_file_has_build_id,
-const void *build_id, size_t build_id_len)
+ struct elf_build_id *build_id)
 {
-  if (! disk_file_has_build_id && build_id_len > 0)
+  if (! disk_file_has_build_id && build_id->len > 0)
 {
   /* Module found in segments with build-id is more reliable
 than a module found via DT_DEBUG on disk without any
 build-id.   */
   return true;
 }
-  if (disk_file_has_build_id && build_id_len > 0)
+  if (disk_file_has_build_id && build_id->len > 0)
 {
   const void *elf_build_id;
   ssize_t elf_build_id_len;
@@ -224,8 +231,8 @@ invalid_elf (Elf *elf, bool disk_file_has_build_id,
   elf_build_id_len = INTUSE(dwelf_elf_gnu_build_id) (elf, &elf_build_id);
   if (elf_build_id_len > 0)
{
- if (build_id_len != (size_t) elf_build_id_len
- || memcmp (build_id, elf_build_id, build_id_len) != 0)
+ if (build_id->len != (size_t) elf_build_id_len
+ || memcmp (build_id->memory, elf_build_id, build_id->len) != 0)
return true;
}
 }
@@ -442,16 +449,17 @@ dwfl_segment_report_module (Dwfl *dwfl, int ndx, const 
char *name,
   GElf_Xword dyn_filesz = 0;
 
   /* Collect the build ID bits here.  */
-  void *build_id = NULL;
-  size_t build_id_len = 0;
-  GElf_Addr build_id_vaddr = 0;
+  struct elf_build_id build_id;
+  build_id.memory = NULL;
+  build_id.len = 0;
+  build_id.vaddr =0;
 
   /* Consider a PT_NOTE we've found in the image.  */
   inline void consider_notes (GElf_Addr vaddr, GElf_Xword filesz,
  GElf_Xword align)
   {
 /* If we have already seen a build ID, we don't care any more.  */
-if (build_id != NULL || filesz == 0)
+if (build_id.memory != NULL || filesz == 0)
   return;
 
 void *data;
@@ -510,11 +518,11 @@ dwfl_segment_report_module (Dwfl *dwfl, int ndx, const 
char *name,
&& nh->n_namesz == sizeof "GNU"
&& !memcmp (note_name, "GNU", sizeof "GNU"))
  {
-   build_id_vaddr = note_desc - (const void *) notes + vaddr;
-   build_id_len = nh->n_descsz;
-   build_id = malloc (nh->n_descsz);
-   if (likely (build_id != NULL))
- memcpy (build_id, note_desc, build_id_len);
+build_id.vaddr = note_desc - (const void *) notes + vaddr;
+build_id.len = nh->n_descsz;
+build_id.memory = malloc (build_id.len);
+if (likely (build_id.memory != NULL))
+  memcpy (build_id.memory, note_desc, build_id.len);
break;
  }
 
@@ -629,7 +637,7 @@ dwfl_segment_report_module (Dwfl *dwfl, int ndx, const char 
*name,
  header we read at START was not produced by these program headers.  */
   if (unlikely (!found_bias))
 {
-  free (build_id);
+  free (build_id.memory);
   goto out;
 }
 
@@ -684,7 +692,7 @@ dwfl_segment_report_module (Dwfl *dwfl, int ndx, const char 
*name,
  {
if (module->elf != NULL
&& invalid_elf (module->elf, module->disk_file_has_build_id,
-   build_id, build_id_len))
+&build_id))
  {
elf_end (module->elf);
close (module->fd);
@@ -700,7 +708,7 @@ dwfl_segment_report_module (Dwfl *dwfl, int ndx, const char 
*name,
  }
   if (skip_this_module)
{
- free (build_id);
+ free (build_id.memory);
  goto out;
}
 }
@@ -719,7 +727,7 @@ dwfl_segment_report_module (Dwfl *dwfl, int ndx, const char 
*name,
  Dwfl_Error error = __libdw_open_file (&fd, &elf, true, false);
  if (error == DWFL_E_NOERROR)
invalid = invalid_elf (elf, true /* disk_file_has_build_id */,
-  build_id, build_id_len);
+   &build_id);
}
   if (invalid)
{
@@ -867,11 +875,11 @@ dwfl_segment_report_module (Dwfl *dwfl, int ndx, const 
char *name,
   if (mod != NULL && (execlike || ehdr.e32.e_type == ET_EXEC))
 mod->is_execut

[PATCH 06/12] segment_report_module: Pull consider_notes() into file scope

2020-11-23 Thread Timm Bäder via Elfutils-devel
From: Timm Bäder 

Signed-off-by: Timm Bäder 
---
 libdwfl/dwfl_segment_report_module.c | 187 +++
 1 file changed, 101 insertions(+), 86 deletions(-)

diff --git a/libdwfl/dwfl_segment_report_module.c 
b/libdwfl/dwfl_segment_report_module.c
index 6abbf992..6c6f9f37 100644
--- a/libdwfl/dwfl_segment_report_module.c
+++ b/libdwfl/dwfl_segment_report_module.c
@@ -282,6 +282,99 @@ read_portion (Dwfl *dwfl,
   return false;
 }
 
+
+/* Consider a PT_NOTE we've found in the image.  */
+static inline void
+consider_notes (Dwfl *dwfl,
+Dwfl_Memory_Callback *memory_callback, void 
*memory_callback_arg,
+GElf_Addr vaddr, GElf_Xword filesz,
+GElf_Xword align,
+void *buffer, size_t buffer_available,
+GElf_Addr start,
+size_t segment,
+unsigned char ei_data,
+struct elf_build_id *build_id,
+Elf_Data *xlatefrom,
+Elf_Data *xlateto,
+unsigned int xencoding)
+{
+  if (build_id->memory != NULL || filesz == 0)
+return;
+
+  void *data;
+  size_t data_size;
+  if (read_portion (dwfl, memory_callback, memory_callback_arg,
+&data, &data_size, vaddr, filesz,
+buffer, buffer_available, start, segment))
+return;
+
+  /* data_size will be zero if we got everything from the initial
+ buffer, otherwise it will be the size of the new buffer that
+ could be read.  */
+  if (data_size != 0)
+filesz = data_size;
+
+  assert (sizeof (Elf32_Nhdr) == sizeof (Elf64_Nhdr));
+
+  void *notes;
+  if (ei_data == MY_ELFDATA)
+notes = data;
+  else
+{
+  notes = malloc (filesz);
+  if (unlikely (notes == NULL))
+return;
+  xlatefrom->d_type = xlateto->d_type = (align == 8
+ ? ELF_T_NHDR8 : ELF_T_NHDR);
+  xlatefrom->d_buf = (void *) data;
+  xlatefrom->d_size = filesz;
+  xlateto->d_buf = notes;
+  xlateto->d_size = filesz;
+  if (elf32_xlatetom (xlateto, xlatefrom, xencoding) == NULL)
+goto done;
+}
+
+  const GElf_Nhdr *nh = notes;
+  size_t len = 0;
+  while (filesz > len + sizeof (*nh))
+{
+  const void *note_name;
+  const void *note_desc;
+
+  len += sizeof (*nh);
+  note_name = notes + len;
+
+  len += nh->n_namesz;
+  len = align == 8 ? NOTE_ALIGN8 (len) : NOTE_ALIGN4 (len);
+  note_desc = notes + len;
+
+  if (unlikely (filesz < len + nh->n_descsz))
+break;
+
+  if (nh->n_type == NT_GNU_BUILD_ID
+  && nh->n_descsz > 0
+  && nh->n_namesz == sizeof "GNU"
+  && !memcmp (note_name, "GNU", sizeof "GNU"))
+{
+  build_id->vaddr = note_desc - (const void *) notes + vaddr;
+  build_id->len = nh->n_descsz;
+  build_id->memory = malloc (build_id->len);
+  if (likely (build_id->memory != NULL))
+memcpy (build_id->memory, note_desc, build_id->len);
+  break;
+}
+
+  len += nh->n_descsz;
+  len = align == 8 ? NOTE_ALIGN8 (len) : NOTE_ALIGN4 (len);
+  nh = (void *) notes + len;
+}
+
+done:
+  if (notes != data)
+free (notes);
+  finish_portion (dwfl, memory_callback, memory_callback_arg, &data, 
&data_size);
+}
+
 int
 dwfl_segment_report_module (Dwfl *dwfl, int ndx, const char *name,
Dwfl_Memory_Callback *memory_callback,
@@ -454,89 +547,6 @@ dwfl_segment_report_module (Dwfl *dwfl, int ndx, const 
char *name,
   build_id.len = 0;
   build_id.vaddr =0;
 
-  /* Consider a PT_NOTE we've found in the image.  */
-  inline void consider_notes (GElf_Addr vaddr, GElf_Xword filesz,
- GElf_Xword align)
-  {
-/* If we have already seen a build ID, we don't care any more.  */
-if (build_id.memory != NULL || filesz == 0)
-  return;
-
-void *data;
-size_t data_size;
-if (read_portion (dwfl, memory_callback, memory_callback_arg,
-  &data, &data_size, vaddr, filesz,
-  buffer, buffer_available, start, segment))
-  return;
-
-/* data_size will be zero if we got everything from the initial
-   buffer, otherwise it will be the size of the new buffer that
-   could be read.  */
-if (data_size != 0)
-  filesz = data_size;
-
-assert (sizeof (Elf32_Nhdr) == sizeof (Elf64_Nhdr));
-
-void *notes;
-if (ei_data == MY_ELFDATA)
-  notes = data;
-else
-  {
-   notes = malloc (filesz);
-   if (unlikely (notes == NULL))
- return;
-   xlatefrom.d_type = xlateto.d_type = (align == 8
-? ELF_T_NHDR8 : ELF_T_NHDR);
-   xlatefrom.d_buf = (void *) data;
-   xlatefrom.d_size = filesz;
-   xlateto.d_buf = notes;
-   xlateto.d_size = filesz;
-   if (elf32_xlatetom (&xlateto, &xlatefrom,
-   ehdr.e32.e_ident[E

[PATCH 07/12] segment_report_module: Get rid of nested final_read() function

2020-11-23 Thread Timm Bäder via Elfutils-devel
From: Timm Bäder 

Signed-off-by: Timm Bäder 
---
 libdwfl/dwfl_segment_report_module.c | 25 ++---
 1 file changed, 14 insertions(+), 11 deletions(-)

diff --git a/libdwfl/dwfl_segment_report_module.c 
b/libdwfl/dwfl_segment_report_module.c
index 6c6f9f37..a69ead8f 100644
--- a/libdwfl/dwfl_segment_report_module.c
+++ b/libdwfl/dwfl_segment_report_module.c
@@ -934,15 +934,6 @@ dwfl_segment_report_module (Dwfl *dwfl, int ndx, const 
char *name,
   if (unlikely (contents == NULL))
goto out;
 
-  inline void final_read (size_t offset, GElf_Addr vaddr, size_t size)
-  {
-   void *into = contents + offset;
-   size_t read_size = size;
-(*memory_callback) (dwfl, addr_segndx (dwfl, segment, vaddr, false),
-&into, &read_size, vaddr, size,
-memory_callback_arg);
-  }
-
   if (contiguous < file_trimmed_end)
{
  /* We can't use the memory image verbatim as the file image.
@@ -952,7 +943,13 @@ dwfl_segment_report_module (Dwfl *dwfl, int ndx, const 
char *name,
 GElf_Off offset, GElf_Xword filesz)
  {
if (type == PT_LOAD)
- final_read (offset, vaddr + bias, filesz);
+  {
+void *into = contents + offset;
+size_t read_size = filesz;
+(*memory_callback) (dwfl, addr_segndx (dwfl, segment, vaddr + 
bias, false),
+&into, &read_size, vaddr + bias, read_size,
+memory_callback_arg);
+  }
  }
 
  if (ei_class == ELFCLASS32)
@@ -973,7 +970,13 @@ dwfl_segment_report_module (Dwfl *dwfl, int ndx, const 
char *name,
  memcpy (contents, buffer, have);
 
  if (have < file_trimmed_end)
-   final_read (have, start + have, file_trimmed_end - have);
+{
+  void *into = contents + have;
+  size_t read_size = file_trimmed_end - have;
+(*memory_callback) (dwfl, addr_segndx (dwfl, segment, start + 
have, false),
+&into, &read_size, start + have, read_size,
+memory_callback_arg);
+}
}
 
   elf = elf_memory (contents, file_trimmed_end);
-- 
2.26.2



[PATCH 09/12] segment_report_module: Inline read_phdr() into only caller

2020-11-23 Thread Timm Bäder via Elfutils-devel
From: Timm Bäder 

There is now only one caller for this nested function, so get rid of it
by just inlining it there.

Signed-off-by: Timm Bäder 
---
 libdwfl/dwfl_segment_report_module.c | 24 +---
 1 file changed, 9 insertions(+), 15 deletions(-)

diff --git a/libdwfl/dwfl_segment_report_module.c 
b/libdwfl/dwfl_segment_report_module.c
index 276e9117..10212964 100644
--- a/libdwfl/dwfl_segment_report_module.c
+++ b/libdwfl/dwfl_segment_report_module.c
@@ -938,29 +938,23 @@ dwfl_segment_report_module (Dwfl *dwfl, int ndx, const 
char *name,
{
  /* We can't use the memory image verbatim as the file image.
 So we'll be reading into a local image of the virtual file.  */
-
- inline void read_phdr (GElf_Word type, GElf_Addr vaddr,
-GElf_Off offset, GElf_Xword filesz)
- {
-   if (type == PT_LOAD)
-  {
-void *into = contents + offset;
-size_t read_size = filesz;
-(*memory_callback) (dwfl, addr_segndx (dwfl, segment, vaddr + 
bias, false),
-&into, &read_size, vaddr + bias, read_size,
-memory_callback_arg);
-  }
- }
-
   for (uint_fast16_t i = 0; i < phnum; ++i)
 {
   bool is32 = (ei_class == ELFCLASS32);
   GElf_Word type = is32 ? p32[i].p_type : p64[i].p_type;
+
+  if (type != PT_LOAD)
+continue;
+
   GElf_Addr vaddr = is32 ? p32[i].p_vaddr : p64[i].p_vaddr;
   GElf_Off offset = is32 ? p32[i].p_offset : p64[i].p_offset;
   GElf_Xword filesz = is32 ? p32[i].p_filesz : p64[i].p_filesz;
 
-  read_phdr (type, vaddr, offset, filesz);
+  void *into = contents + offset;
+  size_t read_size = filesz;
+  (*memory_callback) (dwfl, addr_segndx (dwfl, segment, vaddr + 
bias, false),
+  &into, &read_size, vaddr + bias, read_size,
+  memory_callback_arg);
 }
}
   else
-- 
2.26.2



[PATCH 12/12] segment_report_module: Inline consider_phdr() into only caller

2020-11-23 Thread Timm Bäder via Elfutils-devel
From: Timm Bäder 

Get rid of the nested function this way

Signed-off-by: Timm Bäder 
---
 libdwfl/dwfl_segment_report_module.c | 142 +--
 1 file changed, 66 insertions(+), 76 deletions(-)

diff --git a/libdwfl/dwfl_segment_report_module.c 
b/libdwfl/dwfl_segment_report_module.c
index 046d5381..26d4e80a 100644
--- a/libdwfl/dwfl_segment_report_module.c
+++ b/libdwfl/dwfl_segment_report_module.c
@@ -461,7 +461,7 @@ dwfl_segment_report_module (Dwfl *dwfl, int ndx, const char 
*name,
   /* NOTE if the number of sections is > 0xff00 then e_shnum
 is zero and the actual number would come from the section
 zero sh_size field. We ignore this here because getting shdrs
-is just a nice bonus (see below in consider_phdr PT_LOAD
+is just a nice bonus (see below in the type == PT_LOAD case
 where we trim the last segment).  */
   shdrs_end = ehdr.e32.e_shoff + ehdr.e32.e_shnum * ehdr.e32.e_shentsize;
   break;
@@ -547,80 +547,6 @@ dwfl_segment_report_module (Dwfl *dwfl, int ndx, const 
char *name,
   build_id.len = 0;
   build_id.vaddr =0;
 
-  /* Consider each of the program headers we've read from the image.  */
-  inline void consider_phdr (GElf_Word type,
-GElf_Addr vaddr, GElf_Xword memsz,
-GElf_Off offset, GElf_Xword filesz,
-GElf_Xword align)
-  {
-switch (type)
-  {
-  case PT_DYNAMIC:
-   dyn_vaddr = vaddr;
-   dyn_filesz = filesz;
-   break;
-
-  case PT_NOTE:
-  /* We calculate from the p_offset of the note segment,
-   because we don't yet know the bias for its p_vaddr.  */
-  consider_notes (dwfl, memory_callback, memory_callback_arg,
-  start + offset, filesz, align,
-  buffer, buffer_available, start, segment,
-  ei_data, &build_id,
-  &xlatefrom, &xlateto,
-  ehdr.e32.e_ident[EI_DATA]);
-   break;
-
-  case PT_LOAD:
-   align = dwfl->segment_align > 1 ? dwfl->segment_align : align ?: 1;
-
-   GElf_Addr vaddr_end = (vaddr + memsz + align - 1) & -align;
-   GElf_Addr filesz_vaddr = filesz < memsz ? vaddr + filesz : vaddr_end;
-   GElf_Off filesz_offset = filesz_vaddr - vaddr + offset;
-
-   if (file_trimmed_end < offset + filesz)
- {
-   file_trimmed_end = offset + filesz;
-
-   /* Trim the last segment so we don't bother with zeros
-  in the last page that are off the end of the file.
-  However, if the extra bit in that page includes the
-  section headers, keep them.  */
-   if (shdrs_end <= filesz_offset && shdrs_end > file_trimmed_end)
- {
-   filesz += shdrs_end - file_trimmed_end;
-   file_trimmed_end = shdrs_end;
- }
- }
-
-   total_filesz += filesz;
-
-   if (file_end < filesz_offset)
- {
-   file_end = filesz_offset;
-   if (filesz_vaddr - start == filesz_offset)
- contiguous = file_end;
- }
-
-   if (!found_bias && (offset & -align) == 0
-   && likely (filesz_offset >= phoff + phnum * phentsize))
- {
-   bias = start - vaddr;
-   found_bias = true;
- }
-
-   if ((vaddr & -align) < module_start)
- {
-   module_start = vaddr & -align;
-   module_address_sync = vaddr + memsz;
- }
-
-   if (module_end < vaddr_end)
- module_end = vaddr_end;
-   break;
-  }
-  }
-
   Elf32_Phdr *p32 = phdrsp;
   Elf64_Phdr *p64 = phdrsp;
   if ((ei_class == ELFCLASS32
@@ -632,6 +558,7 @@ dwfl_segment_report_module (Dwfl *dwfl, int ndx, const char 
*name,
 }
   else
 {
+  /* Consider each of the program headers we've read from the image.  */
   for (uint_fast16_t i = 0; i < phnum; ++i)
 {
   bool is32 = (ei_class == ELFCLASS32);
@@ -642,7 +569,70 @@ dwfl_segment_report_module (Dwfl *dwfl, int ndx, const 
char *name,
   GElf_Xword filesz = is32 ? p32[i].p_filesz : p64[i].p_filesz;
   GElf_Xword align = is32 ? p32[i].p_align : p64[i].p_align;
 
-  consider_phdr (type, vaddr, memsz, offset, filesz, align);
+  if (type == PT_DYNAMIC)
+{
+  dyn_vaddr = vaddr;
+  dyn_filesz = filesz;
+}
+  else if (type == PT_NOTE)
+{
+  /* We calculate from the p_offset of the note segment,
+   because we don't yet know the bias for its p_vaddr.  */
+  consider_notes (dwfl, memory_callback, memory_callback_arg,
+  start + offset, filesz, align,
+  buffer, buffer_available, start, segment,
+  ei_data, &build_id,
+  &xlatefrom, &xlateto,
+  

[PATCH 11/12] segment_report_module: Inline consider_dyn() into only caller

2020-11-23 Thread Timm Bäder via Elfutils-devel
From: Timm Bäder 

Signed-off-by: Timm Bäder 
---
 libdwfl/dwfl_segment_report_module.c | 40 +---
 1 file changed, 12 insertions(+), 28 deletions(-)

diff --git a/libdwfl/dwfl_segment_report_module.c 
b/libdwfl/dwfl_segment_report_module.c
index 8613ce21..046d5381 100644
--- a/libdwfl/dwfl_segment_report_module.c
+++ b/libdwfl/dwfl_segment_report_module.c
@@ -770,33 +770,6 @@ dwfl_segment_report_module (Dwfl *dwfl, int ndx, const 
char *name,
   GElf_Addr dynstr_vaddr = 0;
   GElf_Xword dynstrsz = 0;
   bool execlike = false;
-  inline bool consider_dyn (GElf_Sxword tag, GElf_Xword val)
-  {
-switch (tag)
-  {
-  default:
-   return false;
-
-  case DT_DEBUG:
-   execlike = true;
-   break;
-
-  case DT_SONAME:
-   soname_stroff = val;
-   break;
-
-  case DT_STRTAB:
-   dynstr_vaddr = val;
-   break;
-
-  case DT_STRSZ:
-   dynstrsz = val;
-   break;
-  }
-
-return soname_stroff != 0 && dynstr_vaddr != 0 && dynstrsz != 0;
-  }
-
   const size_t dyn_entsize = (ei_class == ELFCLASS32
  ? sizeof (Elf32_Dyn) : sizeof (Elf64_Dyn));
   void *dyn_data = NULL;
@@ -834,7 +807,18 @@ dwfl_segment_report_module (Dwfl *dwfl, int ndx, const 
char *name,
   GElf_Sxword tag = is32 ? d32[i].d_tag : d64[i].d_tag;
   GElf_Xword val = is32 ? d32[i].d_un.d_val : d64[i].d_un.d_val;
 
-  if (consider_dyn (tag, val))
+  if (tag == DT_DEBUG)
+execlike = true;
+  else if (tag == DT_SONAME)
+soname_stroff = val;
+  else if (tag == DT_STRTAB)
+dynstr_vaddr = val;
+  else if (tag == DT_STRSZ)
+dynstrsz = val;
+  else
+continue;
+
+  if (soname_stroff != 0 && dynstr_vaddr != 0 && dynstrsz != 0)
 break;
 }
 }
-- 
2.26.2



[PATCH 03/12] segment_report_module: Pull finish_portion() info file scope

2020-11-23 Thread Timm Bäder via Elfutils-devel
From: Timm Bäder 

Signed-off-by: Timm Bäder 
---
 libdwfl/dwfl_segment_report_module.c | 24 ++--
 1 file changed, 14 insertions(+), 10 deletions(-)

diff --git a/libdwfl/dwfl_segment_report_module.c 
b/libdwfl/dwfl_segment_report_module.c
index 848c3bec..c55168ed 100644
--- a/libdwfl/dwfl_segment_report_module.c
+++ b/libdwfl/dwfl_segment_report_module.c
@@ -232,6 +232,16 @@ invalid_elf (Elf *elf, bool disk_file_has_build_id,
   return false;
 }
 
+static inline void
+finish_portion (Dwfl *dwfl,
+Dwfl_Memory_Callback *memory_callback,
+void *memory_callback_arg,
+void **data, size_t *data_size)
+{
+  if (*data_size != 0 && *data != NULL)
+(*memory_callback) (dwfl, -1, data, data_size, 0, 0, memory_callback_arg);
+}
+
 int
 dwfl_segment_report_module (Dwfl *dwfl, int ndx, const char *name,
Dwfl_Memory_Callback *memory_callback,
@@ -297,12 +307,6 @@ dwfl_segment_report_module (Dwfl *dwfl, int ndx, const 
char *name,
 return false;
   }
 
-  inline void finish_portion (void **data, size_t *data_size)
-  {
-if (*data_size != 0 && *data != NULL)
-  (*memory_callback) (dwfl, -1, data, data_size, 0, 0, 
memory_callback_arg);
-  }
-
   /* Extract the information we need from the file header.  */
   const unsigned char *e_ident;
   unsigned char ei_class;
@@ -509,7 +513,7 @@ dwfl_segment_report_module (Dwfl *dwfl, int ndx, const char 
*name,
   done:
 if (notes != data)
   free (notes);
-finish_portion (&data, &data_size);
+finish_portion (dwfl, memory_callback, memory_callback_arg, &data, 
&data_size);
   }
 
   /* Consider each of the program headers we've read from the image.  */
@@ -606,7 +610,7 @@ dwfl_segment_report_module (Dwfl *dwfl, int ndx, const char 
*name,
 p64[i].p_align);
 }
 
-  finish_portion (&ph_buffer, &ph_buffer_size);
+  finish_portion (dwfl, memory_callback, memory_callback_arg, &ph_buffer, 
&ph_buffer_size);
 
   /* We must have seen the segment covering offset 0, or else the ELF
  header we read at START was not produced by these program headers.  */
@@ -798,7 +802,7 @@ dwfl_segment_report_module (Dwfl *dwfl, int ndx, const char 
*name,
}
   free (dyns);
 }
-  finish_portion (&dyn_data, &dyn_data_size);
+  finish_portion (dwfl, memory_callback, memory_callback_arg, &dyn_data, 
&dyn_data_size);
 
   /* We'll use the name passed in or a stupid default if not DT_SONAME.  */
   if (name == NULL)
@@ -859,7 +863,7 @@ dwfl_segment_report_module (Dwfl *dwfl, int ndx, const char 
*name,
   /* At this point we do not need BUILD_ID or NAME any more.
  They have been copied.  */
   free (build_id);
-  finish_portion (&soname, &soname_size);
+  finish_portion (dwfl, memory_callback, memory_callback_arg, &soname, 
&soname_size);
 
   if (unlikely (mod == NULL))
 {
-- 
2.26.2



[PATCH 04/12] segment_report_module: Pull read_portion() into file scope

2020-11-23 Thread Timm Bäder via Elfutils-devel
From: Timm Bäder 

Signed-off-by: Timm Bäder 
---
 libdwfl/dwfl_segment_report_module.c | 77 +---
 1 file changed, 47 insertions(+), 30 deletions(-)

diff --git a/libdwfl/dwfl_segment_report_module.c 
b/libdwfl/dwfl_segment_report_module.c
index c55168ed..01adfe9e 100644
--- a/libdwfl/dwfl_segment_report_module.c
+++ b/libdwfl/dwfl_segment_report_module.c
@@ -242,6 +242,39 @@ finish_portion (Dwfl *dwfl,
 (*memory_callback) (dwfl, -1, data, data_size, 0, 0, memory_callback_arg);
 }
 
+
+static inline bool
+read_portion (Dwfl *dwfl,
+  Dwfl_Memory_Callback *memory_callback,
+  void *memory_callback_arg,
+  void **data, size_t *data_size,
+  GElf_Addr vaddr, size_t filesz,
+  void *buffer,
+  size_t buffer_available,
+  GElf_Addr start,
+  size_t segment)
+{
+  /* Check whether we will have to read the segment data, or if it
+ can be returned from the existing buffer.  */
+  if (filesz > buffer_available
+  || vaddr - start > buffer_available - filesz
+  /* If we're in string mode, then don't consider the buffer we have
+ sufficient unless it contains the terminator of the string.  */
+  || (filesz == 0 && memchr (vaddr - start + buffer, '\0',
+ buffer_available - (vaddr - start)) == NULL))
+{
+  *data = NULL;
+  *data_size = filesz;
+  return ! (*memory_callback) (dwfl, addr_segndx (dwfl, segment, vaddr, 
false),
+   data, data_size, vaddr, filesz, 
memory_callback_arg);
+}
+
+  /* We already have this whole note segment from our initial read.  */
+  *data = vaddr - start + buffer;
+  *data_size = 0;
+  return false;
+}
+
 int
 dwfl_segment_report_module (Dwfl *dwfl, int ndx, const char *name,
Dwfl_Memory_Callback *memory_callback,
@@ -283,30 +316,6 @@ dwfl_segment_report_module (Dwfl *dwfl, int ndx, const 
char *name,
   || memcmp (buffer, ELFMAG, SELFMAG) != 0)
 goto out;
 
-  inline bool read_portion (void **data, size_t *data_size,
-   GElf_Addr vaddr, size_t filesz)
-  {
-/* Check whether we will have to read the segment data, or if it
-   can be returned from the existing buffer.  */
-if (filesz > buffer_available
-   || vaddr - start > buffer_available - filesz
-   /* If we're in string mode, then don't consider the buffer we have
-  sufficient unless it contains the terminator of the string.  */
-   || (filesz == 0 && memchr (vaddr - start + buffer, '\0',
-  buffer_available - (vaddr - start)) == NULL))
-  {
-   *data = NULL;
-   *data_size = filesz;
-return ! (*memory_callback) (dwfl, addr_segndx (dwfl, segment, vaddr, 
false),
- data, data_size, vaddr, filesz, 
memory_callback_arg);
-  }
-
-/* We already have this whole note segment from our initial read.  */
-*data = vaddr - start + buffer;
-*data_size = 0;
-return false;
-  }
-
   /* Extract the information we need from the file header.  */
   const unsigned char *e_ident;
   unsigned char ei_class;
@@ -387,8 +396,10 @@ dwfl_segment_report_module (Dwfl *dwfl, int ndx, const 
char *name,
 
   void *ph_buffer = NULL;
   size_t ph_buffer_size = 0;
-  if (read_portion (&ph_buffer, &ph_buffer_size,
-   start + phoff, xlatefrom.d_size))
+  if (read_portion (dwfl, memory_callback, memory_callback_arg,
+&ph_buffer, &ph_buffer_size,
+start + phoff, xlatefrom.d_size,
+buffer, buffer_available, start, segment))
 goto out;
 
   /* ph_buffer_size will be zero if we got everything from the initial
@@ -445,7 +456,9 @@ dwfl_segment_report_module (Dwfl *dwfl, int ndx, const char 
*name,
 
 void *data;
 size_t data_size;
-if (read_portion (&data, &data_size, vaddr, filesz))
+if (read_portion (dwfl, memory_callback, memory_callback_arg,
+  &data, &data_size, vaddr, filesz,
+  buffer, buffer_available, start, segment))
   return;
 
 /* data_size will be zero if we got everything from the initial
@@ -766,7 +779,9 @@ dwfl_segment_report_module (Dwfl *dwfl, int ndx, const char 
*name,
   void *dyn_data = NULL;
   size_t dyn_data_size = 0;
   if (dyn_filesz != 0 && dyn_filesz % dyn_entsize == 0
-  && ! read_portion (&dyn_data, &dyn_data_size, dyn_vaddr, dyn_filesz))
+  && ! read_portion (dwfl, memory_callback, memory_callback_arg,
+ &dyn_data, &dyn_data_size, dyn_vaddr, dyn_filesz,
+ buffer, buffer_available, start, segment))
 {
   /* dyn_data_size will be zero if we got everything from the initial
  buffer, otherwise it will be the size of the new buffer that
@@ -834,8 +849,10 @@ dwfl_segment_report_module (Dwfl *dwfl, int ndx, const

[PATCH 08/12] segment_report_module: Use one loop for p32/p64 arrays

2020-11-23 Thread Timm Bäder via Elfutils-devel
From: Timm Bäder 

Do one loop check for 32/64 bit inside the loop, instead of outside.
This way we have only one call site for the function called in the loop
body.

Signed-off-by: Timm Bäder 
---
 libdwfl/dwfl_segment_report_module.c | 52 +++-
 1 file changed, 27 insertions(+), 25 deletions(-)

diff --git a/libdwfl/dwfl_segment_report_module.c 
b/libdwfl/dwfl_segment_report_module.c
index a69ead8f..276e9117 100644
--- a/libdwfl/dwfl_segment_report_module.c
+++ b/libdwfl/dwfl_segment_report_module.c
@@ -623,27 +623,27 @@ dwfl_segment_report_module (Dwfl *dwfl, int ndx, const 
char *name,
 
   Elf32_Phdr *p32 = phdrsp;
   Elf64_Phdr *p64 = phdrsp;
-  if (ei_class == ELFCLASS32)
+  if ((ei_class == ELFCLASS32
+   && elf32_xlatetom (&xlateto, &xlatefrom, ei_data) == NULL)
+  || (ei_class == ELFCLASS64
+  && elf64_xlatetom (&xlateto, &xlatefrom, ei_data) == NULL))
 {
-  if (elf32_xlatetom (&xlateto, &xlatefrom, ei_data) == NULL)
-   found_bias = false; /* Trigger error check.  */
-  else
-   for (uint_fast16_t i = 0; i < phnum; ++i)
- consider_phdr (p32[i].p_type,
-p32[i].p_vaddr, p32[i].p_memsz,
-p32[i].p_offset, p32[i].p_filesz,
-p32[i].p_align);
+  found_bias = false; /* Trigger error check */
 }
   else
 {
-  if (elf64_xlatetom (&xlateto, &xlatefrom, ei_data) == NULL)
-   found_bias = false; /* Trigger error check.  */
-  else
-   for (uint_fast16_t i = 0; i < phnum; ++i)
- consider_phdr (p64[i].p_type,
-p64[i].p_vaddr, p64[i].p_memsz,
-p64[i].p_offset, p64[i].p_filesz,
-p64[i].p_align);
+  for (uint_fast16_t i = 0; i < phnum; ++i)
+{
+  bool is32 = (ei_class == ELFCLASS32);
+  GElf_Word type = is32 ? p32[i].p_type : p64[i].p_type;
+  GElf_Addr vaddr = is32 ? p32[i].p_vaddr : p64[i].p_vaddr;
+  GElf_Xword memsz = is32 ? p32[i].p_memsz : p64[i].p_memsz;
+  GElf_Off offset = is32 ? p32[i].p_offset : p64[i].p_offset;
+  GElf_Xword filesz = is32 ? p32[i].p_filesz : p64[i].p_filesz;
+  GElf_Xword align = is32 ? p32[i].p_align : p64[i].p_align;
+
+  consider_phdr (type, vaddr, memsz, offset, filesz, align);
+}
 }
 
   finish_portion (dwfl, memory_callback, memory_callback_arg, &ph_buffer, 
&ph_buffer_size);
@@ -952,14 +952,16 @@ dwfl_segment_report_module (Dwfl *dwfl, int ndx, const 
char *name,
   }
  }
 
- if (ei_class == ELFCLASS32)
-   for (uint_fast16_t i = 0; i < phnum; ++i)
- read_phdr (p32[i].p_type, p32[i].p_vaddr,
-p32[i].p_offset, p32[i].p_filesz);
- else
-   for (uint_fast16_t i = 0; i < phnum; ++i)
- read_phdr (p64[i].p_type, p64[i].p_vaddr,
-p64[i].p_offset, p64[i].p_filesz);
+  for (uint_fast16_t i = 0; i < phnum; ++i)
+{
+  bool is32 = (ei_class == ELFCLASS32);
+  GElf_Word type = is32 ? p32[i].p_type : p64[i].p_type;
+  GElf_Addr vaddr = is32 ? p32[i].p_vaddr : p64[i].p_vaddr;
+  GElf_Off offset = is32 ? p32[i].p_offset : p64[i].p_offset;
+  GElf_Xword filesz = is32 ? p32[i].p_filesz : p64[i].p_filesz;
+
+  read_phdr (type, vaddr, offset, filesz);
+}
}
   else
{
-- 
2.26.2



[PATCH 10/12] segment_report_module: Unify d32/d64 loops

2020-11-23 Thread Timm Bäder via Elfutils-devel
From: Timm Bäder 

Just like we did before, use only one loop here and check for 32/64 bit
in the loop body. This way we only have one call site for consider_dyn

Signed-off-by: Timm Bäder 
---
 libdwfl/dwfl_segment_report_module.c | 28 ++--
 1 file changed, 14 insertions(+), 14 deletions(-)

diff --git a/libdwfl/dwfl_segment_report_module.c 
b/libdwfl/dwfl_segment_report_module.c
index 10212964..8613ce21 100644
--- a/libdwfl/dwfl_segment_report_module.c
+++ b/libdwfl/dwfl_segment_report_module.c
@@ -824,20 +824,20 @@ dwfl_segment_report_module (Dwfl *dwfl, int ndx, const 
char *name,
   xlateto.d_buf = dyns;
   xlateto.d_size = dyn_filesz;
 
-  if (ei_class == ELFCLASS32)
-   {
- if (elf32_xlatetom (&xlateto, &xlatefrom, ei_data) != NULL)
-   for (size_t i = 0; i < dyn_filesz / sizeof (Elf32_Dyn); ++i)
- if (consider_dyn (d32[i].d_tag, d32[i].d_un.d_val))
-   break;
-   }
-  else
-   {
- if (elf64_xlatetom (&xlateto, &xlatefrom, ei_data) != NULL)
-   for (size_t i = 0; i < dyn_filesz / sizeof (Elf64_Dyn); ++i)
- if (consider_dyn (d64[i].d_tag, d64[i].d_un.d_val))
-   break;
-   }
+  bool is32 = (ei_class == ELFCLASS32);
+  if ((is32 && elf32_xlatetom (&xlateto, &xlatefrom, ei_data) != NULL)
+  || (!is32 && elf64_xlatetom (&xlateto, &xlatefrom, ei_data) != NULL))
+{
+  size_t n = is32 ? (dyn_filesz / sizeof (Elf32_Dyn)) : (dyn_filesz / 
sizeof (Elf64_Dyn));
+  for (size_t i = 0; i < n; ++i)
+{
+  GElf_Sxword tag = is32 ? d32[i].d_tag : d64[i].d_tag;
+  GElf_Xword val = is32 ? d32[i].d_un.d_val : d64[i].d_un.d_val;
+
+  if (consider_dyn (tag, val))
+break;
+}
+}
   free (dyns);
 }
   finish_portion (dwfl, memory_callback, memory_callback_arg, &dyn_data, 
&dyn_data_size);
-- 
2.26.2



Re: [PATCH] debuginfod-client: Add debuginfod_set_verbose_fd and DEBUGINFOD_VERBOSE

2020-11-23 Thread Mark Wielaard
Hi,

On Wed, 2020-11-11 at 22:18 +0100, Mark Wielaard wrote:
> Introduce a new function debuginfod_set_verbose_fd which will produce
> verbose output on a given file descriptor (STDERR_FILENO if the
> environment variable DEBUGINFOD_VERBOSE is set) showing how the search
> for a particular client query is going.

I didn't get more comments on list, but there was some positive
feedback on irc. So I have pushed it now. But please let me know if
there are any concerns with this new interface.

Cheers,

Mark


Buildbot failure in Wildebeest Builder on whole buildset

2020-11-23 Thread buildbot
The Buildbot has detected a failed build on builder whole buildset while 
building elfutils.
Full details are available at:
https://builder.wildebeest.org/buildbot/#builders/1/builds/630

Buildbot URL: https://builder.wildebeest.org/buildbot/

Worker for this Build: centos-x86_64

Build Reason: 
Blamelist: Mark Wielaard 

BUILD FAILED: failed test (failure)

Sincerely,
 -The Buildbot



Re: Buildbot failure in Wildebeest Builder on whole buildset

2020-11-23 Thread Mark Wielaard
On Mon, 2020-11-23 at 17:50 +, build...@builder.wildebeest.org
wrote:
> The Buildbot has detected a failed build on builder whole buildset
> while building elfutils.
> Full details are available at:
> https://builder.wildebeest.org/buildbot/#builders/1/builds/630
> 
> Buildbot URL: https://builder.wildebeest.org/buildbot/
> 
> Worker for this Build: centos-x86_64
> 
> Build Reason: 
> Blamelist: Mark Wielaard 
> 
> BUILD FAILED: failed test (failure)

Interestingly only one buildbot worker found this issue, but I could
replicate it under valgrind locally. And indeed, valgrind is right. We
forgot to initialize the new handler_data errbuf field.

Fixed as attached.

Cheers,

Mark
From 6b82ac67d8a71c14f64fe4932ca7fe822ff75231 Mon Sep 17 00:00:00 2001
From: Mark Wielaard 
Date: Mon, 23 Nov 2020 17:52:02 +0100
Subject: [PATCH] debuginfod-client: Initialize struct handle_data errbuf to
 the empty string.

Signed-off-by: Mark Wielaard 
---
 debuginfod/ChangeLog   | 5 +
 debuginfod/debuginfod-client.c | 1 +
 2 files changed, 6 insertions(+)

diff --git a/debuginfod/ChangeLog b/debuginfod/ChangeLog
index bd4dbf40..cd009fd2 100644
--- a/debuginfod/ChangeLog
+++ b/debuginfod/ChangeLog
@@ -1,3 +1,8 @@
+2020-11-23  Mark Wielaard  
+
+	* debuginfod-client.c (debuginfod_query_server): Initialize
+	struct handle_data errbuf to the empty string.
+
 2020-11-11  Mark Wielaard  
 
 	* debuginfod-client.c (debuginfod_set_verbose_fd): New function.
diff --git a/debuginfod/debuginfod-client.c b/debuginfod/debuginfod-client.c
index 2bf1543a..a99f3c14 100644
--- a/debuginfod/debuginfod-client.c
+++ b/debuginfod/debuginfod-client.c
@@ -756,6 +756,7 @@ debuginfod_query_server (debuginfod_client *c,
 {
   data[i].handle = NULL;
   data[i].fd = -1;
+  data[i].errbuf[0] = '\0';
 }
 
   char *strtok_saveptr;
-- 
2.18.4



patch: debuginfod sqlite3 metrics

2020-11-23 Thread Frank Ch. Eigler via Elfutils-devel
Hi -


>From 0b61f4c758a507fcc2243357363e44140bd13d82 Mon Sep 17 00:00:00 2001 ?!??!!
From: "Frank Ch. Eigler" 
Date: Mon, 23 Nov 2020 19:58:10 -0500
Subject: [PATCH] debuginfod: sqlite3 metrics

Add metrics for tracking sqlite3 error counts and query performance.

The former looks like a new sibling of the "error_count" family, and
is tested by dd-corrupting a live database file then triggering some
debuginfod activity.

error_count{sqlite3="file is not a database"} 1

The latter looks like _count/_sum pairs for each type of sqlite
prepared-statement used in the code, and is grep smoke-tested.  They
should assist a sysadmin in tuning db storage.  This example shows a
6.4 ms/operation cost:

sqlite3_milliseconds_count{step-done="rpm-file-intern"} 318
sqlite3_milliseconds_sum{reset="rpm-file-intern"} 2033

Signed-off-by: Frank Ch. Eigler 
---
 debuginfod/ChangeLog |  6 ++
 debuginfod/debuginfod.cxx| 31 ---
 tests/ChangeLog  |  4 
 tests/run-debuginfod-find.sh | 16 
 4 files changed, 54 insertions(+), 3 deletions(-)

diff --git a/debuginfod/ChangeLog b/debuginfod/ChangeLog
index bd4dbf403f30..a81e3781986d 100644
--- a/debuginfod/ChangeLog
+++ b/debuginfod/ChangeLog
@@ -1,3 +1,9 @@
+2020-11-23  Frank Ch. Eigler  
+
+   * debuginfod.cxx (tmp_ms_metric): New class for RAII timing metrics.
+   (sqlite_ps::reset, step*): Call it to track sqlite3 performance.
+   (sqlite_exception ctor): Increment sqlite3 error_count.
+
 2020-11-11  Mark Wielaard  
 
* debuginfod-client.c (debuginfod_set_verbose_fd): New function.
diff --git a/debuginfod/debuginfod.cxx b/debuginfod/debuginfod.cxx
index 61c778b10893..618620b4c478 100644
--- a/debuginfod/debuginfod.cxx
+++ b/debuginfod/debuginfod.cxx
@@ -435,6 +435,27 @@ class tmp_inc_metric { // a RAII style wrapper for 
exception-safe scoped increme
   }
 };
 
+class tmp_ms_metric { // a RAII style wrapper for exception-safe scoped timing
+  string m, n, v;
+  struct timeval tv_start;
+public:
+  tmp_ms_metric(const string& mname, const string& lname, const string& 
lvalue):
+m(mname), n(lname), v(lvalue)
+  {
+gettimeofday (& tv_start, NULL);
+  }
+  ~tmp_ms_metric()
+  {
+struct timeval tv_end;
+gettimeofday (& tv_end, NULL);
+double deltas = (tv_end.tv_sec - tv_start.tv_sec)
+  + (tv_end.tv_usec - tv_start.tv_usec)*0.01;
+
+add_metric (m + "_milliseconds_sum", n, v, (deltas*1000));
+inc_metric (m + "_milliseconds_count", n, v);
+  }
+};
+
 
 /* Handle program arguments.  */
 static error_t
@@ -559,7 +580,9 @@ struct reportable_exception
 struct sqlite_exception: public reportable_exception
 {
   sqlite_exception(int rc, const string& msg):
-reportable_exception(string("sqlite3 error: ") + msg + ": " + 
string(sqlite3_errstr(rc) ?: "?")) {}
+reportable_exception(string("sqlite3 error: ") + msg + ": " + 
string(sqlite3_errstr(rc) ?: "?")) {
+inc_metric("error_count","sqlite3",sqlite3_errstr(rc));
+  }
 };
 
 struct libc_exception: public reportable_exception
@@ -755,6 +778,7 @@ struct sqlite_ps
 
 public:
   sqlite_ps (sqlite3* d, const string& n, const string& s): db(d), 
nickname(n), sql(s) {
+// tmp_ms_metric tick("sqlite3","prep",nickname);
 if (verbose > 4)
   obatched(clog) << nickname << " prep " << sql << endl;
 int rc = sqlite3_prepare_v2 (db, sql.c_str(), -1 /* to \0 */, & this->pp, 
NULL);
@@ -764,6 +788,7 @@ struct sqlite_ps
 
   sqlite_ps& reset()
   {
+tmp_ms_metric tick("sqlite3","reset",nickname);
 sqlite3_reset(this->pp);
 return *this;
   }
@@ -800,6 +825,7 @@ struct sqlite_ps
 
 
   void step_ok_done() {
+tmp_ms_metric tick("sqlite3","step-done",nickname);
 int rc = sqlite3_step (this->pp);
 if (verbose > 4)
   obatched(clog) << nickname << " step-ok-done(" << sqlite3_errstr(rc) << 
") " << sql << endl;
@@ -810,14 +836,13 @@ struct sqlite_ps
 
 
   int step() {
+tmp_ms_metric tick("sqlite3","step",nickname);
 int rc = sqlite3_step (this->pp);
 if (verbose > 4)
   obatched(clog) << nickname << " step(" << sqlite3_errstr(rc) << ") " << 
sql << endl;
 return rc;
   }
 
-
-
   ~sqlite_ps () { sqlite3_finalize (this->pp); }
   operator sqlite3_stmt* () { return this->pp; }
 };
diff --git a/tests/ChangeLog b/tests/ChangeLog
index c130ce049caa..f5adc315ae38 100644
--- a/tests/ChangeLog
+++ b/tests/ChangeLog
@@ -1,3 +1,7 @@
+2020-11-23  Frank Ch. Eigler  
+
+   * run-debuginfod-find.sh: Add sqlite error injection & stats.
+
 2020-11-02  Mark Wielaard  
 
* run-debuginfod-find.sh: Create bogus R/nothing.rpm with cyclic
diff --git a/tests/run-debuginfod-find.sh b/tests/run-debuginfod-find.sh
index 28aa4263c500..7fd3420ab20a 100755
--- a/tests/run-debuginfod-find.sh
+++ b/tests/run-debuginfod-find.sh
@@ -490,6 +490,22 @@ curl -s http://127.0.0.1:$PORT2/badapi > /dev/null || true
 curl -s http://127.0.0.1:$PORT2/buildid/deadbeef/debuginfo > 

[announce] Void Linux debuginfod server

2020-11-23 Thread Érico Nogueira via Elfutils-devel
Hi!

I am happy to announce that Void Linux is now hosting a debuginfod
server in https://debuginfod.s.voidlinux.org ! It includes all packages
for which we have debug packages (some packages, like chromium, have
debug package generation disabled), for all of official architectures
and libcs which had build-id enabled in their toolchains (!), which are:

- x86_64 glibc and musl
- i686 glibc
- aarch64 glibc
- armv7l glibc
- armv6l glibc

During this effort, I found out the targets below didn't have build-id's
enabled, and will be getting that fixed as soon as possible. Due to
musl's time64 transition, we will likely do a full repository rebuild at
least for armv6l and armv7l, which will help in getting debug files for
those two up to par with the others:

- aarch64 musl
- armv7l musl
- armv6l musl

At the moment, our gdb package is built with libdebuginfod support, and
we are looking into adding it to binutils and systemtap as well. I was
very interested when I first read about the project, and am very happy I
could drive its adoption in Void.

Thank you all for having the idea, for the development work, and for
helping me with the compatibility patches as well!

Érico Nogueira