Place displaced step data directly in inferior structure

This patch moves the per-inferior data related to displaced stepping to
be directly in the inferior structure, rather than in a container on the
side.

On notable difference is that previously, we deleted the state on
inferior exit, which guaranteed a clean state if re-using the inferior
for a new run or attach.  We now need to reset the state manually.

At the same time, I changed step_saved_copy to be a gdb::byte_vector, so
it is automatically freed on destruction (which should plug the leak
reported here [1]).

[1] https://sourceware.org/ml/gdb-patches/2018-11/msg00202.html

gdb/ChangeLog:

	* inferior.h (class inferior) <displaced_step_state>: New field.
	* infrun.h (struct displaced_step_state): Move here from
	infrun.c.  Initialize fields, add constructor.
	<inf>: Remove field.
	<reset>: New method.
	* infrun.c (struct displaced_step_inferior_state): Move to
	infrun.h.
	(displaced_step_inferior_states): Remove.
	(get_displaced_stepping_state): Adust.
	(displaced_step_in_progress_any_inferior): Adjust.
	(displaced_step_in_progress_thread): Adjust.
	(displaced_step_in_progress): Adjust.
	(add_displaced_stepping_state): Remove.
	(get_displaced_step_closure_by_addr): Adjust.
	(remove_displaced_stepping_state): Remove.
	(infrun_inferior_exit): Call displaced_step_state.reset.
	(use_displaced_stepping): Don't check for NULL.
	(displaced_step_prepare_throw): Call
	get_displaced_stepping_state.
	(displaced_step_fixup): Don't check for NULL.
	(prepare_for_detach): Don't check for NULL.

4 files changed, 91 insertions, 122 deletions

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 41889ba92ec..c3846188304 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,27 @@
+2019-01-02  Simon Marchi  <simon.marchi@ericsson.com>
+
+	* inferior.h (class inferior) <displaced_step_state>: New field.
+	* infrun.h (struct displaced_step_state): Move here from
+	infrun.c.  Initialize fields, add constructor.
+	<inf>: Remove field.
+	<reset>: New method.
+	* infrun.c (struct displaced_step_inferior_state): Move to
+	infrun.h.
+	(displaced_step_inferior_states): Remove.
+	(get_displaced_stepping_state): Adust.
+	(displaced_step_in_progress_any_inferior): Adjust.
+	(displaced_step_in_progress_thread): Adjust.
+	(displaced_step_in_progress): Adjust.
+	(add_displaced_stepping_state): Remove.
+	(get_displaced_step_closure_by_addr): Adjust.
+	(remove_displaced_stepping_state): Remove.
+	(infrun_inferior_exit): Call displaced_step_state.reset.
+	(use_displaced_stepping): Don't check for NULL.
+	(displaced_step_prepare_throw): Call
+	get_displaced_stepping_state.
+	(displaced_step_fixup): Don't check for NULL.
+	(prepare_for_detach): Don't check for NULL.
+
 2019-01-02  Philippe Waroquiers  <philippe.waroquiers@skynet.be>
 
 	* infcall.c (call_function_by_hand_dummy): cleanup/destroy sm
diff --git a/gdb/inferior.h b/gdb/inferior.h
index 276c442746a..a82df1a52a5 100644
--- a/gdb/inferior.h
+++ b/gdb/inferior.h
@@ -503,6 +503,9 @@ public:
      this gdbarch.  */
   struct gdbarch *gdbarch = NULL;
 
+  /* Data related to displaced stepping.  */
+  displaced_step_inferior_state displaced_step_state;
+
   /* Per inferior data-pointers required by other GDB modules.  */
   REGISTRY_FIELDS;
 };
diff --git a/gdb/infrun.c b/gdb/infrun.c
index 00eefabf6e5..150288264f4 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -1476,53 +1476,12 @@ step_over_info_valid_p (void)
 
 displaced_step_closure::~displaced_step_closure () = default;
 
-/* Per-inferior displaced stepping state.  */
-struct displaced_step_inferior_state
-{
-  /* The process this displaced step state refers to.  */
-  inferior *inf;
-
-  /* True if preparing a displaced step ever failed.  If so, we won't
-     try displaced stepping for this inferior again.  */
-  int failed_before;
-
-  /* If this is not nullptr, this is the thread carrying out a
-     displaced single-step in process PID.  This thread's state will
-     require fixing up once it has completed its step.  */
-  thread_info *step_thread;
-
-  /* The architecture the thread had when we stepped it.  */
-  struct gdbarch *step_gdbarch;
-
-  /* The closure provided gdbarch_displaced_step_copy_insn, to be used
-     for post-step cleanup.  */
-  struct displaced_step_closure *step_closure;
-
-  /* The address of the original instruction, and the copy we
-     made.  */
-  CORE_ADDR step_original, step_copy;
-
-  /* Saved contents of copy area.  */
-  gdb_byte *step_saved_copy;
-};
-
-/* The list of states of processes involved in displaced stepping
-   presently.  */
-static std::forward_list<displaced_step_inferior_state *>
-  displaced_step_inferior_states;
-
 /* Get the displaced stepping state of process PID.  */
 
 static displaced_step_inferior_state *
 get_displaced_stepping_state (inferior *inf)
 {
-  for (auto *state : displaced_step_inferior_states)
-    {
-      if (state->inf == inf)
-	return state;
-    }
-
-  return nullptr;
+  return &inf->displaced_step_state;
 }
 
 /* Returns true if any inferior has a thread doing a displaced
@@ -1531,9 +1490,9 @@ get_displaced_stepping_state (inferior *inf)
 static bool
 displaced_step_in_progress_any_inferior ()
 {
-  for (auto *state : displaced_step_inferior_states)
+  for (inferior *i : all_inferiors ())
     {
-      if (state->step_thread != nullptr)
+      if (i->displaced_step_state.step_thread != nullptr)
 	return true;
     }
 
@@ -1546,13 +1505,9 @@ displaced_step_in_progress_any_inferior ()
 static int
 displaced_step_in_progress_thread (thread_info *thread)
 {
-  struct displaced_step_inferior_state *displaced;
-
   gdb_assert (thread != NULL);
 
-  displaced = get_displaced_stepping_state (thread->inf);
-
-  return (displaced != NULL && displaced->step_thread == thread);
+  return get_displaced_stepping_state (thread->inf)->step_thread == thread;
 }
 
 /* Return true if process PID has a thread doing a displaced step.  */
@@ -1560,34 +1515,7 @@ displaced_step_in_progress_thread (thread_info *thread)
 static int
 displaced_step_in_progress (inferior *inf)
 {
-  struct displaced_step_inferior_state *displaced;
-
-  displaced = get_displaced_stepping_state (inf);
-  if (displaced != NULL && displaced->step_thread != nullptr)
-    return 1;
-
-  return 0;
-}
-
-/* Add a new displaced stepping state for process PID to the displaced
-   stepping state list, or return a pointer to an already existing
-   entry, if it already exists.  Never returns NULL.  */
-
-static displaced_step_inferior_state *
-add_displaced_stepping_state (inferior *inf)
-{
-  displaced_step_inferior_state *state
-    = get_displaced_stepping_state (inf);
-
-  if (state != nullptr)
-    return state;
-
-  state = XCNEW (struct displaced_step_inferior_state);
-  state->inf = inf;
-
-  displaced_step_inferior_states.push_front (state);
-
-  return state;
+  return get_displaced_stepping_state (inf)->step_thread != nullptr;
 }
 
 /* If inferior is in displaced stepping, and ADDR equals to starting address
@@ -1597,42 +1525,21 @@ add_displaced_stepping_state (inferior *inf)
 struct displaced_step_closure*
 get_displaced_step_closure_by_addr (CORE_ADDR addr)
 {
-  struct displaced_step_inferior_state *displaced
+  displaced_step_inferior_state *displaced
     = get_displaced_stepping_state (current_inferior ());
 
   /* If checking the mode of displaced instruction in copy area.  */
-  if (displaced != NULL
-      && displaced->step_thread != nullptr
+  if (displaced->step_thread != nullptr
       && displaced->step_copy == addr)
     return displaced->step_closure;
 
   return NULL;
 }
 
-/* Remove the displaced stepping state of process PID.  */
-
-static void
-remove_displaced_stepping_state (inferior *inf)
-{
-  gdb_assert (inf != nullptr);
-
-  displaced_step_inferior_states.remove_if
-    ([inf] (displaced_step_inferior_state *state)
-      {
-	if (state->inf == inf)
-	  {
-	    xfree (state);
-	    return true;
-	  }
-	else
-	  return false;
-      });
-}
-
 static void
 infrun_inferior_exit (struct inferior *inf)
 {
-  remove_displaced_stepping_state (inf);
+  inf->displaced_step_state.reset ();
 }
 
 /* If ON, and the architecture supports it, GDB will use displaced
@@ -1669,17 +1576,15 @@ use_displaced_stepping (struct thread_info *tp)
 {
   struct regcache *regcache = get_thread_regcache (tp);
   struct gdbarch *gdbarch = regcache->arch ();
-  struct displaced_step_inferior_state *displaced_state;
-
-  displaced_state = get_displaced_stepping_state (tp->inf);
+  displaced_step_inferior_state *displaced_state
+    = get_displaced_stepping_state (tp->inf);
 
   return (((can_use_displaced_stepping == AUTO_BOOLEAN_AUTO
 	    && target_is_non_stop_p ())
 	   || can_use_displaced_stepping == AUTO_BOOLEAN_TRUE)
 	  && gdbarch_displaced_step_copy_insn_p (gdbarch)
 	  && find_record_target () == NULL
-	  && (displaced_state == NULL
-	      || !displaced_state->failed_before));
+	  && !displaced_state->failed_before);
 }
 
 /* Clean out any stray displaced stepping state.  */
@@ -1734,14 +1639,12 @@ displaced_step_dump_bytes (struct ui_file *file,
 static int
 displaced_step_prepare_throw (thread_info *tp)
 {
-  struct cleanup *ignore_cleanups;
   regcache *regcache = get_thread_regcache (tp);
   struct gdbarch *gdbarch = regcache->arch ();
   const address_space *aspace = regcache->aspace ();
   CORE_ADDR original, copy;
   ULONGEST len;
   struct displaced_step_closure *closure;
-  struct displaced_step_inferior_state *displaced;
   int status;
 
   /* We should never reach this function if the architecture does not
@@ -1760,7 +1663,8 @@ displaced_step_prepare_throw (thread_info *tp)
   /* We have to displaced step one thread at a time, as we only have
      access to a single scratch space per inferior.  */
 
-  displaced = add_displaced_stepping_state (tp->inf);
+  displaced_step_inferior_state *displaced
+    = get_displaced_stepping_state (tp->inf);
 
   if (displaced->step_thread != nullptr)
     {
@@ -1816,10 +1720,8 @@ displaced_step_prepare_throw (thread_info *tp)
     }
 
   /* Save the original contents of the copy area.  */
-  displaced->step_saved_copy = (gdb_byte *) xmalloc (len);
-  ignore_cleanups = make_cleanup (free_current_contents,
-				  &displaced->step_saved_copy);
-  status = target_read_memory (copy, displaced->step_saved_copy, len);
+  displaced->step_saved_copy.resize (len);
+  status = target_read_memory (copy, displaced->step_saved_copy.data (), len);
   if (status != 0)
     throw_error (MEMORY_ERROR,
 		 _("Error accessing memory address %s (%s) for "
@@ -1830,7 +1732,7 @@ displaced_step_prepare_throw (thread_info *tp)
       fprintf_unfiltered (gdb_stdlog, "displaced: saved %s: ",
 			  paddress (gdbarch, copy));
       displaced_step_dump_bytes (gdb_stdlog,
-				 displaced->step_saved_copy,
+				 displaced->step_saved_copy.data (),
 				 len);
     };
 
@@ -1841,7 +1743,6 @@ displaced_step_prepare_throw (thread_info *tp)
       /* The architecture doesn't know how or want to displaced step
 	 this instruction or instruction sequence.  Fallback to
 	 stepping over the breakpoint in-line.  */
-      do_cleanups (ignore_cleanups);
       return -1;
     }
 
@@ -1853,7 +1754,8 @@ displaced_step_prepare_throw (thread_info *tp)
   displaced->step_original = original;
   displaced->step_copy = copy;
 
-  make_cleanup (displaced_step_clear_cleanup, displaced);
+  cleanup *ignore_cleanups
+    = make_cleanup (displaced_step_clear_cleanup, displaced);
 
   /* Resume execution at the copy.  */
   regcache_write_pc (regcache, copy);
@@ -1931,7 +1833,7 @@ displaced_step_restore (struct displaced_step_inferior_state *displaced,
   ULONGEST len = gdbarch_max_insn_length (displaced->step_gdbarch);
 
   write_memory_ptid (ptid, displaced->step_copy,
-		     displaced->step_saved_copy, len);
+		     displaced->step_saved_copy.data (), len);
   if (debug_displaced)
     fprintf_unfiltered (gdb_stdlog, "displaced: restored %s %s\n",
 			target_pid_to_str (ptid),
@@ -1953,10 +1855,6 @@ displaced_step_fixup (thread_info *event_thread, enum gdb_signal signal)
     = get_displaced_stepping_state (event_thread->inf);
   int ret;
 
-  /* Was any thread of this process doing a displaced step?  */
-  if (displaced == NULL)
-    return 0;
-
   /* Was this event for the thread we displaced?  */
   if (displaced->step_thread != event_thread)
     return 0;
@@ -3577,7 +3475,7 @@ prepare_for_detach (void)
 
   /* Is any thread of this process displaced stepping?  If not,
      there's nothing else to do.  */
-  if (displaced == NULL || displaced->step_thread == nullptr)
+  if (displaced->step_thread == nullptr)
     return;
 
   if (debug_infrun)
diff --git a/gdb/infrun.h b/gdb/infrun.h
index 0d1309b1007..8f61b75c15b 100644
--- a/gdb/infrun.h
+++ b/gdb/infrun.h
@@ -258,4 +258,48 @@ struct buf_displaced_step_closure : displaced_step_closure
   gdb::byte_vector buf;
 };
 
+/* Per-inferior displaced stepping state.  */
+struct displaced_step_inferior_state
+{
+  displaced_step_inferior_state ()
+  {
+    reset ();
+  }
+
+  /* Put this object back in its original state.  */
+  void reset ()
+  {
+    failed_before = 0;
+    step_thread = nullptr;
+    step_gdbarch = nullptr;
+    step_closure = nullptr;
+    step_original = 0;
+    step_copy = 0;
+    step_saved_copy.clear ();
+  }
+
+  /* True if preparing a displaced step ever failed.  If so, we won't
+     try displaced stepping for this inferior again.  */
+  int failed_before;
+
+  /* If this is not nullptr, this is the thread carrying out a
+     displaced single-step in process PID.  This thread's state will
+     require fixing up once it has completed its step.  */
+  thread_info *step_thread;
+
+  /* The architecture the thread had when we stepped it.  */
+  gdbarch *step_gdbarch;
+
+  /* The closure provided gdbarch_displaced_step_copy_insn, to be used
+     for post-step cleanup.  */
+  displaced_step_closure *step_closure;
+
+  /* The address of the original instruction, and the copy we
+     made.  */
+  CORE_ADDR step_original, step_copy;
+
+  /* Saved contents of copy area.  */
+  gdb::byte_vector step_saved_copy;
+};
+
 #endif /* INFRUN_H */