From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: by sourceware.org (Postfix, from userid 7833) id B24273858C2C; Mon, 25 Apr 2022 08:15:29 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org B24273858C2C Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable From: Lancelot SIX To: gdb-cvs@sourceware.org Subject: [binutils-gdb] gdb/infrun: assert !step_over_info_valid_p in restart_threads X-Act-Checkin: binutils-gdb X-Git-Author: Lancelot SIX X-Git-Refname: refs/heads/master X-Git-Oldrev: 455fe767086af57738678e9c02d0af068c2700aa X-Git-Newrev: 2b718529b99d2ca53552558ba9b3ff3f22663795 Message-Id: <20220425081529.B24273858C2C@sourceware.org> Date: Mon, 25 Apr 2022 08:15:29 +0000 (GMT) X-BeenThere: gdb-cvs@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gdb-cvs mailing list List-Unsubscribe: , List-Archive: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 25 Apr 2022 08:15:29 -0000 https://sourceware.org/git/gitweb.cgi?p=3Dbinutils-gdb.git;h=3D2b718529b99d= 2ca53552558ba9b3ff3f22663795 commit 2b718529b99d2ca53552558ba9b3ff3f22663795 Author: Lancelot SIX Date: Tue Apr 19 23:13:29 2022 +0100 gdb/infrun: assert !step_over_info_valid_p in restart_threads =20 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). =20 To convince myself that this is true, I have added an assertion to enforce this, and got one regression in the testsuite: =20 FAIL: gdb.base/step-over-syscall.exp: vfork: displaced=3Doff: singl= e step over vfork (GDB internal error) =20 This call to restart_threads originates from handle_vfork_done which does not check if a step over is active when restarting other threads: =20 if (target_is_non_stop_p ()) { scoped_restore_current_thread restore_thread; =20 insert_breakpoints (); restart_threads (event_thread, event_thread->inf); start_step_over (); } =20 In this patch, I=E2=80=AFpropose to: - Call start_step_over before restart_threads. If a step over is alrea= dy in progress (as it is the case in the failing testcase), start_step_over return immediately, and there is no point in restarti= ng 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. =20 In this patch, I also propose to keep the assertion in restart_threads to help enforce this precondition, and state it explicitly. =20 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. =20 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. =20 Tested on x86_64-linux-gnu. =20 Change-Id: If5f5f98ec4cf9aaeaabb5e3aa88ae6ffd70d4f37 Diff: --- gdb/infrun.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) 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, =20 static bool start_step_over (void); =20 +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; =20 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); } } =20 @@ -5877,6 +5881,8 @@ restart_threads (struct thread_info *event_thread, in= ferior *inf) event_thread->ptid.to_string ().c_str (), inf !=3D nullptr ? inf->num : -1); =20 + gdb_assert (!step_over_info_valid_p ()); + /* In case the instruction just stepped spawned a new thread. */ update_thread_list ();