From: Lancelot SIX <lancelot.six@amd.com>
Date: Tue, 19 Apr 2022 22:13:29 +0000 (+0100)
Subject: gdb/infrun: assert !step_over_info_valid_p in restart_threads
X-Git-Url: https://git.libre-soc.org/?a=commitdiff_plain;h=2b718529b99d2ca53552558ba9b3ff3f22663795;p=binutils-gdb.git

gdb/infrun: assert !step_over_info_valid_p in restart_threads

While working in gdb/infrun.c:restart_threads, I was wondering what are
the preconditions to call the function.  It seems to me that
!step_over_info_valid_p should be a precondition (i.e. if we are doing
an inline step over breakpoint, we do not want to resume non stepping
threads as one of them might miss the breakpoint which is temporally
disabled).

To convince myself that this is true, I have added an assertion to
enforce this, and got one regression in the testsuite:

    FAIL: gdb.base/step-over-syscall.exp: vfork: displaced=off: single step over vfork (GDB internal error)

This call to restart_threads originates from handle_vfork_done which
does not check if a step over is active when restarting other threads:

    if (target_is_non_stop_p ())
      {
        scoped_restore_current_thread restore_thread;

        insert_breakpoints ();
        restart_threads (event_thread, event_thread->inf);
        start_step_over ();
      }

In this patch, I propose to:
- Call start_step_over before restart_threads.  If a step over is already
  in progress (as it is the case in the failing testcase),
  start_step_over return immediately, and there is no point in restarting
  all threads just to stop them right away for a step over breakpoint.
- Only call restart_threads if no step over is in progress at this
  point.

In this patch, I also propose to keep the assertion in restart_threads
to help enforce this precondition, and state it explicitly.

I have also checked all other places which call restart_threads, and
they all seem to check that there is no step over currently active
before doing the call.

As for infrun-related things, I am never completely sure I did not miss
something.  So as usual, all feedback and thoughts are very welcome.

Tested on x86_64-linux-gnu.

Change-Id: If5f5f98ec4cf9aaeaabb5e3aa88ae6ffd70d4f37
---

diff --git a/gdb/infrun.c b/gdb/infrun.c
index 4e7ca803c79..e0a5bde037b 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -101,6 +101,8 @@ static void restart_threads (struct thread_info *event_thread,
 
 static bool start_step_over (void);
 
+static bool step_over_info_valid_p (void);
+
 /* 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;
@@ -1088,8 +1090,10 @@ handle_vfork_done (thread_info *event_thread)
       scoped_restore_current_thread restore_thread;
 
       insert_breakpoints ();
-      restart_threads (event_thread, event_thread->inf);
       start_step_over ();
+
+      if (!step_over_info_valid_p ())
+	restart_threads (event_thread, event_thread->inf);
     }
 }
 
@@ -5877,6 +5881,8 @@ restart_threads (struct thread_info *event_thread, inferior *inf)
 				 event_thread->ptid.to_string ().c_str (),
 				 inf != nullptr ? inf->num : -1);
 
+  gdb_assert (!step_over_info_valid_p ());
+
   /* In case the instruction just stepped spawned a new thread.  */
   update_thread_list ();