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 4e7ca803c798e512b3891eab6ed2fabd8acf9ac3..e0a5bde037b85192ef4d941101e822e8f26c0495 100644 (file)
--- 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 ();