[SATYR PATCHv2 1/9] Refactor optimized GDB thread generation

Martin Milata mmilata at redhat.com
Tue May 28 17:33:38 UTC 2013


- make the optimization routine accept thread instead of a stacktrace
- add function that takes a stacktrace and return formatted optimized
  stacktrace as a string
- expose those in python

Signed-off-by: Martin Milata <mmilata at redhat.com>
---
 lib/gdb_frame.c            |  2 --
 lib/gdb_stacktrace.c       | 64 +++++++++-------------------------------------
 lib/gdb_stacktrace.h       | 20 +++++++--------
 lib/gdb_thread.c           | 64 +++++++++++++++++++++++++++++++++++++++++++++-
 lib/gdb_thread.h           | 15 +++++++++++
 python/py_gdb_stacktrace.c | 33 ++++++++++--------------
 python/py_gdb_stacktrace.h |  2 +-
 tests/python/gdb.py        | 11 ++++++++
 8 files changed, 125 insertions(+), 86 deletions(-)

diff --git a/lib/gdb_frame.c b/lib/gdb_frame.c
index b80cee4..70defeb 100644
--- a/lib/gdb_frame.c
+++ b/lib/gdb_frame.c
@@ -243,8 +243,6 @@ sr_gdb_frame_append_to_str(struct sr_gdb_frame *frame,
 
     if (frame->signal_handler_called)
         sr_strbuf_append_str(str, " <signal handler called>");
-
-    sr_strbuf_append_str(str, "\n");
 }
 
 /**
diff --git a/lib/gdb_stacktrace.c b/lib/gdb_stacktrace.c
index 8061481..7dfe37a 100644
--- a/lib/gdb_stacktrace.c
+++ b/lib/gdb_stacktrace.c
@@ -291,6 +291,7 @@ sr_gdb_stacktrace_to_text(struct sr_gdb_stacktrace *stacktrace, bool verbose)
     {
         sr_strbuf_append_str(str, "Crash frame: ");
         sr_gdb_frame_append_to_str(stacktrace->crash, str, verbose);
+        sr_strbuf_append_char(str, '\n');
     }
 
     struct sr_gdb_thread *thread = stacktrace->threads;
@@ -467,44 +468,17 @@ sr_gdb_stacktrace_set_libnames(struct sr_gdb_stacktrace *stacktrace)
     struct sr_gdb_thread *thread = stacktrace->threads;
     while (thread)
     {
-        struct sr_gdb_frame *frame = thread->frames;
-        while (frame)
-        {
-            struct sr_gdb_sharedlib *lib = sr_gdb_sharedlib_find_address(stacktrace->libs,
-                                                                         frame->address);
-            if (lib)
-            {
-                char *s1, *s2;
-
-                /* Strip directory and version after the .so suffix. */
-                s1 = strrchr(lib->soname, '/');
-                if (!s1)
-                    s1 = lib->soname;
-                else
-                    s1++;
-                s2 = strstr(s1, ".so");
-                if (!s2)
-                    s2 = s1 + strlen(s1);
-                else
-                    s2 += strlen(".so");
-
-                if (frame->library_name)
-                    free(frame->library_name);
-                frame->library_name = sr_strndup(s1, s2 - s1);
-            }
-            frame = frame->next;
-        }
+        sr_gdb_thread_set_libnames(thread, stacktrace->libs);
         thread = thread->next;
     }
 }
 
-struct sr_gdb_thread *
-sr_gdb_stacktrace_get_optimized_thread(struct sr_gdb_stacktrace *stacktrace,
-                                       int max_frames)
+char *
+sr_gdb_stacktrace_to_short_text(struct sr_gdb_stacktrace *stacktrace,
+                                int max_frames)
 {
     struct sr_gdb_thread *crash_thread;
 
-    stacktrace = sr_gdb_stacktrace_dup(stacktrace);
     crash_thread = sr_gdb_stacktrace_find_crash_thread(stacktrace);
 
     if (!crash_thread)
@@ -513,26 +487,12 @@ sr_gdb_stacktrace_get_optimized_thread(struct sr_gdb_stacktrace *stacktrace,
         return NULL;
     }
 
-    sr_gdb_stacktrace_remove_threads_except_one(stacktrace, crash_thread);
-    sr_gdb_stacktrace_set_libnames(stacktrace);
-    sr_normalize_gdb_thread(crash_thread);
-    sr_gdb_normalize_optimize_thread(crash_thread);
-
-    /* Remove frames with no function name (i.e. signal handlers). */
-    struct sr_gdb_frame *frame = crash_thread->frames, *frame_next;
-    while (frame)
-    {
-        frame_next = frame->next;
-        if (!frame->function_name)
-            sr_gdb_thread_remove_frame(crash_thread, frame);
-        frame = frame_next;
-    }
-
-    if (max_frames > 0)
-        sr_gdb_thread_remove_frames_below_n(crash_thread, max_frames);
-
-    crash_thread = sr_gdb_thread_dup(crash_thread, false);
-    sr_gdb_stacktrace_free(stacktrace);
+    struct sr_gdb_thread *optimized_thread
+        = sr_gdb_thread_get_optimized(crash_thread, stacktrace->libs,
+                                      max_frames);
+    struct sr_strbuf *strbuf = sr_strbuf_new();
+    sr_gdb_thread_append_to_str(optimized_thread, strbuf, true);
 
-    return crash_thread;
+    sr_gdb_thread_free(optimized_thread);
+    return sr_strbuf_free_nobuf(strbuf);
 }
diff --git a/lib/gdb_stacktrace.h b/lib/gdb_stacktrace.h
index 8dfeec6..889fad4 100644
--- a/lib/gdb_stacktrace.h
+++ b/lib/gdb_stacktrace.h
@@ -290,25 +290,23 @@ sr_gdb_stacktrace_parse_header(const char **input,
 void
 sr_gdb_stacktrace_set_libnames(struct sr_gdb_stacktrace *stacktrace);
 
-// TODO: move somewhere else and refactor
 /**
- * Return crash thread optimized for comparison. It's normalized, with
- * library names set and functions without names (signal handlers) are
- * removed.
+ * Return short text representation of the crash thread.  The trace is
+ * normalized and functions without names (signal handlers) are removed.
  * @param stacktrace
  * It must be non-NULL pointer. It's not modified by calling this
  * function.
  * @param max_frames
  * The maximum number of frames in the returned crash thread.
- * Superfluous frames are removed from the returned thread.
+ * Superfluous frames are not included in the output.
  * @returns
- * A newly allocated thread structure or NULL. NULL is returned when the
- * crashing thread could not be found. The returned structure should be
- * released by sr_gdb_thread_free() by the caller.
+ * Brief text representation of the crash thread suitable for being part of a
+ * bugzilla comment. The string is allocated by the function and must be freed
+ * using the free() function.
  */
-struct sr_gdb_thread *
-sr_gdb_stacktrace_get_optimized_thread(struct sr_gdb_stacktrace *stacktrace,
-                                       int max_frames);
+char *
+sr_gdb_stacktrace_to_short_text(struct sr_gdb_stacktrace *stacktrace,
+                                int max_frames);
 
 #ifdef __cplusplus
 }
diff --git a/lib/gdb_thread.c b/lib/gdb_thread.c
index 1ab6d38..1cd3c60 100644
--- a/lib/gdb_thread.c
+++ b/lib/gdb_thread.c
@@ -19,6 +19,8 @@
 */
 #include "gdb_thread.h"
 #include "gdb_frame.h"
+#include "gdb_sharedlib.h"
+#include "normalize.h"
 #include "location.h"
 #include "utils.h"
 #include "strbuf.h"
@@ -258,7 +260,7 @@ sr_gdb_thread_remove_frames_below_n(struct sr_gdb_thread *thread,
 void
 sr_gdb_thread_append_to_str(struct sr_gdb_thread *thread,
                             struct sr_strbuf *dest,
-                             bool verbose)
+                            bool verbose)
 {
     int frame_count = sr_gdb_thread_get_frame_count(thread);
     if (verbose)
@@ -274,6 +276,7 @@ sr_gdb_thread_append_to_str(struct sr_gdb_thread *thread,
     while (frame)
     {
         sr_gdb_frame_append_to_str(frame, dest, verbose);
+        sr_strbuf_append_char(dest, '\n');
         frame = frame->next;
     }
 }
@@ -478,3 +481,62 @@ sr_gdb_thread_format_funs(struct sr_gdb_thread *thread)
 
     return sr_strbuf_free_nobuf(buf);
 }
+
+void
+sr_gdb_thread_set_libnames(struct sr_gdb_thread *thread, struct sr_gdb_sharedlib *libs)
+{
+    struct sr_gdb_frame *frame = thread->frames;
+    while (frame)
+    {
+        struct sr_gdb_sharedlib *lib = sr_gdb_sharedlib_find_address(libs,
+                                                                     frame->address);
+        if (lib)
+        {
+            char *s1, *s2;
+
+            /* Strip directory and version after the .so suffix. */
+            s1 = strrchr(lib->soname, '/');
+            if (!s1)
+                s1 = lib->soname;
+            else
+                s1++;
+            s2 = strstr(s1, ".so");
+            if (!s2)
+                s2 = s1 + strlen(s1);
+            else
+                s2 += strlen(".so");
+
+            if (frame->library_name)
+                free(frame->library_name);
+            frame->library_name = sr_strndup(s1, s2 - s1);
+        }
+        frame = frame->next;
+    }
+}
+
+struct sr_gdb_thread *
+sr_gdb_thread_get_optimized(struct sr_gdb_thread *thread,
+                            struct sr_gdb_sharedlib *libs, int max_frames)
+{
+    thread = sr_gdb_thread_dup(thread, false);
+
+    if (libs)
+        sr_gdb_thread_set_libnames(thread, libs);
+    sr_normalize_gdb_thread(thread);
+    sr_gdb_normalize_optimize_thread(thread);
+
+    /* Remove frames with no function name (i.e. signal handlers). */
+    struct sr_gdb_frame *frame = thread->frames, *frame_next;
+    while (frame)
+    {
+        frame_next = frame->next;
+        if (!frame->function_name)
+            sr_gdb_thread_remove_frame(thread, frame);
+        frame = frame_next;
+    }
+
+    if (max_frames > 0)
+        sr_gdb_thread_remove_frames_below_n(thread, max_frames);
+
+    return thread;
+}
diff --git a/lib/gdb_thread.h b/lib/gdb_thread.h
index f1afde1..70ae7d3 100644
--- a/lib/gdb_thread.h
+++ b/lib/gdb_thread.h
@@ -35,6 +35,7 @@ extern "C" {
 struct sr_gdb_frame;
 struct sr_strbuf;
 struct sr_location;
+struct sr_gdb_sharedlib;
 
 /**
  * @brief A thread of execution of a GDB-produced stack trace.
@@ -244,6 +245,20 @@ sr_gdb_thread_parse_funs(const char *input);
 char *
 sr_gdb_thread_format_funs(struct sr_gdb_thread *thread);
 
+/**
+ * Set library names in all frames in the thread according to the
+ * the sharedlib data.
+ */
+void
+sr_gdb_thread_set_libnames(struct sr_gdb_thread *thread,
+                           struct sr_gdb_sharedlib *libs);
+
+/**
+ * Return copy of the thread optimized for comparison.
+ */
+struct sr_gdb_thread *
+sr_gdb_thread_get_optimized(struct sr_gdb_thread *thread,
+                            struct sr_gdb_sharedlib *libs, int max_frames);
 #ifdef __cplusplus
 }
 #endif
diff --git a/python/py_gdb_stacktrace.c b/python/py_gdb_stacktrace.c
index 4eaea83..4a269c4 100644
--- a/python/py_gdb_stacktrace.c
+++ b/python/py_gdb_stacktrace.c
@@ -57,8 +57,9 @@
 #define b_normalize_doc "Usage: stacktrace.normalize()\n" \
                         "Normalizes all threads in the stacktrace."
 
-#define b_get_optimized_thread_doc "Usage: stacktrace.get_optimized_thread(max_frames)\n" \
-                        "Returns thread optimized for comparison."
+#define b_to_short_text "Usage: stacktrace.to_short_text([max_frames])\n" \
+                        "Returns short text representation of the crash thread. If max_frames is\n" \
+                        "specified, the result includes only that much topmost frames.\n"
 
 #define b_crashframe_doc (char *)"Readonly. By default the field contains None. After " \
                          "calling the find_crash_frame method, a reference to " \
@@ -88,7 +89,7 @@ gdb_stacktrace_methods[] =
     { "find_address",         sr_py_gdb_stacktrace_find_address,         METH_VARARGS, b_find_address_doc         },
     { "set_libnames",         sr_py_gdb_stacktrace_set_libnames,         METH_NOARGS,  b_set_libnames_doc         },
     { "normalize",            sr_py_gdb_stacktrace_normalize,            METH_NOARGS,  b_normalize_doc            },
-    { "get_optimized_thread", sr_py_gdb_stacktrace_get_optimized_thread, METH_VARARGS, b_get_optimized_thread_doc },
+    { "to_short_text",        sr_py_gdb_stacktrace_to_short_text,        METH_VARARGS, b_to_short_text            },
     { NULL },
 };
 
@@ -681,35 +682,29 @@ sr_py_gdb_stacktrace_normalize(PyObject *self, PyObject *args)
 }
 
 PyObject *
-sr_py_gdb_stacktrace_get_optimized_thread(PyObject *self, PyObject *args)
+sr_py_gdb_stacktrace_to_short_text(PyObject *self, PyObject *args)
 {
     struct sr_py_gdb_stacktrace *this = (struct sr_py_gdb_stacktrace*)self;
     if (stacktrace_prepare_linked_list(this) < 0)
         return NULL;
 
-    int max_frames;
-    if (!PyArg_ParseTuple(args, "i", &max_frames))
+    int max_frames = 0;
+    if (!PyArg_ParseTuple(args, "|i", &max_frames))
         return NULL;
 
-    struct sr_gdb_thread *thread =
-        sr_gdb_stacktrace_get_optimized_thread(this->stacktrace, max_frames);
-    if (!thread)
+    char *text =
+        sr_gdb_stacktrace_to_short_text(this->stacktrace, max_frames);
+    if (!text)
     {
         PyErr_SetString(PyExc_LookupError, "Crash thread not found");
         return NULL;
     }
 
-    struct sr_py_gdb_thread *result = (struct sr_py_gdb_thread*)
-        PyObject_New(struct sr_py_gdb_thread, &sr_py_gdb_thread_type);
-
-    if (!result)
-        return PyErr_NoMemory();
-
-    result->thread = thread;
-    result->frames = frame_linked_list_to_python_list(result->thread);
-
     if (stacktrace_rebuild_thread_python_list(this) < 0)
         return NULL;
 
-    return (PyObject*)result;
+    PyObject *result = PyString_FromString(text);
+
+    free(text);
+    return result;
 }
diff --git a/python/py_gdb_stacktrace.h b/python/py_gdb_stacktrace.h
index 4fce6c1..1581958 100644
--- a/python/py_gdb_stacktrace.h
+++ b/python/py_gdb_stacktrace.h
@@ -73,7 +73,7 @@ PyObject *sr_py_gdb_stacktrace_get_duplication_hash(PyObject *self, PyObject *ar
 PyObject *sr_py_gdb_stacktrace_find_address(PyObject *self, PyObject *args);
 PyObject *sr_py_gdb_stacktrace_set_libnames(PyObject *self, PyObject *args);
 PyObject *sr_py_gdb_stacktrace_normalize(PyObject *self, PyObject *args);
-PyObject *sr_py_gdb_stacktrace_get_optimized_thread(PyObject *self, PyObject *args);
+PyObject * sr_py_gdb_stacktrace_to_short_text(PyObject *self, PyObject *args);
 
 #ifdef __cplusplus
 }
diff --git a/tests/python/gdb.py b/tests/python/gdb.py
index ea04776..e192a22 100755
--- a/tests/python/gdb.py
+++ b/tests/python/gdb.py
@@ -13,6 +13,14 @@ except ImportError:
 path = '../gdb_stacktraces/rhbz-803600'
 threads_expected = 2
 frames_expected = 227
+expected_short_text = '''Thread no. 1 (5 frames)
+ #0 validate_row at gtktreeview.c
+ #1 validate_visible_area at gtktreeview.c
+ #2 gtk_tree_view_bin_expose at gtktreeview.c
+ #3 gtk_tree_view_expose at gtktreeview.c
+ #4 _gtk_marshal_BOOLEAN__BOXED at gtkmarshalers.c
+'''
+
 
 if not os.path.isfile(path):
     path = '../' + path
@@ -58,6 +66,9 @@ class TestGdbStacktrace(BindingsTestCase):
         out = str(self.trace)
         self.assertTrue(('Stacktrace with %d threads' % threads_expected) in out)
 
+    def test_to_short_text(self):
+        self.assertEqual(self.trace.to_short_text(5), expected_short_text)
+
 class TestGdbThread(BindingsTestCase):
     def setUp(self):
         self.thread = satyr.GdbStacktrace(contents).threads[0]
-- 
1.7.11.7



More information about the Crash-catcher mailing list