Hi Serhei,
On Tue, Oct 15, 2024 at 11:28 AM Serhei Makarov <ser...@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/dwflP.h: Add INTDECL for dwfl_frame_unwound_source,
Should be libdwflP.h.
> 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 <ser...@serhei.io>
> ---
> libdw/libdw.map | 2 ++
> libdwfl/dwfl_frame.c | 31 ++++++++++++++++++++++++++++++-
> libdwfl/frame_unwind.c | 14 +++++++++++---
> libdwfl/libdwfl.h | 20 +++++++++++++++++++-
> libdwfl/libdwflP.h | 5 ++++-
> src/stacktrace.c | 31 ++++++++++++++++++++++++++-----
> 6 files changed, 92 insertions(+), 11 deletions(-)
>
> 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..ffd951db 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,17 @@ 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: */
> +typedef enum {
> + DWFL_UNWOUND_NONE = 0,
> + DWFL_UNWOUND_INITIAL_FRAME,
> + DWFL_UNWOUND_EH_CFI,
> + DWFL_UNWOUND_DWARF_CFI,
> + DWFL_UNWOUND_EBL,
> + DWFL_UNWOUND_UNKNOWN,
> + DWFL_UNWOUND_NUM,
> +} Dwfl_Unwound_Source;
It might be worth adding a "Keep this the last entry." comment right
above DWFL_UNWOUND_NUM. This is done for libelf.h enums.
Also is DWFL_UNWOUND_UNKNOWN needed? It isn't used anywhere.
> +
> /* Handle for debuginfod-client connection. */
> #ifndef _ELFUTILS_DEBUGINFOD_CLIENT_TYPEDEF
> typedef struct debuginfod_client debuginfod_client;
> @@ -748,6 +759,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 ebd914e5..369c8c2d 100644
> --- a/src/stacktrace.c
> +++ b/src/stacktrace.c
> @@ -640,6 +640,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
> @@ -849,6 +851,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;
> };
> @@ -1137,9 +1140,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;
What does worst_unwound mean? It looks like the enum values are ordered.
A comment that describes this ordering would be helpful.
> + 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)
> {
> @@ -1231,6 +1249,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)
> {
> @@ -1535,10 +1554,12 @@ Utility is a work-in-progress, see
> README.eu-stacktrace in the source branch.")
> 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"),
> + 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;
> }
> --
> 2.46.2
>
