Re: [PATCH v2 4/4] Don't use dlopen() for libebl modules

2019-08-29 Thread Mark Wielaard
Hi Omar,

On Mon, 2019-08-26 at 10:51 -0700, Omar Sandoval wrote:
> Currently, architecture-specific code for libebl exists in separate
> libebl_$ARCH.so libraries which libebl loads with dlopen() at runtime.
> This makes it impossible to have standalone, statically-linked binaries
> which use libdwfl if they depend on any architecture-specific
> functionality. Additionally, when these libraries cannot be found, the
> failure modes are non-obvious. So, let's get rid of libebl_$arch.so and
> move it all into libdw.so/libdw.a, which simplifies things considerably.

Very nice. I had to tweak the top-level SUBDIRS a bit to apply after
addition of the new doc subdir. But it applied as is otherwise. Tested
in various configurations and it seems to do what is expected. And it
also gets us code coverage for the backends. Which is helpful to see
what can possibly be eliminated (both the backends and libebl have some
functions with zero coverage).

Thanks,

Mark


[PATCH 1/3] Add some supporting framework for C11-style atomics.

2019-08-29 Thread Mark Wielaard
From: Jonathon Anderson 

Uses the stdatomic.h provided by FreeBSD when GCC doesn't (ie. GCC < 4.9)

Signed-off-by: Jonathon Anderson 
Signed-off-by: Srđan Milaković 
---
 configure.ac |  12 ++
 lib/ChangeLog|   6 +
 lib/Makefile.am  |   3 +-
 lib/atomics.h|  37 
 lib/stdatomic-fbsd.h | 442 +++
 5 files changed, 499 insertions(+), 1 deletion(-)
 create mode 100644 lib/atomics.h
 create mode 100644 lib/stdatomic-fbsd.h

diff --git a/configure.ac b/configure.ac
index c443fa3b..b8aba460 100644
--- a/configure.ac
+++ b/configure.ac
@@ -226,6 +226,18 @@ LDFLAGS="$save_LDFLAGS"])
 AS_IF([test "x$ac_cv_tls" != xyes],
   AC_MSG_ERROR([__thread support required]))
 
+dnl Before 4.9 gcc doesn't ship stdatomic.h, but the nessesary atomics are
+dnl available by (at least) 4.7. So if the system doesn't have a stdatomic.h we
+dnl fall back on one copied from FreeBSD that handles the difference.
+AC_CACHE_CHECK([whether gcc provides stdatomic.h], ac_cv_has_stdatomic,
+  [AC_COMPILE_IFELSE([AC_LANG_SOURCE([[#include ]])],
+ac_cv_has_stdatomic=yes, ac_cv_has_stdatomic=no)])
+AM_CONDITIONAL(HAVE_STDATOMIC_H, test "x$ac_cv_has_stdatomic" = xyes)
+AS_IF([test "x$ac_cv_has_stdatomic" = xyes], [AC_DEFINE(HAVE_STDATOMIC_H)])
+
+AH_TEMPLATE([HAVE_STDATOMIC_H], [Define to 1 if `stdatomic.h` is provided by 
the
+ system, 0 otherwise.])
+
 dnl This test must come as early as possible after the compiler configuration
 dnl tests, because the choice of the file model can (in principle) affect
 dnl whether functions and headers are available, whether they work, etc.
diff --git a/lib/ChangeLog b/lib/ChangeLog
index 7381860c..3799c3aa 100644
--- a/lib/ChangeLog
+++ b/lib/ChangeLog
@@ -1,3 +1,9 @@
+2019-08-25  Jonathon Anderson  
+
+   * stdatomic-fbsd.h: New file, taken from FreeBSD.
+   * atomics.h: New file.
+   * Makefile.am (noinst_HEADERS): Added *.h above.
+
 2019-05-03  Rosen Penev  
 
* color.c (parse_opt): Cast program_invocation_short_name to char *.
diff --git a/lib/Makefile.am b/lib/Makefile.am
index 36d21a07..3086cf06 100644
--- a/lib/Makefile.am
+++ b/lib/Makefile.am
@@ -38,7 +38,8 @@ libeu_a_SOURCES = xstrdup.c xstrndup.c xmalloc.c next_prime.c 
\
  color.c printversion.c
 
 noinst_HEADERS = fixedsizehash.h libeu.h system.h dynamicsizehash.h list.h \
-eu-config.h color.h printversion.h bpf.h
+eu-config.h color.h printversion.h bpf.h \
+atomics.h stdatomic-fbsd.h
 EXTRA_DIST = dynamicsizehash.c
 
 if !GPROF
diff --git a/lib/atomics.h b/lib/atomics.h
new file mode 100644
index ..ffd12f87
--- /dev/null
+++ b/lib/atomics.h
@@ -0,0 +1,37 @@
+/* Conditional wrapper header for C11-style atomics.
+   Copyright (C) 2019-2019 Red Hat, Inc.
+   This file is part of elfutils.
+
+   This file is free software; you can redistribute it and/or modify
+   it under the terms of either
+
+ * the GNU Lesser General Public License as published by the Free
+   Software Foundation; either version 3 of the License, or (at
+   your option) any later version
+
+   or
+
+ * the GNU General Public License as published by the Free
+   Software Foundation; either version 2 of the License, or (at
+   your option) any later version
+
+   or both in parallel, as here.
+
+   elfutils is distributed in the hope that it will be useful, but
+   WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   General Public License for more details.
+
+   You should have received copies of the GNU General Public License and
+   the GNU Lesser General Public License along with this program.  If
+   not, see .  */
+
+#include "config.h"
+
+#if HAVE_STDATOMIC_H
+/* If possible, use the compiler's preferred atomics.  */
+# include 
+#else
+/* Otherwise, try to use the builtins provided by this compiler.  */
+# include "stdatomic-fbsd.h"
+#endif /* HAVE_STDATOMIC_H */
diff --git a/lib/stdatomic-fbsd.h b/lib/stdatomic-fbsd.h
new file mode 100644
index ..49626662
--- /dev/null
+++ b/lib/stdatomic-fbsd.h
@@ -0,0 +1,442 @@
+/*-
+ * Copyright (c) 2011 Ed Schouten 
+ *David Chisnall 
+ * All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ * 1. Redistributions of source code must retain the above copyright
+ *notice, this list of conditions and the following disclaimer.
+ * 2. Redistributions in binary form must reproduce the above copyright
+ *notice, this list of conditions and the following disclaimer in the
+ *documentation and/or other materials provided with the distribution.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE AUTHOR AND CONTRIBUTORS ``AS IS'' AND
+ * ANY EXP

Re: [PATCH] libdw: add thread-safety to dwarf_getabbrev()

2019-08-29 Thread Mark Wielaard
Hi Jonathan,

On Mon, 2019-08-26 at 22:52 -0500, Jonathon Anderson wrote:
> Just finished some modifications to the patch series, git request-
> pull output below. This rebases onto the latest master and does a
> little diff cleaning, the major change is that I swapped out the
> memory management to use the pthread_key_* alternative mentioned
> before. I did some performance testing and found it to be notably
> faster in a high-stress microbenchmark (and no measurable difference
> in an application), and it makes the result easier to read overall.
> I've also removed the Valgrind configuration options patch since it
> doesn't add anything at the moment, if it becomes useful I'll submit
> it separately.

Thanks for the pull request. Since I find it nice to also see all
patches on the mailinglist, I am reposting them for easier review.
As replies to this message.

> The following changes since commit 
> 4bcc641d362de4236ae8f0f5bc933c6d84b6f105:
> 
>   libdw: fix latent bug in dwarf_getcfi.c not setting 
> default_same_value. (2019-08-26 15:15:34 +0200)
> 
> are available in the Git repository at:
> 
>   https://github.com/blue42u/elfutils.git parallel-pr-v2
> 
> for you to fetch changes up to
> 1191d9ed292b508d732973a318a01051053e0f61:
> 
>   lib + libdw: Add and use a concurrent version of the dynamic-size 
> hash table. (2019-08-26 22:29:45 -0500)
> 
> 
> Jonathon Anderson (2):
>   Add some supporting framework for C11-style atomics.
>   libdw: Rewrite the memory handler to be thread-safe.
> 
> Srđan Milaković (1):
>   lib + libdw: Add and use a concurrent version of the dynamic-
> size hash table.

[PATCH 1/3] Add some supporting framework for C11-style atomics.
[PATCH 2/3] libdw: Rewrite the memory handler to be thread-safe.
[PATCH 3/3] lib + libdw: Add and use a concurrent version of the

Cheers,

Mark



[PATCH 2/3] libdw: Rewrite the memory handler to be thread-safe.

2019-08-29 Thread Mark Wielaard
From: Jonathon Anderson 

Signed-off-by: Jonathon Anderson 
---
 libdw/ChangeLog |  8 ++
 libdw/Makefile.am   |  4 +--
 libdw/dwarf_begin_elf.c | 12 -
 libdw/dwarf_end.c   |  7 ++---
 libdw/libdwP.h  | 59 ++---
 libdw/libdw_alloc.c |  5 ++--
 6 files changed, 59 insertions(+), 36 deletions(-)

diff --git a/libdw/ChangeLog b/libdw/ChangeLog
index 1d3586f0..ec809070 100644
--- a/libdw/ChangeLog
+++ b/libdw/ChangeLog
@@ -1,3 +1,11 @@
+2019-08-26  Jonathon Anderson  
+
+   * libdw_alloc.c (__libdw_allocate): Added thread-safe stack allocator.
+   * libdwP.h (Dwarf): Likewise.
+   * dwarf_begin_elf.c (dwarf_begin_elf): Support for above.
+   * dwarf_end.c (dwarf_end): Likewise.
+   * Makefile.am: Use -pthread to provide rwlocks.
+
 2019-08-25  Jonathon Anderson  
 
* dwarf_getcfi.c (dwarf_getcfi): Set default_same_value to false.
diff --git a/libdw/Makefile.am b/libdw/Makefile.am
index 7a3d5322..ba5745f3 100644
--- a/libdw/Makefile.am
+++ b/libdw/Makefile.am
@@ -31,7 +31,7 @@ include $(top_srcdir)/config/eu.am
 if BUILD_STATIC
 AM_CFLAGS += $(fpic_CFLAGS)
 endif
-AM_CPPFLAGS += -I$(srcdir)/../libelf -I$(srcdir)/../libdwelf
+AM_CPPFLAGS += -I$(srcdir)/../libelf -I$(srcdir)/../libdwelf -pthread
 VERSION = 1
 
 lib_LIBRARIES = libdw.a
@@ -108,7 +108,7 @@ am_libdw_pic_a_OBJECTS = $(libdw_a_SOURCES:.c=.os)
 libdw_so_LIBS = libdw_pic.a ../libdwelf/libdwelf_pic.a \
  ../libdwfl/libdwfl_pic.a ../libebl/libebl.a
 libdw_so_DEPS = ../lib/libeu.a ../libelf/libelf.so
-libdw_so_LDLIBS = $(libdw_so_DEPS) -ldl -lz $(argp_LDADD) $(zip_LIBS)
+libdw_so_LDLIBS = $(libdw_so_DEPS) -ldl -lz $(argp_LDADD) $(zip_LIBS) -pthread
 libdw_so_SOURCES =
 libdw.so$(EXEEXT): $(srcdir)/libdw.map $(libdw_so_LIBS) $(libdw_so_DEPS)
 # The rpath is necessary for libebl because its $ORIGIN use will
diff --git a/libdw/dwarf_begin_elf.c b/libdw/dwarf_begin_elf.c
index 38c8f5c6..f865f69c 100644
--- a/libdw/dwarf_begin_elf.c
+++ b/libdw/dwarf_begin_elf.c
@@ -397,7 +397,7 @@ dwarf_begin_elf (Elf *elf, Dwarf_Cmd cmd, Elf_Scn *scngrp)
   assert (sizeof (struct Dwarf) < mem_default_size);
 
   /* Allocate the data structure.  */
-  Dwarf *result = (Dwarf *) calloc (1, sizeof (Dwarf) + mem_default_size);
+  Dwarf *result = (Dwarf *) calloc (1, sizeof (Dwarf));
   if (unlikely (result == NULL)
   || unlikely (Dwarf_Sig8_Hash_init (&result->sig8_hash, 11) < 0))
 {
@@ -414,14 +414,12 @@ dwarf_begin_elf (Elf *elf, Dwarf_Cmd cmd, Elf_Scn *scngrp)
   result->elf = elf;
   result->alt_fd = -1;
 
-  /* Initialize the memory handling.  */
+  /* Initialize the memory handling.  Initial blocks are allocated on first
+ actual allocation.  */
   result->mem_default_size = mem_default_size;
   result->oom_handler = __libdw_oom;
-  result->mem_tail = (struct libdw_memblock *) (result + 1);
-  result->mem_tail->size = (result->mem_default_size
-   - offsetof (struct libdw_memblock, mem));
-  result->mem_tail->remaining = result->mem_tail->size;
-  result->mem_tail->prev = NULL;
+  pthread_key_create (&result->mem_key, NULL);
+  atomic_init (&result->mem_tail, (uintptr_t)NULL);
 
   if (cmd == DWARF_C_READ || cmd == DWARF_C_RDWR)
 {
diff --git a/libdw/dwarf_end.c b/libdw/dwarf_end.c
index 29795c10..fc573cb3 100644
--- a/libdw/dwarf_end.c
+++ b/libdw/dwarf_end.c
@@ -94,14 +94,15 @@ dwarf_end (Dwarf *dwarf)
   /* And the split Dwarf.  */
   tdestroy (dwarf->split_tree, noop_free);
 
-  struct libdw_memblock *memp = dwarf->mem_tail;
-  /* The first block is allocated together with the Dwarf object.  */
-  while (memp->prev != NULL)
+  /* Free the internally allocated memory.  */
+  struct libdw_memblock *memp = (struct libdw_memblock *)dwarf->mem_tail;
+  while (memp != NULL)
{
  struct libdw_memblock *prevp = memp->prev;
  free (memp);
  memp = prevp;
}
+  pthread_key_delete (dwarf->mem_key);
 
   /* Free the pubnames helper structure.  */
   free (dwarf->pubnames_sets);
diff --git a/libdw/libdwP.h b/libdw/libdwP.h
index eebb7d12..ad2599eb 100644
--- a/libdw/libdwP.h
+++ b/libdw/libdwP.h
@@ -31,9 +31,11 @@
 
 #include 
 #include 
+#include 
 
 #include 
 #include 
+#include "atomics.h"
 
 
 /* gettext helper macros.  */
@@ -147,6 +149,17 @@ enum
 
 #include "dwarf_sig8_hash.h"
 
+/* Structure for internal memory handling.  This is basically a simplified
+   reimplementation of obstacks.  Unfortunately the standard obstack
+   implementation is not usable in libraries.  */
+struct libdw_memblock
+{
+  size_t size;
+  size_t remaining;
+  struct libdw_memblock *prev;
+  char mem[0];
+};
+
 /* This is the structure representing the debugging state.  */
 struct Dwarf
 {
@@ -218,16 +231,11 @@ struct Dwarf
   /* Similar for addrx/constx, which will come from .debug_addr section.  */
   struct Dwarf_CU *fake_addr_cu;
 
-  /* Internal memory handling.  T

[PATCH 3/3] lib + libdw: Add and use a concurrent version of the dynamic-size hash table.

2019-08-29 Thread Mark Wielaard
From: Srđan Milaković 

Signed-off-by: Srđan Milaković 
---
 lib/ChangeLog|   5 +
 lib/Makefile.am  |   4 +-
 lib/dynamicsizehash_concurrent.c | 522 +++
 lib/dynamicsizehash_concurrent.h | 118 +++
 libdw/ChangeLog  |   4 +
 libdw/dwarf_abbrev_hash.c|   2 +-
 libdw/dwarf_abbrev_hash.h|   2 +-
 7 files changed, 653 insertions(+), 4 deletions(-)
 create mode 100644 lib/dynamicsizehash_concurrent.c
 create mode 100644 lib/dynamicsizehash_concurrent.h

diff --git a/lib/ChangeLog b/lib/ChangeLog
index 3799c3aa..a209729a 100644
--- a/lib/ChangeLog
+++ b/lib/ChangeLog
@@ -1,3 +1,8 @@
+2019-08-26  Srđan Milaković  
+
+   * dynamicsizehash_concurrent.{c,h}: New files.
+   * Makefile.am (noinst_HEADERS): Added dynamicsizehash_concurrent.h.
+
 2019-08-25  Jonathon Anderson  
 
* stdatomic-fbsd.h: New file, taken from FreeBSD.
diff --git a/lib/Makefile.am b/lib/Makefile.am
index 3086cf06..97bf7329 100644
--- a/lib/Makefile.am
+++ b/lib/Makefile.am
@@ -39,8 +39,8 @@ libeu_a_SOURCES = xstrdup.c xstrndup.c xmalloc.c next_prime.c 
\
 
 noinst_HEADERS = fixedsizehash.h libeu.h system.h dynamicsizehash.h list.h \
 eu-config.h color.h printversion.h bpf.h \
-atomics.h stdatomic-fbsd.h
-EXTRA_DIST = dynamicsizehash.c
+atomics.h stdatomic-fbsd.h dynamicsizehash_concurrent.h
+EXTRA_DIST = dynamicsizehash.c dynamicsizehash_concurrent.c
 
 if !GPROF
 xmalloc_CFLAGS = -ffunction-sections
diff --git a/lib/dynamicsizehash_concurrent.c b/lib/dynamicsizehash_concurrent.c
new file mode 100644
index ..d645b143
--- /dev/null
+++ b/lib/dynamicsizehash_concurrent.c
@@ -0,0 +1,522 @@
+/* Copyright (C) 2000-2019 Red Hat, Inc.
+   This file is part of elfutils.
+   Written by Srdan Milakovic , 2019.
+   Derived from Ulrich Drepper , 2000.
+
+   This file is free software; you can redistribute it and/or modify
+   it under the terms of either
+
+ * the GNU Lesser General Public License as published by the Free
+   Software Foundation; either version 3 of the License, or (at
+   your option) any later version
+
+   or
+
+ * the GNU General Public License as published by the Free
+   Software Foundation; either version 2 of the License, or (at
+   your option) any later version
+
+   or both in parallel, as here.
+
+   elfutils is distributed in the hope that it will be useful, but
+   WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   General Public License for more details.
+
+   You should have received copies of the GNU General Public License and
+   the GNU Lesser General Public License along with this program.  If
+   not, see .  */
+
+#include 
+#include 
+#include 
+#include 
+
+/* Before including this file the following macros must be defined:
+
+   NAME  name of the hash table structure.
+   TYPE  data type of the hash table entries
+   COMPARE   comparison function taking two pointers to TYPE objects
+
+   The following macros if present select features:
+
+   ITERATE   iterating over the table entries is possible
+   REVERSE   iterate in reverse order of insert
+ */
+
+
+static size_t
+lookup (NAME *htab, HASHTYPE hval, TYPE val __attribute__ ((unused)))
+{
+  /* First hash function: simply take the modul but prevent zero.  Small values
+  can skip the division, which helps performance when this is common.  */
+  size_t idx = 1 + (hval < htab->size ? hval : hval % htab->size);
+
+#if COMPARE != 0  /* A handful of tables don't actually compare the entries in
+the table, they instead rely on the hash.  In that case, we
+can skip parts that relate to the value. */
+  TYPE val_ptr;
+#endif
+  HASHTYPE hash;
+
+  hash = atomic_load_explicit(&htab->table[idx].hashval,
+  memory_order_acquire);
+  if (hash == hval)
+{
+#if COMPARE == 0
+  return idx;
+#else
+  val_ptr = (TYPE) atomic_load_explicit(&htab->table[idx].val_ptr,
+memory_order_acquire);
+  if (COMPARE(val_ptr, val) == 0)
+  return idx;
+#endif
+}
+  else if (hash == 0)
+{
+  return 0;
+}
+
+  /* Second hash function as suggested in [Knuth].  */
+  HASHTYPE second_hash = 1 + hval % (htab->size - 2);
+
+  for(;;)
+{
+  if (idx <= second_hash)
+  idx = htab->size + idx - second_hash;
+  else
+  idx -= second_hash;
+
+  hash = atomic_load_explicit(&htab->table[idx].hashval,
+  memory_order_acquire);
+  if (hash == hval)
+{
+#if COMPARE == 0
+  return idx;
+#else
+  val_ptr = (TYPE) atomic_load_explicit(&htab->table[idx].val_ptr,
+memory_order_acquire);
+  if (COMPARE(val_ptr,

Buildbot failure in Wildebeest Builder on whole buildset

2019-08-29 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/16/builds/182

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

Worker for this Build: centos-aarch64

Build Reason: 
Blamelist: Omar Sandoval 

BUILD FAILED: failed test (failure)

Sincerely,
 -The Buildbot



Re: Buildbot failure in Wildebeest Builder on whole buildset

2019-08-29 Thread Mark Wielaard
On Thu, 2019-08-29 at 13:59 +, 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/16/builds/182

Well that is unfortunate. The make check (under valgrind) just timed
out. That arm64 machine is really slow. And looking at some of the
other builders I see make check under valgrind (or make distcheck,
which enables the gcc undefined sanitizer and valgrind) became a lot
slower (non-valgrind builds/checks don't seem to be impacted).

It looks like the cause is our self checks. The majority of files we
use for that seem to be exactly those files that became a lot bigger.
Which means the tests use a lot more memory and increases the runtime
(which gets 10x longer under valgrind).

I am replacing some of the self test files with smaller
executables/libraries as attached. Hopefully that will reduce the make
check runtime under valgrind so builder don't time out.

Cheers,

Mark
From bb106065dc6962fabf1c163f18c932650dac6580 Mon Sep 17 00:00:00 2001
From: Mark Wielaard 
Date: Thu, 29 Aug 2019 16:21:35 +0200
Subject: [PATCH] tests: Use smaller self test files.

Don't use the largest executables/libraries to reduce the make check time.

Signed-off-by: Mark Wielaard 
---
 tests/ChangeLog| 6 ++
 tests/test-subr.sh | 8 
 2 files changed, 10 insertions(+), 4 deletions(-)

diff --git a/tests/ChangeLog b/tests/ChangeLog
index 657e73b2..69e43ca8 100644
--- a/tests/ChangeLog
+++ b/tests/ChangeLog
@@ -1,3 +1,9 @@
+2019-08-29  Mark Wielaard  
+
+	* test-subr.sh (self_test_files_exe): replace elfcmp, objdump and
+	readelf with elfclassify, stack and unstrip.
+	(self_test_files_lib): Replace libdw.so with libasm.so.
+
 2019-07-05  Omar Sandoval  
 
 	* Makefile.am: Remove -ldl.
diff --git a/tests/test-subr.sh b/tests/test-subr.sh
index e23a0176..e768c1e5 100644
--- a/tests/test-subr.sh
+++ b/tests/test-subr.sh
@@ -116,12 +116,12 @@ program_transform()
 }
 
 self_test_files_exe=`echo ${abs_top_builddir}/src/addr2line \
-${abs_top_builddir}/src/elfcmp \
-${abs_top_builddir}/src/objdump \
-${abs_top_builddir}/src/readelf`
+${abs_top_builddir}/src/elfclassify \
+${abs_top_builddir}/src/stack \
+${abs_top_builddir}/src/unstrip`
 
 self_test_files_lib=`echo ${abs_top_builddir}/libelf/libelf.so \
-${abs_top_builddir}/libdw/libdw.so`
+${abs_top_builddir}/libasm/libasm.so`
 
 self_test_files_obj=`echo ${abs_top_builddir}/src/size.o \
 ${abs_top_builddir}/src/strip.o`
-- 
2.18.1



Re: Buildbot failure in Wildebeest Builder on whole buildset

2019-08-29 Thread Mark Wielaard
On Thu, Aug 29, 2019 at 04:23:24PM +0200, Mark Wielaard wrote:
> I am replacing some of the self test files with smaller
> executables/libraries as attached. Hopefully that will reduce the make
> check runtime under valgrind so builder don't time out.

That was interesting. It did bring down the make check times under
valgrind. But it also showed a memory leak in eu-nm! Triggered by
chance because we use different self-check binaries now :)

Fix attached and pushed to master.

Cheers,

Mark>From df33285b60290fadefd140ee2fe616f750105d2f Mon Sep 17 00:00:00 2001
From: Mark Wielaard 
Date: Thu, 29 Aug 2019 17:46:52 +0200
Subject: [PATCH] nm: Fix latent memory leak in show_symbols.

If there are just a handful of symbols then memory for them is
allocated on the stack, otherwise the memory is malloced. So before
freeing the memory we need to check the number of entries to know if
the memory was heap allocated or not. But since not all entries might
be used we might have decreased the number of entries to the number
we will actually show. Remember the original symbol entries to not
have a memory leak.

Signed-off-by: Mark Wielaard 
---
 src/ChangeLog | 5 +
 src/nm.c  | 3 ++-
 2 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/src/ChangeLog b/src/ChangeLog
index aeb62328..cb64f7d9 100644
--- a/src/ChangeLog
+++ b/src/ChangeLog
@@ -1,3 +1,8 @@
+2019-08-26  Mark Wielaard  
+
+	* nm.c (show_symbols): Remember nentries_orig and check before
+	freeing sym_mem.
+
 2019-07-05  Omar Sandoval  
 
 	* Makefile.am: Remove -ldl.
diff --git a/src/nm.c b/src/nm.c
index da1350b4..7f6cf2a2 100644
--- a/src/nm.c
+++ b/src/nm.c
@@ -1438,6 +1438,7 @@ show_symbols (int fd, Ebl *ebl, GElf_Ehdr *ehdr,
   free (demangle_buffer);
 #endif
   /* Now we know the exact number.  */
+  size_t nentries_orig = nentries;
   nentries = nentries_used;
 
   /* Sort the entries according to the users wishes.  */
@@ -1472,7 +1473,7 @@ show_symbols (int fd, Ebl *ebl, GElf_Ehdr *ehdr,
 }
 
   /* Free all memory.  */
-  if (nentries * sizeof (sym_mem[0]) >= MAX_STACK_ALLOC)
+  if (nentries_orig * sizeof (sym_mem[0]) >= MAX_STACK_ALLOC)
 free (sym_mem);
 
   obstack_free (&whereob, NULL);
-- 
2.20.1



Re: [PATCH] readelf: Actually dump hex or strings when -p or -x get section number.

2019-08-29 Thread Mark Wielaard
On Tue, Aug 27, 2019 at 04:12:35PM +0200, Mark Wielaard wrote:
> The readelf code did parse section numbers, but then failed to actually
> dump the section found. Fixed by actually calling the dump function
> (either the hex or string variant). Add testcase for readelf -x num.

Pushed to master.


Re: [PATCH] config: Add manpages to spec file.

2019-08-29 Thread Mark Wielaard
On Wed, Aug 28, 2019 at 12:32:17PM +0200, Mark Wielaard wrote:
> On Wed, 2019-08-28 at 00:34 +0200, Mark Wielaard wrote:
> > Now that we have manpages lets also package them.
> > 
> > +2019-08-28  Mark Wielaard  
> > +
> > +   * elfutils.spec.in (%files): Add man1/eu-*.1*.
> > +   (%files libelf-devel): Add man3/elf_*.3*.
> 
> That clearly isn't enough. We should at least add the new license file
> and tags. Adding the license showed that they cannot have the same name
> (even if they are in different subdirs, the spec %license or %doc tag
> will put them in the same subdir). So I renamed it to COPYING-GFDL.
> 
> Actually trying to use the build file for creating an srpm and building
> it showed a couple of other small issues. That is what we get for not
> actually using and/or building it regularly. It would be good to sync
> the whole thing with one of the distro spec files. But for now this at
> least makes the thing consistent.
> [...]
> Subject: [PATCH] config: Fix spec file, add manpages and new GFDL license.
> 
> Now that we have manpages lets also package them. Rename COPYING to
> COPYING-GFDL to make it not clash with the top-level COPYING file.
> Also fix up the spec file so it can be used to create a srpm again.
> Add eu-stack to the file list.

Pushed to master. I have another update on top to deal with libebl
disappering.

Cheers,

Mark



[PATCH] libebl: Don't install libebl.a, libebl.h and remove backends from spec.

2019-08-29 Thread Mark Wielaard
All archive members from libebl.a are now in libdw.a. We don't generate
separate backend shared libraries anymore. So remove them from the
elfutils.spec file.

Signed-off-by: Mark Wielaard 
---
 config/ChangeLog|  7 +++
 config/elfutils.spec.in | 10 ++
 libebl/ChangeLog|  5 +
 libebl/Makefile.am  |  7 ++-
 4 files changed, 16 insertions(+), 13 deletions(-)

diff --git a/config/ChangeLog b/config/ChangeLog
index a7e98d77..b641d0d5 100644
--- a/config/ChangeLog
+++ b/config/ChangeLog
@@ -1,3 +1,10 @@
+2019-08-29  Mark Wielaard  
+
+   * elfutils.spec.in (%description devel): Remove libebl text.
+   (%install): Don't touch backend lib.*.so* files.
+   (%files): Remove backends dir and so files.
+   (%files devel): Remove libebl.h and libebl.a
+
 2019-08-28  Mark Wielaard  
 
* elfutils.spec.in (License): Add GFDL.
diff --git a/config/elfutils.spec.in b/config/elfutils.spec.in
index 513c4e79..6771d13b 100644
--- a/config/elfutils.spec.in
+++ b/config/elfutils.spec.in
@@ -51,9 +51,8 @@ Requires: elfutils-libelf-devel = %{version}-%{release}
 
 %description devel
 The elfutils-devel package contains the libraries to create
-applications for handling compiled objects.  libebl provides some
-higher-level ELF access functionality.  libdw provides access to
-the DWARF debugging information.  libasm provides a programmable
+applications for handling compiled objects.  libdw provides access
+to the DWARF debugging information.  libasm provides a programmable
 assembler interface.
 
 %package devel-static
@@ -131,7 +130,6 @@ mkdir -p ${RPM_BUILD_ROOT}%{_prefix}
 %makeinstall
 
 chmod +x ${RPM_BUILD_ROOT}%{_prefix}/%{_lib}/lib*.so*
-chmod +x ${RPM_BUILD_ROOT}%{_prefix}/%{_lib}/elfutils/lib*.so*
 
 # XXX Nuke unpackaged files
 ( cd ${RPM_BUILD_ROOT}
@@ -184,8 +182,6 @@ rm -rf ${RPM_BUILD_ROOT}
 %{_libdir}/libdw-%{version}.so
 %{_libdir}/libasm.so.*
 %{_libdir}/libdw.so.*
-%dir %{_libdir}/elfutils
-%{_libdir}/elfutils/lib*.so
 %{_mandir}/man1/eu-*.1*
 
 %files devel
@@ -195,12 +191,10 @@ rm -rf ${RPM_BUILD_ROOT}
 %{_includedir}/elfutils/elf-knowledge.h
 %{_includedir}/elfutils/known-dwarf.h
 #%{_includedir}/elfutils/libasm.h
-%{_includedir}/elfutils/libebl.h
 %{_includedir}/elfutils/libdw.h
 %{_includedir}/elfutils/libdwfl.h
 %{_includedir}/elfutils/libdwelf.h
 %{_includedir}/elfutils/version.h
-%{_libdir}/libebl.a
 #%{_libdir}/libasm.so
 %{_libdir}/libdw.so
 %{_libdir}/pkgconfig/libdw.pc
diff --git a/libebl/ChangeLog b/libebl/ChangeLog
index 6ba3a02b..4da7eeeb 100644
--- a/libebl/ChangeLog
+++ b/libebl/ChangeLog
@@ -1,3 +1,8 @@
+2019-08-29  Mark Wielaard  
+
+   * Makefile.am (noinst_LIBRARIES): Add libebl.a.
+   (noinst_HEADERS): Add libebl.h.
+
 2019-07-05  Omar Sandoval  
 
* Makefile.am: Make libebl.a non-PIC by default.
diff --git a/libebl/Makefile.am b/libebl/Makefile.am
index 8af84633..d0d475b8 100644
--- a/libebl/Makefile.am
+++ b/libebl/Makefile.am
@@ -34,10 +34,7 @@ endif
 AM_CPPFLAGS += -I$(srcdir)/../libelf -I$(srcdir)/../libdw -I$(srcdir)/../libasm
 VERSION = 1
 
-lib_LIBRARIES = libebl.a
-noinst_LIBRARIES = libebl_pic.a
-
-pkginclude_HEADERS = libebl.h
+noinst_LIBRARIES = libebl.a libebl_pic.a
 
 libebl_a_SOURCES = eblopenbackend.c eblclosebackend.c eblreloctypename.c \
   eblsegmenttypename.c eblsectiontypename.c \
@@ -62,6 +59,6 @@ libebl_a_SOURCES = eblopenbackend.c eblclosebackend.c 
eblreloctypename.c \
 libebl_pic_a_SOURCES =
 am_libebl_pic_a_OBJECTS = $(libebl_a_SOURCES:.c=.os)
 
-noinst_HEADERS = libeblP.h ebl-hooks.h
+noinst_HEADERS = libebl.h libeblP.h ebl-hooks.h
 
 MOSTLYCLEANFILES = $(am_libebl_pic_a_OBJECTS)
-- 
2.18.1