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

Message ID 20231229104202.7878-5-natalia.saiapova@intel.com
State New
Headers
Series Refinement of scheduler-locking settings |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_gdb_build--master-aarch64 success Testing passed
linaro-tcwg-bot/tcwg_gdb_build--master-arm success Testing passed
linaro-tcwg-bot/tcwg_gdb_check--master-aarch64 success Testing passed
linaro-tcwg-bot/tcwg_gdb_check--master-arm success Testing passed

Commit Message

Natalia Saiapova Dec. 29, 2023, 10:42 a.m. UTC
  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: add 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 run -- control non-stepping commands during replay mode.
replay step -- control stepping commands during replay mode.
run -- control non-stepping commands during normal exection.
step -- control stepping commands during normal exection.

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            | run = false, step = false,
                  | replay run = true, replay step = true
                  |
step              | run = false, step = true,
                  | replay run = false, replay step = true
                  |
on                | all are true
---
 gdb/infrun.c | 122 ++++++++++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 115 insertions(+), 7 deletions(-)
  

Comments

Eli Zaretskii Dec. 29, 2023, 11:49 a.m. UTC | #1
> From: Natalia Saiapova <natalia.saiapova@intel.com>
> Cc: tankut.baris.aktemur@intel.com
> Date: Fri, 29 Dec 2023 10:42:00 +0000
> 
> 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: add 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 run -- control non-stepping commands during replay mode.
> replay step -- control stepping commands during replay mode.
> run -- control non-stepping commands during normal exection.
> step -- control stepping commands during normal exection.

To make this easier to remember, I suggest to use "non-step" instead
of "run", in both cases.
  

Patch

diff --git a/gdb/infrun.c b/gdb/infrun.c
index b8feef6bc2d..f70f615abf8 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -111,6 +111,11 @@  static bool schedlock_applies_to_thread (thread_info *tp);
 
 static bool schedlock_applies (bool step);
 
+struct schedlock_options;
+
+static bool schedlock_applies_to_opts (const schedlock_options &opts,
+				       bool step);
+
 /* Asynchronous signal handler registered as event loop source for
    when we have pending events ready to be passed to the core.  */
 static struct async_event_handler *infrun_async_inferior_event_token;
@@ -2322,7 +2327,69 @@  infrun_thread_ptid_changed (process_stratum_target *target,
     inferior_ptid = new_ptid;
 }
 
-
+/* A structure to hold scheduler locking settings for
+   a mode, replay or normal.  */
+struct schedlock_options
+{
+  struct option {
+    const std::string name;
+    bool value;
+
+    option () = delete;
+    option (std::string name, bool value)
+      : name (std::move (name)), value (value)
+    {}
+
+    /* Forbid accidential copying.  */
+    option (const option &) = delete;
+    option operator= (const option &) = delete;
+    option (option &&) = default;
+    option &operator= (option &&) = default;
+
+    operator bool () const { return value; }
+    const char *c_str () const { return value ? "on" : "off"; }
+    /* Set new value.  Return true, if the value has changed.  */
+    bool set (bool new_value);
+  };
+
+  schedlock_options () = delete;
+  schedlock_options (option run, option step)
+    :  run (std::move (run)), step (std::move (step))
+  {}
+
+  /* Forbid accidential copying.  */
+  schedlock_options (const schedlock_options &) = delete;
+  schedlock_options operator= (const schedlock_options &) = delete;
+  schedlock_options (schedlock_options &&) = default;
+  schedlock_options &operator= (schedlock_options &&) = default;
+
+  /* If true, the scheduler is locked during non-stepping.  */
+  option run;
+  /* If true, the scheduler is locked during stepping.  */
+  option step;
+};
+
+bool
+schedlock_options::option::set (bool new_value)
+{
+  if (value != new_value)
+    {
+      value = new_value;
+      return true;
+    }
+
+  return false;
+}
+
+struct schedlock
+{
+  schedlock (schedlock_options opt, schedlock_options replay_opt)
+    : normal (std::move (opt)), replay (std::move (replay_opt))
+  {}
+
+  schedlock_options normal;
+  schedlock_options replay;
+};
 
 static const char schedlock_off[] = "off";
 static const char schedlock_on[] = "on";
@@ -2335,7 +2402,34 @@  static const char *const scheduler_enums[] = {
   schedlock_replay,
   nullptr
 };
+
 static const char *scheduler_mode = schedlock_replay;
+
+schedlock schedlock {{{"run", false}, {"step", false}},
+		     {{"replay run", 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);
+
+  schedlock.normal.run.set (is_on);
+  schedlock.normal.step.set (is_on || is_step);
+  schedlock.replay.run.set (is_on || is_replay);
+  schedlock.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)
@@ -2352,9 +2446,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
@@ -3102,7 +3200,7 @@  clear_proceed_status (int step)
      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
+  if (!non_stop && schedlock_applies_to_opts (schedlock.replay, step)
       && target_record_is_replaying (minus_one_ptid)
       && !target_record_will_replay (user_visible_resume_ptid (step),
 				     execution_direction))
@@ -3179,6 +3277,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.
+   Note, this does not take into the account the mode (replay or
+   normal execution).  */
+
+static bool
+schedlock_applies_to_opts (const schedlock_options &opts, bool step)
+{
+  return ((opts.run && !step) || (opts.step && step));
+}
+
 /* Returns true if scheduler locking applies to TP.  */
 
 static bool
@@ -3194,11 +3303,10 @@  schedlock_applies_to_thread (thread_info *tp)
 static bool
 schedlock_applies (bool step)
 {
-  return (scheduler_mode == schedlock_on
-	  || (scheduler_mode == schedlock_step && step)
-	  || (scheduler_mode == schedlock_replay
-	      && target_record_will_replay (minus_one_ptid,
-					    execution_direction)));
+  bool is_replay = target_record_will_replay (minus_one_ptid,
+					      execution_direction);
+  schedlock_options &opts = is_replay ? schedlock.replay : schedlock.normal;
+  return schedlock_applies_to_opts (opts, step);
 }
 
 /* Set process_stratum_target::COMMIT_RESUMED_STATE in all target