[SATYR PATCH 1/2] Try harder when looking for crash thread

Martin Milata mmilata at redhat.com
Mon Aug 12 12:19:27 UTC 2013


Backported from btparser 6a1db1e193db4136f2b170b911a946c30001fd98:

  The current algorithm cannot handle backtraces with multiple threads
  containing the designated crash frame such as [1]. We just pick the
  first one (from sorted threads) with the risk that our guess might be
  wrong. Fixes abrt/btparser#3.

  [1] https://bugzilla.redhat.com/attachment.cgi?id=703269

Related to rhbz#994747.

Signed-off-by: Martin Milata <mmilata at redhat.com>
---
 lib/gdb_stacktrace.c | 66 +++++++++++++++++++++++++++++++++++++++++++---------
 1 file changed, 55 insertions(+), 11 deletions(-)

diff --git a/lib/gdb_stacktrace.c b/lib/gdb_stacktrace.c
index 41a4b8d..2147e18 100644
--- a/lib/gdb_stacktrace.c
+++ b/lib/gdb_stacktrace.c
@@ -155,22 +155,51 @@ sr_gdb_stacktrace_remove_threads_except_one(struct sr_gdb_stacktrace *stacktrace
     stacktrace->threads = thread;
 }
 
+/* we do not care about thread numbers and sr_thread_cmp does not allow us not
+ * to compare them */
+static int
+thread_cmp_dont_compare_number(struct sr_gdb_thread *t1, struct sr_gdb_thread *t2)
+{
+    struct sr_gdb_frame *f1 = t1->frames, *f2 = t2->frames;
+    do {
+        if (f1 && !f2)
+            return 1;
+        else if (f2 && !f1)
+            return -1;
+        else if (f1 && f2)
+        {
+            int frames = sr_gdb_frame_cmp(f1, f2, true);
+            if (frames != 0)
+                return frames;
+            f1 = f1->next;
+            f2 = f2->next;
+        }
+    } while (f1 || f2);
+
+    return 0;
+}
+
+enum requirement { ONE_MATCHING, ABORT_FUNCTION, FIRST_MATCHING };
+
 /**
  * Loop through all threads and if a single one contains the crash
  * frame on the top, return it. Otherwise, return NULL.
  *
- * If require_abort is true, it is also required that the thread
- * containing the crash frame contains some known "abort" function. In
- * this case there can be multiple threads with the crash frame on the
- * top, but only one of them might contain the abort function to
- * succeed.
+ * Parameter req controlls the criteria for matching thread:
+ * ONE_MATCHING   - There is only one thread that contains the crash frame.
+ * ABORT_FUNCTION - There may be multiple threads containing the crash frame
+ *                  but only one contains some known "abort" function.
+ * FIRST_MATCHING - If everything else fails, we sort the threads (in order to
+ *                  get deterministic results) and return the first thread
+ *                  containing the crash frame.
  */
 static struct sr_gdb_thread *
 find_crash_thread_from_crash_frame(struct sr_gdb_stacktrace *stacktrace,
-                                   bool require_abort)
+                                   enum requirement req)
 {
     if (sr_debug_parser)
-        printf("%s(stacktrace, %s)\n", __FUNCTION__, require_abort ? "true" : "false");
+        printf("%s(stacktrace, %s)\n", __FUNCTION__,
+               (req == ABORT_FUNCTION) ? "true" : "false");
 
     assert(stacktrace->threads); /* checked by the caller */
     if (!stacktrace->crash || !stacktrace->crash->function_name)
@@ -184,7 +213,7 @@ find_crash_thread_from_crash_frame(struct sr_gdb_stacktrace *stacktrace,
         bool same_name = top_frame &&
             top_frame->function_name &&
             0 == strcmp(top_frame->function_name, stacktrace->crash->function_name);
-        bool abort_requirement_satisfied = !require_abort ||
+        bool abort_requirement_satisfied = (req != ABORT_FUNCTION) ||
             sr_glibc_thread_find_exit_frame(thread);
         if (sr_debug_parser)
         {
@@ -197,12 +226,19 @@ find_crash_thread_from_crash_frame(struct sr_gdb_stacktrace *stacktrace,
         if (same_name && abort_requirement_satisfied)
         {
             if (NULL == result)
+            {
                 result = thread;
-            else
+            }
+            else if (req != FIRST_MATCHING)
             {
                 /* Second frame with the same function. Failure. */
                 return NULL;
             }
+            else if (thread_cmp_dont_compare_number(thread, result) < 0)
+            {
+                /* We're just looking for the first thread that matches. */
+                result = thread;
+            }
         }
 
         thread = thread->next;
@@ -226,7 +262,7 @@ sr_gdb_stacktrace_find_crash_thread(struct sr_gdb_stacktrace *stacktrace)
      * this frame on the top, it is also simple.
      */
     struct sr_gdb_thread *thread;
-    thread = find_crash_thread_from_crash_frame(stacktrace, false);
+    thread = find_crash_thread_from_crash_frame(stacktrace, ONE_MATCHING);
     if (thread)
         return thread;
 
@@ -234,7 +270,15 @@ sr_gdb_stacktrace_find_crash_thread(struct sr_gdb_stacktrace *stacktrace)
      * the crash frame on the top of stack.
      * Try to search for known abort functions.
      */
-    thread = find_crash_thread_from_crash_frame(stacktrace, true);
+    thread = find_crash_thread_from_crash_frame(stacktrace, ABORT_FUNCTION);
+    if (thread)
+        return thread;
+
+    /* There are multiple threads with the same crash frame and none of them
+     * contains known abort function.
+     * Take a guess and return the first one (from sorted threads).
+     */
+    thread = find_crash_thread_from_crash_frame(stacktrace, FIRST_MATCHING);
 
     /* We might want to search a thread with known abort function, and
      * without the crash frame here. However, it hasn't been needed so
-- 
1.8.3.1



More information about the Crash-catcher mailing list