[PATCH 1/4] Consistently define _(Str) using dgettext ("elfutils", Str)

2020-12-16 Thread Dmitry V. Levin
Move the definition of _(Str) macro to lib/eu-config.h which already
provides a definition of N_(Str) macro.  Since lib/eu-config.h is
appended to config.h, it is included into every compilation unit
and therefore both macros are now universally available.

Remove all other definitions of N_(Str) and _(Str) macros from other files
to avoid conflicts and redundancies.

The next step is to replace all uses of gettext(Str) with _(Str).

Signed-off-by: Dmitry V. Levin 
---
 lib/ChangeLog   | 5 +
 lib/eu-config.h | 3 ++-
 lib/xmalloc.c   | 4 
 libasm/ChangeLog| 4 
 libasm/libasmP.h| 3 ---
 libdw/ChangeLog | 4 
 libdw/libdwP.h  | 4 
 libdwfl/ChangeLog   | 5 +
 libdwfl/argp-std.c  | 3 ---
 libdwfl/libdwflP.h  | 3 ---
 libebl/ChangeLog| 4 
 libebl/libeblP.h| 5 -
 libelf/ChangeLog| 4 
 libelf/libelfP.h| 3 ---
 src/ChangeLog   | 4 
 src/unstrip.c   | 4 
 tests/ChangeLog | 4 
 tests/dwflmodtest.c | 4 
 18 files changed, 36 insertions(+), 34 deletions(-)

diff --git a/lib/ChangeLog b/lib/ChangeLog
index 663a7aa5..48b496ce 100644
--- a/lib/ChangeLog
+++ b/lib/ChangeLog
@@ -1,3 +1,8 @@
+2020-12-16  Dmitry V. Levin  
+
+   * eu-config.h (_): New macro.
+   * xmalloc.c (_): Remove.
+
 2020-11-01  Érico N. Rolim  
 
* system.h (ACCESSPERMS): Define macro if it doesn't exist.
diff --git a/lib/eu-config.h b/lib/eu-config.h
index 84b22d7c..f0e3d07a 100644
--- a/lib/eu-config.h
+++ b/lib/eu-config.h
@@ -52,8 +52,9 @@
 # define rwlock_unlock(lock) ((void) (lock))
 #endif /* USE_LOCKS */
 
-/* gettext helper macro.  */
+/* gettext helper macros.  */
 #define N_(Str) Str
+#define _(Str) dgettext ("elfutils", Str)
 
 /* Compiler-specific definitions.  */
 #define strong_alias(name, aliasname) \
diff --git a/lib/xmalloc.c b/lib/xmalloc.c
index 0424afc8..7c094985 100644
--- a/lib/xmalloc.c
+++ b/lib/xmalloc.c
@@ -36,10 +36,6 @@
 #include 
 #include "system.h"
 
-#ifndef _
-# define _(str) gettext (str)
-#endif
-
 
 /* Allocate N bytes of memory dynamically, with error checking.  */
 void *
diff --git a/libasm/ChangeLog b/libasm/ChangeLog
index 78f1baa4..98ac3315 100644
--- a/libasm/ChangeLog
+++ b/libasm/ChangeLog
@@ -1,3 +1,7 @@
+2020-12-16  Dmitry V. Levin  
+
+   * libasmP.h (_): Remove.
+
 2020-12-12  Dmitry V. Levin  
 
* asm_begin.c (prepare_binary_output): Fix spelling typo in comment.
diff --git a/libasm/libasmP.h b/libasm/libasmP.h
index 53d8f3a0..8b72f32b 100644
--- a/libasm/libasmP.h
+++ b/libasm/libasmP.h
@@ -36,9 +36,6 @@
 
 #include "libdwelf.h"
 
-/* gettext helper macros.  */
-#define _(Str) dgettext ("elfutils", Str)
-
 
 /* Known error codes.  */
 enum
diff --git a/libdw/ChangeLog b/libdw/ChangeLog
index ab568f55..20fec602 100644
--- a/libdw/ChangeLog
+++ b/libdw/ChangeLog
@@ -1,3 +1,7 @@
+2020-12-16  Dmitry V. Levin  
+
+   * libdwP.h (_): Remove.
+
 2020-12-12  Dmitry V. Levin  
 
* dwarf.h: Fix spelling typo in comment.
diff --git a/libdw/libdwP.h b/libdw/libdwP.h
index c18eea43..7174ea93 100644
--- a/libdw/libdwP.h
+++ b/libdw/libdwP.h
@@ -38,10 +38,6 @@
 #include "atomics.h"
 
 
-/* gettext helper macros.  */
-#define _(Str) dgettext ("elfutils", Str)
-
-
 /* Known location expressions already decoded.  */
 struct loc_s
 {
diff --git a/libdwfl/ChangeLog b/libdwfl/ChangeLog
index fc64eafd..f9f6f01f 100644
--- a/libdwfl/ChangeLog
+++ b/libdwfl/ChangeLog
@@ -1,3 +1,8 @@
+2020-12-16  Dmitry V. Levin  
+
+   * argp-std.c (_): Remove.
+   * libdwflP.h (_): Likewise.
+
 2020-12-12  Dmitry V. Levin  
 
* libdwfl.h: Fix spelling typos in comments.
diff --git a/libdwfl/argp-std.c b/libdwfl/argp-std.c
index 2aa1b5e0..01ec18e2 100644
--- a/libdwfl/argp-std.c
+++ b/libdwfl/argp-std.c
@@ -38,9 +38,6 @@
 #include 
 #include 
 
-/* gettext helper macros.  */
-#define _(Str) dgettext ("elfutils", Str)
-
 
 #define OPT_DEBUGINFO  0x100
 #define OPT_COREFILE   0x101
diff --git a/libdwfl/libdwflP.h b/libdwfl/libdwflP.h
index 4c6fcb28..4344e356 100644
--- a/libdwfl/libdwflP.h
+++ b/libdwfl/libdwflP.h
@@ -47,9 +47,6 @@
 
 typedef struct Dwfl_Process Dwfl_Process;
 
-/* gettext helper macros.  */
-#define _(Str) dgettext ("elfutils", Str)
-
 #define DWFL_ERRORS  \
   DWFL_ERROR (NOERROR, N_("no error"))   \
   DWFL_ERROR (UNKNOWN_ERROR, N_("unknown error"))\
diff --git a/libebl/ChangeLog b/libebl/ChangeLog
index e0862ec3..33208f0d 100644
--- a/libebl/ChangeLog
+++ b/libebl/ChangeLog
@@ -1,3 +1,7 @@
+2020-12-16  Dmitry V. Levin  
+
+   * libeblP.h (_): Remove.
+
 2020-12-15  Dmitry V. Levin  
 
* eblbackendname.c (ebl_backend_name): Replace gettext(...) with _(...).
diff --git a/libebl/libeblP.h b/libebl/libeblP.h
index 599f6378..fa1c2c9f 100644
--- a/libebl/libeblP.h
+++ b/libebl/libeblP.h
@@ -86,11 +86,6 @@ struct ebl
 typedef Ebl *(*ebl_bhi

[PATCH 2/4] lib: consistently use _(Str) instead of gettext(Str)

2020-12-16 Thread Dmitry V. Levin
eu-config.h defines _(Str) to dgettext ("elfutils", Str) instead of
a simple gettext (Str) for a reason: the library might be indirectly
used by clients that called bindtextdomain with a domain different
from "elfutils".

The change was made automatically using the following command:
$ git grep -l '\
---
 lib/ChangeLog  | 5 +
 lib/color.c| 4 ++--
 lib/printversion.c | 2 +-
 lib/system.h   | 2 +-
 4 files changed, 9 insertions(+), 4 deletions(-)

diff --git a/lib/ChangeLog b/lib/ChangeLog
index 48b496ce..3b603bd0 100644
--- a/lib/ChangeLog
+++ b/lib/ChangeLog
@@ -1,5 +1,10 @@
 2020-12-16  Dmitry V. Levin  
 
+   * color.c (parse_opt): Replace gettext(...) and
+   dgettext("elfutils, ...) with _(...).
+   * printversion.c (print_version): Replace gettext(...) with _(...).
+   * system.h (sgettext): Likewise.
+
* eu-config.h (_): New macro.
* xmalloc.c (_): Remove.
 
diff --git a/lib/color.c b/lib/color.c
index 2cb41eba..454cb7ca 100644
--- a/lib/color.c
+++ b/lib/color.c
@@ -126,7 +126,7 @@ parse_opt (int key, char *arg,
  }
  if (i == nvalues)
{
- error (0, 0, dgettext ("elfutils", "\
+ error (0, 0, _("\
 %s: invalid argument '%s' for '--color'\n\
 valid arguments are:\n\
   - 'always', 'yes', 'force'\n\
@@ -191,7 +191,7 @@ valid arguments are:\n\
if (asprintf (known[i].varp, "\e[%.*sm",
  (int) (env - val), val) < 0)
  error (EXIT_FAILURE, errno,
-gettext ("cannot allocate memory"));
+_("cannot allocate memory"));
break;
  }
}
diff --git a/lib/printversion.c b/lib/printversion.c
index 28981d20..1f3f3d19 100644
--- a/lib/printversion.c
+++ b/lib/printversion.c
@@ -37,7 +37,7 @@ void
 print_version (FILE *stream, struct argp_state *state)
 {
   fprintf (stream, "%s (%s) %s\n", state->name, PACKAGE_NAME, PACKAGE_VERSION);
-  fprintf (stream, gettext ("\
+  fprintf (stream, _("\
 Copyright (C) %s The elfutils developers <%s>.\n\
 This is free software; see the source for copying conditions.  There is NO\n\
 warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.\n\
diff --git a/lib/system.h b/lib/system.h
index 7b650f11..1c478e1c 100644
--- a/lib/system.h
+++ b/lib/system.h
@@ -71,7 +71,7 @@
 
 /* A special gettext function we use if the strings are too short.  */
 #define sgettext(Str) \
-  ({ const char *__res = strrchr (gettext (Str), '|');   \
+  ({ const char *__res = strrchr (_(Str), '|');  \
  __res ? __res + 1 : Str; })
 
 #define gettext_noop(Str) Str
-- 
ldv


[PATCH 3/4] libcpu: consistently use _(Str) instead of gettext(Str)

2020-12-16 Thread Dmitry V. Levin
eu-config.h defines _(Str) to dgettext ("elfutils", Str) instead of
a simple gettext (Str) for a reason: the library might be indirectly
used by clients that called bindtextdomain with a domain different
from "elfutils".

The change was made automatically using the following command:
$ git grep -l '\
---
 libcpu/ChangeLog| 5 +
 libcpu/i386_lex.l   | 4 ++--
 libcpu/i386_parse.y | 4 ++--
 3 files changed, 9 insertions(+), 4 deletions(-)

diff --git a/libcpu/ChangeLog b/libcpu/ChangeLog
index 000105bf..781c8f41 100644
--- a/libcpu/ChangeLog
+++ b/libcpu/ChangeLog
@@ -1,3 +1,8 @@
+2020-12-16  Dmitry V. Levin  
+
+   * i386_lex.l (invalid_char): Replace gettext(...) with _(...).
+   * i386_parse.y (yyerror): Likewise.
+
 2020-12-12  Dmitry V. Levin  
 
* bpf_disasm.c (bswap_bpf_insn): Fix spelling typo in comment.
diff --git a/libcpu/i386_lex.l b/libcpu/i386_lex.l
index a4705aa9..b6ec0f87 100644
--- a/libcpu/i386_lex.l
+++ b/libcpu/i386_lex.l
@@ -119,8 +119,8 @@ static void
 invalid_char (int ch)
 {
   error (0, 0, (isascii (ch)
-   ? gettext ("invalid character '%c' at line %d; ignored")
-   : gettext ("invalid character '\\%o' at line %d; ignored")),
+   ? _("invalid character '%c' at line %d; ignored")
+   : _("invalid character '\\%o' at line %d; ignored")),
 ch, yylineno);
 }
 
diff --git a/libcpu/i386_parse.y b/libcpu/i386_parse.y
index 90c7bd93..9a92c2e0 100644
--- a/libcpu/i386_parse.y
+++ b/libcpu/i386_parse.y
@@ -551,8 +551,8 @@ argcomp:  kBITFIELD
 static void
 yyerror (const char *s)
 {
-  error (0, 0, gettext ("while reading i386 CPU description: %s at line %d"),
- gettext (s), i386_lineno);
+  error (0, 0, _("while reading i386 CPU description: %s at line %d"),
+ _(s), i386_lineno);
 }
 
 
-- 
ldv


[COMMITTED] libelf: Make sure we have at least a full ELF header available.

2020-12-16 Thread Mark Wielaard
When elf_memory is called we could get a slightly too small image
that doesn't contain a full ELF header (but does contain at least
the e_ident values). Require the full header before even validating
the rest of the ELF header fields.

https://sourceware.org/bugzilla/show_bug.cgi?id=27076

Signed-off-by: Mark Wielaard 
---
 libelf/ChangeLog   | 4 
 libelf/elf_begin.c | 7 +++
 2 files changed, 11 insertions(+)

diff --git a/libelf/ChangeLog b/libelf/ChangeLog
index 41727fbd..a280a262 100644
--- a/libelf/ChangeLog
+++ b/libelf/ChangeLog
@@ -1,3 +1,7 @@
+2020-12-15  Mark Wielaard  
+
+   * elf_begin.c (get_shnum): Make sure the full Ehdr is available.
+
 2020-12-12  Dmitry V. Levin  
 
* common.h: Fix spelling typo in comment.
diff --git a/libelf/elf_begin.c b/libelf/elf_begin.c
index 43828c9a..32648c15 100644
--- a/libelf/elf_begin.c
+++ b/libelf/elf_begin.c
@@ -88,6 +88,13 @@ get_shnum (void *map_address, unsigned char *e_ident, int 
fildes,
   } ehdr_mem;
   bool is32 = e_ident[EI_CLASS] == ELFCLASS32;
 
+  if ((is32 && maxsize < sizeof (Elf32_Ehdr))
+  || (!is32 && maxsize < sizeof (Elf64_Ehdr)))
+{
+   __libelf_seterrno (ELF_E_INVALID_ELF);
+  return (size_t) -1l;
+}
+
   /* Make the ELF header available.  */
   if (e_ident[EI_DATA] == MY_ELFDATA
   && (ALLOW_UNALIGNED
-- 
2.18.4



[Bug libelf/27076] heap-buffer-overflow when calling file_read_elf function in elf_begin.c in libelf

2020-12-16 Thread mark at klomp dot org via Elfutils-devel
https://sourceware.org/bugzilla/show_bug.cgi?id=27076

Mark Wielaard  changed:

   What|Removed |Added

 Status|UNCONFIRMED |RESOLVED
 CC||mark at klomp dot org
 Resolution|--- |FIXED

--- Comment #1 from Mark Wielaard  ---
I couldn't reproduce a crash, but there is a small (1 byte) over-read detected
by valgrind:

==12591== Memcheck, a memory error detector
==12591== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al.
==12591== Using Valgrind-3.15.0 and LibVEX; rerun with -h for copyright info
==12591== Command: src/stack --core ./POC -abdilmsv
==12591== 
==12591== Invalid read of size 2
==12591==at 0x4E3A768: get_shnum (elf_begin.c:139)
==12591==by 0x4E3A768: file_read_elf (elf_begin.c:289)
==12591==by 0x4E3AE48: __libelf_read_mmaped_file (elf_begin.c:552)
==12591==by 0x50A969A: dwfl_segment_report_module
(dwfl_segment_report_module.c:955)
==12591==by 0x50AC773: dwfl_core_file_report@@ELFUTILS_0.158
(core-file.c:558)
==12591==by 0x4025B6: parse_opt (stack.c:595)
==12591==by 0x56FACE3: argp_parse (in /usr/lib64/libc-2.17.so)
==12591==by 0x401C12: main (stack.c:695)
==12591==  Address 0x6c182a0 is 48 bytes inside a block of size 49 alloc'd
==12591==at 0x4C2C089: calloc (vg_replace_malloc.c:762)
==12591==by 0x50A961E: dwfl_segment_report_module
(dwfl_segment_report_module.c:907)
==12591==by 0x50AC773: dwfl_core_file_report@@ELFUTILS_0.158
(core-file.c:558)
==12591==by 0x4025B6: parse_opt (stack.c:595)
==12591==by 0x56FACE3: argp_parse (in /usr/lib64/libc-2.17.so)
==12591==by 0x401C12: main (stack.c:695)
==12591== 
src/stack: dwfl_core_file_attach: (null)
==12591== 
==12591== HEAP SUMMARY:
==12591== in use at exit: 2,536 bytes in 11 blocks
==12591==   total heap usage: 43 allocs, 32 frees, 14,913 bytes allocated
==12591== 
==12591== LEAK SUMMARY:
==12591==definitely lost: 0 bytes in 0 blocks
==12591==indirectly lost: 0 bytes in 0 blocks
==12591==  possibly lost: 0 bytes in 0 blocks
==12591==still reachable: 2,536 bytes in 11 blocks
==12591== suppressed: 0 bytes in 0 blocks
==12591== Rerun with --leak-check=full to see details of leaked memory
==12591== 
==12591== For lists of detected and suppressed errors, rerun with: -s
==12591== ERROR SUMMARY: 1 errors from 1 contexts (suppressed: 0 from 0)

Fixed by making sure we have at least a full Ehdr available (which is 52 or 64
bytes in size):
https://sourceware.org/pipermail/elfutils-devel/2020q4/003322.html

-- 
You are receiving this mail because:
You are on the CC list for the bug.

Re: [PATCH 1/2] libelf: Sync elf.h from glibc.

2020-12-16 Thread Dmitry V. Levin
On Sat, Dec 12, 2020 at 11:38:54PM +0100, Mark Wielaard wrote:
> Adds SHF_GNU_RETAIN.

This is obviously OK.


-- 
ldv


Re: [PATCH 2/2] Handle SHF_GNU_RETAIN in eu-readelf and eu-elflint.

2020-12-16 Thread Dmitry V. Levin
On Sat, Dec 12, 2020 at 11:38:55PM +0100, Mark Wielaard wrote:
> readelf -S now shows 'R' when SHF_GNU_RETAIN is set.
> elflint accepts SHF_GNU_RETAIN when set on section in --gnu mode.
> 
> Signed-off-by: Mark Wielaard 
> ---
>  src/ChangeLog   |   5 
>  src/elflint.c   |   3 +++
>  src/readelf.c   |   2 ++
>  tests/ChangeLog |   7 ++
>  tests/Makefile.am   |   2 ++
>  tests/run-retain.sh |  44 
>  tests/testfile-retain.o.bz2 | Bin 0 -> 261 bytes
>  7 files changed, 63 insertions(+)
>  create mode 100755 tests/run-retain.sh
>  create mode 100644 tests/testfile-retain.o.bz2

LGTM, thanks,


-- 
ldv


Re: Q: splitting the top level .gitignore file

2020-12-16 Thread Mark Wielaard
Hi Dmitry,

On Wed, 2020-12-16 at 02:46 +0300, Dmitry V. Levin wrote:
> By the way, what do you think about moving subdirectory parts of the top
> level .gitignore file into subdirectories?  This would be consistent with
> ChangeLog files, currently one has to update the top level ChangeLog file
> when the top level .gitignore file is changed in a way that affects
> specific subdirectories only.

Yes, I think it would make sense to have .gitignore files per subdir. I
don't know how to keep that easily up to date though. I normally use
srcdir != builddir setups and so don't immediately see when there are
generated files in the srcdir.

Thanks,

Mark


Re: [PATCH] Modernize gettext infrastructure

2020-12-16 Thread Mark Wielaard
Hi Dmitry,

On Wed, 2020-12-16 at 02:40 +0300, Dmitry V. Levin wrote:
> Switch to use AM_GNU_GETTEXT, AM_GNU_GETTEXT_VERSION, and
> AM_GNU_GETTEXT_REQUIRE_VERSION, this allows to stop bundling gettext
> infrastructure files and let autoreconf invoke autopoint which will
> set the gettext infrastructure up.

This is a good idea. And as far as I can see this is correct.

But it does mean for a git checkout people now need to have gettext-
tools installed (but not for a released build). I don't know if all the
buildbot workers have it installed, so when you do check it in please
keep an eye on 
https://builder.wildebeest.org/buildbot/#/builders?tags=elfutils

It might also be an idea to list the maintainer dependencies in the
README file (we don't currently list any others though).

Thanks,

Mark


Re: [PATCH] libebl: consistently use _(Str) instead of gettext(Str)

2020-12-16 Thread Mark Wielaard
Hi Dmitry,

On Wed, 2020-12-16 at 04:48 +0300, Dmitry V. Levin wrote:
> libeblP.h defines _(Str) to dgettext ("elfutils", Str) instead of
> a simple gettext (Str) for a reason: the library might be indirectly
> used by clients that called bindtextdomain with a domain different
> from "elfutils".

This, and the other gettext -> _ changes look OK.
For the src tools it is probably unnecessary, but good for consistency.
Please push.

Thanks,

Mark


Re: [PATCH] Modernize gettext infrastructure

2020-12-16 Thread Dmitry V. Levin
Hi Mark,

On Wed, Dec 16, 2020 at 03:05:54PM +0100, Mark Wielaard wrote:
> Hi Dmitry,
> 
> On Wed, 2020-12-16 at 02:40 +0300, Dmitry V. Levin wrote:
> > Switch to use AM_GNU_GETTEXT, AM_GNU_GETTEXT_VERSION, and
> > AM_GNU_GETTEXT_REQUIRE_VERSION, this allows to stop bundling gettext
> > infrastructure files and let autoreconf invoke autopoint which will
> > set the gettext infrastructure up.
> 
> This is a good idea. And as far as I can see this is correct.
> 
> But it does mean for a git checkout people now need to have gettext-
> tools installed (but not for a released build).

I suppose people are used to it nowadays, given that AM_GNU_GETTEXT
and AM_GNU_GETTEXT_VERSION are quite widespread macros.

> I don't know if all the
> buildbot workers have it installed, so when you do check it in please
> keep an eye on 
> https://builder.wildebeest.org/buildbot/#/builders?tags=elfutils

Is there a way to find it out beforehand?


-- 
ldv


Re: [PATCH] Modernize gettext infrastructure

2020-12-16 Thread Mark Wielaard
Hi Dmitry,

On Wed, 2020-12-16 at 17:59 +0300, Dmitry V. Levin wrote:
> > I don't know if all the
> > buildbot workers have it installed, so when you do check it in please
> > keep an eye on 
> > https://builder.wildebeest.org/buildbot/#/builders?tags=elfutils
> 
> Is there a way to find it out beforehand?

No, sorry, not at the moment. You'll just have to push and see.

I'll see if I can add try-server support to the buildbot this end-of-
year vacation, but no promises.

If there are issues on any of the buildbots that aren't clear then
shell access can be arranged, but different people maintain different
machines.

Cheers,

Mark


Re: [PATCH 1/2] libelf: Sync elf.h from glibc.

2020-12-16 Thread Mark Wielaard
On Wed, Dec 16, 2020 at 01:44:44PM +0300, Dmitry V. Levin wrote:
> On Sat, Dec 12, 2020 at 11:38:54PM +0100, Mark Wielaard wrote:
> > Adds SHF_GNU_RETAIN.
> 
> This is obviously OK.

Thanks, pushed.


Re: [PATCH 2/2] Handle SHF_GNU_RETAIN in eu-readelf and eu-elflint.

2020-12-16 Thread Mark Wielaard
On Wed, Dec 16, 2020 at 01:49:30PM +0300, Dmitry V. Levin wrote:
> On Sat, Dec 12, 2020 at 11:38:55PM +0100, Mark Wielaard wrote:
> > readelf -S now shows 'R' when SHF_GNU_RETAIN is set.
> > elflint accepts SHF_GNU_RETAIN when set on section in --gnu mode.
> > 
> > Signed-off-by: Mark Wielaard 
> > ---
> >  src/ChangeLog   |   5 
> >  src/elflint.c   |   3 +++
> >  src/readelf.c   |   2 ++
> >  tests/ChangeLog |   7 ++
> >  tests/Makefile.am   |   2 ++
> >  tests/run-retain.sh |  44 
> >  tests/testfile-retain.o.bz2 | Bin 0 -> 261 bytes
> >  7 files changed, 63 insertions(+)
> >  create mode 100755 tests/run-retain.sh
> >  create mode 100644 tests/testfile-retain.o.bz2
> 
> LGTM, thanks,

Thanks, pushed.


[PATCH] libcpu: linking i386_gendis requires obstack.

2020-12-16 Thread Érico Nogueira via Elfutils-devel
From: Érico Rolim 

---

Noticed while building from master today.

 libcpu/ChangeLog   | 4 
 libcpu/Makefile.am | 2 +-
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/libcpu/ChangeLog b/libcpu/ChangeLog
index 781c8f41..af7ea96c 100644
--- a/libcpu/ChangeLog
+++ b/libcpu/ChangeLog
@@ -1,3 +1,7 @@
+2020-12-16  Érico Nogueira  
+
+   * Makefile.am (i386_gendis_LDADD): Add obstack_LIBS.
+
 2020-12-16  Dmitry V. Levin  
 
* i386_lex.l (invalid_char): Replace gettext(...) with _(...).
diff --git a/libcpu/Makefile.am b/libcpu/Makefile.am
index 59def7d1..43844ecf 100644
--- a/libcpu/Makefile.am
+++ b/libcpu/Makefile.am
@@ -86,7 +86,7 @@ i386_lex_CFLAGS = -Wno-unused-label -Wno-unused-function 
-Wno-sign-compare \
 i386_parse.o: i386_parse.c i386.mnemonics
 i386_parse_CFLAGS = -DNMNES="`wc -l < i386.mnemonics`"
 i386_lex.o: i386_parse.h
-i386_gendis_LDADD = $(libeu) -lm
+i386_gendis_LDADD = $(libeu) -lm $(obstack_LIBS)
 
 i386_parse.h: i386_parse.c ;
 
-- 
2.29.2



[PATCH] libdwfl: use XSI-compliant strerror_r.

2020-12-16 Thread Érico Nogueira via Elfutils-devel
From: Érico Rolim 

The Linux man pages recommend this version of the function for portable
applications.
---

This change could be made into its own tiny compat source file, if we
want to keep _GNU_SOURCE undefined for the smallest piece of code.
I'm okay with either approach.

I ran the testsuite on a glibc system, didn't spot any issues.

 libdwfl/ChangeLog|  4 
 libdwfl/dwfl_error.c | 11 ++-
 2 files changed, 14 insertions(+), 1 deletion(-)

diff --git a/libdwfl/ChangeLog b/libdwfl/ChangeLog
index f9f6f01f..d22f9892 100644
--- a/libdwfl/ChangeLog
+++ b/libdwfl/ChangeLog
@@ -1,3 +1,7 @@
+2020-12-16  Érico Nogueira  
+
+   * dwfl_error.c (strerror_r): Always use the XSI-compliant version.
+
 2020-12-16  Dmitry V. Levin  
 
* argp-std.c (_): Remove.
diff --git a/libdwfl/dwfl_error.c b/libdwfl/dwfl_error.c
index 7bcf61cc..e5db1217 100644
--- a/libdwfl/dwfl_error.c
+++ b/libdwfl/dwfl_error.c
@@ -30,6 +30,11 @@
 # include 
 #endif
 
+/* Guarantee that we get the XSI compliant strerror_r */
+#ifdef _GNU_SOURCE
+#undef _GNU_SOURCE
+#endif
+
 #include 
 #include 
 #include 
@@ -136,6 +141,8 @@ __libdwfl_seterrno (Dwfl_Error error)
   global_error = canonicalize (error);
 }
 
+/* To store the error message from strerror_r */
+static __thread char errormsg[128];
 
 const char *
 dwfl_errmsg (int error)
@@ -154,7 +161,9 @@ dwfl_errmsg (int error)
   switch (error &~ 0x)
 {
 case OTHER_ERROR (ERRNO):
-  return strerror_r (error & 0x, "bad", 0);
+  if (strerror_r (error & 0x, errormsg, sizeof errormsg))
+   return "strerror_r() failed()";
+  return errormsg;
 case OTHER_ERROR (LIBELF):
   return elf_errmsg (error & 0x);
 case OTHER_ERROR (LIBDW):
-- 
2.29.2



[PATCH] src/readelf: use qsort instead of qsort_r.

2020-12-16 Thread Érico Nogueira via Elfutils-devel
From: Érico Rolim 

This program is single threaded, so using qsort with a global variable
isn't a danger. The interface for qsort_r isn't standardized (and
diverges between glibc and FreeBSD, for example), which makes usage of
qsort, where possible, preferrable.
---

FreeBSD's qsort_r can be seen in:

http://man.bsd.lv/FreeBSD-12.0/qsort

 src/ChangeLog |  4 
 src/readelf.c | 14 ++
 2 files changed, 14 insertions(+), 4 deletions(-)

diff --git a/src/ChangeLog b/src/ChangeLog
index a7b227db..9bb90732 100644
--- a/src/ChangeLog
+++ b/src/ChangeLog
@@ -1,3 +1,7 @@
+2020-12-16  Érico Nogueira  
+
+   * readelf.c (qsort_r): Use qsort for improved portability.
+
 2020-12-16  Dmitry V. Levin  
 
* *.c: Replace gettext(...) with _(...).
diff --git a/src/readelf.c b/src/readelf.c
index 824ab31b..f2ebd206 100644
--- a/src/readelf.c
+++ b/src/readelf.c
@@ -4829,10 +4829,13 @@ listptr_base (struct listptr *p)
   return cudie_base (&cu);
 }
 
+/* To store the name used in compare_listptr */
+static const char *sort_listptr_name;
+
 static int
-compare_listptr (const void *a, const void *b, void *arg)
+compare_listptr (const void *a, const void *b)
 {
-  const char *name = arg;
+  const char *name = sort_listptr_name;
   struct listptr *p1 = (void *) a;
   struct listptr *p2 = (void *) b;
 
@@ -4942,8 +4945,11 @@ static void
 sort_listptr (struct listptr_table *table, const char *name)
 {
   if (table->n > 0)
-qsort_r (table->table, table->n, sizeof table->table[0],
-&compare_listptr, (void *) name);
+{
+  sort_listptr_name = name;
+  qsort (table->table, table->n, sizeof table->table[0],
+&compare_listptr);
+}
 }
 
 static bool
-- 
2.29.2



Re: [PATCH] libcpu: linking i386_gendis requires obstack.

2020-12-16 Thread Mark Wielaard
On Wed, Dec 16, 2020 at 03:56:14PM -0300, Érico Nogueira via Elfutils-devel 
wrote:
> Noticed while building from master today.

Yes, you are right. i386_parse.y uses obstacks.  Thanks for the
ChangeLog entry. Next time please do use a Signed-off-by line. See
CONTRIBUTING.

Pushed,

Mark



Re: [PATCH] libcpu: linking i386_gendis requires obstack.

2020-12-16 Thread Érico Nogueira via Elfutils-devel
On Wed Dec 16, 2020 at 6:48 PM -03, Mark Wielaard wrote:
> On Wed, Dec 16, 2020 at 03:56:14PM -0300, Érico Nogueira via
> Elfutils-devel wrote:
> > Noticed while building from master today.
>
> Yes, you are right. i386_parse.y uses obstacks. Thanks for the
> ChangeLog entry. Next time please do use a Signed-off-by line. See
> CONTRIBUTING.

I completely forgot about the Signed-off-by part, sorry! Will re-send
my other patches with it.

>
> Pushed,
>
> Mark



[PATCH v2 2/2] src/readelf: use qsort instead of qsort_r.

2020-12-16 Thread Érico Nogueira via Elfutils-devel
From: Érico Rolim 

This program is single threaded, so using qsort with a global variable
isn't a danger. The interface for qsort_r isn't standardized (and
diverges between glibc and FreeBSD, for example), which makes usage of
qsort, where possible, preferrable.

Signed-off-by: Érico Rolim 
---

Only difference from the initial patch is that this includes the
Signed-off-by line.

 src/ChangeLog |  4 
 src/readelf.c | 14 ++
 2 files changed, 14 insertions(+), 4 deletions(-)

diff --git a/src/ChangeLog b/src/ChangeLog
index 2e428e0b..5c1ad1a2 100644
--- a/src/ChangeLog
+++ b/src/ChangeLog
@@ -1,3 +1,7 @@
+2020-12-16  Érico Nogueira  
+
+   * readelf.c (qsort_r): Use qsort for improved portability.
+
 2020-12-12  Mark Wielaard  
 
* elflint.c (check_sections): Handle SHF_GNU_RETAIN.
diff --git a/src/readelf.c b/src/readelf.c
index 829a418d..0001a3d8 100644
--- a/src/readelf.c
+++ b/src/readelf.c
@@ -4831,10 +4831,13 @@ listptr_base (struct listptr *p)
   return cudie_base (&cu);
 }
 
+/* To store the name used in compare_listptr */
+static const char *sort_listptr_name;
+
 static int
-compare_listptr (const void *a, const void *b, void *arg)
+compare_listptr (const void *a, const void *b)
 {
-  const char *name = arg;
+  const char *name = sort_listptr_name;
   struct listptr *p1 = (void *) a;
   struct listptr *p2 = (void *) b;
 
@@ -4944,8 +4947,11 @@ static void
 sort_listptr (struct listptr_table *table, const char *name)
 {
   if (table->n > 0)
-qsort_r (table->table, table->n, sizeof table->table[0],
-&compare_listptr, (void *) name);
+{
+  sort_listptr_name = name;
+  qsort (table->table, table->n, sizeof table->table[0],
+&compare_listptr);
+}
 }
 
 static bool
-- 
2.29.2



[PATCH v2 1/2] libdwfl: use XSI-compliant strerror_r.

2020-12-16 Thread Érico Nogueira via Elfutils-devel
From: Érico Rolim 

The Linux man pages recommend this version of the function for portable
applications.

Signed-off-by: Érico Rolim 
---

Only difference from the initial patch is that this includes the
Signed-off-by line.

 libdwfl/ChangeLog|  4 
 libdwfl/dwfl_error.c | 11 ++-
 2 files changed, 14 insertions(+), 1 deletion(-)

diff --git a/libdwfl/ChangeLog b/libdwfl/ChangeLog
index f9f6f01f..d22f9892 100644
--- a/libdwfl/ChangeLog
+++ b/libdwfl/ChangeLog
@@ -1,3 +1,7 @@
+2020-12-16  Érico Nogueira  
+
+   * dwfl_error.c (strerror_r): Always use the XSI-compliant version.
+
 2020-12-16  Dmitry V. Levin  
 
* argp-std.c (_): Remove.
diff --git a/libdwfl/dwfl_error.c b/libdwfl/dwfl_error.c
index 7bcf61cc..e5db1217 100644
--- a/libdwfl/dwfl_error.c
+++ b/libdwfl/dwfl_error.c
@@ -30,6 +30,11 @@
 # include 
 #endif
 
+/* Guarantee that we get the XSI compliant strerror_r */
+#ifdef _GNU_SOURCE
+#undef _GNU_SOURCE
+#endif
+
 #include 
 #include 
 #include 
@@ -136,6 +141,8 @@ __libdwfl_seterrno (Dwfl_Error error)
   global_error = canonicalize (error);
 }
 
+/* To store the error message from strerror_r */
+static __thread char errormsg[128];
 
 const char *
 dwfl_errmsg (int error)
@@ -154,7 +161,9 @@ dwfl_errmsg (int error)
   switch (error &~ 0x)
 {
 case OTHER_ERROR (ERRNO):
-  return strerror_r (error & 0x, "bad", 0);
+  if (strerror_r (error & 0x, errormsg, sizeof errormsg))
+   return "strerror_r() failed()";
+  return errormsg;
 case OTHER_ERROR (LIBELF):
   return elf_errmsg (error & 0x);
 case OTHER_ERROR (LIBDW):
-- 
2.29.2



Buildbot failure in Wildebeest Builder on whole buildset

2020-12-16 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/11/builds/634

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

Worker for this Build: fedora-ppc64le

Build Reason: 
Blamelist: Dmitry V. Levin 

BUILD FAILED: failed 'autoreconf -f ...' (failure)

Sincerely,
 -The BuildbotThe Buildbot has detected a failed build on builder whole 
buildset while building elfutils.
Full details are available at:
https://builder.wildebeest.org/buildbot/#builders/12/builds/633

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

Worker for this Build: fedora-ppc64

Build Reason: 
Blamelist: Dmitry V. Levin 

BUILD FAILED: failed 'autoreconf -f ...' (failure)

Sincerely,
 -The Buildbot



Re: Buildbot failure in Wildebeest Builder on whole buildset

2020-12-16 Thread Mark Wielaard
Hi,

Don't worry...

On Wed, Dec 16, 2020 at 11:54:16PM +, 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/11/builds/634
> 
> Buildbot URL: https://builder.wildebeest.org/buildbot/
> 
> Worker for this Build: fedora-ppc64le
> 
> Build Reason: 
> Blamelist: Dmitry V. Levin 
> 
> BUILD FAILED: failed 'autoreconf -f ...' (failure)
> 
> Sincerely,
>  -The BuildbotThe Buildbot has detected a failed build on builder whole 
> buildset while building elfutils.
> Full details are available at:
> https://builder.wildebeest.org/buildbot/#builders/12/builds/633
> 
> Buildbot URL: https://builder.wildebeest.org/buildbot/
> 
> Worker for this Build: fedora-ppc64
> 
> Build Reason: 
> Blamelist: Dmitry V. Levin 
> 
> BUILD FAILED: failed 'autoreconf -f ...' (failure)

The ppc64 and ppc64le builders didn't have autopoint installed
(gettext-devel) they have now and newer builds have secceeded.

Cheers,

Mark


Re: [PATCH v2 1/2] libdwfl: use XSI-compliant strerror_r.

2020-12-16 Thread Dmitry V. Levin
On Wed, Dec 16, 2020 at 07:30:11PM -0300, Érico Nogueira via Elfutils-devel 
wrote:
> From: Érico Rolim 
> 
> The Linux man pages recommend this version of the function for portable
> applications.
> 
> Signed-off-by: Érico Rolim 

I'd rather not do it this way because __xpg_strerror_r in glibc is a
wrapper around GNU strerror_r which does this almost always unneeded
string copying.  Instead of penalizing GNU systems, I'd suggest checking
at configure time what kind of strerror_r do we have, and choosing the
code appropriately.

> ---
> 
> Only difference from the initial patch is that this includes the
> Signed-off-by line.
> 
>  libdwfl/ChangeLog|  4 
>  libdwfl/dwfl_error.c | 11 ++-
>  2 files changed, 14 insertions(+), 1 deletion(-)
> 
> diff --git a/libdwfl/ChangeLog b/libdwfl/ChangeLog
> index f9f6f01f..d22f9892 100644
> --- a/libdwfl/ChangeLog
> +++ b/libdwfl/ChangeLog
> @@ -1,3 +1,7 @@
> +2020-12-16  Érico Nogueira  
> +
> + * dwfl_error.c (strerror_r): Always use the XSI-compliant version.
> +
>  2020-12-16  Dmitry V. Levin  
>  
>   * argp-std.c (_): Remove.
> diff --git a/libdwfl/dwfl_error.c b/libdwfl/dwfl_error.c
> index 7bcf61cc..e5db1217 100644
> --- a/libdwfl/dwfl_error.c
> +++ b/libdwfl/dwfl_error.c
> @@ -30,6 +30,11 @@
>  # include 
>  #endif
>  
> +/* Guarantee that we get the XSI compliant strerror_r */
> +#ifdef _GNU_SOURCE
> +#undef _GNU_SOURCE
> +#endif
> +
>  #include 
>  #include 
>  #include 
> @@ -136,6 +141,8 @@ __libdwfl_seterrno (Dwfl_Error error)
>global_error = canonicalize (error);
>  }
>  
> +/* To store the error message from strerror_r */
> +static __thread char errormsg[128];
>  
>  const char *
>  dwfl_errmsg (int error)
> @@ -154,7 +161,9 @@ dwfl_errmsg (int error)
>switch (error &~ 0x)
>  {
>  case OTHER_ERROR (ERRNO):
> -  return strerror_r (error & 0x, "bad", 0);
> +  if (strerror_r (error & 0x, errormsg, sizeof errormsg))
> + return "strerror_r() failed()";
> +  return errormsg;
>  case OTHER_ERROR (LIBELF):
>return elf_errmsg (error & 0x);
>  case OTHER_ERROR (LIBDW):

What if we had something like this:

static const char *
errnomsg(int error)
{
  static const char unknown[] = "unknown error";

#ifdef HAVE_STRERROR_R_POSIX_SIGNATURE
  static __thread char msg[128];
  return strerror_r (error, msg, sizeof (msg)) ? unknown : msg;
#else
  return strerror_r (error, unknown, 0);
#endif
}

and use it in dwfl_errmsg instead of strerror_r?


-- 
ldv


Re: [PATCH v2 1/2] libdwfl: use XSI-compliant strerror_r.

2020-12-16 Thread Érico Nogueira via Elfutils-devel
On Wed Dec 16, 2020 at 9:08 PM -03, Dmitry V. Levin wrote:
> On Wed, Dec 16, 2020 at 07:30:11PM -0300, Érico Nogueira via
> Elfutils-devel wrote:
> > From: Érico Rolim 
> > 
> > The Linux man pages recommend this version of the function for portable
> > applications.
> > 
> > Signed-off-by: Érico Rolim 
>
> I'd rather not do it this way because __xpg_strerror_r in glibc is a
> wrapper around GNU strerror_r which does this almost always unneeded
> string copying. Instead of penalizing GNU systems, I'd suggest checking
> at configure time what kind of strerror_r do we have, and choosing the
> code appropriately.

Fair enough. The GNU version of strerror_r does have a nicer API :)

>
> > ---
> > 
> > Only difference from the initial patch is that this includes the
> > Signed-off-by line.
> > 
> >  libdwfl/ChangeLog|  4 
> >  libdwfl/dwfl_error.c | 11 ++-
> >  2 files changed, 14 insertions(+), 1 deletion(-)
> > 
> > diff --git a/libdwfl/ChangeLog b/libdwfl/ChangeLog
> > index f9f6f01f..d22f9892 100644
> > --- a/libdwfl/ChangeLog
> > +++ b/libdwfl/ChangeLog
> > @@ -1,3 +1,7 @@
> > +2020-12-16  Érico Nogueira  
> > +
> > +   * dwfl_error.c (strerror_r): Always use the XSI-compliant version.
> > +
> >  2020-12-16  Dmitry V. Levin  
> >  
> > * argp-std.c (_): Remove.
> > diff --git a/libdwfl/dwfl_error.c b/libdwfl/dwfl_error.c
> > index 7bcf61cc..e5db1217 100644
> > --- a/libdwfl/dwfl_error.c
> > +++ b/libdwfl/dwfl_error.c
> > @@ -30,6 +30,11 @@
> >  # include 
> >  #endif
> >  
> > +/* Guarantee that we get the XSI compliant strerror_r */
> > +#ifdef _GNU_SOURCE
> > +#undef _GNU_SOURCE
> > +#endif
> > +
> >  #include 
> >  #include 
> >  #include 
> > @@ -136,6 +141,8 @@ __libdwfl_seterrno (Dwfl_Error error)
> >global_error = canonicalize (error);
> >  }
> >  
> > +/* To store the error message from strerror_r */
> > +static __thread char errormsg[128];
> >  
> >  const char *
> >  dwfl_errmsg (int error)
> > @@ -154,7 +161,9 @@ dwfl_errmsg (int error)
> >switch (error &~ 0x)
> >  {
> >  case OTHER_ERROR (ERRNO):
> > -  return strerror_r (error & 0x, "bad", 0);
> > +  if (strerror_r (error & 0x, errormsg, sizeof errormsg))
> > +   return "strerror_r() failed()";
> > +  return errormsg;
> >  case OTHER_ERROR (LIBELF):
> >return elf_errmsg (error & 0x);
> >  case OTHER_ERROR (LIBDW):
>
> What if we had something like this:
>
> static const char *
> errnomsg(int error)
> {
> static const char unknown[] = "unknown error";
>
> #ifdef HAVE_STRERROR_R_POSIX_SIGNATURE
> static __thread char msg[128];
> return strerror_r (error, msg, sizeof (msg)) ? unknown : msg;
> #else
> return strerror_r (error, unknown, 0);
> #endif
> }

Would you prefer this go in lib/ as a "general usage" function or do I
just leave it in libdwfl/ for now?

As a side note, the trick of passing a constant string and buflen=0 is
quite neat, and isn't obvious from reading the man page :)

>
> and use it in dwfl_errmsg instead of strerror_r?
>
>
> --
> ldv