[v2,3/5] libdwfl: add unwind origin diagnostics

Message ID b763a4b7-26f7-4b83-8838-740d6e850287@app.fastmail.com
State Committed
Headers
Series [v2,1/5] src: add eu-stacktrace tool |

Commit Message

Serhei Makarov Oct. 17, 2024, 6:52 p.m. UTC
  Track the method used to unwind each Dwfl_Frame using an enum field
unwound_source; provide access to the field. Then use that in
eu-stacktrace to display the unwind methods used for each process.

This is an important diagnostic to verify how many processes are
adequately covered by the eh_frame unwinder.

* libdw/libdw.map (ELFUTILS_0.192): Add dwfl_frame_unwound_source,
  dwfl_unwound_source_str.
* libdwfl/libdwfl.h (Dwfl_Unwound_Source): New enum.
  (dwfl_frame_unwound_source)
  (dwfl_unwound_source_str): New functions.
* libdwfl/dwfl_frame.c (dwfl_frame_unwound_source)
  (dwfl_unwound_source_str): New functions.
* libdwfl/libdwflP.h: Add INTDECL for dwfl_frame_unwound_source,
  dwfl_unwound_source_str.
  (struct Dwfl_Frame): Add unwound_source field.
* libdwfl/frame_unwind.c (__libdwfl_frame_unwind): Set
  state->unwound_source depending on the unwind method used.
* src/stacktrace.c (struct sysprof_unwind_info): Add last_pid
  field to provide access to the current sample's dwfltab entry.
  (sysprof_unwind_frame_cb): Add unwound_source to the data displayed
  with the show_frames option.
  (sysprof_unwind_cb): Set last_pid when processing a sample.
  (main): Add unwind_source to the data displayed in the final summary
  table.

Signed-off-by: Serhei Makarov <serhei@serhei.io>
---
 libdw/libdw.map        |  2 ++
 libdwfl/dwfl_frame.c   | 31 ++++++++++++++++++++++++++++++-
 libdwfl/frame_unwind.c | 14 +++++++++++---
 libdwfl/libdwfl.h      | 22 +++++++++++++++++++++-
 libdwfl/libdwflP.h     |  5 ++++-
 src/stacktrace.c       | 33 ++++++++++++++++++++++++++++-----
 6 files changed, 96 insertions(+), 11 deletions(-)
  

Comments

Aaron Merey Oct. 17, 2024, 9:02 p.m. UTC | #1
On Thu, Oct 17, 2024 at 2:53 PM Serhei Makarov <serhei@serhei.io> wrote:
>
> Track the method used to unwind each Dwfl_Frame using an enum field
> unwound_source; provide access to the field. Then use that in
> eu-stacktrace to display the unwind methods used for each process.
>
> This is an important diagnostic to verify how many processes are
> adequately covered by the eh_frame unwinder.
>
> * libdw/libdw.map (ELFUTILS_0.192): Add dwfl_frame_unwound_source,
>   dwfl_unwound_source_str.
> * libdwfl/libdwfl.h (Dwfl_Unwound_Source): New enum.
>   (dwfl_frame_unwound_source)
>   (dwfl_unwound_source_str): New functions.
> * libdwfl/dwfl_frame.c (dwfl_frame_unwound_source)
>   (dwfl_unwound_source_str): New functions.
> * libdwfl/libdwflP.h: Add INTDECL for dwfl_frame_unwound_source,
>   dwfl_unwound_source_str.
>   (struct Dwfl_Frame): Add unwound_source field.
> * libdwfl/frame_unwind.c (__libdwfl_frame_unwind): Set
>   state->unwound_source depending on the unwind method used.
> * src/stacktrace.c (struct sysprof_unwind_info): Add last_pid
>   field to provide access to the current sample's dwfltab entry.
>   (sysprof_unwind_frame_cb): Add unwound_source to the data displayed
>   with the show_frames option.
>   (sysprof_unwind_cb): Set last_pid when processing a sample.
>   (main): Add unwind_source to the data displayed in the final summary
>   table.
>
> Signed-off-by: Serhei Makarov <serhei@serhei.io>

LGTM, please merge.

Thanks,
Aaron
  

Patch

diff --git a/libdw/libdw.map b/libdw/libdw.map
index 552588a9..bc53385f 100644
--- a/libdw/libdw.map
+++ b/libdw/libdw.map
@@ -382,4 +382,6 @@  ELFUTILS_0.191 {
 ELFUTILS_0.192 {
   global:
     dwfl_set_sysroot;
+    dwfl_frame_unwound_source;
+    dwfl_unwound_source_str;
 } ELFUTILS_0.191;
diff --git a/libdwfl/dwfl_frame.c b/libdwfl/dwfl_frame.c
index 8af8843f..2e6c6de8 100644
--- a/libdwfl/dwfl_frame.c
+++ b/libdwfl/dwfl_frame.c
@@ -1,5 +1,5 @@ 
 /* Get Dwarf Frame state for target PID or core file.
-   Copyright (C) 2013, 2014 Red Hat, Inc.
+   Copyright (C) 2013, 2014, 2024 Red Hat, Inc.
    This file is part of elfutils.
 
    This file is free software; you can redistribute it and/or modify
@@ -98,6 +98,7 @@  state_alloc (Dwfl_Thread *thread)
   state->signal_frame = false;
   state->initial_frame = true;
   state->pc_state = DWFL_FRAME_STATE_ERROR;
+  state->unwound_source = DWFL_UNWOUND_INITIAL_FRAME;
   memset (state->regs_set, 0, sizeof (state->regs_set));
   thread->unwound = state;
   state->unwound = NULL;
@@ -248,6 +249,34 @@  dwfl_frame_thread (Dwfl_Frame *state)
 }
 INTDEF(dwfl_frame_thread)
 
+Dwfl_Unwound_Source
+dwfl_frame_unwound_source (Dwfl_Frame *state)
+{
+  return state->unwound_source;
+}
+INTDEF(dwfl_frame_unwound_source)
+
+const char *
+dwfl_unwound_source_str (Dwfl_Unwound_Source unwound_source)
+{
+  switch (unwound_source)
+    {
+    case DWFL_UNWOUND_NONE:
+      return "none";
+    case DWFL_UNWOUND_INITIAL_FRAME:
+      return "initial";
+    case DWFL_UNWOUND_EH_CFI:
+      return "eh_frame";
+    case DWFL_UNWOUND_DWARF_CFI:
+      return "dwarf";
+    case DWFL_UNWOUND_EBL:
+      return "ebl";
+    default:
+      return "unknown";
+    }
+}
+INTDEF(dwfl_unwound_source_str)
+
 int
 dwfl_getthreads (Dwfl *dwfl, int (*callback) (Dwfl_Thread *thread, void *arg),
 		 void *arg)
diff --git a/libdwfl/frame_unwind.c b/libdwfl/frame_unwind.c
index ab444d25..de65e09c 100644
--- a/libdwfl/frame_unwind.c
+++ b/libdwfl/frame_unwind.c
@@ -1,5 +1,5 @@ 
 /* Get previous frame state for an existing frame state.
-   Copyright (C) 2013, 2014, 2016 Red Hat, Inc.
+   Copyright (C) 2013, 2014, 2016, 2024 Red Hat, Inc.
    This file is part of elfutils.
 
    This file is free software; you can redistribute it and/or modify
@@ -515,6 +515,7 @@  new_unwound (Dwfl_Frame *state)
   unwound->signal_frame = false;
   unwound->initial_frame = false;
   unwound->pc_state = DWFL_FRAME_STATE_ERROR;
+  unwound->unwound_source = DWFL_UNWOUND_NONE;
   memset (unwound->regs_set, 0, sizeof (unwound->regs_set));
   return unwound;
 }
@@ -742,14 +743,20 @@  __libdwfl_frame_unwind (Dwfl_Frame *state)
 	{
 	  handle_cfi (state, pc - bias, cfi_eh, bias);
 	  if (state->unwound)
-	    return;
+	    {
+	      state->unwound->unwound_source = DWFL_UNWOUND_EH_CFI;
+	      return;
+	    }
 	}
       Dwarf_CFI *cfi_dwarf = INTUSE(dwfl_module_dwarf_cfi) (mod, &bias);
       if (cfi_dwarf)
 	{
 	  handle_cfi (state, pc - bias, cfi_dwarf, bias);
 	  if (state->unwound)
-	    return;
+	    {
+	      state->unwound->unwound_source = DWFL_UNWOUND_DWARF_CFI;
+	      return;
+	    }
 	}
     }
   assert (state->unwound == NULL);
@@ -774,6 +781,7 @@  __libdwfl_frame_unwind (Dwfl_Frame *state)
       // __libdwfl_seterrno has been called above.
       return;
     }
+  state->unwound->unwound_source = DWFL_UNWOUND_EBL;
   assert (state->unwound->pc_state == DWFL_FRAME_STATE_PC_SET);
   state->unwound->signal_frame = signal_frame;
 }
diff --git a/libdwfl/libdwfl.h b/libdwfl/libdwfl.h
index 4cbeab55..90523283 100644
--- a/libdwfl/libdwfl.h
+++ b/libdwfl/libdwfl.h
@@ -1,5 +1,5 @@ 
 /* Interfaces for libdwfl.
-   Copyright (C) 2005-2010, 2013 Red Hat, Inc.
+   Copyright (C) 2005-2010, 2013, 2024 Red Hat, Inc.
    This file is part of elfutils.
 
    This file is free software; you can redistribute it and/or modify
@@ -49,6 +49,19 @@  typedef struct Dwfl_Thread Dwfl_Thread;
    PC location described by an FDE belonging to Dwfl_Thread.  */
 typedef struct Dwfl_Frame Dwfl_Frame;
 
+/* This identifies the method used to unwind a particular Dwfl_Frame.
+   The enum order matches the order of preference for unwinding
+   (i.e. eh_frame is preferred to dwarf is preferred to ebl). */
+typedef enum {
+    DWFL_UNWOUND_NONE = 0,
+    DWFL_UNWOUND_INITIAL_FRAME,
+    DWFL_UNWOUND_EH_CFI,
+    DWFL_UNWOUND_DWARF_CFI,
+    DWFL_UNWOUND_EBL,
+    /* Keep this the last entry.  */
+    DWFL_UNWOUND_NUM,
+} Dwfl_Unwound_Source;
+
 /* Handle for debuginfod-client connection.  */
 #ifndef _ELFUTILS_DEBUGINFOD_CLIENT_TYPEDEF
 typedef struct debuginfod_client debuginfod_client;
@@ -748,6 +761,13 @@  pid_t dwfl_thread_tid (Dwfl_Thread *thread)
 Dwfl_Thread *dwfl_frame_thread (Dwfl_Frame *state)
   __nonnull_attribute__ (1);
 
+/* Return unwind method for frame STATE.  This function never fails.  */
+Dwfl_Unwound_Source dwfl_frame_unwound_source (Dwfl_Frame *state)
+  __nonnull_attribute__ (1);
+
+/* Return a string suitable for printing based on UNWOUND_SOURCE.  */
+const char *dwfl_unwound_source_str (Dwfl_Unwound_Source unwound_source);
+
 /* Called by Dwfl_Thread_Callbacks.set_initial_registers implementation.
    For every known continuous block of registers <FIRSTREG..FIRSTREG+NREGS)
    (inclusive..exclusive) set their content to REGS (array of NREGS items).
diff --git a/libdwfl/libdwflP.h b/libdwfl/libdwflP.h
index d0a5f056..6ec5c966 100644
--- a/libdwfl/libdwflP.h
+++ b/libdwfl/libdwflP.h
@@ -1,5 +1,5 @@ 
 /* Internal definitions for libdwfl.
-   Copyright (C) 2005-2015, 2018 Red Hat, Inc.
+   Copyright (C) 2005-2015, 2018, 2024 Red Hat, Inc.
    This file is part of elfutils.
 
    This file is free software; you can redistribute it and/or modify
@@ -272,6 +272,7 @@  struct Dwfl_Frame
        outermost frame.  */
     DWFL_FRAME_STATE_PC_UNDEFINED
   } pc_state;
+  Dwfl_Unwound_Source unwound_source;
   /* Either initialized from appropriate REGS element or on some archs
      initialized separately as the return address has no DWARF register.  */
   Dwarf_Addr pc;
@@ -795,6 +796,8 @@  INTDECL (dwfl_pid)
 INTDECL (dwfl_thread_dwfl)
 INTDECL (dwfl_thread_tid)
 INTDECL (dwfl_frame_thread)
+INTDECL (dwfl_frame_unwound_source)
+INTDECL (dwfl_unwound_source_str)
 INTDECL (dwfl_thread_state_registers)
 INTDECL (dwfl_thread_state_register_pc)
 INTDECL (dwfl_getthread_frames)
diff --git a/src/stacktrace.c b/src/stacktrace.c
index 3e13e26c..438cb1dd 100644
--- a/src/stacktrace.c
+++ b/src/stacktrace.c
@@ -642,6 +642,8 @@  typedef struct
   int max_frames; /* for diagnostic purposes */
   int total_samples; /* for diagnostic purposes */
   int lost_samples; /* for diagnostic purposes */
+  Dwfl_Unwound_Source last_unwound; /* track CFI source, for diagnostic purposes */
+  Dwfl_Unwound_Source worst_unwound; /* track CFI source, for diagnostic purposes */
 } dwfltab_ent;
 
 typedef struct
@@ -851,6 +853,7 @@  struct sysprof_unwind_info
 #ifdef DEBUG_MODULES
   Dwfl *last_dwfl; /* for diagnostic purposes */
 #endif
+  pid_t last_pid; /* for diagnostic purposes, to provide access to dwfltab */
   Dwarf_Addr *addrs; /* allocate blocks of UNWIND_ADDR_INCREMENT */
   void *outbuf;
 };
@@ -1139,9 +1142,24 @@  sysprof_unwind_frame_cb (Dwfl_Frame *state, void *arg)
     }
 #endif
 
-  if (show_frames)
-    fprintf(stderr, "* frame %d: pc_adjusted=%lx sp=%lx+(%lx)\n",
-	    sui->n_addrs, pc_adjusted, sui->last_base, sp - sui->last_base);
+  dwfltab_ent *dwfl_ent = dwfltab_find(sui->last_pid);
+  if (dwfl_ent != NULL)
+    {
+      Dwfl_Unwound_Source unwound_source = dwfl_frame_unwound_source(state);
+      if (unwound_source > dwfl_ent->worst_unwound)
+	dwfl_ent->worst_unwound = unwound_source;
+      dwfl_ent->last_unwound = unwound_source;
+      if (show_frames)
+	fprintf(stderr, "* frame %d: pc_adjusted=%lx sp=%lx+(%lx) [%s]\n",
+		sui->n_addrs, pc_adjusted, sui->last_base, sp - sui->last_base,
+		dwfl_unwound_source_str(unwound_source));
+    }
+  else
+    {
+      if (show_frames)
+	fprintf(stderr, N_("* frame %d: pc_adjusted=%lx sp=%lx+(%lx) [dwfl_ent not found]\n"),
+		sui->n_addrs, pc_adjusted, sui->last_base, sp - sui->last_base);
+    }
 
   if (sui->n_addrs > maxframes)
     {
@@ -1233,6 +1251,7 @@  sysprof_unwind_cb (SysprofCaptureFrame *frame, void *arg)
 #ifdef DEBUG_MODULES
   sui->last_dwfl = dwfl;
 #endif
+  sui->last_pid = frame->pid;
   int rc = dwfl_getthread_frames (dwfl, ev->tid, sysprof_unwind_frame_cb, sui);
   if (rc < 0)
     {
@@ -1538,10 +1557,14 @@  https://sourceware.org/cgit/elfutils/tree/README.eu-stacktrace?h=users/serhei/eu
 	      dwfltab_ent *t = default_table.table;
 	      if (!t[idx].used)
 		continue;
-	      fprintf(stderr, N_("%d %s -- max %d frames, received %d samples, lost %d samples (%.1f%%)\n"),
+	      /* XXX worst_unwound gives least preferred unwind method used for this process
+		 (i.e. eh_frame is preferred to dwarf is preferred to ebl) */
+	      fprintf(stderr, N_("%d %s -- max %d frames, received %d samples, lost %d samples (%.1f%%) (last %s, worst %s)\n"),
 		      t[idx].pid, t[idx].comm, t[idx].max_frames,
 		      t[idx].total_samples, t[idx].lost_samples,
-		      PERCENT(t[idx].lost_samples, t[idx].total_samples));
+		      PERCENT(t[idx].lost_samples, t[idx].total_samples),
+		      dwfl_unwound_source_str(t[idx].last_unwound),
+		      dwfl_unwound_source_str(t[idx].worst_unwound));
 	      total_samples += t[idx].total_samples;
 	      total_lost_samples += t[idx].lost_samples;
 	    }