- 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 <[email protected]> --- 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
