[PATCH v2] Avoid using err() and errx() in tests
fprintf() and exit() do the same job and are portable.
(v1 was missing the errno.h include in allfcts.c)
Signed-off-by: Ulf Hermann
---
tests/ChangeLog | 6 ++
tests/allfcts.c | 47 ---
tests/buildid.c | 6 +++---
tests/debugaltlink.c | 6 +++---
4 files changed, 44 insertions(+), 21 deletions(-)
diff --git a/tests/ChangeLog b/tests/ChangeLog
index 6dab801..c2619d0 100644
--- a/tests/ChangeLog
+++ b/tests/ChangeLog
@@ -1,5 +1,11 @@
2017-05-04 Ulf Hermann
+ * allfcts.c: Use fprintf and exit rather than err and errx.
+ * buildid.c: Likewise.
+ * debugaltlink.c: Likewise.
+
+2017-05-04 Ulf Hermann
+
* elfstrmerge.c: Don't fchmod or fchown the output file if fchmod or
fchown don't exist.
diff --git a/tests/allfcts.c b/tests/allfcts.c
index d3c8d26..84ac53d 100644
--- a/tests/allfcts.c
+++ b/tests/allfcts.c
@@ -18,12 +18,14 @@
# include
#endif
-#include
+#include
#include
#include ELFUTILS_HEADER(dw)
#include ELFUTILS_HEADER(dwelf)
#include
#include
+#include
+#include
static int
@@ -47,16 +49,24 @@ setup_alt (Dwarf *main)
ssize_t ret = dwelf_dwarf_gnu_debugaltlink (main, &alt_name, &build_id);
if (ret == 0)
return NULL;
- if (ret == -1)
-errx (1, "dwelf_dwarf_gnu_debugaltlink: %s", dwarf_errmsg (-1));
+ if (ret == -1) {
+fprintf (stderr, "allfcts: dwelf_dwarf_gnu_debugaltlink: %s\n",
dwarf_errmsg (-1));
+exit(1);
+ }
int fd = open (alt_name, O_RDONLY);
- if (fd < 0)
-err (1, "open (%s)", alt_name);
+ if (fd < 0) {
+fprintf (stderr, "allfcts: open (%s): %s\n", alt_name, strerror(errno));
+exit(1);
+ }
Dwarf *dbg_alt = dwarf_begin (fd, DWARF_C_READ);
- if (dbg_alt == NULL)
-errx (1, "dwarf_begin (%s): %s", alt_name, dwarf_errmsg (-1));
- if (elf_cntl (dwarf_getelf (dbg_alt), ELF_C_FDREAD) != 0)
-errx (1, "elf_cntl (%s, ELF_C_FDREAD): %s", alt_name, elf_errmsg (-1));
+ if (dbg_alt == NULL) {
+fprintf (stderr, "dwarf_begin (%s): %s\n", alt_name, dwarf_errmsg (-1));
+exit(1);
+ }
+ if (elf_cntl (dwarf_getelf (dbg_alt), ELF_C_FDREAD) != 0) {
+fprintf (stderr, "elf_cntl (%s, ELF_C_FDREAD): %s\n", alt_name, elf_errmsg
(-1));
+exit(1);
+ }
close (fd);
dwarf_setalt (main, dbg_alt);
return dbg_alt;
@@ -68,8 +78,10 @@ main (int argc, char *argv[])
for (int i = 1; i < argc; ++i)
{
int fd = open (argv[i], O_RDONLY);
- if (fd < 0)
- err (1, "open (%s)", argv[i]);
+ if (fd < 0) {
+ fprintf (stderr, "open (%s): %s\n", argv[i], strerror(errno));
+ exit(1);
+ }
Dwarf *dbg = dwarf_begin (fd, DWARF_C_READ);
if (dbg != NULL)
@@ -89,9 +101,11 @@ main (int argc, char *argv[])
do
{
doff = dwarf_getfuncs (die, cb, NULL, doff);
- if (dwarf_errno () != 0)
- errx (1, "dwarf_getfuncs (%s): %s",
- argv[i], dwarf_errmsg (-1));
+ if (dwarf_errno () != 0) {
+ fprintf (stderr, "dwarf_getfuncs (%s): %s\n",
+argv[i], dwarf_errmsg (-1));
+ exit(1);
+ }
}
while (doff != 0);
@@ -102,7 +116,10 @@ main (int argc, char *argv[])
dwarf_end (dbg);
}
else
- errx (1, "dwarf_begin (%s): %s", argv[i], dwarf_errmsg (-1));
+{
+ fprintf (stderr, "dwarf_begin (%s): %s\n", argv[i], dwarf_errmsg
(-1));
+ exit(1);
+ }
close (fd);
}
diff --git a/tests/buildid.c b/tests/buildid.c
index 87c1877..2d33402 100644
--- a/tests/buildid.c
+++ b/tests/buildid.c
@@ -18,7 +18,6 @@
#include
#include
#include
-#include
#include
#include ELFUTILS_HEADER(elf)
#include ELFUTILS_HEADER(dwelf)
@@ -62,8 +61,9 @@ main (int argc, char *argv[])
printf ("%s: \n", file);
break;
case -1:
- errx (1, "dwelf_elf_gnu_build_id (%s): %s",
- file, elf_errmsg (-1));
+ fprintf (stderr, "dwelf_elf_gnu_build_id (%s): %s\n",
+ file, elf_errmsg (-1));
+ return 1;
default:
printf ("%s: build ID: ", file);
const unsigned char *p = build_id;
diff --git a/tests/debugaltlink.c b/tests/debugaltlink.c
index 6d97d50..b470e31 100644
--- a/tests/debugaltlink.c
+++ b/tests/debugaltlink.c
@@ -18,7 +18,6 @@
#include
#include
#include
-#include
#include
#include ELFUTILS_HEADER(dw)
#include ELFUTILS_HEADER(dwelf)
@@ -64,8 +63,9 @@ main (int argc, char *argv[])
printf ("%s: \n", file);
break;
case -1:
- errx (1, "dwelf_dwarf_gnu_debugaltlink (%s): %s",
- file, dwarf_errmsg (-1));
+ fprintf (stderr, "dwelf_dwarf_gnu_debugaltlink (%s): %s\n",
+ file, dwarf_errmsg (-1));
+ return 1;
default:
printf ("%s:
Re: windows patches
Hi Mark, > Thanks a lot for all your work and posting the patches. > I will go through them next week. Thanks for reviewing them. The ones that got accepted will already make my life easier. > I quickly scanned some just now. Some seem fine to go in. But others are > a bit more iffy. I think some should really go towards gnulib if we are > going to use that anyway so other projects get the same portability > benefits. Let's do the easy ones first. I will handle the gnulib and glibc material when I get back in fall. > And I do worry a bit about others, like the O_BINARY one for > example that patches every open call. That seems impossible to properly > maintain and is clearly intended for a platform that is really not even > POSIX/Unix-like. If we want elfutils to work on windows, we need to take care of the text vs. binary issue somehow. If you open a file in text mode, any "\n" not prefixed by "\r" will be replaced with "\r\n" when reading or writing. This is quite disastrous. I couldn't come up with anything better than just adding O_BINARY everywhere. If there is a better way, I'll be happy to change the code. cheers, Ulf
[PATCH v2] Cast pid_t to long long when printing
On windows x86_64 pid_t is 64bit wide. We need to adapt our printf
format respectively.
(We can actually print the extra bits in a portable way, so let's do it
rather than casting to int and ignoring them.)
Signed-off-by: Ulf Hermann
---
src/ChangeLog | 4
src/stack.c | 22 +++---
2 files changed, 15 insertions(+), 11 deletions(-)
diff --git a/src/ChangeLog b/src/ChangeLog
index a474331..1c67b57 100644
--- a/src/ChangeLog
+++ b/src/ChangeLog
@@ -1,5 +1,9 @@
2017-05-04 Ulf Hermann
+ * stack.c: Cast pid_t to int when printing using %d.
+
+2017-05-04 Ulf Hermann
+
* strip.c: Close and reopen file when renaming.
2017-05-04 Ulf Hermann
diff --git a/src/stack.c b/src/stack.c
index 1f5a1c6..da912e5 100644
--- a/src/stack.c
+++ b/src/stack.c
@@ -362,7 +362,7 @@ print_frames (struct frames *frames, pid_t tid, int
dwflerr, const char *what)
if (frames->frames > 0)
frames_shown = true;
- printf ("TID %d:\n", tid);
+ printf ("TID %lld:\n", (long long)tid);
int frame_nr = 0;
for (int nr = 0; nr < frames->frames && (maxframes == 0
|| frame_nr < maxframes); nr++)
@@ -419,8 +419,8 @@ print_frames (struct frames *frames, pid_t tid, int
dwflerr, const char *what)
}
if (frames->frames > 0 && frame_nr == maxframes)
-error (0, 0, "tid %d: shown max number of frames "
- "(%d, use -n 0 for unlimited)", tid, maxframes);
+error (0, 0, "tid %lld: shown max number of frames "
+ "(%d, use -n 0 for unlimited)", (long long)tid, maxframes);
else if (dwflerr != 0)
{
if (frames->frames > 0)
@@ -440,11 +440,11 @@ print_frames (struct frames *frames, pid_t tid,
int dwflerr, const char *what)
else
modname = "";
}
- error (0, 0, "%s tid %d at 0x%" PRIx64 " in %s: %s", what, tid,
+ error (0, 0, "%s tid %lld at 0x%" PRIx64 " in %s: %s", what, (long
long)tid,
pc_adjusted, modname, dwfl_errmsg (dwflerr));
}
else
- error (0, 0, "%s tid %d: %s", what, tid, dwfl_errmsg (dwflerr));
+ error (0, 0, "%s tid %lld: %s", what, (long long)tid, dwfl_errmsg
(dwflerr));
}
}
@@ -575,10 +575,10 @@ parse_opt (int key, char *arg __attribute__
((unused)),
int err = dwfl_linux_proc_report (dwfl, pid);
if (err < 0)
- error (EXIT_BAD, 0, "dwfl_linux_proc_report pid %d: %s", pid,
+ error (EXIT_BAD, 0, "dwfl_linux_proc_report pid %lld: %s", (long
long)pid,
dwfl_errmsg (-1));
else if (err > 0)
- error (EXIT_BAD, err, "dwfl_linux_proc_report pid %d", pid);
+ error (EXIT_BAD, err, "dwfl_linux_proc_report pid %lld", (long
long)pid);
}
if (core != NULL)
@@ -597,10 +597,10 @@ parse_opt (int key, char *arg __attribute__
((unused)),
{
int err = dwfl_linux_proc_attach (dwfl, pid, false);
if (err < 0)
- error (EXIT_BAD, 0, "dwfl_linux_proc_attach pid %d: %s", pid,
+ error (EXIT_BAD, 0, "dwfl_linux_proc_attach pid %lld: %s", (long
long)pid,
dwfl_errmsg (-1));
else if (err > 0)
- error (EXIT_BAD, err, "dwfl_linux_proc_attach pid %d", pid);
+ error (EXIT_BAD, err, "dwfl_linux_proc_attach pid %lld", (long
long)pid);
}
if (core != NULL)
@@ -688,7 +688,7 @@ invoked with bad or missing arguments it will exit
with return code 64.")
if (show_modules)
{
- printf ("PID %d - %s module memory map\n", dwfl_pid (dwfl),
+ printf ("PID %lld - %s module memory map\n", (long long)dwfl_pid
(dwfl),
pid != 0 ? "process" : "core");
if (dwfl_getmodules (dwfl, module_callback, NULL, 0) != 0)
error (EXIT_BAD, 0, "dwfl_getmodules: %s", dwfl_errmsg (-1));
@@ -721,7 +721,7 @@ invoked with bad or missing arguments it will exit
with return code 64.")
}
else
{
- printf ("PID %d - %s\n", dwfl_pid (dwfl), pid != 0 ? "process" :
"core");
+ printf ("PID %lld - %s\n", (long long)dwfl_pid (dwfl), pid != 0 ?
"process" : "core");
switch (dwfl_getthreads (dwfl, thread_callback, &frames))
{
case DWARF_CB_OK:
--
2.8.1.windows.1
[PATCH] On non-linux systems, don't use native signal numbers
We assume core files from linux systems, so we should use the linux
version of the signals when reading them. Other OS might have different
signal numbers.
Signed-off-by: Ulf Hermann
---
src/readelf.c | 25 -
1 file changed, 20 insertions(+), 5 deletions(-)
diff --git a/src/readelf.c b/src/readelf.c
index 6811ace..01d2a56 100644
--- a/src/readelf.c
+++ b/src/readelf.c
@@ -57,6 +57,21 @@
#include "../libdw/known-dwarf.h"
+#ifdef __linux__
+#define CORE_SIGILL SIGILL
+#define CORE_SIGBUS SIGBUS
+#define CORE_SIGFPE SIGFPE
+#define CORE_SIGSEGV SIGSEGV
+#define CORE_SI_USER SI_USER
+#else
+/* We want the linux version of those as that is what shows up in the
core files. */
+#define CORE_SIGILL 4 /* Illegal instruction (ANSI). */
+#define CORE_SIGBUS 7 /* BUS error (4.2 BSD). */
+#define CORE_SIGFPE 8 /* Floating-point exception (ANSI). */
+#define CORE_SIGSEGV 11 /* Segmentation violation (ANSI). */
+#define CORE_SI_USER 0 /* Sent by kill, sigsend. */
+#endif
+
/* Name and version of program. */
ARGP_PROGRAM_VERSION_HOOK_DEF = print_version;
@@ -9244,10 +9259,10 @@ handle_siginfo_note (Elf *core, GElf_Word
descsz, GElf_Off desc_pos)
if (si_code > 0)
switch (si_signo)
{
- case SIGILL:
- case SIGFPE:
- case SIGSEGV:
- case SIGBUS:
+ case CORE_SIGILL:
+ case CORE_SIGFPE:
+ case CORE_SIGSEGV:
+ case CORE_SIGBUS:
{
uint64_t addr;
if (! buf_read_ulong (core, &ptr, end, &addr))
@@ -9258,7 +9273,7 @@ handle_siginfo_note (Elf *core, GElf_Word descsz,
GElf_Off desc_pos)
default:
;
}
- else if (si_code == SI_USER)
+ else if (si_code == CORE_SI_USER)
{
int pid, uid;
if (! buf_read_int (core, &ptr, end, &pid)
--
2.8.1.windows.1
Re: don't run elfutils as root in ABRT
Hi Adam, On Fri, 2017-05-05 at 18:25 +0200, Adam Šulc wrote: > I work on ABRT improvement in order to increase security related to > core backtrace generating using elfutils library. > Here is a short description of my problem: > > Goal is to not call base code in elfutils and gdb functions under root. > If you are more interested you can read more there: > https://github.com/abrt/abrt/issues/890 > > We need root for opening /proc files only. And, depending on system settings, for ptrace attach or other interprocess services like reading memory with process_vm_read. > First, we open these files under root, > then we drop capabilities & privileges and finally, we generate > core_backtrace. If you just drop privileges to the user owning the process you should keep having access. > We have one problem that still persists, we need to pass the opened > /proc/[tid]/mem file to this function: > dwfl_linux_proc_find_elf > Because this function opens the /proc/[tid]/mem file itself, thus it > is hard coded and we cannot pass our /proc/[tid]/mem file pointer: > https://github.com/abrt/satyr/blob/master/lib/core_unwind_elfutils.c#L246 > So we dont know how to pass the opened file to this function. > > Do you have any idea how to pass the open file descriptor into the > function? Or what is the best way how to achieve this? You cannot easily unless you write your own Dwfl_Callbacks.find_elf handler. But as long as you only drop privileges to the user owning the process you should be able to open that file. Note that this code path should only be called if the ELF module couldn't be found on the file system. In that case it will try to slurp it from the process memory. Does that fallback path not work as intended for your setup? Cheers, Mark
Re: dwfl_module_addrdie fails for binaries built with clang++
On Sat, 2017-05-06 at 13:30 +0200, Milian Wolff wrote:
> On Freitag, 5. Mai 2017 15:06:48 CEST Mark Wielaard wrote:
> > On Thu, 2017-05-04 at 18:05 +0200, Milian Wolff wrote:
> > > I noticed that elfutils fails to handle clang binaries when we want to
> > > find a DIE for a certain address. I.e. dwfl_module_addrdie returns
> > > nullptr, and eu- addr2line fails to resolve inlined frames.
> > >
> > > To reproduce this:
> > >[...]
> > >
> > > This also affects us in our perfparser. Not being able to find a cudie
> > > means not finding inlined frames nor file/line mappings, which is quite a
> > > set-back.
> > >
> > > I have noticed that backward-cpp contains a (partially) work-around for
> > > this:
> > >
> > > https://github.com/bombela/backward-cpp/blob/master/backward.hpp#L1216
> >
> > O urgh how utterly broken (not backward-cpp, but the bogus DWARF clang
> > generates). As that comment says:
> >
> > // Sadly clang does not generate the section .debug_aranges,
> > thus
> > // dwfl_module_addrdie will fail early. Clang doesn't either set
> > // the lowpc/highpc/range info for every compilation unit.
> > //
> > // So in order to save the world:
> > // for every compilation unit, we will iterate over every single
> > // DIEs. Normally functions should have a lowpc/highpc/range, which
> > // we will use to infer the compilation unit.
> >
> > // note that this is probably badly inefficient.
> >
> > And indeed having to scan through every CU to find a matching function
> > DIE is badly inefficient :{
>
> But this code comment is relatively old. Are we sure it's really still the
> case?
If you were able to replicate it then yes.
> > > Is this the right approach and also what the non-eu addr2line does? If so,
> > > can that be added upstream too, such that dwfl_module_addrdie can be
> > > relied on?
> > >
> > > I've seen it on clang 3.6, 4 and 5. Neither passing -g3 nor
> > > -gdwarf-aranges
> > > helps.
> >
> > Thanks for reporting this. I think this might be the same issue seen
> > here: https://sourceware.org/bugzilla/show_bug.cgi?id=21247
> > ... or at least it seems related. The function/address not found in that
> > case also comes from a CU generated by clang. It does have a lowpc and
> > ranges, but the lowpc looks bogus (zero) and the ranges don't seem to
> > cover the function in question. So it seems even worse than your example
> > where there are no lowpc/ranges. We cannot even trust them if they are
> > there. Sigh.
>
> So the situation is different from the comment in backward-cpp...
Only in how the lowpc/ranges were broken. The core issue is that we
cannot rely on the lowpc/ranges (and aranges) being correct for a CU. We
assume the DWARF producer doesn't really feed us garbage, but apparently
clang does :{
> > I have to think about how to handle this. We clearly need something that
> > just ignores the lowpc/highpc/ranges on CUs and parses every CU till the
> > function/address DIE is found to know which CU and line_table to use.
> > But that is so inefficient that I don't want to do that by default.
>
> So, if this is really that bad - what are the binutils doing - does anyone
> know?
They scan every CU just in case. Which is terrible for performance. Just
compare binutils addr2line vs elfutils eu-addr2line on a large binary.
e.g. on my local machine (best of 3):
$ time eu-addr2line -e /usr/lib64/firefox/libxul.so 0x0157a892
/usr/src/debug/firefox-52.1.0/firefox-52.1.0esr/objdir/dom/bindings/ScrollViewChangeEventBinding.cpp:541
real0m0.067s
user0m0.050s
sys 0m0.017s
$ time addr2line -e /usr/lib64/firefox/libxul.so 0x0157a892
/usr/src/debug/firefox-52.1.0/firefox-52.1.0esr/objdir/dom/bindings/ScrollViewChangeEventBinding.cpp:541
real0m25.984s
user0m20.847s
sys 0m4.193s
So we definitely don't want to do what binutils does by default.
Note that the worst case is an address that doesn't match against any
function (e.g. what you might get if an unwind goes wrong). Currently
that is the cheapest case (not covered by any CU, so done). But if we
cannot rely on which addresses are covered by which CU then we have to
scan all of them just to make sure there really isn't a subroutine
description in there that does cover the address. I want to prevent us
having to do that "just in case" and only if we (or the user) knows the
DWARF might come from a bad producer. So I am pondering whether we
should add something like -b, --bad, as command line argument for things
like eu-addr2line, eu-stack, to indicate that we need some workarounds
for bad DWARF. Which then would call something like dwarf_force_aranges
() or something which would setup an aranges table created by explicit
scanning of all CUs.
> Also, if it's really against all your expectations, shouldn't we report
> this upstream at clang and ask for input there? I can't believe they
> knowingly
> break their generated code in
Problems with dwarf-getmacros test
Hi, I frequently get failures from the run-dwarf-getmacros.sh test, on testfile-macros:0xb. The test repeatedly outputs "(null)" instead of the actual macros and then runs into the assert at dwarf-getmacros.c:50. The failure is very nondeterministic, though. I haven't found a reliable way to trigger it. Further examination reveals that the __libdw_in_section check in READ_AND_RELOCATE (libdwP.h:656), when called from __libdw_read_offset seems to be bogus. The "return -1" in there is what produces the null results and ultimately the assert. Experiments show that the address is frequently not in the section we're checking there, but still valid. Just dropping the check makes the test succeed. I'm currently at a loss about why this happens. One thing that strikes me was that the additional dbg_ret mechamism was added in 2012 with commit 775375e3, but the check in READ_AND_RELOCATE was not adapted then. However, the address is also not necessarily in dbg_ret at that point. Checking dbg_ret in addition to dbg still fails sometimes, and also that wouldn't explain the nondeterminism. Ulf
