In commit 3ce0d5ed, I missed the fact that
dwflst_perf_sample_getframes needs to handle the case of an unattached
Dwfl, when dwfl->process->ebl is not yet available to translate the
registers. Thus, it can't be a straightforward wrapper of
dwfl_sample_getframes, but should instead handle the attaching logic
identically to that function.

Also fix a leakage of sample_arg in dwflst_sample_getframes that was
happening on attach failure.

* libdwfl_stacktrace (dwflst_sample_getframes): Fix a leak of
  sample_arg on attach failure.
* libdwfl_stacktrace (dwflst_perf_sample_getframes): Implement
  attaching the Dwfl identically to dwflst_sample_getframes.
  Avoid leaking sample_arg on attach failure.

Signed-off-by: Serhei Makarov <[email protected]>
---
 libdwfl_stacktrace/dwflst_sample_frame.c | 57 +++++++++++++++++++-----
 1 file changed, 46 insertions(+), 11 deletions(-)

diff --git a/libdwfl_stacktrace/dwflst_sample_frame.c 
b/libdwfl_stacktrace/dwflst_sample_frame.c
index 090d3220..cf33a439 100644
--- a/libdwfl_stacktrace/dwflst_sample_frame.c
+++ b/libdwfl_stacktrace/dwflst_sample_frame.c
@@ -238,7 +238,10 @@ dwflst_sample_getframes (Dwfl *dwfl, Elf *elf,
   if (! attached
       && ! INTUSE(dwfl_attach_state) (dwfl, elf, pid,
                                      &sample_thread_callbacks, sample_arg))
-    return -1;
+    {
+      free(sample_arg);
+      return -1;
+    }
 
   Dwfl_Process *process = dwfl->process;
   Ebl *ebl = process->ebl;
@@ -259,26 +262,58 @@ dwflst_perf_sample_getframes (Dwfl *dwfl, Elf *elf,
                              int (*callback) (Dwfl_Frame *state, void *arg),
                              void *arg)
 {
+  /* TODO: Lock the dwfl to ensure attach_state does not interfere
+     with other dwfl_perf_sample_getframes calls. */
+
+  struct sample_info *sample_arg;
+  bool attached = false;
+  if (dwfl->process != NULL)
+    {
+      sample_arg = dwfl->process->callbacks_arg;
+      attached = true;
+    }
+  else
+    {
+      sample_arg = malloc (sizeof *sample_arg);
+      if (sample_arg == NULL)
+       {
+         __libdwfl_seterrno(DWFL_E_NOMEM);
+         return -1;
+       }
+    }
+
+  sample_arg->pid = pid;
+  sample_arg->tid = tid;
+  sample_arg->stack = (const uint8_t *)stack;
+  sample_arg->stack_size = stack_size;
+  sample_arg->regs = regs;
+  sample_arg->n_regs = n_regs;
+
+  if (! attached
+      && ! INTUSE(dwfl_attach_state) (dwfl, elf, pid,
+                                     &sample_thread_callbacks, sample_arg))
+    {
+      free(sample_arg);
+      return -1;
+    }
+
   /* Select the regs_mapping based on architecture.  This will be
      cached in ebl to avoid having to recompute the regs_mapping array
      when perf_regs_mask is consistent for the entire session: */
-  const int *regs_mapping;
-  size_t n_regs_mapping;
   Dwfl_Process *process = dwfl->process;
   Ebl *ebl = process->ebl;
-  /* XXX May want to check if abi matches ebl_get_elfclass(ebl). */
   if (!ebl_sample_perf_regs_mapping(ebl,
                                    perf_regs_mask, abi,
-                                   &regs_mapping, &n_regs_mapping))
+                                   &sample_arg->regs_mapping, 
&sample_arg->n_regs_mapping))
     {
       __libdwfl_seterrno(DWFL_E_LIBEBL_BAD);
       return -1;
     }
+  sample_arg->elfclass = ebl_get_elfclass(ebl);
+  ebl_sample_sp_pc(ebl, regs, n_regs,
+                   sample_arg->regs_mapping, sample_arg->n_regs_mapping,
+                   &sample_arg->base_addr, &sample_arg->pc);
 
-  /* Then we can call dwflst_sample_getframes: */
-  return dwflst_sample_getframes (dwfl, elf, pid, tid,
-                                 stack, stack_size,
-                                 regs, n_regs,
-                                 regs_mapping, n_regs_mapping,
-                                 callback, arg);
+  /* XXX May want to check if abi matches ebl_get_elfclass(ebl). */
+  return INTUSE(dwfl_getthread_frames) (dwfl, tid, callback, arg);
 }
-- 
2.52.0

Reply via email to