[PATCH v9 4/6] gdb: change the internal representation of scheduler locking.

[Klaus Gerlicher <klaus.gerlicher@intel.com>]

From: Natalia Saiapova <natalia.saiapova@intel.com>

Introduce a new structure to manage different options of the scheduler
locking.  The options can coexist together and be set individually.
In the next patch

  gdb: refine commands to control scheduler locking.

we introduce the commands to control these options.  In this patch we do
not introduce new commands and keep the previous API.

New scheduler locking options are:
replay continue -- control continuing commands during replay mode.
replay step -- control stepping commands during replay mode.
continue -- control continuing commands during normal execution.
step -- control stepping commands during normal execution.

Internally they hold a bool value, when true the locking is enabled.

Mapping to the old settings

Old Settings      |  New settings
-----------------------------------
off               | all are false
                  |
replay            | continue = false, step = false,
                  | replay continue = true, replay step = true
                  |
step              | continue = false, step = true,
                  | replay continue = false, replay step = true
                  |
on                | all are true

Behaviour change in clear_proceed_status:

Previously, the convenience feature that stops replaying other threads
when we're not replaying the selected thread was gated on
"scheduler_mode == schedlock_replay", i.e. it only fired for the
"replay" shortcut.  It is now gated on schedlock_applies_to_opts on the
replay options, which also evaluates to true for "scheduler-locking
on", and for "scheduler-locking step" while stepping.  This means
threads that are mid-replay now get pulled out of replay under "on"
and "step" as well, not just under "replay".  This is intentional: any
setting that locks the scheduler for replay-mode commands should also
stop other threads from silently continuing to replay behind the
user's back.
---
 gdb/infrun.c | 161 ++++++++++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 146 insertions(+), 15 deletions(-)

diff --git a/gdb/infrun.c b/gdb/infrun.c
index cf072237f48..a501c8a1ebd 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -107,8 +107,12 @@ static bool start_step_over (void);
 
 static bool step_over_info_valid_p (void);
 
+struct schedlock_mode_options;
 static bool schedlock_applies (thread_info *tp);
-static bool schedlock_applies (bool step, bool record_will_replay);
+static bool schedlock_applies (bool step,
+			       bool record_will_replay,
+			       thread_info *tp = nullptr);
+static bool schedlock_applies_to_opts (const schedlock_mode_options &, bool step);
 
 static void handle_process_exited (struct execution_control_state *ecs);
 
@@ -2374,7 +2378,70 @@ infrun_thread_ptid_changed (process_stratum_target *target,
     inferior_ptid = new_ptid;
 }
 
-\f
+/* A single scheduler locking option, holding a name and boolean value.  */
+struct schedlock_option
+{
+  schedlock_option () = delete;
+  schedlock_option (std::string name, bool value)
+    : m_name (std::move (name)), m_value (value)
+  {}
+
+  DISABLE_COPY_AND_ASSIGN (schedlock_option);
+  schedlock_option (schedlock_option &&) = default;
+  schedlock_option &operator= (schedlock_option &&) = default;
+
+  operator bool () const { return m_value; }
+  const char *c_str () const { return m_value ? "on" : "off"; }
+
+  /* Set new value.  Return true if the value has changed.  */
+  bool set (bool new_value);
+
+private:
+  const std::string m_name;
+  bool m_value;
+};
+
+bool
+schedlock_option::set (bool new_value)
+{
+  if (m_value != new_value)
+    {
+      m_value = new_value;
+      return true;
+    }
+
+  return false;
+}
+
+/* Scheduler locking settings for a mode (replay or normal).  */
+struct schedlock_mode_options
+{
+  schedlock_mode_options () = delete;
+  schedlock_mode_options (schedlock_option cont, schedlock_option step)
+    : cont (std::move (cont)), step (std::move (step))
+  {}
+
+  DISABLE_COPY_AND_ASSIGN (schedlock_mode_options);
+  schedlock_mode_options (schedlock_mode_options &&) = default;
+  schedlock_mode_options &operator= (schedlock_mode_options &&) = default;
+
+  /* If true, the scheduler is locked during continuing.  */
+  schedlock_option cont;
+  /* If true, the scheduler is locked during stepping.  */
+  schedlock_option step;
+};
+
+/* All scheduler locking options for both normal execution and replay mode.  */
+struct schedlock_state
+{
+  schedlock_state (schedlock_mode_options normal_opts,
+		   schedlock_mode_options replay_opts)
+    : normal (std::move (normal_opts)), replay (std::move (replay_opts))
+  {}
+
+  schedlock_mode_options normal;
+  schedlock_mode_options replay;
+};
 
 static const char schedlock_off[] = "off";
 static const char schedlock_on[] = "on";
@@ -2387,7 +2454,43 @@ static const char *const scheduler_enums[] = {
   schedlock_replay,
   nullptr
 };
+
 static const char *scheduler_mode = schedlock_replay;
+
+/* Global scheduler locking state.  */
+static schedlock_state scheduler_locking_state {
+  {
+    {"cont", false},
+    {"step", false}
+  },
+  {
+    {"replay cont", true},
+    {"replay step", true}
+  }
+};
+
+/* A helper function to set scheduler locking shortcuts:
+   set scheduler-locking on: all options are on.
+   set scheduler-locking off: all options are off.
+   set scheduler-locking replay: only replay options are on.
+   set scheduler-locking step: only "step" and "replay step" are on.  */
+
+static void
+set_schedlock_shortcut_option (const char *shortcut)
+{
+  bool is_on = (shortcut == schedlock_on);
+  bool is_step = (shortcut == schedlock_step);
+  bool is_replay = (shortcut == schedlock_replay);
+  bool is_off = (shortcut == schedlock_off);
+  /* Check that we got a valid shortcut option.  */
+  gdb_assert (is_on || is_step || is_replay || is_off);
+
+  scheduler_locking_state.normal.cont.set (is_on);
+  scheduler_locking_state.normal.step.set (is_on || is_step);
+  scheduler_locking_state.replay.cont.set (is_on || is_replay);
+  scheduler_locking_state.replay.step.set (is_on || is_replay || is_step);
+}
+
 static void
 show_scheduler_mode (struct ui_file *file, int from_tty,
 		     struct cmd_list_element *c, const char *value)
@@ -2404,9 +2507,13 @@ set_schedlock_func (const char *args, int from_tty, struct cmd_list_element *c)
   if (!target_can_lock_scheduler ())
     {
       scheduler_mode = schedlock_off;
+      /* Set scheduler locking off.  */
+      set_schedlock_shortcut_option (schedlock_off);
       error (_("Target '%s' cannot support this command."),
 	     target_shortname ());
     }
+
+  set_schedlock_shortcut_option (scheduler_mode);
 }
 
 /* True if execution commands resume all threads of all processes by
@@ -2437,6 +2544,10 @@ ptid_t
 user_visible_resume_ptid (int step)
 {
   ptid_t resume_ptid;
+  thread_info *tp = nullptr;
+
+  if (inferior_ptid != null_ptid)
+    tp = inferior_thread ();
 
   if (non_stop)
     {
@@ -2446,14 +2557,14 @@ user_visible_resume_ptid (int step)
     }
   else if (schedlock_applies (step,
 			      target_record_will_replay (inferior_ptid,
-							 execution_direction)))
+							 execution_direction),
+			      tp))
     {
       /* User-settable 'scheduler' mode requires solo thread
 	 resume.  */
       resume_ptid = inferior_ptid;
     }
-  else if (inferior_ptid != null_ptid
-	   && inferior_thread ()->control.in_cond_eval)
+  else if (tp != nullptr && tp->control.in_cond_eval)
     {
       /* The inferior thread is evaluating a BP condition.  Other threads
 	 might be stopped or running and we do not want to change their
@@ -3159,14 +3270,22 @@ notify_about_to_proceed ()
 void
 clear_proceed_status (int step, bool about_to_proceed)
 {
-  /* With scheduler-locking replay, stop replaying other threads in the
-     same process if we're not replaying the selected thread.
+  /* When scheduler locking applies to replay mode, stop replaying other
+     threads in the same process if we're not replaying the selected thread.
 
      This is a convenience feature to not require the user to explicitly
      stop replaying the other threads.  We're assuming that the user's
-     intent is to resume tracing the recorded process.  */
-  if (!non_stop && scheduler_mode == schedlock_replay
-      && !target_record_will_replay (inferior_ptid, execution_direction))
+     intent is to resume tracing the recorded process.
+
+     This check uses schedlock_applies_to_opts on the replay mode options,
+     which means it applies not only for "set scheduler-locking replay",
+     but also for "set scheduler-locking on" and (when stepping)
+     "set scheduler-locking step".  This ensures that when any form of
+     scheduler locking is active that would affect replay mode, we stop
+     replaying threads that have finished their replay.  */
+  if (!non_stop && schedlock_applies_to_opts (scheduler_locking_state.replay, step)
+      && !target_record_will_replay (inferior_ptid,
+				     execution_direction))
     target_record_stop_replaying ();
 
   if (!non_stop && inferior_ptid != null_ptid)
@@ -3242,6 +3361,17 @@ thread_still_needs_step_over (struct thread_info *tp)
   return what;
 }
 
+/* Return true if OPTS lock the scheduler.
+   STEP indicates whether a thread is about to step.
+   This function does not take into account the mode (replay or
+   normal execution).  */
+
+static bool
+schedlock_applies_to_opts (const schedlock_mode_options &opts, bool step)
+{
+  return ((opts.cont && !step) || (opts.step && step));
+}
+
 /* Returns true if scheduler locking applies to non-NULL thread TP.  */
 
 static bool
@@ -3251,7 +3381,7 @@ schedlock_applies (thread_info *tp)
   bool step = tp->control.stepping_command;
   bool record_will_replay
     = target_record_will_replay (tp->ptid, execution_direction);
-  return schedlock_applies (step, record_will_replay);
+  return schedlock_applies (step, record_will_replay, tp);
 }
 
 /* Returns true if scheduler locking applies.  STEP indicates whether
@@ -3259,11 +3389,12 @@ schedlock_applies (thread_info *tp)
    indicates whether we're about to replay.  */
 
 static bool
-schedlock_applies (bool step, bool record_will_replay)
+schedlock_applies (bool step, bool record_will_replay, thread_info *tp)
 {
-  return (scheduler_mode == schedlock_on
-	  || (scheduler_mode == schedlock_step && step)
-	  || (scheduler_mode == schedlock_replay && record_will_replay));
+  schedlock_mode_options &opts
+    = record_will_replay ? scheduler_locking_state.replay
+			 : scheduler_locking_state.normal;
+  return schedlock_applies_to_opts (opts, step);
 }
 
 /* When FORCE_P is false, set process_stratum_target::COMMIT_RESUMED_STATE
-- 
2.34.1

________________________________________
Intel Deutschland GmbH 

Registered Address: Dornacher Strasse 1, 85622 Feldkirchen, Germany 

Tel: +49 (89) 99143-0 

www.intel.de 

Managing Directors: Candice Moore, Jeffrey Schneiderman, Ramachandran Sitaraman

Chairperson of the Supervisory Board: Sonja Pierer

Registered Seat: Munich Commercial Register B: Amtsgericht Munich HRB 186928

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.