[SATYR PATCH 1/3] Fix potentially undefined return value

[Martin Milata]

Signed-off-by: Martin Milata <mmilata(a)redhat.com&gt;
---
 lib/core_unwind_libunwind.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/lib/core_unwind_libunwind.c b/lib/core_unwind_libunwind.c
index 1c014a9..06734a3 100644
--- a/lib/core_unwind_libunwind.c
+++ b/lib/core_unwind_libunwind.c
@@ -434,7 +434,7 @@ sr_parse_coredump(const char *core_file,
                    const char *exe_file,
                    char **error_msg)
 {
-    struct sr_core_thread *trace = NULL;
+    struct sr_core_stacktrace *stacktrace = NULL;
 
     /* Initialize error_msg to 'no error'. */
     if (error_msg)
@@ -472,12 +472,12 @@ sr_parse_coredump(const char *core_file,
         }
     }
 
-    struct sr_core_stacktrace *stacktrace = sr_core_stacktrace_new();
+    stacktrace = sr_core_stacktrace_new();
 
     int tnum, nthreads = _UCD_get_num_threads(ui);
     for (tnum = 0; tnum < nthreads; ++tnum)
     {
-        trace = unwind_thread(ui, as, ch->dwfl, tnum, error_msg);
+        struct sr_core_thread *trace = unwind_thread(ui, as, ch->dwfl, tnum,
error_msg);
         if (trace)
         {
             stacktrace->threads = sr_core_thread_append(stacktrace->threads,
trace);
-- 
1.7.11.7

[SATYR PATCH 2/3] Find signal number when unwinding with libunwind

[Martin Milata]

Signed-off-by: Martin Milata <mmilata(a)redhat.com&gt;
---
 lib/core_stacktrace.h       |  2 +-
 lib/core_unwind_libunwind.c | 20 ++++++++++++++++++--
 2 files changed, 19 insertions(+), 3 deletions(-)

diff --git a/lib/core_stacktrace.h b/lib/core_stacktrace.h
index 0552475..ac0804d 100644
--- a/lib/core_stacktrace.h
+++ b/lib/core_stacktrace.h
@@ -41,7 +41,7 @@ struct sr_json_value;
 struct sr_core_stacktrace
 {
     /** Signal number. */
-    uint8_t signal;
+    uint16_t signal;
 
     char *executable;
 
diff --git a/lib/core_unwind_libunwind.c b/lib/core_unwind_libunwind.c
index 06734a3..29805da 100644
--- a/lib/core_unwind_libunwind.c
+++ b/lib/core_unwind_libunwind.c
@@ -429,6 +429,23 @@ unwind_thread(struct UCD_info *ui,
     return thread;
 }
 
+static int
+get_signal_number(struct UCD_info *ui)
+{
+    int tnum, nthreads = _UCD_get_num_threads(ui);
+    for (tnum = 0; tnum < nthreads; ++tnum)
+    {
+        _UCD_select_thread(ui, tnum);
+        int signo = _UCD_get_cursig(ui);
+
+        /* Return first nonzero signal, gdb/bfd seem to work this way. */
+        if (signo)
+            return signo;
+    }
+
+    return 0;
+}
+
 struct sr_core_stacktrace *
 sr_parse_coredump(const char *core_file,
                    const char *exe_file,
@@ -491,8 +508,7 @@ sr_parse_coredump(const char *core_file,
     }
 
     stacktrace->executable = realpath(exe_file, NULL);
-    /* FIXME: determine signal */
-    stacktrace->signal = 0;
+    stacktrace->signal = get_signal_number(ui);
     /* FIXME: is this the best we can do? */
     stacktrace->crash_thread = stacktrace->threads;
 
-- 
1.7.11.7

[SATYR PATCH 3/3] Find signal number when unwinding with elfutils

[Martin Milata]

Closes #52 together with the previous patch.

Signed-off-by: Martin Milata <mmilata(a)redhat.com&gt;
---
 lib/core_unwind_elfutils.c | 90 ++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 88 insertions(+), 2 deletions(-)

diff --git a/lib/core_unwind_elfutils.c b/lib/core_unwind_elfutils.c
index a3e8d03..bb8a5b7 100644
--- a/lib/core_unwind_elfutils.c
+++ b/lib/core_unwind_elfutils.c
@@ -39,6 +39,7 @@
 #include <libelf.h>
 #include <gelf.h>
 #include <elfutils/libdwfl.h>
+#include <sys/procfs.h> /* struct elf_prstatus */
 
 /* Error/warning reporting macros. Allows the error reporting code to be less
  * verbose with the restrictions that:
@@ -332,12 +333,96 @@ unwind_thread(Dwfl *dwfl, Dwfl_Frame_State *state, char
**error_msg)
     return thread;
 }
 
+static short
+get_signal(Elf *e, const char *elf_file)
+{
+    const char NOTE_CORE[] = "CORE";
+
+    size_t nphdr;
+    if (elf_getphdrnum(e, &nphdr) != 0)
+    {
+        warn_elf("elf_getphdrnum");
+        return 0;
+    }
+
+    /* Go through phdrs, look for prstatus note */
+    int i;
+    for (i = 0; i < nphdr; i++)
+    {
+        GElf_Phdr phdr;
+        if (gelf_getphdr(e, i, &phdr) != &phdr)
+        {
+            warn_elf("gelf_getphdr");
+            continue;
+        }
+
+        if (phdr.p_type != PT_NOTE)
+        {
+            continue;
+        }
+
+        Elf_Data *data, *name_data, *desc_data;
+        GElf_Nhdr nhdr;
+        size_t note_offset = 0;
+        size_t name_offset, desc_offset;
+        /* Elf_Data buffers are freed when elf_end is called. */
+        data = elf_getdata_rawchunk(e, phdr.p_offset, phdr.p_filesz,
+                                    ELF_T_NHDR);
+        if (!data)
+        {
+            warn_elf("elf_getdata_rawchunk");
+            continue;
+        }
+
+        while ((note_offset = gelf_getnote(data, note_offset, &nhdr,
+                                           &name_offset, &desc_offset)) != 0)
+        {
+            /*
+            printf("Note: type:%x name:%x+%d desc:%x+%d\n", nhdr.n_type,
+                   name_offset, nhdr.n_namesz, desc_offset, nhdr.n_descsz);
+            */
+
+            if (nhdr.n_type != NT_PRSTATUS
+                || nhdr.n_namesz < sizeof(NOTE_CORE))
+                continue;
+
+            name_data = elf_getdata_rawchunk(e, phdr.p_offset + name_offset,
+                                             nhdr.n_namesz, ELF_T_BYTE);
+            desc_data = elf_getdata_rawchunk(e, phdr.p_offset + desc_offset,
+                                             nhdr.n_descsz, ELF_T_BYTE);
+            if (!(name_data && desc_data))
+                continue;
+
+            if (name_data->d_size < sizeof(NOTE_CORE))
+                continue;
+
+            if (strcmp(NOTE_CORE, name_data->d_buf))
+                continue;
+
+            if (desc_data->d_size != sizeof(struct elf_prstatus))
+            {
+                warn("PRSTATUS core note of size %d found, expected size: %d",
+                    desc_data->d_size, sizeof(struct elf_prstatus));
+                continue;
+            }
+
+            struct elf_prstatus *prstatus = (struct elf_prstatus*)desc_data->d_buf;
+            short signal = prstatus->pr_cursig;
+            if (signal)
+                return signal;
+        }
+    }
+
+    return 0;
+}
+
 struct sr_core_stacktrace *
 sr_parse_coredump(const char *core_file,
                    const char *exe_file,
                    char **error_msg)
 {
     struct sr_core_stacktrace *stacktrace = NULL;
+    short signal = 0;
 
     /* Initialize error_msg to 'no error'. */
     if (error_msg)
@@ -374,6 +459,8 @@ sr_parse_coredump(const char *core_file,
         state = dwfl_frame_thread_next(state);
     } while (state);
 
+    signal = get_signal(ch->eh, core_file);
+
 fail_destroy_trace:
     if (*error_msg)
     {
@@ -384,8 +471,7 @@ fail_destroy_handle:
     core_handle_free(ch);
 
     stacktrace->executable = sr_strdup(executable_file);
-    /* FIXME: determine signal */
-    stacktrace->signal = 0;
+    stacktrace->signal = signal;
     /* FIXME: is this the best we can do? */
     stacktrace->crash_thread = stacktrace->threads;
     return stacktrace;
-- 
1.7.11.7

[SATYR PATCH 3/3] Find signal number when unwinding with elfutils

[Jakub Filak]

On Monday 22 of April 2013 18:24:11 Martin Milata wrote:
>  +            if (desc_data->d_size != sizeof(struct
> elf_prstatus))
>  +            {
>  +                warn("PRSTATUS core note of size %d found, expected size:
>  %d", +                    desc_data->d_size, sizeof(struct elf_prstatus));
I would say %z should be used instead of %d because return value of sizefo() 
is not signed.

How about to add __sr_printf(1,2) to warn() declaration?

>  +                continue;
>  +            }

[SATYR PATCHv2] Find signal number when unwinding with elfutils

[Martin Milata]

Closes #52 together with the libunwind signal patch.

Signed-off-by: Martin Milata <mmilata(a)redhat.com&gt;
---
 lib/core_unwind_elfutils.c | 118 ++++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 105 insertions(+), 13 deletions(-)

diff --git a/lib/core_unwind_elfutils.c b/lib/core_unwind_elfutils.c
index a3e8d03..0fb9fc0 100644
--- a/lib/core_unwind_elfutils.c
+++ b/lib/core_unwind_elfutils.c
@@ -39,6 +39,7 @@
 #include <libelf.h>
 #include <gelf.h>
 #include <elfutils/libdwfl.h>
+#include <sys/procfs.h> /* struct elf_prstatus */
 
 /* Error/warning reporting macros. Allows the error reporting code to be less
  * verbose with the restrictions that:
@@ -68,6 +69,9 @@
     } while(0)
 
 static void
+_set_error(char **error_msg, const char *fmt, ...) __sr_printf(2, 3);
+
+static void
 _set_error(char **error_msg, const char *fmt, ...)
 {
     va_list ap;
@@ -80,17 +84,8 @@ _set_error(char **error_msg, const char *fmt, ...)
     va_end(ap);
 }
 
-struct core_handle
-{
-    int fd;
-    Elf *eh;
-    Dwfl *dwfl;
-    Dwfl_Callbacks cb;
-};
-
-/* FIXME: is there another way to pass the executable name to the find_elf
- * callback? */
-const char *executable_file = NULL;
+static void
+warn(const char *fmt, ...) __sr_printf(1, 2);
 
 static void
 warn(const char *fmt, ...)
@@ -107,6 +102,18 @@ warn(const char *fmt, ...)
 
 }
 
+/* FIXME: is there another way to pass the executable name to the find_elf
+ * callback? */
+const char *executable_file = NULL;
+
+struct core_handle
+{
+    int fd;
+    Elf *eh;
+    Dwfl *dwfl;
+    Dwfl_Callbacks cb;
+};
+
 static void
 core_handle_free(struct core_handle *ch)
 {
@@ -332,12 +339,96 @@ unwind_thread(Dwfl *dwfl, Dwfl_Frame_State *state, char
**error_msg)
     return thread;
 }
 
+static short
+get_signal_number(Elf *e, const char *elf_file)
+{
+    const char NOTE_CORE[] = "CORE";
+
+    size_t nphdr;
+    if (elf_getphdrnum(e, &nphdr) != 0)
+    {
+        warn_elf("elf_getphdrnum");
+        return 0;
+    }
+
+    /* Go through phdrs, look for prstatus note */
+    int i;
+    for (i = 0; i < nphdr; i++)
+    {
+        GElf_Phdr phdr;
+        if (gelf_getphdr(e, i, &phdr) != &phdr)
+        {
+            warn_elf("gelf_getphdr");
+            continue;
+        }
+
+        if (phdr.p_type != PT_NOTE)
+        {
+            continue;
+        }
+
+        Elf_Data *data, *name_data, *desc_data;
+        GElf_Nhdr nhdr;
+        size_t note_offset = 0;
+        size_t name_offset, desc_offset;
+        /* Elf_Data buffers are freed when elf_end is called. */
+        data = elf_getdata_rawchunk(e, phdr.p_offset, phdr.p_filesz,
+                                    ELF_T_NHDR);
+        if (!data)
+        {
+            warn_elf("elf_getdata_rawchunk");
+            continue;
+        }
+
+        while ((note_offset = gelf_getnote(data, note_offset, &nhdr,
+                                           &name_offset, &desc_offset)) != 0)
+        {
+            /*
+            printf("Note: type:%x name:%x+%d desc:%x+%d\n", nhdr.n_type,
+                   name_offset, nhdr.n_namesz, desc_offset, nhdr.n_descsz);
+            */
+
+            if (nhdr.n_type != NT_PRSTATUS
+                || nhdr.n_namesz < sizeof(NOTE_CORE))
+                continue;
+
+            name_data = elf_getdata_rawchunk(e, phdr.p_offset + name_offset,
+                                             nhdr.n_namesz, ELF_T_BYTE);
+            desc_data = elf_getdata_rawchunk(e, phdr.p_offset + desc_offset,
+                                             nhdr.n_descsz, ELF_T_BYTE);
+            if (!(name_data && desc_data))
+                continue;
+
+            if (name_data->d_size < sizeof(NOTE_CORE))
+                continue;
+
+            if (strcmp(NOTE_CORE, name_data->d_buf))
+                continue;
+
+            if (desc_data->d_size != sizeof(struct elf_prstatus))
+            {
+                warn("PRSTATUS core note of size %zu found, expected size:
%zu",
+                    desc_data->d_size, sizeof(struct elf_prstatus));
+                continue;
+            }
+
+            struct elf_prstatus *prstatus = (struct elf_prstatus*)desc_data->d_buf;
+            short signal = prstatus->pr_cursig;
+            if (signal)
+                return signal;
+        }
+    }
+
+    return 0;
+}
+
 struct sr_core_stacktrace *
 sr_parse_coredump(const char *core_file,
                    const char *exe_file,
                    char **error_msg)
 {
     struct sr_core_stacktrace *stacktrace = NULL;
+    short signal = 0;
 
     /* Initialize error_msg to 'no error'. */
     if (error_msg)
@@ -374,6 +465,8 @@ sr_parse_coredump(const char *core_file,
         state = dwfl_frame_thread_next(state);
     } while (state);
 
+    signal = get_signal_number(ch->eh, core_file);
+
 fail_destroy_trace:
     if (*error_msg)
     {
@@ -384,8 +477,7 @@ fail_destroy_handle:
     core_handle_free(ch);
 
     stacktrace->executable = sr_strdup(executable_file);
-    /* FIXME: determine signal */
-    stacktrace->signal = 0;
+    stacktrace->signal = signal;
     /* FIXME: is this the best we can do? */
     stacktrace->crash_thread = stacktrace->threads;
     return stacktrace;
-- 
1.7.11.7