[PATCH 2/5] libdwfl: only use thread->unwound for initial frame

Omar Sandoval osandov@osandov.com
Mon Oct 7 09:05:00 GMT 2019


From: Omar Sandoval <osandov@fb.com>

thread->unwound is only used for set_initial_registers (via
dwfl_thread_state_registers, dwfl_thread_state_register_pc, and a
special case in core_set_initial_registers). At that point,
thread->unwound is always the initial frame, so there's no need to
update it as we unwind the stack. Let's set it to NULL after we do the
initial setup. This simplifies the next change.

Signed-off-by: Omar Sandoval <osandov@fb.com>
---
 libdwfl/ChangeLog    |  5 +++++
 libdwfl/dwfl_frame.c | 48 ++++++++++++++++++--------------------------
 libdwfl/libdwflP.h   |  3 +--
 3 files changed, 25 insertions(+), 31 deletions(-)

diff --git a/libdwfl/ChangeLog b/libdwfl/ChangeLog
index f4a3cad9..07a1e8df 100644
--- a/libdwfl/ChangeLog
+++ b/libdwfl/ChangeLog
@@ -3,6 +3,11 @@
 	* dwfl_frame.c (dwfl_getthreads): Get rid of unnecessary
 	thread_free_all_states calls.
 	(getthread): Ditto.
+	(state_free): Remove function.
+	(thread_free_all_states): Remove function.
+	(free_states): Add function.
+	(dwfl_thread_getframes): Don't update thread->unwound while unwinding.
+	* libdwflP.h (struct Dwfl_Thread): Update comment for unwound member.
 
 2019-08-12  Mark Wielaard  <mark@klomp.org>
 
diff --git a/libdwfl/dwfl_frame.c b/libdwfl/dwfl_frame.c
index 20bdbd9b..5bbf850e 100644
--- a/libdwfl/dwfl_frame.c
+++ b/libdwfl/dwfl_frame.c
@@ -71,19 +71,14 @@ state_fetch_pc (Dwfl_Frame *state)
 /* Do not call it on your own, to be used by thread_* functions only.  */
 
 static void
-state_free (Dwfl_Frame *state)
+free_states (Dwfl_Frame *state)
 {
-  Dwfl_Thread *thread = state->thread;
-  assert (thread->unwound == state);
-  thread->unwound = state->unwound;
-  free (state);
-}
-
-static void
-thread_free_all_states (Dwfl_Thread *thread)
-{
-  while (thread->unwound)
-    state_free (thread->unwound);
+  while (state)
+    {
+      Dwfl_Frame *next = state->unwound;
+      free(state);
+      state = next;
+    }
 }
 
 static Dwfl_Frame *
@@ -399,12 +394,6 @@ dwfl_thread_getframes (Dwfl_Thread *thread,
 		       int (*callback) (Dwfl_Frame *state, void *arg),
 		       void *arg)
 {
-  if (thread->unwound != NULL)
-    {
-      /* We had to be called from inside CALLBACK.  */
-      __libdwfl_seterrno (DWFL_E_ATTACH_STATE_CONFLICT);
-      return -1;
-    }
   Ebl *ebl = thread->process->ebl;
   if (ebl_frame_nregs (ebl) == 0)
     {
@@ -420,33 +409,34 @@ dwfl_thread_getframes (Dwfl_Thread *thread,
   if (! process->callbacks->set_initial_registers (thread,
 						   thread->callbacks_arg))
     {
-      thread_free_all_states (thread);
+      free_states (thread->unwound);
+      thread->unwound = NULL;
       return -1;
     }
-  if (! state_fetch_pc (thread->unwound))
+  Dwfl_Frame *state = thread->unwound;
+  thread->unwound = NULL;
+  if (! state_fetch_pc (state))
     {
       if (process->callbacks->thread_detach)
 	process->callbacks->thread_detach (thread, thread->callbacks_arg);
-      thread_free_all_states (thread);
+      free_states (state);
       return -1;
     }
-
-  Dwfl_Frame *state;
   do
     {
-      state = thread->unwound;
       int err = callback (state, arg);
       if (err != DWARF_CB_OK)
 	{
 	  if (process->callbacks->thread_detach)
 	    process->callbacks->thread_detach (thread, thread->callbacks_arg);
-	  thread_free_all_states (thread);
+	  free_states (state);
 	  return err;
 	}
       __libdwfl_frame_unwind (state);
+      Dwfl_Frame *next = state->unwound;
       /* The old frame is no longer needed.  */
-      state_free (thread->unwound);
-      state = thread->unwound;
+      free (state);
+      state = next;
     }
   while (state && state->pc_state == DWFL_FRAME_STATE_PC_SET);
 
@@ -455,12 +445,12 @@ dwfl_thread_getframes (Dwfl_Thread *thread,
     process->callbacks->thread_detach (thread, thread->callbacks_arg);
   if (state == NULL || state->pc_state == DWFL_FRAME_STATE_ERROR)
     {
-      thread_free_all_states (thread);
+      free_states (state);
       __libdwfl_seterrno (err);
       return -1;
     }
   assert (state->pc_state == DWFL_FRAME_STATE_PC_UNDEFINED);
-  thread_free_all_states (thread);
+  free_states (state);
   return 0;
 }
 INTDEF(dwfl_thread_getframes)
diff --git a/libdwfl/libdwflP.h b/libdwfl/libdwflP.h
index 941a8b66..6b2d4867 100644
--- a/libdwfl/libdwflP.h
+++ b/libdwfl/libdwflP.h
@@ -242,8 +242,7 @@ struct Dwfl_Thread
 {
   Dwfl_Process *process;
   pid_t tid;
-  /* The current frame being unwound.  Initially it is the bottom frame.
-     Later the processed frames get freed and this pointer is updated.  */
+  /* Bottom (innermost) frame while we're initializing, NULL afterwards.  */
   Dwfl_Frame *unwound;
   void *callbacks_arg;
 };
-- 
2.23.0



More information about the Elfutils-devel mailing list