Consider the following two backtraces:
Backtrace 0:
  #0 A crash function

  thread0:
   #0 A
   #1 B
   #2 C
   #3 D

  thread1:
   #0 A
   #1 E
   #2 F
   #3 G

Backtrace 1:
  #0 A crash function

  thread0:
   #0 A
   #1 E
   #2 F
   #3 G

  thread1:
   #0 A
   #1 B
   #2 C
   #3 D


The crash thread in Backtrace 0 is thread 0 and the crash thread in Bactrace 1 
is 
thread 0 as well. But the crash threads are not the same. IMHO the algorithm 
must 
return same threads. What do you think about this?


Regards
Jakub




On Friday, March 22, 2013 07:54:52 PM Martin Milata wrote:
> The current algorithm cannot handle backtraces with multiple threads
> containing the designated crash frame such as [1]. We just pick the
> shortest one with the risk that our guess might be wrong. Fixes #3.
> 
> [1] https://bugzilla.redhat.com/attachment.cgi?id=703269
> 
> Signed-off-by: Martin Milata <mmil...@redhat.com>
> ---
>  lib/backtrace.c | 45 
++++++++++++++++++++++++++++++++++-----------
>  1 file changed, 34 insertions(+), 11 deletions(-)
> 
> diff --git a/lib/backtrace.c b/lib/backtrace.c
> index a871eee..11050f8 100644
> --- a/lib/backtrace.c
> +++ b/lib/backtrace.c
> @@ -117,27 +117,31 @@ btp_backtrace_remove_threads_except_one(struct
> btp_backtrace *backtrace, backtrace->threads = thread;
>  }
> 
> +enum requirement { ONE_MATCHING, ABORT_FUNCTION, SHORTEST_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. + * SHORTEST_MATCHING - If everything else fails, we just
> return the shortest + *                     thread containing the crash
> frame.
>   */
>  static struct btp_thread *
>  btp_backtrace_find_crash_thread_from_crash_frame(struct btp_backtrace
> *backtrace, -                                                 bool
> require_abort) +                                                 enum
> requirement req) {
>      if (btp_debug_parser)
> -        printf("%s(backtrace, %s)\n", __FUNCTION__, require_abort ? "true"
> : "false"); +        printf("%s(backtrace, %s)\n", __FUNCTION__,
> +               (req == ABORT_FUNCTION) ? "true" : "false");
> 
>      assert(backtrace->threads); /* checked by the caller */
>      if (!backtrace->crash || !backtrace->crash->function_name)
>          return NULL;
> 
> +    int result_frames;
>      struct btp_thread *result = NULL;
>      struct btp_thread *thread = backtrace->threads;
>      while (thread)
> @@ -146,7 +150,7 @@ btp_backtrace_find_crash_thread_from_crash_frame(struct
> btp_backtrace *backtrace bool same_name = top_frame &&
>              top_frame->function_name &&
>              0 == strcmp(top_frame->function_name,
> backtrace->crash->function_name); -        bool abort_requirement_satisfied
> = !require_abort ||
> +        bool abort_requirement_satisfied = (req != ABORT_FUNCTION) ||
>              btp_glibc_thread_find_exit_frame(thread);
>          if (btp_debug_parser)
>          {
> @@ -158,13 +162,24 @@
> btp_backtrace_find_crash_thread_from_crash_frame(struct btp_backtrace
> *backtrace
> 
>          if (same_name && abort_requirement_satisfied)
>          {
> +            int current_frames = btp_thread_get_frame_count(thread);
> +
>              if (NULL == result)
> +            {
>                  result = thread;
> -            else
> +                result_frames = current_frames;
> +            }
> +            else if (req != SHORTEST_MATCHING)
>              {
>                  /* Second frame with the same function. Failure. */
>                  return NULL;
>              }
> +            else if (current_frames < result_frames)
> +            {
> +                /* We're just looking for the shortest frame that matches.
> */ +                result = thread;
> +                result_frames = current_frames;

Reply via email to