[PATCH 1/4] Consistently define _(Str) using dgettext ("elfutils", Str)
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)
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)
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.
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
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.
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.
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
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
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)
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
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
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.
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.
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.
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.
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.
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.
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.
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.
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.
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
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
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.
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.
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